Skip to content

HAR-9261 - Comment permissions fixes, dialog fixes#392

Merged
edoversb merged 1 commit into
mainfrom
nick/har-9261-comments-permissions-fixes
Mar 19, 2025
Merged

HAR-9261 - Comment permissions fixes, dialog fixes#392
edoversb merged 1 commit into
mainfrom
nick/har-9261-comments-permissions-fixes

Conversation

@harbournick

Copy link
Copy Markdown
Collaborator
  • Fix missing comment header in external comments
  • Allow overflow in child comments
  • Fix permissions for edit/delete comment with internal/external
  • Fixes comment deletion

…ild comments, fix permissions for edit/delete internal/external

@edoversb edoversb left a comment

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.

LGTM

@edoversb edoversb merged commit 0a8018a into main Mar 19, 2025
@edoversb edoversb deleted the nick/har-9261-comments-permissions-fixes branch March 19, 2025 18:28
superdoc-bot Bot pushed a commit that referenced this pull request Jul 9, 2026
…#392)

* fix(super-editor): sync custom xml writes through part-sync

- metadata.* and customXml.parts.* writes routed through the parts
  mutation layer so partChanged fires and part-sync publishes
- a shared sandbox-and-diff wrapper commits every package path a custom
  xml create/update/remove touches (item, itemProps, item rels, document
  rels), not just the item xml
- two-client regression tests cover metadata attach and customXml
  create/patch/remove, including the remote client's converted xml

* fix(super-editor): record custom xml tombstones on remote part-sync deletes

Remote part deletions removed the part from convertedXml but never
populated removedCustomXmlPaths, so a collaborator whose original docx
contained the part re-emitted it on export. The consumer now records
tombstones for deleted custom xml storage parts, props parts, and item
rels (including rels-linked props paths), and clears stale tombstones
when a part is re-created remotely.

* fix(super-editor): tombstone absent custom xml parts on late-join and clear bibliography cache

- late joiners prune local custom xml parts absent from an authoritative
  parts map (a peer deleted them before this client joined), recording
  export tombstones through the same mutateParts mechanism hydration uses
- tombstoning a path now also invalidates converter caches, so deleting
  the bibliography storage part clears converter.bibliographyPart and
  export cannot resurrect it

* fix(super-editor): record hydration tombstones only after the prune mutation commits

Tombstones (and the bibliography cache clear) were applied before
mutateParts ran; a mutation failure rolled back the part store but left
the converter tombstoned. The prune now collects paths and records them
only after the mutation succeeds.

* fix(super-editor): clear stale custom xml tombstones during hydration

A client that tombstoned a custom xml part (e.g. deleted while offline)
and then rehydrated from an authoritative map containing the recreated
part kept the stale tombstone, so export dropped the recreated part.
Hydration now clears tombstones for present custom xml paths at the same
post-mutation point where absent paths are tombstoned - symmetric to the
live consumer path.

* fix(super-editor): invalidate bibliography cache on remote custom xml writes

Remote writes and hydration cleared stale tombstones but left the
bibliography cache pointing at the old content, so export rebuilt the
part from the stale cache over the collaborator's version. Written and
present custom xml paths now invalidate converter caches, and the part
path predicates agree on casing.

* fix(super-editor): clear hydration tombstones only for parts that hydrated

A key present in the authoritative map whose envelope failed to decode
never lands in convertedXml; clearing its tombstone would let export
copy the stale original zip entry through instead of dropping the part.

* ci: retrigger checks on current head

* fix(super-editor): scope hydration tombstone clearing to hydrated parts and make converter-state application rollback-safe

- hydration only clears a custom xml tombstone when the part actually
  hydrated this pass (a decode+op succeeded), not merely because a stale
  local copy is present; a failed-to-hydrate authoritative part no longer
  resurrects the stale local one on export
- applyConverterState is atomic and runs only after the part-store
  mutation durably commits, so a compound rollback cannot leave
  removedCustomXmlPaths / bibliographyPart mutated while the store is
  restored

* fix: address PR review comments on #392

* fix(super-editor): validate metadata payload before mutating on attach

metadata.attach serialized the payload only in the live path, inside the
part-sync sandbox that runs after the anchor is inserted. A non-serializable
payload (a BigInt or a cyclic object, neither of which the payload type can
rule out) therefore threw while building the envelope, after wrapRangeInAnchor
had already dispatched the anchor transaction, leaving an inline content
control with no matching customXml payload.

Build the envelope during the attach preview as well, so serialization fails
before the operation touches the document. Adds a regression test asserting a
cyclic payload throws without dispatching an anchor transaction.

* refactor(super-editor): serialize only the new entry in the attach preview

The attach preview rebuilt the whole metadata envelope to validate one payload,
re-serializing every existing entry in the namespace before the live path
serialized them again. Serialize just the new entry — existing entries were
already validated when they were written — so the preview stays a fail-fast
check for non-serializable payloads without the per-attach serialization cost.

* test(super-editor): fix concurrent-seed race in collaborator export test

The two-client export test created both collaborative editors on unsynced
ydocs, so both bootstrapped as a new room and both seeded the baseline
package into their parts maps as concurrent Yjs writes. The map-key conflict
on word/_rels/document.xml.rels is resolved by clientID, and with the wrong
random winner the second client's baseline seed discarded the first client's
published rels update, dropping the customXml document relationship from the
export (~50% failure rate; deterministically reproducible by pinning
clientIDs). Real clients receive room state before bootstrapping, see the
capability marker, and hydrate instead of seeding, so the conflict is an
artifact of the test's join order.

Create the second editor only after syncing the first editor's seeded state,
matching the real join flow, and wait for the received document rels content
(the baseline part always exists, so an existence check passed before the
remote update landed).

Ported-From-Source-Repo: superdoc/orbit
Ported-From-Source-Commit: 13119b74e8785850867256c4a8fffb06b41e73c3
Ported-Public-Prefix: superdoc/public
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants