Phase 5: REST permissions + PHP tests for suggestions#77407
Closed
adamsilverstein wants to merge 69 commits into
Closed
Phase 5: REST permissions + PHP tests for suggestions#77407adamsilverstein wants to merge 69 commits into
adamsilverstein wants to merge 69 commits into
Conversation
When clicking "Add Note" on a block that already has an existing note,
the user was being redirected to the reply field of the existing note.
This prevented users from creating multiple independent note threads
on the same block.
This change:
- Adds an `addNewNote` parameter to `openTheSidebar()` to differentiate
between clicking "Add Note" (should always create new thread) and
clicking the avatar indicator (should focus existing thread)
- Updates `AddCommentMenuItem` to pass `{ addNewNote: true }`
- Adds E2E tests to verify multiple notes can be added to the same block
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
getNoteIdsFromMetadata is a trivial pure function that normalizes a scalar/array value. The overhead of useMemo exceeds the cost of calling it directly each render. Addresses review feedback from @Mamaduka. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
Use direct property access for noteId instead of extracting metadata into a separate variable. Addresses review feedback from @Mamaduka. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
Clarify why the blur handler needs to bail out during submission: clicking the submit button triggers blur before the async onSubmit completes, which would otherwise close the form prematurely. Addresses review feedback from @Mamaduka. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
This reverts commit 7d2ff81.
Prevent blur from closing the add-note form while the async submit is in progress by guarding with isSubmittingRef. Also stop the auto-select effect from overwriting the "new" note selection, and use a ref for noteThreads so the effect only runs when blockNoteIds changes, not on every thread update.
The first "Note added." snackbar persists when adding a second note, causing a strict mode violation (2 elements) and a premature metadata assertion. Dismissing the first snackbar before adding the second note fixes both tests.
Extend E2E tests with scenarios for deleting one of multiple notes, resolving individual notes, auto-selecting the first unresolved note, and verifying metadata cleanup when all notes are removed. Add unit test edge cases for falsy noteId values, string coercion, and null/undefined metadata handling.
adamsilverstein
changed the title
This was referenced Apr 16, 2026
|
Size Change: +2.48 kB (+0.03%) Total Size: 7.92 MB 📦 View Changed
ℹ️ View Unchanged
|
adamsilverstein
force-pushed
the
add-suggestion-mode
branch
from
f2ea085 to
476c8eb
Compare
adamsilverstein
force-pushed
the
suggest-mode-phase-4
branch
2 times, most recently
from
37bd62c to
8111615
Compare
adamsilverstein
force-pushed
the
add-suggestion-mode
branch
3 times, most recently
from
ffd4a57 to
218f599
Compare
The auto-save's `syncOnce` decided between updating an existing note and creating a new one only by whether the in-memory overlay had a `commentId`. When a collaborator accepted or rejected the linked note mid-session, the overlay's reference outlived the open note: the next edit would `updateSuggestion` against the now-resolved comment, clobbering its payload while the header stayed "Applied" / "Rejected". Check the linked comment's status before taking the update branch. If it's no longer `hold`, drop the stale reference and fall through to `createSuggestion` so the new edit lands on a fresh note that coexists with the resolved one — possible since #75147 lets a block carry an array of note ids in `metadata.noteId`.
…uggestion-mode # Conflicts: # packages/editor/src/components/suggestion-mode/provider.js
The trunk react-hooks plugin re-enable (#69962) and per-file suppression sweep flag pre-existing violations in suggestion-mode files and on grid/resize-handle.tsx that this branch has not yet picked up from trunk. Add per-file entries to the suppressions list so CI lint passes here without rewriting the underlying code. Refs and immutability fixes for these files belong with the broader hooks migration, not in this REST-permissions PR.
# Conflicts: # docs/reference-guides/data/data-core-editor.md # packages/editor/src/components/suggestion-mode/provider.js # packages/editor/src/store/actions.js
Add module headers explaining the auto-save debounce/queue discipline, the SuggestionSummary line-category model (Add/Delete/Format/Formatting), and the multi-noteId migration rationale (PR #75147 widened metadata.noteId from a scalar to an array). Expand the PHPUnit test file header to enumerate coverage areas and call out the meta-registration quirk in the test setUp. Update the architecture doc's implementation files table to drop the removed commit-bar.js and add suggestion-summary and auto-save.
This was referenced May 4, 2026
The auto-select effect in collab-sidebar/notes.js depended on selectedNote, so when the user collapsed a thread (Escape, Cancel, ArrowLeft, etc.) and selectNote(undefined) ran, the effect re-fired and immediately re-selected the same targetNoteId — making it impossible to keep a thread collapsed while its block stayed selected. Read selectedNote via a ref so the bailouts (in-progress new note, explicit pick on the same block) still work, and depend only on the block context (targetNoteId, blockNoteIds). The effect now fires for block changes and note metadata changes but ignores user-driven selectedNote toggles. Fixes the Playwright-3 regressions in block-notes.spec.js on the add-suggestion-mode stack: - can resolve and reopen a block note - should expand or collapse a note with enter / arrow keys - should collapse a note with Escape key - should collapse a note after canceling note form - should collapse a note when the focus moves outside the note
adamsilverstein
requested review from
Mamaduka and
t-hamano
and removed request for
ajitbohra,
fabiankaegy,
juanmaguitar and
spacedmonkey
Move the SuggestionAutoSave subsystem (auto-save.js, its test, and the overlay/provider plumbing it required — updateSuggestion, deleteSuggestion, setCommentId, setSyncedOpsKey, and the corresponding reducer cases) out of this PR so the Phase 5 diff stays focused on REST permissions and PHP tests. Restore the SuggestionCommitBar toolbar button as the submit affordance until the auto-save follow-up lands.
Member
Author
|
Closing this PR in favor of three focused replacements:
Splitting because this PR had grown to 33 files / +2,699 / −265 across the REST backend, the suggestion-mode component, the collab-sidebar UI, the architecture docs, the editor store, and three e2e specs — under a title that only named the backend work. The new stack is reviewable slice-by-slice: Combined, the new stack is byte-identical to this branch (verified The |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
This is one of a stack of PRs implementing Suggest mode for the WordPress editor — a Google Docs–style workflow where reviewers can propose changes that the post author can Accept or Reject. Tracked in #73411, with design direction from #73410 and jasmussen's mockups.
Closes #73411
This PR (Phase 5)
Focuses on the REST permissions + PHP tests required for the suggestion lifecycle:
class-gutenberg-rest-comment-controller-6-9.php) — overridesupdate_item_permissions_checkso users withedit_poston the parent can updatenotecomments, but only for suggestion-lifecycle fields (statuslimited toapproved/hold, plusmeta._wp_suggestion_status). Any other field in the update body falls back to core'sedit_commentcheck, preventing post editors from rewriting another user's note content.auth_callbacks —_wp_suggestionand_wp_suggestion_statusboth authorize onedit_postagainst the parent so editors can toggle status without needingedit_commenton someone else's note._wp_suggestionpayload, enforced at the controller (with a matching client-side pre-flight check).editor.BlockListBlockfilter tags any block with a pending overlay with anis-suggestion-pendingclass, which paints a green outline + corner brackets.applySuggestionfalls back to scanning the live block tree for a block whosemetadata.noteIdmatches the comment id, covering the case where the Suggest author's post-save hasn't landed yet.WP_Test_REST_Comments_Controller_Gutenbergcovers meta round-trip, editor-can-apply, contributor-cannot-apply, restricted-field pass-through, and payload-size truncation.Scope reductions vs prior revisions
Three feature areas have been split into separate PRs so this one stays focused on REST/permissions:
SuggestionCommitBartoolbar button.Stack base
This branch is merged on top of #75147 (Notes: Support multiple note threads per block). Multiple-notes-per-block is a hard prerequisite for the per-block suggestion lifecycle — without it, accepting a suggestion irreversibly resolves the only thread the block has, so a follow-up edit cannot surface a new Accept/Reject affordance. Land #75147 before merging this PR.
What's NOT in this PR
Phase 5 test plan
REST permissions
edit_post:PATCH /wp/v2/comments/<id>with{ status: 'approved', meta: { _wp_suggestion_status: 'applied' } }— succeeds (200).PATCH— fails (403).PATCHwith{ content: 'rewritten' }— falls back to coreedit_commentcheck (Contributor 403, Editor 200 only if they own the note).PATCHwith{ status: 'spam' }— falls back to coreedit_comment(status outside the suggestion-lifecycle allowlist)._wp_suggestionmeta payload exceeding 64 KB — server rejects with a 400.Automated tests
```bash
PHP (requires wp-env running)
npm run wp-env run cli -- --env-cwd='wp-content/plugins/gutenberg' vendor/bin/phpunit --filter=WP_Test_REST_Comments_Controller_Gutenberg
PHP lint
vendor/bin/phpcs lib/compat/wordpress-6.9/class-gutenberg-rest-comment-controller-6-9.php lib/compat/wordpress-6.9/block-comments.php
```
E2E
```bash
npm run test:e2e -- test/e2e/specs/editor/various/suggestion-mode.spec.js
npm run test:e2e -- test/e2e/specs/editor/various/editor-intent-switcher.spec.js
```
Phase 5 checklist
PATCHa note's status toapprovedand toggle_wp_suggestion_statusedit_commentcheck🗺️ PR Stack Navigation
📋 Tracking issue: #73411