-
Notifications
You must be signed in to change notification settings - Fork 215
Add end-to-end tests for vendor delivery time settings React #2770
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
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughA new Playwright end-to-end test suite was introduced to verify the delivery time functionality in the vendor React dashboard. Supporting test data was updated with a new URL path for the delivery time settings page. The tests cover navigation, UI element verification, and interaction with delivery time settings. Changes
Sequence Diagram(s)sequenceDiagram
participant Vendor as Vendor User
participant Browser as Playwright Browser
participant Dashboard as Vendor React Dashboard
Vendor->>Browser: Start test with vendor authentication
Browser->>Dashboard: Navigate to dashboard URL
Browser->>Dashboard: Click on Delivery Time section
Dashboard-->>Browser: Render delivery time UI
Browser->>Dashboard: Interact with filters and calendar buttons
Browser->>Dashboard: Navigate to settings page
Dashboard-->>Browser: Show delivery time settings
Browser->>Dashboard: Interact with settings (toggle options, fill inputs)
Browser->>Dashboard: Save changes
Dashboard-->>Browser: Display success message
Browser->>Vendor: Close page after tests
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. 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 error Exit handler never called! ✨ 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 (
|
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: 6
🧹 Nitpick comments (2)
tests/pw/tests/e2e/deliverytime-react.spec.ts (2)
20-20
: Improve test name specificity.The test name "vendor can go to dashboard page and successfully found the heading" doesn't clearly describe what functionality is being tested beyond navigation.
Consider a more descriptive name like:
-test('vendor can go to dashboard page and successfully found the heading', { tag: ['@pro', '@vendor'] }, async () => { +test('vendor can navigate delivery time page and interact with filters and calendar controls', { tag: ['@pro', '@vendor'] }, async () => {
75-75
: Remove empty line for consistency.Extra blank line should be removed for better code formatting.
const orderPerSlotLabel = aPage.locator('label:has-text("Order per slot :")'); await expect(orderPerSlotLabel).toBeVisible(); - await aPage.getByRole('checkbox', { name: 'Home Delivery' }).check();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/pw/tests/e2e/deliverytime-react.spec.ts
(1 hunks)tests/pw/utils/testData.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (4)
tests/pw/utils/testData.ts (1)
1232-1232
: Add React delivery time route entry
The newsettingsDeliveryTimeReact
key is correctly placed next to the legacy setting and will allow the Playwright suite to navigate to the React-based delivery time page.tests/pw/tests/e2e/deliverytime-react.spec.ts (3)
1-3
: LGTM: Clean imports and proper structure.The imports follow Playwright best practices with proper use of page objects and test data utilities.
9-14
: LGTM: Proper test setup with authenticated context.The beforeAll hook correctly creates a vendor-authenticated browser context and initializes the page object.
53-53
: Verify the testData property exists.The test references
data.subUrls.frontend.vDashboard.settingsDeliveryTimeReact
which should be defined in the test data file.#!/bin/bash # Description: Verify that the settingsDeliveryTimeReact property exists in testData.ts # Expected: Property should be defined in the test data structure rg -A 5 -B 5 "settingsDeliveryTimeReact" tests/pw/utils/testData.ts
// Wait for 5 seconds | ||
await aPage.waitForTimeout( 3000 ); |
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.
🛠️ Refactor suggestion
Replace hard-coded timeout with explicit wait.
Hard-coded timeouts make tests flaky and slower. Use explicit waits for better reliability.
-// Wait for 5 seconds
-await aPage.waitForTimeout( 3000 );
+// Wait for the delivery time page to load
+await aPage.waitForLoadState('networkidle');
📝 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.
// Wait for 5 seconds | |
await aPage.waitForTimeout( 3000 ); | |
// Wait for the delivery time page to load | |
await aPage.waitForLoadState('networkidle'); |
🤖 Prompt for AI Agents
In tests/pw/tests/e2e/deliverytime-react.spec.ts around lines 24 to 25, replace
the hard-coded waitForTimeout(3000) call with an explicit wait that targets a
specific element or condition relevant to the test. Identify a reliable selector
or state change to wait for using Playwright's waitForSelector or
waitForFunction methods to improve test stability and speed.
// Wait for the page to load | ||
await aPage.waitForTimeout(3000); |
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.
🛠️ Refactor suggestion
Replace hard-coded timeout with explicit wait.
Another hard-coded timeout that should be replaced with proper wait conditions.
-// Wait for the page to load
-await aPage.waitForTimeout(3000);
+// Wait for the delivery time settings page to load
+await aPage.waitForSelector('.dokan-header-title h3');
📝 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.
// Wait for the page to load | |
await aPage.waitForTimeout(3000); | |
// Wait for the delivery time settings page to load | |
await aPage.waitForSelector('.dokan-header-title h3'); |
🤖 Prompt for AI Agents
In tests/pw/tests/e2e/deliverytime-react.spec.ts around lines 55 to 56, replace
the hard-coded waitForTimeout(3000) with an explicit wait condition that waits
for a specific element or state indicating the page has loaded. Identify a
reliable selector or event that confirms the page readiness and use a
waitForSelector or similar Playwright method instead of a fixed timeout.
await aPage.locator('.css-1nxca15-UA').click(); | ||
await aPage.getByRole('option', { name: 'Delivery' }).click(); | ||
await aPage.getByRole('button', { name: 'Filter' }).click(); | ||
await aPage.locator('.css-1nxca15-UA').click(); | ||
await aPage.getByRole('option', { name: 'Store Pickup' }).click(); | ||
await aPage.getByRole('button', { name: 'Filter' }).click(); | ||
await aPage.locator('.css-1nxca15-UA').click(); | ||
await aPage.getByRole('option', { name: 'All Events' }).click(); | ||
await aPage.getByRole('button', { name: 'Filter' }).click(); |
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.
🛠️ Refactor suggestion
Refactor repetitive dropdown interactions and avoid brittle CSS selectors.
The CSS class .css-1nxca15-UA
appears to be auto-generated and could break easily. The repetitive dropdown interaction pattern should be extracted to a helper method.
-await aPage.locator('.css-1nxca15-UA').click();
-await aPage.getByRole('option', { name: 'Delivery' }).click();
-await aPage.getByRole('button', { name: 'Filter' }).click();
-await aPage.locator('.css-1nxca15-UA').click();
-await aPage.getByRole('option', { name: 'Store Pickup' }).click();
-await aPage.getByRole('button', { name: 'Filter' }).click();
-await aPage.locator('.css-1nxca15-UA').click();
-await aPage.getByRole('option', { name: 'All Events' }).click();
-await aPage.getByRole('button', { name: 'Filter' }).click();
+// Use a more stable selector and extract to helper method
+const selectDeliveryFilter = async (optionName: string) => {
+ await aPage.getByRole('combobox', { name: 'Event Type Filter' }).click();
+ await aPage.getByRole('option', { name: optionName }).click();
+ await aPage.getByRole('button', { name: 'Filter' }).click();
+};
+
+await selectDeliveryFilter('Delivery');
+await selectDeliveryFilter('Store Pickup');
+await selectDeliveryFilter('All Events');
📝 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.
await aPage.locator('.css-1nxca15-UA').click(); | |
await aPage.getByRole('option', { name: 'Delivery' }).click(); | |
await aPage.getByRole('button', { name: 'Filter' }).click(); | |
await aPage.locator('.css-1nxca15-UA').click(); | |
await aPage.getByRole('option', { name: 'Store Pickup' }).click(); | |
await aPage.getByRole('button', { name: 'Filter' }).click(); | |
await aPage.locator('.css-1nxca15-UA').click(); | |
await aPage.getByRole('option', { name: 'All Events' }).click(); | |
await aPage.getByRole('button', { name: 'Filter' }).click(); | |
// Use a more stable selector and extract to helper method | |
const selectDeliveryFilter = async (optionName: string) => { | |
await aPage.getByRole('combobox', { name: 'Event Type Filter' }).click(); | |
await aPage.getByRole('option', { name: optionName }).click(); | |
await aPage.getByRole('button', { name: 'Filter' }).click(); | |
}; | |
await selectDeliveryFilter('Delivery'); | |
await selectDeliveryFilter('Store Pickup'); | |
await selectDeliveryFilter('All Events'); |
🤖 Prompt for AI Agents
In tests/pw/tests/e2e/deliverytime-react.spec.ts around lines 31 to 39, the code
repeatedly interacts with a dropdown using a brittle auto-generated CSS selector
'.css-1nxca15-UA'. Refactor by creating a helper function that accepts the
option name as a parameter, performs the dropdown click, selects the option by
role and name, and clicks the filter button. Replace the repetitive code with
calls to this helper, and avoid using the fragile CSS selector by finding a more
stable locator or passing it as part of the helper.
await aPage.waitForTimeout(100); | ||
const successMessage = aPage.locator('p:has-text("Delivery settings has been saved successfully!")'); | ||
await expect(successMessage).toBeVisible(); |
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.
Replace minimal timeout with proper wait condition.
The 100ms timeout before checking the success message is too short and unreliable.
-await aPage.waitForTimeout(100);
-const successMessage = aPage.locator('p:has-text("Delivery settings has been saved successfully!")');
-await expect(successMessage).toBeVisible();
+// Wait for the success message to appear after save
+const successMessage = aPage.locator('p:has-text("Delivery settings has been saved successfully!")');
+await expect(successMessage).toBeVisible({ timeout: 10000 });
📝 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.
await aPage.waitForTimeout(100); | |
const successMessage = aPage.locator('p:has-text("Delivery settings has been saved successfully!")'); | |
await expect(successMessage).toBeVisible(); | |
// Wait for the success message to appear after save | |
const successMessage = aPage.locator('p:has-text("Delivery settings has been saved successfully!")'); | |
await expect(successMessage).toBeVisible({ timeout: 10000 }); |
🤖 Prompt for AI Agents
In tests/pw/tests/e2e/deliverytime-react.spec.ts around lines 91 to 93, replace
the fixed 100ms waitForTimeout with a proper wait condition that waits for the
success message locator to be visible or attached. Remove the arbitrary timeout
and instead use Playwright's built-in waiting mechanisms like waitFor or expect
with toBeVisible to ensure the success message is reliably detected before
proceeding.
await aPage.getByRole('button', { name: 'week' }).click(); | ||
await aPage.getByRole('button', { name: 'day', exact: true }).click(); | ||
await aPage.getByRole('button', { name: 'list' }).click(); | ||
await aPage.getByRole('button', { name: 'month' }).click(); | ||
await aPage.getByRole('button', { name: 'Next month' }).click(); | ||
await aPage.getByRole('button', { name: 'Next month' }).click(); | ||
await aPage.getByRole('button', { name: 'Previous month' }).click(); | ||
await aPage.getByRole('button', { name: 'Previous month' }).click(); | ||
await aPage.getByRole('button', { name: 'Previous month' }).click(); | ||
await aPage.getByRole('button', { name: 'today' }).click(); |
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.
🛠️ Refactor suggestion
Add assertions for calendar view interactions.
The test interacts with multiple calendar controls but doesn't verify that the interactions actually work. Consider adding assertions to validate the UI state changes.
await aPage.getByRole('button', { name: 'week' }).click();
+await expect(aPage.getByRole('button', { name: 'week' })).toHaveClass(/active|selected/);
+
await aPage.getByRole('button', { name: 'day', exact: true }).click();
+await expect(aPage.getByRole('button', { name: 'day', exact: true })).toHaveClass(/active|selected/);
📝 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.
await aPage.getByRole('button', { name: 'week' }).click(); | |
await aPage.getByRole('button', { name: 'day', exact: true }).click(); | |
await aPage.getByRole('button', { name: 'list' }).click(); | |
await aPage.getByRole('button', { name: 'month' }).click(); | |
await aPage.getByRole('button', { name: 'Next month' }).click(); | |
await aPage.getByRole('button', { name: 'Next month' }).click(); | |
await aPage.getByRole('button', { name: 'Previous month' }).click(); | |
await aPage.getByRole('button', { name: 'Previous month' }).click(); | |
await aPage.getByRole('button', { name: 'Previous month' }).click(); | |
await aPage.getByRole('button', { name: 'today' }).click(); | |
await aPage.getByRole('button', { name: 'week' }).click(); | |
await expect(aPage.getByRole('button', { name: 'week' })).toHaveClass(/active|selected/); | |
await aPage.getByRole('button', { name: 'day', exact: true }).click(); | |
await expect(aPage.getByRole('button', { name: 'day', exact: true })).toHaveClass(/active|selected/); | |
await aPage.getByRole('button', { name: 'list' }).click(); | |
await aPage.getByRole('button', { name: 'month' }).click(); | |
await aPage.getByRole('button', { name: 'Next month' }).click(); | |
await aPage.getByRole('button', { name: 'Next month' }).click(); | |
await aPage.getByRole('button', { name: 'Previous month' }).click(); | |
await aPage.getByRole('button', { name: 'Previous month' }).click(); | |
await aPage.getByRole('button', { name: 'Previous month' }).click(); | |
await aPage.getByRole('button', { name: 'today' }).click(); |
🤖 Prompt for AI Agents
In tests/pw/tests/e2e/deliverytime-react.spec.ts around lines 40 to 49, the test
clicks various calendar view buttons but lacks assertions to verify the UI state
changes. Add assertions after each interaction to confirm that the calendar view
updates as expected, such as checking for visible elements or active states
corresponding to the selected view or month.
await aPage.getByRole('checkbox', { name: 'Home Delivery' }).check(); | ||
await aPage.getByRole('checkbox', { name: 'Home Delivery' }).uncheck(); | ||
await aPage.getByRole('checkbox', { name: 'Store Pickup' }).check(); | ||
await aPage.getByRole('checkbox', { name: 'Store Pickup' }).uncheck(); | ||
await aPage.getByRole('spinbutton', { name: 'Delivery blocked buffer :' }).click(); | ||
await aPage.getByRole('spinbutton', { name: 'Delivery blocked buffer :' }).fill('10'); | ||
await aPage.getByRole('spinbutton', { name: 'Time slot :' }).click(); | ||
await aPage.getByRole('spinbutton', { name: 'Time slot :' }).press('ControlOrMeta+a'); | ||
await aPage.getByRole('spinbutton', { name: 'Time slot :' }).fill('60'); | ||
await aPage.getByRole('spinbutton', { name: 'Order per slot :' }).click(); | ||
await aPage.getByRole('spinbutton', { name: 'Order per slot :' }).press('ControlOrMeta+a'); | ||
await aPage.getByRole('spinbutton', { name: 'Order per slot :' }).fill('1'); |
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.
🛠️ Refactor suggestion
Improve form interaction reliability.
The form interactions could be more robust with proper waits and state verification.
await aPage.getByRole('checkbox', { name: 'Home Delivery' }).check();
+await expect(aPage.getByRole('checkbox', { name: 'Home Delivery' })).toBeChecked();
+
await aPage.getByRole('checkbox', { name: 'Home Delivery' }).uncheck();
+await expect(aPage.getByRole('checkbox', { name: 'Home Delivery' })).not.toBeChecked();
+
await aPage.getByRole('checkbox', { name: 'Store Pickup' }).check();
+await expect(aPage.getByRole('checkbox', { name: 'Store Pickup' })).toBeChecked();
+
await aPage.getByRole('checkbox', { name: 'Store Pickup' }).uncheck();
+await expect(aPage.getByRole('checkbox', { name: 'Store Pickup' })).not.toBeChecked();
📝 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.
await aPage.getByRole('checkbox', { name: 'Home Delivery' }).check(); | |
await aPage.getByRole('checkbox', { name: 'Home Delivery' }).uncheck(); | |
await aPage.getByRole('checkbox', { name: 'Store Pickup' }).check(); | |
await aPage.getByRole('checkbox', { name: 'Store Pickup' }).uncheck(); | |
await aPage.getByRole('spinbutton', { name: 'Delivery blocked buffer :' }).click(); | |
await aPage.getByRole('spinbutton', { name: 'Delivery blocked buffer :' }).fill('10'); | |
await aPage.getByRole('spinbutton', { name: 'Time slot :' }).click(); | |
await aPage.getByRole('spinbutton', { name: 'Time slot :' }).press('ControlOrMeta+a'); | |
await aPage.getByRole('spinbutton', { name: 'Time slot :' }).fill('60'); | |
await aPage.getByRole('spinbutton', { name: 'Order per slot :' }).click(); | |
await aPage.getByRole('spinbutton', { name: 'Order per slot :' }).press('ControlOrMeta+a'); | |
await aPage.getByRole('spinbutton', { name: 'Order per slot :' }).fill('1'); | |
await aPage.getByRole('checkbox', { name: 'Home Delivery' }).check(); | |
await expect(aPage.getByRole('checkbox', { name: 'Home Delivery' })).toBeChecked(); | |
await aPage.getByRole('checkbox', { name: 'Home Delivery' }).uncheck(); | |
await expect(aPage.getByRole('checkbox', { name: 'Home Delivery' })).not.toBeChecked(); | |
await aPage.getByRole('checkbox', { name: 'Store Pickup' }).check(); | |
await expect(aPage.getByRole('checkbox', { name: 'Store Pickup' })).toBeChecked(); | |
await aPage.getByRole('checkbox', { name: 'Store Pickup' }).uncheck(); | |
await expect(aPage.getByRole('checkbox', { name: 'Store Pickup' })).not.toBeChecked(); | |
await aPage.getByRole('spinbutton', { name: 'Delivery blocked buffer :' }).click(); | |
await aPage.getByRole('spinbutton', { name: 'Delivery blocked buffer :' }).fill('10'); | |
await aPage.getByRole('spinbutton', { name: 'Time slot :' }).click(); | |
await aPage.getByRole('spinbutton', { name: 'Time slot :' }).press('ControlOrMeta+a'); | |
await aPage.getByRole('spinbutton', { name: 'Time slot :' }).fill('60'); | |
await aPage.getByRole('spinbutton', { name: 'Order per slot :' }).click(); | |
await aPage.getByRole('spinbutton', { name: 'Order per slot :' }).press('ControlOrMeta+a'); | |
await aPage.getByRole('spinbutton', { name: 'Order per slot :' }).fill('1'); |
🤖 Prompt for AI Agents
In tests/pw/tests/e2e/deliverytime-react.spec.ts around lines 76 to 87, the form
interactions lack waits and state verification, which can cause flakiness. Add
explicit waits for elements to be visible and enabled before interacting, and
verify the state after each action (e.g., check that checkboxes are actually
checked or unchecked). Use appropriate Playwright waitFor methods to ensure
elements are ready for interaction to improve test reliability.
All Submissions:
Closes
Summary by CodeRabbit