-
Notifications
You must be signed in to change notification settings - Fork 205
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
fix: Vendor categories tranlation compatibility stuff with WPML. #2364
fix: Vendor categories tranlation compatibility stuff with WPML. #2364
Conversation
WalkthroughThe changes involve significant modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
includes/ProductCategory/Categories.php (1)
Line range hint
32-32
: Correct typographical errors in documentation comments.The annotation
@sience
should be corrected to@since
to adhere to standard documentation practices.Apply this diff to correct the typographical errors:
- * @sience 3.6.2 + * @since 3.6.2Also applies to: 187-187
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- includes/ProductCategory/Categories.php (1 hunks)
Additional comments not posted (1)
includes/ProductCategory/Categories.php (1)
137-158
: Refactor ofget_categories
method enhances maintainability and security.The switch from direct SQL queries to using
get_terms
is a significant improvement. This change not only simplifies the code but also leverages WordPress's built-in functionality, which is more secure and compatible with plugins like WPML. The use ofarray_map
andarray_column
for data transformation and indexing is well implemented, enhancing the readability and efficiency of the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
includes/ProductCategory/Categories.php (2)
129-133
: Consider Using Pagination for Large Category SetsFetching all categories at once may lead to performance issues on sites with a vast number of categories.
Consider implementing pagination or lazy loading to improve performance:
$product_categories = get_terms( [ 'taxonomy' => 'product_cat', 'hide_empty' => false, + 'number' => 100, + 'offset' => 0, ] );You may need to loop through pages of categories if there are more than the specified limit.
23-23
: Maintain Consistency with Coding StandardsEnsure that all methods and properties have proper documentation blocks, and that annotations like
@since
are correctly spelled.For example, correct the typo in the docblock:
- * @sience 3.6.4 + * @since 3.6.4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- includes/ProductCategory/Categories.php (2 hunks)
🔇 Additional comments (1)
includes/ProductCategory/Categories.php (1)
129-133
:⚠️ Potential issueVerify Language Compatibility with WPML When Fetching Terms
Using
get_terms()
without specifying language parameters may not retrieve categories in the correct language when WPML is active. Ensure that the categories fetched are appropriately filtered based on the current language.Run the following script to verify that
get_terms()
respects the current language settings:If necessary, modify the arguments passed to
get_terms()
to include language considerations, possibly using WPML's language filters.
// set category data to cache | ||
Cache::set_transient( $transient_key, $this->categories, '', MONTH_IN_SECONDS ); | ||
} | ||
$this->get_categories(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential Performance Impact Due to Removal of Caching
By removing the transient caching mechanism in the get_all_categories()
method, the categories will be fetched from the database on every call. This could lead to increased database load and slower performance, especially on sites with a large number of categories or high traffic.
[performance]
Consider reintroducing caching to improve performance. You might implement caching with a unique key that respects the current language, ensuring compatibility with WPML.
// Transform the categories | ||
$transformed_categories = array_map( | ||
function ( $category ) { | ||
return (object) [ | ||
'term_id' => $category->term_id, | ||
'name' => $category->name, | ||
'parent_id' => $category->parent, | ||
]; | ||
}, | ||
$product_categories |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Simplify Data Transformation by Avoiding Unnecessary Object Casting and JSON Encoding
The current transformation process casts categories to objects and then decodes them back to arrays using JSON encoding. This is inefficient and can be simplified.
Refactor the code to work directly with arrays:
// Transform the categories
$transformed_categories = array_map(
function ( $category ) {
- return (object) [
+ return [
'term_id' => $category->term_id,
'name' => $category->name,
'parent_id' => $category->parent,
];
},
$product_categories
);
// Index the array by term_id.
$categories = array_column( $transformed_categories, null, 'term_id' );
- // Convert category data to array
- $this->categories = json_decode( wp_json_encode( $categories ), true );
- unset( $categories );
+ $this->categories = $categories;
This eliminates unnecessary conversions and improves code readability and performance.
Committable suggestion was skipped due to low confidence.
// Index the array by term_id. | ||
$categories = array_column( $transformed_categories, null, 'term_id' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure Proper Handling of Empty Categories
If $categories
is empty after processing, the method should return early to prevent errors in subsequent operations.
Add a condition to handle empty category data:
+ if ( empty( $categories ) ) {
+ $this->categories = [];
+ return;
+ }
This ensures that the class property $this->categories
is always set, and avoids potential undefined index errors later in the code.
Committable suggestion was skipped due to low confidence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…tegory-translation-compatible-with-wpml
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
https://github.com/getdokan/client-issue/issues/168,
https://github.com/getdokan/client-issue/issues/219
How to test the changes in this Pull Request:
Changelog entry
Fix: Vendor product category translation compatibility issue with WPML.
Detailed Description of the pull request. What was previous behaviour
and what will be changed in this PR.
Before Changes
Describe the issue before changes with screenshots(s).
After Changes
Describe the issue after changes with screenshot(s).
Feature Video (optional)
Link of detailed video if this PR is for a feature.
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit
New Features
Bug Fixes