Enahance/product category rest api #2508
Conversation
WalkthroughThis pull request introduces a new REST API endpoint for managing product categories specifically for vendors in the Dokan plugin. The implementation includes a new Changes
Sequence DiagramsequenceDiagram
participant Client
participant REST API
participant ProductCategoriesVendorController
participant WordPress
Client->>REST API: GET Product Categories Request
REST API->>ProductCategoriesVendorController: Validate Request
ProductCategoriesVendorController->>WordPress: Query Product Categories
WordPress-->>ProductCategoriesVendorController: Return Categories
ProductCategoriesVendorController->>ProductCategoriesVendorController: Format Response
ProductCategoriesVendorController-->>REST API: Return Filtered Categories
REST API-->>Client: Send Response
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Finishing Touches
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: 5
🧹 Nitpick comments (3)
includes/REST/ProductCategoriesVendorController.php (1)
34-43: Simplify parameter retrieval by using sanitized and defaulted valuesThe parameters retrieved from the request can be accessed directly using
$request['param_name']. Since you've defined defaults and sanitization callbacks inget_collection_params(), these parameters are already sanitized and have default values applied. This simplifies the code and ensures consistency.Apply this diff to streamline parameter retrieval:
- $per_page = $request->get_param( 'per_page' ) ? $request->get_param( 'per_page' ) : 10; - $page = $request->get_param( 'page' ) ? $request->get_param( 'page' ) : 1; - $search = $request->get_param( 'search' ); - $exclude = $request->get_param( 'exclude' ); - $include = $request->get_param( 'include' ); - $order = $request->get_param( 'order' ); - $orderby = $request->get_param( 'orderby' ); - $hide_empty = $request->get_param( 'hide_empty' ); - $parent = $request->get_param( 'parent' ); - $fields = $request->get_param( '_fields' ); + $per_page = $request['per_page']; + $page = $request['page']; + $search = $request['search']; + $exclude = $request['exclude']; + $include = $request['include']; + $order = $request['order']; + $orderby = $request['orderby']; + $hide_empty = $request['hide_empty']; + $parent = $request['parent']; + $fields = $request['_fields'];tests/php/src/ProductCategory/ProductCategoryApiTest.php (2)
69-69: Remove debug statements from test methodsThere are
print_rstatements in your test methods, likely leftover from debugging. These should be removed to keep the test output clean.Apply this diff to remove the debug statements:
// Check if the response is an array and has 5 items - print_r( $data );$this->assertIsArray( $data ); - print_r( $data[0]['name'] );Also applies to: 97-97
80-99: Make the search test more reliableThe
test_get_categories_with_searchmethod may not reliably find categories matching the search term'term'since the generated category names are random. To ensure the test is robust, create a category with a specific name that includes the search term.Consider adjusting the test as follows:
// Create a category with a known name - $this->factory()->term->create_many( 10, [ 'taxonomy' => 'product_cat' ] ); + $search_term = 'TestTerm'; + $this->factory()->term->create( [ + 'taxonomy' => 'product_cat', + 'name' => $search_term, + ] ); $route = $this->get_route( $this->rest_base . '/' ); $request = new \WP_REST_Request( 'GET', $route ); $seller_id = $this->seller_id1; wp_set_current_user( $seller_id ); - $request->set_param( 'search', 'term' ); + $request->set_param( 'search', $search_term ); $response = $this->server->dispatch( $request ); $this->assertEquals( 200, $response->get_status() ); $data = $response->get_data(); // Check if the response includes the category with the search term $this->assertIsArray( $data ); $this->assertCount( 1, $data ); - $this->assertStringContainsStringIgnoringCase( 'term', $data[0]['name'] ); + $this->assertEquals( $search_term, $data[0]['name'] );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
includes/REST/Manager.php(1 hunks)includes/REST/ProductCategoriesVendorController.php(1 hunks)tests/php/src/ProductCategory/ProductCategoryApiTest.php(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: api tests (1, 1)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (1, 3)
🔇 Additional comments (2)
tests/php/src/ProductCategory/ProductCategoryApiTest.php (1)
21-25: Endpoint registration test is correctly implementedThe test
test_endpoint_existseffectively verifies that the REST API route for product categories is registered as expected.includes/REST/Manager.php (1)
204-204: Controller registration added to REST API managerThe
ProductCategoriesVendorControllerhas been correctly added to the REST API class map in theget_rest_api_class_mapmethod, ensuring that the new endpoint is registered and accessible.
| $include = $request->get_param( 'include' ); | ||
| $order = $request->get_param( 'order' ); | ||
| $orderby = $request->get_param( 'orderby' ); | ||
| $hide_empty = $request->get_param( 'hide_empty' ); |
There was a problem hiding this comment.
Use boolean values directly for the 'hide_empty' parameter
The parameter hide_empty is defined as a boolean in get_collection_params(). Comparing it to the string 'true' may not yield the expected results. You can use the boolean value directly in your arguments.
Apply this diff to correct the handling of the hide_empty parameter:
- $hide_empty = $request->get_param( 'hide_empty' );
...
- 'hide_empty' => $hide_empty === 'true',
+ $hide_empty = $request['hide_empty'];
...
+ 'hide_empty' => $hide_empty,Also applies to: 49-49
| $total_args = $args; | ||
| unset( $total_args['number'] ); | ||
| unset( $total_args['offset'] ); | ||
| $total_categories = wp_count_terms( $args ); |
There was a problem hiding this comment.
Ensure correct total category count for pagination
After unsetting 'number' and 'offset' from $total_args, you should use $total_args when calling wp_count_terms() to get an accurate total count for pagination. Currently, $args is used, which still contains 'number' and 'offset', potentially leading to incorrect counts.
Apply this diff to fix the total count calculation:
unset( $total_args['number'] );
unset( $total_args['offset'] );
- $total_categories = wp_count_terms( $args );
+ $total_categories = wp_count_terms( $total_args );📝 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.
| $total_args = $args; | |
| unset( $total_args['number'] ); | |
| unset( $total_args['offset'] ); | |
| $total_categories = wp_count_terms( $args ); | |
| $total_args = $args; | |
| unset( $total_args['number'] ); | |
| unset( $total_args['offset'] ); | |
| $total_categories = wp_count_terms( $total_args ); |
| 'description' => $category->description, | ||
| 'count' => (int) $category->count, | ||
| 'thumbnail' => $thumbnail_url, | ||
| 'link' => get_term_link( $category ), |
There was a problem hiding this comment.
Handle potential WP_Error from get_term_link
The get_term_link function can return a WP_Error object if there is an error generating the term link. To prevent potential issues, you should check for WP_Error before using the link.
Apply this diff to handle possible errors:
- 'link' => get_term_link( $category ),
+ $term_link = get_term_link( $category );
+ 'link' => is_wp_error( $term_link ) ? '' : $term_link,Committable suggestion skipped: line range outside the PR's diff.
| public function test_get_categories_with_exclude() { | ||
|
|
||
| // create 10 categories for testing with factory | ||
| $categories = $this->factory()->term->create_many( 10, [ 'taxonomy' => 'product_cat' ] ); | ||
| $exclude = $categories[0]; | ||
| // convert the exclude integer to string | ||
| $exclude = strval( $exclude ); | ||
| $route = $this->get_route( $this->rest_base . '/' ); | ||
| $request = new \WP_REST_Request( 'GET', $route ); | ||
| $seller_id = $this->seller_id1; | ||
| wp_set_current_user( $seller_id ); | ||
| $request->set_param( 'search', 'term' ); | ||
|
|
||
| $response = $this->server->dispatch( $request ); | ||
| $request->set_param( 'exclude', $exclude ); | ||
|
|
||
| $response = $this->server->dispatch( $request ); | ||
|
|
||
| $this->assertEquals( 200, $response->get_status() ); | ||
| $data = $response->get_data(); | ||
| // Check if the response is an array and does not include the excluded category | ||
| $this->assertIsArray( $data ); | ||
| $this->assertNotContains( $exclude, $data ); | ||
| } |
There was a problem hiding this comment.
Correct the test logic for excluding categories
In the test_get_categories_with_exclude method:
- The initial dispatch before setting the
'exclude'parameter is unnecessary. - The assertion
assertNotContains( $exclude, $data );may not work as intended because$datais an array of category arrays, not IDs.
Apply this diff to correct the test logic:
// Remove the unnecessary initial request
- $response = $this->server->dispatch( $request );
$request->set_param( 'exclude', $exclude );
$response = $this->server->dispatch( $request );
$this->assertEquals( 200, $response->get_status() );
$data = $response->get_data();
// Check if the response is an array and does not include the excluded category
$this->assertIsArray( $data );
- $this->assertNotContains( $exclude, $data );
+ foreach ( $data as $category ) {
+ $this->assertNotEquals( $exclude, $category['id'] );
+ }📝 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.
| public function test_get_categories_with_exclude() { | |
| // create 10 categories for testing with factory | |
| $categories = $this->factory()->term->create_many( 10, [ 'taxonomy' => 'product_cat' ] ); | |
| $exclude = $categories[0]; | |
| // convert the exclude integer to string | |
| $exclude = strval( $exclude ); | |
| $route = $this->get_route( $this->rest_base . '/' ); | |
| $request = new \WP_REST_Request( 'GET', $route ); | |
| $seller_id = $this->seller_id1; | |
| wp_set_current_user( $seller_id ); | |
| $request->set_param( 'search', 'term' ); | |
| $response = $this->server->dispatch( $request ); | |
| $request->set_param( 'exclude', $exclude ); | |
| $response = $this->server->dispatch( $request ); | |
| $this->assertEquals( 200, $response->get_status() ); | |
| $data = $response->get_data(); | |
| // Check if the response is an array and does not include the excluded category | |
| $this->assertIsArray( $data ); | |
| $this->assertNotContains( $exclude, $data ); | |
| } | |
| public function test_get_categories_with_exclude() { | |
| // create 10 categories for testing with factory | |
| $categories = $this->factory()->term->create_many( 10, [ 'taxonomy' => 'product_cat' ] ); | |
| $exclude = $categories[0]; | |
| // convert the exclude integer to string | |
| $exclude = strval( $exclude ); | |
| $route = $this->get_route( $this->rest_base . '/' ); | |
| $request = new \WP_REST_Request( 'GET', $route ); | |
| $seller_id = $this->seller_id1; | |
| wp_set_current_user( $seller_id ); | |
| $request->set_param( 'search', 'term' ); | |
| $request->set_param( 'exclude', $exclude ); | |
| $response = $this->server->dispatch( $request ); | |
| $this->assertEquals( 200, $response->get_status() ); | |
| $data = $response->get_data(); | |
| // Check if the response is an array and does not include the excluded category | |
| $this->assertIsArray( $data ); | |
| foreach ( $data as $category ) { | |
| $this->assertNotEquals( $exclude, $category['id'] ); | |
| } | |
| } |
| public function test_get_categories_with_include() { | ||
|
|
||
| // create 10 categories for testing with factory | ||
| $categories = $this->factory()->term->create_many( 10, [ 'taxonomy' => 'product_cat' ] ); | ||
| $include = $categories[0]; | ||
| // convert the include integer to string | ||
| $include = strval( $include ); | ||
| $route = $this->get_route( $this->rest_base . '/' ); | ||
| $request = new \WP_REST_Request( 'GET', $route ); | ||
| $seller_id = $this->seller_id1; | ||
| wp_set_current_user( $seller_id ); | ||
| $request->set_param( 'search', 'term' ); | ||
|
|
||
| $response = $this->server->dispatch( $request ); | ||
| $request->set_param( 'include', $include ); | ||
|
|
||
| $response = $this->server->dispatch( $request ); | ||
|
|
||
| $this->assertEquals( 200, $response->get_status() ); | ||
| $data = $response->get_data(); | ||
|
|
||
| // Check if the response is an array and includes the included category | ||
| $this->assertIsArray( $data ); | ||
| $this->assertCount( 1, $data ); | ||
| $this->assertEquals( $include, $data[0]['id'] ); | ||
| } |
There was a problem hiding this comment.
Correct the test logic for including categories
In the test_get_categories_with_include method:
- The initial dispatch before setting the
'include'parameter is unnecessary. - Ensure that the response data is correctly validated to contain only the included category.
Apply this diff to correct the test logic:
// Remove the unnecessary initial request
- $response = $this->server->dispatch( $request );
$request->set_param( 'include', $include );
$response = $this->server->dispatch( $request );
$this->assertEquals( 200, $response->get_status() );
$data = $response->get_data();
// Check if the response includes only the included category
$this->assertIsArray( $data );
$this->assertCount( 1, $data );
$this->assertEquals( $include, $data[0]['id'] );📝 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.
| public function test_get_categories_with_include() { | |
| // create 10 categories for testing with factory | |
| $categories = $this->factory()->term->create_many( 10, [ 'taxonomy' => 'product_cat' ] ); | |
| $include = $categories[0]; | |
| // convert the include integer to string | |
| $include = strval( $include ); | |
| $route = $this->get_route( $this->rest_base . '/' ); | |
| $request = new \WP_REST_Request( 'GET', $route ); | |
| $seller_id = $this->seller_id1; | |
| wp_set_current_user( $seller_id ); | |
| $request->set_param( 'search', 'term' ); | |
| $response = $this->server->dispatch( $request ); | |
| $request->set_param( 'include', $include ); | |
| $response = $this->server->dispatch( $request ); | |
| $this->assertEquals( 200, $response->get_status() ); | |
| $data = $response->get_data(); | |
| // Check if the response is an array and includes the included category | |
| $this->assertIsArray( $data ); | |
| $this->assertCount( 1, $data ); | |
| $this->assertEquals( $include, $data[0]['id'] ); | |
| } | |
| public function test_get_categories_with_include() { | |
| // create 10 categories for testing with factory | |
| $categories = $this->factory()->term->create_many( 10, [ 'taxonomy' => 'product_cat' ] ); | |
| $include = $categories[0]; | |
| // convert the include integer to string | |
| $include = strval( $include ); | |
| $route = $this->get_route( $this->rest_base . '/' ); | |
| $request = new \WP_REST_Request( 'GET', $route ); | |
| $seller_id = $this->seller_id1; | |
| wp_set_current_user( $seller_id ); | |
| $request->set_param( 'search', 'term' ); | |
| $request->set_param( 'include', $include ); | |
| $response = $this->server->dispatch( $request ); | |
| $this->assertEquals( 200, $response->get_status() ); | |
| $data = $response->get_data(); | |
| // Check if the response is an array and includes the included category | |
| $this->assertIsArray( $data ); | |
| $this->assertCount( 1, $data ); | |
| $this->assertEquals( $include, $data[0]['id'] ); | |
| } |
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
Tests