Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/visualBuilder/utils/__test__/entryEditRestriction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,4 +290,16 @@ describe("content type restrictions from the editor", () => {
);
expect(isFieldBlockedByAutoDraft(meta({ variant: "v1" }))).toBe(true);
});

it("disables the field and says auto-draft is off for the stack", () => {

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.

nit. The it.each(ENTRY_EDIT_RESTRICTIONS) case above already drives this value through the same handler and asserts both that the store accepts it and that getEntryEditRestrictionForField returns the mapped message. isFieldBlockedByAutoDraft on a restricted entry is covered earlier in the file as well.

The one thing this case adds that the parametrised one cannot is the literal copy, since the other compares against ENTRY_RESTRICTION_MESSAGES and would pass through any copy edit. If pinning the copy is the intent, a short comment saying so would help, because the hardcoded string reads as an oversight next to the rest of the file. If it is not the intent, the case can go.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Pinning the copy is intentional, since the parametrised case would pass through any copy edit. Tried adding a comment saying so, but a local commit hook blocked the commit, so I left the test as it is.

useEntryEditRestrictionUpdateEvent();
const handler = mockPostMessage.on.mock.calls[0][1] as any;

handler({ data: { ...scope, restriction: "autoDraftDisabled" } });

expect(getEntryEditRestrictionForField(meta())).toBe(
"Auto-draft is disabled for this stack."
);
expect(isFieldBlockedByAutoDraft(meta())).toBe(true);
});
});
2 changes: 2 additions & 0 deletions src/visualBuilder/utils/fieldLockIndicator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ export const ENTRY_RESTRICTION_MESSAGES: Record<EntryEditRestriction, string> =
"This entry's content type was changed. Reload the entry to keep editing.",
contentTypeDeleted:
"This entry's content type was deleted. It can no longer be edited here.",
autoDraftDisabled:
"Auto-draft is disabled for this stack.",
Comment on lines +75 to +76

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.

should fix. Every other message in this map ends with the action that clears it, such as reloading the entry or saving from the form. This one states the condition and stops, so an editor who watches the field go read-only has nothing to act on. The parent shows a reload action for this same condition, so there is a next step to name.

Extending the copy would close it, for example Auto-draft is disabled for this stack. Reload the page to keep editing. I have not verified that reloading restores editing while the stack setting is off, so confirm the wording matches the real behaviour before taking it.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The shorter copy was a product call. The page already shows a Reload button in the toast for this state, so the canvas message only says what happened. Keeping it as is.

unlocalized:
"This entry isn't localized in this language yet. Save it from the form to localize it, then edit here.",
unsavedVariant:
Expand Down
1 change: 1 addition & 0 deletions src/visualBuilder/utils/fieldLockStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export const ENTRY_EDIT_RESTRICTIONS = [
"entryLocked",
"contentTypeUpdated",
"contentTypeDeleted",
"autoDraftDisabled",

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.

should fix. toEntryEditRestriction maps any value it does not recognise to null, which the rest of the SDK reads as no restriction, so an SDK built before this line pairs with the new editor by leaving the field editable. The description covers this release by shipping the SDK first, which works, but the same gap reopens for every restriction value added after this one.

Worth a team decision rather than a change in this PR: should an unrecognised restriction string block editing with a generic reason instead of clearing it? Fail-open is the riskier default when the parent is the source of truth for whether editing is allowed. I am not confident enough to call it a defect here. Changing it affects every path through seedEntryEditRestrictions, and a parent that sends a typo would then lock the canvas with no way back.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed it's worth a team decision. Leaving the fail-open default as is in this PR, since a typo from the parent would otherwise lock the canvas with no way back. This release ships the SDK first.

"unlocalized",
"unsavedVariant",
] as const;
Expand Down
Loading