The naming of some of the WP_Theme_JSON_Resolver's methods is not optimal

[antonvlasenko]

As reported by @desrosj:
I wanted to raise a few concerns I have about some of the method names and phrasing within WP_Theme_JSON_Resolver.

The get_user_*() functions are a bit misleading. In reading their names, it's reasonable to assume that they are retrieving user specific data. But that's not true. A few ocblocks also reference the "user's origin configuration", but it does not have anything to do with users in the WordPress sense.
A handful of functions refer to a custom post type. Anything built into WordPress Core is not really custom. Could these be changed to "site global styles" or "get current global styles"?

Trac ticket: https://core.trac.wordpress.org/ticket/54517

@anton-vlasenko can you resolve the merge conflicts please?

@noisysocks what do you think of the new naming?

[hellofromTonya]

Marking for refresh as there are merge conflicts in the PR.

Also, 5.9 is in feature freeze meaning no enhancements can be merged.

In this case, it's discussing the best name for a new public method in an existing API, new method introduced in 5.9. Reclassified this ticket to defect though that's not a great classification for it either.

It's okay to discuss it now as once 5.9 final is released, the name has be maintained for backwards-compatibility.

@noisysocks what do you think of the new naming?

We do typically avoid referencing CPTs in other parts of Core as it's usually more of an implementation detail.

I defer to @ockham who ported most of this 😀

One consideration is that we'd have to update Gutenberg with these changes too.

[SergeyBiryukov]

Replying to hellofromTonya:

It's okay to discuss it now as once 5.9 final is released, the name has be maintained for backwards-compatibility.

Just noting that it looks like the block editor might have a different approach to backward compatibility than the rest of core. For example, in [52049]:

• The WP_Theme_JSON::get_merged_preset_by_slug() and ::has_properties() methods have been completely removed.
• The WP_Theme_JSON::get_block_styles() method was renamed.
• The parameters of WP_Theme_JSON::get_merged_data() and ::get_stylesheet() methods have been completely changed, though _deprecated_argument() calls were added to handle the older format.
• The WP_Theme_JSON::ALLOWED_TOP_LEVEL_KEYS, ::ALLOWED_SETTINGS, and ::ALLOWED_STYLES constants were renamed.

So it looks like it could still be an option to rename some methods at a later stage, though I agree it would be preferable to do that now if there is enough consensus.

I defer to @ockham who ported most of this 😀

Hey @ockham, what do you think of renaming?

One consideration is that we'd have to update Gutenberg with these changes too.

@anton-vlasenko is there a matching PR in Gutenberg? Also can you fix the merge conflicts please?

I defer to @ockham who ported most of this 😀

Hey @ockham, what do you think of renaming?

Hey, sorry for the late reply 😅 TBH I'm less familiar with the theme.json resolver -- that would be more @oandregal's or @jorgefilipecosta's area of expertise, so maybe they can weigh in.

(FWIW, personally I think that something like get_something_from_post makes sense, when e.g. compared to get_user_something_from_custom_post_type. The _from_post suffix is also something we do in template resolution, at least in one private function, _build_block_template_result_from_post (which has a counterpart, _build_block_template_result_from_file).)

@anton-vlasenko These changes will need to be backported to Gutenberg. Can you take care of that?

[hellofromTonya]

​PR 1957 is ready for commit.

@anton-vlasenko are you available to resolve the merge conflicts?

[hellofromTonya]

In 52372:

Themes: Rename public static functions in WP_Theme_JSON_Resolver to remove custom_post_type references.

WordPress Core is not really custom and does not reference "custom post type" in its function naming. This commit renames 2 public static methods:

• WP_Theme_JSON_Resolver::get_user_custom_post_type_id() renamed to WP_Theme_JSON_Resolver::get_user_global_styles_post_id().
• WP_Theme_JSON_Resolver::get_user_data_from_custom_post_type() renamed to WP_Theme_JSON_Resolver:: get_user_data_from_wp_global_styles().

Follow-up to [52049], [52051], [52069], [52232], [52275], [52364].

Props antonvlasenko, bernhard-reiter, costdev, desrosj, hellofromTonya, noisysocks, oandregal, SergeyBiryukov.
Fixes #54517.

Committed via changeset https://core.trac.wordpress.org/changeset/52372.

[hellofromTonya]

I opened an issue on Gutenberg ​https://github.com/WordPress/gutenberg/issues/37379 to also rename the methods there, i.e. to resync Gutenberg with Core.