update : support filter & pagination in Dokan product REST API v2 #2496
Conversation
WalkthroughThe pull request modifies the Changes
Assessment against linked issues
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
includes/REST/ProductControllerV2.php (1)
179-196: Good addition of flexible product ID filtering, with optional validationsThe new
includeandexcludeparameters allow for refined product queries. The usage ofwp_parse_id_listfor sanitization is aligned with WordPress best practices. Consider adding avalidate_callbackto guard against overly large arrays or unexpected ID ranges, preventing potential performance issues or malformed input.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
includes/REST/ProductControllerV2.php(2 hunks)
🔇 Additional comments (1)
includes/REST/ProductControllerV2.php (1)
121-121: Potential breaking change: method rename requires careful verification
The method has been renamed from get_product_collection_params() to get_collection_params(). Ensure all references in the codebase—including child classes and any dynamic invocations—are updated to point to the new method name to prevent runtime errors.
Would you like a shell script to scan for references to the old method name across the repository?
osmansufy
changed the title
osmansufy
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
includes/REST/ProductControllerV2.php (1)
Line range hint
74-85: Fix undefined variable in set_chosen_categories methodThe method uses
$requestvariable to accesschosen_catbut this variable is not passed as a parameter. This will cause a PHP notice/warning.Apply this diff to fix the issue:
- private function set_chosen_categories( $response ) { + private function set_chosen_categories( $response, $request ) { if ( ! is_wp_error( $response ) ) { $product = $response->get_data();Also update the calling methods:
public function create_item( $request ) { $response = parent::create_item( $request ); - $this->set_chosen_categories( $response ); + $this->set_chosen_categories( $response, $request ); return $response; } public function update_item( $request ) { $response = parent::update_item( $request ); - $this->set_chosen_categories( $response ); + $this->set_chosen_categories( $response, $request ); return $response; }
🧹 Nitpick comments (2)
includes/REST/ProductControllerV2.php (2)
234-240: Use consistent parameter access patternThe code shows inconsistent parameter access patterns. Some places use array access (
isset($request['per_page'])) while others use the getter method ($request->get_param()). For consistency and proper type handling, prefer usingget_param().Apply this diff:
$args = array_merge( $args, array( - 'posts_per_page' => isset( $request['per_page'] ) ? $request['per_page'] : 10, - 'paged' => isset( $request['page'] ) ? $request['page'] : 1, + 'posts_per_page' => $request->get_param( 'per_page' ) ? $request->get_param( 'per_page' ) : 10, + 'paged' => $request->get_param( 'page' ) ? $request->get_param( 'page' ) : 1, 'author' => dokan_get_current_user_id(), - 'orderby' => isset( $request['orderby'] ) ? $request['orderby'] : 'date', + 'orderby' => $request->get_param( 'orderby' ) ? $request->get_param( 'orderby' ) : 'date',🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[warning] 234-234:
Found precision alignment of 1 spaces.
[failure] 234-234:
Opening statement of multi-line function call not indented correctly; expected 8 spaces but found 9
[failure] 235-235:
Multi-line function call not indented correctly; expected 13 spaces but found 12
[failure] 236-236:
Multi-line function call not indented correctly; expected 13 spaces but found 12
234-236: Fix indentation in array_merge callThe indentation in the
array_mergecall is inconsistent with the codebase standards.Apply this diff to fix the indentation:
- $args = array_merge( - $args, - array( + $args = array_merge( + $args, + array(🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[warning] 234-234:
Found precision alignment of 1 spaces.
[failure] 234-234:
Opening statement of multi-line function call not indented correctly; expected 8 spaces but found 9
[failure] 235-235:
Multi-line function call not indented correctly; expected 13 spaces but found 12
[failure] 236-236:
Multi-line function call not indented correctly; expected 13 spaces but found 12
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
includes/REST/ProductControllerV2.php(3 hunks)
🧰 Additional context used
🪛 GitHub Check: Run PHPCS inspection
includes/REST/ProductControllerV2.php
[warning] 234-234:
Found precision alignment of 1 spaces.
[failure] 234-234:
Opening statement of multi-line function call not indented correctly; expected 8 spaces but found 9
[failure] 235-235:
Multi-line function call not indented correctly; expected 13 spaces but found 12
[failure] 236-236:
Multi-line function call not indented correctly; expected 13 spaces but found 12
🔇 Additional comments (1)
includes/REST/ProductControllerV2.php (1)
Line range hint 121-196: LGTM! Verify usage of new parameters
The implementation of get_collection_params including the new include and exclude parameters looks good. The parameters are properly defined with appropriate sanitization and validation.
Let's verify that the new parameters are properly handled in query preparation:
✅ Verification successful
The include/exclude parameters are properly handled in the query preparation
The verification shows that the include and exclude parameters are properly handled in the base DokanRESTController class's prepare_objects_query method:
$args['post__in']is set from$request['include']$args['post__not_in']is set from$request['exclude']
These arguments are then passed to WordPress query which will properly filter the results based on the included/excluded post IDs.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of include/exclude parameters in query preparation
ast-grep --pattern 'function prepare_objects_query($request) {
$$$
if ($request->get_param("include")) {
$$$
}
$$$
}'
Length of output: 131
Script:
#!/bin/bash
# Let's try a broader search for prepare_objects_query method
ast-grep --pattern 'function prepare_objects_query($$$) {
$$$
}'
# Also search for any usage of include/exclude parameters
rg -A 5 "get_param\(['\"]include['\"]|get_param\(['\"]exclude['\"]"
Length of output: 146
Script:
#!/bin/bash
# Let's try searching for the class containing prepare_objects_query
rg -A 10 "class ProductControllerV2"
# Also search for any file containing prepare_objects_query
fd -e php -x grep -l "prepare_objects_query"
# Let's also check the parent class to see if it handles these parameters
rg -A 5 "class ProductControllerV2 extends"
Length of output: 1231
Script:
#!/bin/bash
# Let's check the parent class's implementation
rg -A 20 "function prepare_objects_query" includes/REST/ProductController.php
# And check the abstract controller as well
rg -A 20 "function prepare_objects_query" includes/Abstracts/DokanRESTController.php
Length of output: 2127
osmansufy
changed the title
mrabbani
changed the title
mrabbani
changed the title
mrabbani
added
Needs: Author Reply
and removed
Needs: Dev Review
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
Title
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
includeandexcludeparameters for more flexible product querying.Refactor
get_product_collection_params()method toget_collection_params().posts_per_pageandpagedparameters based on incoming requests.