Skip to content

Commit 726dc1c

Browse files
gnapsegnapse
committed
fix(view-options): address review feedback
1 parent 9b2222d commit 726dc1c

8 files changed

Lines changed: 112 additions & 43 deletions

File tree

‎src/clients/view-options-client.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { performSyncRequest } from '../transport/sync-request'
22
import type { ViewOptions, ViewOptionsDeleteArgs, ViewOptionsSetArgs } from '../types/sync'
33
import { createCommand } from '../utils/sync-helpers'
4+
import { isActiveViewOption } from '../utils/view-options'
45
import { BaseClient } from './base-client'
56

67
/** Internal sub-client for saved view-option reads and writes. */
@@ -11,7 +12,7 @@ export class ViewOptionsClient extends BaseClient {
1112
syncToken: '*',
1213
})
1314

14-
return (response.viewOptions ?? []).filter((options) => options.isDeleted !== true)
15+
return (response.viewOptions ?? []).filter(isActiveViewOption)
1516
}
1617

1718
async setViewOptions(args: ViewOptionsSetArgs, requestId?: string): Promise<void> {

‎src/todoist-api.ts‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -358,17 +358,33 @@ export class TodoistApi {
358358
)
359359
}
360360

361-
/** Retrieves the authenticated user's active saved view options. */
361+
/**
362+
* Retrieves the authenticated user's active saved view options.
363+
*
364+
* @returns A promise that resolves to the active saved view options.
365+
*/
362366
async getViewOptions(): Promise<ViewOptions[]> {
363367
return this.viewOptionsClient.getViewOptions()
364368
}
365369

366-
/** Sets saved options for a view. */
370+
/**
371+
* Sets saved options for a view.
372+
*
373+
* @param args - The saved view options to set.
374+
* @param requestId - Optional custom identifier for the request.
375+
* @returns A promise that resolves when the view options are saved.
376+
*/
367377
async setViewOptions(args: ViewOptionsSetArgs, requestId?: string): Promise<void> {
368378
return this.viewOptionsClient.setViewOptions(args, requestId)
369379
}
370380

371-
/** Deletes the saved options for a view. */
381+
/**
382+
* Deletes the saved options for a view.
383+
*
384+
* @param args - The view whose saved options to delete.
385+
* @param requestId - Optional custom identifier for the request.
386+
* @returns A promise that resolves when the view options are deleted.
387+
*/
372388
async deleteViewOptions(args: ViewOptionsDeleteArgs, requestId?: string): Promise<void> {
373389
return this.viewOptionsClient.deleteViewOptions(args, requestId)
374390
}

‎src/todoist-api.view-options.test.ts‎

Lines changed: 18 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { TodoistApi } from '.'
22
import { ENDPOINT_SYNC, getSyncBaseUri } from './consts/endpoints'
33
import { HttpResponse, http, server } from './test-utils/msw-setup'
44
import { DEFAULT_AUTH_TOKEN, DEFAULT_REQUEST_ID } from './test-utils/test-defaults'
5-
import { findViewOptions } from './utils/view-options'
65

76
function getTarget() {
87
return new TodoistApi(DEFAULT_AUTH_TOKEN)
@@ -111,7 +110,7 @@ describe('TodoistApi view options', () => {
111110
})
112111
})
113112

