Skip to content

[Expreimental] Show preview taxonomy filter options in hierarchy order #60144

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.

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: tweak

Experimental: Show preview taxonomy filter options in hierarchy order.
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,13 @@ const CheckboxListEdit = ( props: EditProps ): JSX.Element => {
).map( ( item, index ) => (
<div
key={ index }
className="wc-block-product-filter-checkbox-list__item"
className={ clsx(
'wc-block-product-filter-checkbox-list__item',
{
[ `has-depth-${ item?.depth }` ]:
item?.depth,
}
) }
>
<label
htmlFor={ `interactive-checkbox-${ index }` }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,56 @@ import { Notice } from '../../components/notice';
import { getTaxonomyLabel } from './utils';
import { sortFilterOptions } from '../../utils/sort-filter-options';

// Create hierarchical structure: parents followed by their children
function createHierarchicalList(
terms: FilterOptionItem[],
sortOrder: string
) {
const children = new Map();

// First: categorize terms
terms.forEach( ( term ) => {
if ( ! children.has( term.parent ) ) {
children.set( term.parent, [] );
}
children.get( term.parent ).push( term );
} );

// Next: sort them
children.keys().forEach( ( key ) => {
children.set(
key,
sortFilterOptions( children.get( key ), sortOrder )
);
} );

// Last: build hierarchical list
const result: FilterOptionItem[] = [];
function addTermsRecursively(
termList: FilterOptionItem[],
depth = 0,
visited = new Set< number >()
) {
if ( depth > 10 ) {
return;
}
termList.forEach( ( term ) => {
if ( ! term.id || visited.has( term.id ) ) {
return;
}
visited.add( term.id );
result.push( { ...term, depth } );
const termChildren = children.get( term.id ) || [];
if ( termChildren.length > 0 ) {
addTermsRecursively( termChildren, depth + 1, visited );
}
} );
}

addTermsRecursively( children.get( 0 ) );
return result;
}

const Edit = ( props: EditProps ) => {
const { attributes: blockAttributes } = props;

Expand Down Expand Up @@ -58,23 +108,21 @@ const Edit = ( props: EditProps ) => {

const { getEntityRecords, hasFinishedResolution } =
select( coreStore );
const selectorArgs = [
'taxonomy',
taxonomy,
{
per_page: 15,
hide_empty: hideEmpty,
orderby: 'name',
order: 'asc',
},
];

const selectArgs = {
per_page: 15,
hide_empty: hideEmpty,
orderby: 'name',
order: 'asc',
};
return {
taxonomyTerms: getEntityRecords( ...selectorArgs ) || [],
isTermsLoading: ! hasFinishedResolution(
'getEntityRecords',
selectorArgs
),
taxonomyTerms:
getEntityRecords( 'taxonomy', taxonomy, selectArgs ) || [],
isTermsLoading: ! hasFinishedResolution( 'getEntityRecords', [
'taxonomy',
taxonomy,
selectArgs,
] ),
};
},
[ taxonomy, hideEmpty, isPreview ]
Expand Down Expand Up @@ -132,15 +180,21 @@ const Edit = ( props: EditProps ) => {
value: term.slug,
selected: false,
count,
id: term.id,
parent: term.parent || 0,
Comment on lines +183 to +184
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 4, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add validation for hierarchy properties.

The code assumes term.id exists and term.parent is a valid number when present, but doesn't validate these assumptions.

Apply this diff to add proper validation:

 				acc.push( {
 					label: term.name,
 					value: term.slug,
 					selected: false,
 					count,
-					id: term.id,
-					parent: term.parent || 0,
+					id: typeof term.id === 'number' ? term.id : 0,
+					parent: typeof term.parent === 'number' ? term.parent : 0,
 				} );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
id: term.id,
parent: term.parent || 0,
acc.push( {
label: term.name,
value: term.slug,
selected: false,
count,
id: typeof term.id === 'number' ? term.id : 0,
parent: typeof term.parent === 'number' ? term.parent : 0,
} );
🤖 Prompt for AI Agents
In
plugins/woocommerce/client/blocks/assets/js/blocks/product-filters/inner-blocks/taxonomy-filter/edit.tsx
around lines 173 to 174, the code assumes term.id exists and term.parent is a
valid number without validation. Add checks to ensure term.id is defined and
term.parent is a valid number before assigning them, defaulting to safe fallback
values if not valid, to prevent runtime errors.

Copy link
Member Author

Choose a reason for hiding this comment

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

Missing input validation: No checks for null/undefined inputs or required properties
Missing error handling: Assumes term.id and term.parent exist without validation

The input of function is built internally, not from users.

Copy link
Contributor

Choose a reason for hiding this comment

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

@dinhtungdu Thank you for the clarification! You're absolutely right - since the taxonomy terms come from WordPress core data APIs via getEntityRecords(), the data structure is controlled and consistent. The term.id and term.parent properties are guaranteed to be present and properly typed from the WordPress core data store.

I appreciate you explaining the context about internal vs external data sources. This makes sense given that WooCommerce can trust the data integrity from WordPress core APIs.


✏️ Learnings added
Learnt from: dinhtungdu
PR: woocommerce/woocommerce#60144
File: plugins/woocommerce/client/blocks/assets/js/blocks/product-filters/inner-blocks/taxonomy-filter/edit.tsx:183-184
Timestamp: 2025-08-04T14:32:56.238Z
Learning: In WooCommerce taxonomy filter components like plugins/woocommerce/client/blocks/assets/js/blocks/product-filters/inner-blocks/taxonomy-filter/edit.tsx, taxonomy term objects fetched from WordPress core data APIs via getEntityRecords() have guaranteed structure and don't require additional validation for properties like term.id and term.parent since they come from trusted internal WordPress core data sources.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: dinhtungdu
PR: woocommerce/woocommerce#60142
File: plugins/woocommerce/src/Internal/ProductFilters/TaxonomyHierarchyData.php:71-93
Timestamp: 2025-08-01T05:52:57.768Z
Learning: In plugins/woocommerce/src/Internal/ProductFilters/TaxonomyHierarchyData.php, the get_descendants() and get_ancestors() methods don't need extensive input validation because: 1) the taxonomy parameter is always validated upstream in the controlled WooCommerce core context, and 2) the null coalescing operator (??) already efficiently handles invalid term_id keys in the returned map.

Learnt from: dinhtungdu
PR: woocommerce/woocommerce#59900
File: plugins/woocommerce/client/blocks/assets/js/blocks/product-filters/inner-blocks/attribute-filter/inspector.tsx:0-0
Timestamp: 2025-07-24T05:37:00.907Z
Learning: The DisplayStyleSwitcher component in plugins/woocommerce/client/blocks/assets/js/blocks/product-filters/components/display-style-switcher/index.tsx has been updated so that its onChange prop accepts only a string type (not string | number | undefined), eliminating the need for type assertions when using this component.

Learnt from: dinhtungdu
PR: woocommerce/woocommerce#59499
File: plugins/woocommerce/src/Internal/ProductFilters/QueryClauses.php:327-332
Timestamp: 2025-07-10T04:22:27.648Z
Learning: In the WooCommerce ProductFilters QueryClauses class, the $chosen_taxonomies parameter in add_taxonomy_clauses() already contains only validated public product taxonomies. The validation occurs upstream in the Params class during parameter registration, so additional taxonomy existence validation in the processing methods is redundant.

Learnt from: samueljseay
PR: woocommerce/woocommerce#58716
File: plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/iapi-frontend.ts:83-101
Timestamp: 2025-06-17T07:07:53.443Z
Learning: In WooCommerce blocks, when porting existing code patterns that have known issues (like parseInt truncating decimal money values), maintain consistency with existing implementation rather than making isolated fixes. The preference is for systematic refactoring approaches (like broader Dinero adoption) over piecemeal changes.

Learnt from: triple0t
PR: woocommerce/woocommerce#59186
File: packages/js/email-editor/src/store/initial-state.ts:9-10
Timestamp: 2025-06-26T12:13:32.062Z
Learning: In WooCommerce email editor store initialization (packages/js/email-editor/src/store/initial-state.ts), the current_post_id and current_post_type from window.WooCommerceEmailEditor are required parameters that should cause explicit errors if missing, rather than using fallback values or optional chaining. The design preference is to fail fast when critical initialization data is unavailable.

Learnt from: samueljseay
PR: woocommerce/woocommerce#59051
File: plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/mini-cart-contents/inner-blocks/mini-cart-footer-block/index.tsx:66-70
Timestamp: 2025-06-23T05:47:52.696Z
Learning: For WooCommerce mini-cart blocks in plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/, the standardized conditional pattern for experimental features should be `if ( isExperimentalMiniCartEnabled() ) { blockSettings.save = () => <InnerBlocks.Content />; }` - defaulting to the traditional Save component and only overriding when the experimental feature is enabled.

Learnt from: jorgeatorres
PR: woocommerce/woocommerce#59675
File: .github/workflows/release-bump-as-requirement.yml:48-65
Timestamp: 2025-07-15T15:39:21.856Z
Learning: In WooCommerce core repository, changelog entries for all PRs live in `plugins/woocommerce/changelog/` directory and are processed during releases, not at the repository root level.

Learnt from: CR
PR: woocommerce/woocommerce#0
File: .cursor/rules/generate-pr-description.mdc:0-0
Timestamp: 2025-07-21T05:22:46.426Z
Learning: Provide clear, step-by-step instructions for how to test the changes in the PR description.

Learnt from: lysyjan
PR: woocommerce/woocommerce#59632
File: packages/js/email-editor/src/layouts/flex-email.tsx:116-122
Timestamp: 2025-07-14T10:41:46.200Z
Learning: In WooCommerce projects, formatting suggestions should respect the project's Prettier configuration and linting rules. Changes that would break the lint job should be avoided, even if they appear to improve readability.

} );

return acc;
},
[]
);

// Sort the processed terms
setTermOptions( sortFilterOptions( processedTerms, sortOrder ) );
// Create hierarchical structure then apply sorting
const hierarchicalTerms = createHierarchicalList(
processedTerms,
sortOrder
);
setTermOptions( hierarchicalTerms );
setIsOptionsLoading( false );
}, [
taxonomy,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ export type FilterOptionItem = (
value: string;
selected?: boolean;
count: number;
id?: number;
parent?: number;
depth?: number;
};

export type FilterBlockContext = {
Expand Down
Loading