Make WordPress Core

Opened 9 months ago

Closed 9 months ago

Last modified 9 months ago

#58839 closed enhancement (fixed)

Editor: Add test coverage for context setting in Comment Template block

Reported by: bernhard-reiter's profile Bernhard Reiter Owned by: bernhard-reiter's profile Bernhard Reiter
Milestone: 6.3 Priority: low
Severity: minor Version:
Component: Editor Keywords: has-patch has-unit-tests commit
Focuses: Cc:

Description

In https://github.com/WordPress/gutenberg/pull/50279, the way that commentId context is set for the Comment Template block was changed: Previously, that context was passed directly as an argument to the WP_Block constructor, whereas since then, it has been set via the render_block_context filter. (This was done so that other potential blocks that are inserted programmatically as inner blocks into the Comment Template block would have access to that same block context, which is crucial for their functioning.) This change was later amended by https://github.com/WordPress/gutenberg/pull/50883.

Since the relevant code is a dynamic block's PHP code, it was eventually synced to Core (via the @wordpress/block-library npm) for WP 6.3 Beta 1.

Test coverage for this new behavior was added to Gutenberg in https://github.com/WordPress/gutenberg/pull/50879, as it became apparent during discussion that it's easily broken.

The block code was amended once again in https://github.com/WordPress/gutenberg/pull/52364, which was then synced to Core (WP 6.3 Beta 4).

In order to guard Core against regressions of the Comment Template block, we should carry over test coverage from https://github.com/WordPress/gutenberg/pull/50879 (and potentially from https://github.com/WordPress/gutenberg/pull/50883, which is related).

Change History (7)

@Bernhard Reiter commented on PR #4507:


9 months ago
#2

Took me a while, but I’ve filed the ticket. I've also addressed everyone's feedback (thank you!)

We're pretty close to RC1, so I'm not sure if folks are comfortable merging this so late in the game. OTOH, it's a test-only change that's supposed to guard against regressions 🤔

@Bernhard Reiter commented on PR #4507:


9 months ago
#3

Added some more coverage in d3e3ffd880 to cover the scenario fixed by https://github.com/WordPress/gutenberg/pull/52364.

cc/ @dlh01

#4 @Bernhard Reiter
9 months ago

  • Milestone changed from Awaiting Review to 6.3
  • Owner set to Bernhard Reiter
  • Status changed from new to accepted

I'll set the Milestone to 6.3 for folks to decide if it's fine to include it in that version.

Quoting https://github.com/WordPress/wordpress-develop/pull/4507#issuecomment-1640361068:

We're pretty close to RC1, so I'm not sure if folks are comfortable merging this so late in
the game. OTOH, it's a test-only change that's supposed to guard against regressions

#5 @Bernhard Reiter
9 months ago

  • Keywords commit added

#6 @Bernhard Reiter
9 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 56262:

Editor: Add test for context setting in Comment Template block.

Test that commentId context is correctly set and made available by the Comment Template block to the render_block_context filter (at priority 2 and higher), and to the render_block filter (important when programmatically inserting child blocks into the Comment Template block).

Furthermore, test that child blocks inserted via the render_block_data filter are retained and thus present at render_block filter stage.

Props andrewserong, ramonopoly, peterwilsoncc, costdev, mukesh27, flixos90.
Fixes #58839.

@Bernhard Reiter commented on PR #4507:


9 months ago
#7

Thanks all! Committed to Core in https://core.trac.wordpress.org/changeset/56262 😊

Note: See TracTickets for help on using tickets.