Make WordPress Core

Opened 10 months ago

Closed 9 months ago

#59569 closed defect (bug) (fixed)

wp_pattern_category is set public to true, when it should have been false.

Reported by: vrajadas's profile vrajadas Owned by: hellofromtonya's profile hellofromTonya
Milestone: 6.4 Priority: normal
Severity: normal Version: 6.4
Component: Taxonomy Keywords: has-patch commit dev-reviewed
Focuses: Cc:

Description (last modified by SergeyBiryukov)

The pattern categories taxonomy is registered in wp-includes/taxonomy.php at lines 226-245, added by ticket #59379. Not sure why e.g. public is set to true, when it should have been false.
Wordpress version 6.4beta

Change History (18)

#1 @SergeyBiryukov
10 months ago

  • Description modified (diff)

#2 @hellofromTonya
9 months ago

  • Milestone changed from Awaiting Review to 6.4

Hello @vrajadas,

Welcome to WordPress Core's Trac :) Sorry for the delay in responding to your ticket.

The change: #59379 / [56642].
It was originally set to false, but then changed in Gutenberg PR 53835, which added the UI and ability to assign categories to the user created patterns.

Hmm I also wonder why the public arg is set to true. Tested with it set to false and am able to add, see, and update pattern categories.

Pinging the contributors for discover the reasoning @glendaviesnz @isabel_brison @mamaduka.

Pulling into 6.4 for awareness and discussion.

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


9 months ago

#4 follow-up: @glendaviesnz
9 months ago

It was initially set to false as the initial PR was just intended to get the infrastructure in place and not make anything available to users.

With 53835 it was set to public, because this taxonomy is intended for use by users either via the admin interface at /wp-admin/edit-tags.php?taxonomy=wp_pattern_category or via the UI added to the editor, so setting public to true seemed like the correct option. We may have misunderstood the meaning of public in this context though because /wp-admin/edit-tags.php?taxonomy=wp_pattern_category is still accessible even if it is set to false as you note.

Should we put up a patch to switch this back to false?

#5 in reply to: ↑ 4 @hellofromTonya
9 months ago

  • Keywords needs-patch added

Thank you @glendaviesnz for explaining :) I get it.

Replying to glendaviesnz:

With 53835 it was set to public, because this taxonomy is intended for use by users either via the admin interface at /wp-admin/edit-tags.php?taxonomy=wp_pattern_category or via the UI added to the editor, so setting public to true seemed like the correct option. We may have misunderstood the meaning of public in this context though because /wp-admin/edit-tags.php?taxonomy=wp_pattern_category is still accessible even if it is set to false as you note.

As long as it's intended for use within the admin only, it should work with the public set to false. In my testing, the pattern categories are still accessible within:

  • Pattern Categories wp-admin/edit-tags.php?taxonomy=wp_pattern_category
  • Site editor
  • Post/Page editors

Should we put up a patch to switch this back to false?

As long as it is working in all admin UIs, then yes, please.

This ticket was mentioned in PR #5604 on WordPress/wordpress-develop by @glendaviesnz.


9 months ago
#6

  • Keywords has-patch added; needs-patch removed

## What?
Sets the pattern categories taxonomy public param to false instead of true

## Why?
It was suggested here that this would be a better setting for this taxonomy as it only needs to be accessed in admin, not the frontend.

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

#8 @glendaviesnz
9 months ago

@hellofromTonya ignore the notification you may have got asking for an explanation of the reasoning for this, after rereading the docs I see it is just a matter of limiting the permissions to the minimum needed, and there is no need for access to this taxonomy from the front-end.

@ramonopoly commented on PR #5604:


9 months ago
#9

I think the REST API fixtures need to be regenerated.

Running the unit test WP_Test_REST_Schema_Initialization should do it I think.

npm run test:php -- --filter WP_Test_REST_Schema_Initialization

@hellofromTonya commented on PR #5604:


9 months ago
#10

Hmm, the tests are failing. Pulling it down locally to have a peek.

#11 @hellofromTonya
9 months ago

Thank you @glendaviesnz for quickly investigating and creating the patch.

#12 @hellofromTonya
9 months ago

  • Keywords commit added
  • Owner set to hellofromTonya
  • Status changed from new to reviewing

Patch: https://github.com/WordPress/wordpress-develop/pull/5604

This patch is ready for commit.

#13 @hellofromTonya
9 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 57044:

Taxonomy: Set "public" to "false" for user pattern categories.

Changes the 'wp_pattern_category' taxonomy's 'public' argument to false.

Follow-up to [56642].

Props vrajadas, glendaviesnz, hellofromTonya, ramonopoly.
Fixes #59569.

#14 @hellofromTonya
9 months ago

  • Keywords dev-feedback added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for 2nd committer review to backport [57044] to the 6.4 branch.

#15 @azaozz
9 months ago

LGTM.

#17 @hellofromTonya
9 months ago

  • Keywords dev-reviewed added; dev-feedback removed

Marking as dev-reviewed with @azaozz and @mikeschroder reviewing it. Also see discussion in Make/Core slack.

#18 @hellofromTonya
9 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 57045:

Taxonomy: Set "public" to "false" for user pattern categories.

Changes the 'wp_pattern_category' taxonomy's 'public' argument to false.

Follow-up to [56642].

Reviewed by azaozz, mikeschroder.
Merges [57044] to the 6.4 branch.

Props vrajadas, glendaviesnz, hellofromTonya, ramonopoly.
Fixes #59569.

Note: See TracTickets for help on using tickets.