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.

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

Check if object exists before accessing its properties. #52870

Merged
merged 3 commits into from
Jul 25, 2023

Conversation

tellthemachines
Copy link
Contributor

@tellthemachines tellthemachines commented Jul 24, 2023

What?

Fixes PHP test error encountered while updating core.

Also, based on feedback from the core patch I've changed the anonymous functions to named ones without gutenberg prefixes.

Testing Instructions

Check revisions still work for footnotes.

@tellthemachines tellthemachines added [Type] Bug An existing feature does not function as intended [Block] Footnotes Affects the Footnotes Block Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Jul 24, 2023
@github-actions
Copy link

github-actions bot commented Jul 24, 2023

Flaky tests detected in 117e5d0.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5651693766
📝 Reported issues:

Copy link
Member

@ramonjd ramonjd left a comment

Choose a reason for hiding this comment

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

This check makes sense.
Thanks for the fix.

@@ -142,7 +142,12 @@ static function( $post ) {

if ( $_gutenberg_revision_id ) {
$revision = get_post( $_gutenberg_revision_id );
Copy link
Member

@ramonjd ramonjd Jul 24, 2023

Choose a reason for hiding this comment

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

I know it's not part of this fix, but we could use wp_get_post_revision for its extra checks (there's no difference in functionality aside from that). Might be more explicit too.

Mentioned in Core patch here: WordPress/wordpress-develop#4892 (review)

Copy link
Member

@ramonjd ramonjd left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks for moving the static functions to named.

Just left some minor comments

packages/block-library/src/footnotes/index.php Outdated Show resolved Hide resolved
packages/block-library/src/footnotes/index.php Outdated Show resolved Hide resolved
@tellthemachines tellthemachines merged commit 4228be8 into trunk Jul 25, 2023
49 checks passed
@tellthemachines tellthemachines deleted the fix/core-php-errors branch July 25, 2023 04:16
@github-actions github-actions bot added this to the Gutenberg 16.4 milestone Jul 25, 2023
tellthemachines added a commit that referenced this pull request Jul 25, 2023
* Check if object exists before accessing its properties.

* Name anonymous functions

* Update names
@tellthemachines
Copy link
Contributor Author

I just cherry-picked this PR to the update/further-bugfixes-rc2 branch to get it included in the next release: 560bc18

@tellthemachines tellthemachines 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 Jul 25, 2023
ramonjd added a commit that referenced this pull request Jul 25, 2023
* Fix image block v6 deprecation (#52822)

* Remove incorrect comment

* Add missing attribtes and supports

* Add integration tests for the deprecation

* Fix incorrect import

* Fix fixtures

* Image: Use the correct method for caption class in recent deprecation (#52853)

* Distraction Free: Fix notices in site editor (#52867)

* Distraction Free: Add missing command in site editor (#52868)

* Site Editor: Fix the template parts link on the list page (#52891)

* Return focus more from focus return hook (#52710)

* Disambiguate "Import" button string. (#52907)

* Disambiguate "Import" button string.

* Add _x to import

---------

Co-authored-by: Andrew Serong <14988353+andrewserong@users.noreply.github.com>

* My patterns page: Increase color contrast for the toggle group (#52678)

* Update style.scss

* Update style.scss

* Check if object exists before accessing its properties. (#52870)

* Check if object exists before accessing its properties.

* Name anonymous functions

* Update names

* Sidebar: Restore Back buton 'go to parent' functionality (#52910)

* Global styles revisions: display text if no revisions are found (#52865)

* If somehow a user lands on the revisions panel when there are no revisions, show some helpful text rather than a loading spinner. Also, add an E2E test!

* Updated unit tests to reflect resolver logic changes

* Use existing string

* Only open edit view when testing the revisions panel itself

* ResizableFrame: Account for window resizing (#52697)

* ResizableFrame: Account for window resizing

* Don't memoize

---------

Co-authored-by: Robert Anderson <robert@noisysocks.com>

* Distraction Free: Fix conflict with showListViewByDefault preference (#52914)

* Backporting changes from WordPress/wordpress-develop#4891

* Site Editor: Open template parts in view canvas view mode (#52916)

---------

Co-authored-by: Alex Lende <alex+github.com@lende.xyz>
Co-authored-by: George Mamadashvili <georgemamadashvili@gmail.com>
Co-authored-by: Andrew Serong <14988353+andrewserong@users.noreply.github.com>
Co-authored-by: Mitchell Austin <mr.fye@oneandthesame.net>
Co-authored-by: Jb Audras <audrasjb@gmail.com>
Co-authored-by: Carolina Nymark <myazalea@hotmail.com>
Co-authored-by: Robert Anderson <robert@noisysocks.com>
Co-authored-by: Ramon <ramonjd@users.noreply.github.com>
Co-authored-by: Lena Morita <lena@jaguchi.com>
Co-authored-by: ramon <ramonjd@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Footnotes Affects the Footnotes Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants