Skip to content

fix(notification): support live notification variants - #485

Open
jacobo-doist wants to merge 2 commits into
mainfrom
jacobo/fix-notifications-list
Open

fix(notification): support live notification variants#485
jacobo-doist wants to merge 2 commits into
mainfrom
jacobo/fix-notifications-list

Conversation

@jacobo-doist

Copy link
Copy Markdown
Contributor

Summary

  • update @doist/todoist-sdk to 14.0.2 for its live-notification schema fixes
  • map validated camel-cased SDK fields into CLI notifications
  • handle notifications that provide fromUser without fromUid
  • add API-boundary coverage for sender, project, task, invitation, deletion, and sorting fields

Testing

  • npm run build
  • npm test
  • npm run type-check
  • npm run check
  • npm run check:skill-sync
  • live notification list smoke test using the local build

@doistbot
doistbot requested a review from gnapse August 19, 2026 09:17

@doistbot doistbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR updates the notification parsing to use the SDK 14.0.2 camelCase live-notification schema, adds a fromUser/fromUid fallback, and introduces API-boundary tests covering sender, project, task, invitation, deletion, and sorting paths.

No inline issues were flagged.

I also included a few optional follow-up notes in the details below.

Optional follow-up notes (2)
  • P3 src/lib/api/notifications.test.ts:19: This hand-rolls the API mock setup that setupApiMock() already provides. setupApiMock() (from src/test-support/api-mock.ts) creates the mock API and wires getApi() to it, and CODEBASE.md documents it as the standard test pattern. Keep the hoisted vi.mock('./core.js', ...) but replace the createMockApi() import, mockGetApi, and the mockApi = createMockApi(); mockGetApi.mockResolvedValue(mockApi) lines with mockApi = setupApiMock().
  • P3 src/lib/api/notifications.test.ts:63: toEqual ignores object keys whose value is undefined, so task: undefined, invitationId: undefined, and invitationSecret: undefined in the expected objects assert nothing. Since this test is meant to pin the API-boundary shape, use toStrictEqual (which checks the exact keys, including undefined-valued ones) or drop those no-op keys.

Share FeedbackReview Logs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants