Make WordPress Core

Opened 5 years ago

Closed 3 years ago

Last modified 3 years ago

#49701 closed defect (bug) (fixed)

Quick Edit fails to populate taxonomy terms when show_in_quick_edit = true, and show_ui = false

Reported by: figureone's profile figureone Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.0 Priority: normal
Severity: normal Version: 5.4
Component: Quick/Bulk Edit Keywords: commit fixed-major has-patch has-unit-tests
Focuses: Cc:

Description (last modified by SergeyBiryukov)

Aloha, we discovered that the show_in_quick_edit property is not being checked when populating a post's taxonomy terms in /wp-admin/edit.php. As a consequence, if show_in_quick_edit is true, but show_ui is false for a given taxonomy, the Quick Edit UI will not check any of the term checkboxes, which can result in losing existing terms when a post is updated via Quick Edit.

Likely this was just an oversight when show_in_quick_edit was introduced here:
[31307].

And it should be an easy fix to check show_in_quick_edit instead of show_ui here:
https://github.com/WordPress/WordPress/blob/5.3.2/wp-admin/includes/template.php#L343
https://github.com/WordPress/WordPress/blob/5.3.2/wp-admin/includes/template.php#L354

show_in_quick_edit defaults to the value of show_ui if it isn't specified, so this change shouldn't introduce any issues.
https://github.com/WordPress/WordPress/blob/5.3.2/wp-includes/class-wp-taxonomy.php#L334

Our use case:
We have a taxonomy and terms defined in a plugin that is not meant to be edited in the WordPress UI. However, we do want users to be able to assign taxonomy terms to posts in the Quick Edit UI.

Extra details if needed:
When clicking the Quick Edit button, inline-edit-post.js refers to the term divs to mark the correct checkboxes:
https://github.com/WordPress/WordPress/blob/5.3.2/wp-admin/js/inline-edit-post.js#L305-L316
Term divs are echoed here:
https://github.com/WordPress/WordPress/blob/5.3.2/wp-admin/includes/template.php#L343-L364

Change History (27)

This ticket was mentioned in PR #205 on WordPress/wordpress-develop by figureone.


5 years ago
#1

Added tests for hierarchical and nonhierarchical taxonomies with show_ui = false and show_on_quick_edit = true.

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

#2 @figureone
5 years ago

  • Keywords has-unit-tests added

This ticket was mentioned in PR #207 on WordPress/wordpress-develop by figureone.


5 years ago
#3

Fixes 49701 by changing show_ui to show_in_quick_edit when deciding whether to echo the hidden div containing the assigned taxonomy terms for each post on the All Posts screen in WordPress Dashboard.

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

#4 @figureone
5 years ago

  • Keywords has-patch added

#5 @figureone
5 years ago

  • Keywords has-unit-tests has-patch removed

#6 @figureone
5 years ago

Removing keywords; not sure if I jumped the gun on those and that affected core workflow.

#7 @sabernhardt
5 years ago

Related/duplicate: #42916

(The same two changes were proposed for /wp-admin/includes/template.php)

#8 @SergeyBiryukov
3 years ago

  • Description modified (diff)
  • Milestone changed from Awaiting Review to 6.0

Hi there, welcome back to WordPress Trac!

Thanks for the ticket, sorry it took so long for someone to get back to you.

The patch does fix the issue in my testing.

#9 @SergeyBiryukov
3 years ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 52841:

Quick/Bulk Edit: Check the show_in_quick_edit taxonomy property when populating the data for the posts list table.

Previously, setting the show_in_quick_edit property to false removed the taxonomy from the inline edit form, but the terms were still being populated in the data for each table row via the get_inline_data() function, which only checked the $taxonomy->show_ui property.

This commit:

  • Improves performance by ensuring that taxonomy terms are not unnecessarily populated for each table row when show_in_quick_edit is false.
  • Properly populates the taxonomy terms when show_in_quick_edit is true and show_ui is false.

Follow-up to [31307].

Props jazbek, figureone, sabernhardt, ovidiul, webcommsat, SergeyBiryukov.
Fixes #42916, #49701.

figureone commented on PR #207:


3 years ago
#11

Thanks @SergeyBiryukov

There are also 2 unit tests in a separate pull request:
https://github.com/WordPress/wordpress-develop/pull/205

I just confirmed they do pass with the change you just merged (and fail without it).

Cheers!

SergeyBiryukov commented on PR #207:


3 years ago
#12

Great! I missed the second PR, thank you for bringing my attention to it. I will reopen the Trac ticket to commit the tests as well.

#13 @SergeyBiryukov
3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening to review and commit the unit tests from PR #205.

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


3 years ago

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


3 years ago

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


3 years ago

peterwilsoncc commented on PR #205:


3 years ago
#17

I've re-targeted this to trunk to account for the new default branch name and merged in an update to get the tests running.

#18 @costdev
3 years ago

  • Keywords commit added

I have reviewed the PR. The unit tests just need @covers annotations then I think it's good to go.

Adding commit to draw the attention of committers, as these changes can even be made just prior to commit.

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


3 years ago

audrasjb commented on PR #205:


3 years ago
#20

I committed changes suggested by @costdev to the PR :)
Now leaving this to @SergeyBiryukov for final commit :)

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


3 years ago

#22 @peterwilsoncc
3 years ago

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

In 53368:

Quick/Bulk Edit: Additional tests for showing taxonomies.

Additional tests to ensure taxonomies show in the quick/bulk edit froms based on the show_in_quick_edit setting rather than the the show_ui setting.

Follow up to [52841,31307].

Props figureone, costdev, audrasjb.
Fixes #49701.

#23 @peterwilsoncc
3 years ago

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

Reopening to merge [53368] to the 6.0 branch.

As these are test only changes, no second sign off is needed but I'll set up a PR to make sure the tests pass on the 6.0 branch.

peterwilsoncc commented on PR #205:


3 years ago
#24

These tests were merged in https://core.trac.wordpress.org/changeset/53368, thank you!

This ticket was mentioned in PR #2691 on WordPress/wordpress-develop by peterwilsoncc.


3 years ago
#25

  • Keywords has-patch has-unit-tests added

#26 @peterwilsoncc
3 years ago

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

In 53369:

Quick/Bulk Edit: Additional tests for showing taxonomies.

Additional tests to ensure taxonomies show in the quick/bulk edit froms based on the show_in_quick_edit setting rather than the the show_ui setting.

Follow up to [52841,31307].

Props figureone, costdev, audrasjb.
Merges [53368] to the 6.0 branch.
Fixes #49701.

Note: See TracTickets for help on using tickets.