-
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 dashboard product edit page variation products #2336
Conversation
WalkthroughThe recent changes streamline product searching and retrieval in the application by consolidating logic into fewer methods. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Ajax
participant Manager
participant Product
User->>Ajax: Search for products
Ajax->>Manager: Get linked products with criteria
Manager->>Product: Retrieve and filter products
Product-->>Manager: Return filtered product IDs
Manager-->>Ajax: Return product details
Ajax-->>User: Display product results
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: 1
Outside diff range, codebase verification and nitpick comments (1)
includes/Ajax.php (1)
654-660
: Ensure proper sanitization and validation ofexclude
,include
, andlimit
parameters.The
exclude
,include
, andlimit
parameters from$_GET
are directly passed to theget_linked_products
method without explicit sanitization or validation. This could pose a security risk. Ensure these parameters are properly sanitized and validated to prevent potential vulnerabilities.
- Review and sanitize the
exclude
,include
, andlimit
parameters in thejson_search_product
function.Analysis chain
Ensure proper sanitization and validation of GET parameters.
The
exclude
,include
, andlimit
parameters from$_GET
are directly passed to theget_linked_products
method. Ensure these parameters are properly sanitized and validated to prevent security vulnerabilities.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the proper sanitization and validation of GET parameters in `json_search_product`. # Test: Search for the function usage. Expect: Proper sanitization and validation of parameters. rg --type php -A 5 $'json_search_product'Length of output: 1752
Script:
#!/bin/bash # Description: Search for handling of `exclude`, `include`, and `limit` parameters in `json_search_product`. # Test: Search for the specific parameters within the function to check their sanitization and validation. rg --type php -A 10 $'public function json_search_product()' | rg 'exclude|include|limit'Length of output: 769
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- includes/Ajax.php (1 hunks)
- includes/Product/Manager.php (1 hunks)
- includes/Product/ProductAttribute.php (1 hunks)
- includes/REST/ProductAttributeController.php (1 hunks)
Additional comments not posted (3)
includes/REST/ProductAttributeController.php (1)
240-240
: Improved API response detail.The change in the
update_product_attribute
function enhances the response by providing detailed product attribute information. This improves the utility of the API for clients.Ensure that the consumers of this API are updated or notified about the change in the response structure.
includes/Product/ProductAttribute.php (1)
176-176
: Enhanced robustness in attribute handling.The condition in the
set
method now checks if$this->request_attributes
is an array, improving robustness and preventing potential errors.includes/Ajax.php (1)
654-662
: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
json_search_product
match the new implementation.Verification successful
Function usage verified for
json_search_product
.The function
json_search_product
is only used inadd_action
hooks withinincludes/Ajax.php
. No other calls or references were found in the codebase. The changes are correctly scoped.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `json_search_product` match the new implementation. # Test: Search for the function usage. Expect: Only occurrences of the new implementation. rg --type php -A 5 $'json_search_product'Length of output: 1752
Script:
#!/bin/bash # Comprehensive search for any usage of the `json_search_product` function across the codebase. rg --type php 'json_search_product'Length of output: 569
includes/Product/Manager.php
Outdated
public function get_linked_products( $term = '', $user_ids = false, $exclude = null, $included_id = null, $limit = 0 ) { | ||
$term = ! empty( $term ) ? sanitize_text_field( wp_unslash( $term ) ) : ''; | ||
$user_ids = ! empty( $user_ids ) ? array_filter( array_map( 'absint', (array) wp_unslash( $user_ids ) ) ) : false; | ||
|
||
if ( empty( $term ) ) { | ||
wp_die(); | ||
} | ||
|
||
$ids = dokan_search_seller_products( $term, $user_ids, '', true ); | ||
|
||
if ( ! empty( $exclude ) ) { | ||
$ids = array_diff( $ids, (array) sanitize_text_field( wp_unslash( $exclude ) ) ); | ||
} | ||
|
||
if ( ! empty( $included_id ) ) { | ||
$ids = array_intersect( $ids, (array) sanitize_text_field( wp_unslash( $included_id ) ) ); | ||
} | ||
|
||
if ( ! empty( $limit ) ) { | ||
$ids = array_slice( $ids, 0, absint( $limit ) ); | ||
} | ||
|
||
return array_filter( array_map( 'wc_get_product', $ids ), 'dokan_products_array_filter_editable' ); | ||
} |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- includes/Product/Manager.php (1 hunks)
Additional comments not posted (1)
includes/Product/Manager.php (1)
718-754
: Review ofget_linked_products
method: Well-implemented with considerations for security and functionality.The method
get_linked_products
is well-implemented with appropriate input sanitization and handling of parameters. It efficiently fetches and filters product IDs based on the provided criteria, which aligns with the PR's objectives to enhance product variation handling on the vendor dashboard.Suggestions:
- Performance: Ensure that the method's performance is evaluated, especially with large datasets. Consider implementing caching if the method is called frequently.
- Integration Testing: Verify the integration of this method with other components of the system, particularly how it interacts with the front-end and other backend services.
Verification successful
Integration of
get_linked_products
is correct but consider early input sanitization.The
get_linked_products
method is integrated intoincludes/Ajax.php
, where it is used to fetch product objects based on user input. While the method itself sanitizes inputs, it is advisable to sanitize inputs as early as possible in the code to prevent potential issues.
- Early Input Sanitization: Consider sanitizing
$_GET
parameters inincludes/Ajax.php
before passing them toget_linked_products
.- Performance Testing: Evaluate the performance of this AJAX call, especially with large datasets, and consider implementing caching if necessary.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify integration of `get_linked_products` with other system components. # Test: Search for method usage in frontend and backend services. Expect: Proper integration without issues. rg --type php -A 5 $'get_linked_products'Length of output: 1474
Issue: For variable products, the min-max quantity vales cannot be saved Please refer to the video attached @Aunshon bhai |
includes/Ajax.php
Outdated
if ( ! empty( $_GET['limit'] ) ) { | ||
$ids = array_slice( $ids, 0, absint( $_GET['limit'] ) ); | ||
} | ||
$product_objects = dokan()->product->get_linked_products( |
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.
Does the get_linked_products
method exist?
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
Summary by CodeRabbit
New Features
Bug Fixes
Refactor