Prepend the textdomain to the slug #180

kienstra · 2023-05-18T17:59:57Z

Summary of changes

An addition to #175

Prepends the texdomain to the slug. This will only apply on changing the title, or creating a new pattern.

All PM patterns created before this PR still won't have a textdomain prepended to the slug, unless they edit the title.

Before

* Slug: my-new-pattern

After

* Slug: twentytwentythree/my-new-pattern

How to test

Create a new pattern
Don't change anything, just publish it (click 'Create Pattern')
Look at the pattern's PHP file
Expected: The slug is <textdomain>/my-new-pattern*

<?php
/**
 * Title: My New Pattern 4
 * Slug: twentytwentythree/my-new-pattern-4
 * Description:

wp-modules/editor/tests/UtilsTest.php

kienstra · 2023-05-18T18:29:23Z

If you agree with this, your call when we release it.

No need to release it in 0.2.0.

mike-day

Great work, @kienstra! I think this would be a nice companion to #175 and #177, and it helps encourage best practices for PM users when creating pattern files.

Before updating the title, the slug had no prepended text domain:

After updating the title, the text domain is prepended:

kienstra · 2023-05-19T03:43:36Z

@mike-day,
Thanks for your review, and for your idea to do this.

You put it very well:

…it helps encourage best practices for PM users when creating pattern files.

dreamwhisper

Looks good!

kienstra · 2023-05-19T23:36:27Z

Hi @dreamwhisper,
Thanks a lot for testing this!

kienstra · 2023-05-19T23:37:47Z

If it's alright, let's release this in the upcoming 0.2.0 release, with #175

johnstonphilip · 2023-05-23T19:44:22Z

@kienstra It looks like a forward slash is getting left in the slug after this change. At least, that's what the failed test is showing. I'm not sure if we need to strip that out?

kienstra · 2023-05-23T20:44:21Z

Ah…let me see

kienstra · 2023-05-23T21:05:10Z

wp-modules/editor/tests/ModelTest.php

-		// Make sure the post_name (aka "slug") of the post and the slug in the file match.
-		$this->assertSame( $post->post_name, $pattern['slug'] );
+		// Make sure the ->post_name is the same as the slug, other than the prefixed textdomain.
+		$this->assertStringEndsWith( $post->post_name, $pattern['slug'] );


$pattern['slug'] was '/new-pattern-originally-created-with-pattern-manager' because prepend_textdomain() added the /.

The unit tests don't have a theme textdomain, so prepend_textdomain() only prefixed / to it, instead of a textdomain.

Now, it doesn't prefix a textdomain if there is no textdomain.

kienstra · 2023-05-23T21:25:14Z

wp-modules/editor/utils.php

+function prepend_textdomain( $name ) {
+	return implode(
+		'/',
+		array_filter( [ wp_get_theme()->get( 'TextDomain' ), $name ] )


Filters out an empty textdomain or $name.

There's both a textdomain and pattern name: yourtheme/foo-pattern-name
There's no textdomain: foo-pattern-name
There's no pattern name (shouldn't happen): yourtheme

kienstra · 2023-05-23T21:35:47Z

Hi @johnstonphilip,
How does this look now? Here's the fix.

kienstra · 2023-05-23T21:42:11Z

wp-modules/editor/tests/ModelTest.php

@@ -128,7 +128,7 @@ public function test_slug_and_filename_stay_the_same_after_content_update() {
 		$post->post_title   = 'This title remains the same after content changes.';
 		$post->post_content = 'test pattern content';
 		$post->post_status  = 'publish';
-		$post->post_name    = 'this-slug-remains-the-same-after-content-changes';
+		$post->post_name    = 'remains-the-same-after-content-changes';


Hope it's OK I changed this, as it's not only the slug.

Can you elaborate on that? What is it other than the slug?

It's also the filename (the filename is saved as the ->post_name, where the pattern slug is never in the WP_Post).

Happy to revert this, it's a nitpick of mine.

kienstra · 2023-05-23T21:43:01Z

wp-modules/editor/tests/UtilsTest.php

+	public function test_prepend_textdomain() {
+		$this->assertStringEndsWith(
+			'my-new-pattern',
+			prepend_textdomain( 'my-new-pattern' )


Not a complete test, but it's hard to mock wp_get_theme() in prepend_textdomain(), to mock the textdomain being empty.

kienstra · 2023-05-24T16:57:08Z

Thanks a lot, @johnstonphilip! Happy to revert #180 (comment) if you'd like.

…#185) * Add a naive function to update a slug * Use preg_replace() to update the slug * Fix the conversion, using addslashes * Add a helper to get whether a pattern has a pattern block * Add a function to update all patterns * Prepend the textdomain to the slug (#180) * Add a naive function to update a slug * Handle multiple attributes in pattern block * Only output the name and textdomain if they exist --------- Co-authored-by: Phil Johnston <phil.johnston@wpengine.com>

kienstra added 20 commits May 17, 2023 12:20

Add a naive function to update a slug

74073bc

Use preg_replace() to update the slug

c35b2be

Fix the conversion, using addslashes

293e24a

Add a helper to get whether a pattern has a pattern block

7b4ab6a

Add a function to update all patterns

1f48cb8

Fix fatal error

e883ce6

Replace ... with array_merge()

35e3e49

Pass all 3 arguments to update_slug()

6c45631

Merge in update/patternception-saving-bug, resolve conflict

082122f

Use the previous pattern slug

ccb273b

Make a test case cover 2 of the same pattern being renamed

8b4f516

Test there being no content

5221859

Update comments

2bfc017

Handle multiple attributes in pattern block

c25f236

Add another test case for changing both patterns

586eced

Rename to something without a slash

5734314

Allow a pattern slug to start with a number

44cac0e

Still handle multiple attributes

fb14449

Update the expected value

eacceff

Prefix a pattern slug with the textdomain

cdf3b2b

kienstra changed the base branch from main to update/patternception-slugs May 18, 2023 18:00

kienstra commented May 18, 2023

View reviewed changes

wp-modules/editor/tests/UtilsTest.php Outdated Show resolved Hide resolved

kienstra mentioned this pull request May 18, 2023

Allow the slug and name to be different for non-PM patterns that never edited their title #177

Merged

Fix linting error, remove needless dataProvider

cdba6c6

kienstra changed the title ~~Prepend a slug with the textdomain~~ Prepend the textdomain to the slug May 18, 2023

Fix a comment to refer to the textdomain

3491301

kienstra requested a review from mike-day May 18, 2023 18:27

kienstra marked this pull request as ready for review May 18, 2023 18:28

kienstra requested a review from dreamwhisper May 18, 2023 18:29

kienstra requested a review from johnstonphilip May 18, 2023 18:29

mike-day approved these changes May 18, 2023

View reviewed changes

dreamwhisper approved these changes May 19, 2023

View reviewed changes

mike-day mentioned this pull request May 22, 2023

Allow adding new categories in Editor UI #123

Merged

Base automatically changed from update/patternception-slugs to update/patternception-saving-bug May 23, 2023 18:00

kienstra added 2 commits May 23, 2023 12:51

Merge in target branch, resolve merge conflicts

20a84e2

Add space to the bottom of package.json

13d64c2

Fix unit tests for prefixing

019e781

kienstra commented May 23, 2023

View reviewed changes

kienstra added 2 commits May 23, 2023 15:09

Only prepend a textdomain if there's a textdomain

e951357

Only output the name and textdomain if they exist

23e03b4

kienstra commented May 23, 2023

View reviewed changes

johnstonphilip approved these changes May 24, 2023

View reviewed changes

kienstra merged commit ac955a1 into update/patternception-saving-bug May 24, 2023

kienstra deleted the add/slug-prefixing branch May 24, 2023 16:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prepend the textdomain to the slug #180

Prepend the textdomain to the slug #180

kienstra commented May 18, 2023 •

edited

Loading

kienstra commented May 18, 2023 •

edited

Loading

mike-day left a comment

kienstra commented May 19, 2023 •

edited

Loading

dreamwhisper left a comment

kienstra commented May 19, 2023

kienstra commented May 19, 2023

johnstonphilip commented May 23, 2023

kienstra commented May 23, 2023

kienstra May 23, 2023 •

edited

Loading

kienstra May 23, 2023 •

edited

Loading

kienstra May 23, 2023 •

edited

Loading

kienstra commented May 23, 2023 •

edited

Loading

kienstra May 23, 2023

johnstonphilip May 24, 2023

kienstra May 24, 2023

kienstra May 23, 2023

kienstra commented May 24, 2023

Prepend the textdomain to the slug #180

Prepend the textdomain to the slug #180

Conversation

kienstra commented May 18, 2023 • edited Loading

Summary of changes

Before

After

How to test

kienstra commented May 18, 2023 • edited Loading

mike-day left a comment

Choose a reason for hiding this comment

kienstra commented May 19, 2023 • edited Loading

dreamwhisper left a comment

Choose a reason for hiding this comment

kienstra commented May 19, 2023

kienstra commented May 19, 2023

johnstonphilip commented May 23, 2023

kienstra commented May 23, 2023

kienstra May 23, 2023 • edited Loading

Choose a reason for hiding this comment

kienstra May 23, 2023 • edited Loading

Choose a reason for hiding this comment

kienstra May 23, 2023 • edited Loading

Choose a reason for hiding this comment

kienstra commented May 23, 2023 • edited Loading

kienstra May 23, 2023

Choose a reason for hiding this comment

johnstonphilip May 24, 2023

Choose a reason for hiding this comment

kienstra May 24, 2023

Choose a reason for hiding this comment

kienstra May 23, 2023

Choose a reason for hiding this comment

kienstra commented May 24, 2023

kienstra commented May 18, 2023 •

edited

Loading

kienstra commented May 18, 2023 •

edited

Loading

kienstra commented May 19, 2023 •

edited

Loading

kienstra May 23, 2023 •

edited

Loading

kienstra May 23, 2023 •

edited

Loading

kienstra May 23, 2023 •

edited

Loading

kienstra commented May 23, 2023 •

edited

Loading