Skip to content

Sync tussen vertalingen#26

Open
nhoenderop wants to merge 7 commits intomainfrom
add-sync-across-translations
Open

Sync tussen vertalingen#26
nhoenderop wants to merge 7 commits intomainfrom
add-sync-across-translations

Conversation

@nhoenderop
Copy link
Copy Markdown

@nhoenderop nhoenderop commented Nov 3, 2025

Een proof of concept voor het synchroniseren van velden tussen vertalingen.
Optioneel, backward compatible, granulair. (althans, dat waren mijn uitgangspunten)

Introduceert een syncAcrossTranslations flag op fields in schemas en een document actie 'Publish & sync', die op het moment van publiceren het volgende doet:

  1. Een groq query die en alle vertalingen (siblings) ophaalt
  2. De velden die gemarkeerd zijn als "moeten gelijk blijven tussen vertalingen" op het huidige document verzamelen
  3. Daar een payload van maken
  4. Die waarden op alle documenten in de dataset met hetzelfde translationId probeert te schrijven in een transactie
  5. Afwacht of dit lukt,
    zo ja: publiceer ook het huidige document
    zo nee: error gooien en ook het huidige document niet publiceren om te voorkomen dat ze uit elkaar gaan lopen

Dat is grofweg wat het doet, wat kleine dingen:

De actie is niet mogelijk op drafts.
addFields wrapped desbetreffende velden met een SyncFieldWrapper (een icoontje rechtsboven het veld met een tooltip)
de message daar in is nog hardcoded
het mechanisme controleert niet of het een veld moet updaten, op dit moment doet het dat gewoon.

Om het te gebruiken in een project moet je in sanity.config.js publishAndSyncAction importeren:

import { addFields as addMultiLanguageFields, publishAndSyncAction } from '@kaliber/sanity-plugin-multi-language'

en meegeven in document.actions:

return [...filteredActionsBySingletonTypes, publishAndSyncAction]

daarna kun je op elk veld binnen options syncAcrossTranslations: true toevoegen om hem mee te nemen in de sync.

Summary by CodeRabbit

  • New Features
    • Added a "Publish & Sync" document action to publish a document and synchronize designated fields across all language translations.
    • Added visual sync indicators on fields to show which fields are synced across translations.
    • Added support to configure which fields automatically sync their values during the publish workflow.

@nhoenderop nhoenderop requested a review from daveykropf November 3, 2025 16:15
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 3, 2025

Walkthrough

Adds a "Publish & Sync" document action that, when invoked, finds sibling translations, builds a payload from fields configured to sync, patches siblings in a single transaction, and then publishes the current document. Also wraps sync-enabled fields with a small UI indicator.

Changes

Cohort / File(s) Summary
Publish & Sync Action Module
src/actions/publishAndSync.js
New file exporting publishAndSyncAction({ id, type, draft }), querySiblings(documentType, translationId, currentLanguage, sanityClient), buildSyncPayload(document, syncFieldNames), and syncToSiblings(siblings, payload, sanityClient). Implements early exits when no sync fields or draft, queries non-draft sibling translations, builds a field→value payload, patches siblings in a single transaction, publishes the current document, and wraps errors with contextual messages.
Field Wrapping & Configuration
src/addFields.js
Reads fields with options.syncAcrossTranslations, collects syncFieldNames, replaces eligible field components.input with createSyncFieldWrapper(...), and stores syncFieldNames on type.options.kaliber. Preserves field order and existing API surface.
Sync Field UI Components
src/components/SyncFieldWrapper.js
New file exporting createSyncFieldWrapper(BaseComponent) (HOC that forwards props and renders a sync indicator alongside the base input) and SyncFieldIndicator() (UI element showing a SyncIcon with a tooltip "Synced across all translations.").
Plugin Exports
src/sanityPluginMultiLanguage.js
Adds named export publishAndSyncAction from ./actions/publishAndSync.

