Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixed Taxonomy Term Limit On Product Collection Filters #47155

Merged
merged 8 commits into from
May 13, 2024
Merged

Fixed Taxonomy Term Limit On Product Collection Filters #47155

merged 8 commits into from
May 13, 2024

Conversation

ObliviousHarmony
Copy link
Contributor

Submission Review Guidelines:

Changes proposed in this Pull Request:

This pull request refactors the TaxonomyItem component. Previously, useEntityRecords was used to populate a cache of all of the terms, but without any pagination, this list only contains the first 10 terms. We don't really need this cache since the query for existing terms and search terms will provide all of the information we need to display suggestions and render tokens. With this in mind I have moved towards using getEntityRecords directly from the data store. I was also able to reduce the amount of code in the process.

Screenshot 2024-05-03 at 17 58 08

There are now three different presentations for term names:

  • Terms that are missing (have been deleted but not removed from block) are notated as missing.
  • Terms with duplicates have the ID appended to differentiate them.
  • Terms will otherwise show the name.

Closes #46850.

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

  1. Add a "Product categories" and "Product tags" filter to a Product Collection block.
  2. Perform the search and add a category. Confirm that there is no limit to the number of terms allowed on the site.
  3. Create two categories with the same name and add them. Note that when they are both present they both have an ID.
  4. Delete a term after saving it to the block. It should show that it's missing in the filter block on next view.

Changelog entry

  • Automatically create a changelog entry from the details below.

Significance

  • Patch
  • Minor
  • Major

Type

  • Fix - Fixes an existing bug
  • Add - Adds functionality
  • Update - Update existing functionality
  • Dev - Development related task
  • Tweak - A minor adjustment to the codebase
  • Performance - Address performance issues
  • Enhancement - Improvement to existing functionality

Message

Comment

@ObliviousHarmony
Refactored Product Collection's TaxonomyItem
2b15166 
In the interest of avoiding pagination, this refactor replaces the
`useEntityRecords` hook with direct requests to the data store.
We are now loading the terms from the set IDs and the search
query and relying on those instead of a cache of all terms.
@ObliviousHarmony
Expanded Term Token Parsing
338dad5 
We're going to parse it into a Term object so that we can
use it to make determinations about whether there is a
duplicate or not.
@ObliviousHarmony
Remove Term ID From Display
0679198 
In an effort to clean up the presentation I have removed the
term ID from the display output.
@ObliviousHarmony
Improved Deleted Term Rendering
bee5f6b
@ObliviousHarmony ObliviousHarmony added the block: product collection Issues related to the Product Collection block label May 4, 2024
@woocommercebot woocommercebot requested review from a team and Aljullu and removed request for a team May 4, 2024 01:01
@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label May 4, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 4, 2024

Hi @Aljullu,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

@ObliviousHarmony ObliviousHarmony changed the title Refactored Product Collection's TaxonomyItem Fixed Taxonomy Term Limit On Product Collection Filters May 4, 2024
@ObliviousHarmony
Added Duplicate Term IDs
b4a70fb 
When rendering a term that has a duplicate name we will
also display the ID to differentiate between them.
Aljullu
Aljullu approved these changes May 6, 2024
View reviewed changes
Copy link
Contributor

@Aljullu Aljullu left a comment

Choose a reason for hiding this comment

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

Nice improvement! I tested it and everything works as expected. The code looks good as well.

I left a couple of suggestions, but pre-approving anyway. I also think it would be a good idea to add automated tests to make sure the duplicate taxonomies and missing taxonomies are rendered with the correct labels. But I don't think it should block this PR as those tests didn't exist before anyway.

plugins/woocommerce/changelog/fix-46850-collection-taxonomy-filter-pagination Outdated Show resolved Hide resolved
...ets/js/blocks/product-collection/edit/inspector-controls/taxonomy-controls/taxonomy-item.tsx Outdated Show resolved Hide resolved
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 9, 2024 •
edited
Loading

Test using WordPress Playground

The changes in this pull request can be previewed and tested using a WordPress Playground instance.
WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Test this pull request with WordPress Playground.

Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit.

@ObliviousHarmony ObliviousHarmony merged commit b8aabe9 into trunk May 13, 2024
35 checks passed
@ObliviousHarmony ObliviousHarmony deleted the fix/46850-collection-taxonomy-filter-pagination branch May 13, 2024 23:00
@github-actions github-actions bot added this to the 9.0.0 milestone May 13, 2024
@github-actions github-actions bot added the needs: analysis Indicates if the PR requires a PR testing scrub session. label May 13, 2024
@rodelgc rodelgc added needs: external testing Indicates if the PR requires further testing conducted by testers external to the development team. status: analysis complete Indicates if a PR has been analysed by Solaris and removed needs: analysis Indicates if the PR requires a PR testing scrub session. labels May 15, 2024
thealexandrelara pushed a commit that referenced this pull request Jul 18, 2024
@ObliviousHarmony @thealexandrelara
Fixed Taxonomy Term Limit On Product Collection Filters (#47155)
e1836ec 
In the interest of avoiding pagination this refactor swaps the current term loading with one that doesn't require a cache of all terms. We've removed the need for the cache by storing the ID and term name together in the token/suggestion list and using a display transformation to hide any unnecessary information.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Aljullu Aljullu Aljullu approved these changes

Assignees
No one assigned
Labels
block: product collection Issues related to the Product Collection block needs: external testing Indicates if the PR requires further testing conducted by testers external to the development team. plugin: woocommerce Issues related to the WooCommerce Core plugin. status: analysis complete Indicates if a PR has been analysed by Solaris
Projects
None yet
Milestone
9.0.0
Development

Successfully merging this pull request may close these issues.

Product Collection: Product Categories Filter Only Supports First 10 Categories
3 participants
@ObliviousHarmony @Aljullu @rodelgc