Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Query: Require queryId for enhanced pagination to prevent PHP notices. #55720

Merged
merged 1 commit into from
Oct 31, 2023
Merged

Query: Require queryId for enhanced pagination to prevent PHP notices. #55720

merged 1 commit into from
Oct 31, 2023

Conversation

peterwilsoncc
Copy link
Contributor

@peterwilsoncc peterwilsoncc commented Oct 31, 2023 •
edited
Loading

What?

Adds a check to ensure the Query ID is set before rendering enhanced pagination.

Fixes #55719.

Why?

Without a Query ID the enhanced pagination will throw a notice. The Query ID doesn't include a default so can be unset.

How?

Within render_block_core_query() the conditions checking $attributes['enhancedPagination'] are modified to also check isset( $attributes['queryId'] ). If either item is not set then the feature is not enabled.

Testing Instructions

  1. Enable debug mode in your wp-config file: 
    define( 'WP_DEBUG', true );
define( 'WP_DEBUG_LOG', true );
define( 'WP_DEBUG_DISPLAY', true );
  2. Download and activate peters-sample-theme.zip which contains bad markup in templates/index.html
  3. Generate some sample posts via WP CLI: wp post generate
  4. Visit the the front end of the site.
    • on trunk you should see some notices thrown
    • on this branch the notices should not throw
  5. Activate the TT2 theme
  6. Edit the home page to enable advanced pagination on the query loop
  7. Navigate posts, ensure advanced pagination works.

Testing Instructions for Keyboard

N/A

Screenshots or screencast

@peterwilsoncc peterwilsoncc added [Type] Bug An existing feature does not function as intended [Block] Query Loop Affects the Query Loop Block labels Oct 31, 2023
@peterwilsoncc peterwilsoncc force-pushed the fix/55719-prevent-enhanced-pagination-notices branch from cb33d90 to 21480de Compare October 31, 2023 04:29
@peterwilsoncc peterwilsoncc marked this pull request as ready for review October 31, 2023 04:44
@hellofromtonya
Copy link
Contributor

Test Report

This report validates that the indicated patch addresses the issue.

Environment

Actual Results

  • Before the patch, can reproduce the PHP Warnings 🐞❌

  • After applying the patch: no PHP Warnings ✅

    Test Report Icons:
    🐞 <= Indicates where issue ("bug") occurs.
    ✅ <= Behavior is ''expected''.
    ❌ <= Behavior is ''NOT expected''.

hellofromtonya
hellofromtonya approved these changes Oct 31, 2023
View reviewed changes
Copy link
Contributor

@hellofromtonya hellofromtonya left a comment

Choose a reason for hiding this comment

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

See my test report which confirms:

  • can reproduce the PHP Warning before this PR
  • this PR resolves the issue.

@hellofromtonya hellofromtonya added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Oct 31, 2023
@hellofromtonya
Copy link
Contributor

React Native e2e tests (Android):
The failing test is unrelated to this PR, as it seems the service is unavailable. Restarting.

@hellofromtonya
Copy link
Contributor

@SiobhyB @mikachan marking this PR for cherry-picking for 6.4 RC3.

DAreRodz
DAreRodz approved these changes Oct 31, 2023
View reviewed changes
Copy link
Contributor

@DAreRodz DAreRodz left a comment

Choose a reason for hiding this comment

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

Thanks for the fix, @peterwilsoncc! 🙏

The enhanced pagination indeed requires the queryId attribute to be set to generate the data-wp-navigation-id prop, so this change makes total sense.

