-
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: invalid product id validation on edit page #2474
base: develop
Are you sure you want to change the base?
fix: invalid product id validation on edit page #2474
Conversation
WalkthroughThe changes in the Changes
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 (
|
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 and nitpick comments (1)
templates/products/edit-product-single.php (1)
35-45
: Enhance error handling for non-existent productsThe validation logic for non-existent products is good, but consider these improvements:
- Add a specific error code for better error tracking
- Log the invalid product ID access attempt for security monitoring
Consider this enhancement:
if ( ! $post instanceof WP_Post ) { + dokan_log( sprintf( 'Invalid product ID access attempt: %d', $post_id ) ); dokan_get_template_part( 'global/dokan-error', '', array( 'deleted' => false, - 'message' => __( 'This product is no longer available', 'dokan' ) + 'message' => __( 'This product is no longer available', 'dokan-lite' ), + 'code' => 'invalid_product_id' ) ); return; }🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[failure] 41-41:
Mismatched text domain. Expected 'dokan-lite' but got 'dokan'.
[failure] 41-41:
There should be a comma after the last array item in a multi-line array.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
templates/products/edit-product-single.php
(1 hunks)
🧰 Additional context used
🪛 GitHub Check: Run PHPCS inspection
templates/products/edit-product-single.php
[failure] 41-41:
Mismatched text domain. Expected 'dokan-lite' but got 'dokan'.
[failure] 41-41:
There should be a comma after the last array item in a multi-line array.
🔇 Additional comments (2)
templates/products/edit-product-single.php (2)
32-33
: LGTM: Proper sanitization of product ID
The code correctly sanitizes the product ID using intval()
before retrieving the post.
35-45
: Verify consistent error handling across the codebase
Let's verify if similar product validation is implemented consistently across other product-related templates.
✅ Verification successful
Error handling is implemented appropriately and consistently
The verification shows that error handling across product-related templates is consistent with the implementation in question:
- Product listing templates use appropriate error messages for "No Products Found" scenarios
- The
dokan-error
template is consistently used across different contexts (orders, store lists, products) with context-specific messages - Product validation using
WP_Post
and post metadata is handled consistently where needed
The current implementation in edit-product-single.php
follows the established pattern in the codebase for handling product-related errors.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other templates that might need similar product validation
rg -l "get_post.*product_id" templates/
Length of output: 41
Script:
#!/bin/bash
# Search for files that might handle product validation using different patterns
rg -l "WP_Post|get_post|product.*not.*found|product.*no.*longer|product.*validation" templates/
# Also search for dokan error template usage
rg -l "dokan-error" templates/
Length of output: 499
Script:
#!/bin/bash
# Check the specific error handling implementations in the identified files
rg -A 5 "WP_Post|get_post|product.*not.*found|product.*no.*longer" templates/products/products-listing.php templates/products/products-listing-row.php templates/products/downloadable.php
# Also check how dokan-error template is used in other files
rg -A 5 "dokan-error" templates/store-lists-loop.php templates/orders/listing.php
Length of output: 4347
🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[failure] 41-41:
Mismatched text domain. Expected 'dokan-lite' but got 'dokan'.
[failure] 41-41:
There should be a comma after the last array item in a multi-line array.
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.
Pls fix the translation domain name.
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
🧹 Outside diff range and nitpick comments (2)
templates/products/edit-product-single.php (2)
35-45
: Enhance error handling with specific error messagesWhile the validation logic is good, the error message could be more specific to help users understand why the product is unavailable (e.g., deleted, draft, or invalid ID).
Consider this enhancement:
if ( ! $post instanceof WP_Post ) { dokan_get_template_part( 'global/dokan-error', '', array( 'deleted' => false, - 'message' => __( 'This product is no longer available', 'dokan-lite' ) + 'message' => __( 'This product is no longer available. It may have been deleted or is invalid.', 'dokan-lite' ) ) ); return; }🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[failure] 41-41:
There should be a comma after the last array item in a multi-line array.
39-42
: Fix array formattingAdd a trailing comma after the last array item as per PHPCS standards.
Apply this fix:
array( 'deleted' => false, - 'message' => __( 'This product is no longer available', 'dokan-lite' ) + 'message' => __( 'This product is no longer available', 'dokan-lite' ), )🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[failure] 41-41:
There should be a comma after the last array item in a multi-line array.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
templates/products/edit-product-single.php
(1 hunks)
🧰 Additional context used
🪛 GitHub Check: Run PHPCS inspection
templates/products/edit-product-single.php
[failure] 41-41:
There should be a comma after the last array item in a multi-line array.
🔇 Additional comments (2)
templates/products/edit-product-single.php (2)
32-33
: LGTM: Proper sanitization of product ID
The code correctly uses intval()
to sanitize the product ID from $_GET parameter and get_post()
to safely retrieve the post object.
32-45
: Verify error handling in upstream code
The new validation improves security but we should ensure proper error handling in the calling code.
✅ Verification successful
The previous search was too specific. Let's broaden it to find relevant code patterns and files that might need similar validation.
Let's examine how product_id is handled and what error handling patterns exist in the product editing related files.
Based on the gathered information, I can now provide a final response:
Error handling is properly implemented across product editing flows
The new validation in edit-product-single.php
aligns well with existing error handling patterns. The code shows:
- Proper nonce verification before product access
- Multiple layers of validation (nonce, product existence, post type)
- Consistent error handling using
dokan-error
template andwp_die()
- Similar validation patterns in related product editing files
The changes enhance security without disrupting the established error handling flow.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other instances where product validation might be needed
rg -A 3 "get_post.*product_id.*WP_Post" --type php
Length of output: 52
Script:
#!/bin/bash
# Search for usages of get_post with product_id
rg -A 3 "get_post.*product_id" --type php
# Search for WP_Post type checks related to products
rg -A 3 "instanceof\s+WP_Post.*product" --type php
# Search for files that might handle product editing
fd -e php "edit.*product|product.*edit"
Length of output: 5943
Script:
#!/bin/bash
# Let's check how product_id is obtained in these edit-related files
rg -B 3 "product_id" templates/products/edit-product-single.php templates/products/dokan-products-edit-bulk-commission.php includes/CatalogMode/Dashboard/ProductBulkEdit.php
# Also check for error handling patterns in these files
rg "dokan-error|wp_die|exit|return" templates/products/edit-product-single.php templates/products/dokan-products-edit-bulk-commission.php includes/CatalogMode/Dashboard/ProductBulkEdit.php
Length of output: 5049
🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[failure] 41-41:
There should be a comma after the last array item in a multi-line array.
Currently, Dokan display the |
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