114-
test('deletes view options through Sync', async () => {
113+
test('clears calendar settings through Sync', async () => {
115114
let requestBody: { commands?: Array<Record<string, unknown>> } | undefined
116115
server.use(
117116
http.post(`${getSyncBaseUri()}${ENDPOINT_SYNC}`, async ({ request }) => {
@@ -121,41 +120,29 @@ describe('TodoistApi view options', () => {
121120
}),
122121
)
123122

124-
await getTarget().deleteViewOptions({ viewType: 'TODAY' })
123+
await getTarget().setViewOptions({ viewType: 'TODAY', calendarSettings: null })
125124

126125
expect(requestBody?.commands?.[0]).toMatchObject({
127-
type: 'view_options_delete',
128-
args: { view_type: 'TODAY' },
126+
type: 'view_options_set',
127+
args: { view_type: 'TODAY', calendar_settings: null },
129128
})
130129
})
131-
})
132130

133-
describe('findViewOptions', () => {
134-
const options = [
135-
{ viewType: 'FILTER' as const, objectId: 'filter1', sortedBy: 'DUE_DATE' as const },
136-
{
137-
viewType: 'WORKSPACE_FILTER' as const,
138-
objectId: 'filter2',
139-
sortedBy: 'PRIORITY' as const,
140-
},
141-
{ viewType: 'UPCOMING' as const, objectId: null, sortedBy: 'DEADLINE' as const },
142-
{ viewType: 'TODAY' as const, isDeleted: true },
143-
]
144-
145-
test('matches object-backed views by type and ID', () => {
146-
expect(
147-
findViewOptions(options, {
148-
viewTypes: ['FILTER', 'WORKSPACE_FILTER'],
149-
objectId: 'filter2',
150-
})?.sortedBy,
151-
).toBe('PRIORITY')
152-
})
131+
test('deletes view options through Sync', async () => {
132+
let requestBody: { commands?: Array<Record<string, unknown>> } | undefined
133+
server.use(
134+
http.post(`${getSyncBaseUri()}${ENDPOINT_SYNC}`, async ({ request }) => {
135+
requestBody = (await request.json()) as typeof requestBody
136+
const commandId = requestBody?.commands?.[0]?.uuid as string
137+
return HttpResponse.json({ sync_status: { [commandId]: 'ok' } })
138+
}),
139+
)
153140

154-
test('matches singleton views with null or omitted object IDs', () => {
155-
expect(findViewOptions(options, { viewTypes: ['UPCOMING'] })?.sortedBy).toBe('DEADLINE')
156-
})
141+
await getTarget().deleteViewOptions({ viewType: 'TODAY' })
157142

158-
test('ignores deleted options', () => {
159-
expect(findViewOptions(options, { viewTypes: ['TODAY'] })).toBeUndefined()
143+
expect(requestBody?.commands?.[0]).toMatchObject({
144+
type: 'view_options_delete',
145+
args: { view_type: 'TODAY' },
146+
})
160147
})
161148
})

‎src/types/sync/commands/view-options.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@ export type ViewOptionsSetArgs = {
1616
showCompletedTasks?: boolean
1717
sortedBy?: SortedBy | null
1818
sortOrder?: SortOrder | null
19-
deadline?: string
20-
calendarSettings?: CalendarSettings
19+
calendarSettings?: CalendarSettings | null
2120
}
2221

2322
export type ViewOptionsDeleteArgs = {

‎src/types/sync/resources/resources.test.ts‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,6 @@ describe('Sync resource schemas', () => {
369369
showCompletedTasks: false,
370370
sortedBy: 'DUE_DATE' as const,
371371
sortOrder: 'ASC' as const,
372-
deadline: 'no deadline',
373372
calendarSettings: {
374373
layout: 'WEEK' as const,
375374
visibleDayCount: 3 as const,
@@ -399,6 +398,29 @@ describe('Sync resource schemas', () => {
399398
const result = ViewOptionsSchema.parse(withExtra)
400399
expect(result).toHaveProperty('futureOption', true)
401400
})
401+
402+
test('validates cleared calendar settings', () => {
403+
expect(
404+
ViewOptionsSchema.parse({
405+
...validViewOptions,
406+
calendarSettings: {
407+
layout: null,
408+
visibleDayCount: null,
409+
color: null,
410+
},
411+
}),
412+
).toEqual({
413+
...validViewOptions,
414+
calendarSettings: {
415+
layout: null,
416+
visibleDayCount: null,
417+
color: null,
418+
},
419+
})
420+
expect(
421+
ViewOptionsSchema.parse({ ...validViewOptions, calendarSettings: null }),
422+
).toEqual({ ...validViewOptions, calendarSettings: null })
423+
})
402424
})
403425

404426
describe('ProjectViewOptionsDefaultsSchema', () => {

‎src/types/sync/resources/view-options.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,12 @@ export const CALENDAR_COLORS = ['PRIORITY', 'LABEL', 'PROJECT'] as const
7979
export type CalendarColor = (typeof CALENDAR_COLORS)[number]
8080

8181
export const CalendarSettingsSchema = z.looseObject({
82-
layout: z.enum(CALENDAR_LAYOUTS).optional(),
83-
visibleDayCount: z.union([z.literal(1), z.literal(3), z.literal(7)]).optional(),
84-
color: z.enum(CALENDAR_COLORS).optional(),
82+
layout: z.enum(CALENDAR_LAYOUTS).nullable().optional(),
83+
visibleDayCount: z
84+
.union([z.literal(1), z.literal(3), z.literal(7)])
85+
.nullable()
86+
.optional(),
87+
color: z.enum(CALENDAR_COLORS).nullable().optional(),
8588
})
8689

8790
export type CalendarSettings = z.infer<typeof CalendarSettingsSchema>
@@ -95,7 +98,6 @@ export const ViewOptionsSchema = z.looseObject({
9598
showCompletedTasks: z.boolean().optional(),
9699
sortedBy: SortedBySchema.optional(),
97100
sortOrder: SortOrderSchema.optional(),
98-
deadline: z.string().nullable().optional(),
99101
calendarSettings: CalendarSettingsSchema.nullable().optional(),
100102
isDeleted: z.boolean().optional(),
101103
})

‎src/utils/view-options.test.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import type { ViewOptions } from '../types/sync'
2+
import { findViewOptions, isActiveViewOption } from './view-options'
3+
4+
describe('view option utilities', () => {
5+
const options: ViewOptions[] = [
6+
{ viewType: 'FILTER', objectId: 'filter1', sortedBy: 'DUE_DATE' },
7+
{
8+
viewType: 'WORKSPACE_FILTER',
9+
objectId: 'filter2',
10+
sortedBy: 'PRIORITY',
11+
},
12+
{ viewType: 'UPCOMING', objectId: null, sortedBy: 'DEADLINE' },
13+
{ viewType: 'TODAY', isDeleted: true },
14+
]
15+
16+
test('identifies active options and deletion tombstones', () => {
17+
expect(isActiveViewOption(options[0])).toBe(true)
18+
expect(isActiveViewOption(options[3])).toBe(false)
19+
})
20+
21+
test('matches object-backed views by type and ID', () => {
22+
expect(
23+
findViewOptions(options, {
24+
viewTypes: ['FILTER', 'WORKSPACE_FILTER'],
25+
objectId: 'filter2',
26+
})?.sortedBy,
27+
).toBe('PRIORITY')
28+
})
29+
30+
test('matches singleton views with null or omitted object IDs', () => {
31+
expect(findViewOptions(options, { viewTypes: ['UPCOMING'] })?.sortedBy).toBe('DEADLINE')
32+
})
33+
34+
test('ignores deleted options', () => {
35+
expect(findViewOptions(options, { viewTypes: ['TODAY'] })).toBeUndefined()
36+
})
37+
})

‎src/utils/view-options.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ export interface FindViewOptionsArgs {
77
objectId?: string | null
88
}
99

10+
/** Returns whether saved view options are active rather than a deletion tombstone. */
11+
export function isActiveViewOption(options: ViewOptions): boolean {
12+
return options.isDeleted !== true
13+
}
14+
1015
/** Finds the active saved options for one logical view. */
1116
export function findViewOptions(
1217
options: readonly ViewOptions[],
@@ -16,7 +21,7 @@ export function findViewOptions(
1621

1722
return options.find(
1823
(entry) =>
19-
entry.isDeleted !== true &&
24+
isActiveViewOption(entry) &&
2025
(entry.objectId ?? null) === targetObjectId &&
2126
args.viewTypes.includes(entry.viewType),
2227
)

0 commit comments

Comments
 (0)