Patterns: Fix pattern categories on import #58926
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ntsekouras
added
[Type] Bug
ntsekouras
requested review from
glendaviesnz,
kevin940726,
annezazu and
aaronrobertshaw
|
Size Change: +114 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
|
I ran into an error message:
Was using
|
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
ntsekouras
force-pushed
the
fix/pattern-categories-on-import
branch
from
3957efc to
007aaca
Compare
Thanks for testing @bph! TBH I wasn't aware of the import button in the admin view, but on the other hand I made no changes there. Regarding the error you saw in site editor, it seems weird at first sight and should't happen. It indicates the return value of Can you share the pattern you're testing, because I cannot reproduce in my tests? Additionally, can you test the same pattern in trunk and see if there are errors there too? Thanks! |
There is a big room for UX improvement in general on error messages. Linking for visibility: #53095 |
Both ran into the error message. The one from the admin view as a bit more helpful. The pattern And importing it into trunk work as expected. |
|
@bph can you share the |
|
|
@bph thank you for sharing. That's weird though.. I quickly tested the |
There was a problem hiding this comment.
It's been a while since I've been heavily involved with patterns but this is looking pretty good from what I can tell 👍
✅ Importing a pattern adds it to the currently viewed category
✅ Stays on the my patterns category page when importing a pattern there
✅ Didn't spot any regressions around adding patterns and custom categories
FWIW I also tested with the pattern file @bph shared and it worked fine for me.
Comment on lines
102
to
125
| async function createPatternCategory( existingTerm ) { | ||
| try { | ||
| // Since we have an existing core category we need to match the new user category to the | ||
| // correct slug rather than autogenerating it to prevent duplicates, eg. the core `Headers` | ||
| // category uses the singular `header` as the slug. | ||
| const termData = { | ||
| name: existingTerm.label, | ||
| slug: existingTerm.name, | ||
| }; | ||
| const newTerm = await saveEntityRecord( | ||
| 'taxonomy', | ||
| 'wp_pattern_category', | ||
| termData, | ||
| { throwOnError: true } | ||
| ); | ||
| invalidateResolution( 'getUserPatternCategories' ); | ||
| return newTerm.id; | ||
| } catch ( error ) { | ||
| if ( error.code !== 'term_exists' ) { | ||
| throw error; | ||
| } | ||
| return error.data.term_id; | ||
| } | ||
| } |
There was a problem hiding this comment.
Could we call the new hook useAddPatternCategory and have it return createPatternCategory and categoryMap so we don't have to replicate this method here and in the patterns package?
There was a problem hiding this comment.
Sounds good. I updated the code.
ntsekouras
force-pushed
the
fix/pattern-categories-on-import
branch
from
007aaca to
53e83eb
Compare
|
Flaky tests detected in 53e83eb. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7883076114
|
|
I tested this PR again this morning, on a new installation, and it worked without fail or error messages.
Great work! For now, I attribute yesterday's error messages to a corrupt local installation. It was time to retire the test site and start a new one. Or it was a corrupt download of the PR artifact. 🤷 |
glendaviesnz
approved these changes
Feb 13, 2024
|
Is this one meant to be backported to WP 6.5? |
youknowriad
added
the
Backport to WP 6.7 Beta/RC
Yes. Also I'll add in my backlog to add some tests. |
youknowriad
pushed a commit
that referenced
this pull request
Feb 20, 2024
Co-authored-by: ntsekouras <[email protected]> Co-authored-by: aaronrobertshaw <[email protected]> Co-authored-by: glendaviesnz <[email protected]> Co-authored-by: bph <[email protected]> Co-authored-by: hanneslsm <[email protected]> Co-authored-by: colorful-tones <[email protected]> Co-authored-by: annezazu <[email protected]>
|
I just cherry-picked this PR to the more/backports-for-beta3 branch to get it included in the next release: 942cc3e |
youknowriad
removed
the
Backport to WP 6.7 Beta/RC
youknowriad
added
the
Backported to WP Core
youknowriad
pushed a commit
that referenced
this pull request
Feb 20, 2024
Co-authored-by: ntsekouras <[email protected]> Co-authored-by: aaronrobertshaw <[email protected]> Co-authored-by: glendaviesnz <[email protected]> Co-authored-by: bph <[email protected]> Co-authored-by: hanneslsm <[email protected]> Co-authored-by: colorful-tones <[email protected]> Co-authored-by: annezazu <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Fixes: #58901
Currently in patterns page when we import a pattern, there is an issue where we only add the pattern category if it's an already created user pattern category. This PR adds similar logic to the creation pattern flow, where we also check the core categories and create a user category with same name and slug.
This results in adding the current category in the imported pattern properly and remaining in the same view we were.
How?
I created a private hook to get the same patterns category map with the creation flow.
Testing Instructions
my patterns, we stay at the same page