refactor: update selectors and tests for vendor return request functionality #2748
Conversation
WalkthroughSelectors for vendor return request menus and tables were updated from XPath to CSS, with new menu items added. Test cases were refactored to use direct navigation and explicit selector checks instead of helper methods. The return request dashboard URL was updated to a new path in test data. Changes
Sequence Diagram(s)sequenceDiagram
participant Vendor
participant Browser
participant Selectors
participant TestData
Vendor->>Browser: Navigate to new return request URL (from TestData)
Browser->>Selectors: Use updated CSS selectors for menu/table
Browser->>Vendor: Display return request UI
Vendor->>Browser: Perform UI checks (menu, table, details)
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm warn config production Use ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 2
🧹 Nitpick comments (2)
tests/pw/tests/e2e/vendorReturnRequest.spec.ts (1)
107-107: Remove debug console.log statement.This console.log appears to be debug code and should be removed before merging.
- console.log('data.subUrls.frontend.vDashboard.returnRequest', data.subUrls.frontend.vDashboard.returnRequest)tests/pw/pages/selectors.ts (1)
5101-5104: Consider unifying selector styles for table headers
You’ve added a CSS selector for the table root (#dokan-rma-requests-data-view), which is great. For consistency and maintainability, consider migrating the header selectors (detailsColumn,productsColumn,typeColumn) from XPath to CSS as well.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tests/pw/pages/selectors.ts(3 hunks)tests/pw/tests/e2e/vendorReturnRequest.spec.ts(3 hunks)tests/pw/utils/testData.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/pw/tests/e2e/vendorReturnRequest.spec.ts (2)
tests/pw/pages/selectors.ts (1)
selector(3-8233)tests/pw/utils/testData.ts (1)
data(63-2914)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (7)
tests/pw/utils/testData.ts (1)
1193-1193: LGTM! URL path updated to support hash-based navigation.The update to the vendor return request URL from
'dashboard/return-request'to'dashboard/new/#/return-request'correctly reflects the migration to client-side hash-based navigation. This change aligns well with the broader refactoring effort for vendor return request functionality mentioned in the PR objectives.tests/pw/tests/e2e/vendorReturnRequest.spec.ts (4)
8-13: LGTM! Good refactoring to centralize selectors.This change properly imports selectors from the centralized module and creates well-named variables for reuse throughout the test file. This improves maintainability and follows good testing practices.
68-82: Excellent refactoring to explicit test steps.The test is now much more readable and explicit about what it's verifying. The conditional logic properly handles both scenarios (with and without return requests), and the selector usage is consistent with the centralized selectors.
86-102: Well-structured explicit test implementation.The test now clearly verifies all the important elements of the RMA settings page with proper selector usage. The explicit assertions make it easy to understand what functionality is being tested.
109-125: Verify orderId availability and good refactoring pattern.The explicit test steps and proper destructuring to exclude
sendRefundfrom status checks are well implemented. However,orderIdmay be undefined due to the commented order creation code inbeforeAll.Ensure that
orderIdis properly initialized before this test runs, either through the commented setup code or via API-based order creation.tests/pw/pages/selectors.ts (2)
5092-5096: Updated menu selectors look solid
Switching from XPath to CSS with:has-textimproves readability and robustness.
5109-5111: Verify the row count selector accuracy
Ensure that#dokan-rma-requests-data-view tbody trcorrectly captures all rows in the table (e.g., including cases like grouped rows or nested<tbody>).
| // await customer1.addProductToCartFromSingleProductPage(data.predefined.simpleProduct.product1.name); | ||
| // await customer1.goToCheckout(); | ||
| // orderId = await customer1.paymentOrder(); | ||
| // await vendor1.updateOrderStatusOnTable(orderId, 'processing'); | ||
| // await customer.customerRequestWarranty(orderId, data.predefined.simpleProduct.product1.name, data.rma.requestWarranty); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Flag potential dependency issue with commented order creation.
The commented order creation code may cause orderId to be undefined, which is used in later tests (lines 109, 129, 133, 137, 142, 147). This could lead to test failures.
Consider either:
- Uncommenting this code if tests depend on it
- Implementing API-based order creation as suggested by the todo comment
- Adding proper setup for orderId in tests that require it
🏁 Script executed:
#!/bin/bash
# Check if orderId is used elsewhere in the test file without proper initialization
rg -n "orderId" tests/pw/tests/e2e/vendorReturnRequest.spec.tsLength of output: 1637
Ensure orderId is initialized before use in vendorReturnRequest tests
In tests/pw/tests/e2e/vendorReturnRequest.spec.ts, the top-level declaration
let orderId: string;never receives a value because both the UI-based setup (lines 43–45) and the API-based setup (lines 49–50) are commented out. Meanwhile, orderId is referenced later at lines 109, 129, 134, 138, 142, and 147, which will lead to runtime errors.
Locations needing attention:
- Line 23:
let orderId: string;(declared but unassigned) - Lines 43–45: commented UI-based setup
- Lines 49–50: commented API-based setup
- Lines 109, 129, 134, 138, 142, 147: usages of
orderIdwithout initialization
Recommended fixes:
- Uncomment one of the existing setup blocks (UI or API) so
orderIdis assigned before use - Or implement a new API-based order creation in a
beforeAll/beforeEach - Or initialize
orderIddirectly within each test before it’s referenced
🤖 Prompt for AI Agents
In tests/pw/tests/e2e/vendorReturnRequest.spec.ts around lines 41 to 45, the
variable orderId is declared but never assigned because both the UI-based and
API-based order creation code blocks are commented out. Since orderId is used
later in the test file at lines 109, 129, 134, 138, 142, and 147, this will
cause runtime errors. To fix this, uncomment one of the existing order creation
setups (either the UI-based code at lines 41-45 or the API-based code at lines
49-50) to ensure orderId is properly initialized before use, or implement a new
API-based order creation in a beforeAll or beforeEach hook, or assign orderId
directly within each test before it is referenced.
| // “p1_v1-simple” has been added to your cart. | ||
| productAddedSuccessMessage: (productName: string) => `//div[@class="woocommerce-message" and contains(.,"“${productName}” has been added to your cart.")]`, | ||
| productWithQuantityAddedSuccessMessage: (productName: string, quantity: string) => `//div[@class="woocommerce-message" and contains(.,"${quantity} × “${productName}” have been added to your cart.")]`, |
There was a problem hiding this comment.
Fix smart quotes in success message selectors
Using curly quotes (“/”) in the XPath may not match the actual DOM text if the UI uses straight quotes. Replace them with standard quotes or adjust to match the exact markup.
Proposed fix:
- // “p1_v1-simple” has been added to your cart.
+ // "{productName}" has been added to your cart.
- productAddedSuccessMessage: (productName: string) => `//div[@class="woocommerce-message" and contains(.,"“${productName}” has been added to your cart.")]`,
+ productAddedSuccessMessage: (productName: string) => `//div[@class="woocommerce-message" and contains(.,"\"${productName}\" has been added to your cart.")]`,
- productWithQuantityAddedSuccessMessage: (productName: string, quantity: string) => `//div[@class="woocommerce-message" and contains(.,"${quantity} × “${productName}” have been added to your cart.")]`,
+ productWithQuantityAddedSuccessMessage: (productName: string, quantity: string) => `//div[@class="woocommerce-message" and contains(.,"${quantity} × \"${productName}\" have been added to your cart.")]`,📝 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.
| // “p1_v1-simple” has been added to your cart. | |
| productAddedSuccessMessage: (productName: string) => `//div[@class="woocommerce-message" and contains(.,"“${productName}” has been added to your cart.")]`, | |
| productWithQuantityAddedSuccessMessage: (productName: string, quantity: string) => `//div[@class="woocommerce-message" and contains(.,"${quantity} × “${productName}” have been added to your cart.")]`, | |
| // "{productName}" has been added to your cart. | |
| productAddedSuccessMessage: (productName: string) => `//div[@class="woocommerce-message" and contains(.,"\"${productName}\" has been added to your cart.")]`, | |
| productWithQuantityAddedSuccessMessage: (productName: string, quantity: string) => `//div[@class="woocommerce-message" and contains(.,"${quantity} × \"${productName}\" have been added to your cart.")]`, |
🤖 Prompt for AI Agents
In tests/pw/pages/selectors.ts around lines 7276 to 7278, the XPath selectors
use curly smart quotes around product names, which may not match the actual DOM
text if the UI uses straight quotes. Replace the curly quotes with standard
straight quotes in the XPath strings to ensure correct matching of the success
message elements.
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
Bug Fixes
Tests