Sequence Diagram

sequenceDiagram
    actor User
    participant Editor as Sanity Editor
    participant Action as Publish & Sync Action
    participant Query as querySiblings
    participant Payload as buildSyncPayload
    participant Sync as syncToSiblings
    participant SanityAPI as Sanity API

    User->>Editor: Trigger "Publish & Sync"
    Editor->>Action: invoke publishAndSyncAction

    rect rgb(245,250,255)
    Note over Action: Early checks: are syncFieldNames configured? is draft present?
    alt No syncFieldNames
        Action-->>Editor: action unavailable (returns null)
    else Draft missing
        Action-->>Editor: action disabled
    end
    end

    rect rgb(230,245,230)
    Action->>Query: querySiblings(type, translationId, currentLanguage)
    Query->>SanityAPI: fetch sibling documents (non-draft, same translationId, other language)
    Query-->>Action: sibling list
    end

    rect rgb(255,240,230)
    alt siblings found
        Action->>Payload: buildSyncPayload(draft, syncFieldNames)
        Payload-->>Action: payload { fieldName: value, ... }
        Action->>Sync: syncToSiblings(siblings, payload)
        Sync->>SanityAPI: apply patches in single transaction
        SanityAPI-->>Sync: transaction committed
        Sync-->>Action: done
    end
    end

    rect rgb(235,245,235)
    Action->>SanityAPI: publish current document
    SanityAPI-->>Action: publish complete
    Action-->>Editor: action finished
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Areas requiring attention:
    • publishAndSyncAction: verify early-exit/disabled semantics and error wrapping.
    • syncToSiblings: ensure transaction construction correctly patches all siblings and handles failures.
    • addFields.js: confirm detection of options.syncAcrossTranslations, preservation of original field order, and correct placement of syncFieldNames on type.options.kaliber.
    • SyncFieldWrapper: ensure props are forwarded and UI layout doesn't interfere with existing field rendering.

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Sync tussen vertalingen' (Sync between translations) is directly related to the main change—a proof of concept for synchronizing fields between translations.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch add-sync-across-translations

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 051a102 and a13101c.

📒 Files selected for processing (2)
  • src/addFields.js (3 hunks)
  • src/components/SyncFieldWrapper.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/SyncFieldWrapper.js
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: nhoenderop
Repo: kaliberjs/sanity-plugin-multi-language PR: 26
File: src/utils/sync.js:7-11
Timestamp: 2025-11-03T16:27:28.868Z
Learning: In the sanity-plugin-multi-language sync feature (src/utils/sync.js), the buildSyncPayload function intentionally includes fields with undefined values. This allows deletions to be propagated across translations, which is the desired behavior for keeping translations synchronized.
📚 Learning: 2025-11-03T16:27:28.868Z
Learnt from: nhoenderop
Repo: kaliberjs/sanity-plugin-multi-language PR: 26
File: src/utils/sync.js:7-11
Timestamp: 2025-11-03T16:27:28.868Z
Learning: In the sanity-plugin-multi-language sync feature (src/utils/sync.js), the buildSyncPayload function intentionally includes fields with undefined values. This allows deletions to be propagated across translations, which is the desired behavior for keeping translations synchronized.

Applied to files:

  • src/addFields.js
🧬 Code graph analysis (1)
src/addFields.js (2)
src/actions/publishAndSync.js (1)
  • syncFieldNames (17-17)
src/components/SyncFieldWrapper.js (1)
  • createSyncFieldWrapper (10-21)
🔇 Additional comments (3)
src/addFields.js (3)

16-29: LGTM! Transformation logic correctly identifies and wraps sync fields.

The implementation properly collects syncFieldNames and wraps eligible fields with the sync indicator. The use of field.options?.syncAcrossTranslations (nested under options) addresses the earlier design feedback about avoiding root-level properties, and wrapping all flagged fields (not just those with custom components) ensures the sync indicator appears consistently.


