Skip to content

fix: remove stale Note references from cloned posts - #550

Open
faisalahammad wants to merge 1 commit into
Yoast:trunkfrom
faisalahammad:fix/420-strip-note-metadata
Open

faisalahammad wants to merge 1 commit into
Yoast:trunkfrom
faisalahammad:fix/420-strip-note-metadata

Conversation

@faisalahammad

Copy link
Copy Markdown

Context

WordPress 6.9 stores the editor Notes as comments with comment_type note, and the block markup keeps a reference to the note in the block metadata.noteId attribute. Notes are not copied when a post is cloned, so the copy kept block references pointing at Notes that only exist on the original post. The editor shows these as broken references on the copy.

Fixes #420

Summary

This PR can be summarized in the following changelog entry:

  • Fixes a bug where a cloned post kept block references to Notes that belonged to the original post.

Relevant technical choices:

  • New Notes_Cleaner class in src/notes-cleaner.php. It collects the note IDs that belong to the duplicated post with get_comments( [ 'type' => 'note' ] ) and removes every other metadata.noteId reference from the block markup, including references inside nested blocks.
  • The cleanup is validation based instead of an unconditional strip. References that point to Notes actually belonging to the copy are kept, so the fix does not conflict with copying Notes (Add Notes as an "element to copy" option #421) and it does not remove references the copy owns.
  • The block markup is walked with parse_blocks() and saved back with serialize_blocks(), so only block metadata is touched and the rest of the content stays as it is.
  • Both the scalar and the array form of metadata.noteId are handled, since newer block markup stores a list of note IDs.
  • The handler is wired into the legacy duplicate_post_after_duplicated hook at priority 46, right after the comments handler at 40 and before taxonomies at 50, so Notes copied to the duplicate (when that option is on) are already in place before the validation runs.
  • Rewrite & Republish is deliberately not touched. Its copy is written back to the original post, where the referenced Notes still exist, so stripping the references there would lose valid Notes.

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

  1. Use WordPress 6.9 or newer and open a post in the block editor.
  2. Select a block and add a Note from the block toolbar.
  3. Clone the post (or use New Draft) from the posts list.
  4. Open the copy in the block editor and check that the block no longer shows a Note reference from the original post, and that the original post still has its Note.
  5. Go back to the original post and check that its Note is unchanged and still attached to the same block.
  6. Repeat step 3 with the Bulk duplicate action on the posts list and check the copies the same way.
  7. Optional, with Add Notes as an "element to copy" option #421 enabled: turn on the Notes option in Settings > Duplicate Post, clone a post with a Note, and check that the copy shows its own Note and not a broken reference.

Relevant test scenarios

  • Changes should be tested with the browser console open
  • Changes should be tested on different posts/pages/taxonomies/custom post types/custom taxonomies
  • Changes should be tested on different editors (Default Block/Gutenberg/Classic/Elementor/other)
  • Changes should be tested on different browsers
  • Changes should be tested on multisite

The check was limited to the block editor on posts and pages. Classic editor posts have no block metadata, so nothing changes for them.

Test instructions for QA when the code is in the RC

  • QA should use the same steps as above.

QA can test this PR by following these steps:

Use the same steps as above.

Impact check

This PR affects the following parts of the plugin, which may require extra testing:

  • The legacy duplication routine, which now runs one extra hook handler on every clone. The handler reads the post content and only writes it back when an invalid reference was found, so posts without Notes are not updated.
  • Posts that have block references to Notes, which is the case the fix targets.

UI changes

  • This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Documentation

  • I have written documentation for this change. For example, comments in the Relevant technical choices, comments in the code, documentation on Confluence / shared Google Drive / Yoast developer portal, or other.

Quality assurance

  • I have tested this code to the best of my abilities
  • I have added unittests to verify the code works as intended

Innovation

  • No innovation project is applicable for this PR.
  • This PR falls under an innovation project. I have attached the innovation label and noted the work hours.

Fixes #420

WordPress 6.9 stores editor Notes as comments with the note type, and
blocks reference them through attrs.metadata.noteId. Comments of the
note type are not copied when a post is cloned, so the copy kept block
markup pointing at Notes that only exist on the original post.

The new Notes_Cleaner service collects the note IDs that belong to the
duplicated post and removes every other noteId reference from the block
markup, including references nested in inner blocks. Valid references
are kept, so the fix works alongside copying Notes.

Rewrite and Republish is left alone, because its copy is written back to
the original post, where the referenced Notes still exist.

The hook runs at priority 46, after comments are copied, so Notes copied
to the duplicate are already in place when the validation runs.

Adds WP integration tests for the clone flow and for the retention of
valid references.

Fixes Yoast#420
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.

Block meta related to the Notes is included in new post

1 participant