Update/tests manual order creation #2749
Conversation
WalkthroughThe manual order configuration end-to-end test suite was refactored to use a structured approach with a Changes
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: 5
🧹 Nitpick comments (1)
tests/pw/tests/e2e/ManualOrder.spec.ts (1)
24-79: Consider test dependencies and state isolationBoth tests might have implicit dependencies on the application state (e.g., existence of active vendors, specific settings). Consider adding setup/teardown for test state isolation.
Consider:
- Adding a test fixture to create test vendors if none exist
- Resetting manual order settings to a known state before each test
- Using test-specific data attributes to avoid conflicts with real data
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/pw/tests/e2e/ManualOrder.spec.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/pw/tests/e2e/ManualOrder.spec.ts (2)
tests/pw/pages/selectors.ts (1)
selector(3-8231)tests/pw/pages/settingsPage.ts (1)
SettingsPage(12-725)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (3)
tests/pw/tests/e2e/ManualOrder.spec.ts (3)
1-4: LGTM: Clean import structureThe imports are well-organized and properly typed, importing necessary Playwright modules and the project's helper classes.
6-7: LGTM: Good selector centralizationUsing centralized selectors from the selector object improves maintainability.
13-22: LGTM: Proper test lifecycle managementThe beforeAll/afterAll setup correctly creates an authenticated browser context and cleans up resources afterward.
| const activeVendorRow = admin.page.locator('.wp-list-table tr:has(.enabled [type="checkbox"]:checked)'); | ||
| const vendorNameCell = activeVendorRow.locator('td.store_name').nth(1); | ||
| await vendorNameCell.click(); | ||
|
|
||
| // Navigate to vendor edit page | ||
| const editLink = vendorNameCell.getByRole('link').nth(1); | ||
| await editLink.click(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify complex vendor selection logic
The current vendor selection logic is complex and fragile. Consider abstracting this into a helper method for better maintainability.
Add this method to SettingsPage class:
async selectActiveVendorForEdit(): Promise<void> {
const activeVendorRow = this.page.locator('.wp-list-table tr:has(.enabled [type="checkbox"]:checked)').first();
await activeVendorRow.locator('td.store_name a').nth(1).click();
}Then simplify the test:
- // Select an active vendor
- const activeVendorRow = admin.page.locator('.wp-list-table tr:has(.enabled [type="checkbox"]:checked)');
- const vendorNameCell = activeVendorRow.locator('td.store_name').nth(1);
- await vendorNameCell.click();
-
- // Navigate to vendor edit page
- const editLink = vendorNameCell.getByRole('link').nth(1);
- await editLink.click();
+ // Select an active vendor and navigate to edit page
+ await admin.selectActiveVendorForEdit();🤖 Prompt for AI Agents
In tests/pw/tests/e2e/ManualOrder.spec.ts around lines 49 to 55, the vendor
selection logic is complex and fragile. Refactor by moving this logic into a new
helper method named selectActiveVendorForEdit inside the SettingsPage class,
which locates the first active vendor row and clicks the second link in the
store_name cell. Then update the test code to call this new method instead of
manually locating and clicking elements, improving maintainability and
readability.
| const manualOrderCheckbox = admin.page.locator('.payment-info.edit-mode .checkbox-left:has(input[type="checkbox"][value="enableManualOrder"])'); | ||
| const manualOrderToggle = manualOrderCheckbox.locator('.switch > .slider'); | ||
| await manualOrderToggle.click(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use more reliable selectors for vendor settings
The current selector is overly complex and could be simplified using better targeting strategies.
- const manualOrderCheckbox = admin.page.locator('.payment-info.edit-mode .checkbox-left:has(input[type="checkbox"][value="enableManualOrder"])');
- const manualOrderToggle = manualOrderCheckbox.locator('.switch > .slider');
+ const manualOrderToggle = admin.page.locator('input[value="enableManualOrder"]').locator('..').locator('.switch .slider');📝 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.
| const manualOrderCheckbox = admin.page.locator('.payment-info.edit-mode .checkbox-left:has(input[type="checkbox"][value="enableManualOrder"])'); | |
| const manualOrderToggle = manualOrderCheckbox.locator('.switch > .slider'); | |
| await manualOrderToggle.click(); | |
| const manualOrderToggle = admin.page.locator('input[value="enableManualOrder"]').locator('..').locator('.switch .slider'); | |
| await manualOrderToggle.click(); |
🤖 Prompt for AI Agents
In tests/pw/tests/e2e/ManualOrder.spec.ts around lines 58 to 60, the current
selector for manualOrderCheckbox is too complex and fragile. Simplify the
selector by targeting a more specific and stable attribute or element unique to
the manual order setting, such as a data-testid or a unique class/id, to make
the selector more reliable and maintainable. Then update the manualOrderToggle
locator accordingly to use this simplified selector.
| const manualOrderOption = admin.page.getByRole('group').filter({ | ||
| hasText: 'Allow Vendors to Create Orders Enable vendors to create orders manually from' | ||
| }); | ||
| const toggleSwitch = manualOrderOption.locator('span').nth(1); | ||
| await toggleSwitch.click(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve selector reliability and maintainability
The current selector relies on hardcoded text content which is brittle and could break if the UI text changes. Consider using more reliable selectors.
- const manualOrderOption = admin.page.getByRole('group').filter({
- hasText: 'Allow Vendors to Create Orders Enable vendors to create orders manually from'
- });
- const toggleSwitch = manualOrderOption.locator('span').nth(1);
+ // Use data attributes or more specific CSS selectors
+ const manualOrderToggle = admin.page.locator('[data-testid="manual-order-toggle"]')
+ .or(admin.page.locator('.manual_order_create .switch'));Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In tests/pw/tests/e2e/ManualOrder.spec.ts around lines 30 to 34, the selector
uses hardcoded text which is fragile and prone to break if UI text changes.
Replace the text-based filter with more stable selectors such as data-testid
attributes, role with accessible names, or unique element IDs to improve
reliability and maintainability. Adjust the locator logic accordingly to avoid
dependence on exact text content.
| // await admin.reload(); | ||
| // await admin.toBeChecked(manualOrderOption.locator('span').nth(1)); |
There was a problem hiding this comment.
Re-enable test verification
The verification logic is commented out, which means the test doesn't actually verify that the setting was saved correctly. This reduces test reliability.
- // Verify setting is saved correctly
- // await admin.reload();
- // await admin.toBeChecked(manualOrderOption.locator('span').nth(1));
+ // Verify setting is saved correctly
+ await admin.reload();
+ await admin.goToSingleDokanSettings(settingsAdmin.menus.sellingOptions, data.dokanSettings.selling.settingTitle);
+ const verifyToggle = admin.page.locator('.manual_order_create .switch input[type="checkbox"]');
+ await admin.toBeChecked(verifyToggle);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In tests/pw/tests/e2e/ManualOrder.spec.ts around lines 40 to 41, the test
verification code is commented out, so the test does not confirm that the
setting was saved correctly. Uncomment the lines that reload the admin and check
the manualOrderOption locator to ensure the test properly verifies the expected
behavior.
| const activeVendorRow = admin.page.locator('.wp-list-table tr:has(.enabled [type="checkbox"]:checked)'); | ||
| const vendorNameCell = activeVendorRow.locator('td.store_name').nth(1); | ||
| await vendorNameCell.click(); | ||
|
|
||
| // Navigate to vendor edit page | ||
| const editLink = vendorNameCell.getByRole('link').nth(1); | ||
| await editLink.click(); | ||
|
|
||
| // Enable manual order option for this vendor | ||
| const manualOrderCheckbox = admin.page.locator('.payment-info.edit-mode .checkbox-left:has(input[type="checkbox"][value="enableManualOrder"])'); | ||
| const manualOrderToggle = manualOrderCheckbox.locator('.switch > .slider'); | ||
| await manualOrderToggle.click(); | ||
|
|
||
| // Save vendor profile changes | ||
| const saveButton = admin.page.locator('.profile-banner.edit-mode .action-links.edit-mode .button.button-primary'); | ||
| await saveButton.click(); | ||
|
|
||
| // Confirm update dialog | ||
| const updateDialog = admin.page.getByRole('dialog', { name: 'Vendor Updated' }); | ||
| await admin.toBeVisible(updateDialog); | ||
| await admin.page.getByRole('button', { name: 'OK' }).click(); | ||
|
|
||
| // Verify settings by going to edit mode again | ||
| const editButton = admin.page.locator('.profile-banner .action-links .button.router-link-active'); | ||
| await editButton.click(); | ||
|
|
||
| // Assertions to verify enabling was successful | ||
| await admin.toBeVisible(manualOrderCheckbox); | ||
| const checkboxInput = admin.page.locator('.payment-info.edit-mode .checkbox-left input[type="checkbox"][value="enableManualOrder"]'); | ||
| await admin.toBeChecked(checkboxInput); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for missing elements
The test assumes that active vendors exist and that navigation succeeds. Add error handling to make the test more robust.
+ // Verify active vendors exist before proceeding
+ const activeVendorCount = await admin.page.locator('.wp-list-table tr:has(.enabled [type="checkbox"]:checked)').count();
+ if (activeVendorCount === 0) {
+ throw new Error('No active vendors found for testing');
+ }
+
// Select an active vendor
const activeVendorRow = admin.page.locator('.wp-list-table tr:has(.enabled [type="checkbox"]:checked)');📝 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.
| const activeVendorRow = admin.page.locator('.wp-list-table tr:has(.enabled [type="checkbox"]:checked)'); | |
| const vendorNameCell = activeVendorRow.locator('td.store_name').nth(1); | |
| await vendorNameCell.click(); | |
| // Navigate to vendor edit page | |
| const editLink = vendorNameCell.getByRole('link').nth(1); | |
| await editLink.click(); | |
| // Enable manual order option for this vendor | |
| const manualOrderCheckbox = admin.page.locator('.payment-info.edit-mode .checkbox-left:has(input[type="checkbox"][value="enableManualOrder"])'); | |
| const manualOrderToggle = manualOrderCheckbox.locator('.switch > .slider'); | |
| await manualOrderToggle.click(); | |
| // Save vendor profile changes | |
| const saveButton = admin.page.locator('.profile-banner.edit-mode .action-links.edit-mode .button.button-primary'); | |
| await saveButton.click(); | |
| // Confirm update dialog | |
| const updateDialog = admin.page.getByRole('dialog', { name: 'Vendor Updated' }); | |
| await admin.toBeVisible(updateDialog); | |
| await admin.page.getByRole('button', { name: 'OK' }).click(); | |
| // Verify settings by going to edit mode again | |
| const editButton = admin.page.locator('.profile-banner .action-links .button.router-link-active'); | |
| await editButton.click(); | |
| // Assertions to verify enabling was successful | |
| await admin.toBeVisible(manualOrderCheckbox); | |
| const checkboxInput = admin.page.locator('.payment-info.edit-mode .checkbox-left input[type="checkbox"][value="enableManualOrder"]'); | |
| await admin.toBeChecked(checkboxInput); | |
| }); | |
| // Verify active vendors exist before proceeding | |
| const activeVendorCount = await admin.page | |
| .locator('.wp-list-table tr:has(.enabled [type="checkbox"]:checked)') | |
| .count(); | |
| if (activeVendorCount === 0) { | |
| throw new Error('No active vendors found for testing'); | |
| } | |
| // Select an active vendor | |
| const activeVendorRow = admin.page.locator('.wp-list-table tr:has(.enabled [type="checkbox"]:checked)'); | |
| const vendorNameCell = activeVendorRow.locator('td.store_name').nth(1); | |
| await vendorNameCell.click(); | |
| // Navigate to vendor edit page | |
| const editLink = vendorNameCell.getByRole('link').nth(1); | |
| await editLink.click(); | |
| // Enable manual order option for this vendor | |
| const manualOrderCheckbox = admin.page.locator('.payment-info.edit-mode .checkbox-left:has(input[type="checkbox"][value="enableManualOrder"])'); | |
| const manualOrderToggle = manualOrderCheckbox.locator('.switch > .slider'); | |
| await manualOrderToggle.click(); | |
| // Save vendor profile changes | |
| const saveButton = admin.page.locator('.profile-banner.edit-mode .action-links.edit-mode .button.button-primary'); | |
| await saveButton.click(); | |
| // Confirm update dialog | |
| const updateDialog = admin.page.getByRole('dialog', { name: 'Vendor Updated' }); | |
| await admin.toBeVisible(updateDialog); | |
| await admin.page.getByRole('button', { name: 'OK' }).click(); | |
| // Verify settings by going to edit mode again | |
| const editButton = admin.page.locator('.profile-banner .action-links .button.router-link-active'); | |
| await editButton.click(); | |
| // Assertions to verify enabling was successful | |
| await admin.toBeVisible(manualOrderCheckbox); | |
| const checkboxInput = admin.page.locator('.payment-info.edit-mode .checkbox-left input[type="checkbox"][value="enableManualOrder"]'); | |
| await admin.toBeChecked(checkboxInput); | |
| }); |
🤖 Prompt for AI Agents
In tests/pw/tests/e2e/ManualOrder.spec.ts around lines 49 to 79, the test
assumes the presence of active vendors and successful navigation without checks.
Add error handling by verifying the existence of elements like activeVendorRow,
vendorNameCell, and editLink before interacting with them. Use conditional
checks or try-catch blocks to handle cases where elements are missing, and fail
the test gracefully with informative error messages to improve robustness.
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