From 606c73f6ded09996bbe758482e7f120a19dfcc53 Mon Sep 17 00:00:00 2001 From: Magnus Holm Date: Mon, 22 Sep 2025 11:12:51 +0200 Subject: [PATCH] feat: make applyDocumentActions more similar to the rest `applyDocumentActions` was the only function in the `core` package which interpreted the `SanityInstance` as a tree, automatically traversing the tree to find a matching instance. This is inconsistent with the other stores (e.g. query store) which assumes that it's being called with the right instance. In addition, this changes `applyDocumentActions` to only accept actions as an array (so we don't have to check if it's an array or not) and also it places the `actions` as part of option. This is consistent with e.g. `getQueryState` which has a required `query` field in its options. This now means that all functions which are wrapped in `bindActionByDataset` are now consistently getting passed an object as their first argument. This is going to be key so that we later can modify this function to consider the options (instead of the instance). There should be no breaking changes in the React package. --- .../src/document/applyDocumentActions.test.ts | 9 +- .../core/src/document/applyDocumentActions.ts | 58 +---- .../core/src/document/documentStore.test.ts | 219 ++++++++++-------- packages/core/src/document/documentStore.ts | 12 +- .../core/src/document/permissions.test.ts | 18 +- packages/core/src/document/permissions.ts | 6 +- packages/core/src/document/reducers.ts | 3 +- .../document/useApplyDocumentActions.test.ts | 133 ++++++++++- .../hooks/document/useApplyDocumentActions.ts | 48 +++- .../document/useDocumentPermissions.test.ts | 6 +- .../hooks/document/useDocumentPermissions.ts | 15 +- 11 files changed, 334 insertions(+), 193 deletions(-) diff --git a/packages/core/src/document/applyDocumentActions.test.ts b/packages/core/src/document/applyDocumentActions.test.ts index 7f5f0d14d..f3e1ef78e 100644 --- a/packages/core/src/document/applyDocumentActions.test.ts +++ b/packages/core/src/document/applyDocumentActions.test.ts @@ -72,7 +72,8 @@ describe('applyDocumentActions', () => { } // Call applyDocumentActions with a fixed transactionId for reproducibility. - const applyPromise = applyDocumentActions(instance, action, { + const applyPromise = applyDocumentActions(instance, { + actions: [action], transactionId: 'txn-success', }) @@ -128,7 +129,8 @@ describe('applyDocumentActions', () => { } // Call applyDocumentActions with a fixed transactionId. - const applyPromise = applyDocumentActions(instance, action, { + const applyPromise = applyDocumentActions(instance, { + actions: [action], transactionId: 'txn-error', }) @@ -160,7 +162,8 @@ describe('applyDocumentActions', () => { dataset: 'd', } // Call applyDocumentActions with the context using childInstance, but with action requiring parent's config - const applyPromise = applyDocumentActions(childInstance, action, { + const applyPromise = applyDocumentActions(childInstance, { + actions: [action], transactionId: 'txn-child-match', }) diff --git a/packages/core/src/document/applyDocumentActions.ts b/packages/core/src/document/applyDocumentActions.ts index 9da491b92..0d6351b8b 100644 --- a/packages/core/src/document/applyDocumentActions.ts +++ b/packages/core/src/document/applyDocumentActions.ts @@ -24,6 +24,11 @@ export interface ActionsResult( instance: SanityInstance, - action: - | DocumentAction - | DocumentAction[], - options?: ApplyDocumentActionsOptions, + options: ApplyDocumentActionsOptions, ): Promise>> /** @beta */ export function applyDocumentActions( instance: SanityInstance, - action: DocumentAction | DocumentAction[], - options?: ApplyDocumentActionsOptions, + options: ApplyDocumentActionsOptions, ): Promise /** @beta */ @@ -64,50 +65,9 @@ const boundApplyDocumentActions = bindActionByDataset(documentStore, _applyDocum /** @internal */ async function _applyDocumentActions( - {instance, state}: StoreContext, - actionOrActions: DocumentAction | DocumentAction[], - {transactionId = crypto.randomUUID(), disableBatching}: ApplyDocumentActionsOptions = {}, + {state}: StoreContext, + {actions, transactionId = crypto.randomUUID(), disableBatching}: ApplyDocumentActionsOptions, ): Promise { - const actions = Array.isArray(actionOrActions) ? actionOrActions : [actionOrActions] - - let projectId - let dataset - for (const action of actions) { - if (action.projectId) { - if (!projectId) projectId = action.projectId - if (action.projectId !== projectId) { - throw new Error( - `Mismatched project IDs found in actions. All actions must belong to the same project. Found "${action.projectId}" but expected "${projectId}".`, - ) - } - - if (action.dataset) { - if (!dataset) dataset = action.dataset - if (action.dataset !== dataset) { - throw new Error( - `Mismatched datasets found in actions. All actions must belong to the same dataset. Found "${action.dataset}" but expected "${dataset}".`, - ) - } - } - } - } - - if ( - (projectId && projectId !== instance.config.projectId) || - (dataset && dataset !== instance.config.dataset) - ) { - const matchedInstance = instance.match({projectId, dataset}) - if (!matchedInstance) { - throw new Error( - `Could not find a matching instance for projectId: "${projectId}" and dataset: "${dataset}"`, - ) - } - return boundApplyDocumentActions(matchedInstance, actionOrActions, { - disableBatching, - transactionId, - }) - } - const {events} = state.get() const transaction: QueuedTransaction = { diff --git a/packages/core/src/document/documentStore.test.ts b/packages/core/src/document/documentStore.test.ts index 2ed3b51ac..6c7ed3069 100644 --- a/packages/core/src/document/documentStore.test.ts +++ b/packages/core/src/document/documentStore.test.ts @@ -90,19 +90,23 @@ it('creates, edits, and publishes a document', async () => { const unsubscribe = documentState.subscribe() // Create a new document - const {appeared} = await applyDocumentActions(instance, createDocument(doc)) + const {appeared} = await applyDocumentActions(instance, {actions: [createDocument(doc)]}) expect(appeared).toContain(getDraftId(doc.documentId)) let currentDoc = documentState.getCurrent() expect(currentDoc?._id).toEqual(getDraftId(doc.documentId)) // Edit the document – add a title - await applyDocumentActions(instance, editDocument(doc, {set: {title: 'My First Article'}})) + await applyDocumentActions(instance, { + actions: [editDocument(doc, {set: {title: 'My First Article'}})], + }) currentDoc = documentState.getCurrent() expect(currentDoc?.title).toEqual('My First Article') // Publish the document; the resulting transactionId is used as the new _rev - const {transactionId, submitted} = await applyDocumentActions(instance, publishDocument(doc)) + const {transactionId, submitted} = await applyDocumentActions(instance, { + actions: [publishDocument(doc)], + }) await submitted() currentDoc = documentState.getCurrent() @@ -127,7 +131,9 @@ it('edits existing documents', async () => { title: 'existing doc', }) - await applyDocumentActions(instance, editDocument(doc, {set: {title: 'updated title'}})) + await applyDocumentActions(instance, { + actions: [editDocument(doc, {set: {title: 'updated title'}})], + }) expect(state.getCurrent()).toMatchObject({ _id: getDraftId(doc.documentId), title: 'updated title', @@ -150,12 +156,11 @@ it('sets optimistic changes synchronously', async () => { // then the actions are synchronous expect(state1.getCurrent()).toBeNull() - applyDocumentActions(instance1, createDocument(doc)) + applyDocumentActions(instance1, {actions: [createDocument(doc)]}) expect(state1.getCurrent()).toMatchObject({_id: getDraftId(doc.documentId)}) - const actionResult1Promise = applyDocumentActions( - instance1, - editDocument(doc, {set: {title: 'initial title'}}), - ) + const actionResult1Promise = applyDocumentActions(instance1, { + actions: [editDocument(doc, {set: {title: 'initial title'}})], + }) expect(state1.getCurrent()?.title).toBe('initial title') // notice how state2 doesn't have the value yet because it's a different @@ -173,10 +178,9 @@ it('sets optimistic changes synchronously', async () => { expect(state2.getCurrent()?.title).toBe('initial title') // synchronous for state 2 - const actionResult2Promise = applyDocumentActions( - instance2, - editDocument(doc, {set: {title: 'updated title'}}), - ) + const actionResult2Promise = applyDocumentActions(instance2, { + actions: [editDocument(doc, {set: {title: 'updated title'}})], + }) expect(state2.getCurrent()?.title).toBe('updated title') // async for state 1 expect(state1.getCurrent()?.title).toBe('initial title') @@ -198,7 +202,7 @@ it('propagates changes between two instances', async () => { const state2Unsubscribe = state2.subscribe() // Create the document from instance1. - await applyDocumentActions(instance1, createDocument(doc)).then((r) => r.submitted()) + await applyDocumentActions(instance1, {actions: [createDocument(doc)]}).then((r) => r.submitted()) const doc1 = state1.getCurrent() const doc2 = state2.getCurrent() @@ -206,9 +210,9 @@ it('propagates changes between two instances', async () => { expect(doc2?._id).toEqual(getDraftId(doc.documentId)) // Now, edit the document from instance2. - await applyDocumentActions(instance2, editDocument(doc, {set: {title: 'Hello world!'}})).then( - (r) => r.submitted(), - ) + await applyDocumentActions(instance2, { + actions: [editDocument(doc, {set: {title: 'Hello world!'}})], + }).then((r) => r.submitted()) const updated1 = state1.getCurrent() const updated2 = state2.getCurrent() @@ -230,20 +234,22 @@ it('handles concurrent edits and resolves conflicts', async () => { const oneOffInstance = createSanityInstance({projectId: 'p', dataset: 'd'}) // Create the initial document from a one-off instance. - await applyDocumentActions(oneOffInstance, [ - createDocument(doc), - editDocument(doc, {set: {title: 'The quick brown fox jumps over the lazy dog'}}), - ]).then((res) => res.submitted()) + await applyDocumentActions(oneOffInstance, { + actions: [ + createDocument(doc), + editDocument(doc, {set: {title: 'The quick brown fox jumps over the lazy dog'}}), + ], + }).then((res) => res.submitted()) // Both instances now issue an edit simultaneously. - const p1 = applyDocumentActions( - instance1, - editDocument(doc, {set: {title: 'The quick brown fox jumps over the lazy cat'}}), - ).then((r) => r.submitted()) - const p2 = applyDocumentActions( - instance2, - editDocument(doc, {set: {title: 'The quick brown elephant jumps over the lazy dog'}}), - ).then((r) => r.submitted()) + const p1 = applyDocumentActions(instance1, { + actions: [editDocument(doc, {set: {title: 'The quick brown fox jumps over the lazy cat'}})], + }).then((r) => r.submitted()) + const p2 = applyDocumentActions(instance2, { + actions: [ + editDocument(doc, {set: {title: 'The quick brown elephant jumps over the lazy dog'}}), + ], + }).then((r) => r.submitted()) // Wait for both actions to complete (or reject). await Promise.allSettled([p1, p2]) @@ -264,8 +270,8 @@ it('unpublishes and discards a document', async () => { const unsubscribe = documentState.subscribe() // Create and publish the document. - await applyDocumentActions(instance, createDocument(doc)) - const afterPublish = await applyDocumentActions(instance, publishDocument(doc)) + await applyDocumentActions(instance, {actions: [createDocument(doc)]}) + const afterPublish = await applyDocumentActions(instance, {actions: [publishDocument(doc)]}) const publishedDoc = documentState.getCurrent() expect(publishedDoc).toMatchObject({ _id: getPublishedId(doc.documentId), @@ -273,13 +279,13 @@ it('unpublishes and discards a document', async () => { }) // Unpublish the document (which should delete the published version and create a draft). - await applyDocumentActions(instance, unpublishDocument(doc)) + await applyDocumentActions(instance, {actions: [unpublishDocument(doc)]}) const afterUnpublish = documentState.getCurrent() // In our mock implementation the _id remains the same but the published copy is removed. expect(afterUnpublish?._id).toEqual(getDraftId(doc.documentId)) // Discard the draft (which deletes the draft version). - await applyDocumentActions(instance, discardDocument(doc)) + await applyDocumentActions(instance, {actions: [discardDocument(doc)]}) const afterDiscard = documentState.getCurrent() expect(afterDiscard).toBeNull() @@ -292,12 +298,12 @@ it('deletes a document', async () => { const documentState = getDocumentState(instance, doc) const unsubscribe = documentState.subscribe() - await applyDocumentActions(instance, [createDocument(doc), publishDocument(doc)]) + await applyDocumentActions(instance, {actions: [createDocument(doc), publishDocument(doc)]}) const docValue = documentState.getCurrent() expect(docValue).toBeDefined() // Delete the document. - await applyDocumentActions(instance, deleteDocument(doc)) + await applyDocumentActions(instance, {actions: [deleteDocument(doc)]}) const afterDelete = documentState.getCurrent() expect(afterDelete).toBeNull() @@ -312,7 +318,7 @@ it('cleans up document state when there are no subscribers', async () => { const unsubscribe = documentState.subscribe() // Create a document. - await applyDocumentActions(instance, createDocument(doc)) + await applyDocumentActions(instance, {actions: [createDocument(doc)]}) expect(documentState.getCurrent()).toBeDefined() // Unsubscribe from the document. @@ -337,7 +343,9 @@ it('fetches documents if there are no active subscriptions for the actions appli // there are no active subscriptions so applying this action will create one // for this action. this subscription will be removed when the outgoing // transaction for this action has been accepted by the server - const setNewTitle = applyDocumentActions(instance, editDocument(doc, {set: {title: 'new title'}})) + const setNewTitle = applyDocumentActions(instance, { + actions: [editDocument(doc, {set: {title: 'new title'}})], + }) expect(getCurrent()?.title).toBeUndefined() expect(getDocumentSyncStatus(instance, doc).getCurrent()).toBe(false) @@ -345,27 +353,25 @@ it('fetches documents if there are no active subscriptions for the actions appli expect(getCurrent()?.title).toBe('new title') // there is an active subscriber now so the edits are synchronous - applyDocumentActions(instance, editDocument(doc, {set: {title: 'updated title'}})) + applyDocumentActions(instance, {actions: [editDocument(doc, {set: {title: 'updated title'}})]}) expect(getCurrent()?.title).toBe('updated title') - applyDocumentActions(instance, editDocument(doc, {set: {title: 'updated title!'}})) + applyDocumentActions(instance, {actions: [editDocument(doc, {set: {title: 'updated title!'}})]}) expect(getCurrent()?.title).toBe('updated title!') expect(getDocumentSyncStatus(instance, doc).getCurrent()).toBe(false) // await submitted in order to test that there is no subscriptions - const result = await applyDocumentActions( - instance, - editDocument(doc, {set: {title: 'updated title'}}), - ) + const result = await applyDocumentActions(instance, { + actions: [editDocument(doc, {set: {title: 'updated title'}})], + }) await result.submitted() // test that there isn't any document state expect(getDocumentSyncStatus(instance, doc).getCurrent()).toBeUndefined() - const setNewNewTitle = applyDocumentActions( - instance, - editDocument(doc, {set: {title: 'new new title'}}), - ) + const setNewNewTitle = applyDocumentActions(instance, { + actions: [editDocument(doc, {set: {title: 'new new title'}})], + }) // now we'll have to await again expect(getCurrent()?.title).toBe(undefined) @@ -379,13 +385,15 @@ it('batches edit transaction into one outgoing transaction', async () => { const unsubscribe = getDocumentState(instance, doc).subscribe() // this creates its own transaction - applyDocumentActions(instance, createDocument(doc)) + applyDocumentActions(instance, {actions: [createDocument(doc)]}) // these get batched into one - applyDocumentActions(instance, editDocument(doc, {set: {title: 'name!'}})) - applyDocumentActions(instance, editDocument(doc, {set: {title: 'name!!'}})) - applyDocumentActions(instance, editDocument(doc, {set: {title: 'name!!!'}})) - const res = await applyDocumentActions(instance, editDocument(doc, {set: {title: 'name!!!!'}})) + applyDocumentActions(instance, {actions: [editDocument(doc, {set: {title: 'name!'}})]}) + applyDocumentActions(instance, {actions: [editDocument(doc, {set: {title: 'name!!'}})]}) + applyDocumentActions(instance, {actions: [editDocument(doc, {set: {title: 'name!!!'}})]}) + const res = await applyDocumentActions(instance, { + actions: [editDocument(doc, {set: {title: 'name!!!!'}})], + }) await res.submitted() expect(client.action).toHaveBeenCalledTimes(2) @@ -406,7 +414,7 @@ it('provides the consistency status via `getDocumentSyncStatus`', async () => { const unsubscribe = syncStatus.subscribe() expect(syncStatus.getCurrent()).toBe(true) - const applied = applyDocumentActions(instance, createDocument(doc)) + const applied = applyDocumentActions(instance, {actions: [createDocument(doc)]}) expect(syncStatus.getCurrent()).toBe(false) const createResult = await applied @@ -415,11 +423,11 @@ it('provides the consistency status via `getDocumentSyncStatus`', async () => { await createResult.submitted() expect(syncStatus.getCurrent()).toBe(true) - applyDocumentActions(instance, editDocument(doc, {set: {title: 'initial name'}})) + applyDocumentActions(instance, {actions: [editDocument(doc, {set: {title: 'initial name'}})]}) expect(syncStatus.getCurrent()).toBe(false) - applyDocumentActions(instance, editDocument(doc, {set: {title: 'updated name'}})) - const publishResult = applyDocumentActions(instance, publishDocument(doc)) + applyDocumentActions(instance, {actions: [editDocument(doc, {set: {title: 'updated name'}})]}) + const publishResult = applyDocumentActions(instance, {actions: [publishDocument(doc)]}) expect(syncStatus.getCurrent()).toBe(false) await publishResult.then((res) => res.submitted()) expect(syncStatus.getCurrent()).toBe(true) @@ -449,25 +457,23 @@ it('reverts failed outgoing transaction locally', async () => { const {getCurrent, subscribe} = getDocumentState(instance, doc) const unsubscribe = subscribe() - await applyDocumentActions(instance, createDocument(doc)) - applyDocumentActions(instance, editDocument(doc, {set: {title: 'the'}})) - applyDocumentActions(instance, editDocument(doc, {set: {title: 'the quick'}})) + await applyDocumentActions(instance, {actions: [createDocument(doc)]}) + applyDocumentActions(instance, {actions: [editDocument(doc, {set: {title: 'the'}})]}) + applyDocumentActions(instance, {actions: [editDocument(doc, {set: {title: 'the quick'}})]}) // this edit action is simulated to fail from the backend and will be reverted - const revertedActionResult = applyDocumentActions( - instance, - editDocument(doc, {set: {title: 'the quick brown'}}), - { - transactionId: 'force-revert', - disableBatching: true, - }, - ) + const revertedActionResult = applyDocumentActions(instance, { + actions: [editDocument(doc, {set: {title: 'the quick brown'}})], + transactionId: 'force-revert', + disableBatching: true, + }) - applyDocumentActions(instance, editDocument(doc, {set: {title: 'the quick brown fox'}})) - await applyDocumentActions( - instance, - editDocument(doc, {set: {title: 'the quick brown fox jumps'}}), - ).then((e) => e.submitted()) + applyDocumentActions(instance, { + actions: [editDocument(doc, {set: {title: 'the quick brown fox'}})], + }) + await applyDocumentActions(instance, { + actions: [editDocument(doc, {set: {title: 'the quick brown fox jumps'}})], + }).then((e) => e.submitted()) await expect(revertedEventPromise).resolves.toMatchObject({ type: 'reverted', @@ -484,7 +490,9 @@ it('reverts failed outgoing transaction locally', async () => { expect(getCurrent()?.title).toBe('the quick fox jumps') // check that we can still edit after recovering from the error - applyDocumentActions(instance, editDocument(doc, {set: {title: 'TEST the quick fox jumps'}})) + applyDocumentActions(instance, { + actions: [editDocument(doc, {set: {title: 'TEST the quick fox jumps'}})], + }) expect(getCurrent()?.title).toBe('TEST the quick fox jumps') unsubscribe() @@ -506,7 +514,7 @@ it('removes a queued transaction if it fails to apply', async () => { const unsubscribe = state.subscribe() await expect( - applyDocumentActions(instance, editDocument(doc, {set: {title: "can't set"}})), + applyDocumentActions(instance, {actions: [editDocument(doc, {set: {title: "can't set"}})]}), ).rejects.toThrowError(/Cannot edit document/) await expect(actionErrorEventPromise).resolves.toMatchObject({ @@ -516,8 +524,8 @@ it('removes a queued transaction if it fails to apply', async () => { }) // editing should still work after though (no crashing) - await applyDocumentActions(instance, createDocument(doc)) - applyDocumentActions(instance, editDocument(doc, {set: {title: 'can set!'}})) + await applyDocumentActions(instance, {actions: [createDocument(doc)]}) + applyDocumentActions(instance, {actions: [editDocument(doc, {set: {title: 'can set!'}})]}) expect(state.getCurrent()?.title).toBe('can set!') @@ -537,13 +545,17 @@ it('returns allowed true when no permission errors occur', async () => { }) const state = getDocumentState(instance, doc) const unsubscribe = state.subscribe() - await applyDocumentActions(instance, createDocument(doc)).then((r) => r.submitted()) + await applyDocumentActions(instance, {actions: [createDocument(doc)]}).then((r) => r.submitted()) // Use an action that includes a patch (so that update permission check is bypassed). const permissionsState = getPermissionsState(instance, { - ...doc, - type: 'document.edit', - patches: [{set: {title: 'New Title'}}], + actions: [ + { + ...doc, + type: 'document.edit', + patches: [{set: {title: 'New Title'}}], + }, + ], }) // Wait briefly to allow permissions calculation. await new Promise((resolve) => setTimeout(resolve, 10)) @@ -555,7 +567,7 @@ it('returns allowed true when no permission errors occur', async () => { it("should reject applying the action if a precondition isn't met", async () => { const doc = createDocumentHandle({documentId: 'does-not-exist', documentType: 'article'}) - await expect(applyDocumentActions(instance, deleteDocument(doc))).rejects.toThrow( + await expect(applyDocumentActions(instance, {actions: [deleteDocument(doc)]})).rejects.toThrow( 'The document you are trying to delete does not exist.', ) }) @@ -566,7 +578,7 @@ it("should reject applying the action if a permission isn't met", async () => { const datasetAcl = [{filter: 'false', permissions: ['create']}] vi.mocked(client.request).mockResolvedValue(datasetAcl) - await expect(applyDocumentActions(instance, createDocument(doc))).rejects.toThrow( + await expect(applyDocumentActions(instance, {actions: [createDocument(doc)]})).rejects.toThrow( 'You do not have permission to create a draft for document "does-not-exist".', ) }) @@ -576,7 +588,7 @@ it('returns allowed false with reasons when permission errors occur', async () = vi.mocked(client.request).mockResolvedValue(datasetAcl) const doc = createDocumentHandle({documentId: 'doc-perm-denied', documentType: 'article'}) - const result = await resolvePermissions(instance, createDocument(doc)) + const result = await resolvePermissions(instance, {actions: [createDocument(doc)]}) const message = 'You do not have permission to create a draft for document "doc-perm-denied".' expect(result).toMatchObject({ @@ -597,8 +609,10 @@ it('fetches dataset ACL and updates grants in the document store state', async ( const book = createDocumentHandle({documentId: crypto.randomUUID(), documentType: 'book'}) const author = createDocumentHandle({documentId: crypto.randomUUID(), documentType: 'author'}) - expect(await resolvePermissions(instance, createDocument(book))).toEqual({allowed: true}) - expect(await resolvePermissions(instance, createDocument(author))).toMatchObject({ + expect(await resolvePermissions(instance, {actions: [createDocument(book)]})).toEqual({ + allowed: true, + }) + expect(await resolvePermissions(instance, {actions: [createDocument(author)]})).toMatchObject({ allowed: false, message: expect.stringContaining('You do not have permission to create a draft for document'), }) @@ -611,10 +625,9 @@ it('returns a promise that resolves when a document has been loaded in the store // use one-off instance to create the document in the mock backend const oneOffInstance = createSanityInstance({projectId: 'p', dataset: 'd'}) - const result = await applyDocumentActions(oneOffInstance, [ - createDocument(doc), - editDocument(doc, {set: {title: 'initial title'}}), - ]) + const result = await applyDocumentActions(oneOffInstance, { + actions: [createDocument(doc), editDocument(doc, {set: {title: 'initial title'}})], + }) await result.submitted() // wait till submitted to server before resolving await expect(resolveDocument(instance, doc)).resolves.toMatchObject({ @@ -633,19 +646,23 @@ it('emits an event for each action after an outgoing transaction has been accept const doc = createDocumentHandle({documentId, documentType: 'article'}) expect(handler).toHaveBeenCalledTimes(0) - const tnx1 = await applyDocumentActions(instance, [ - createDocument(doc), - editDocument(doc, {set: {title: 'new name'}}), - publishDocument(doc), - ]).then((e) => e.submitted()) + const tnx1 = await applyDocumentActions(instance, { + actions: [ + createDocument(doc), + editDocument(doc, {set: {title: 'new name'}}), + publishDocument(doc), + ], + }).then((e) => e.submitted()) expect(handler).toHaveBeenCalledTimes(4) - const tnx2 = await applyDocumentActions(instance, [ - unpublishDocument(doc), - publishDocument(doc), - editDocument(doc, {set: {title: 'updated name'}}), - discardDocument(doc), - ]).then((e) => e.submitted()) + const tnx2 = await applyDocumentActions(instance, { + actions: [ + unpublishDocument(doc), + publishDocument(doc), + editDocument(doc, {set: {title: 'updated name'}}), + discardDocument(doc), + ], + }).then((e) => e.submitted()) expect(handler).toHaveBeenCalledTimes(9) expect(handler.mock.calls).toMatchObject([ @@ -660,7 +677,7 @@ it('emits an event for each action after an outgoing transaction has been accept [{type: 'accepted', outgoing: {transactionId: tnx2.transactionId}}], ]) - await applyDocumentActions(instance, deleteDocument(doc)) + await applyDocumentActions(instance, {actions: [deleteDocument(doc)]}) unsubscribe() }) diff --git a/packages/core/src/document/documentStore.ts b/packages/core/src/document/documentStore.ts index f343e45f6..935fcc53f 100644 --- a/packages/core/src/document/documentStore.ts +++ b/packages/core/src/document/documentStore.ts @@ -259,16 +259,20 @@ export const getDocumentSyncStatus = bindActionByDataset( }), ) +type PermissionsStateOptions = { + actions: DocumentAction[] +} + /** @beta */ export const getPermissionsState = bindActionByDataset( documentStore, createStateSourceAction({ selector: calculatePermissions, - onSubscribe: (context, actions) => + onSubscribe: (context, {actions}: PermissionsStateOptions) => manageSubscriberIds(context, getDocumentIdsFromActions(actions)), }) as StoreAction< DocumentStoreState, - [DocumentAction | DocumentAction[]], + [PermissionsStateOptions], StateSource> >, ) @@ -276,9 +280,9 @@ export const getPermissionsState = bindActionByDataset( /** @beta */ export const resolvePermissions = bindActionByDataset( documentStore, - ({instance}, actions: DocumentAction | DocumentAction[]) => { + ({instance}, options: PermissionsStateOptions) => { return firstValueFrom( - getPermissionsState(instance, actions).observable.pipe(filter((i) => i !== undefined)), + getPermissionsState(instance, options).observable.pipe(filter((i) => i !== undefined)), ) }, ) diff --git a/packages/core/src/document/permissions.test.ts b/packages/core/src/document/permissions.test.ts index 1dd2393e1..6961c6471 100644 --- a/packages/core/src/document/permissions.test.ts +++ b/packages/core/src/document/permissions.test.ts @@ -75,7 +75,7 @@ describe('calculatePermissions', () => { const actions: DocumentAction[] = [ {documentId: 'doc1', type: 'document.create', documentType: 'article'}, ] - const result = calculatePermissions({instance, state}, actions) + const result = calculatePermissions({instance, state}, {actions}) expect(result).toEqual({allowed: true}) }) @@ -91,7 +91,7 @@ describe('calculatePermissions', () => { const actions: DocumentAction[] = [ {documentId: 'doc1', type: 'document.create', documentType: 'article'}, ] - expect(calculatePermissions({instance, state}, actions)).toBeUndefined() + expect(calculatePermissions({instance, state}, {actions})).toBeUndefined() }) it('should catch PermissionActionError from processActions and return allowed false with a reason', () => { @@ -107,7 +107,7 @@ describe('calculatePermissions', () => { const actions: DocumentAction[] = [ {documentId: 'doc1', type: 'document.create', documentType: 'article'}, ] - const result = calculatePermissions({instance, state}, actions) + const result = calculatePermissions({instance, state}, {actions}) expect(result).toBeDefined() expect(result?.allowed).toBe(false) expect(result?.reasons).toEqual( @@ -135,7 +135,7 @@ describe('calculatePermissions', () => { const actions: DocumentAction[] = [ {documentId: 'doc1', documentType: 'book', type: 'document.edit'}, ] - const result = calculatePermissions({instance, state}, actions) + const result = calculatePermissions({instance, state}, {actions}) expect(result).toBeDefined() expect(result?.allowed).toBe(false) expect(result?.reasons).toEqual( @@ -161,7 +161,7 @@ describe('calculatePermissions', () => { const actions: DocumentAction[] = [ {documentId: 'doc1', documentType: 'book', type: 'document.edit'}, ] - const result = calculatePermissions({instance, state}, actions) + const result = calculatePermissions({instance, state}, {actions}) expect(result).toBeDefined() expect(result?.allowed).toBe(false) expect(result?.reasons).toEqual( @@ -185,7 +185,7 @@ describe('calculatePermissions', () => { const actions: DocumentAction[] = [ {documentId: 'doc1', type: 'document.create', documentType: 'article'}, ] - expect(calculatePermissions({instance, state}, actions)).toBeUndefined() + expect(calculatePermissions({instance, state}, {actions})).toBeUndefined() }) it('should catch ActionError from processActions and return a precondition error reason', () => { @@ -200,7 +200,7 @@ describe('calculatePermissions', () => { const actions: DocumentAction[] = [ {documentId: 'doc1', documentType: 'book', type: 'document.delete'}, ] - const result = calculatePermissions({instance, state}, actions) + const result = calculatePermissions({instance, state}, {actions}) expect(result).toBeDefined() expect(result?.allowed).toBe(false) expect(result?.reasons).toEqual( @@ -228,8 +228,8 @@ describe('calculatePermissions', () => { documentType: 'article', } // notice how the action is a copy - const result1 = calculatePermissions({instance, state}, [{...action}]) - const result2 = calculatePermissions({instance, state}, [{...action}]) + const result1 = calculatePermissions({instance, state}, {actions: [{...action}]}) + const result2 = calculatePermissions({instance, state}, {actions: [{...action}]}) expect(result1).toBe(result2) }) }) diff --git a/packages/core/src/document/permissions.ts b/packages/core/src/document/permissions.ts index fce169ed3..402cdffdf 100644 --- a/packages/core/src/document/permissions.ts +++ b/packages/core/src/document/permissions.ts @@ -58,12 +58,12 @@ const nullReplacer: object = {} const documentsSelector = createSelector( [ ({state: {documentStates}}: SelectorContext) => documentStates, - (_context: SelectorContext, actions: DocumentAction | DocumentAction[]) => + (_context: SelectorContext, {actions}: {actions: DocumentAction[]}) => actions, ], (documentStates, actions) => { const documentIds = new Set( - (Array.isArray(actions) ? actions : [actions]) + actions .map((i) => i.documentId) .filter((i) => typeof i === 'string') .flatMap((documentId) => [getPublishedId(documentId), getDraftId(documentId)]), @@ -99,7 +99,7 @@ const documentsSelector = createSelector( const memoizedActionsSelector = createSelector( [ documentsSelector, - (_state: SelectorContext, actions: DocumentAction | DocumentAction[]) => + (_state: SelectorContext, {actions}: {actions: DocumentAction[]}) => actions, ], (documents, actions) => { diff --git a/packages/core/src/document/reducers.ts b/packages/core/src/document/reducers.ts index 56979c8f7..b252067f2 100644 --- a/packages/core/src/document/reducers.ts +++ b/packages/core/src/document/reducers.ts @@ -579,8 +579,7 @@ export function manageSubscriberIds( } } -export function getDocumentIdsFromActions(action: DocumentAction | DocumentAction[]): string[] { - const actions = Array.isArray(action) ? action : [action] +export function getDocumentIdsFromActions(actions: DocumentAction[]): string[] { return Array.from( new Set( actions diff --git a/packages/react/src/hooks/document/useApplyDocumentActions.test.ts b/packages/react/src/hooks/document/useApplyDocumentActions.test.ts index a488f96db..28770b101 100644 --- a/packages/react/src/hooks/document/useApplyDocumentActions.test.ts +++ b/packages/react/src/hooks/document/useApplyDocumentActions.test.ts @@ -1,20 +1,135 @@ -import {applyDocumentActions} from '@sanity/sdk' +import {applyDocumentActions, type SanityInstance} from '@sanity/sdk' import {describe, it} from 'vitest' -import {createCallbackHook} from '../helpers/createCallbackHook' +import {useSanityInstance} from '../context/useSanityInstance' +import {useApplyDocumentActions} from './useApplyDocumentActions' -vi.mock('../helpers/createCallbackHook', () => ({ - createCallbackHook: vi.fn((cb) => () => cb), -})) vi.mock('@sanity/sdk', async (importOriginal) => { const original = await importOriginal() return {...original, applyDocumentActions: vi.fn()} }) +vi.mock('../context/useSanityInstance') + +// These are quite fragile mocks, but they are useful enough for now. +const instances: Record = { + 'p123.d': {__id: 'p123.d'} as unknown as SanityInstance, + 'p.d123': {__id: 'p.d123'} as unknown as SanityInstance, + 'p123.d123': {__id: 'p123.d123'} as unknown as SanityInstance, +} + +const instance = { + match({projectId = 'p', dataset = 'd'}): SanityInstance | undefined { + return instances[`${projectId}.${dataset}`] + }, +} as unknown as SanityInstance + describe('useApplyDocumentActions', () => { - it('calls `createCallbackHook` with `applyActions`', async () => { - expect(createCallbackHook).not.toHaveBeenCalled() - await import('./useApplyDocumentActions') - expect(createCallbackHook).toHaveBeenCalledWith(applyDocumentActions) + beforeEach(() => { + vi.resetAllMocks() + vi.mocked(useSanityInstance).mockReturnValueOnce(instance) + }) + + it('uses the SanityInstance', async () => { + const apply = useApplyDocumentActions() + apply({ + type: 'document.edit', + documentType: 'post', + documentId: 'abc', + }) + + expect(applyDocumentActions).toHaveBeenCalledExactlyOnceWith(instance, { + actions: [ + { + type: 'document.edit', + documentType: 'post', + documentId: 'abc', + }, + ], + }) + }) + + it('uses SanityInstance.match when projectId is overrideen', async () => { + const apply = useApplyDocumentActions() + apply({ + type: 'document.edit', + documentType: 'post', + documentId: 'abc', + + projectId: 'p123', + }) + + expect(applyDocumentActions).toHaveBeenCalledExactlyOnceWith(instances['p123.d'], { + actions: [ + { + type: 'document.edit', + documentType: 'post', + documentId: 'abc', + + projectId: 'p123', + }, + ], + }) + }) + + it('uses SanityInstance when dataset is overrideen', async () => { + const apply = useApplyDocumentActions() + apply({ + type: 'document.edit', + documentType: 'post', + documentId: 'abc', + + dataset: 'd123', + }) + + expect(applyDocumentActions).toHaveBeenCalledExactlyOnceWith(instance, { + actions: [ + { + type: 'document.edit', + documentType: 'post', + documentId: 'abc', + + dataset: 'd123', + }, + ], + }) + }) + + it('uses SanityInstance.amcth when projectId and dataset is overrideen', async () => { + const apply = useApplyDocumentActions() + apply({ + type: 'document.edit', + documentType: 'post', + documentId: 'abc', + + projectId: 'p123', + dataset: 'd123', + }) + + expect(applyDocumentActions).toHaveBeenCalledExactlyOnceWith(instances['p123.d123'], { + actions: [ + { + type: 'document.edit', + documentType: 'post', + documentId: 'abc', + + projectId: 'p123', + dataset: 'd123', + }, + ], + }) + }) + + it("throws if SanityInstance.match doesn't find anything", async () => { + const apply = useApplyDocumentActions() + expect(() => { + apply({ + type: 'document.edit', + documentType: 'post', + documentId: 'abc', + + projectId: 'other', + }) + }).toThrow() }) }) diff --git a/packages/react/src/hooks/document/useApplyDocumentActions.ts b/packages/react/src/hooks/document/useApplyDocumentActions.ts index 622d5e137..42ee50011 100644 --- a/packages/react/src/hooks/document/useApplyDocumentActions.ts +++ b/packages/react/src/hooks/document/useApplyDocumentActions.ts @@ -6,7 +6,7 @@ import { } from '@sanity/sdk' import {type SanityDocument} from 'groq' -import {createCallbackHook} from '../helpers/createCallbackHook' +import {useSanityInstance} from '../context/useSanityInstance' // this import is used in an `{@link useEditDocument}` // eslint-disable-next-line unused-imports/no-unused-imports, import/consistent-type-specifier-style import type {useEditDocument} from './useEditDocument' @@ -119,6 +119,46 @@ interface UseApplyDocumentActions { * } * ``` */ -export const useApplyDocumentActions = createCallbackHook( - applyDocumentActions, -) as UseApplyDocumentActions +export const useApplyDocumentActions: UseApplyDocumentActions = () => { + const instance = useSanityInstance() + + return (actionOrActions, options) => { + const actions = Array.isArray(actionOrActions) ? actionOrActions : [actionOrActions] + + let projectId + let dataset + for (const action of actions) { + if (action.projectId) { + if (!projectId) projectId = action.projectId + if (action.projectId !== projectId) { + throw new Error( + `Mismatched project IDs found in actions. All actions must belong to the same project. Found "${action.projectId}" but expected "${projectId}".`, + ) + } + + if (action.dataset) { + if (!dataset) dataset = action.dataset + if (action.dataset !== dataset) { + throw new Error( + `Mismatched datasets found in actions. All actions must belong to the same dataset. Found "${action.dataset}" but expected "${dataset}".`, + ) + } + } + } + } + + if (projectId || dataset) { + const actualInstance = instance.match({projectId, dataset}) + if (!actualInstance) { + throw new Error( + `Could not find a matching Sanity instance for the requested action: ${JSON.stringify({projectId, dataset}, null, 2)}. + Please ensure there is a ResourceProvider component with a matching configuration in the component hierarchy.`, + ) + } + + return applyDocumentActions(actualInstance, {actions, ...options}) + } + + return applyDocumentActions(instance, {actions, ...options}) + } +} diff --git a/packages/react/src/hooks/document/useDocumentPermissions.test.ts b/packages/react/src/hooks/document/useDocumentPermissions.test.ts index 380404011..59d62c2df 100644 --- a/packages/react/src/hooks/document/useDocumentPermissions.test.ts +++ b/packages/react/src/hooks/document/useDocumentPermissions.test.ts @@ -99,7 +99,7 @@ describe('usePermissions', () => { projectId: mockAction.projectId, dataset: mockAction.dataset, }) - expect(getPermissionsState).toHaveBeenCalledWith(mockInstance, mockAction) + expect(getPermissionsState).toHaveBeenCalledWith(mockInstance, {actions: [mockAction]}) expect(result.current).toEqual(mockPermissionAllowed) }) @@ -122,7 +122,7 @@ describe('usePermissions', () => { renderHook(() => useDocumentPermissions(actions)) - expect(getPermissionsState).toHaveBeenCalledWith(mockInstance, actions) + expect(getPermissionsState).toHaveBeenCalledWith(mockInstance, {actions}) }) it('should throw an error if actions have mismatched project IDs', () => { @@ -175,7 +175,7 @@ describe('usePermissions', () => { // Now it should render properly await waitFor(() => { - expect(getPermissionsState).toHaveBeenCalledWith(mockInstance, mockAction) + expect(getPermissionsState).toHaveBeenCalledWith(mockInstance, {actions: [mockAction]}) }) }) diff --git a/packages/react/src/hooks/document/useDocumentPermissions.ts b/packages/react/src/hooks/document/useDocumentPermissions.ts index 3d0366fcb..7a349256c 100644 --- a/packages/react/src/hooks/document/useDocumentPermissions.ts +++ b/packages/react/src/hooks/document/useDocumentPermissions.ts @@ -84,7 +84,10 @@ import {useSanityInstance} from '../context/useSanityInstance' export function useDocumentPermissions( actionOrActions: DocumentAction | DocumentAction[], ): DocumentPermissionsResult { - const actions = Array.isArray(actionOrActions) ? actionOrActions : [actionOrActions] + const actions = useMemo( + () => (Array.isArray(actionOrActions) ? actionOrActions : [actionOrActions]), + [actionOrActions], + ) // if actions is an array, we need to check that all actions belong to the same project and dataset let projectId let dataset @@ -111,20 +114,20 @@ export function useDocumentPermissions( const instance = useSanityInstance({projectId, dataset}) const isDocumentReady = useCallback( - () => getPermissionsState(instance, actionOrActions).getCurrent() !== undefined, - [actionOrActions, instance], + () => getPermissionsState(instance, {actions}).getCurrent() !== undefined, + [actions, instance], ) if (!isDocumentReady()) { throw firstValueFrom( - getPermissionsState(instance, actionOrActions).observable.pipe( + getPermissionsState(instance, {actions}).observable.pipe( filter((result) => result !== undefined), ), ) } const {subscribe, getCurrent} = useMemo( - () => getPermissionsState(instance, actionOrActions), - [actionOrActions, instance], + () => getPermissionsState(instance, {actions}), + [actions, instance], ) return useSyncExternalStore(subscribe, getCurrent) as DocumentPermissionsResult