33-39: LGTM! syncFieldNames storage aligns with consumer expectations.

The nesting under type.options.kaliber.syncFieldNames matches how publishAndSync.js retrieves this data, and the spread operators correctly preserve existing options without overwriting other properties.


73-73: LGTM! Correct use of transformedFields.

Spreading transformedFields maintains field order and correctly applies the transformations from the earlier mapping logic.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@nhoenderop nhoenderop requested a review from EECOLOR November 3, 2025 16:15
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (3)
src/components/SyncFieldIndicator.js (1)

15-30: Improve screen-reader support for the indicator

Right now the raw “⟳” glyph has no accessible name, so assistive tech users won’t learn that the field syncs translations. Consider giving the rendered element a role/label (e.g. role="img" + aria-label="Synced across all translations" or visually hidden text).

-      <Box display="inline-block" paddingLeft={2} style={{ fontSize: '20px' }}>
-
-      </Box>
+      <Box
+        display="inline-block"
+        paddingLeft={2}
+        role="img"
+        aria-label="Synced across all translations"
+        style={{ fontSize: '20px' }}
+      >
+
+      </Box>
src/utils/fieldDetection.js (1)

28-30: Consider filtering out fields without a name.

While well-formed schemas should always have field.name, adding a filter would make this more defensive.

 export function getSyncFieldNames(schemaType) {
-  return findSyncFields(schemaType).map(field => field.name)
+  return findSyncFields(schemaType)
+    .filter(field => field.name)
+    .map(field => field.name)
 }
src/utils/sync.js (1)

20-32: Consider adding input validation for robustness.

