Refactor seller registration to enable bypassing nonce verification #2301
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe updates encompass enhancements mainly in test infrastructure and refinement of the registration process. Key additions include TDD documentation, efficient test data creation through factories, and refactored nonce validation logic. These changes aim to streamline testing and ensure robust, maintainable code, including crucial adjustments for integration with the Blocksy theme. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BlocksyTheme
participant DokanRegistration
User->>BlocksyTheme: Register using modal
BlocksyTheme->>DokanRegistration: Initiate vendor registration
DokanRegistration->>DokanRegistration: validate_nonce
DokanRegistration->>DokanRegistration: set_new_vendor_names
DokanRegistration->DokanRegistration: save_vendor_info
DokanRegistration->>BlocksyTheme: Registration success response
BlocksyTheme->>User: Vendor registered
Assessment against linked issues
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 6
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (24)
- composer.json (2 hunks)
- docs/tdd/factories.md (1 hunks)
- docs/tdd/readme.md (1 hunks)
- includes/Registration.php (4 hunks)
- phpunit.xml (1 hunks)
- tests/php/CommissionTest.php (6 hunks)
- tests/php/CustomFactoriesTest.php (1 hunks)
- tests/php/DokanTestCase.php (1 hunks)
- tests/php/Factories/CouponFactory.php (1 hunks)
- tests/php/Factories/CustomerFactory.php (1 hunks)
- tests/php/Factories/DokanFactory.php (1 hunks)
- tests/php/Factories/OrderFactory.php (1 hunks)
- tests/php/Factories/ProductFactory.php (1 hunks)
- tests/php/Factories/SellerFactory.php (1 hunks)
- tests/php/Factories/ShippingFactory.php (1 hunks)
- tests/php/Helpers/Dokan_Test_Helpers.php (1 hunks)
- tests/php/Helpers/WC_Helper_Coupon.php (1 hunks)
- tests/php/Helpers/WC_Helper_Customer.php (1 hunks)
- tests/php/Helpers/WC_Helper_Order.php (1 hunks)
- tests/php/Helpers/WC_Helper_Product.php (1 hunks)
- tests/php/Helpers/WC_Helper_Shipping.php (1 hunks)
- tests/php/bootstrap.php (2 hunks)
- tests/php/phpunit-wp-config.php (2 hunks)
- tests/php/test-seller.php (2 hunks)
Files skipped from review due to trivial changes (6)
- phpunit.xml
- tests/php/CommissionTest.php
- tests/php/Helpers/Dokan_Test_Helpers.php
- tests/php/bootstrap.php
- tests/php/phpunit-wp-config.php
- tests/php/test-seller.php
Additional context used
Markdownlint
docs/tdd/readme.md
32-32: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
6-6: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
18-18: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
25-25: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
35-35: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
38-38: null
Files should end with a single newline character(MD047, single-trailing-newline)
docs/tdd/factories.md
8-8: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
51-51: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
1-1: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
25-25: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
19-19: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
127-127: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
2-2: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
19-19: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
28-28: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
55-55: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
127-127: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
148-148: null
Files should end with a single newline character(MD047, single-trailing-newline)
GitHub Check: Run PHPCS inspection
tests/php/Factories/CustomerFactory.php
[failure] 22-22:
Processing form data without nonce verification.includes/Registration.php
[failure] 47-47:
Processing form data without nonce verification.
[failure] 47-47:
Processing form data without nonce verification.
[failure] 51-51:
Processing form data without nonce verification.
[failure] 52-52:
Processing form data without nonce verification.
[failure] 52-52:
Processing form data without nonce verification.
[failure] 65-65:
Processing form data without nonce verification.
[failure] 65-65:
Processing form data without nonce verification.
[failure] 93-93:
Processing form data without nonce verification.
[failure] 93-93:
Processing form data without nonce verification.
LanguageTool
docs/tdd/factories.md
[uncategorized] ~10-~10: When ‘Vendor-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ... for different entities such as Vendor, Vendor specific Products, Coupons, Shipping, Orders, et...(SPECIFIC_HYPHEN)
Additional comments not posted (49)
docs/tdd/readme.md (5)
6-6: Add blank line above the Introduction heading.To comply with Markdown best practices, ensure that headings are surrounded by blank lines.
+ ### IntroductionTools
Markdownlint
6-6: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
18-18: Add blank line above the 'How to Run' heading.This will enhance the readability and structure of the document.
+ ### How to RunTools
Markdownlint
18-18: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
25-25: Specify language for code blocks.Adding a language specification to fenced code blocks improves syntax highlighting and readability.
- ``` + ```bash - ``` + ```bashAlso applies to: 35-35
Tools
Markdownlint
25-25: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
32-32: Remove extra blank line.To maintain a clean and consistent document structure, avoid multiple consecutive blank lines.
-Tools
Markdownlint
32-32: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
38-38: Add a newline at the end of the file.Files should end with a single newline character to comply with POSIX standards.
+Tools
Markdownlint
38-38: null
Files should end with a single newline character(MD047, single-trailing-newline)
tests/php/Factories/ShippingFactory.php (2)
12-17: Refactor suggestion for default generation definitions:Consider using simple associative arrays for static values to improve readability.
[REFACTOR_SUGGESTion]- 'method_title' => new WP_UnitTest_Generator_Sequence( 'Shipping Method %s' ), - 'method_id' => new WP_UnitTest_Generator_Sequence( 'shipping_method_%s' ), - 'total' => 10 + 'method_title' => 'Shipping Method 1', + 'method_id' => 'shipping_method_1', + 'total' => 10
36-49: Good practice in field existence checks before updating.tests/php/Helpers/WC_Helper_Shipping.php (2)
21-35: Well-implemented method for creating simple flat rate shipping.
84-89: Proper cleanup indelete_simple_flat_ratemethod.tests/php/Factories/CouponFactory.php (4)
18-27: Good use of helper class to maintain modularity increate_object.
29-44: Proper field checks before updating inupdate_object.
54-56: Correct instantiation ofWC_Couponinget_object_by_id.
67-122: Effective use of delegation to maintain separation of concerns in helper methods.tests/php/Helpers/WC_Helper_Customer.php (1)
19-72: Well-implemented methods for creating and updating customer data.tests/php/Helpers/WC_Helper_Coupon.php (5)
22-61: Ensure proper handling of coupon creation with default and custom metadata.The method
create_couponcorrectly creates a WooCommerce coupon usingwp_insert_postand sets various default properties for the coupon. It's important to ensure that the default meta properties align with typical use cases or can be easily overridden by the user.
71-74: Proper cleanup of coupon resources.The method
delete_couponuseswp_delete_postto remove a coupon, ensuring resources are cleaned up properly. This is a straightforward and effective way to handle coupon deletion.
82-95: Custom coupon type registration handled efficiently.The
register_custom_typemethod efficiently handles the registration of custom coupon types, using a static variable to prevent redundant filter additions. This method prevents the duplication of filters if a type is already registered, which is a good practice to maintain performance.
103-109: Ensure clean unregistration of custom coupon types.The method
unregister_custom_typeproperly cleans up by removing custom types and associated filters when no custom types are left. This helps prevent potential memory leaks or unexpected behavior due to stale filter hooks.
118-119: Custom discount types merging logic.The method
filter_discount_typesmerges custom discount types with existing ones. It's crucial to ensure that this does not unintentionally overwrite existing discount types unless explicitly intended.Verification successful
Custom discount types merging logic verified.
The search results indicate that the custom discount types are being merged with existing ones without unintentional overwriting. The handling of
discount_typein various contexts confirms that the implementation is consistent.
tests/php/Helpers/WC_Helper_Coupon.phptests/php/Factories/CouponFactory.phptests/pw/utils/schemas.tstests/pw/utils/testData.tstests/pw/utils/interfaces.tstests/pw/utils/payloads.tstests/pw/pages/selectors.tstests/pw/tests/api/calculation.spec.tsdocs/tdd/factories.mdScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the custom discount types do not overwrite existing WooCommerce discount types. # Test: Search for the use of existing discount types in the WooCommerce codebase. Expect: No conflicts with custom types. rg --type php 'discount_type' -- 'wp-content/plugins/woocommerce'Length of output: 134
Script:
#!/bin/bash # Locate WooCommerce plugin files and search for discount types usage or definitions. # Step 1: Find WooCommerce related files in the repository fd 'woocommerce' > woocommerce_files.txt # Step 2: Search for 'discount_type' within the located WooCommerce files rg --type php 'discount_type' --file woocommerce_files.txtLength of output: 129
Script:
#!/bin/bash # Locate WooCommerce plugin files and search for discount types usage or definitions. # Step 1: Find WooCommerce related files and pipe the results to rg fd 'woocommerce' | xargs rg --type php 'discount_type'Length of output: 52
Script:
#!/bin/bash # Use ast-grep to search for functions related to discount types in the WooCommerce codebase. ast-grep --lang php --pattern 'function $_($$$_) { $$$ }' | xargs rg 'discount_type'Length of output: 3271
tests/php/CustomFactoriesTest.php (6)
10-81: Comprehensive testing of order creation with various components.The
test_create_ordermethod thoroughly tests the creation of an order with fees, shipping, and coupons. It ensures that all aspects of the order are correctly initialized and asserts the final state of the order. This is a good practice for ensuring that all components integrate well.
83-97: Testing customer creation with essential attributes.The
test_create_customermethod correctly creates a customer and verifies important attributes such as email and name. This ensures that the customer creation process is robust and that all necessary information is correctly stored.
100-112: Seller creation and verification of critical properties.The
test_create_sellermethod tests the creation of a seller and checks essential properties like email and shop name. This is crucial for ensuring that sellers are correctly registered and that their information is accurately recorded.
115-131: Shipping item creation and validation of properties.The
test_create_shippingmethod effectively tests the creation of a shipping item and validates its properties, such as method title and total cost. This ensures that shipping configurations are correctly applied and retrievable.
134-150: Coupon creation and attribute verification.The
test_create_couponmethod tests the creation of a coupon and verifies its critical attributes, such as the discount type and amount. This ensures that coupons are created with the correct settings and behave as expected in transactions.
153-170: Product creation and validation for a specific seller.The
test_create_productmethod tests the creation of a product for a specific seller and verifies key product attributes. This is important for ensuring that products are correctly linked to sellers and have accurate pricing information.tests/php/Helpers/WC_Helper_Order.php (3)
25-37: Ensure proper deletion of orders and associated products.The
delete_ordermethod correctly handles the deletion of orders and associated products. This is important to prevent orphaned records and ensure clean database states after tests.
52-141: Comprehensive order creation with detailed attribute setup.The
create_ordermethod is well-structured, setting up detailed attributes for the order, including billing and shipping addresses, and adding products and shipping costs. This thorough setup is crucial for testing complex order scenarios.
152-170: Enhanced order creation with fees and shipping.The
create_order_with_fees_and_shippingmethod extends the basic order creation by adding fees and shipping details. This is useful for testing more complex order setups and ensuring that additional costs are handled correctly.tests/php/Factories/OrderFactory.php (5)
37-75: Order creation logic with flexibility for custom configurations.The
create_objectmethod inOrderFactoryallows for flexible order creation with customizable line items and statuses. This method is well-implemented, providing the necessary flexibility for testing different order scenarios.
77-98: Order update functionality with comprehensive attribute handling.The
update_objectmethod allows for updating various order attributes, such as status and customer ID. This method is crucial for testing order modifications and ensuring that updates are applied correctly.
132-141: Discount application logic in orders.The
add_discountmethod applies a coupon to an order and recalculates totals, ensuring that discounts are correctly factored into the order's total. This method is critical for testing the impact of discounts on order totals.
150-162: Fee item configuration and addition to orders.The
set_item_feemethod configures and adds a fee item to an order. This is important for testing how additional charges affect the order total and ensuring that fees are correctly added to the order.
165-177: Shipping item configuration and addition to orders.The
set_item_shippingmethod configures and adds a shipping item to an order, testing the handling of shipping costs within orders. This ensures that shipping charges are correctly calculated and applied.docs/tdd/factories.md (1)
1-148: Documentation for factory usage in test-driven development.The document provides a comprehensive guide on using factories in test-driven development within the Dokan plugin. It includes examples and explanations for creating and manipulating test data for products, orders, and coupons. This documentation is essential for new contributors and for maintaining consistency in testing practices.
Tools
LanguageTool
[uncategorized] ~10-~10: When ‘Vendor-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ... for different entities such as Vendor, Vendor specific Products, Coupons, Shipping, Orders, et...(SPECIFIC_HYPHEN)
Markdownlint
8-8: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
51-51: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
1-1: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
25-25: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
19-19: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
127-127: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
2-2: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
19-19: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
28-28: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
55-55: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
127-127: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
148-148: null
Files should end with a single newline character(MD047, single-trailing-newline)
tests/php/Factories/ProductFactory.php (5)
11-19: Initialization of default product properties in constructor.The constructor initializes default product properties effectively. However, consider adding a doc comment for the constructor to improve code documentation.
+ /** + * Constructor for ProductFactory. + * Initializes default generation definitions for products. + * @param WP_UnitTest_Factory|null $factory Factory instance for creating objects. + */ public function __construct( $factory = null ) {
65-69: Setter method forseller_idis concise and clear.The
set_seller_idmethod is implemented effectively. It's good practice to return$thisfor method chaining in setter methods.
90-92: Wrapper methods for product creation are well-implemented.The methods for creating various types of products (simple, downloadable, external, grouped, variation) are correctly delegating to the
WC_Helper_Productclass. This maintains separation of concerns and reuses existing logic effectively.Also applies to: 104-106, 114-116, 124-126, 136-138, 151-153, 163-165, 176-178
21-34: Ensure proper handling of seller ID in product creation.The method
create_objectsets thepost_authorbased onseller_id. This is a good use of conditional logic to associate products with sellers. However, it's crucial to ensure that theseller_idis always a valid user ID to avoid potential integrity issues.
199-201: Creation of product reviews and direct metadata updates.The methods
create_product_reviewandsave_post_test_update_meta_data_directare useful for testing scenarios. Ensure that the review content and metadata updates are relevant and meaningful in your test cases.Also applies to: 209-211
includes/Registration.php (3)
40-41: Centralized nonce validation logic invalidate_registration.The refactoring to use a centralized
validate_noncemethod is a good practice as it avoids code duplication and centralizes security logic. Ensure that the nonce validation is correctly implemented in thevalidate_noncemethod.
88-88: Centralized nonce validation logic inset_new_vendor_names.Refactoring to use a centralized nonce validation method in
set_new_vendor_namesis consistent with changes in other methods. This maintains uniformity and reduces error-prone duplicate code.
118-118: Centralized nonce validation logic insave_vendor_info.Using the centralized nonce validation in
save_vendor_infoenhances security and consistency across the registration process. Ensure that all paths that modify user data are protected by appropriate security checks.tests/php/Helpers/WC_Helper_Product.php (7)
25-29: Proper deletion logic indelete_product.The method
delete_productcorrectly checks if the product exists before attempting to delete it. This prevents potential errors from attempting to delete a non-existent product.
40-63: Creation logic increate_simple_product.The method
create_simple_productis well-implemented, allowing for properties to be set via an associative array. Ensure that the default properties meet the requirements for all tests that utilize this method.
107-121: Creation of external, grouped, and variation products.The methods for creating external, grouped, and variation products are implemented correctly. These methods facilitate comprehensive testing by allowing the creation of diverse product types.
Also applies to: 129-142, 153-246
260-273: Variation object creation logic increate_product_variation_object.The method correctly sets the properties and attributes for product variations. This is essential for tests that need to simulate different product configurations.
284-293: Attribute object creation logic increate_product_attribute_object.The method
create_product_attribute_objectis well-implemented, facilitating the creation of product attributes for testing. Ensure that all attributes created are relevant to the tests being conducted.
305-383: Attribute creation logic increate_attribute.The
create_attributemethod handles the creation of custom attributes efficiently, including cleaning up existing attributes to avoid conflicts. This method is crucial for tests that involve products with custom attributes.
76-99: Creation logic for downloadable products increate_downloadable_product.This method correctly sets the downloadable and virtual properties for the product. It's important to verify that all downloadable products created in tests have valid download data.
| $_POST = $args; // $_POST global variable is required in Dokan Registration. | ||
| // Todo: Need to handle the nonce verification in Dokan includes/Registration.php class. | ||
| $customer_id = parent::create_object( $args ); | ||
|
|
||
| $customer_id; | ||
| return parent::create_object( $args ); |
There was a problem hiding this comment.
Consider avoiding direct manipulation of global variables.
Directly setting $_POST within a factory method can lead to side effects that are hard to track and can affect other tests. Consider using dependency injection or mock objects instead.
- $_POST = $args; // $_POST global variable is required in Dokan Registration.
+ // Consider using a mock object or dependency injection for setting necessary POST data.Committable suggestion was skipped due to low confidence.
| public function create_object( $args ) { | ||
| add_filter( 'dokan_register_nonce_check', '__return_false' ); // Disable Dokan nonce verification. | ||
| if (! isset( $_POST['role'] ) ) { // 'role' is required to pass the Dokan Validation. | ||
|
|
||
| if ( ! isset( $_POST['role'] ) ) { // 'role' is required to pass the Dokan Validation. | ||
| $_POST['role'] = 'customer'; | ||
| } | ||
|
|
||
| $customer = $this->create_customer( $args['email'], $args[ 'password' ], $args['email'] ); | ||
| $customer = $this->create_customer( $args['email'], $args['password'], $args['email'] ); | ||
|
|
||
| if ( ! is_wp_error( $customer ) ) { | ||
| return $this->update_object( $customer->get_id(), $args );; | ||
| return $this->update_object( $customer->get_id(), $args ); | ||
| } | ||
|
|
||
| return 0; | ||
| } |
There was a problem hiding this comment.
Security Issue: Nonce verification is disabled.
Consider implementing a more secure way to handle nonce verification that does not disable it entirely.
- add_filter( 'dokan_register_nonce_check', '__return_false' );
+ // Implement a more secure method to handle nonce verification.Committable suggestion was skipped due to low confidence.
Tools
GitHub Check: Run PHPCS inspection
[failure] 22-22:
Processing form data without nonce verification.
mrabbani
force-pushed
the
fix/seller-registration-nonce
branch
from
776b56a
to
95e50d0
Compare
mrabbani
force-pushed
the
fix/seller-registration-nonce
branch
from
f6b5f82
to
4198f32
Compare
mrabbani
added
Upcoming Release
Needs: Testing
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
Enable to bypass the nonce verification
Verify nonce through method call which enable third-party developers to override the nonce check if necessary.
Bypass nonce:
add_filter( 'dokan_register_nonce_check', '__return_false' );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
Documentation
docs/tdd/readme.md.docs/tdd/factories.mdfor efficient test data creation using factory classes.Refactor
New Features
DokanTestCasefor fixture creation and database assertions.Tests