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

Site Editor: Fix Template Parts post type preload path #64401

Merged
merged 2 commits into from
Aug 12, 2024

Conversation

Mamaduka
Copy link
Member

@Mamaduka Mamaduka commented Aug 9, 2024

What?

PR fixes a typo in the Template Parts post-type preload path. This type has been around since the beginning—#36488.

Why?

Incorrect preload paths result in a silent WP error, and noting gets preloaded. I only noticed this because I was checking REST API responses and was curious where WP_Error originated.

Maybe rest_preload_api_request should log a _doing_it_wrong() when a provided path resolves with an error.

Testing Instructions

  1. Navigate to site-editor.php?postType=wp_template_part.
  2. The app shouldn't make a types/wp_template_part request. It does on the trunk.

Testing Instructions for Keyboard

Same.

Screenshots or screencast

CleanShot 2024-08-09 at 17 17 11

@Mamaduka Mamaduka added [Type] Bug An existing feature does not function as intended [Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") labels Aug 9, 2024
@Mamaduka Mamaduka self-assigned this Aug 9, 2024
Copy link

github-actions bot commented Aug 9, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

Wow, nice find @Mamaduka 💯

This fixes it, FWIW, so I'm approving 👍

I assume you preferred fixing it here instead of here so it gets fixed ASAP? I wonder if it should be fixed in core, though, and then once Gutenberg is merged into core, this filter would just be removed.

@Mamaduka
Copy link
Member Author

Mamaduka commented Aug 9, 2024

Thanks for testing, @tyxla!

I think this is how we usually fix editor preload paths. Example #39256.

I'll create core PR separately for backport and a ticket to log _doing_it_wrong when the path resolves with an error, making it easier to catch similar bugs.

@tyxla
Copy link
Member

tyxla commented Aug 9, 2024

Thought that might be it. Thanks for confirming and for fixing it @Mamaduka 🙌

Copy link

Flaky tests detected in 2bc08c2.
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/10347976331
📝 Reported issues:

@Mamaduka Mamaduka merged commit 21cfde3 into trunk Aug 12, 2024
61 checks passed
@Mamaduka Mamaduka deleted the fix/site-editor-preload-paths branch August 12, 2024 08:37
@github-actions github-actions bot added this to the Gutenberg 19.1 milestone Aug 12, 2024
getdave pushed a commit that referenced this pull request Aug 14, 2024
* Site Editor: Fix Template Parts post type preload path
* Add backport changelog file

Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") [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