Adding basic validation of inputs would make this function more defensive, though it may not be necessary if inputs are always controlled.

 export async function syncToSiblings(siblings, payload, client) {
+  if (!Array.isArray(siblings) || !payload || typeof payload !== 'object' || !client) {
+    throw new Error('Invalid arguments to syncToSiblings')
+  }
+  
   if (siblings.length === 0) {
     return // No siblings to sync
   }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cbb608e and 763c64e.

📒 Files selected for processing (9)
  • src/actions/publishAndSync.js (1 hunks)
  • src/addFields.js (3 hunks)
  • src/components/SyncFieldIndicator.js (1 hunks)
  • src/components/SyncFieldWrapper.js (1 hunks)
  • src/hooks/useSyncFields.js (1 hunks)
  • src/sanityPluginMultiLanguage.js (1 hunks)
  • src/utils/fieldDetection.js (1 hunks)
  • src/utils/sibling.js (1 hunks)
  • src/utils/sync.js (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
src/actions/publishAndSync.js (2)
src/utils/sibling.js (2)
  • siblings (15-27)
  • querySiblings (9-33)
src/utils/sync.js (2)
  • buildSyncPayload (7-11)
  • syncToSiblings (20-32)
src/utils/sync.js (2)
src/actions/publishAndSync.js (2)
  • syncFieldNames (17-17)
  • client (15-15)
src/utils/sibling.js (1)
  • siblings (15-27)
src/utils/sibling.js (2)
src/components/Translations.js (1)
  • translationId (33-33)
src/actions/publishAndSync.js (1)
  • client (15-15)
src/addFields.js (2)
src/actions/publishAndSync.js (1)
  • syncFieldNames (17-17)
src/components/SyncFieldWrapper.js (1)
  • createSyncFieldWrapper (9-20)
src/components/SyncFieldWrapper.js (1)
src/components/SyncFieldIndicator.js (1)
  • SyncFieldIndicator (10-32)
src/utils/fieldDetection.js (2)
src/actions/publishAndSync.js (1)
  • schemaType (16-16)
src/hooks/useSyncFields.js (1)
  • schemaType (11-11)
src/hooks/useSyncFields.js (1)
src/utils/fieldDetection.js (1)
  • findSyncFields (6-12)
🔇 Additional comments (6)
src/sanityPluginMultiLanguage.js (1)

4-4: Export addition looks good

Re-exporting publishAndSyncAction here keeps the public surface aligned with the new action module. 👍

src/hooks/useSyncFields.js (1)

9-14: Hook implementation is solid

The hook correctly defers to findSyncFields and handles unknown types by returning an empty array. Looks good.

src/components/SyncFieldWrapper.js (1)

9-19: Wrapper composition looks good

The wrapper cleanly preserves the base field (or renderDefault) and appends the indicator—no issues spotted.

src/utils/fieldDetection.js (2)

6-12: LGTM!

The null safety checks and strict equality filtering are appropriate. The function correctly handles edge cases and returns an empty array when no sync fields are found.


19-21: LGTM!

Clean delegation to findSyncFields with appropriate boolean conversion.

src/utils/sync.js (1)

20-32: Error handling is properly in place in the calling action.

The syncToSiblings call at src/actions/publishAndSync.js:31 is wrapped in a try-catch block (lines 26-37). If the transaction fails, the error is caught and rethrown before operation.publish.execute() can run, preventing publishing. The implementation correctly handles transaction failures as intended.

Comment on lines +23 to +40
return {
label: 'Publish & Sync',
onHandle: async () => {
try {
const siblings = await querySiblings(type, draft.translationId, draft.language, client)

if (siblings.length > 0) {
const syncPayload = buildSyncPayload(draft, syncFieldNames)
await syncToSiblings(siblings, syncPayload, client)
}

await operation.publish.execute()
} catch (err) {
throw new Error(`Failed to publish and sync: ${err.message}`)
}
},
disabled: !draft
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Call onComplete so the action finishes cleanly

After the publish executes, the action never calls props.onComplete(), so Studio keeps the action in a “running” state (spinner stays up, button disabled). Please invoke onComplete in a finally block so the UI tidies up even when an error is thrown.

-export function publishAndSyncAction(props) {
-  const { type, id, draft } = props
+export function publishAndSyncAction(props) {
+  const { type, id, draft, onComplete } = props
   const operation = useDocumentOperation(id, type)
   const schema = useSchema()
   const client = useClient(apiVersion)
   const schemaType = schema.get(type)
   const syncFieldNames = schemaType?.options?.kaliber?.syncFieldNames ?? []
 
   if (syncFieldNames.length === 0)
     return null
 
   return {
     label: 'Publish & Sync',
     onHandle: async () => {
       try {
         const siblings = await querySiblings(type, draft.translationId, draft.language, client)
 
         if (siblings.length > 0) {
           const syncPayload = buildSyncPayload(draft, syncFieldNames)
           await syncToSiblings(siblings, syncPayload, client)
         }
 
         await operation.publish.execute()
       } catch (err) {
         throw new Error(`Failed to publish and sync: ${err.message}`)
+      } finally {
+        onComplete?.()
       }
     },
     disabled: !draft
   }
 }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/actions/publishAndSync.js around lines 23 to 40, the action never signals
completion so Studio keeps the action in a running state; add a finally block
after the try/catch that calls props.onComplete() (or checks and calls
onComplete if provided) so the UI spinner/button state is cleared even on error,
leaving the existing error throwing behavior intact.

Comment thread src/addFields.js Outdated
Comment thread src/utils/sibling.js Outdated
Comment on lines +15 to +31
const siblings = await client.fetch(
`*[
_type == $type &&
translationId == $translationId &&
language != $currentLanguage &&
!(_id in path("drafts.**"))
]`,
{
type: documentType,
translationId,
currentLanguage
}
)

return siblings || []
} catch (error) {
throw new Error(`Failed to query sibling translations: ${error.message}`)
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Nov 3, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Include sibling drafts in the sync query

Line 20 filters out every draft (!(_id in path("drafts.**"))), so we only patch published siblings. If a translator has an open draft, it keeps the old values; when they eventually publish, their outdated draft overwrites the freshly synced data. That breaks the “keep fields identical” guarantee. Please fetch both published and draft siblings (and patch both) so synced values survive future publishes.

-        language != $currentLanguage &&
-        !(_id in path("drafts.**"))
+        language != $currentLanguage
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const siblings = await client.fetch(
`*[
_type == $type &&
translationId == $translationId &&
language != $currentLanguage &&
!(_id in path("drafts.**"))
]`,
{
type: documentType,
translationId,
currentLanguage
}
)
return siblings || []
} catch (error) {
throw new Error(`Failed to query sibling translations: ${error.message}`)
const siblings = await client.fetch(
`*[
_type == $type &&
translationId == $translationId &&
language != $currentLanguage
]`,
{
type: documentType,
translationId,
currentLanguage
}
)
return siblings || []
} catch (error) {
throw new Error(`Failed to query sibling translations: ${error.message}`)
🤖 Prompt for AI Agents
In src/utils/sibling.js around lines 15 to 31, the query currently excludes
draft documents via !(_id in path("drafts.**")), so only published siblings are
returned and patched; remove that draft-exclusion filter so the query returns
both published and draft siblings, then ensure subsequent patch logic applies
updates to all returned documents (including those with ids like "drafts.<id>")
so draft copies are kept in sync and won't overwrite fresh values on publish.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Dit is niet eens zo'n gekke suggestie eigenlijk, wat denken jullie?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@nhoenderop, thank you for considering this! The risk is subtle but real: imagine a translator opens a draft to fix a typo in a different field, then days later you "Publish & sync" from another language. The translator's draft still has the old values in the synced fields. When they eventually publish their typo fix, those stale values overwrite your fresh sync, silently breaking synchronization.

Including drafts ensures that both the published document and any pending draft get updated together, so the next publish won't revert the sync.

Happy to discuss further with the team!

/)/)

(^.^)
(")(")
Thinking about drafts... 🤔

Comment thread src/utils/sync.js Outdated
Comment on lines +7 to +11
export function buildSyncPayload(document, syncFieldNames) {
return Object.fromEntries(
syncFieldNames.map(fieldName => [fieldName, document[fieldName]])
)
}
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Nov 3, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Filter out undefined field values to prevent unintended data clearing.

If document[fieldName] is undefined, the payload will include that field with an undefined value. When applied via patch.set(), this may unintentionally clear the field in sibling documents, potentially causing data loss.

Apply this diff to filter out undefined values:

 export function buildSyncPayload(document, syncFieldNames) {
-  return Object.fromEntries(
-    syncFieldNames.map(fieldName => [fieldName, document[fieldName]])
-  )
+  const entries = syncFieldNames
+    .map(fieldName => [fieldName, document[fieldName]])
+    .filter(([, value]) => value !== undefined)
+  return Object.fromEntries(entries)
 }
🤖 Prompt for AI Agents
In src/utils/sync.js around lines 7 to 11, the buildSyncPayload function
currently includes fields with undefined values which can cause unintended
clearing when applied; update the implementation to skip any fieldNames whose
document[fieldName] is undefined so the returned object only contains keys with
defined values (e.g., map or reduce/filter to omit undefined entries), ensuring
patch.set() won't clear sibling document fields.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Als hij is verwijderd lijkt me dat juist wenselijk

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@nhoenderop, you're absolutely right! If a field is removed from one translation, it makes sense to propagate that deletion to all sibling translations to keep them in sync. The current implementation correctly includes undefined values to ensure deletions are synchronized across translations.

I'll note this design decision for future reviews. Thank you for the clarification!


✏️ Learnings added
Learnt from: nhoenderop
Repo: kaliberjs/sanity-plugin-multi-language PR: 26
File: src/utils/sync.js:7-11
Timestamp: 2025-11-03T16:27:28.868Z
Learning: In the sanity-plugin-multi-language sync feature (src/utils/sync.js), the buildSyncPayload function intentionally includes fields with undefined values. This allows deletions to be propagated across translations, which is the desired behavior for keeping translations synchronized.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Comment thread src/actions/publishAndSync.js
Comment thread src/actions/publishAndSync.js Outdated
Comment thread src/addFields.js Outdated
}

const syncFieldNames = (type.fields ?? [])
.filter(f => f.syncAcrossTranslations)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Het voelt voor mij nog wel spannend om properties aan velden toe te voegen op root niveau. Misschien is het dan fijner om het zo te doen

{
  title: 'Genre',
  name: 'genre',
  type: 'string',
  options: {
    kaliber: {
      syncAcrossTranslations: true,
    }
  }
}

Comment thread src/components/SyncFieldIndicator.js Outdated
Comment on lines +11 to +13
if (!isSyncField) {
return null
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ik zou deze hier uithalen, en de check toevoegen op de plek waar je het component aanroept

{isSyncField && <SyncFieldIndicator />}

Comment thread src/components/SyncFieldIndicator.js Outdated
portal
>
<Box display="inline-block" paddingLeft={2} style={{ fontSize: '20px' }}>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Kunnen we hier eventueel een officiele sanity-icon voor gebruiken? https://icons.sanity.build/all?scheme=dark&value=eJyrrgUAAXUA+Q==

Comment on lines +11 to +18
return (
<div style={{ display: 'flex', alignItems: 'center', gap: '8px' }}>
<div style={{ flex: 1 }}>
{BaseFieldComponent ? <BaseFieldComponent {...props} /> : props.renderDefault(props)}
</div>
<SyncFieldIndicator isSyncField={true} />
</div>
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
return (
<div style={{ display: 'flex', alignItems: 'center', gap: '8px' }}>
<div style={{ flex: 1 }}>
{BaseFieldComponent ? <BaseFieldComponent {...props} /> : props.renderDefault(props)}
</div>
<SyncFieldIndicator isSyncField={true} />
</div>
)
return BaseFieldComponent ? (
<div style={{ display: 'flex', alignItems: 'center', gap: '8px' }}>
<div style={{ flex: 1 }}>
<BaseFieldComponent {...props} />
</div>
<SyncFieldIndicator isSyncField={true} />
</div>
) : props.renderDefault(props)

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (2)
src/actions/publishAndSync.js (2)

15-33: Call onComplete so the action finishes cleanly.

This issue has already been flagged in a previous review. After the publish executes, the action never calls props.onComplete(), leaving the Studio UI in a "running" state.


50-62: Draft siblings excluded from sync may cause divergence.

The GROQ query on line 55 excludes draft documents with !(_id in path("drafts.**")). This means if a sibling translation has a draft version, only its published version receives the synced fields. When that sibling's draft is later published, it will overwrite the synced values with its own draft content, causing the translations to diverge.

Consider whether drafts should also be synced, or document this limitation clearly:

 const siblings = await sanityClient.fetch(
   `*[
     _type == $type &&
     translationId == $translationId &&
-    language != $currentLanguage &&
-    !(_id in path("drafts.**"))
+    language != $currentLanguage
   ]`,

Alternatively, if excluding drafts is intentional, add a comment explaining the trade-off.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8b65b30 and fa3b6b0.

📒 Files selected for processing (1)
  • src/actions/publishAndSync.js (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: nhoenderop
Repo: kaliberjs/sanity-plugin-multi-language PR: 26
File: src/utils/sync.js:7-11
Timestamp: 2025-11-03T16:27:28.868Z
Learning: In the sanity-plugin-multi-language sync feature (src/utils/sync.js), the buildSyncPayload function intentionally includes fields with undefined values. This allows deletions to be propagated across translations, which is the desired behavior for keeping translations synchronized.
📚 Learning: 2025-11-03T16:27:28.868Z
Learnt from: nhoenderop
Repo: kaliberjs/sanity-plugin-multi-language PR: 26
File: src/utils/sync.js:7-11
Timestamp: 2025-11-03T16:27:28.868Z
Learning: In the sanity-plugin-multi-language sync feature (src/utils/sync.js), the buildSyncPayload function intentionally includes fields with undefined values. This allows deletions to be propagated across translations, which is the desired behavior for keeping translations synchronized.

Applied to files:

  • src/actions/publishAndSync.js
🧬 Code graph analysis (1)
src/actions/publishAndSync.js (1)
src/components/Translations.js (1)
  • translationId (33-33)
🔇 Additional comments (3)
src/actions/publishAndSync.js (3)

19-19: Verify draft properties exist before accessing.

Line 19 accesses draft.translationId and draft.language without checking whether these properties exist on the draft object. While querySiblings handles a missing translationId by returning an empty array, passing undefined for language could lead to unexpected query behavior.

Consider adding validation:

 onHandle: async () => {
   try {
+    if (!draft.translationId || !draft.language) {
+      throw new Error('Draft is missing required translation fields')
+    }
     const siblings = await querySiblings(type, draft.translationId, draft.language, client)

76-80: Clean implementation that correctly handles field deletions.

The function appropriately includes fields even when their values are undefined, allowing deletions to propagate across translations.

Based on learnings.


89-100: Revise transaction limit concern—Sanity's actual constraint is request body size, not operation count.

The stated "100 operations" limit does not appear in Sanity's official documentation. Sanity enforces a 4MB request body size limit, not an operation count limit. For typical translation systems with 10-50 sibling languages and selective field syncing, the payload is unlikely to exceed this. However, if your codebase supports many languages or large sync payloads, batching may be prudent to avoid hitting the 4MB request body limit. Verify the expected sibling count and typical payload size for your use case.

await syncToSiblings(siblings, syncPayload, client)
}

await operation.publish.execute()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify that the publish operation is available before executing.

The code calls operation.publish.execute() without checking whether the publish operation exists or is disabled. While useDocumentOperation should always return an operation object, the publish action itself might be unavailable in certain states.


🏁 Script executed:

#!/bin/bash
# Check if other usages of operation.publish in the codebase check for availability
rg -nP --type=js -C3 'operation\.publish\.execute\(\)'

Length of output: 445


I need to gather more context to verify this concern. Let me check the full function and search for other patterns.


---

🏁 Script executed:

```shell
#!/bin/bash
# Get full function context for publishAndSync - from beginning to understand operation initialization
cat src/actions/publishAndSync.js | head -n 35

Length of output: 1012


🏁 Script executed:

#!/bin/bash
# Search for all operation.publish usages (not just .execute)
rg -nP --type=js 'operation\.publish' -C2

Length of output: 309


🏁 Script executed:

#!/bin/bash
# Search for useDocumentOperation hook to understand what it returns
rg -nP --type=js 'useDocumentOperation' -C3

Length of output: 636


Let me verify the Sanity library behavior to determine if the publish operation can actually be unavailable:


🌐 Web query:

Sanity useDocumentOperation publish operation availability disabled states

💡 Result:

Short answer — useDocumentOperation's publish operation exposes a disabled flag that you should treat as either a boolean or a reason token. Common availability/disabled cases you’ll see in Studio code and examples:

  • publish.disabled true/false — simple boolean disable state. [1]
  • publish.disabled can be a reason token (string) that callers use to show a localized tooltip/title (examples in Studio code show reason tokens such as live-edit enabled / not-ready; the action UI maps those tokens to a human message). [2][3]
  • Other runtime cases that make the publish action unavailable (observed in Studio code and examples):
    • Live edit mode is enabled (publish/unpublish hidden or disabled). [2]
    • Validation errors on the draft (prevent publish). [1][3]
    • A publish is already scheduled or currently running (button disabled while publishing/scheduled). [1]
    • Insufficient permissions (publish disabled until permission granted). [2]
    • Edit transaction locks / syncing state while client is syncing (temporarily disabled). [1]

How to handle in a custom action:

  • Read publish.disabled and use it for your button’s disabled state and title/tooltip.
  • If you need to show progress, track a local "isPublishing" state and watch for draft -> null to detect completion (current Studio pattern). [3]

Sources

  • Sanity docs — Document actions / useDocumentOperation (operations expose disabled + execute). [1]
  • Sanity Studio source/examples (publish/unpublish action handling and disabled reason usage). [2][3]

Check the publish operation's disabled state before attempting to execute.

The Sanity useDocumentOperation hook returns publish with a disabled property that can be a boolean or a reason token string. Multiple scenarios disable publish: live edit mode, validation errors on draft, publish already in progress, insufficient permissions, or syncing state.

Your code calls operation.publish.execute() at line 27 without checking operation.publish.disabled, which could fail silently or throw an unhandled error in any of those scenarios. Add a guard before executing:

if (!operation.publish.disabled) {
  await operation.publish.execute()
}

Alternatively, return early or show a user message using the disabled reason token.

🤖 Prompt for AI Agents
In src/actions/publishAndSync.js around line 27, the code calls
operation.publish.execute() without checking operation.publish.disabled; add a
guard that checks operation.publish.disabled (it may be boolean or a reason
token) and only call execute when it is falsy, or return early / surface the
disabled reason to the user instead of attempting to execute when disabled.

const transaction = sanityClient.transaction()

siblings.forEach(sibling => {
transaction.patch(sibling._id, patch => patch.set(payload))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Shallow set operation may lose nested field data.

Using patch.set(payload) performs a shallow overwrite. If any synchronized field contains a nested object, the entire nested object is replaced rather than merged. For example, if the payload is { author: { name: "John" } } and a sibling has { author: { name: "Jane", email: "jane@example.com" } }, the email property will be lost after sync.

If synced fields can contain nested objects, consider whether a deep merge is needed, or clearly document that only top-level field replacement is supported.

🤖 Prompt for AI Agents
In src/actions/publishAndSync.js around line 96, the current
transaction.patch(sibling._id, patch => patch.set(payload)) does a shallow
overwrite and will drop nested properties; instead, load the sibling's existing
document fields, perform a deep merge of payload into those existing field
objects (using a utility like lodash.merge or a small recursive merge helper),
then call patch.set(mergedFields) (or set individual top-level keys to the
merged values) so nested properties are preserved during sync.

@nhoenderop nhoenderop requested a review from daveykropf November 4, 2025 12:39
@EECOLOR
Copy link
Copy Markdown
Member

EECOLOR commented Nov 4, 2025

Cool!

Ik zou nog wel even nadenken over de acties. In mijn hoofd is het onhandig om sync & publish te hebben wanneer ik alleen wil syncen. Daarnaast is het ook een beetje gevaarlijk wanneer een document waar je niet in werkt gepubliceerd wordt.

Ik zou em dus wat uitkleden en iets van sync fields to other translations te doen. Wat mij betreft worden aanpassingen alleen in drafts gedaan en publiceren wij niets. Het is te gemakkelijk om dan buiten de validatie om te gaan en dingen corrupt te maken.

Wat ik nog mis is: waar komt deze wens vandaan?

Het is nu een knop die alle velden doet, maar kan me ook voorstellen dat het wrapper component zelf gaat synchroniseren bij wijzigingen zodat het op veld niveau gebeurt. Dan heb je ook iets aan je sync indicator :-D Mijn vorige vraag is daarin waarschijnlijk belangrijk. Pas als we weten wat de onderliggende use-case is kunnen we kijken welke kant we op willen.

@nhoenderop
Copy link
Copy Markdown
Author

Check, publiceren en syncen lostrekken. Een sync moet niet een publish vereisen.
Check die de aanname dat het veld waar je naar verwijst in een sync veld geen vertaalbaar iets is

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