Skip to content
This repository has been archived by the owner on Oct 16, 2024. It is now read-only.

Update/patternception saving bug tests #183

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import convertToSlug from '../convertToSlug';
import toKebabCase from '../toKebabCase';

describe( 'convertToSlug', () => {
describe( 'toKebabCase', () => {
it.each( [
[ undefined, '' ],
[ '', '' ],
Expand All @@ -14,7 +14,7 @@ describe( 'convertToSlug', () => {
[ 'with&&&***non.,.word$$characters', 'with-non-word-characters' ],
[ 'EndingInApostrophe!', 'endinginapostrophe' ],
[ 'With 🥁Emojis🏇', 'with-emojis' ],
] )( 'should convert to the slug', ( toConvert, expected ) => {
expect( convertToSlug( toConvert ) ).toEqual( expected );
] )( 'should convert to kebab case', ( toConvert, expected ) => {
expect( toKebabCase( toConvert ) ).toEqual( expected );
} );
} );
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/** Converts a string to a slug. */
export default function convertToSlug( toConvert = '' ) {
/** Converts a string to kebab-case. */
export default function toKebabCase( toConvert = '' ) {
return toConvert
.replace( /[_\W]+(?=\w+)/g, '-' )
.replace( /[^-\w]/g, '' )
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ export default function PatternEdit( {
className: pattern ? 'alignfull' : 'is-layout-constrained',
} );
const { currentName } = useSavedPostData();
const patternSlug = Object.values( patternManager.patterns ).find(
( ownPattern ) => ownPattern.name === currentName
)?.slug;

return (
<>
Expand All @@ -90,7 +93,7 @@ export default function PatternEdit( {
patternCategories={ patternManager.patternCategories }
patterns={ filterOutPatterns(
patternManager.patterns,
currentName
patternSlug
) }
siteUrl={ patternManager.siteUrl }
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { PluginDocumentSettingPanel } from '@wordpress/edit-post';
import { PanelRow, TextControl } from '@wordpress/components';
import { RichText } from '@wordpress/block-editor';

import convertToSlug from '../../utils/convertToSlug';
import toKebabCase from '../../utils/toKebabCase';

import type { AdditionalSidebarProps, BaseSidebarProps } from './types';
import type { Pattern } from '../../types';
Expand All @@ -15,8 +15,8 @@ function isTitleTaken(
currentName: string,
patternNames: Array< Pattern[ 'name' ] >
) {
const newSlug = convertToSlug( patternTitle );
return patternNames.includes( newSlug ) && newSlug !== currentName;
const newName = toKebabCase( patternTitle );
return patternNames.includes( newName ) && newName !== currentName;
}

export default function TitlePanel( {
Expand Down Expand Up @@ -47,7 +47,7 @@ export default function TitlePanel( {
value={ title }
onChange={ ( newTitle: typeof title ) => {
editPost( { title: newTitle } );
handleChange( 'name', convertToSlug( newTitle ) );
handleChange( 'name', toKebabCase( newTitle ) );

if ( ! newTitle ) {
lockPostSaving();
Expand Down
6 changes: 3 additions & 3 deletions wp-modules/editor/js/src/utils/filterOutPatterns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import type { Pattern, Patterns } from '../types';
*/
export default function filterOutPatterns(
patterns: Patterns,
patternName: Pattern[ 'name' ]
patternSlug: Pattern[ 'slug' ]
) {
return Object.entries( patterns ).reduce(
( accumulator, [ key, pattern ] ) => {
Expand All @@ -17,8 +17,8 @@ export default function filterOutPatterns(
...( ! hasBlock(
'core/pattern',
parse( pattern.content ),
patternName
) && pattern.name !== patternName
patternSlug
) && pattern.slug !== patternSlug
? { [ key ]: pattern }
: {} ),
};
Expand Down
6 changes: 3 additions & 3 deletions wp-modules/editor/js/src/utils/hasBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ import type { Block, Pattern } from '../types';
export default function hasBlock(
blockName: String,
blocks: Block[],
patternName: Pattern[ 'name' ]
patternSlug: Pattern[ 'slug' ]
) {
return blocks.some( ( block ) => {
return (
( block.name === blockName &&
block.attributes?.slug === patternName ) ||
hasBlock( blockName, block?.innerBlocks ?? [], patternName )
block.attributes?.slug === patternSlug ) ||
hasBlock( blockName, block?.innerBlocks ?? [], patternSlug )
);
} );
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/** Converts a string to a slug. */
export default function convertToSlug( toConvert = '' ) {
/** Converts a string to a kebab-case. */
export default function toKebabCase( toConvert = '' ) {
return toConvert
.replace( /[_\W]+(?=\w+)/g, '-' )
.replace( /[^-\w]/g, '' )
Expand Down
24 changes: 16 additions & 8 deletions wp-modules/editor/model.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function populate_pattern_from_file( $post ) {
add_action( 'the_post', __NAMESPACE__ . '\populate_pattern_from_file' );

/**
* Saves the pattern to the .php file.
* Saves the pattern to the .php file, and also removes the mocked post required for the WP editing UI.
*
* @param int $post_id The post ID.
* @param WP_Post $post The post.
Expand All @@ -54,13 +54,16 @@ function save_pattern_to_file( WP_Post $post ) {
return;
}

$pattern = get_pattern_by_name( $post->post_name );
$name = $post->post_name;
$pattern = get_pattern_by_name( $name );
update_pattern(
array_merge(
$pattern ? $pattern : [],
// Only set the slug to the name for new patterns.
// Patterns created without PM might have a different slug and name.
$pattern ? $pattern : [ 'slug' => $name ],
[
'content' => $post->post_content,
'name' => $post->post_name,
'name' => $name,
],
$post->post_title
? [ 'title' => $post->post_title ]
Expand Down Expand Up @@ -108,6 +111,7 @@ function save_metadata_to_pattern_file( $override, $post_id, $meta_key, $meta_va

$pattern_name = $post->post_name;
$pattern = get_pattern_by_name( $pattern_name );
$name_changed = 'name' === $meta_key && $pattern_name !== $meta_value;

if ( 'name' === $meta_key ) {
wp_update_post(
Expand All @@ -116,16 +120,20 @@ function save_metadata_to_pattern_file( $override, $post_id, $meta_key, $meta_va
'post_name' => $meta_value,
]
);
}

if ( $pattern_name !== $meta_value ) {
delete_pattern( $pattern_name );
}
if ( $name_changed ) {
delete_pattern( $pattern_name );
}

return update_pattern(
array_merge(
get_pattern_defaults(),
$pattern ? $pattern : [ 'title' => $post->post_title ],
$pattern ? $pattern : [
'title' => $post->post_title,
'slug' => $pattern_name,
],
$name_changed ? [ 'slug' => $meta_value ] : [],
[
'name' => $pattern_name,
$meta_key => $meta_value,
Expand Down
99 changes: 99 additions & 0 deletions wp-modules/editor/tests/ModelTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,103 @@ public function test_delete_pattern_posts_correct_post() {
get_posts( [ 'post_type' => 'pm_pattern ' ] )
);
}

/**
* Tests that a pattern newly created with Pattern Manager results in a slug and filename that matches the post_name.
*/
public function test_new_pattern_title_matches_slug() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick that shouldn't block merging:

The pattern PHP files written in these tests shouldn't persist into the next tests.

Maybe:

function setUp() {
  // Create a tmp directory
  // $this->stylesheet_dir = created tmp directory
  add_filter( 'stylesheet_directory', [ $this, 'get_stylesheet_dir' ] ); // So update_pattern() will save patterns to the tmp dir, not the actual theme.
}

function tearDown() {
   // Delete $this->stylesheet_dir, maybe with the WP filesystem API
}

function get_stylesheet_dir() {
   return $this->stylesheet_dir;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

If you'd like, I'll open a PR for this, as it's my nitpick 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah if you don't mind, I'd love to fully understand how this works :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, I'll do that. Still, don't wait to merge this.


// Mock a post object so we can test it.
$post_id = -998; // negative ID, to avoid clash with a valid post.
$post = new \stdClass();
$post->ID = $post_id;
$post->post_author = 1;
$post->post_title = 'New Pattern, originally created with Pattern Manager.';
$post->post_content = 'test pattern content';
$post->post_status = 'publish';
$post->post_name = 'new-pattern-originally-created-with-pattern-manager';
$post->post_type = get_pattern_post_type();
$post->filter = 'raw';

// Convert to WP_Post object.
$wp_post = new \WP_Post( $post );
Copy link
Contributor

Choose a reason for hiding this comment

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

Sometimes the post factory from WP_UnitTestCase helps:

		$wp_post = $this->factory()->post->create_and_get(
			[
				'post_title'   => 'New Pattern, originally created with Pattern Manager.',
				'post_content' => 'test pattern content',
				'post_name'    => 'new-pattern-originally-created-with-pattern-manager',
				'post_type'    => get_pattern_post_type(),
			]
		);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh nice!


// Save the mocked pattern to the disk.
\PatternManager\Editor\save_pattern_to_file( $wp_post );

// Get the contents of the file that was saved.
$pattern = \PatternManager\PatternDataHandlers\get_pattern_by_name( $post->post_name );

// 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 (aka "slug") of the post and the filename match.
$this->assertSame( $post->post_name, $pattern['name'] );
}

/**
* Tests a pattern with a slug that does not match the filename remains the same if something other than the title was not changed.
*/
public function test_slug_and_filename_stay_the_same_after_content_update() {

// Mock a post object so we can test it.
$post_id = -997; // negative ID, to avoid clash with a valid post.
$post = new \stdClass();
$post->ID = $post_id;
$post->post_author = 1;
$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_type = get_pattern_post_type();
$post->filter = 'raw';

// Convert to WP_Post object.
$wp_post = new \WP_Post( $post );

// Save the pattern to the disk.
\PatternManager\Editor\save_pattern_to_file( $wp_post );

// Rename the pattern's filename to something different than the slug.
// This is to mock how it might be for for a non-pm-made pattern, with mismatching filenames and slugs.
$wp_filesystem = \PatternManager\GetWpFilesystem\get_wp_filesystem_api();
$patterns_dir = \PatternManager\PatternDataHandlers\get_patterns_directory();
$original_name = $post->post_name . '.php';
$mocked_mismatching_name = 'mismatched-name.php';
$wp_filesystem->move( $patterns_dir . $original_name, $patterns_dir . $mocked_mismatching_name );
$wp_filesystem->delete( $patterns_dir . $original_name );

// Get the contents of the file.
$pattern = \PatternManager\PatternDataHandlers\get_pattern_by_name( 'mismatched-name' );

// Mock a post object so we can test modifying only the content.
$post_id = -997; // negative ID, to avoid clash with a valid post.
Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea to mock a new WP_Post.

When the filename is changed, the WP_Post in the editor will be different.

$post = new \stdClass();
$post->ID = $post_id;
$post->post_author = 1;
$post->post_title = $pattern['title'];
$post->post_content = 'This is modified pattern content, but nothing else is changed!';
$post->post_status = 'publish';
$post->post_name = $pattern['slug'];
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick:

Suggested change
$post->post_name = $pattern['slug'];
$post->post_name = 'mismatched-name';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a reason you'd set the slug to this as opposed to pulling from the file?

Copy link
Contributor

@kienstra kienstra May 23, 2023

Choose a reason for hiding this comment

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

It's a nitpick 😄

But if it were to come from the file, it should be the $pattern['name'].

In tests, I like setting values to string literals, as string variables are mutable.

$post->post_type = get_pattern_post_type();
$post->filter = 'raw';

// Convert to WP_Post object.
$wp_post = new \WP_Post( $post );

// Save the pattern to the disk.
\PatternManager\Editor\save_pattern_to_file( $wp_post );

// Get the contents of the file.
$content_modified_pattern = \PatternManager\PatternDataHandlers\get_pattern_by_name( 'mismatched-name' );

// Make sure the slug does not get changed when only the content is modified.
$this->assertSame( 'this-slug-remains-the-same-after-content-changes', $content_modified_pattern['slug'] );
Copy link
Contributor

Choose a reason for hiding this comment

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

Great work figuring out a test for this. Now, we can be more confident in our PRs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Really nice naming of 'this-slug-remains-the-same-after-content-changes'


// Make sure the title does not get changed when only the content is modified.
$this->assertSame( 'This title remains the same after content changes.', $content_modified_pattern['title'] );

// Make sure the filename does not get changed when only the content is modified.
$this->assertSame( 'mismatched-name', $content_modified_pattern['name'] );
}
}
2 changes: 1 addition & 1 deletion wp-modules/pattern-data-handlers/pattern-data-handlers.php
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ function construct_pattern_php_file_contents( $pattern_data ) {
$file_contents = '<?php
/**
* Title: ' . addcslashes( $pattern['title'], '\'' ) . '
* Slug: ' . $pattern['name'] . '
* Slug: ' . $pattern['slug'] . '
* Description: ' . $pattern['description'] . '
* Categories: ' . implode( ', ', $pattern['categories'] ) . '
* Keywords: ' . implode( ', ', $pattern['keywords'] ) . '
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ public function test_construct_pattern_php_file_contents_empty() {
construct_pattern_php_file_contents(
[
'name' => 'empty',
'slug' => 'empty',
'title' => 'Empty',
]
)
Expand Down