Enhancement: product commission bulk edit #2464
Conversation
Aunshon
added
Needs: Testing
WalkthroughThe changes in this pull request enhance the functionality of the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
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: 4
🧹 Outside diff range and nitpick comments (7)
includes/Product/Hooks.php (3)
541-541: Ensure Default Commission Type is Properly SetThe ternary condition can be simplified to ensure
Fixed::SOURCEis always used as the default when the commission type is not provided.Simplify the assignment as follows:
- $commission_type = ! empty( $data['_per_product_admin_commission_type'] ) ? sanitize_text_field( $data['_per_product_admin_commission_type'] ) : Fixed::SOURCE; + $commission_type = isset( $data['_per_product_admin_commission_type'] ) && ! empty( $data['_per_product_admin_commission_type'] ) + ? sanitize_text_field( $data['_per_product_admin_commission_type'] ) + : Fixed::SOURCE;
553-554: Use Appropriate Sanitization for Numeric InputsUsing
sanitize_text_fieldfor numeric inputs may not be ideal. Consider usingwc_format_decimaldirectly after ensuring the input is sanitized appropriately.Refactor as follows:
- $additional_fee = ( '' === $data['_per_product_admin_additional_fee'] ) ? '' : sanitize_text_field( $data['_per_product_admin_additional_fee'] ); - $additional_fee = wc_format_decimal( $additional_fee ); + $additional_fee_input = isset( $data['_per_product_admin_additional_fee'] ) ? $data['_per_product_admin_additional_fee'] : ''; + $additional_fee = ( '' === $additional_fee_input ) ? '' : wc_format_decimal( $additional_fee_input );
557-564: Check for Successful Saving of Commission SettingsAfter calling
dokan()->product->save_commission_settings(), it's good practice to verify that the settings were saved successfully and handle any errors if needed.Consider adding error handling:
dokan()->product->save_commission_settings( $post_id, [ 'type' => $commission_type, 'percentage' => $admin_commission, 'flat' => $additional_fee, ] ); + // You may want to add logging or error handling here in case saving fails.templates/products/dokan-products-edit-bulk-commission.php (3)
19-19: Associate Label with Checkbox for AccessibilityThe
<label>element should be properly associated with the<input>checkbox by adding theforattribute, enhancing accessibility.Modify the label tag as follows:
- <input type="checkbox" name="dokan_override_bulk_product_commission" id="dokan_override_bulk_product_commission"> + <input type="checkbox" name="dokan_override_bulk_product_commission" id="dokan_override_bulk_product_commission"> </label>
31-31: Add Input Validation for Commission PercentageThe commission percentage input should have validation to ensure values between 0 and 100 are entered.
Add
stepandmaxattributes:<input class="input-text wc_input_price" min="0" max="100" type="text" name="_per_product_admin_commission" value=""/> +<input class="input-text wc_input_price" min="0" max="100" step="0.01" type="number" name="_per_product_admin_commission" value=""/>
33-33: Ensure Consistent Input TypesFor the additional fee input, consider using
type="number"to match the commission percentage input and facilitate numeric input validation.Modify the input type:
-<input type="text" name="_per_product_admin_additional_fee" class="input-text wc_input_price" value=""> +<input type="number" name="_per_product_admin_additional_fee" class="input-text wc_input_price" value="">includes/Admin/Hooks.php (1)
253-255: Improve Conditional Logic for Better ReadabilityThe conditional statement can be simplified for clarity by returning early if the product type is excluded.
Refactor the condition:
- if ( ! current_user_can( 'manage_woocommerce' ) || in_array( $product->get_type(), $excluded_product_types, true ) ) { + if ( ! current_user_can( 'manage_woocommerce' ) ) { + return; + } + if ( in_array( $product->get_type(), $excluded_product_types, true ) ) { return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
includes/Admin/Hooks.php(3 hunks)includes/Product/Hooks.php(3 hunks)templates/products/dokan-products-edit-bulk-commission.php(1 hunks)
| $_per_product_admin_commission = wc_format_decimal( sanitize_text_field( $data['_per_product_admin_commission'] ) ); | ||
|
|
||
| if ( 0 <= $_per_product_admin_commission && 100 >= $_per_product_admin_commission ) { | ||
| $admin_commission = ( '' === $_POST['_per_product_admin_commission'] ) ? '' : $_per_product_admin_commission; // phpcs:ignore | ||
| $admin_commission = ( '' === $data['_per_product_admin_commission'] ) ? '' : $_per_product_admin_commission; |
There was a problem hiding this comment.
Validate Commission Percentage Input
When processing the commission percentage, consider adding validation to ensure the input is numeric and within the expected range (0 to 100). Currently, non-numeric input could cause unintended behavior.
Add a check to validate the input:
$_per_product_admin_commission = wc_format_decimal( sanitize_text_field( $data['_per_product_admin_commission'] ) );
+ if ( ! is_numeric( $_per_product_admin_commission ) ) {
+ $_per_product_admin_commission = 0;
+ }
if ( 0 <= $_per_product_admin_commission && 100 >= $_per_product_admin_commission ) {
$admin_commission = ( '' === $data['_per_product_admin_commission'] ) ? '' : $_per_product_admin_commission;
}📝 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.
| $_per_product_admin_commission = wc_format_decimal( sanitize_text_field( $data['_per_product_admin_commission'] ) ); | |
| if ( 0 <= $_per_product_admin_commission && 100 >= $_per_product_admin_commission ) { | |
| $admin_commission = ( '' === $_POST['_per_product_admin_commission'] ) ? '' : $_per_product_admin_commission; // phpcs:ignore | |
| $admin_commission = ( '' === $data['_per_product_admin_commission'] ) ? '' : $_per_product_admin_commission; | |
| $_per_product_admin_commission = wc_format_decimal( sanitize_text_field( $data['_per_product_admin_commission'] ) ); | |
| if ( ! is_numeric( $_per_product_admin_commission ) ) { | |
| $_per_product_admin_commission = 0; | |
| } | |
| if ( 0 <= $_per_product_admin_commission && 100 >= $_per_product_admin_commission ) { | |
| $admin_commission = ( '' === $data['_per_product_admin_commission'] ) ? '' : $_per_product_admin_commission; |
| public static function save_per_product_commission_options( $post_id, $data = [] ) { | ||
| if ( ! current_user_can( 'manage_woocommerce' ) ) { | ||
| return; | ||
| } | ||
|
|
||
| $commission_type = ''; | ||
| $commission_type = Fixed::SOURCE; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor to Avoid Direct Access to $_POST and PHPCS Ignore
Accessing $_POST directly is discouraged due to security and code quality concerns. The use of // phpcs:ignore suppresses potential warnings, but it's better to adhere to coding standards.
Consider refactoring to use filter_input_array for better input handling and compliance:
public static function save_per_product_commission_options( $post_id, $data = [] ) {
if ( ! current_user_can( 'manage_woocommerce' ) ) {
return;
}
- $data = empty( $data ) ? $_POST : $data; // phpcs:ignore
+ if ( empty( $data ) ) {
+ $data = filter_input_array( INPUT_POST, [
+ '_per_product_admin_commission_type' => FILTER_SANITIZE_STRING,
+ '_per_product_admin_commission' => FILTER_SANITIZE_STRING,
+ '_per_product_admin_additional_fee' => FILTER_SANITIZE_STRING,
+ ] );
+ }Committable suggestion skipped: line range outside the PR's diff.
includes/Admin/Hooks.php
Outdated
| if ( ! isset( $_REQUEST['dokan_override_bulk_product_commission'] ) || sanitize_text_field( $_REQUEST['dokan_override_bulk_product_commission'] ) !== 'on' ) { // phpcs:ignore | ||
| return; | ||
| } |
There was a problem hiding this comment.
Add Nonce Verification to Prevent CSRF Attacks
There is no nonce verification when processing bulk edit form data. This can expose the function to Cross-Site Request Forgery (CSRF) attacks.
Implement nonce verification to enhance security:
+ if ( ! isset( $_REQUEST['_wpnonce'] ) || ! wp_verify_nonce( sanitize_key( $_REQUEST['_wpnonce'] ), 'bulk_edit_product' ) ) {
+ return;
+ }
if ( ! isset( $_REQUEST['dokan_override_bulk_product_commission'] ) || sanitize_text_field( $_REQUEST['dokan_override_bulk_product_commission'] ) !== 'on' ) { // phpcs:ignore
return;
}📝 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.
| if ( ! isset( $_REQUEST['dokan_override_bulk_product_commission'] ) || sanitize_text_field( $_REQUEST['dokan_override_bulk_product_commission'] ) !== 'on' ) { // phpcs:ignore | |
| return; | |
| } | |
| if ( ! isset( $_REQUEST['_wpnonce'] ) || ! wp_verify_nonce( sanitize_key( $_REQUEST['_wpnonce'] ), 'bulk_edit_product' ) ) { | |
| return; | |
| } | |
| if ( ! isset( $_REQUEST['dokan_override_bulk_product_commission'] ) || sanitize_text_field( $_REQUEST['dokan_override_bulk_product_commission'] ) !== 'on' ) { // phpcs:ignore | |
| return; | |
| } |
| return; | ||
| } | ||
|
|
||
| ProductHooks::save_per_product_commission_options( $product->get_id(), $_REQUEST ); // phpcs:ignore |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid Direct Use of $_REQUEST and Suppressing PHPCS Warnings
Directly accessing $_REQUEST and ignoring PHPCS warnings is discouraged. Instead, sanitize and validate the input properly without suppressing coding standards.
Refactor to sanitize inputs and adhere to coding standards:
- ProductHooks::save_per_product_commission_options( $product->get_id(), $_REQUEST ); // phpcs:ignore
+ $sanitized_data = filter_input_array( INPUT_POST, [
+ '_per_product_admin_commission_type' => FILTER_SANITIZE_STRING,
+ '_per_product_admin_commission' => FILTER_SANITIZE_STRING,
+ '_per_product_admin_additional_fee' => FILTER_SANITIZE_STRING,
+ ] );
+ ProductHooks::save_per_product_commission_options( $product->get_id(), $sanitized_data );Committable suggestion skipped: line range outside the PR's diff.
shashwatahalder01
added
QA In Progress
and removed
Needs: Testing
mrabbani
changed the title
mrabbani
added
👍 Dev Review Done
and removed
Needs: Dev Review
|
Scenario: 
|
@jamil-mahmud vai please check this out. |
Instead of the checkbox, a dropdown will be more appropriate for this scenario to maintain consistency with other edit fields. |
Skip reverse withdrawal and advertisement product id
Skip reverse withdrawal and advertisement product id
shashwatahalder01
added
QA approved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
includes/Product/Hooks.php (1)
535-541: Consider making commission type configurableThe commission type is hardcoded to
Fixed::SOURCE. This might limit flexibility if other commission types need to be supported in the future.- $commission_type = Fixed::SOURCE; + $commission_type = apply_filters( 'dokan_product_commission_type_default', Fixed::SOURCE );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
includes/Product/Hooks.php(2 hunks)
🔇 Additional comments (3)
includes/Product/Hooks.php (3)
557-564: LGTM! Well-structured commission settings
The commission settings are saved in a clean and organized manner using the appropriate Dokan method.
530-538:
Improve input data handling and validation
The direct use of $_POST superglobal should be avoided. Consider using WordPress or WooCommerce functions for safer input handling.
- $data = empty( $data ) ? $_POST : $data; // phpcs:ignore
+ if ( empty( $data ) ) {
+ $data = array(
+ '_per_product_admin_commission_type' => isset( $_POST['_per_product_admin_commission_type'] ) ? sanitize_text_field( wp_unslash( $_POST['_per_product_admin_commission_type'] ) ) : '',
+ '_per_product_admin_commission' => isset( $_POST['_per_product_admin_commission'] ) ? sanitize_text_field( wp_unslash( $_POST['_per_product_admin_commission'] ) ) : '',
+ '_per_product_admin_additional_fee' => isset( $_POST['_per_product_admin_additional_fee'] ) ? sanitize_text_field( wp_unslash( $_POST['_per_product_admin_additional_fee'] ) ) : '',
+ );
+ }544-548:
Enhance commission percentage validation
The code validates the range but should also ensure numeric input to prevent potential issues.
if ( isset( $data['_per_product_admin_commission'] ) ) {
$_per_product_admin_commission = wc_format_decimal( sanitize_text_field( $data['_per_product_admin_commission'] ) );
+ if ( ! is_numeric( $_per_product_admin_commission ) ) {
+ return;
+ }
+
if ( 0 <= $_per_product_admin_commission && 100 >= $_per_product_admin_commission ) {
$admin_commission = ( '' === $data['_per_product_admin_commission'] ) ? '' : $_per_product_admin_commission;
}Likely invalid or redundant comment.
| if ( isset( $data['_per_product_admin_additional_fee'] ) ) { | ||
| $additional_fee = ( '' === $data['_per_product_admin_additional_fee'] ) ? '' : sanitize_text_field( $data['_per_product_admin_additional_fee'] ); | ||
| $additional_fee = wc_format_decimal( $additional_fee ); |
There was a problem hiding this comment.
Add validation for additional fee
The additional fee should be validated to prevent negative values.
if ( isset( $data['_per_product_admin_additional_fee'] ) ) {
$additional_fee = ( '' === $data['_per_product_admin_additional_fee'] ) ? '' : sanitize_text_field( $data['_per_product_admin_additional_fee'] );
$additional_fee = wc_format_decimal( $additional_fee );
+ if ( $additional_fee < 0 ) {
+ $additional_fee = 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.
| if ( isset( $data['_per_product_admin_additional_fee'] ) ) { | |
| $additional_fee = ( '' === $data['_per_product_admin_additional_fee'] ) ? '' : sanitize_text_field( $data['_per_product_admin_additional_fee'] ); | |
| $additional_fee = wc_format_decimal( $additional_fee ); | |
| if ( isset( $data['_per_product_admin_additional_fee'] ) ) { | |
| $additional_fee = ( '' === $data['_per_product_admin_additional_fee'] ) ? '' : sanitize_text_field( $data['_per_product_admin_additional_fee'] ); | |
| $additional_fee = wc_format_decimal( $additional_fee ); | |
| if ( $additional_fee < 0 ) { | |
| $additional_fee = 0; | |
| } |
* chore: bump version to 3.14.0 * Add geolocation map tests * Update feature map * fix coverage map * Auction tests (getdokan#2452) * Add a test * Update feature map * Fix lint issue * change filename temporary * Revert filename to original * Fix notice test * Fix commission test * Add new tests to featuremap (getdokan#2453) * Add new tests to feature map * Add new tests to featuremap * Skip shipstation tests * Add notice for version compatibility. (getdokan#2450) * Add notice for version compatibility. * Added dokan pro update message. * Added dokan pro update message. * Added dokan pro update message. * fix: plugin version after activation (getdokan#2447) * chore: bump version to 3.14.0 * chore: Released Version 3.14.0 * Fix commission upgrader (getdokan#2463) * Fix commission upgrader * Add database backup message in upgrade. * chore: Released Version 3.14.1 * Fix skipped product tests (getdokan#2457) * Fix and update skipped product tests * Update a variable * Add store list & reviews test (getdokan#2460) * Fix skipped store tests * Add store reviews tests * Update comment * skipped a test * Enhancement: product commission bulk edit (getdokan#2464) * Remove $commission_type variable it was not used * Save fixed as default commission type * Save bulk product commission. * Update bulk edit ui Skip reverse withdrawal and advertisement product id * Update bulk edit ui Skip reverse withdrawal and advertisement product id * Revert alignment * chore: bump version to v3.14.2 * Add Commission tests (getdokan#2471) * add new commission tests * Add commission tests * Add vendor coupon tests (getdokan#2470) * Add vendor coupon tests * Update test tags --------- Co-authored-by: Aunshon <[email protected]> Co-authored-by: Aunshon <[email protected]> Co-authored-by: KAMRUZZAMAN <[email protected]>
* chore: bump version to 3.14.0 * Add dokan tracking tests * Add commission meta box test * Update feature map * Update config file * Update feature map * Add vendor filters test * Add a shortcode test * Fix commission tests * Add geolocation tests * Auction tests (getdokan#2452) * Add a test * Update feature map * Fix lint issue * change filename temporary * Revert filename to original * Fix notice test * Fix commission test * Add new tests to featuremap (getdokan#2453) * Add new tests to feature map * Add new tests to featuremap * Skip shipstation tests * Add notice for version compatibility. (getdokan#2450) * Add notice for version compatibility. * Added dokan pro update message. * Added dokan pro update message. * Added dokan pro update message. * fix: plugin version after activation (getdokan#2447) * chore: bump version to 3.14.0 * chore: Released Version 3.14.0 * Fix commission upgrader (getdokan#2463) * Fix commission upgrader * Add database backup message in upgrade. * chore: Released Version 3.14.1 * Fix skipped product tests (getdokan#2457) * Fix and update skipped product tests * Update a variable * Add store list & reviews test (getdokan#2460) * Fix skipped store tests * Add store reviews tests * Update comment * skipped a test * Enhancement: product commission bulk edit (getdokan#2464) * Remove $commission_type variable it was not used * Save fixed as default commission type * Save bulk product commission. * Update bulk edit ui Skip reverse withdrawal and advertisement product id * Update bulk edit ui Skip reverse withdrawal and advertisement product id * Revert alignment * chore: bump version to v3.14.2 * Add Commission tests (getdokan#2471) * add new commission tests * Add commission tests * Add vendor coupon tests (getdokan#2470) * Add vendor coupon tests * Update test tags --------- Co-authored-by: Aunshon <[email protected]> Co-authored-by: Aunshon <[email protected]> Co-authored-by: KAMRUZZAMAN <[email protected]>
* Add booking product details test * Add booking product tests * Fix mix-max flaky issue * Update product name & id reference * update tags test * chore: bump version to 3.14.0 * Fix failed tests * Fix flaky tests * Fix commission tests * Update commission test logic * Add new tests & update feature map * Auction tests (getdokan#2452) * Add a test * Update feature map * Fix lint issue * change filename temporary * Revert filename to original * Fix notice test * Fix commission test * Add new tests to featuremap (getdokan#2453) * Add new tests to feature map * Add new tests to featuremap * Skip shipstation tests * Add notice for version compatibility. (getdokan#2450) * Add notice for version compatibility. * Added dokan pro update message. * Added dokan pro update message. * Added dokan pro update message. * fix: plugin version after activation (getdokan#2447) * chore: bump version to 3.14.0 * chore: Released Version 3.14.0 * Fix commission upgrader (getdokan#2463) * Fix commission upgrader * Add database backup message in upgrade. * chore: Released Version 3.14.1 * Fix skipped product tests (getdokan#2457) * Fix and update skipped product tests * Update a variable * Add store list & reviews test (getdokan#2460) * Fix skipped store tests * Add store reviews tests * Update comment * skipped a test * Enhancement: product commission bulk edit (getdokan#2464) * Remove $commission_type variable it was not used * Save fixed as default commission type * Save bulk product commission. * Update bulk edit ui Skip reverse withdrawal and advertisement product id * Update bulk edit ui Skip reverse withdrawal and advertisement product id * Revert alignment * chore: bump version to v3.14.2 * Add changes * Add Commission tests (getdokan#2471) * add new commission tests * Add commission tests * Add vendor coupon tests (getdokan#2470) * Add vendor coupon tests * Update test tags --------- Co-authored-by: Aunshon <[email protected]> Co-authored-by: Aunshon <[email protected]> Co-authored-by: KAMRUZZAMAN <[email protected]>
All Submissions:
Changes proposed in this Pull Request:
Added commission setting in product bulk edit.
Closes
How to test the changes in this Pull Request:
[ 'product_pack', 'external', 'grouped' ]products these products will not be affected even if the checkbox enabled.Changelog entry
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation