🎯 fix: DB related PHPCS security issues #2271
Conversation
devAsadNur
changed the title
devAsadNur
added
Needs: Testing
WalkthroughThe recent updates primarily modify 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 (
|
![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
Outside diff range and nitpick comments (10)
includes/DummyData/Importer.php (5)
Line range hint
5-5: Remove leading backslash from import statement.The use statement for
WP_Errorshould not start with a leading backslash as per PHP standards.- use \WP_Error; + use WP_Error;Tools
GitHub Check: Run PHPCS inspection
[failure] 334-334:
Functions must not contain multiple empty lines in a row; found 2 empty lines
Line range hint
6-6: Remove leading backslash from import statement.The use statement for
WP_Queryshould not start with a leading backslash.- use \WP_Query; + use WP_Query;Tools
GitHub Check: Run PHPCS inspection
[failure] 334-334:
Functions must not contain multiple empty lines in a row; found 2 empty lines
Line range hint
8-8: Add file-level docblock after the opening PHP tag.The file should start with a file-level docblock to describe the purpose and scope of the file, immediately following the
<?phptag./** * Handles importing of dummy data for Dokan plugin. */Tools
GitHub Check: Run PHPCS inspection
[failure] 334-334:
Functions must not contain multiple empty lines in a row; found 2 empty lines
Line range hint
246-246: Use pre-increment instead of post-increment and fix spacing.Change the post-increment operator to pre-increment for performance optimization and remove the extra space.
- $index ++; + ++$index;Tools
GitHub Check: Run PHPCS inspection
[failure] 334-334:
Functions must not contain multiple empty lines in a row; found 2 empty lines
Line range hint
276-277: Optimize meta key usage and fix array formatting.Using
meta_keydirectly can lead to slow database queries. Consider using a custom query or indexing. Also, add a comma after the last array item in a multi-line array.$args = [ 'post_type' => 'product', 'posts_per_page' => -1, 'post_status' => 'any', 'fields' => 'ids', 'meta_key' => 'dokan_dummy_data', // Consider optimizing this query 'meta_value' => '1', ];Tools
GitHub Check: Run PHPCS inspection
[failure] 334-334:
Functions must not contain multiple empty lines in a row; found 2 empty linesincludes/Withdraw/Manager.php (1)
Line range hint
535-549: Validate and optimize the summary retrieval method.The
get_user_withdraw_summarymethod uses a direct database query to compute a summary of withdrawals. While the use ofphpcs:disableis necessary, ensure that all input data is properly sanitized and validated. Additionally, consider using more efficient SQL queries or caching mechanisms to optimize performance.+ $cache_summary = wp_cache_get($cache_key, $cache_group); + if (false !== $cache_summary) { + return $cache_summary; + } - $results = $wpdb->get_row(...); + wp_cache_set($cache_key, $results, $cache_group);includes/Product/functions.php (1)
Line range hint
402-423: Address incorrect number of replacements in SQL query.The function
dokan_search_seller_productsuses a complex SQL query to search for products. The static analysis tool flagged an issue with the incorrect number of replacements passed to$wpdb->prepare(). This needs to be corrected to prevent potential SQL injection vulnerabilities.- $product_ids = $wpdb->get_col($wpdb->prepare(...)); + $product_ids = $wpdb->get_col($wpdb->prepare( + "SELECT DISTINCT posts.ID FROM {$wpdb->posts} posts + LEFT JOIN {$wpdb->postmeta} postmeta ON posts.ID = postmeta.post_id + $type_join + WHERE ( + posts.post_title LIKE %s + OR posts.post_content LIKE %s + OR ( + postmeta.meta_key = '_sku' AND postmeta.meta_value LIKE %s + ) + ) + AND posts.post_type IN ('" . implode( "','", $post_types ) . "') + AND posts.post_status IN ('" . implode( "','", $post_statuses ) . "') + $type_where + $users_where + ORDER BY posts.post_parent ASC, posts.post_title ASC", + $query_args + ));includes/Commission.php (1)
Line range hint
571-571: Avoid using reserved keywords as parameter namesThe use of
callableas a parameter name should be avoided as it is a reserved keyword in PHP.- public function prepare_for_calculation( $callable, $product_id = 0, $product_price = 0 ) + public function prepare_for_calculation( $callback, $product_id = 0, $product_price = 0 )includes/REST/StoreController.php (2)
Line range hint
608-620: Approve use of PHPCS directives but recommend caching.The use of
phpcs:disableandphpcs:enableis appropriate here to bypass the PHPCS rules for direct database queries. However, consider implementing caching mechanisms to optimize performance and reduce database load, especially if this query is executed frequently.
Line range hint
441-441: Consider renaming the parameter$object.The use of the reserved keyword "object" as a parameter name might lead to issues in PHP, especially in strict typing contexts. Consider renaming it to something more specific, like
$dataObject.- protected function prepare_links( $object, $request ) { + protected function prepare_links( $dataObject, $request ) {
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (11)
- includes/Commission.php (3 hunks)
- includes/DummyData/Importer.php (2 hunks)
- includes/Product/functions.php (5 hunks)
- includes/REST/OrderControllerV2.php (3 hunks)
- includes/REST/StoreController.php (2 hunks)
- includes/Tracker.php (2 hunks)
- includes/Withdraw/Hooks.php (2 hunks)
- includes/Withdraw/Manager.php (10 hunks)
- includes/Withdraw/Withdraw.php (5 hunks)
- includes/Withdraw/Withdraws.php (1 hunks)
- uninstall.php (2 hunks)
Files skipped from review due to trivial changes (2)
- includes/Withdraw/Withdraws.php
- uninstall.php
Additional context used
GitHub Check: Run PHPCS inspection
includes/REST/OrderControllerV2.php
[warning] 216-216:
Stand-alone post-increment statement found. Use pre-increment instead: ++$file_counter.includes/DummyData/Importer.php
[failure] 5-5:
An import use statement should never start with a leading backslash
[failure] 6-6:
An import use statement should never start with a leading backslash
[failure] 8-8:
The file-level docblock must follow the opening PHP tag in the file header
[warning] 246-246:
Stand-alone post-increment statement found. Use pre-increment instead: ++$index .
[failure] 246-246:
Expected no spaces between $index and the increment operator; 1 found
[warning] 276-276:
Detected usage of meta_key, possible slow query.
[warning] 277-277:
Detected usage of meta_value, possible slow query.
[failure] 277-277:
There should be a comma after the last array item in a multi-line array.
[warning] 327-327:
When a multi-item array uses associative keys, each value should start on a new line.
[failure] 334-334:
Functions must not contain multiple empty lines in a row; found 2 empty linesincludes/Product/functions.php
[warning] 404-404:
Incorrect number of replacements passed to $wpdb->prepare(). Found 1 replacement parameters, expected 3.includes/Commission.php
[warning] 571-571:
It is recommended not to use reserved keyword "callable" as function parameter name. Found: $callableincludes/REST/StoreController.php
[warning] 441-441:
It is recommended not to use reserved keyword "object" as function parameter name. Found: $object
Additional comments not posted (16)
includes/Withdraw/Hooks.php (1)
Line range hint
97-133: Verify the necessity of PHPCS suppression directives.The use of
phpcs:disableandphpcs:enablearound direct database queries is noted. While this is sometimes necessary, it's important to ensure that there's a valid reason for bypassing these checks, especially concerning database query optimization and security.includes/REST/OrderControllerV2.php (1)
Line range hint
133-142: Verify the necessity of PHPCS suppression directives.Similar to previous observations, ensure there's a valid reason for bypassing PHPCS checks around direct database queries. This is crucial for maintaining code quality and security.
Verification successful
Line range hint
133-142: Verified the necessity of PHPCS suppression directives.The PHPCS suppression directives for
WordPress.DB.DirectDatabaseQuery.DirectQueryandWordPress.DB.DirectDatabaseQuery.NoCachingare used consistently across multiple files in the project. This indicates a pattern or necessity for bypassing these specific checks in certain scenarios, likely due to the nature of the operations being performed.
includes/Commission.phpincludes/Withdraw/Hooks.phpincludes/Withdraw/Withdraw.phpincludes/Withdraw/Manager.phpincludes/DummyData/Importer.phpincludes/REST/StoreController.phpincludes/REST/OrderControllerV2.phpScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Confirm if similar suppressions are used elsewhere in the project for consistency and ensure they are justified. # Search for similar PHPCS suppressions in PHP files rg --type php 'phpcs:disable WordPress.DB.DirectDatabaseQuery.DirectQuery'Length of output: 2209
includes/Tracker.php (1)
68-68: Verify the necessity of PHPCS suppression directive.The use of
phpcs:ignoreto suppress warnings for direct database queries is noted. Confirm that this is the best approach and consistent with other similar cases in the codebase.Verification successful
PHPCS suppression directive is consistent and necessary.
The use of
phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQueryis consistent across the codebase, indicating that direct database queries are a common and necessary practice in this project.
uninstall.phpincludes/Withdraw/Withdraws.phpincludes/Tracker.phpincludes/Withdraw/Manager.phpincludes/functions-compatibility.phpincludes/Upgrade/Upgrades/V_2_9_13.phpincludes/Upgrade/Upgrades/V_3_3_1.phpincludes/ReverseWithdrawal/Manager.phpincludes/Upgrade/Upgrades/V_3_3_7.phpincludes/ProductCategory/Hooks.phpincludes/Product/functions.phpincludes/Upgrade/Upgrades/BackgroundProcesses/V_2_8_3_VendorBalance.phpincludes/Upgrade/Upgrades/BackgroundProcesses/V_2_9_4_OrderPostAuthor.phpincludes/Order/Manager.phpincludes/Order/MiscHooks.phpScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistency in the use of PHPCS suppression directives across the project. # Search for similar PHPCS suppressions in PHP files rg --type php 'phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery'Length of output: 7159
includes/DummyData/Importer.php (2)
Line range hint
327-327: Format array items on new lines for readability.When using associative arrays, each item should start on a new line to improve readability.
$args = [ 'task' => 'clear_dummy_vendors_orders', 'vendor_id' => $vendor->id, ];Tools
GitHub Check: Run PHPCS inspection
[failure] 334-334:
Functions must not contain multiple empty lines in a row; found 2 empty lines
Line range hint
336-357: Ensure proper disabling and re-enabling of PHPCS checks.The use of
phpcs:disableandphpcs:enableis correctly placed around direct database operations to suppress PHPCS warnings. Ensure that these directives are necessary and used judiciously.Tools
GitHub Check: Run PHPCS inspection
[failure] 334-334:
Functions must not contain multiple empty lines in a row; found 2 empty linesincludes/Withdraw/Withdraw.php (3)
Line range hint
451-466: Ensure proper disabling and re-enabling of PHPCS checks.The use of
phpcs:disableandphpcs:enableis correctly placed around direct database operations to suppress PHPCS warnings. Ensure that these directives are necessary and used judiciously.
Line range hint
498-514: Ensure proper disabling and re-enabling of PHPCS checks.The use of
phpcs:disableandphpcs:enableis correctly placed around direct database operations to suppress PHPCS warnings. Ensure that these directives are necessary and used judiciously.
551-557: Ensure proper disabling and re-enabling of PHPCS checks.The use of
phpcs:disableandphpcs:enableis correctly placed around direct database operations to suppress PHPCS warnings. Ensure that these directives are necessary and used judiciously.includes/Withdraw/Manager.php (4)
100-108: Ensure proper handling of direct database queries.The
phpcs:disabledirective is used to bypass the PHPCS checks for direct database queries. While this is necessary for the operation, it's crucial to ensure that these queries do not open up SQL injection vulnerabilities. The usage of$wpdb->prepareis appropriate here, which helps in preventing SQL injection by ensuring that the values are parameterized.
Line range hint
140-158: Validate the handling of status updates in database.The
update_statusmethod correctly uses thephpcs:disabledirective to allow direct database queries. Thewpdb->updatemethod is used appropriately with parameterization to prevent SQL injection risks. However, ensure that the status values are validated before they are passed to this method to prevent any illegal status updates.
Line range hint
226-235: Review the method for checking pending requests.The
has_pending_requestmethod correctly uses thephpcs:disabledirective for a direct database query. The query is parameterized using$wpdb->prepare, which is a good practice to prevent SQL injection. Ensure that the user_id is validated before being used in the query.
409-416: Ensure secure handling of direct database queries.The method
getuses direct database queries to fetch withdrawal details. The use ofphpcs:disableis necessary due to the nature of the operation. Ensure that the ID is sanitized and validated before being used in the query to prevent SQL injection.includes/Product/functions.php (2)
41-42: Review category selection validation logic.The changes to
dokan_save_productinclude updated logic for category selection validation. This is crucial for ensuring that products are correctly categorized. The use of error handling withWP_Erroris appropriate for user feedback. Ensure that the logic covers all possible user inputs to prevent any issues with product saving.
90-91: Ensure robust category handling in product data.The refactored logic for handling multiple categories in
dokan_save_productis a significant improvement. It ensures that all categories are processed and stored correctly, which is crucial for product organization and filtering. Ensure that the array mapping functionarray_mapis used correctly to sanitize and validate all category IDs before use.includes/Commission.php (2)
Line range hint
92-111: Review ofcalculate_gateway_fee: Use of PHPCS directives around database queriesThe use of
phpcs:disableandphpcs:enableis appropriate here due to the direct database queries involved. However, it's crucial to ensure these queries are secure and optimized.Consider conducting a security audit on these queries to ensure they are safe from SQL injection and other vulnerabilities.
740-747: Review ofget_earning_from_order_table: Use of PHPCS directives around database queriesThe use of
phpcs:disableandphpcs:enablearound the SQL query is justified due to the direct database access. However, ensuring the security of this query is essential.Consider conducting a security audit on this query to ensure it is safe from SQL injection and other vulnerabilities.
| // Deleting vendors orders. | ||
| foreach ( $orders as $order ) { | ||
| $order->delete( true ); | ||
| } | ||
|
|
||
| global $wpdb; | ||
|
|
There was a problem hiding this comment.
Remove extra empty lines.
Ensure there are no multiple consecutive empty lines within functions.
global $wpdb;
-
-
// phpcs:disable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCachingCommittable 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.
| global $wpdb; | |
| // phpcs:disable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching |
Tools
GitHub Check: Run PHPCS inspection
[failure] 334-334:
Functions must not contain multiple empty lines in a row; found 2 empty lines
includes/REST/OrderControllerV2.php
Outdated
| @@ -211,7 +213,7 @@ | |||
| $inserted_id = wc_downloadable_file_permission( $download_id, $product_id, $order ); | |||
|
|
|||
| if ( $inserted_id ) { | |||
| $file_counter ++; | |||
| $file_counter++; | |||
There was a problem hiding this comment.
Optimize the increment operation.
The static analysis tool has flagged the use of post-increment. Using pre-increment is generally more efficient in PHP.
- $file_counter++;
+ ++$file_counter;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.
| $file_counter++; | |
| ++$file_counter; |
Tools
GitHub Check: Run PHPCS inspection
[warning] 216-216:
Stand-alone post-increment statement found. Use pre-increment instead: ++$file_counter.
| // phpcs:disable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching | ||
| $result = $wpdb->get_results( $wpdb->prepare( "SELECT * FROM {$wpdb->dokan_withdraw} WHERE status = %d LIMIT %d, %d", $status, $offset, $limit ) ); | ||
| } else { | ||
| $result = $wpdb->get_results( $wpdb->prepare( "SELECT * FROM {$wpdb->dokan_withdraw} WHERE user_id = %d AND status = %d ORDER BY id DESC LIMIT %d, %d", $user_id, $status, $offset, $limit ) ); | ||
| //phpcs:enable |
There was a problem hiding this comment.
Optimize the retrieval of withdrawal requests.
The method get_withdraw_requests uses direct database queries to fetch data. The use of phpcs:disable is appropriate here to bypass the PHPCS checks. Consider caching the results of these queries to improve performance, especially if this method is called frequently.
+ $cache_result = wp_cache_get($cache_key, $cache_group);
+ if (false !== $cache_result) {
+ return $cache_result;
+ }
- $result = $wpdb->get_results(...);
+ wp_cache_set($cache_key, $result, $cache_group);Committable suggestion was skipped due to low confidence.
| @@ -185,7 +189,7 @@ | |||
|
|
|||
| $format = [ '%d', '%f', '%s', '%d', '%s', '%s', '%s', '%s' ]; | |||
|
|
|||
| $inserted = $wpdb->insert( $wpdb->dokan_withdraw, $data, $format ); | |||
| $inserted = $wpdb->insert( $wpdb->dokan_withdraw, $data, $format ); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery | |||
There was a problem hiding this comment.
Tip
Codebase Verification
Potential Issue with Direct Database Insertion and Lack of Data Sanitization
The insert_withdraw method directly inserts data into the database using $wpdb->insert without apparent sanitization of the input data. This can lead to SQL injection and other security vulnerabilities.
- File:
includes/Withdraw/Manager.php - Line: 192
Please ensure that all data passed to the database is properly sanitized and validated.
Analysis chain
Check for potential issues with direct database insertions.
The method insert_withdraw uses a direct database query without any PHPCS checks. This is indicated by the phpcs:ignore directive. Ensure that all data passed to the database is properly sanitized and validated to prevent SQL injection and other security vulnerabilities.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify data sanitization in `insert_withdraw` method.
# Test: Search for the method usage. Expect: Proper sanitization before data is passed.
rg --type php $'insert_withdraw'
Length of output: 113
Script:
#!/bin/bash
# Description: Extract the full implementation of the `insert_withdraw` method to verify data sanitization.
# Searching for the full implementation of the `insert_withdraw` method in the PHP file.
ast-grep --lang php --pattern 'public function insert_withdraw($_) { $$$ }' includes/Withdraw/Manager.php
Length of output: 214
Script:
#!/bin/bash
# Description: Extract the full implementation of the `insert_withdraw` method to verify data sanitization.
# Using ripgrep to search for the full implementation of the `insert_withdraw` method.
rg --context 20 'public function insert_withdraw' includes/Withdraw/Manager.php
Length of output: 1407
Script:
#!/bin/bash
# Description: Search for all instances where `insert_withdraw` is called to verify data sanitization.
# Using ripgrep to find all invocations of the `insert_withdraw` method.
rg 'insert_withdraw\('
Length of output: 106
| @@ -189,7 +185,7 @@ | |||
| } | |||
| } | |||
|
|
|||
| $variations_count = absint( $wpdb->get_var( $wpdb->prepare( "SELECT COUNT(ID) FROM $wpdb->posts WHERE post_parent = %d AND post_type = 'product_variation' AND post_status IN ('publish', 'private', 'pending')", $post->ID ) ) ); | |||
| $variations_count = absint( $wpdb->get_var( $wpdb->prepare( "SELECT COUNT(ID) FROM $wpdb->posts WHERE post_parent = %d AND post_type = 'product_variation' AND post_status IN ('publish', 'private', 'pending')", $post->ID ) ) ); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching | |||
There was a problem hiding this comment.
Optimize the retrieval of product variations.
The method dokan_product_output_variations uses a direct database query to count product variations. While necessary, consider optimizing this query or using caching to improve performance, especially for products with a large number of variations.
+ $variations_count = wp_cache_get('variations_count_' . $post->ID);
+ if (false === $variations_count) {
+ $variations_count = $wpdb->get_var(...);
+ wp_cache_set('variations_count_' . $post->ID, $variations_count);
+ }Committable suggestion was skipped due to low confidence.
mrabbani
added
Needs: Author Reply
and removed
Needs: Testing
includes/DummyData/Importer.php
Outdated
| @@ -332,6 +332,8 @@ public function delete_dummy_vendor_orders( $args ) { | |||
|
|
|||
| global $wpdb; | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Pls fix the others PHPCS issues.
mrabbani
removed
the
Needs: Dev Review
# Conflicts: # includes/Commission.php
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
includes/REST/StoreController.php (3)
Line range hint
608-620: Ensure thorough sanitization of query parameters
Although the query is safely parameterized via$wpdb->prepare(), consider adding explicit sanitization for$statusand$post_type(e.g.,sanitize_text_field($status)) before using them, to further reinforce security best practices.
Line range hint
608-620: Consider using WP APIs for better maintainability
A direct query with$wpdb->commentsand$wpdb->postscan be replaced with a more standardized API (e.g.,WP_Comment_Query) if performance or future maintainability becomes a concern. This approach leverages native caching and simplifies compliance with coding standards.
Line range hint
608-620: Gracefully handle potential query failures
If the query fails or returnsnull, the current code will still passintval(null)without indicating any error. Consider checking if$totalisnullbefore returning, so you can log or handle failures more visibly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
includes/Commission.php(1 hunks)includes/REST/StoreController.php(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- includes/Commission.php
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
includes/Product/functions.php (1)
188-188: Improve performance with caching
This query directly counts product variations. If products can have many variations, it may impact performance. Consider using transient or object caching to reduce repeated queries.includes/DummyData/Importer.php (1)
273-274: Consider avoiding PHPCS ignores if feasible.
These lines only define a static meta key and value for use inWP_Query. If you’re ignoring PHPCS solely to bypass potential direct database warnings, it may be unnecessary here since it’s not a raw SQL statement.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
includes/DummyData/Importer.php(4 hunks)includes/Product/functions.php(5 hunks)includes/REST/OrderControllerV2.php(3 hunks)includes/REST/StoreController.php(3 hunks)
🔇 Additional comments (12)
includes/REST/OrderControllerV2.php (2)
Line range hint
133-142: Consider input validation for direct DB queries
Disabling PHPCS checks around direct queries is acceptable under some circumstances, but ensure that all parameters (especiallyorder_id) are already validated and sanitized. Consider using parameterized queries (such as$wpdb->prepare) for safety, and double-check that no user-generated input can flow unsanitized into the query.
216-216: Great job using pre-increment
Thank you for addressing the previous feedback regarding increment operations. Using++$file_counteris good for compliance and is also slightly more efficient in PHP.includes/Product/functions.php (3)
41-42: Properly handle absence of category
The updated check (elseif) reading “Please select at least one category” clarifies the condition. This ensures that a vendor must pick a category. Looks good!
90-91: Good use ofarray_map('absint', [...])for product categories
This effectively sanitizes all input category IDs. Remember to confirm that returned categories exist before proceeding with subsequent logic.
402-405: Validate query parameters for increased security
Disabling PHPCS checks is permissible if carefully reviewed. Confirm that$termand$user_idsare always sanitized, and ensure your query logic cannot be manipulated.Also applies to: 419-419, 423-423
includes/REST/StoreController.php (3)
436-441: Improved parameter naming
Renaming the parameter from$objectto$storeincreases clarity—good practice for maintainability.
444-444: Ensure array structure for$store
You are accessing$store['id']. Double-check$storeis consistently an array containing'id'. If$storecan be an object in other calls, this might cause errors.
Line range hint
608-620: Sanitize store ID before the query
You correctly disable PHPCS checks here, but ensure$idand other parameters are sanitized. Even though$wpdb->prepareis used, verifying all parameters remain integer values will help avoid potential injection.includes/DummyData/Importer.php (4)
5-6: Imports look appropriate and necessary.
It appears that bothWP_ErrorandWP_Queryare required for error handling and retrieving posts.
324-330: Validatevendor_idmore thoroughly.
Although you check if$vendor_idis empty, consider further validation (e.g., ensuring it’s numeric) to avoid edge cases.
338-338: Direct database queries are parameterized properly.
Using$wpdb->delete()with string format specifiers (%d) is secure. Disabling PHPCS here is acceptable, given the explicit placeholders for prevention of SQL injection.
359-359: Re-enabled PHPCS checks.
Good practice to limit the ignore region as narrowly as possible.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
includes/Tracker.php(3 hunks)
🔇 Additional comments (3)
includes/Tracker.php (3)
25-25: Aligns with WordPress PHPDoc standardsReinstating the
@return voidannotation is consistent with WordPress coding standards. Good to see the clarity in the docblock.
36-36: Docblock clarity maintainedLikewise, restoring the
@return voidannotation strengthens the codebase documentation.
74-74: Consistent version annotationAdding
@since 3.0.15enhances the code’s maintainability and ensures accurate version tracking of newly introduced features.
| @@ -65,18 +65,18 @@ function () { | |||
| protected function get_order_count() { | |||
| global $wpdb; | |||
|
|
|||
| return (int) $wpdb->get_var( "SELECT count(id) FROM {$wpdb->prefix}dokan_orders WHERE order_status IN ('wc-completed', 'wc-processing', 'wc-on-hold', 'wc-refunded');" ); | |||
| return (int) $wpdb->get_var( "SELECT count(id) FROM {$wpdb->prefix}dokan_orders WHERE order_status IN ('wc-completed', 'wc-processing', 'wc-on-hold', 'wc-refunded');" ); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use of raw SQL and ignoring PHPCS checks
While the comment clarifies why PHPCS checks are disabled, consider using $wpdb->prepare() to defend against potential SQL injection vulnerabilities, even if the risk appears minimal. Prepared statements also align with best practices and WordPress coding standards.
- return (int) $wpdb->get_var( "SELECT count(id) FROM {$wpdb->prefix}dokan_orders WHERE order_status IN ('wc-completed', 'wc-processing', 'wc-on-hold', 'wc-refunded');" ); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching
+ $statuses = [ 'wc-completed', 'wc-processing', 'wc-on-hold', 'wc-refunded' ];
+ $placeholders = implode( ', ', array_fill(0, count($statuses), '%s') );
+ $query = $wpdb->prepare(
+ "SELECT COUNT(id) FROM {$wpdb->prefix}dokan_orders WHERE order_status IN ($placeholders)",
+ $statuses
+ );
+ return (int) $wpdb->get_var( $query ); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.NoCaching📝 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.
| return (int) $wpdb->get_var( "SELECT count(id) FROM {$wpdb->prefix}dokan_orders WHERE order_status IN ('wc-completed', 'wc-processing', 'wc-on-hold', 'wc-refunded');" ); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching | |
| $statuses = [ 'wc-completed', 'wc-processing', 'wc-on-hold', 'wc-refunded' ]; | |
| $placeholders = implode( ', ', array_fill(0, count($statuses), '%s') ); | |
| $query = $wpdb->prepare( | |
| "SELECT COUNT(id) FROM {$wpdb->prefix}dokan_orders WHERE order_status IN ($placeholders)", | |
| $statuses | |
| ); | |
| return (int) $wpdb->get_var( $query ); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.NoCaching |
devAsadNur
added
Needs: Dev Review
|
@mrabbani Bhai, The PHPCS issues have been fixed based on your PR. Please review once again. |
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
Bug Fixes
Refactor
Chores