@DAreRodz DAreRodz merged commit 12694ff into trunk Oct 31, 2023
52 checks passed
@DAreRodz DAreRodz deleted the fix/55719-prevent-enhanced-pagination-notices branch October 31, 2023 15:56
@github-actions github-actions bot added this to the Gutenberg 17.0 milestone Oct 31, 2023
mikachan pushed a commit that referenced this pull request Oct 31, 2023
@peterwilsoncc @mikachan
Require queryId for enhanced pagination. (#55720)
de980d5
@mikachan
Copy link
Member

I just cherry-picked this PR to the 6.4-rc3-2 branch to get it included in the next release: de980d5

@mikachan mikachan removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Oct 31, 2023
@mikachan mikachan mentioned this pull request Oct 31, 2023
Merged
@peterwilsoncc
Copy link
Contributor Author

Thanks for merging this promptly. I've created #55747 as a follow up to investigate whether the ID can be autogenerated if it's missing in order to respect developer intent.

mikachan added a commit that referenced this pull request Nov 1, 2023
@mikachan @artemiomorales
@peterwilsoncc @glendaviesnz @luisherranz @DAreRodz @afercia
Cherry-picked commits for WordPress 6.4 RC3 v2 (#55736)
60cfa82 
* Update label for lightbox editor UI (#55724)

* Update label for lightbox editor UI

* Change lightbox label in Global Styles

* Require queryId for enhanced pagination. (#55720)

* Change pattern category taxonomy public param to false (#55748)

* Query block enhanced pagination: Detect inner plugin blocks during render (#55714)

* Flag inner plugin blocks inside query loop

* Improve PHP logic a little

* Only disallow plugin blocks and post content

* Get rid of global variables

* Fix returned content from render callback

* Handle composed query stacks

* Disable navigation on the browser

* Replace `count` with `empty`

* Add PHPdocs and improve var naming

* Lint PHP

* Clarify docs a little

* Move the disable check before preventDefault

* Restore previous navigate logic

* Set filter priorities back to 10

* Basic inspector warnings

* Make render query filter static

* Add stable modal logic

* Switch back to ToggleControl

* Auto remove filter when query stack is empty

* Add first unit tests

* Switch to inverse control

* Add test case for nested queries

* Prevent passing `null` to the Tag Processr

* Get rid of explicit auto mode and notices

* Test directives in the Pagination Previous block

* Minor typos and improvements

* Improve modal texts

* Fix WPCS

* Reorder teardowns

* Reset the dirty flag after it's used

* Prevent usage of post content block

---------

Co-authored-by: David Arenas <[email protected]>

* Reuse existing screen-reader-text CSS class. (#55309)

---------

Co-authored-by: Artemio Morales <[email protected]>
Co-authored-by: Peter Wilson <[email protected]>
Co-authored-by: Glen Davies <[email protected]>
Co-authored-by: Luis Herranz <[email protected]>
Co-authored-by: David Arenas <[email protected]>
Co-authored-by: Andrea Fercia <[email protected]>
@hellofromtonya hellofromtonya mentioned this pull request Nov 1, 2023
Closed
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Nov 1, 2023
@hellofromtonya
Editor: 2nd update of npm packages for 6.4 RC3.
8dfcdc8 
This second update for RC3 includes the following fixes:

* [WordPress/gutenberg#55724 Update label for lightbox editor UI] - string change.
* [WordPress/gutenberg#55720 Query: Require queryId for enhanced pagination to prevent PHP notices] and warnings.
* [WordPress/gutenberg#55714 Query block enhanced pagination: Detect inner plugin blocks during render] - which avoids turning off enhanced pagination in TT4, includes string changes.
* [WordPress/gutenberg#55309 Query Loop block: Reuse existing screen-reader-text CSS class for the enhanced pagination aria-live region].

Follow up to [57034], [56987], [56961], [56849], [56818], [56816].

Props afercia, aristath, artemiosans, czapla, darerodz, glendaviesnz, hellofromTonya, jameskoster, joen, luisherranz, mikachan, ocean90, peterwilsoncc, ramonopoly, rajinsharwar, swissspidy.
Fixes #59411.
Built from https://develop.svn.wordpress.org/trunk@57048


git-svn-id: http://core.svn.wordpress.org/trunk@56559 1a063a9b-81f0-0310-95a4-ce76da25c4cd
github-actions bot pushed a commit to platformsh/wordpress-performance that referenced this pull request Nov 1, 2023
@hellofromtonya
Editor: 2nd update of npm packages for 6.4 RC3.
7d613b0 
This second update for RC3 includes the following fixes:

* [WordPress/gutenberg#55724 Update label for lightbox editor UI] - string change.
* [WordPress/gutenberg#55720 Query: Require queryId for enhanced pagination to prevent PHP notices] and warnings.
* [WordPress/gutenberg#55714 Query block enhanced pagination: Detect inner plugin blocks during render] - which avoids turning off enhanced pagination in TT4, includes string changes.
* [WordPress/gutenberg#55309 Query Loop block: Reuse existing screen-reader-text CSS class for the enhanced pagination aria-live region].

Follow up to [57034], [56987], [56961], [56849], [56818], [56816].

Props afercia, aristath, artemiosans, czapla, darerodz, glendaviesnz, hellofromTonya, jameskoster, joen, luisherranz, mikachan, ocean90, peterwilsoncc, ramonopoly, rajinsharwar, swissspidy.
Fixes #59411.
Built from https://develop.svn.wordpress.org/trunk@57048


git-svn-id: https://core.svn.wordpress.org/trunk@56559 1a063a9b-81f0-0310-95a4-ce76da25c4cd
pento pushed a commit to WordPress/wordpress-develop that referenced this pull request Nov 1, 2023
@hellofromtonya
Editor: 2nd update of npm packages for 6.4 RC3.
72be64b 
This second update for RC3 includes the following fixes:

* [WordPress/gutenberg#55724 Update label for lightbox editor UI] - string change.
* [WordPress/gutenberg#55720 Query: Require queryId for enhanced pagination to prevent PHP notices] and warnings.
* [WordPress/gutenberg#55714 Query block enhanced pagination: Detect inner plugin blocks during render] - which avoids turning off enhanced pagination in TT4, includes string changes.
* [WordPress/gutenberg#55309 Query Loop block: Reuse existing screen-reader-text CSS class for the enhanced pagination aria-live region].

Follow up to [57034], [56987], [56961], [56849], [56818], [56816].

Reviewed by davidbaumwald , jorbin.
Merges [57048] to the 6.4 branch.

Props afercia, aristath, artemiosans, czapla, darerodz, glendaviesnz, hellofromTonya, jameskoster, joen, luisherranz, mikachan, ocean90, peterwilsoncc, ramonopoly, rajinsharwar, swissspidy.
Fixes #59411.

git-svn-id: https://develop.svn.wordpress.org/branches/6.4@57049 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Nov 1, 2023
@hellofromtonya
Editor: 2nd update of npm packages for 6.4 RC3.
acef64f 
This second update for RC3 includes the following fixes:

* [WordPress/gutenberg#55724 Update label for lightbox editor UI] - string change.
* [WordPress/gutenberg#55720 Query: Require queryId for enhanced pagination to prevent PHP notices] and warnings.
* [WordPress/gutenberg#55714 Query block enhanced pagination: Detect inner plugin blocks during render] - which avoids turning off enhanced pagination in TT4, includes string changes.
* [WordPress/gutenberg#55309 Query Loop block: Reuse existing screen-reader-text CSS class for the enhanced pagination aria-live region].

Follow up to [57034], [56987], [56961], [56849], [56818], [56816].

Reviewed by davidbaumwald , jorbin.
Merges [57048] to the 6.4 branch.

Props afercia, aristath, artemiosans, czapla, darerodz, glendaviesnz, hellofromTonya, jameskoster, joen, luisherranz, mikachan, ocean90, peterwilsoncc, ramonopoly, rajinsharwar, swissspidy.
Fixes #59411.
Built from https://develop.svn.wordpress.org/branches/6.4@57049


git-svn-id: http://core.svn.wordpress.org/branches/6.4@56560 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hellofromtonya hellofromtonya hellofromtonya approved these changes

@DAreRodz DAreRodz DAreRodz approved these changes

@ajitbohra ajitbohra Awaiting requested review from ajitbohra ajitbohra is a code owner

Assignees
No one assigned
Labels
[Block] Query Loop Affects the Query Loop Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Milestone
Gutenberg 17.0
Development

Successfully merging this pull request may close these issues.

Enhanced Pagination throws notices if Query ID is not defined.
4 participants
@peterwilsoncc @hellofromtonya @mikachan @DAreRodz