fix(caldav): honour fetch override in todo API - #32
Merged
Conversation
The todo module was modelled on the calendar one but predates the fetch override, and never gained it. All six functions lacked `fetch?: typeof fetch`, and neither createDAVClient nor the DAVClient class forwarded an override to any of them -- the only remaining gap after makeAddressBook, and silent rather than red because client.ts never attempted to pass it. Each function now matches its calendar counterpart exactly: todoQuery and todoMultiGet mirror calendarQuery and calendarMultiGet, fetchTodos mirrors fetchCalendarObjects including threading the override through its internal query and multiget calls, and the write trio mirrors create/update/deleteCalendarObject. Consumers on a platform that needs a custom fetch -- KaiOS mozSystem, a Cloudflare Worker, any environment the default cross-fetch cannot serve -- can now read and write VTODOs at all. Before, every todo request bypassed their fetch, so the whole todo surface was unusable there while calendars and contacts worked.
Ten tests across three independently reverted halves. todo.test.ts follows calendar.test.ts: mock ./request only, let the real collectionQuery run through to davRequest, and assert each of the six functions forwards the override -- plus that it stays undefined when none is given. Reverting the todo.ts threading fails six of the seven. client.test.ts extends the existing fetch-override describe with fetchTodos and createTodo. Reverting the class wiring fails the two DAVClient cases while the createDAVClient one still passes; reverting the factory wiring fails only the createDAVClient one.
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.
The follow-up identified in #31. This closes the last fetch-override gap in the codebase.
The defect
The todo module was modelled on the calendar one but predates the fetch override and never gained it. All six functions lacked
fetch?: typeof fetch, and neither wiring surface forwarded an override to any of them:createDAVClientwired all six asdefaultParam(rawTodoQuery, { headers: authHeaders })— nofetch, unlike every calendar equivalent.DAVClientforwardedthis.fetchOverridein 29 methods but in none of the six todo ones.Unlike
makeAddressBook, this was never a type error, becauseclient.tsnever attempted to passfetch. It was silent in both directions — which is why a green typecheck did not surface it.Matching the calendar reference
Each function now mirrors its counterpart exactly rather than improvising:
todoQuerycalendarQuerycollectionQuerytodoMultiGetcalendarMultiGetcollectionQueryfetchTodosfetchCalendarObjectscreateTodocreateCalendarObjectcreateObjectupdateTodoupdateCalendarObjectupdateObjectdeleteTododeleteCalendarObjectdeleteObjectfetchTodoswas the one needing care: likefetchCalendarObjects, it callstodoQuery/todoMultiGetinternally, so the override has to be passed down or the outer signature is decorative. All three internal call sites now pass it.The JSDoc gains a
@param params.fetchline per function, matching this file's own convention (calendar.tscarries no JSDoc at all).Both wiring surfaces are done too: six
fetch: fetchOverrideincreateDAVClient, sixfetch: this.fetchOverridein the class, written in the same multi-line shape the calendar entries already use.Verification
A scan for every exported function that takes
fetchOptionsbut notfetch?: typeof fetchis now empty acrossaccount,addressBook,calendar,collection,requestandtodo— it previously returned these six. Likewise everydefaultParamincreateDAVClientforwardsfetchOverride, and every class method forwardsthis.fetchOverride.Runtime behaviour change
The override goes from dead to live across the whole todo surface. Consumers who configure a custom fetch previously had every todo request bypass it and go out through the module-level default (native
globalThis.fetch, elsecross-fetch).Concretely: on KaiOS with
mozSystem, in a Cloudflare Worker, or anywhere the default fetch cannot serve the request, the todo API was entirely unusable — reads and writes both — while calendars and contacts worked, since those already honoured the override. Those consumers can now use VTODOs at all. Anyone not passing a custom fetch is unaffected:fetchOverrideisundefinedand the request helpers fall back exactly as before, which one test pins down explicitly.Tests
Ten, and each of the three halves was confirmed to fail on its own when that half is reverted:
todo.test.ts(new file, but the existingcalendar.test.tsharness — mock./requestonly and let the realcollectionQueryrun through todavRequest): one test per function plus an undefined case. Reverting thetodo.tsthreading fails 6 of 7; the undefined case still passes.client.test.tsextends the existing "DAVClient fetch override" describe withfetchTodos(which also exercises the internaltodoQuerythreading) andcreateTodo. Reverting the class wiring fails the twoDAVClientcases while thecreateDAVClientone still passes; reverting the factory wiring fails only thecreateDAVClientone.Side effect worth flagging
pnpm lintdrops from 10 pre-existing errors to 4, without--fixbeing run: six of the ten sat on the exactDAVClienttodo lines this PR rewrote into the multi-line form. No new lint errors. The remaining 4 areaddressBook.ts:355,client.ts:709,index.ts:63,todo.ts:205— all pre-existing and untouched here; they belong to the hygiene PR.