Repository navigation
refactor: delete filters through the REST endpoint - #614
Merged
Merged
Conversation
`td filter delete` used the Sync `filter_delete` command. Switch to
`deleteFilter()` from @doist/todoist-sdk 15.8.0, which calls the REST
`DELETE /api/v1/filters/{id}` endpoint, matching how the other delete
commands work.
scottlovegrove
added
the
👀 Show
doistbot
reviewed
Oct 23, 2026
doistbot
left a comment
doistbot
left a comment
Member
There was a problem hiding this comment.
td filter delete now calls the SDK's REST deleteFilter() (bumped to 15.8.0) instead of the Sync filter_delete command, keeping filter deletion consistent with the other delete commands and the MCP server, with read-only mode still blocking it via the default-deny allowlist.
Few things worth tightening:
- Add a colocated
src/lib/api/filters.test.tsthat verifiesdeleteFilteris called with the resolved ID throughsetupApiMock()(and that no Syncfilter_deletecommand is sent) — the current command tests mock the wrapper, so a regression back to the sync path would slip through. - Stub
deleteFilter: vi.fn()increateMockApifirst, since the shared mock doesn't include the new SDK method.
I also included a few optional follow-up notes in the details below.
Optional follow-up notes (2)
src/lib/api/filters.ts:87:
createMockApi()insrc/test-support/mock-api.tsdoesn't stubdeleteFilter, so the standard test mock cannot exercise this new code path (api.deleteFilter is not a function). AdddeleteFilter: vi.fn()to the mock; the repo's add-command checklist calls for a barevi.fn()stub for each mutation method.deleteFilteris missing fromAPI_SPINNER_MESSAGESinsrc/lib/api/core.ts. The previousapi.syncpath showed the 'Syncing...' spinner, but this REST call now runs without one, unlikedeleteTask,deleteLabel, anddeleteSection. AdddeleteFilter: { text: 'Deleting filter...', color: 'yellow' }for consistency.
Add a test that `deleteFilter` calls the SDK's REST `deleteFilter()` and sends no Sync command, stub `deleteFilter` in the shared API mock, and give it a "Deleting filter..." spinner like the other deletes.
scottlovegrove
added
🚢 Ship
Contributor
|
🎉 This PR is included in version 5.8.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
td filter deletenow usesdeleteFilter()from@doist/todoist-sdk15.8.0 (bumped from 15.3.2), which calls the new RESTDELETE /api/v1/filters/{id}endpoint, instead of the Syncfilter_deletecommand. No behaviour change for users: the CLI's OAuth login already requestsdata:delete, so it never hit the 403 that the hosted MCP server did (Doist/Issues#21370). This keeps filter deletion consistent with the other delete commands and with the MCP server (Doist/todoist-mcp#634).deleteFilterisn't in the read-only allowlist, so read-only mode still blocks it. Type check, lint/format and the full test suite (2252 passing) pass.