Skip to content

Add unit test for get_user_data_from_wp_global_styles.#63721

Draft
joemcgill wants to merge 2 commits intotrunkfrom
update/test_get_user_data_from_wp_global_styles
Draft

Add unit test for get_user_data_from_wp_global_styles.#63721
joemcgill wants to merge 2 commits intotrunkfrom
update/test_get_user_data_from_wp_global_styles

Conversation

@joemcgill
Copy link
Copy Markdown
Member

@joemcgill joemcgill commented Jul 18, 2024

What?

This adds a unit test to demonstrate that WP_Theme_JSON_Resolver_Gutenberg::get_user_data_from_wp_global_styles returns an empty array when no $create_post param is passed after being previously called with the $create_post param set to true.

See: #63722

Why?

While attempting to write a unit test to confirm that caching added to wp_add_global_styles_for_blocks() would get cleared if user data changed (related issue), I noticed that after calling WP_Theme_JSON_Resolver_Gutenberg::get_user_data_from_wp_global_styles() and passing a value to the second parameter to create a post, the next time that method is called, the created post is not returned.

How?

This PR demonstrates a possible bug. If valid, the PR would need to be updated to fix the issue.

Testing Instructions

Review the Unit test failure on the PR or run the test locally:

npm run test:unit:php:base -- --filter test_get_user_data_from_wp_global_styles_returns_created_post

Testing Instructions for Keyboard

n/a

This adds a unit test to demonstrate that `WP_Theme_JSON_Resolver_Gutenberg::get_user_data_from_wp_global_styles` returns an empty array when no `$create_post` param is passed after being previously called with the `$create_post` param set to `true`.
@github-actions
Copy link
Copy Markdown

Warning: Type of PR label mismatch

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

  • Type-related labels to choose from: [Type] Automated Testing, [Type] Breaking Change, [Type] Bug, [Type] Build Tooling, [Type] Code Quality, [Type] Copy, [Type] Developer Documentation, [Type] Enhancement, [Type] Experimental, [Type] Feature, [Type] New API, [Type] Task, [Type] Technical Prototype, [Type] Performance, [Type] Project Management, [Type] Regression, [Type] Security, [Type] WP Core Ticket, Backport from WordPress Core.
  • Labels found: .

Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task.

@joemcgill
Copy link
Copy Markdown
Member Author

Failing test that demonstrates the issue can be seen here: https://github.com/WordPress/gutenberg/actions/runs/9997910616/job/27636197439?pr=63721

@andrewserong
Copy link
Copy Markdown
Contributor

I'm not very familiar with the inner workings of get_user_data_from_wp_global_styles, and @tellthemachines isn't around at the moment, so just adding @oandregal as a reviewer for visibility. Looks like this PR adds a test to flag the issue 👍, but I'm not sure what the appropriate fix would be.

@joemcgill
Copy link
Copy Markdown
Member Author

Thanks @andrewserong. You're exactly right that this is just demonstrating the issue. Before attempting to suggest a fix, I wanted to confirm whether this is actually a bug, or if I'm misunderstanding something about how this method is meant to work. Generally speaking, when the initial post type gets created for custom styles, it happens through a REST endpoint, and I'm uncertain if this method is actually even used in that process (I suspect not).

What I'm mostly unsure about is whether the code in this method that creates the post is missing something, or whether the query that returns the post is incorrect.

Copy link
Copy Markdown
Contributor

@aaronrobertshaw aaronrobertshaw left a comment

Choose a reason for hiding this comment

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

I had a bit of a look into this. I believe get_user_data_from_wp_global_styles is intended to return the global styles custom post type as this test suggests.

The catch is if the current user does not have the capabilities to assign terms, they are not added when the post is inserted. The subsequent query then fails to match on the taxonomy.

In the proposed test, if the user is set to an administrator, the test passes (see inline comment below).

I would suspect that in the case of global styles, the taxonomy term needs to be set regardless of user capabilities so the post relates to the appropriate theme. Whether the user has caps to create the global styles post itself might be a different question.

If the above suspicion is correct, then a small tweak to manually call wp_set_object_terms( $cpt_post_id, array( $stylesheet ), 'wp_theme' ); instead of relying on tax_input in the args for wp_insert_post makes the test pass as well.

diff --git a/lib/class-wp-theme-json-resolver-gutenberg.php b/lib/class-wp-theme-json-resolver-gutenberg.php
index a2153e639d..98c4912081 100644
--- a/lib/class-wp-theme-json-resolver-gutenberg.php
+++ b/lib/class-wp-theme-json-resolver-gutenberg.php
@@ -504,12 +504,10 @@ class WP_Theme_JSON_Resolver_Gutenberg {
 					'post_title'   => 'Custom Styles', // Do not make string translatable, see https://core.trac.wordpress.org/ticket/54518.
 					'post_type'    => $post_type_filter,
 					'post_name'    => sprintf( 'wp-global-styles-%s', urlencode( $stylesheet ) ),
-					'tax_input'    => array(
-						'wp_theme' => array( $stylesheet ),
-					),
 				),
 				true
 			);
+			wp_set_object_terms( $cpt_post_id, array( $stylesheet ), 'wp_theme' );
 			if ( ! is_wp_error( $cpt_post_id ) ) {
 				$user_cpt = get_object_vars( get_post( $cpt_post_id ) );
 			}

Comment thread phpunit/class-wp-theme-json-resolver-test.php
Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants