From 5cae33d41e59c72f38ec38b2fc4d5bb21935b80d Mon Sep 17 00:00:00 2001 From: Kevin Jahns Date: Fri, 28 Aug 2026 16:39:45 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A5(frontend)=20remove=20two=20dead=20?= =?UTF-8?q?paths=20to=20the=20Django=20document=20api?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Document content stopped going through Django when the collaboration server took it over, and these two are what the move left behind. Both are unreachable rather than merely unused, so they are removed instead of being carried forward. useUpdateDoc sent websocket: true whenever the provider was synced, to unlock a cache lock the backend used to hold while another user was connected. That lock is gone - there is no websocket field on the serializer and nothing reads one - so DRF has been silently dropping the key. Removing it also drops the store's only isSynced reader. The service worker still wrote an entry to a doc-content table keyed on documents/{id}/content/, an endpoint that no longer exists. Nothing ever read it back: handlerDidErrorRead only ever asks for doc-list and doc-item. The table is dropped on the next upgrade so it does not linger in browsers that already have it. This also clears the five standing typescript errors in the repository, which were all stale doc-content literals in the tests for the code being removed. Signed-off-by: Kevin Jahns --- .../docs/doc-management/api/useUpdateDoc.tsx | 13 +- .../src/features/service-worker/DocsDB.ts | 25 ++-- .../__tests__/ApiPlugin.test.tsx | 137 +----------------- .../service-worker/plugins/ApiPlugin.ts | 11 -- 4 files changed, 14 insertions(+), 172 deletions(-) diff --git a/src/frontend/apps/impress/src/features/docs/doc-management/api/useUpdateDoc.tsx b/src/frontend/apps/impress/src/features/docs/doc-management/api/useUpdateDoc.tsx index 5a9cf397e..63791ea88 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-management/api/useUpdateDoc.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-management/api/useUpdateDoc.tsx @@ -6,13 +6,11 @@ import { import { APIError, errorCauses, fetchAPI } from '@/api'; -import { useProviderStore } from '../stores'; import { Doc } from '../types'; export interface UpdateDocParams { id: Doc['id']; title?: string; - websocket?: boolean; } export const updateDoc = async ({ @@ -40,16 +38,7 @@ type UseUpdateDoc = UseMutationOptions & { export function useUpdateDoc(queryConfig?: UseUpdateDoc) { const queryClient = useQueryClient(); return useMutation({ - /** - * Tell the backend when we hold a live collaboration connection, - * otherwise its no-websocket cache lock blocks the update while - * another user is connected. - */ - mutationFn: (params) => - updateDoc({ - ...(useProviderStore.getState().isSynced ? { websocket: true } : {}), - ...params, - }), + mutationFn: updateDoc, ...queryConfig, onSuccess: (data, variables, onMutateResult, context) => { queryConfig?.listInvalidQueries?.forEach((queryKey) => { diff --git a/src/frontend/apps/impress/src/features/service-worker/DocsDB.ts b/src/frontend/apps/impress/src/features/service-worker/DocsDB.ts index 0ceb7da52..1d98a7018 100644 --- a/src/frontend/apps/impress/src/features/service-worker/DocsDB.ts +++ b/src/frontend/apps/impress/src/features/service-worker/DocsDB.ts @@ -11,12 +11,6 @@ export type DBRequest = { key: string; }; -export interface DocContentCacheEntry { - etag: string; - lastModified: string; - content: string; -} - interface IDocsDB extends DBSchema { 'doc-list': { key: string; @@ -34,13 +28,9 @@ interface IDocsDB extends DBSchema { key: 'version'; value: number; }; - 'doc-content': { - key: string; - value: DocContentCacheEntry; - }; } -type TableName = 'doc-list' | 'doc-item' | 'doc-mutation' | 'doc-content'; +type TableName = 'doc-list' | 'doc-item' | 'doc-mutation'; /** * IndexDB prefers incremental versioning when upgrading the database, @@ -88,8 +78,15 @@ export class DocsDB { if (!db.objectStoreNames.contains('doc-version')) { db.createObjectStore('doc-version'); } - if (!db.objectStoreNames.contains('doc-content')) { - db.createObjectStore('doc-content'); + /** + * Dropped with the Django `documents/{id}/content/` endpoint it + * mirrored: document content is the collaboration server's alone now. + * Existing browsers still carry the store, so it is removed here + * rather than left orphaned. Cast because it is deliberately absent + * from the schema above. + */ + if (db.objectStoreNames.contains('doc-content' as never)) { + db.deleteObjectStore('doc-content' as never); } }, }); @@ -140,7 +137,7 @@ export class DocsDB { */ public static async cacheResponse( key: string, - body: DocsResponse | Doc | DBRequest | DocContentCacheEntry, + body: DocsResponse | Doc | DBRequest, tableName: TableName, isRetry = false, ): Promise { diff --git a/src/frontend/apps/impress/src/features/service-worker/__tests__/ApiPlugin.test.tsx b/src/frontend/apps/impress/src/features/service-worker/__tests__/ApiPlugin.test.tsx index b4ef36d85..14605a283 100644 --- a/src/frontend/apps/impress/src/features/service-worker/__tests__/ApiPlugin.test.tsx +++ b/src/frontend/apps/impress/src/features/service-worker/__tests__/ApiPlugin.test.tsx @@ -139,60 +139,6 @@ describe('ApiPlugin', () => { }); }); - it(`calls requestWillFetch with type content and sets If-None-Match when etag is cached`, async () => { - const mockedSync = vi.fn().mockResolvedValue({}); - const apiPlugin = new ApiPlugin({ - type: 'content', - tableName: 'doc-content', - syncManager: { sync: () => mockedSync() } as any, - }); - - mockedGet.mockResolvedValue({ - etag: '"abc123"', - lastModified: '', - content: 'hello', - }); - - const requestInit = { - request: new Request('http://test.jest/documents/123456/content/'), - } as any; - - const request = await apiPlugin.requestWillFetch?.(requestInit); - expect(mockedGet).toHaveBeenCalledWith( - 'doc-content', - 'http://test.jest/documents/123456/content/', - ); - expect(request?.headers.get('If-None-Match')).toBe('"abc123"'); - }); - - it(`calls requestWillFetch with type content and sets If-Modified-Since when only lastModified is cached`, async () => { - const mockedSync = vi.fn().mockResolvedValue({}); - const apiPlugin = new ApiPlugin({ - type: 'content', - tableName: 'doc-content', - syncManager: { sync: () => mockedSync() } as SyncManager, - }); - - mockedGet.mockResolvedValue({ - etag: '', - lastModified: 'Mon, 14 Apr 2026 00:00:00 GMT', - content: 'hello', - }); - - const requestInit = { - request: new Request('http://test.jest/documents/123456/content/'), - } as any; - - const request = await apiPlugin.requestWillFetch?.(requestInit); - expect(mockedGet).toHaveBeenCalledWith( - 'doc-content', - 'http://test.jest/documents/123456/content/', - ); - expect(request?.headers.get('If-Modified-Since')).toBe( - 'Mon, 14 Apr 2026 00:00:00 GMT', - ); - }); - it(`checks getApiCatchHandler`, async () => { const response = ApiPlugin.getApiCatchHandler(); expect(await response.json()).toEqual({ error: 'Network is unavailable.' }); @@ -201,7 +147,6 @@ describe('ApiPlugin', () => { [ { type: 'list', tableName: 'doc-list' }, { type: 'item', tableName: 'doc-item' }, - { type: 'content', tableName: 'doc-content' }, ].forEach(({ type, tableName }) => { it(`checks handlerDidError with type ${type}`, async () => { const requestInit = { @@ -299,72 +244,6 @@ describe('ApiPlugin', () => { expect(response?.status).toBe(200); }); - it(`checks handlerDidError with type content-update`, async () => { - const requestInit = { - request: { - url: 'http://test.jest/documents/123456/content/', - clone: () => mockedClone(), - headers: new Headers({ - 'Content-Type': 'application/json', - }), - arrayBuffer: () => - RequestSerializer.objectToArrayBuffer({ - content: 'test', - }), - json: () => ({ - content: 'test', - }), - } as unknown as Request, - } as any; - - const mockedClone = vi.fn().mockReturnValue(requestInit.request); - - const mockedSync = vi.fn().mockResolvedValue({}); - const apiPlugin = new ApiPlugin({ - type: 'content-update', - syncManager: { - sync: () => mockedSync(), - } as any, - }); - - mockedGet.mockResolvedValue({ - etag: '', - lastModified: '', - content: '', - }); - - await apiPlugin.requestWillFetch?.(requestInit); - await apiPlugin.fetchDidFail?.({} as any); - const response = await apiPlugin.handlerDidError?.(requestInit); - expect(mockedGet).toHaveBeenCalledWith( - 'doc-content', - 'http://test.jest/documents/123456/content/', - ); - - expect(mockedPut).toHaveBeenCalledWith( - 'doc-mutation', - expect.objectContaining({ - key: expect.any(String), - requestData: expect.objectContaining({ - url: 'http://test.jest/documents/123456/content/', - headers: { - 'content-type': 'application/json', - }, - }), - }), - expect.any(String), - ); - expect(mockedPut).toHaveBeenCalledWith( - 'doc-content', - { etag: '', lastModified: '', content: 'test' }, - 'http://test.jest/documents/123456/content/', - ); - - expect(mockedPut).toHaveBeenCalledTimes(2); - expect(mockedClose).toHaveBeenCalled(); - expect(response?.status).toBe(204); - }); - it(`checks handlerDidError with type delete`, async () => { const requestInit = { request: { @@ -414,10 +293,6 @@ describe('ApiPlugin', () => { 'doc-item', 'http://test.jest/documents/123456/', ); - expect(mockedDelete).toHaveBeenCalledWith( - 'doc-content', - 'http://test.jest/documents/123456/content/', - ); expect(mockedGetAllKeys).toHaveBeenCalledWith('doc-list'); expect(mockedGet).toHaveBeenCalledWith( 'doc-list', @@ -509,15 +384,6 @@ describe('ApiPlugin', () => { expect.objectContaining({}), 'http://test.jest/documents/444555/', ); - expect(mockedPut).toHaveBeenCalledWith( - 'doc-content', - expect.objectContaining({ - content: '', - etag: '', - lastModified: '', - }), - 'http://test.jest/documents/444555/content/', - ); expect(mockedPut).toHaveBeenCalledWith( 'doc-list', expect.objectContaining({ @@ -534,7 +400,8 @@ describe('ApiPlugin', () => { 'doc-list', 'http://test.jest/documents/?page=1', ); - expect(mockedPut).toHaveBeenCalledTimes(4); + // doc-item, doc-list and the queued mutation — the doc-content entry is gone + expect(mockedPut).toHaveBeenCalledTimes(3); expect(mockedClose).toHaveBeenCalled(); expect(response?.status).toBe(201); }); diff --git a/src/frontend/apps/impress/src/features/service-worker/plugins/ApiPlugin.ts b/src/frontend/apps/impress/src/features/service-worker/plugins/ApiPlugin.ts index 500e144ef..935a21526 100644 --- a/src/frontend/apps/impress/src/features/service-worker/plugins/ApiPlugin.ts +++ b/src/frontend/apps/impress/src/features/service-worker/plugins/ApiPlugin.ts @@ -258,16 +258,6 @@ export class ApiPlugin implements WorkboxPlugin { 'doc-item', ); - /** - * Create an empty content for the new document in the cache, so the client can use it while offline, - * and it will be updated later when the request will be synced. - */ - await DocsDB.cacheResponse( - `${request.url}${uuid}/content/`, - { etag: '', lastModified: '', content: '' }, - 'doc-content', - ); - /** * Add the new entry to the cache list. */ @@ -310,7 +300,6 @@ export class ApiPlugin implements WorkboxPlugin { */ const db = await DocsDB.open(); await db.delete('doc-item', request.url); - await db.delete('doc-content', `${request.url}content/`); /** * Delete entry from the cache list.