Migrate webhooks.go to use SDK WebhooksService - #58
Conversation
Replace direct API calls with SDK WebhooksService methods: - List, Get, Create, Update, Delete webhooks via SDK - Use basecamp.Webhook type and request structs - Apply consistent error handling with convertSDKError
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f309908276
ℹ️ 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".
| } else { | ||
| // Default to all types if not specified | ||
| typeArray = []string{"Todo", "Todolist", "Message", "Comment", "Document", "Upload", "Vault", "Schedule::Entry", "Kanban::Card", "Question", "Question::Answer"} | ||
| } |
There was a problem hiding this comment.
If the API’s default behavior for a missing types field is “all supported types,” hard-coding a fixed list changes semantics: bcq webhooks create without --types will silently omit any event types the server adds later (or any account-specific types), so those events never fire. This regression happens whenever Basecamp introduces new webhook types beyond this list. Consider leaving Types unset when the flag is omitted so the server can supply its current default set.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in PR #73 - removed hard-coded default types. Now when --types is not specified, we leave Types nil so the server uses its default set.
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 WebhooksService methods: - List, Get, Create, Update, Delete webhooks via SDK - Use basecamp.Webhook type and request structs - Apply consistent error handling with convertSDKError
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)
Summary
Test plan
make checkpasses (vet, lint, tests)