Add module tests #2481
Add module tests #2481
Conversation
WalkthroughThe pull request encompasses extensive modifications across several files, primarily focusing on enhancing the configuration and functionality of the Dokan platform. Key updates include the addition of numerous environment variables related to payment processing, the introduction of new asynchronous methods for managing various modules (such as enabling and disabling features), and improvements to the test suite for better coverage of these functionalities. The changes also involve restructuring of existing methods and properties to improve clarity and maintainability. Changes
Possibly related PRs
Suggested reviewers
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: 53
🧹 Outside diff range and nitpick comments (84)
tests/pw/pages/liveChatPage.ts (1)
26-41: Consider enhancing test coverage for module state transitionsThe new methods verify module visibility but consider adding:
- Tests that verify chat functionality is actually disabled when module is disabled
- Tests for state transitions (enabling/disabling) and their impact on existing chats
- Edge cases like disabling module while active chats exist
Consider creating a new test file
moduleState.spec.tsto cover these scenarios. Would you like me to help create this file?tests/pw/tests/api/_env.setup.ts (2)
Line range hint
269-284: Add error handling for product creation failuresThe product recreation logic could be more robust. Consider adding proper error handling and logging for product creation failures.
Here's a suggested improvement:
setup('recreate reverse withdrawal payment product', { tag: ['@lite'] }, async () => { let product = await apiUtils.checkProductExistence('Reverse Withdrawal Payment', payloads.adminAuth); if (!product) { console.log("Reverse withdrawal payment product doesn't exists!!"); - const [, reverseWithdrawalPaymentProduct] = await apiUtils.createProduct(payloads.reverseWithdrawalPaymentProduct, payloads.adminAuth); - await dbUtils.setOptionValue(dbData.dokan.paymentProducts.reverseWithdraw, reverseWithdrawalPaymentProduct, false); - product = await apiUtils.checkProductExistence('Reverse Withdrawal Payment', payloads.adminAuth); - expect(product).toBeTruthy(); - console.log('Recreated reverse withdrawal payment product'); + try { + const [, reverseWithdrawalPaymentProduct] = await apiUtils.createProduct(payloads.reverseWithdrawalPaymentProduct, payloads.adminAuth); + await dbUtils.setOptionValue(dbData.dokan.paymentProducts.reverseWithdraw, reverseWithdrawalPaymentProduct, false); + product = await apiUtils.checkProductExistence('Reverse Withdrawal Payment', payloads.adminAuth); + if (!product) { + throw new Error('Failed to verify product creation'); + } + console.log('Successfully recreated reverse withdrawal payment product'); + } catch (error) { + console.error('Failed to recreate reverse withdrawal payment product:', error); + throw error; + } } expect(product).toBeTruthy(); });
Line range hint
11-264: Consider refactoring repeated setup/teardown patternsThe ApiUtils setup and teardown logic is duplicated across test suites. Consider extracting this into a shared fixture or helper.
Here's a suggested approach:
// Create a shared fixture in a separate file, e.g., fixtures.ts export const withApiUtils = setup.extend<{ apiUtils: ApiUtils }>({ apiUtils: async ({ }, use) => { const apiUtils = new ApiUtils(await request.newContext()); await use(apiUtils); await apiUtils.dispose(); }, }); // Then in your test file: const test = withApiUtils; test.describe('setup woocommerce settings', () => { test.use({ extraHTTPHeaders: payloads.adminAuth }); test('set woocommerce settings', { tag: ['@lite'] }, async ({ apiUtils }) => { // Use apiUtils directly from the fixture await apiUtils.updateBatchWcSettingsOptions('general', payloads.general); // ... }); // ... });This approach would:
- Reduce code duplication
- Ensure consistent setup/teardown
- Make the tests more maintainable
tests/pw/pages/sellerVacationPage.ts (2)
16-21: Enhance error handling and method behavior.The method could be improved in several ways:
- Add error handling for navigation failures
- Return a boolean indicating success
- Actually enable the module instead of just checking visibility
Consider this implementation:
- async enableSellerVacationModule() { + async enableSellerVacationModule(): Promise<boolean> { // vendor dashboard settings - await this.goto(data.subUrls.frontend.vDashboard.settingsStore); - await this.toBeVisible(settingsVendor.vacation.vacationDiv); + try { + await this.goto(data.subUrls.frontend.vDashboard.settingsStore); + await this.toBeVisible(settingsVendor.vacation.vacationDiv); + // TODO: Add actual module enabling logic here + return true; + } catch (error) { + console.error('Failed to enable seller vacation module:', error); + return false; + } }
23-28: Reduce duplication and enhance error handling.The method shares similar patterns with
enableSellerVacationModule. Consider extracting common functionality and adding error handling.Consider this implementation:
+ private async navigateToSettings(): Promise<boolean> { + try { + await this.goto(data.subUrls.frontend.vDashboard.settingsStore); + return true; + } catch (error) { + console.error('Failed to navigate to settings:', error); + return false; + } + } + - async disableSellerVacationModule() { + async disableSellerVacationModule(): Promise<boolean> { // vendor dashboard settings - await this.goto(data.subUrls.frontend.vDashboard.settingsStore); - await this.notToBeVisible(settingsVendor.vacation.vacationDiv); + try { + if (!await this.navigateToSettings()) { + return false; + } + await this.notToBeVisible(settingsVendor.vacation.vacationDiv); + // TODO: Add actual module disabling logic here + return true; + } catch (error) { + console.error('Failed to disable seller vacation module:', error); + return false; + } }tests/pw/pages/selectors.ts (1)
Line range hint
1-8250: Consider improving code organization and maintainabilityWhile the selectors are well defined, consider the following suggestions to improve maintainability:
- Split the large selector object into smaller, more focused files based on functionality
- Add TypeScript interfaces to define the structure of selector objects
- Consider using constants for commonly used selector patterns
- Add JSDoc comments to document complex selector patterns
tests/pw/pages/printfulPage.ts (1)
14-24: Add JSDoc documentation and improve error handling.The method should include JSDoc documentation explaining its purpose, expected state, and return value. Consider adding error handling for navigation failures.
Add documentation and error handling:
+ /** + * Enables the Printful module and verifies its visibility in both admin and vendor areas. + * Assumes the module starts in a disabled state. + * @throws Error if navigation fails or if module elements are not visible + */ async enablePrintfulModule() { + try { // dokan settings await this.goto(data.subUrls.backend.dokan.settings); await this.toBeVisible(selector.admin.dokan.settings.menus.printful); // vendor dashboard menu await this.goto(data.subUrls.frontend.vDashboard.dashboard); await this.hover(selector.vendor.vDashboard.menus.primary.settings); await this.toBeVisible(selector.vendor.vDashboard.menus.subMenus.printful); + } catch (error) { + throw new Error(`Failed to enable Printful module: ${error.message}`); + } }tests/pw/pages/storeSupportsPage.ts (2)
24-45: Consider enhancing error handling and readability.The method has a clear structure and good separation of concerns. However, consider these improvements:
- Add error handling for navigation failures
- Extract URL constants to a configuration object
- Add JSDoc comments to document the expected behavior
+ /** + * Enables the store support module and verifies visibility across different pages + * @param storeName - The name of the store to check + * @throws {Error} When navigation fails or elements are not visible + */ async enableStoreSupportModule(storeName: string) { + const pages = { + dokanMenu: data.subUrls.backend.dokan.dokan, + dokanSettings: data.subUrls.backend.dokan.settings, + vendorDashboard: data.subUrls.frontend.vDashboard.dashboard, + customerDashboard: data.subUrls.frontend.myAccount, + }; + try { - await this.goto(data.subUrls.backend.dokan.dokan); + await this.goto(pages.dokanMenu); await this.toBeVisible(selector.admin.dokan.menus.storeSupport); // ... rest of the method + } catch (error) { + throw new Error(`Failed to enable store support module: ${error.message}`); + } }
Line range hint
1-11: Enhance type safety and documentation.The class has good organization but could benefit from these architectural improvements:
- Define TypeScript interfaces for common parameter types
- Add comprehensive JSDoc documentation
- Consider splitting the class into smaller role-specific classes
+ /** Interface for module verification options */ + interface ModuleVerificationOptions { + storeName: string; + checkUrls: string[]; + selectors: string[]; + } + + /** Interface for support ticket actions */ + interface SupportTicketAction { + ticketId: string; + message?: string; + status?: 'open' | 'closed'; + }Consider splitting this class into:
AdminStoreSupportPageVendorStoreSupportPageCustomerStoreSupportPageThis would improve maintainability and follow the Single Responsibility Principle.
tests/pw/tests/e2e/vendorStaff.spec.ts (2)
8-10: Consider adding type annotations for better code clarityThe page variables could benefit from explicit type annotations to improve code readability and maintainability.
-let aPage: Page, vPage: Page; +let aPage: Page; +let vPage: Page;
15-18: Add error handling for page creationThe page creation process should include error handling to gracefully handle initialization failures.
-aPage = await adminContext.newPage(); -admin = new VendorStaffPage(aPage); +try { + aPage = await adminContext.newPage(); + admin = new VendorStaffPage(aPage); +} catch (error) { + console.error('Failed to initialize admin page:', error); + throw error; +}tests/pw/pages/vendorStaffPage.ts (2)
18-23: Add timeout and custom error message for visibility checkThe visibility check should include a reasonable timeout and descriptive error message for better debugging.
async enableVendorStaffModule() { await this.goto(data.subUrls.frontend.vDashboard.dashboard); - await this.toBeVisible(selector.vendor.vDashboard.menus.primary.staff); + await this.toBeVisible(selector.vendor.vDashboard.menus.primary.staff, { + timeout: 5000, + message: 'Staff menu should be visible after enabling the module' + }); }
Line range hint
123-124: Track TODOs for booking and auction module testsThere are two TODO comments for adding booking and auction module checks. These should be tracked and implemented.
Would you like me to create GitHub issues to track the implementation of these module checks?
Also applies to: 132-133
tests/pw/pages/spmvPage.ts (2)
20-38: Consider improving error handling and maintainability.While the method effectively verifies SPMV module visibility across different pages, consider these improvements:
- Add error handling for navigation failures
- Extract page-specific checks into separate methods
- Add more descriptive comments about expected states
Consider refactoring to:
async enableSpmvModule() { + // Helper method to handle navigation with error checking + const navigateAndCheck = async (url: string, checkFn: () => Promise<void>) => { + try { + await this.goto(url); + await checkFn(); + } catch (error) { + throw new Error(`Failed to verify SPMV module at ${url}: ${error}`); + } + }; + + // Check SPMV visibility in admin settings + const checkAdminSettings = async () => { + await this.toBeVisible(selector.admin.dokan.settings.menus.singleProductMultiVendor); + }; + + // Check SPMV visibility in admin dashboard + const checkAdminDashboard = async () => { + await this.toBeVisible(spmvAdmin.spmvDiv); + }; + + // Check SPMV visibility in vendor dashboard + const checkVendorDashboard = async () => { + await this.clickAndWaitForLoadState(productsVendor.addNewProduct); + await this.toBeVisible(spmvVendor.search.searchDiv); + }; + + // Check SPMV visibility in vendor menu + const checkVendorMenu = async () => { + await this.multipleElementVisible([spmvVendor.search.searchDiv, spmvVendor.spmvDetailsDiv]); + }; + + // Verify SPMV module visibility across all pages + await navigateAndCheck(data.subUrls.backend.dokan.settings, checkAdminSettings); + await navigateAndCheck(data.subUrls.backend.wc.addNewProducts, checkAdminDashboard); + await navigateAndCheck(data.subUrls.frontend.vDashboard.products, checkVendorDashboard); + await navigateAndCheck(data.subUrls.frontend.vDashboard.spmv, checkVendorMenu); }
40-58: Consider applying similar maintainability improvements as enableSpmvModule.For consistency and maintainability, consider applying the same refactoring pattern suggested for
enableSpmvModule.tests/pw/tests/e2e/requestForQuotes.spec.ts (1)
Line range hint
1-190: Consider enhancing test robustnessThe test structure is well-organized, but consider these improvements:
- Add explicit error handling for API calls in
beforeAllhooks- Add retry logic for potentially flaky network-dependent tests
- Consider adding more granular assertions in the payment flow
Example implementation for retry logic:
test('customer can pay for order converted from quote request', async ({ page }) => { test.slow(); await test.step('setup payment gateways', async () => { const maxRetries = 3; for (const gateway of ['dokan-stripe-connect', 'dokan_paypal_marketplace']) { await retry(maxRetries, async () => { await apiUtils.updatePaymentGateway(gateway, { enabled: false }, payloads.adminAuth); }); } }); // ... rest of the test }); async function retry(maxRetries: number, fn: () => Promise<void>) { let lastError; for (let i = 0; i < maxRetries; i++) { try { await fn(); return; } catch (error) { lastError = error; await new Promise(resolve => setTimeout(resolve, 1000 * Math.pow(2, i))); } } throw lastError; }tests/pw/tests/e2e/sellerVacation.spec.ts (1)
20-24: Add error handling in afterAll hookThe cleanup in afterAll should handle potential failures gracefully to ensure proper test cleanup.
test.afterAll(async () => { - await apiUtils.activateModules(payloads.moduleIds.sellerVacation, payloads.adminAuth); - await aPage.close(); - await apiUtils.dispose(); + try { + await apiUtils.activateModules(payloads.moduleIds.sellerVacation, payloads.adminAuth); + } catch (error) { + console.error('Failed to activate module during cleanup:', error); + } finally { + await aPage.close(); + await apiUtils.dispose(); + } });tests/pw/pages/minMaxQuantitiesPage.ts (2)
10-12: Remove unnecessary constructorThe constructor doesn't add any functionality beyond the parent class.
- constructor(page: Page) { - super(page); - }🧰 Tools
🪛 Biome (1.9.4)
[error] 10-12: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
26-26: Fix comment typoThe comment has a redundant word "enable".
- // disable enable min max quantities module + // disable min max quantities moduletests/pw/tests/e2e/colors.spec.ts (2)
Line range hint
33-35: Document reason for skipped testThe test is skipped with a vague comment "need to fix". Please document the specific issue that needs to be fixed and create a tracking issue.
Line range hint
28-48: Consider test isolation and validationThe current test sequence creates implicit dependencies:
- Enable module
- Run color palette tests
- Disable module
Consider:
- Adding validation after module enable/disable operations
- Making each test independent by handling module state in beforeEach/afterEach
Example structure:
test.describe('Color scheme customizer test', () => { test.beforeEach(async () => { // Ensure module is enabled before each test await apiUtils.activateModules(payloads.moduleIds.colorSchemeCustomizer, payloads.adminAuth); }); test.afterEach(async () => { // Reset to known state after each test await dbUtils.setOptionValue(dbData.dokan.optionName.colors, dbData.dokan.colorsSettings); }); // Individual tests... });tests/pw/tests/e2e/shipstation.spec.ts (1)
10-11: Document skip reasonThe entire test suite is skipped with the comment "remove after pr is merged". Please:
- Document why the suite needs to be skipped
- Create a tracking issue for re-enabling
tests/pw/tests/e2e/productEnquiry.spec.ts (2)
37-44: Well-organized test structureGood job on organizing tests by user role with clear comments. This pattern should be applied to other test files for consistency.
Consider extracting this pattern into a shared test utility:
interface UserRole { name: string; tags: string[]; setup: () => Promise<void>; } function describeUserTests(role: UserRole, tests: () => void) { test.describe(`${role.name} tests`, () => { test.beforeEach(role.setup); tests(); }); }Also applies to: 54-59
Line range hint
1-59: Consider shared test utilities across filesAll three test files share similar patterns for:
- Module activation/deactivation
- Page object initialization
- Context management
- Environment setup
Consider creating a shared test utility:
class ModuleTestHelper { constructor( private moduleId: string, private PageClass: typeof BasePage ) {} async setup() { // Common setup logic } async teardown() { // Common teardown logic } async enableModule() { // Common enable logic } async disableModule() { // Common disable logic } }tests/pw/pages/liveSearchPage.ts (1)
25-34: Refactor duplicate code between enable/disable methodsThe enable and disable methods share similar structure but opposite assertions. Consider extracting common logic.
+private async verifyLiveSearchState(shouldBeVisible: boolean) { + // Navigate to Dokan settings and verify live search menu state + await this.goto(data.subUrls.backend.dokan.settings); + await this.page.waitForLoadState('networkidle'); + await (shouldBeVisible ? this.toBeVisible : this.notToBeVisible)( + selector.admin.dokan.settings.menus.liveSearch + ); + + // Navigate to My Account page and verify live search visibility + await this.goto(data.subUrls.frontend.myAccount); + await this.page.waitForLoadState('networkidle'); + await (shouldBeVisible ? this.toBeVisible : this.notToBeVisible)( + liveSearchCustomer.liveSearchDiv + ); +} -async disableLiveSearchModule() { - // dokan settings - await this.goto(data.subUrls.backend.dokan.settings); - await this.notToBeVisible(selector.admin.dokan.settings.menus.liveSearch); - - // my account - await this.goto(data.subUrls.frontend.myAccount); - await this.notToBeVisible(liveSearchCustomer.liveSearchDiv); +async enableLiveSearchModule() { + await this.verifyLiveSearchState(true); +} + +async disableLiveSearchModule() { + await this.verifyLiveSearchState(false); }tests/pw/pages/productEnquiryPage.ts (1)
29-39: Consider adding validation for product existenceThe disable method assumes the product exists. Add validation to prevent false negatives.
async disableProductEnquiryModule(productName: string) { + // Validate product exists before proceeding + const productExists = await this.validateProductExists(productName); + if (!productExists) { + throw new Error(`Product ${productName} not found`); + } + // dokan settings await this.goto(data.subUrls.backend.dokan.settings); await this.click(selector.admin.dokan.settings.menus.sellingOptions); await this.notToBeVisible(selector.admin.dokan.settings.selling.guestProductEnquiry); // single product page await this.goToProductDetails(productName); await this.notToBeVisible(selector.customer.cSingleProduct.menus.productEnquiry); }tests/pw/tests/e2e/vendorTools.spec.ts (2)
Line range hint
8-30: Improve test setup and teardown robustnessThe setup and teardown could be more reliable with better error handling and cleanup.
-let admin: VendorToolsPage; -let vendor: VendorToolsPage; -let aPage: Page, vPage: Page; -let apiUtils: ApiUtils; +let admin: VendorToolsPage | undefined; +let vendor: VendorToolsPage | undefined; +let aPage: Page | undefined, vPage: Page | undefined; +let apiUtils: ApiUtils | undefined; test.beforeAll(async ({ browser }) => { + try { const adminContext = await browser.newContext(data.auth.adminAuth); aPage = await adminContext.newPage(); admin = new VendorToolsPage(aPage); const vendorContext = await browser.newContext(data.auth.vendorAuth); vPage = await vendorContext.newPage(); vendor = new VendorToolsPage(vPage); apiUtils = new ApiUtils(await request.newContext()); await apiUtils.deleteAllProducts('p0_v1', payloads.vendorAuth); + } catch (error) { + console.error('Setup failed:', error); + throw error; + } }); test.afterAll(async () => { + try { await apiUtils.activateModules(payloads.moduleIds.vendorImportExport, payloads.adminAuth); - await vPage.close(); + await vPage?.close(); + await aPage?.close(); await apiUtils.dispose(); + } catch (error) { + console.error('Teardown failed:', error); + } });
Line range hint
34-65: Restructure tests for better organization and independenceThe test structure could be improved to better handle dependencies and grouping.
-test('admin can enable product importer and exporter module', { tag: ['@pro', '@admin'] }, async () => { +test.describe('Admin module management', () => { + test('can enable product importer and exporter module', { tag: ['@pro', '@admin'] }, async () => { + test.fail(!admin, 'Admin page object not initialized'); await admin.enableProductImporterExporterModule(); -}); + }); + test('can disable product importer and exporter module', { tag: ['@pro', '@admin'] }, async () => { + test.fail(!admin, 'Admin page object not initialized'); + await apiUtils.deactivateModules(payloads.moduleIds.vendorImportExport, payloads.adminAuth); + await admin.disableProductImporterExporterModule(); + }); +}); -//vendor +test.describe('Vendor tools', () => { + test.beforeEach(async () => { + test.fail(!vendor, 'Vendor page object not initialized'); + }); test('vendor can view tools menu page'...); test('vendor can export product as xml'...); // ... other vendor tests ... +}); -test('admin can disable product importer and exporter module'...);tests/pw/tests/e2e/printful.spec.ts (2)
7-10: Consider adding test suite descriptionThe test suite lacks a clear description of what functionality is being tested. Adding a description would improve test documentation and maintainability.
Add a description when declaring the test suite:
-test.describe('Printful test', () => { +test.describe('Printful integration - Admin and vendor functionality tests', () => {
40-45: Document reason for skipped testsTwo vendor-related tests are skipped without explanation. If these tests are temporarily skipped, add a TODO comment explaining why and when they should be re-enabled.
Add documentation for skipped tests:
-test.skip('vendor can connect printful account', { tag: ['@pro', '@vendor'] }, async () => { +test.skip('vendor can connect printful account', { tag: ['@pro', '@vendor'] }, async () => { + // TODO: Test skipped due to <reason>. Should be re-enabled when <condition> is met. await vendor.connectPrintful(); }); -test.skip('vendor can disconnect printful account', { tag: ['@pro', '@vendor'] }, async () => { +test.skip('vendor can disconnect printful account', { tag: ['@pro', '@vendor'] }, async () => { + // TODO: Test skipped due to <reason>. Should be re-enabled when <condition> is met. await vendor.disconnectPrintful(); });tests/pw/pages/shipStationPage.ts (1)
37-43: Document removed success message checkThe commented-out success message check needs proper documentation explaining why it was removed and if there's an alternative verification method.
Add better documentation for the removed check:
- // await this.toBeVisible(settingsShipStation.generateSuccessMessage); //todo: success message is removed + // Success message verification removed as the UI no longer shows this element + // Alternative verification is done by checking for: + // 1. Visible revoke credentials button + // 2. Visible credential fields + // 3. Visible secret key warningtests/pw/types/environment.d.ts (1)
39-50: Review security implications of payment credentialsEnsure that test credentials are properly handled and not exposed in logs or error messages. Consider adding a warning comment about credential handling.
Add security warning:
+ // WARNING: These are test/sandbox credentials for payment integrations. + // Never use production credentials in tests. + // Ensure these values are properly secured and not logged or exposed. TEST_PUBLISH_KEY_STRIPE: string;tests/pw/pages/vendorAnalyticsPage.ts (2)
16-25: Consider enhancing error handling and return valuesThe
enableVendorAnalyticsModulemethod should include error handling and return a success state for better test reliability.Consider this enhancement:
async enableVendorAnalyticsModule() { + try { // dokan settings await this.goto(data.subUrls.backend.dokan.settings); await this.toBeVisible(selector.admin.dokan.settings.menus.vendorAnalytics); // vendor dashboard menu await this.goto(data.subUrls.frontend.vDashboard.dashboard); await this.toBeVisible(selector.vendor.vDashboard.menus.primary.analytics); + return true; + } catch (error) { + console.error('Failed to enable vendor analytics module:', error); + throw error; + } }
27-40: Add validation for complete module deactivationThe
disableVendorAnalyticsModulemethod thoroughly checks multiple UI elements, but could benefit from error handling and explicit success confirmation.Consider this enhancement:
async disableVendorAnalyticsModule() { + try { // dokan settings await this.goto(data.subUrls.backend.dokan.settings); await this.notToBeVisible(selector.admin.dokan.settings.menus.vendorAnalytics); // vendor dashboard menu await this.goto(data.subUrls.frontend.vDashboard.dashboard); await this.notToBeVisible(selector.vendor.vDashboard.menus.primary.analytics); // vendor dashboard menu page await this.goto(data.subUrls.frontend.vDashboard.analytics); await this.notToBeVisible(selector.vendor.vDashboard.dashboardDiv); + return true; + } catch (error) { + console.error('Failed to disable vendor analytics module:', error); + throw error; + } }tests/pw/tests/e2e/_auth.setup.ts (1)
6-6: Remove or document commented codeThe commented-out code related to dbUtils and map location functionality should either be removed or documented with a clear explanation of why it's preserved.
If the map location functionality is planned for future implementation, consider:
- Adding a TODO comment with JIRA/GitHub issue reference
- Or removing the commented code entirely and tracking it in version control
-// import { dbUtils } from '@utils/dbUtils'; -// await dbUtils.addStoreMapLocation(sellerId); +// TODO: Map location functionality to be implemented in ISSUE-XXX +// Blocked by: <reason>Also applies to: 46-46, 65-65
tests/pw/tests/e2e/vendorDeliveryTime.spec.ts (2)
Line range hint
66-80: Skipped tests should include clear conditions for re-enablingThe customer tests are skipped with a vague comment "run when chart & checkout block pr is merged". This should include:
- The specific PR number that needs to be merged
- Clear conditions for re-enabling
Consider updating the skip message to be more specific:
- test.skip(true, 'run when chart & checkout block pr is merged'); + test.skip(true, 'Blocked by PR #XXXX: Pending chart & checkout block implementation');
31-36: Consider adding cleanup in afterAll hookThe afterAll hook activates the module but doesn't clean up the test data or reset settings.
Consider adding cleanup:
test.afterAll(async () => { await apiUtils.activateModules(payloads.moduleIds.deliveryTime, payloads.adminAuth); + await dbUtils.setOptionValue(dbData.dokan.optionName.deliveryTime, dbData.dokan.deliveryTimeSettings); await vPage.close(); await cPage.close(); + await apiUtils.dispose(); });tests/pw/tests/e2e/geolocation.spec.ts (2)
Line range hint
21-30: Consider consolidating database operationsThe test uses both beforeEach and afterAll hooks to set the same geolocation settings.
Consider consolidating the database operations:
test.beforeEach(async () => { - await dbUtils.setOptionValue(dbData.dokan.optionName.geolocation, dbData.dokan.geolocationSettings); + // Reset only the specific settings being tested + await dbUtils.resetTestSettings(); }); test.afterAll(async () => { - await dbUtils.setOptionValue(dbData.dokan.optionName.geolocation, dbData.dokan.geolocationSettings); await apiUtils.activateModules(payloads.moduleIds.geolocation, payloads.adminAuth); await aPage.close(); + await apiUtils.dispose(); });
Line range hint
38-75: Consider grouping related test casesThe test file has multiple parameterized tests that could be grouped for better organization.
Consider using test.describe for related tests:
+test.describe('Map position settings', () => { ['top', 'left', 'right'].forEach((position: string) => { test(`admin can set map position (${position})`, ...); }); +}); +test.describe('Map display settings', () => { ['all', 'store_listing', 'shop'].forEach((place: string) => { test(`admin can set map display page (${place})`, ...); }); +});tests/pw/pages/followStorePage.ts (2)
12-21: Consider adding error handling and assertionsThe
enableFollowStoreModulemethod performs navigation and visibility checks but could benefit from:
- Error handling for failed navigation attempts
- Assertions to verify successful module activation
Consider adding try-catch blocks and assertions:
async enableFollowStoreModule() { + try { // vendor dashboard menu await this.goto(data.subUrls.frontend.vDashboard.dashboard); await this.toBeVisible(selector.vendor.vDashboard.menus.primary.followers); // my account menu await this.goto(data.subUrls.frontend.myAccount); await this.toBeVisible(selector.customer.cMyAccount.menus.vendors); + + // Assert module is enabled + const isEnabled = await this.isVisible(selector.vendor.vDashboard.menus.primary.followers) + && await this.isVisible(selector.customer.cMyAccount.menus.vendors); + if (!isEnabled) { + throw new Error('Follow store module activation failed'); + } + } catch (error) { + throw new Error(`Failed to enable follow store module: ${error.message}`); + } }
23-40: Add timeout handling for page not found checksThe
disableFollowStoreModulemethod performs multiple navigation and visibility checks, but should handle timeouts for "page not found" scenarios.Consider adding timeout configuration:
async disableFollowStoreModule() { + const pageNotFoundTimeout = 5000; // 5 seconds try { // vendor dashboard menu await this.goto(data.subUrls.frontend.vDashboard.dashboard); await this.notToBeVisible(selector.vendor.vDashboard.menus.primary.followers); // vendor dashboard menu page await this.goto(data.subUrls.frontend.vDashboard.followers); - await this.toBeVisible(selector.frontend.pageNotFound); + await this.toBeVisible(selector.frontend.pageNotFound, { timeout: pageNotFoundTimeout }); // my account menu await this.goto(data.subUrls.frontend.myAccount); await this.notToBeVisible(selector.customer.cMyAccount.menus.vendors); // my account menu page await this.goto(data.subUrls.frontend.followingStores); - await this.toBeVisible(selector.frontend.pageNotFound); + await this.toBeVisible(selector.frontend.pageNotFound, { timeout: pageNotFoundTimeout }); + + // Assert module is disabled + const isDisabled = await this.notToBeVisible(selector.vendor.vDashboard.menus.primary.followers) + && await this.notToBeVisible(selector.customer.cMyAccount.menus.vendors); + if (!isDisabled) { + throw new Error('Follow store module deactivation failed'); + } + } catch (error) { + throw new Error(`Failed to disable follow store module: ${error.message}`); + } }tests/pw/tests/e2e/followStore.spec.ts (2)
Line range hint
11-38: Consider using test.step() for better test organizationThe test setup is good but could benefit from using test steps for better organization and reporting.
Consider restructuring the beforeAll hook:
test.beforeAll(async ({ browser }) => { + await test.step('Setup admin context', async () => { const adminContext = await browser.newContext(data.auth.adminAuth); aPage = await adminContext.newPage(); admin = new FollowStorePage(aPage); + }); + await test.step('Setup vendor context', async () => { const vendorContext = await browser.newContext(data.auth.vendorAuth); vPage = await vendorContext.newPage(); vendor = new FollowStorePage(vPage); + }); + await test.step('Setup customer context', async () => { const customerContext = await browser.newContext(data.auth.customerAuth); cPage = await customerContext.newPage(); customer = new FollowStorePage(cPage); + }); apiUtils = new ApiUtils(await request.newContext()); });
77-77: Address TODO comment about test parameterizationThe TODO comment indicates a need for test parameterization.
Would you like me to help create a parameterized version of these tests or create a GitHub issue to track this task?
tests/pw/tests/e2e/vendorProductSubscription.spec.ts (2)
39-44: Reorganize admin test cases for better readabilityThe admin test cases are currently split between the beginning and end of the test suite. Consider grouping them together for better organization and maintainability.
test.afterAll(async () => { await apiUtils.activateModules(payloads.moduleIds.productSubscription, payloads.adminAuth); await vPage.close(); await cPage.close(); await apiUtils.dispose(); }); -// admin +describe('admin', () => { test('admin can enable product subscription module', { tag: ['@pro', '@admin'] }, async () => { await admin.enableProductSubscriptionModule(); }); + + test('admin can disable product subscription module', { tag: ['@pro', '@admin'] }, async () => { + await apiUtils.deactivateModules(payloads.moduleIds.productSubscription, payloads.adminAuth); + await admin.disableProductSubscriptionModule(); + }); +}); //vendor // ... vendor tests ... // customer // ... customer tests ... -test('admin can disable product subscription module', { tag: ['@pro', '@admin'] }, async () => { - await apiUtils.deactivateModules(payloads.moduleIds.productSubscription, payloads.adminAuth); - await admin.disableProductSubscriptionModule(); -});Also applies to: 96-100
Line range hint
51-94: Address skipped tests to improve coverageThere are 11 skipped tests in the vendor and customer sections. This indicates significant gaps in test coverage for core functionality.
Would you like me to help implement these skipped tests? I can:
- Generate the test implementations
- Create a GitHub issue to track this work
Please let me know your preference.tests/pw/.env.example (1)
23-34: Add validation requirements for payment gateway credentialsThe payment gateway credentials should include validation requirements in the comments to prevent configuration errors.
-TEST_PUBLISH_KEY_STRIPE=test_publish_key_stripe [Stripe publish key] -TEST_SECRET_KEY_STRIPE=test_secret_key_stripe [Stripe secret key] +TEST_PUBLISH_KEY_STRIPE=test_publish_key_stripe [Stripe publish key - Must start with 'pk_test_'] +TEST_SECRET_KEY_STRIPE=test_secret_key_stripe [Stripe secret key - Must start with 'sk_test_']Also consider:
- Adding validation for other payment gateway credentials
- Including links to documentation for obtaining these credentials
Consider implementing environment variable validation in the test setup to catch configuration issues early.
tests/pw/tests/e2e/vendorReturnRequest.spec.ts (3)
43-46: Clean up commented code that's no longer neededThese commented lines appear to be old implementation approaches that are no longer used. Consider removing them to maintain code clarity.
- // [,, orderId, ] = await apiUtils.createOrderWithStatus(PRODUCT_ID, { ...payloads.createOrder, customer_id: CUSTOMER_ID }, data.order.orderStatus.processing, payloads.vendorAuth); - // [,, orderId, ] = await apiUtils.createOrderWithStatus(payloads.createProduct(), { ...payloads.createOrder, customer_id: CUSTOMER_ID }, data.order.orderStatus.processing, payloads.vendorAuth);
Line range hint
82-86: Improve test isolation by removing dependenciesThe test depends on previous test execution as indicated by the TODO comment. This creates brittle tests that are harder to maintain and debug.
Consider refactoring to make each test independent:
- Move the common setup to
beforeEach- Create separate RMA requests for each test case
113-116: Document reason for skipped testThe test for disabling the RMA module is skipped without explanation. Add a comment explaining why it's skipped and when it should be enabled.
- test.skip('admin can disable RMA module', { tag: ['@pro', '@admin'] }, async () => { + // TODO: Skipped because <reason>. Can be enabled when <condition> + test.skip('admin can disable RMA module', { tag: ['@pro', '@admin'] }, async () => {tests/pw/tests/e2e/sellerBadges.spec.ts (2)
27-27: Consider standardizing module state managementThe pattern of activating modules in
afterAllis consistent across test files. Consider creating a shared helper function to manage module states consistently.// utils/moduleHelper.ts export async function resetModuleState(apiUtils: ApiUtils, moduleId: string, auth: any) { await apiUtils.activateModules(moduleId, auth); }
Line range hint
67-75: Address skipped tests with background process dependenciesMultiple tests are skipped due to timing dependencies on background processes. This indicates a potential architectural issue in testing.
Consider:
- Implementing a test helper to manage background process timing
- Mocking the background process for faster tests
- Moving these to a separate integration test suite
tests/pw/tests/e2e/productAddons.spec.ts (3)
Line range hint
11-11: Add environment variable validationThe
VENDOR_IDenvironment variable is used without validation. This could lead to runtime errors if the variable is not set.-const { VENDOR_ID } = process.env; +const VENDOR_ID = process.env.VENDOR_ID; +if (!VENDOR_ID) { + throw new Error('VENDOR_ID environment variable is required'); +}
Line range hint
91-94: Optimize slow testsThe test is marked as slow without explanation. Consider documenting why it's slow and explore optimization opportunities.
- test.slow(); + // This test is slow due to product creation and addon attachment + // TODO: Consider optimizing by: + // 1. Reusing products across tests + // 2. Using batch operations for setup + test.slow();
49-54: Consider reorganizing module management testsModule enabling/disabling tests are split between the beginning and end of the test suite. Consider grouping them together for better organization and clarity.
Move all module management tests to the beginning of the suite, after the "admin" comment marker.
Also applies to: 113-118
tests/pw/tests/e2e/spmv.spec.ts (1)
41-41: Add condition check before activating moduleThe afterAll hook should verify if the module needs activation before attempting to activate it.
- await apiUtils.activateModules(payloads.moduleIds.spmv, payloads.adminAuth); + const moduleStatus = await apiUtils.getModuleStatus(payloads.moduleIds.spmv); + if (moduleStatus !== 'active') { + await apiUtils.activateModules(payloads.moduleIds.spmv, payloads.adminAuth); + }tests/pw/tests/e2e/productAdvertising.spec.ts (1)
26-26: Refactor duplicate afterAll cleanup logicThe same module activation logic appears in both test describes. Consider extracting this into a shared helper function.
+ // Add to test utils + async function ensureModuleActivation(apiUtils: ApiUtils, moduleId: string) { + const moduleStatus = await apiUtils.getModuleStatus(moduleId); + if (moduleStatus !== 'active') { + await apiUtils.activateModules(moduleId, payloads.adminAuth); + } + } // Use in both afterAll hooks - await apiUtils.activateModules(payloads.moduleIds.productAdvertising, payloads.adminAuth); + await ensureModuleActivation(apiUtils, payloads.moduleIds.productAdvertising);Also applies to: 92-92
tests/pw/pages/geolocationPage.ts (1)
17-17: Add JSDoc comments for public methodsAdd documentation for the public methods to improve code maintainability.
+ /** + * Enables the geolocation module and verifies its visibility across different pages + * @throws {Error} If navigation fails or module state verification fails + */ async enableGeolocationModule() {tests/pw/pages/settingPage.ts (1)
55-59: LGTM! Consider adding JSDoc documentation.The selector path updates are consistent and maintain the same functionality. Consider adding JSDoc documentation to describe the method's purpose and parameters.
/** * Sets the visibility of store terms and conditions. * @param status - 'on' to show terms and conditions, any other value to hide */ async setStoreTermsAndConditions(status: string)tests/pw/tests/e2e/vendorBooking.spec.ts (1)
49-51: LGTM! Consider adding error assertions.The module enable/disable test cases are well-structured and properly tagged. Consider adding assertions for error states to ensure robust testing.
// Example enhancement for error handling test('admin can enable woocommerce booking integration module', { tag: ['@pro', '@admin'] }, async () => { try { await admin.enableBookingModule(); } catch (error) { throw new Error(`Failed to enable booking module: ${error.message}`); } });Also applies to: 146-149
tests/pw/pages/abuseReportsPage.ts (2)
22-23: Standardize page load options.The
waitUntiloption is used inconsistently between enable/disable methods. Consider standardizing this across both methods.-await this.goto(data.subUrls.backend.dokan.dokan); +await this.goto(data.subUrls.backend.dokan.dokan, { waitUntil: 'domcontentloaded' }, true);Also applies to: 37-38
19-51: Consider enhancing code organization and error handling.The implementation is functional but could benefit from:
- Centralizing navigation paths into a separate method
- Adding error handling for failed assertions
- Adding JSDoc documentation
/** * Navigates to all required pages for module verification * @param productName - Name of the product to verify */ private async navigateToModulePages(productName: string) { await this.goto(data.subUrls.backend.dokan.dokan, { waitUntil: 'domcontentloaded' }, true); await this.goto(data.subUrls.backend.dokan.settings); await this.goto(data.subUrls.frontend.productDetails(helpers.slugify(productName))); } /** * Enables the report abuse module and verifies its visibility * @param productName - Name of the product to enable reporting for * @throws Error if any visibility check fails */ async enableReportAbuseModule(productName: string) { try { await this.navigateToModulePages(productName); await this.toBeVisible(selector.admin.dokan.menus.abuseReports); await this.toBeVisible(selector.admin.dokan.settings.menus.productReportAbuse); await this.toBeVisible(abuseReportCustomer.reportAbuse); } catch (error) { throw new Error(`Failed to enable report abuse module: ${error.message}`); } }tests/pw/tests/e2e/vendorSubscriptions.spec.ts (2)
11-11: Address the skip reason with more details.The skip reason "need to update create dokan subscription product" is vague. Consider adding more context about what specifically needs to be updated.
43-48: Consider adding cleanup in afterAll hook.The afterAll hook activates modules but doesn't clean up test data. Consider adding cleanup for:
- Created subscription products
- Assigned subscriptions
- Test stores
tests/pw/pages/productAddonsPage.ts (1)
13-16: Remove unnecessary constructor.The constructor doesn't add any functionality beyond the parent class constructor.
- constructor(page: Page) { - super(page); - }🧰 Tools
🪛 Biome (1.9.4)
[error] 13-15: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
tests/pw/tests/e2e/productQA.spec.ts (1)
46-48: Consider adding assertions for module stateWhile the test structure is good, consider adding assertions to verify the module's enabled state after calling
enableProductQaModule.test('admin can enable product Q&A module', { tag: ['@pro', '@admin'] }, async () => { await admin.enableProductQaModule(data.predefined.simpleProduct.product1.name); + // Add assertions to verify module is enabled + await expect(page.locator('selector-for-qa-module')).toBeVisible(); });tests/pw/pages/vendorProductSubscriptionPage.ts (1)
32-45: Consider adding error handling for navigation failuresWhile the method is well-structured, consider adding try-catch blocks for navigation failures and element visibility checks.
async disableProductSubscriptionModule() { + try { // vendor dashboard menu await this.goto(data.subUrls.frontend.vDashboard.dashboard); await this.notToBeVisible(selector.vendor.vDashboard.menus.primary.userSubscriptions); // vendor dashboard menu page await this.goto(data.subUrls.frontend.vDashboard.userSubscriptions); await this.notToBeVisible(selector.vendor.vDashboard.dashboardDiv); // my account menu await this.goto(data.subUrls.frontend.myAccount); await this.toBeVisible(selector.customer.cMyAccount.menus.subscriptions); + } catch (error) { + throw new Error(`Failed to disable subscription module: ${error.message}`); + } }tests/pw/tests/e2e/wholesale.spec.ts (2)
45-47: Consider adding test data cleanupWhile the test structure is good, consider adding cleanup for any test data created during the enable process.
test('admin can enable wholesale module', { tag: ['@pro', '@admin'] }, async () => { await admin.enableWholesaleModule(); + // Add cleanup if test creates any data + await apiUtils.cleanupTestData(); });
189-192: Verify module state before disablingConsider adding a check to verify the module's current state before attempting to disable it.
test('admin can disable wholesale module', { tag: ['@pro', '@admin'] }, async () => { + // Verify module is enabled before attempting to disable + await expect(page.locator('selector-for-wholesale-module')).toBeVisible(); await apiUtils.deactivateModules(payloads.moduleIds.wholesale, payloads.adminAuth); await admin.disableWholesaleModule(); });tests/pw/tests/e2e/euCompliance.spec.ts (2)
169-175: Consider grouping related module activation tests togetherThe disable test is currently separated from its corresponding enable test. Consider moving this test to follow line 57 to keep related functionality together, improving test readability and maintenance.
169-169: Document reasons for skipped product EU compliance testsThe TODO comment indicates more tests are needed, but there's no explanation for why the existing product EU compliance tests are skipped. Consider adding comments explaining the reason for skipping these tests and create tracking issues.
Would you like me to help create GitHub issues to track these TODO items?
tests/pw/pages/vendorDeliveryTimePage.ts (1)
36-56: LGTM: Thorough module deactivation verificationThe method provides comprehensive verification of the disabled state. Consider extracting the common URLs and selectors to class-level constants to reduce duplication and improve maintainability.
tests/pw/pages/vendorSubscriptionsPage.ts (1)
37-49: Consider creating a base class for module activation patternsThe enable/disable methods across different modules (EU Compliance, Delivery Time, Vendor Subscriptions) follow similar patterns. Consider creating a base class that implements these common patterns to reduce code duplication and improve maintainability.
Example structure:
abstract class BaseModulePage { protected abstract get moduleSettingsSelector(): string; protected abstract get moduleDashboardSelector(): string; async enableModule() { await this.goto(data.subUrls.backend.dokan.settings); await this.toBeVisible(this.moduleSettingsSelector); await this.goto(data.subUrls.frontend.vDashboard.dashboard); await this.toBeVisible(this.moduleDashboardSelector); } async disableModule() { // Common disable logic } }tests/pw/pages/productAdvertisingPage.ts (1)
27-41: Add error handling for navigation failuresWhile the implementation is clean and well-structured, consider adding try-catch blocks to handle potential navigation failures and provide meaningful error messages.
async enableProductAdvertisingModule() { + try { // dokan menu await this.goto(data.subUrls.backend.dokan.dokan); await this.toBeVisible(selector.admin.dokan.menus.advertising); // dokan settings await this.goto(data.subUrls.backend.dokan.settings); await this.toBeVisible(selector.admin.dokan.settings.menus.productAdvertising); // vendor dashboard await this.goto(data.subUrls.frontend.vDashboard.products); await this.toBeVisible(productsVendor.table.productAdvertisementColumn); await this.clickAndWaitForLoadState(productsVendor.addNewProduct); await this.toBeVisible(productsVendor.advertisement.advertisementSection); + } catch (error) { + throw new Error(`Failed to enable product advertising module: ${error.message}`); + } }tests/pw/README.MD (1)
145-156: Enhance payment integration variable descriptionsWhile the documentation is clear, consider adding more details about:
- Required formats for API keys
- Links to relevant documentation for each payment provider
- Example values (with masked sensitive data)
tests/pw/pages/euCompliancePage.ts (2)
27-39: Maintain consistency with other module management methodsFor consistency with the product advertising module, consider:
- Adding error handling
- Using the common navigation pattern
- Adding JSDoc comments to document the method's purpose
+/** + * Enables EU compliance fields module and verifies visibility across different sections + * @throws {Error} If navigation fails or elements are not visible + */ async enableEuComplianceFieldsModule() { + try { // dokan settings await this.goto(data.subUrls.backend.dokan.settings); await this.toBeVisible(selector.admin.dokan.settings.menus.euComplianceFields); // vendor dashboard settings await this.goto(data.subUrls.frontend.vDashboard.settingsStore); await this.multipleElementVisible(selector.vendor.vStoreSettings.euFields); // my account await this.goto(data.subUrls.frontend.billingAddress); await this.multipleElementVisible(customerAddress.billing.euFields); + } catch (error) { + throw new Error(`Failed to enable EU compliance fields module: ${error.message}`); + } }
129-132: Add type safety for EU compliance dataConsider adding runtime type checks for the euData parameter to ensure all required fields are present before attempting to use them.
+private validateEuData(euData: eUComplianceData): void { + const requiredFields = ['companyId', 'vatNumber', 'bankName', 'bankIban']; + const missingFields = requiredFields.filter(field => !euData[field]); + if (missingFields.length > 0) { + throw new Error(`Missing required EU compliance fields: ${missingFields.join(', ')}`); + } +} async addUserEuCompliance(userId: string, euData: eUComplianceData) { + this.validateEuData(euData); // ... rest of the method } async customerAddEuComplianceData(euData: eUComplianceData): Promise<void> { + this.validateEuData(euData); // ... rest of the method }Also applies to: 139-142, 180-183, 188-191
tests/pw/pages/vendorReturnRequestPage.ts (1)
34-55: Consider adding error handling for navigation failuresThe
disableRmaModulemethod should handle potential navigation failures when checking visibility.async disableRmaModule() { + try { // dokan settings await this.goto(data.subUrls.backend.dokan.settings); await this.notToBeVisible(selector.admin.dokan.settings.menus.rma); + } catch (error) { + throw new Error(`Failed to verify RMA module disable state: ${error.message}`); + }tests/pw/utils/dbUtils.ts (1)
228-233: Consider parameterizing location dataThe
addStoreMapLocationmethod has hardcoded values for New York. Consider making these parameters to support different locations in tests.-async addStoreMapLocation(sellerId: string) { +async addStoreMapLocation( + sellerId: string, + location = { + latitude: '40.7127753', + longitude: '-74.0059728', + address: 'New York, NY, USA' + } +) { await dbUtils.setUserMeta(sellerId, 'dokan_geo_latitude', '40.7127753', false); await dbUtils.setUserMeta(sellerId, 'dokan_geo_longitude', '-74.0059728', false); await dbUtils.setUserMeta(sellerId, 'dokan_geo_public', '1', false); - await dbUtils.setUserMeta(sellerId, 'dokan_geo_address', 'New York, NY, USA', false); + await dbUtils.setUserMeta(sellerId, 'dokan_geo_address', location.address, false); }tests/pw/tests/e2e/storeSupports.spec.ts (1)
247-250: Consider adding cleanup verificationThe module disable test should verify that all related UI elements are properly cleaned up.
test('admin can disable store support module', { tag: ['@pro', '@admin'] }, async () => { await apiUtils.deactivateModules(payloads.moduleIds.storeSupport, payloads.adminAuth); await admin.disableStoreSupportModule(data.predefined.vendorStores.vendor1); + // Verify cleanup + await admin.verifyModuleCleanup(); });tests/pw/pages/wholesalePage.ts (2)
25-43: LGTM! Consider adding error handling.The method follows a clear structure for enabling and verifying the wholesale module across different pages. Each step is well-documented and follows async/await patterns correctly.
Consider adding try-catch blocks for better error handling and debugging:
async enableWholesaleModule() { + try { // dokan menu await this.goto(data.subUrls.backend.dokan.dokan); await this.toBeVisible(selector.admin.dokan.menus.wholesaleCustomer); // dokan settings await this.goto(data.subUrls.backend.dokan.settings); await this.toBeVisible(selector.admin.dokan.settings.menus.wholesale); // vendor dashboard await this.goIfNotThere(data.subUrls.frontend.vDashboard.products); await this.clickAndWaitForLoadState(productsVendor.addNewProduct); await this.toBeVisible(productsVendor.wholesale.wholesaleSection); // customer dashboard menu await this.goto(data.subUrls.frontend.myAccount); await this.toBeVisible(selector.customer.cDashboard.becomeWholesaleCustomer); + } catch (error) { + throw new Error(`Failed to enable wholesale module: ${error.message}`); + } }
45-67: LGTM! Consider adding error handling.The method effectively verifies the wholesale module is disabled across different pages. The verification steps are comprehensive and well-structured.
Consider adding try-catch blocks for better error handling and debugging:
async disableWholesaleModule() { + try { // dokan menu await this.goto(data.subUrls.backend.dokan.dokan); await this.notToBeVisible(selector.admin.dokan.menus.wholesaleCustomer); // dokan menu page await this.goto(data.subUrls.backend.dokan.wholeSaleCustomer); await this.notToBeVisible(wholesaleAdmin.wholesaleCustomerDiv); // dokan settings await this.goto(data.subUrls.backend.dokan.settings); await this.notToBeVisible(selector.admin.dokan.settings.menus.wholesale); // vendor dashboard await this.goIfNotThere(data.subUrls.frontend.vDashboard.products); await this.clickAndWaitForLoadState(productsVendor.addNewProduct); await this.notToBeVisible(productsVendor.wholesale.wholesaleSection); // customer dashboard menu await this.goto(data.subUrls.frontend.myAccount); await this.notToBeVisible(selector.customer.cDashboard.becomeWholesaleCustomer); + } catch (error) { + throw new Error(`Failed to disable wholesale module: ${error.message}`); + } }tests/pw/tests/e2e/productsDetailsBookings.spec.ts (1)
55-55: Consider implementing the skipped test.The test is skipped with a TODO comment about needing min & max duration.
Would you like help implementing the min & max duration functionality for accommodation booking options?
Also applies to: 56-56
tests/pw/utils/schemas.ts (1)
791-792: LGTM! Consider adding JSDoc comments.The type flexibility added to
tax_totalsandrestock_itemsfields is a good improvement to handle API responses that may return numbers as strings. Consider adding JSDoc comments to document this behavior./** * Tax totals can be returned as either strings or numbers from the API */ tax_totals: z.record(z.record(z.string().or(z.number()))), /** * Restock items quantity can be returned as either strings or numbers from the API */ restock_items: z.string().or(z.number()),tests/pw/utils/apiUtils.ts (1)
2370-2376: LGTM! Consider extracting common addon logic.The implementation correctly handles bookable products with addons. Consider extracting the common addon update logic into a private helper method to avoid duplication with
createProductWithAddon.private async updateProductWithAddons(productId: string, addonPayload: object[], auth?: auth): Promise<[responseBody, string, string[]]> { const responseBody = await this.updateProduct(productId, { meta_data: [{ key: '_product_addons', value: addonPayload }] }, auth); const productName = String(responseBody?.name); const addonNames = addonPayload.map((item: any) => item.name); return [responseBody, productName, addonNames]; } async createBookingProductWithAddon(productPayload: string | object, addonPayload: object[], auth?: auth): Promise<[responseBody, string, string, string[]]> { const productId = typeof productPayload === 'object' ? (await this.createBookableProduct(productPayload, auth))[1] : productPayload; const [responseBody, productName, addonNames] = await this.updateProductWithAddons(productId, addonPayload, auth); return [responseBody, productId, productName, addonNames]; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/pw/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (82)
tests/pw/.env.example(1 hunks)tests/pw/README.MD(1 hunks)tests/pw/feature-map/feature-map.yml(33 hunks)tests/pw/package.json(1 hunks)tests/pw/pages/abuseReportsPage.ts(1 hunks)tests/pw/pages/catalogModePage.ts(1 hunks)tests/pw/pages/colorsPage.ts(1 hunks)tests/pw/pages/customerPage.ts(1 hunks)tests/pw/pages/euCompliancePage.ts(3 hunks)tests/pw/pages/followStorePage.ts(1 hunks)tests/pw/pages/geolocationPage.ts(1 hunks)tests/pw/pages/liveChatPage.ts(1 hunks)tests/pw/pages/liveSearchPage.ts(1 hunks)tests/pw/pages/minMaxQuantitiesPage.ts(1 hunks)tests/pw/pages/paymentsPage.ts(6 hunks)tests/pw/pages/printfulPage.ts(1 hunks)tests/pw/pages/productAddonsPage.ts(1 hunks)tests/pw/pages/productAdvertisingPage.ts(2 hunks)tests/pw/pages/productEnquiryPage.ts(1 hunks)tests/pw/pages/productQAPage.ts(4 hunks)tests/pw/pages/selectors.ts(38 hunks)tests/pw/pages/sellerBadgesPage.ts(1 hunks)tests/pw/pages/sellerVacationPage.ts(1 hunks)tests/pw/pages/settingPage.ts(1 hunks)tests/pw/pages/shipStationPage.ts(1 hunks)tests/pw/pages/spmvPage.ts(2 hunks)tests/pw/pages/storeReviewsPage.ts(1 hunks)tests/pw/pages/storeSupportsPage.ts(1 hunks)tests/pw/pages/vendorAnalyticsPage.ts(1 hunks)tests/pw/pages/vendorAuctionsPage.ts(1 hunks)tests/pw/pages/vendorBookingPage.ts(2 hunks)tests/pw/pages/vendorDeliveryTimePage.ts(1 hunks)tests/pw/pages/vendorProductSubscriptionPage.ts(1 hunks)tests/pw/pages/vendorReturnRequestPage.ts(2 hunks)tests/pw/pages/vendorSettingsPage.ts(6 hunks)tests/pw/pages/vendorStaffPage.ts(1 hunks)tests/pw/pages/vendorSubscriptionsPage.ts(1 hunks)tests/pw/pages/vendorToolsPage.ts(1 hunks)tests/pw/pages/vendorVerificationsPage.ts(1 hunks)tests/pw/pages/wholesalePage.ts(2 hunks)tests/pw/tests/api/_env.setup.ts(1 hunks)tests/pw/tests/e2e/_auth.setup.ts(3 hunks)tests/pw/tests/e2e/_env.setup.ts(2 hunks)tests/pw/tests/e2e/abuseReports.spec.ts(3 hunks)tests/pw/tests/e2e/colors.spec.ts(2 hunks)tests/pw/tests/e2e/euCompliance.spec.ts(3 hunks)tests/pw/tests/e2e/followStore.spec.ts(4 hunks)tests/pw/tests/e2e/geolocation.spec.ts(3 hunks)tests/pw/tests/e2e/liveChat.spec.ts(3 hunks)tests/pw/tests/e2e/liveSearch.spec.ts(2 hunks)tests/pw/tests/e2e/minMaxQuantities.spec.ts(1 hunks)tests/pw/tests/e2e/payments.spec.ts(3 hunks)tests/pw/tests/e2e/printful.spec.ts(2 hunks)tests/pw/tests/e2e/productAddons.spec.ts(2 hunks)tests/pw/tests/e2e/productAdvertising.spec.ts(4 hunks)tests/pw/tests/e2e/productEnquiry.spec.ts(3 hunks)tests/pw/tests/e2e/productQA.spec.ts(3 hunks)tests/pw/tests/e2e/productsDetailsBookings.spec.ts(5 hunks)tests/pw/tests/e2e/requestForQuotes.spec.ts(1 hunks)tests/pw/tests/e2e/sellerBadges.spec.ts(2 hunks)tests/pw/tests/e2e/sellerVacation.spec.ts(1 hunks)tests/pw/tests/e2e/shipstation.spec.ts(1 hunks)tests/pw/tests/e2e/spmv.spec.ts(2 hunks)tests/pw/tests/e2e/storeSupports.spec.ts(4 hunks)tests/pw/tests/e2e/vendorAnalytics.spec.ts(1 hunks)tests/pw/tests/e2e/vendorAuction.spec.ts(3 hunks)tests/pw/tests/e2e/vendorBooking.spec.ts(3 hunks)tests/pw/tests/e2e/vendorDeliveryTime.spec.ts(3 hunks)tests/pw/tests/e2e/vendorProductSubscription.spec.ts(3 hunks)tests/pw/tests/e2e/vendorReturnRequest.spec.ts(4 hunks)tests/pw/tests/e2e/vendorStaff.spec.ts(4 hunks)tests/pw/tests/e2e/vendorSubscriptions.spec.ts(3 hunks)tests/pw/tests/e2e/vendorTools.spec.ts(3 hunks)tests/pw/tests/e2e/vendorVerifications.spec.ts(3 hunks)tests/pw/tests/e2e/wholesale.spec.ts(3 hunks)tests/pw/types/environment.d.ts(1 hunks)tests/pw/utils/apiUtils.ts(1 hunks)tests/pw/utils/dbData.ts(20 hunks)tests/pw/utils/dbUtils.ts(1 hunks)tests/pw/utils/payloads.ts(4 hunks)tests/pw/utils/schemas.ts(2 hunks)tests/pw/utils/testData.ts(8 hunks)
👮 Files not reviewed due to content moderation or server errors (3)
- tests/pw/pages/vendorBookingPage.ts
- tests/pw/utils/dbData.ts
- tests/pw/feature-map/feature-map.yml
🧰 Additional context used
🪛 Biome (1.9.4)
tests/pw/pages/minMaxQuantitiesPage.ts
[error] 10-12: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
tests/pw/pages/sellerVacationPage.ts
[error] 10-12: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
tests/pw/pages/productAddonsPage.ts
[error] 13-15: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
🔇 Additional comments (78)
tests/pw/tests/api/_env.setup.ts (1)
230-230: Verify consistency of the renamed option across the codebase
The change from productReportAbuse to reportAbuse looks good, but let's ensure consistency across the codebase.
Let's verify the option name change is consistent:
tests/pw/pages/sellerVacationPage.ts (1)
1-8: LGTM! Clean import organization and selector definition.
The imports are well-structured and the selector constant provides good maintainability.
tests/pw/pages/selectors.ts (18)
206-206: LGTM: Added new selector for reverse withdrawal menu
The selector for the reverse withdrawal menu item looks correct and follows the existing pattern.
209-210: LGTM: Added new selectors for RFQ and Seller Badge menus
The selectors for RFQ and Seller Badge menu items are properly defined and consistent with other menu selectors.
212-212: LGTM: Updated menu selectors
The updates to store support, product Q&A, and subscriptions menu selectors maintain consistency with the existing pattern.
Also applies to: 219-219, 221-221
867-867: LGTM: Added store support div selector
The selector for the store support div container follows the standard naming convention.
1127-1127: LGTM: Added seller badge div selector
The selector for the seller badge div container is properly defined.
1631-1632: LGTM: Updated product Q&A selectors
The selectors for product Q&A div and text are correctly defined and follow the existing pattern.
3471-3476: LGTM: Added EU compliance fields selectors
The selectors for EU compliance fields (company ID, VAT number, bank details) are properly structured.
Line range hint 6161-6182: LGTM: Added EU compliance fields selectors for store settings
The selectors for EU compliance fields in store settings are well organized and follow the existing pattern.
6177-6182: LGTM: Added terms and conditions selectors
The selectors for terms and conditions section are properly defined with clear naming.
6183-6204: LGTM: Added store opening/closing time selectors
The selectors for store opening/closing time functionality are comprehensive and well structured.
6207-6224: LGTM: Added vacation mode selectors
The selectors for vacation mode settings are properly organized and follow a consistent pattern.
6227-6231: LGTM: Added catalog mode selectors
The selectors for catalog mode settings are correctly defined.
6234-6238: LGTM: Added discount settings selectors
The selectors for discount settings are properly structured.
6241-6244: LGTM: Added biography selectors
The selectors for biography section are correctly defined.
6247-6251: LGTM: Added store support selectors
The selectors for store support settings are properly organized.
Line range hint 6258-6262: LGTM: Added min-max settings selectors
The selectors for min-max settings are correctly defined and follow the existing pattern.
Line range hint 6271-6679: LGTM: Added addon settings selectors
The selectors for addon settings are comprehensive and well organized, following a consistent pattern.
Line range hint 8053-8057: LGTM: Added live search selectors
The selectors for live search functionality are properly defined and follow the existing pattern.
tests/pw/pages/printfulPage.ts (1)
14-40: Consider aligning with existing test patterns.
The new methods could benefit from following the established patterns in other test methods:
- Using
clickAndWaitForResponsefor state changes - Adding success verification similar to
disconnectPrintful's success message check - Following the
goIfNotTherepattern instead of directgotocalls
Let's verify the existing patterns in the codebase:
tests/pw/package.json (5)
Line range hint 1-73: LGTM! Package updates look appropriate
The dependency updates are well-structured and maintain compatibility with the project's testing framework. The changes support the PR objective of adding module tests by updating the testing infrastructure.
53-53: Verify TypeScript compatibility with Playwright
The TypeScript update to 5.7.2 introduces new features. Please ensure compatibility with Playwright 1.49.
Also applies to: 63-63
✅ Verification successful
Let me gather more information about the Playwright configuration and TypeScript usage.
TypeScript and Playwright versions are compatible
The verification shows that:
- Playwright version 1.49 is compatible with TypeScript 5.7.2
- The project has proper TypeScript configuration in
tests/pw/tsconfig.jsontargeting modern environments - No type-related issues are present in the configuration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any TypeScript configuration files and their settings
fd -e tsconfig.json -x cat {} \;
# Look for any TypeScript-specific configurations in Playwright config
rg -A 5 "typescript" . --type ts
Length of output: 1869
Script:
#!/bin/bash
# Check Playwright config for TypeScript settings
fd playwright.config.ts -x cat {} \;
# Check package.json for Playwright version and TypeScript-related dependencies
cat tests/pw/package.json | jq '.dependencies["@playwright/test"], .devDependencies["typescript"], .devDependencies["@types/node"]'
# Look for any type-related issues or TypeScript configurations
rg "compilerOptions|tsconfig" -A 5
Length of output: 6167
54-56: Review ESLint major version upgrade
The upgrade from ESLint 8.x to 9.x is a major version change that may introduce breaking changes. Please ensure:
- The ESLint configuration is compatible with v9
- All ESLint plugins are compatible with ESLint v9
Also applies to: 58-58
60-61: Verify Prettier configuration for version 3.4
The Prettier update might introduce formatting changes. Please ensure the configuration is compatible and consistent with the project's coding standards.
✅ Verification successful
Prettier configuration is properly set up and maintained
The project has well-defined Prettier configurations:
- A base configuration extending
@wordpress/prettier-configwith custom overrides - A comprehensive
.prettierignorefile - Proper npm scripts for checking (
prettier . --check) and fixing (prettier . --write) formatting
The update to Prettier 3.4.2 is safe to proceed with as the configuration is properly maintained and compatible with the new version.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for Prettier configuration
fd -e .prettierrc -e .prettierrc.js -e .prettierrc.json -x cat {} \;
# Look for any Prettier configurations in package.json
rg '"prettier"' package.json
Length of output: 104
Script:
#!/bin/bash
# Search for Prettier configuration files with any extension
fd -t f -g '.prettier*' -x cat {} \;
# Look for Prettier configurations in all package.json files
fd -t f -g 'package.json' -x cat {} \; | grep -A 5 -B 5 '"prettier":'
# Check for Prettier-related scripts in package.json files
fd -t f -g 'package.json' -x cat {} \; | grep -A 2 '"scripts"' | grep -i 'prettier'
# Look for Prettier ignore files
fd -t f -g '.prettierignore' -x cat {} \;
Length of output: 1416
Script:
#!/bin/bash
# Search for Prettier configuration files with hidden flag
fd -H -t f -g '.prettier*' -x cat {} \;
# Look for Prettier ignore files with hidden flag
fd -H -t f -g '.prettierignore' -x cat {} \;
# Check for any recent Prettier-related changes in git history
git log -n 5 --oneline --grep="prettier"
Length of output: 1117
66-70: Verify Playwright 1.49 compatibility
The update to Playwright 1.49 and related dependencies requires verification:
- Check if existing tests are compatible with Playwright 1.49
- Ensure WordPress environment setup works with the updated @wordpress/env
- Review the changelog for any breaking changes
✅ Verification successful
Let me analyze the Playwright configuration files to understand the test setup better.
Let me check the Playwright configuration files one more time specifically.
Based on the analysis of the configuration files and test structure, I can now provide the final response:
Playwright 1.49 update is compatible with the existing test setup
The codebase's test configuration is well-structured and doesn't use any deprecated features that would be affected by the Playwright 1.49 update. Key observations:
- Standard configuration patterns are used for test setup, teardown, and project organization
- No usage of deprecated APIs in the configuration files
- Test timeouts, retries, and reporter configurations follow recommended practices
- The test architecture uses supported fixtures and page object patterns
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Look for Playwright configuration files
fd -e playwright.config.ts -e playwright.config.js -x cat {} \;
# Check for custom test utilities or helpers that might need updates
rg -t ts "import.*@playwright/test"
Length of output: 20102
Script:
#!/bin/bash
# Check Playwright configuration files content
fd -e playwright.config.ts -e api.config.ts -e e2e.config.ts -x cat {} \;
# Look for any custom test setup or teardown
rg -t ts "^export const test = base" -A 5
Length of output: 793
Script:
#!/bin/bash
# Get content of main Playwright config files
cat tests/pw/playwright.config.ts
cat tests/pw/api.config.ts
cat tests/pw/e2e.config.ts
Length of output: 16727
tests/pw/pages/spmvPage.ts (1)
11-11: LGTM!
The constant follows the established pattern and naming convention for selector constants.
tests/pw/tests/e2e/requestForQuotes.spec.ts (1)
Line range hint 142-144: Investigate skipped test case
The customer can update quote request test is currently skipped without any explanation in the code or commit message.
Let's check for any related issues or recent changes:
Would you like me to:
- Review the test implementation and identify potential issues?
- Create a new issue to track this skipped test?
tests/pw/pages/vendorSettingsPage.ts (1)
312-318: Update reflects correct handling of EU compliance information
The method setEuComplianceInfo correctly updates EU compliance fields. The changes from setCompanyInfo to setEuComplianceInfo are appropriate and improve clarity regarding the purpose of the method.
tests/pw/tests/e2e/shipstation.spec.ts (1)
7-8:
Validate environment variables and handle test re-run issues
- The
VENDOR_IDenvironment variable should be validated at startup - The TODOs about test re-run issues should be addressed before merging
Add environment validation:
const VENDOR_ID = process.env.VENDOR_ID;
if (!VENDOR_ID) {
throw new Error('VENDOR_ID environment variable is required');
}Also applies to: 42-42, 47-47
tests/pw/tests/e2e/printful.spec.ts (1)
23-27: Verify cleanup in afterAll hook
The afterAll hook reactivates the Printful module but doesn't clean up other potential test side effects. Consider adding complete cleanup.
tests/pw/types/environment.d.ts (1)
39-50: Group and document payment integration environment variables
The new payment integration variables should be grouped together and documented for better maintainability. Consider adding JSDoc comments and organizing related variables.
Improve organization and documentation:
+ // Stripe Integration Configuration
+ /** Stripe Publishable Key for test environment */
TEST_PUBLISH_KEY_STRIPE: string;
+ /** Stripe Secret Key for test environment */
TEST_SECRET_KEY_STRIPE: string;
+ /** Stripe Client ID for test environment */
TEST_CLIENT_ID_STRIPE: string;
+
+ // Mangopay Integration Configuration
+ /** Mangopay Client ID for sandbox environment */
SANDBOX_CLIENT_ID_MANGOPAY: string;
+ /** Mangopay API Key for sandbox environment */
SANDBOX_API_KEY_MANGOPAY: string;Also, consider adding validation patterns for these keys to ensure they match the expected format:
✅ Verification successful
Let me check the validation patterns in the identified file.
Group and document payment integration environment variables - Suggestion Verified
The environment variables are already being used in a structured way in payloads.ts with proper ID mappings and commented documentation (though currently disabled). The suggestion to group and document these variables in environment.d.ts aligns well with the existing usage pattern and would improve maintainability.
- The variables are used in configuration objects with clear IDs
- Each variable has associated (though currently commented out) labels and descriptions
- The variables are logically grouped by payment provider in the implementation
The original suggestion for adding JSDoc comments and grouping related variables remains valid and would enhance code documentation and organization.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if any test files validate these environment variables
rg -l "process\.env\.(TEST_PUBLISH_KEY_STRIPE|TEST_SECRET_KEY_STRIPE|TEST_CLIENT_ID_STRIPE|SANDBOX_CLIENT_ID_MANGOPAY|SANDBOX_API_KEY_MANGOPAY)"
Length of output: 173
Script:
#!/bin/bash
# Check the content of payloads.ts to see how these environment variables are used
rg -A 5 "process\.env\.(TEST_PUBLISH_KEY_STRIPE|TEST_SECRET_KEY_STRIPE|TEST_CLIENT_ID_STRIPE|SANDBOX_CLIENT_ID_MANGOPAY|SANDBOX_API_KEY_MANGOPAY)" tests/pw/utils/payloads.ts
Length of output: 2190
tests/pw/tests/e2e/_auth.setup.ts (1)
59-59: LGTM! Clean usage of spread operator
The spread operator usage for combining store fields and open-close time settings improves code readability and maintainability.
tests/pw/tests/e2e/vendorDeliveryTime.spec.ts (1)
40-42: 🛠️ Refactor suggestion
Verify module state changes through API response
The enable/disable module tests don't verify if the API calls were successful.
Consider verifying the API responses:
test('admin can enable delivery time module', { tag: ['@pro', '@admin'] }, async () => {
+ const response = await apiUtils.activateModules(payloads.moduleIds.deliveryTime, payloads.adminAuth);
+ expect(response.status).toBe(200);
await admin.enableDeliveryTimeModule();
});Also applies to: 83-86
tests/pw/pages/catalogModePage.ts (1)
41-41: LGTM! Selector path update improves maintainability
The change to use settingsVendor.catalogMode.hideProductPrice provides better organization and maintainability by properly namespacing the selector.
tests/pw/tests/e2e/liveChat.spec.ts (1)
34-35:
Add verification after DB operations
The test suite performs DB operations but doesn't verify their success. Consider adding checks to ensure the operations completed successfully.
Add verification after DB operations:
await dbUtils.updateUserMeta(VENDOR_ID, 'dokan_profile_settings', { live_chat: 'yes' });
+ const updatedSettings = await dbUtils.getUserMeta(VENDOR_ID, 'dokan_profile_settings');
+ if (updatedSettings.live_chat !== 'yes') {
+ throw new Error('Failed to update live chat settings');
+ }Also applies to: 40-44
tests/pw/tests/e2e/vendorReturnRequest.spec.ts (1)
50-50: Verify module activation strategy
Activating modules in afterAll might affect subsequent test runs. Consider if this should be moved to beforeAll or if there's a specific reason for activation during cleanup.
✅ Verification successful
Based on the search results, I can now provide a definitive response about the module activation strategy. The codebase search reveals that:
- Module activation in
afterAllis consistently used across test files to restore the module state after tests - The tests that deactivate modules have corresponding
afterAllhooks to reactivate them - Module activation is also properly handled in
beforeAllwhere needed for test setup - There's a global setup that activates all modules initially (
_site.setup.ts)
Module activation in afterAll is the correct cleanup strategy
The current implementation follows a proper test pattern where:
- Tests that disable modules need to restore the module state after completion
- The
afterAllactivation ensures subsequent test suites start with expected module state - This prevents test interdependencies and maintains isolation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if other test files follow the same pattern of module activation in afterAll
rg -l "activateModules.*afterAll" tests/pw/tests/e2e/
Length of output: 55
Script:
#!/bin/bash
# Let's check for module activation patterns in test files more broadly
rg "activateModules" tests/pw/tests/e2e/ -B2 -A2
# Also check for beforeAll usage patterns
rg "beforeAll" tests/pw/tests/e2e/ -B2 -A2
Length of output: 65947
tests/pw/tests/e2e/vendorBooking.spec.ts (1)
40-40: LGTM! Good cleanup practice.
Ensuring modules are reactivated after tests is a good practice for maintaining a consistent test environment.
tests/pw/tests/e2e/vendorSubscriptions.spec.ts (3)
55-59: LGTM! Well-structured module activation tests.
The tests properly verify both UI and database states for enabling and disabling the vendor subscription module. Good use of tags for categorization.
Also applies to: 134-137
Line range hint 60-90: LGTM! Comprehensive admin test coverage.
The admin test cases provide good coverage of subscription management features including:
- Viewing subscriptions
- Filtering vendors
- Cancellation flows
- Bulk actions
Line range hint 91-130: LGTM! Well-structured vendor test cases.
The vendor test cases effectively cover the subscription lifecycle:
- Registration with subscription
- Post-registration subscription purchase
- Subscription switching
- Cancellation
tests/pw/pages/storeReviewsPage.ts (2)
22-44: LGTM! Thorough module state verification.
The enable/disable methods effectively verify the module state across multiple pages and UI elements. Good practice to check both menu visibility and functional elements.
Line range hint 45-180: LGTM! Well-structured review management methods.
The methods provide comprehensive coverage of review management features with consistent patterns for:
- State verification
- Error handling
- Response waiting
- UI interaction
tests/pw/pages/productAddonsPage.ts (2)
17-45: LGTM! Well-structured module activation methods.
The enable/disable methods effectively verify the module state across multiple pages and UI elements. Good practice to check both menu visibility and functional elements.
Line range hint 47-150: LGTM! Comprehensive addon management implementation.
The methods provide thorough coverage of addon management features with consistent patterns for:
- State verification
- Error handling
- Response waiting
- UI interaction
🧰 Tools
🪛 Biome (1.9.4)
[error] 13-15: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
tests/pw/tests/e2e/productQA.spec.ts (2)
37-37: LGTM: Proper cleanup in afterAll hook
The addition of module activation in the afterAll hook ensures proper test isolation and cleanup.
158-161: Verify module deactivation order
The test correctly deactivates via API first, then UI. However, consider adding assertions to verify the module's disabled state.
test('admin can disable product Q&A module', { tag: ['@pro', '@admin'] }, async () => {
await apiUtils.deactivateModules(payloads.moduleIds.productQa, payloads.adminAuth);
await admin.disableProductQaModule(data.predefined.simpleProduct.product1.name);
+ // Add assertions to verify module is disabled
+ await expect(page.locator('selector-for-qa-module')).not.toBeVisible();
});tests/pw/pages/vendorProductSubscriptionPage.ts (1)
21-30: LGTM: Well-structured enable module method
The method follows good practices:
- Clear navigation steps
- Visibility checks for UI elements
- Proper separation of vendor dashboard and my account checks
tests/pw/tests/e2e/wholesale.spec.ts (1)
37-37: LGTM: Consistent module cleanup across test suites
The afterAll hooks in both test suites ensure proper module state cleanup.
Also applies to: 152-152
tests/pw/tests/e2e/euCompliance.spec.ts (1)
Line range hint 46-50: LGTM: Good cleanup practice in afterAll hook
The cleanup ensures a consistent state after test execution by reactivating the EU compliance modules.
tests/pw/pages/vendorDeliveryTimePage.ts (1)
21-33: LGTM: Well-structured module activation verification
The method follows a clear pattern of navigation and verification, ensuring the module is properly enabled across different parts of the application.
tests/pw/pages/vendorSubscriptionsPage.ts (1)
26-34: LGTM: Consistent module activation pattern
The method follows the established pattern for module activation verification, maintaining consistency across different modules.
tests/pw/pages/vendorReturnRequestPage.ts (4)
9-10: LGTM: Constants renamed for better clarity
The renaming of vendorReturnRequest to returnRequestVendor and addition of returnRequestSettingsVendor improves code readability by making the purpose of each constant more explicit.
19-32: LGTM: Well-structured module activation verification
The enableRmaModule method properly verifies the RMA module activation across different UI elements:
- Admin settings visibility
- Vendor dashboard menu visibility
- Vendor settings menu visibility
75-93: LGTM: Comprehensive settings verification
The vendorRmaSettingsRenderProperly method thoroughly verifies all RMA settings elements:
- Return and warranty text
- Store link
- Labels and types
- Refund reasons
- Policy iframe
- Save changes button
137-153: Consider adding validation for refund amounts
The vendorRmaRefund method should validate that refund amounts are non-negative and don't exceed the original amount.
tests/pw/tests/e2e/storeSupports.spec.ts (3)
22-22: LGTM: Proper module state management
The test suite correctly manages the Store Support module state in beforeAll and afterAll hooks.
Also applies to: 27-27
172-174: LGTM: Clean page object initialization
The vendor test suite properly initializes both admin and vendor page objects with separate contexts.
Also applies to: 179-182
34-36: Consider adding verification for module state
The enableStoreSupportModule test should verify the module's state after enabling.
tests/pw/tests/e2e/_env.setup.ts (2)
256-258: LGTM! Good to enable the delivery time settings test.
Enabling this previously skipped test improves test coverage.
269-269: LGTM! Consider verifying the option name change.
The option name change from productReportAbuse to reportAbuse looks correct.
tests/pw/pages/sellerBadgesPage.ts (2)
21-30: LGTM! Well-structured module enablement verification.
The method properly verifies seller badge module enablement by checking visibility in both backend and frontend contexts.
32-49: LGTM! Comprehensive module disablement verification.
The method thoroughly verifies seller badge module disablement across all relevant pages and contexts.
tests/pw/pages/productQAPage.ts (4)
9-9: LGTM! Consistent selector casing.
Updated selector follows consistent casing standards.
32-45: LGTM! Well-structured module enablement verification.
The method properly verifies product QA module enablement across backend, vendor dashboard, and single product contexts.
47-68: LGTM! Comprehensive module disablement verification.
The method thoroughly verifies product QA module disablement across all relevant pages and contexts.
234-234: LGTM! Consistent URL path updates.
Updated URL paths maintain consistency with the new casing standards.
Also applies to: 267-267, 273-273
tests/pw/tests/e2e/productsDetailsBookings.spec.ts (2)
50-50: LGTM! Consistent method naming for bookable products.
Updated to use the more specific createBookableProduct method consistently across all test cases.
Also applies to: 56-56, 61-61, 79-79, 171-171, 176-176
397-397: LGTM! Updated addon creation for booking products.
Changed to use the specialized createBookingProductWithAddon method for better type safety and clarity.
Also applies to: 402-402
tests/pw/pages/vendorVerificationsPage.ts (2)
24-38: LGTM! Well-structured enable method.
The enableVendorVerificationModule method is well-implemented with:
- Clear navigation flow
- Proper visibility checks
- Good error handling with explicit waits
40-62: LGTM! Comprehensive disable method.
The disableVendorVerificationModule method is well-implemented with:
- Thorough verification across multiple pages
- Proper visibility checks
- Good error handling
tests/pw/pages/customerPage.ts (1)
167-170: LGTM! Improved organization of EU billing fields.
The restructuring of EU-specific billing fields under euFields improves code organization and maintainability.
tests/pw/utils/testData.ts (4)
35-46: LGTM! Environment variables added for payment gateways.
The addition of environment variables for payment gateway credentials improves security and configuration flexibility.
870-872: LGTM! Proper fallback values for environment variables.
Good practice using nullish coalescing operator (??) to provide fallback values for missing environment variables.
Also applies to: 878-880, 891-892, 904-905, 913-914
1109-1113: LGTM! Payment gateway settings added.
Payment gateway settings are properly configured with appropriate URLs.
1190-1190: LGTM! Naming convention standardized.
Renamed productQA to productQa to follow camelCase naming convention.
tests/pw/utils/payloads.ts (2)
Line range hint 3631-3664: LGTM! Module ID changes improve naming consistency.
The renaming of module IDs better reflects their functionality:
germanized→euComplianceorder_min_max→minMaxQuantitiesvsp→productSubscriptionproduct_subscription→vendorSubscriptionexport_import→vendorImportExport
4934-4934: LGTM! Shipment comment update is correct.
The update to the shipment comments field is properly implemented.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (5)
tests/pw/pages/vendorSettingsPage.ts (4)
59-84: Consider grouping related visibility checks.The pro features visibility checks could be organized better for maintainability.
Consider grouping related checks into separate methods:
-// dokan pro options are visible -if (DOKAN_PRO) { - // catalog is visible - await this.toBeVisible(settingsVendor.catalogMode.removeAddToCartButton); - // toc is visible - // await this.toBeVisible(settingsVendor.toc.termsAndConditions); - // open close is visible - await this.toBeVisible(settingsVendor.storeOpeningClosingTime.storeOpeningClosingTime); - // vacation is visible - await this.toBeVisible(settingsVendor.vacation.vacationDiv); - // discount is visible - await this.toBeVisible(settingsVendor.discount.enableStoreWideDiscount); - // biography is visible - await this.toBeVisible(settingsVendor.biography.biographyIframe); - // store support elements are visible - await this.multipleElementVisible(settingsVendor.storeSupport); - // min max elements are visible - await this.multipleElementVisible(settingsVendor.minMax); -} +if (DOKAN_PRO) { + await this.checkProFeatureVisibility(); +} + +private async checkProFeatureVisibility(): Promise<void> { + await this.checkCatalogVisibility(); + await this.checkOpeningClosingTimeVisibility(); + await this.checkVacationVisibility(); + await this.checkDiscountVisibility(); + await this.checkBiographyVisibility(); + await this.checkStoreSupportVisibility(); + await this.checkMinMaxVisibility(); +}
398-402: Add confirmation handling in vacation deletion.The vacation deletion process should handle confirmation failures.
Consider adding error handling for the confirmation process:
async deletePreviousDatewiseVacation() { const noVacationIsSetIsVisible = await this.isVisible(settingsVendor.vacation.noVacationIsSet); if (!noVacationIsSetIsVisible) { - await this.hover(settingsVendor.vacation.vacationRow); - await this.click(settingsVendor.vacation.deleteSavedVacationSchedule); - await this.clickAndWaitForResponse(data.subUrls.ajax, settingsVendor.vacation.confirmDeleteSavedVacationSchedule); + try { + await this.hover(settingsVendor.vacation.vacationRow); + await this.click(settingsVendor.vacation.deleteSavedVacationSchedule); + await this.clickAndWaitForResponse(data.subUrls.ajax, settingsVendor.vacation.confirmDeleteSavedVacationSchedule); + } catch (error) { + throw new Error(`Failed to delete vacation schedule: ${error.message}`); + } } }
440-440: Add content validation for biography and support settings.The biography and support button text should be validated for content and length.
Consider adding content validation:
async biographySettings(biography: string): Promise<void> { + if (biography.length > 1000) { // Adjust max length as needed + throw new Error('Biography content exceeds maximum length'); + } await this.typeFrameSelector(settingsVendor.biography.biographyIframe, settingsVendor.biography.biographyHtmlBody, biography); } async storeSupportSettings(supportButtonText: string): Promise<void> { + if (supportButtonText.length > 50) { // Adjust max length as needed + throw new Error('Support button text exceeds maximum length'); + } await this.check(settingsVendor.storeSupport.showSupportButtonInStore); await this.check(settingsVendor.storeSupport.showSupportButtonInSingleProduct); await this.clearAndType(settingsVendor.storeSupport.supportButtonText, supportButtonText); }Also applies to: 445-447
450-453: Add state verification for live chat settings.The live chat settings should verify the state after toggling.
Consider adding state verification:
async liveChatSettings(): Promise<void> { await this.check(settingsVendor.liveChat); + // Verify the checkbox state after toggling + const isChecked = await this.isChecked(settingsVendor.liveChat); + if (!isChecked) { + throw new Error('Failed to enable live chat settings'); + } }tests/pw/pages/vendorAuctionsPage.ts (1)
25-39: Add error handling and improve robustnessThe method could benefit from improved error handling and robustness:
- Add error handling for navigation failures
- Implement retry mechanism for flaky UI interactions
- Add cleanup in case of intermediate failures
- Consider extracting selectors to constants
Consider this implementation:
async enableAuctionIntegrationModule() { + try { // dokan settings - await this.goto(data.subUrls.backend.dokan.settings); + await this.gotoWithRetry(data.subUrls.backend.dokan.settings); await this.click(selector.admin.dokan.settings.menus.sellingOptions); - await this.toBeVisible(selector.admin.dokan.settings.selling.newVendorEnableAuction); + await this.toBeVisibleWithTimeout(selector.admin.dokan.settings.selling.newVendorEnableAuction); // vendor dashboard menu - await this.goto(data.subUrls.frontend.vDashboard.dashboard); + await this.gotoWithRetry(data.subUrls.frontend.vDashboard.dashboard); await this.toBeVisible(selector.vendor.vDashboard.menus.primary.auction); // customer dashboard menu - await this.goto(data.subUrls.frontend.myAccount); + await this.gotoWithRetry(data.subUrls.frontend.myAccount); await this.toBeVisible(selector.customer.cMyAccount.menus.auctions); + } catch (error) { + console.error('Failed to enable auction module:', error); + throw error; + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
tests/pw/pages/paymentsPage.ts(6 hunks)tests/pw/pages/vendorAuctionsPage.ts(1 hunks)tests/pw/pages/vendorSettingsPage.ts(6 hunks)tests/pw/tests/e2e/payments.spec.ts(4 hunks)tests/pw/tests/e2e/vendorAnalytics.spec.ts(1 hunks)tests/pw/tests/e2e/vendorAuction.spec.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/pw/tests/e2e/payments.spec.ts
🔇 Additional comments (16)
tests/pw/tests/e2e/vendorAnalytics.spec.ts (5)
1-6: LGTM! Import statements are well-organized
The new imports for request, ApiUtils, and payloads are properly organized and necessary for the added functionality.
8-11: LGTM! Variable declarations are well-structured
The new variables are properly typed and the combined declaration of aPage and vPage improves code readability.
14-22: LGTM! Test setup follows best practices
The beforeAll hook properly initializes separate browser contexts for admin and vendor users, ensuring test isolation. The ApiUtils initialization is also handled correctly.
26-30: LGTM! Proper cleanup implementation
The afterAll hook now includes comprehensive cleanup for both admin and vendor pages, and properly disposes of apiUtils. This addresses the previous review comment about admin page cleanup.
32-49: 🛠️ Refactor suggestion
Enhance test organization and add assertions
The test cases should be reorganized and enhanced:
- Group admin tests together (enable/disable)
- Add assertions to verify module state changes
// admin
test('admin can enable vendor analytics module', { tag: ['@pro', '@admin'] }, async () => {
await admin.enableVendorAnalyticsModule();
+ // Verify module is enabled
+ await expect(await admin.getModuleStatus()).toBe('enabled');
});
+test('admin can disable vendor analytics module', { tag: ['@pro', '@admin'] }, async () => {
+ await apiUtils.deactivateModules(payloads.moduleIds.vendorAnalytics, payloads.adminAuth);
+ await admin.disableVendorAnalyticsModule();
+ // Verify module is disabled
+ await expect(await admin.getModuleStatus()).toBe('disabled');
+});
// vendor
test('vendor can view analytics menu page', { tag: ['@pro', '@exploratory', '@vendor'] }, async () => {
await vendor.vendorAnalyticsRenderProperly();
});
-
-// admin
-test('admin can disable vendor analytics module', { tag: ['@pro', '@admin'] }, async () => {
- await apiUtils.deactivateModules(payloads.moduleIds.vendorAnalytics, payloads.adminAuth);
- await admin.disableVendorAnalyticsModule();
-});tests/pw/pages/paymentsPage.ts (7)
39-68: Methods should be renamed to reflect their actual functionality
The enableXModule methods only verify visibility of setup elements and don't actually enable the modules.
69-113: Methods should be renamed to reflect their actual functionality
The disableXModule methods only verify invisibility of setup elements and don't actually disable the modules.
133-134: Incorrect selector used in setupStripeConnect method
The method is using MangoPay's save changes selector instead of Stripe Connect's.
161-162: Incorrect selector used in setupPaypalMarketPlace method
The method is using MangoPay's save changes selector instead of PayPal Marketplace's.
233-234: Incorrect selector used in setupRazorpay method
The method is using MangoPay's save changes selector instead of Razorpay's.
276-277: Incorrect selector used in setupStripeExpress method
The method is using MangoPay's save changes selector instead of Stripe Express's.
Line range hint 282-368: LGTM! Well-structured vendor payment methods
The vendor-related payment methods are well-implemented with proper error handling, consistent patterns, and clear structure.
tests/pw/pages/vendorSettingsPage.ts (1)
49-51: LGTM: EU compliance visibility check is properly implemented.
The conditional check for EU compliance elements is correctly implemented under the DOKAN_PRO environment variable.
tests/pw/tests/e2e/vendorAuction.spec.ts (3)
34-34: Move module activation to beforeAll hook
The module activation in afterAll seems counterintuitive as it restores the module state after all tests. This should ideally be in beforeAll to ensure a known state before tests run.
43-45: Add assertions for module enablement
The enable module test lacks assertions to verify the module was actually enabled.
111-114: 🛠️ Refactor suggestion
Improve disable module test implementation
The test has several issues:
- Inconsistent use of both
apiUtilsand page object methods - No assertions to verify the module was actually disabled
- Placement at the end could affect other tests if the module needs to be enabled
Consider this implementation:
-test('admin can disable auction integration module', { tag: ['@pro', '@admin'] }, async () => {
- await apiUtils.deactivateModules(payloads.moduleIds.auction, payloads.adminAuth);
- await admin.disableAuctionIntegrationModule();
-});
+test('admin can disable auction integration module', { tag: ['@pro', '@admin'] }, async () => {
+ await admin.disableAuctionIntegrationModule();
+ // Add assertions
+ await expect(admin.page.locator('selector-for-disabled-state')).toBeVisible();
+ // Verify API state
+ const moduleState = await apiUtils.getModuleState(payloads.moduleIds.auction);
+ expect(moduleState).toBe('disabled');
+});Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/pw/pages/paymentsPage.ts(6 hunks)
🔇 Additional comments (2)
tests/pw/pages/paymentsPage.ts (2)
39-112: Rename enable/disable methods to reflect their actual functionality
The methods like enableMangoPayModule() and disableMangoPayModule() only verify the visibility state of elements and don't actually enable or disable the modules. This could be misleading to users of this class.
Consider renaming these methods to better reflect their purpose:
-async enableMangoPayModule() {
+async verifyMangoPayModuleEnabled() {-async disableMangoPayModule() {
+async verifyMangoPayModuleDisabled() {Apply similar changes to other enable/disable methods.
133-134:
Fix incorrect selector usage in setup methods
Multiple setup methods are incorrectly using MangoPay's save changes selector (paymentSettingsAdmin.dokanMangoPay.dokanMangopaySaveChanges) instead of their respective module's selector.
Apply these fixes:
- For Stripe Connect (lines 133-134):
-await this.removeAttribute(paymentSettingsAdmin.dokanMangoPay.dokanMangopaySaveChanges, 'disabled');
+await this.removeAttribute(paymentSettingsAdmin.stripe.stripeSaveChanges, 'disabled');- For PayPal Marketplace (lines 161-162):
-await this.removeAttribute(paymentSettingsAdmin.dokanMangoPay.dokanMangopaySaveChanges, 'disabled');
+await this.removeAttribute(paymentSettingsAdmin.paypalMarketPlace.paypalMarketPlaceSaveChanges, 'disabled');- For Razorpay (lines 233-234):
-await this.removeAttribute(paymentSettingsAdmin.dokanMangoPay.dokanMangopaySaveChanges, 'disabled');
+await this.removeAttribute(paymentSettingsAdmin.dokanRazorpay.dokanRazorpaySaveChanges, 'disabled');- For Stripe Express (lines 276-277):
-await this.removeAttribute(paymentSettingsAdmin.dokanMangoPay.dokanMangopaySaveChanges, 'disabled');
+await this.removeAttribute(paymentSettingsAdmin.stripeExpress.stripeExpressSaveChanges, 'disabled');Also applies to: 161-162, 209-210, 233-234, 276-277
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
New Features
Bug Fixes
Documentation
Tests
Chores