Migrate recordings.go to use SDK RecordingsService - #68
Conversation
121e517 to
6194a82
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 121e517b77
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case "active": | ||
| err = app.SDK.Recordings().Unarchive(cmd.Context(), bucketID, recordingID) |
There was a problem hiding this comment.
The CLI help says recordings restore should bring back items from trash or archive, but the active branch only calls Recordings().Unarchive. That only targets archived recordings; a trashed recording won’t be restored (it will likely 404 or stay trashed), so bcq recordings restore <id> fails for trashed items. Consider calling the SDK method that maps to the status/active/restore endpoint (or adding a trash-restore path) so trashed recordings are handled too.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Verified that SDK Unarchive() calls PUT /status/active.json which works for BOTH archived AND trashed recordings per the Basecamp API. Added clarifying comment in PR #73.
Replace direct API calls with SDK client methods: - List uses app.SDK.Recordings().List() - Trash/Archive/Restore use respective SDK methods - Visibility uses app.SDK.Recordings().SetClientVisibility() Remove local Recording type in favor of basecamp.Recording.
6194a82 to
ada8af5
Compare
Fixes for Codex review comments: 1. webhooks.go: Let server use default webhook types instead of hard-coding (PR #58 feedback) 2. templates.go: Use raw API for archived/trashed status since SDK List() only returns active templates (PR #60 feedback) 3. campfire.go: Validate ParseInt errors before DeleteLine to avoid silent 0 IDs (PR #64 feedback) 4. recordings.go: Add comment clarifying that Unarchive() works for both archived AND trashed recordings via status/active endpoint (PR #68) 5. files.go: Preserve SDK errors in auto-detect mode instead of masking auth/permission errors as "not found" (PR #70 feedback)
Fixes for Codex review comments: 1. webhooks.go: Let server use default webhook types instead of hard-coding (PR #58 feedback) 2. templates.go: Use raw API for archived/trashed status since SDK List() only returns active templates (PR #60 feedback) 3. campfire.go: Validate ParseInt errors before DeleteLine to avoid silent 0 IDs (PR #64 feedback) 4. recordings.go: Add comment clarifying that Unarchive() works for both archived AND trashed recordings via status/active endpoint (PR #68) 5. files.go: Preserve SDK errors in auto-detect mode instead of masking auth/permission errors as "not found" (PR #70 feedback)
Replace direct API calls with SDK client methods: - List uses app.SDK.Recordings().List() - Trash/Archive/Restore use respective SDK methods - Visibility uses app.SDK.Recordings().SetClientVisibility() Remove local Recording type in favor of basecamp.Recording.
Fixes for Codex review comments: 1. webhooks.go: Let server use default webhook types instead of hard-coding (PR #58 feedback) 2. templates.go: Use raw API for archived/trashed status since SDK List() only returns active templates (PR #60 feedback) 3. campfire.go: Validate ParseInt errors before DeleteLine to avoid silent 0 IDs (PR #64 feedback) 4. recordings.go: Add comment clarifying that Unarchive() works for both archived AND trashed recordings via status/active endpoint (PR #68) 5. files.go: Preserve SDK errors in auto-detect mode instead of masking auth/permission errors as "not found" (PR #70 feedback)
Replace direct API calls with SDK client methods:
Remove local Recording type in favor of basecamp.Recording.