Commission tests #83
Commission tests #83
Conversation
WalkthroughThe pull request introduces significant enhancements to the Changes
Possibly related PRs
Suggested labels
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: 6
🧹 Outside diff range and nitpick comments (8)
tests/pw/pages/storeSupportsPage.ts (1)
229-232: Consider adding error handling for visibility checks.The visibility checks could benefit from try-catch blocks to handle cases where elements might not be present as expected, improving test reliability.
- await this.toBeVisible(closeTicketText); + try { + await this.toBeVisible(closeTicketText); + } catch (error) { + console.error(`Failed to verify closeTicketText visibility: ${error.message}`); + throw error; + }tests/pw/tests/e2e/commission.spec.ts (1)
Line range hint
82-89: ImplementcreateDokanSubscriptionProductfunction to enable subscription product testsThe tests for setting commission to Dokan subscription products are currently skipped because the
createDokanSubscriptionProductfunction is not implemented. Implementing this function will enable these tests to run and ensure proper coverage.Would you like assistance in implementing the
createDokanSubscriptionProductfunction, or should we open a new GitHub issue to track this task?tests/pw/pages/commissionPage.ts (1)
283-283: Avoid directly removing theclassattribute to handle flakinessUsing
removeAttributeto force row actions to be visible may lead to unintended side effects. Consider using more robust methods, such as ensuring the element is visible with explicit waits or adjusting the hover actions to prevent flakiness.tests/pw/utils/apiUtils.ts (1)
2070-2070: Address the TODO: ReplacecreateLineItemswithcreateLineItemsEnhancedThe TODO comment indicates that
createLineItemsshould be replaced withcreateLineItemsEnhanced, and the tests need updating. Completing this task will help remove deprecated code and ensure consistency.Would you like assistance in updating the tests and refactoring the code to use
createLineItemsEnhanced?tests/pw/utils/payloads.ts (1)
403-403: Consider uncommentingpost_authorif product author assignment is neededThe
post_authorfield is commented out in the product creation payload. If assigning the product to a specific author is required for your tests, please uncomment and set this field accordingly.Apply this diff if necessary:
-// post_author: '', +post_author: VENDOR_ID,tests/pw/pages/productsPage.ts (3)
20-26: Add JSDoc documentation for the new method.The
adminSearchProductmethod implementation looks good, but would benefit from JSDoc documentation describing its purpose, parameters, and return type.+/** + * Searches for a product in the admin interface + * @param productName - The name of the product to search for + * @returns Promise<void> + */ async adminSearchProduct(productName: string) {
313-313: Add error handling for earning element visibility check.Consider adding a try-catch block to handle cases where the earning element might not be present, especially since this is a vendor-specific feature.
- await this.toBeVisible(productsVendor.earning); + try { + await this.toBeVisible(productsVendor.earning); + } catch (error) { + console.warn('Earning element not visible for vendor:', error); + // Handle appropriately based on business requirements + }
Line range hint
1-1106: Consider breaking down the ProductsPage class into smaller, focused classes.The
ProductsPageclass has grown quite large and handles both admin and vendor functionalities. Consider splitting it into separate classes likeAdminProductsPageandVendorProductsPagefor better maintainability and separation of concerns.Address TODO comments in the codebase.
There are several TODO comments throughout the code that need attention:
- Line 4: "TODO: Add tests"
- Line 391: "todo: need a check for germanized plugin"
- Line 573: "todo: add more assertions"
- Line 595: "todo: add more assertions"
- Line 617: "todo: add more assertions"
Would you like me to help create GitHub issues to track these TODOs or provide implementations for any of these missing pieces?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
tests/pw/feature-map/feature-map.yml(1 hunks)tests/pw/pages/basePage.ts(3 hunks)tests/pw/pages/commissionPage.ts(2 hunks)tests/pw/pages/productsPage.ts(2 hunks)tests/pw/pages/selectors.ts(6 hunks)tests/pw/pages/storeSupportsPage.ts(1 hunks)tests/pw/tests/e2e/commission.spec.ts(4 hunks)tests/pw/utils/apiUtils.ts(3 hunks)tests/pw/utils/payloads.ts(3 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
tests/pw/pages/basePage.ts
[error] 1434-1435: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
🔇 Additional comments (8)
tests/pw/pages/storeSupportsPage.ts (1)
230-230: LGTM! Correct usage of visibility check method.
The change from multipleElementVisible to toBeVisible is appropriate since closeTicketText is a single element selector.
tests/pw/utils/payloads.ts (1)
1957-2006: Ensure environment variables for product IDs are properly set
In the createMultiVendorOrder payload, PRODUCT_ID and PRODUCT_ID_V2 are used. If these environment variables are undefined, the product IDs will be empty strings, which can cause order creation to fail.
Run the following script to verify that PRODUCT_ID and PRODUCT_ID_V2 are defined and valid numbers:
tests/pw/pages/selectors.ts (6)
2981-2983: Looks good!
The commissionColumn selector for the orders table has been added correctly.
2991-2997: LGTM!
The table column selectors for the commission meta box have been added correctly under the table object.
3034-3045: Looks good!
The menus object has been added correctly under the products object to encapsulate the product-related menu selectors.
3053-3058: Verify the usage of the new selectors in the codebase.
The commissionColumn, firstRowProductCommission, and earningColumn selectors have been added correctly for the products table. However, ensure that these selectors are being used appropriately in the relevant test files to retrieve the commission and earning data for products.
Run the following script to verify the usage of these selectors:
✅ Verification successful
Based on the search results, I can generate the final response as I have sufficient information about the usage of these selectors in the codebase.
The selectors are properly used in the test files
The selectors are being used appropriately in tests/pw/pages/commissionPage.ts:
commissionColumnis used in both product and order list viewsfirstRowProductCommissionis used to verify commission data in the products tableearningColumnis used in vendor-specific views for products and orders
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of `commissionColumn`, `firstRowProductCommission`, and `earningColumn` selectors.
# Test: Search for the selector usage. Expect: At least one occurrence of each selector.
rg -A 5 $'commissionColumn|firstRowProductCommission|earningColumn' tests/
Length of output: 6141
2985-2987: Verify the usage of the new selectors in the codebase.
The firstRow and firstRowOrderCommission selectors have been added correctly. However, ensure that these selectors are being used appropriately in the relevant test files to retrieve the first order row and its commission value.
Run the following script to verify the usage of these selectors:
✅ Verification successful
The selectors are being used appropriately in the codebase
The verification shows that both selectors are part of a well-structured selectors object and are being used correctly:
firstRowOrderCommissionis used incommissionPage.tswithin theviewCommissionOnOrderList()method to verify commission visibilityfirstRowis consistently used across different contexts (admin/vendor) for accessing first row elements in tables- The selectors follow the same XPath pattern as other similar selectors in the codebase
- They are used in conjunction with page object methods like
toBeVisible()for proper test assertions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of `firstRow` and `firstRowOrderCommission` selectors.
# Test: Search for the selector usage. Expect: At least one occurrence of each selector.
rg -A 5 $'firstRow|firstRowOrderCommission' tests/
Length of output: 6259
3001-3014: Verify the usage of the new selectors in the codebase.
The subOrdersMetaBox object and its child selectors have been added correctly. However, ensure that these selectors are being used appropriately in the relevant test files to interact with the sub-orders meta box and retrieve its data.
Run the following script to verify the usage of these selectors:
✅ Verification successful
Let me generate another script to verify how these selectors are being used in the test files:
This will help us understand:
- The context in which subOrdersMetaBox is being used in tests
- If there are any other usages of the sub-orders related selectors that we should be aware of
Let me generate one final script to check if there are any test cases that actually use the viewSubOrdersMetaBox method:
This will help us confirm if the selectors are being used in actual test scenarios.
Selectors are properly integrated and used in test scenarios
The subOrdersMetaBox selectors are correctly integrated into the test framework and actively used in:
- A dedicated test case that verifies sub-orders meta-box visibility on parent order details
- The
viewSubOrdersMetaBoxmethod incommissionPage.tsthat uses these selectors to verify the presence of meta-box elements - Part of a comprehensive test suite alongside related features (commission meta-box, related orders meta-box)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of `subOrdersMetaBox` selectors.
# Test: Search for the selector usage. Expect: At least one occurrence of the main selector.
rg -A 10 $'subOrdersMetaBox' tests/
Length of output: 2025
Script:
#!/bin/bash
# Search for test files that use the multipleElementVisible method with subOrdersMetaBox
rg -B 5 -A 10 "multipleElementVisible.*subOrdersMetaBox" tests/
# Search for any other usage of individual selectors within subOrdersMetaBox
rg "dokan_sub_or_related_orders" tests/
Length of output: 3380
Script:
#!/bin/bash
# Search for test files that use viewSubOrdersMetaBox method
rg -B 5 -A 10 "viewSubOrdersMetaBox" tests/
Length of output: 2675
| admin can view sub orders meta-box on parent order details [lite]: true | ||
| admin can view related orders meta-box on child order details [lite]: true | ||
| admin can view commission on product list [lite]: true | ||
| admin can view commission on order list [lite]: true | ||
| vendor can view earning on product list [lite]: true | ||
| vendor can view earning on product add page [lite]: true | ||
| vendor can view earning on product edit page [lite]: true | ||
| vendor can view earning on order list [lite]: true | ||
| vendor can view earning on order details [lite]: true |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Correct the grouping of 'vendor' features under the appropriate role
The vendor features added in lines 355-359 are currently placed under the admin role within the Commission page. To maintain consistency and clarity, these features should be grouped under the vendor role, similar to other sections in the file.
Apply this diff to adjust the grouping:
admin can set commission to Dokan subscription product (specific category based) [lite]: true
admin can view commission meta-box on order details [lite]: true
admin can view commission on product list [lite]: true
+vendor:
+ vendor can view earning on product list [lite]: true
+ vendor can view earning on product add page [lite]: true
+ vendor can view earning on product edit page [lite]: true
+ vendor can view earning on order list [lite]: true
+ vendor can view earning on order details [lite]: trueCommittable suggestion skipped: line range outside the PR's diff.
| // create multivendor line items | ||
| async createLineItemsEnhanced(multivendorLineItem: { author: string; products: string | string[]; quantities: string | string[] }[]) { | ||
| const lineItems = []; | ||
| for (const item of multivendorLineItem) { | ||
| const { author, products, quantities } = item; | ||
|
|
||
| // Validate lengths only if both `products` and `quantities` are arrays | ||
| if (Array.isArray(products) && Array.isArray(quantities) && products.length !== quantities.length) { | ||
| throw new Error('products and quantities must be the same length'); | ||
| } | ||
|
|
||
| const quantitiesArray = Array.isArray(quantities) ? quantities : [quantities]; | ||
| const length = Array.isArray(products) ? products.length : Number(item.products); | ||
| for (let i = 0; i < length; i++) { | ||
| const productId = Array.isArray(products) ? products[i] : (await this.createProduct({ ...payloads.createProduct(), post_author: author }, payloads.adminAuth))[1]; | ||
| const quantity = quantitiesArray[i % quantitiesArray.length]; | ||
| lineItems.push({ product_id: productId, quantity: quantity }); | ||
| } | ||
| } | ||
| return lineItems; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate inputs in createLineItemsEnhanced to prevent errors
-
Issue with
lengthcalculation: Thelengthvariable may becomeNaNifitem.productsis not a valid number whenproductsis not an array. This can cause unexpected behavior in the loop starting at line 2096. -
Potential undefined behavior: If
quantitiesis not properly formatted, usingquantitiesArray[i % quantitiesArray.length]may result in incorrect quantities or errors.
Apply this diff to improve input validation:
const quantitiesArray = Array.isArray(quantities) ? quantities : [quantities];
const length = Array.isArray(products) ? products.length : Number(item.products);
+if (isNaN(length) || length <= 0) {
+ throw new Error(`Invalid number of products specified: ${item.products}`);
+}
+if (!Array.isArray(quantitiesArray) || quantitiesArray.length === 0) {
+ throw new Error(`Quantities must be a non-empty array or a valid number`);
+}
for (let i = 0; i < length; i++) {
const productId = Array.isArray(products)
? products[i]
: (await this.createProduct({ ...payloads.createProduct(), post_author: author }, payloads.adminAuth))[1];
const quantity = quantitiesArray[i % quantitiesArray.length];
lineItems.push({ product_id: productId, quantity: quantity });
}📝 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.
| // create multivendor line items | |
| async createLineItemsEnhanced(multivendorLineItem: { author: string; products: string | string[]; quantities: string | string[] }[]) { | |
| const lineItems = []; | |
| for (const item of multivendorLineItem) { | |
| const { author, products, quantities } = item; | |
| // Validate lengths only if both `products` and `quantities` are arrays | |
| if (Array.isArray(products) && Array.isArray(quantities) && products.length !== quantities.length) { | |
| throw new Error('products and quantities must be the same length'); | |
| } | |
| const quantitiesArray = Array.isArray(quantities) ? quantities : [quantities]; | |
| const length = Array.isArray(products) ? products.length : Number(item.products); | |
| for (let i = 0; i < length; i++) { | |
| const productId = Array.isArray(products) ? products[i] : (await this.createProduct({ ...payloads.createProduct(), post_author: author }, payloads.adminAuth))[1]; | |
| const quantity = quantitiesArray[i % quantitiesArray.length]; | |
| lineItems.push({ product_id: productId, quantity: quantity }); | |
| } | |
| } | |
| return lineItems; | |
| } | |
| // create multivendor line items | |
| async createLineItemsEnhanced(multivendorLineItem: { author: string; products: string | string[]; quantities: string | string[] }[]) { | |
| const lineItems = []; | |
| for (const item of multivendorLineItem) { | |
| const { author, products, quantities } = item; | |
| // Validate lengths only if both `products` and `quantities` are arrays | |
| if (Array.isArray(products) && Array.isArray(quantities) && products.length !== quantities.length) { | |
| throw new Error('products and quantities must be the same length'); | |
| } | |
| const quantitiesArray = Array.isArray(quantities) ? quantities : [quantities]; | |
| const length = Array.isArray(products) ? products.length : Number(item.products); | |
| if (isNaN(length) || length <= 0) { | |
| throw new Error(`Invalid number of products specified: ${item.products}`); | |
| } | |
| if (!Array.isArray(quantitiesArray) || quantitiesArray.length === 0) { | |
| throw new Error(`Quantities must be a non-empty array or a valid number`); | |
| } | |
| for (let i = 0; i < length; i++) { | |
| const productId = Array.isArray(products) ? products[i] : (await this.createProduct({ ...payloads.createProduct(), post_author: author }, payloads.adminAuth))[1]; | |
| const quantity = quantitiesArray[i % quantitiesArray.length]; | |
| lineItems.push({ product_id: productId, quantity: quantity }); | |
| } | |
| } | |
| return lineItems; | |
| } |
| multivendorLineItems: [ | ||
| { | ||
| author: VENDOR_ID, | ||
| products: '3', // product Ids | ||
| quantities: '1', | ||
| }, | ||
| { | ||
| author: VENDOR2_ID, | ||
| products: '3', // product Ids | ||
| quantities: '1', | ||
| }, | ||
| ], |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace hardcoded product IDs and quantities with dynamic values
The products and quantities fields in multivendorLineItems are hardcoded as strings '3' and '1'. This may lead to issues if the product IDs do not exist or if multiple instances require different products. Use dynamic values or retrieve actual product IDs to enhance flexibility and reliability.
Apply this diff to use dynamic product IDs:
multivendorLineItems: [
{
author: VENDOR_ID,
- products: '3', // product Ids
- quantities: '1',
+ products: [PRODUCT_ID],
+ quantities: [1],
},
{
author: VENDOR2_ID,
- products: '3', // product Ids
- quantities: '1',
+ products: [PRODUCT_ID_V2],
+ quantities: [1],
},
],Committable suggestion skipped: line range outside the PR's diff.
| relatedOrdersMetaBox: { | ||
| metaBoxDiv: 'div#dokan_sub_or_related_orders', | ||
| relatedOrdersText: '//h2[normalize-space()="Related orders"]', | ||
| relatedOrdersItemInfo: 'div#dokan_sub_or_related_orders div#woocommerce-order-items', | ||
| table: { | ||
| orderColumn: '//div[@id="dokan_sub_or_related_orders"]//th[normalize-space()="Order"]', | ||
| dateColumn: '//div[@id="dokan_sub_or_related_orders"]//th[normalize-space()="Date"]', | ||
| statusColumn: '//div[@id="dokan_sub_or_related_orders"]//th[normalize-space()="Status"]', | ||
| totalColumn: '//div[@id="dokan_sub_or_related_orders"]//th[normalize-space()="Total"]', | ||
| vendorColumn: '//div[@id="dokan_sub_or_related_orders"]//th[normalize-space()="Vendor"]', | ||
| }, | ||
| relatedOrderTable: 'div#dokan_sub_or_related_orders table.woocommerce_order_items', | ||
| parentOrderRow: '//td[contains(.,"(Parent order)")]/..', | ||
| parentOrderVendor: '//td[contains(.,"(Parent order)")]/..//td[contains(.,"(no name)")]/..', | ||
| }, |
There was a problem hiding this comment.
💡 Codebase verification
The selectors are only partially utilized in the test implementation
The verification reveals that while the relatedOrdersMetaBox selector object is defined with detailed child selectors, only a basic visibility check is implemented in commissionPage.ts. The child selectors (parentOrderRow, parentOrderVendor, relatedOrderTable, etc.) are not being used in any test cases, suggesting incomplete test coverage for the related orders functionality.
tests/pw/pages/commissionPage.ts: Only checks for the visibility of the meta box without validating individual components- Missing test coverage for:
- Related orders table columns
- Parent order row identification
- Vendor information verification
- Table structure validation
🔗 Analysis chain
Verify the usage of the new selectors in the codebase.
The relatedOrdersMetaBox object and its child selectors have been added correctly. However, ensure that these selectors are being used appropriately in the relevant test files to interact with the related orders meta box and retrieve its data.
Run the following script to verify the usage of these selectors:
Let me search for more specific usage of the child selectors to ensure they are being properly utilized.
Let me check for any test files that might be using these selectors in test cases.
Let me try one final search to look for any test implementations that might be using these selectors, even if they're not in .spec or .test files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of `relatedOrdersMetaBox` selectors.
# Test: Search for the selector usage. Expect: At least one occurrence of the main selector.
rg -A 10 $'relatedOrdersMetaBox' tests/
Length of output: 2096
Script:
#!/bin/bash
# Search for usage of specific child selectors of relatedOrdersMetaBox
rg "parentOrderRow|parentOrderVendor|relatedOrderTable|metaBoxDiv" tests/
Length of output: 730
Script:
#!/bin/bash
# Search for test files that might be using these related order selectors
rg -g "*.spec.ts" -g "*.test.ts" "dokan_sub_or_related_orders|Related orders" tests/
Length of output: 86
Script:
#!/bin/bash
# Search for any test implementations using the related orders meta box
rg -g "*.ts" "multipleElementVisible.*relatedOrdersMetaBox|elementVisible.*relatedOrdersMetaBox|click.*relatedOrdersMetaBox|expect.*relatedOrdersMetaBox" tests/
Length of output: 294
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
Release Notes
New Features
Bug Fixes
Documentation