Auction product details tests #2427
Conversation
WalkthroughThis pull request introduces significant changes to the permissions and functionalities across various pages, particularly enhancing vendor and customer capabilities. Key updates include modifications to the Changes
Possibly related PRs
Suggested labels
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: 17
🧹 Outside diff range and nitpick comments (20)
tests/pw/pages/helpPage.ts (1)
15-15: LGTM! Good addition of the networkidle wait state.The addition of 'networkidle' wait state is a good practice that will make the test more reliable by ensuring the page is fully loaded before checking element visibility.
Consider adding the same wait state to
adminGetHelpDropdownRenderProperlymethod for consistency:- await this.goIfNotThere(data.subUrls.backend.dokan.help); + await this.goIfNotThere(data.subUrls.backend.dokan.help, 'networkidle');tests/pw/tests/e2e/liveSearch.spec.ts (2)
Line range hint
8-24: Consider adding error handling and setup verification.While the setup is comprehensive, consider these improvements for better test reliability:
- Add error handling for database operations
- Verify widget setup was successful before proceeding with tests
Consider this enhancement:
test.beforeAll(async ({ browser }) => { const customerContext = await browser.newContext(data.auth.customerAuth); cPage = await customerContext.newPage(); customer = new LiveSearch(cPage); - await dbUtils.updateOptionValue('widget_dokna_product_search', dbData.liveSearchWidget); - await dbUtils.updateOptionValue('sidebars_widgets', { ...dbData.sidebarWidgets, 'sidebar-1': ['dokna_product_search-2'] }); + try { + await dbUtils.updateOptionValue('widget_dokna_product_search', dbData.liveSearchWidget); + await dbUtils.updateOptionValue('sidebars_widgets', { ...dbData.sidebarWidgets, 'sidebar-1': ['dokna_product_search-2'] }); + + // Verify setup + const widgetConfig = await dbUtils.getOptionValue('widget_dokna_product_search'); + if (!widgetConfig) { + throw new Error('Widget setup failed'); + } + } catch (error) { + console.error('Setup failed:', error); + throw error; + } });
Line range hint
26-46: Consider enhancing test coverage and reliability.While the existing tests cover basic functionality, consider these improvements:
- Add tests for edge cases (empty search, special characters, very long search terms)
- Isolate tests by resetting state before each test rather than modifying shared state
- Make assertions explicit in test descriptions
Consider adding these test cases:
test('customer sees empty results message for non-existent product search', { tag: ['@pro', '@customer'] }, async () => { await customer.searchByLiveSearch('nonexistentproduct123456'); }); test('customer can search with special characters', { tag: ['@pro', '@customer'] }, async () => { await customer.searchByLiveSearch('product-with-hyphen'); });Also, consider moving the live search option update to a beforeEach hook:
+test.beforeEach(async () => { + // Reset to default state before each test + await dbUtils.updateOptionValue(dbData.dokan.optionName.liveSearch, { live_search_option: 'new_live_search' }); +}); test('customer can search product using live search (autoload content)', { tag: ['@pro', '@customer'] }, async () => { - await dbUtils.updateOptionValue(dbData.dokan.optionName.liveSearch, { live_search_option: 'old_live_search' }); await customer.searchByLiveSearch(data.predefined.simpleProduct.product1.name, true); });tests/pw/pages/followStorePage.ts (1)
45-49: Consider adding error handling for store visibility timeout.The implementation looks good, but since we're dealing with network-dependent operations, consider adding a try-catch block to handle potential timeout errors when waiting for store visibility.
Here's a suggested improvement:
async customerViewFollowedVendors(storeName: string) { await this.goIfNotThere(data.subUrls.frontend.followingStores, 'domcontentloaded', true); - await this.toBeVisible(selector.customer.cVendors.followedStore(storeName)); + try { + await this.toBeVisible(selector.customer.cVendors.followedStore(storeName)); + } catch (error) { + throw new Error(`Store "${storeName}" not visible in followed stores: ${error.message}`); + } }tests/pw/utils/dbUtils.ts (2)
234-241: Add input validation and error handling.While the implementation is secure and prevents duplicate entries, consider these improvements:
- Add input validation for
followerIdandvendorId- Add specific error handling for this operation
Consider this enhancement:
async followVendor(followerId: string, vendorId: string) { + // Validate inputs + if (!followerId || !vendorId) { + throw new Error('followerId and vendorId are required'); + } + const currentTime = helpers.currentDateTimeFullFormat; - const query = `INSERT INTO ${dbPrefix}_dokan_follow_store_followers (vendor_id, follower_id, followed_at) - SELECT ?, ?, ? - WHERE NOT EXISTS ( SELECT 1 FROM ${dbPrefix}_dokan_follow_store_followers WHERE vendor_id = ? AND follower_id = ? );`; - const res = await dbUtils.dbQuery(query, [vendorId, followerId, currentTime, vendorId, followerId]); - return res; + try { + const query = `INSERT INTO ${dbPrefix}_dokan_follow_store_followers (vendor_id, follower_id, followed_at) + SELECT ?, ?, ? + WHERE NOT EXISTS ( SELECT 1 FROM ${dbPrefix}_dokan_follow_store_followers WHERE vendor_id = ? AND follower_id = ? );`; + const res = await dbUtils.dbQuery(query, [vendorId, followerId, currentTime, vendorId, followerId]); + return res; + } catch (error) { + throw new Error(`Failed to follow vendor: ${error.message}`); + } }
234-234: Add TypeScript interface for the return type.The function should specify its return type for better type safety.
-async followVendor(followerId: string, vendorId: string) { +async followVendor(followerId: string, vendorId: string): Promise<mysql.ResultSetHeader> {tests/pw/tests/e2e/productsDetails.spec.ts (3)
235-238: LGTM! Consider adding state verification.The test correctly validates adding virtual option functionality. Consider adding a verification step to confirm the product's virtual state after the change.
test('vendor can add product virtual option', { tag: ['@lite', '@vendor'] }, async () => { const [, , productName] = await apiUtils.createProduct(payloads.createProductRequiredFields(), payloads.vendorAuth); await vendor.addProductVirtualOption(productName, true); + // Verify the product's virtual state + const product = await apiUtils.getProduct(productName, payloads.vendorAuth); + expect(product.virtual).toBe(true); });
240-243: LGTM! Consider adding state verification.The test correctly validates removing virtual option functionality. Consider adding a verification step to confirm the product's virtual state after the change.
test('vendor can remove product virtual option', { tag: ['@lite', '@vendor'] }, async () => { const [, , productName] = await apiUtils.createProduct({ ...payloads.createProductRequiredFields(), virtual: true }, payloads.vendorAuth); await vendor.addProductVirtualOption(productName, false); + // Verify the product's virtual state + const product = await apiUtils.getProduct(productName, payloads.vendorAuth); + expect(product.virtual).toBe(false); });
383-385: LGTM! Consider validating error message.The test correctly validates the prevention of duplicate attributes. Consider adding validation for the error message to ensure proper user feedback.
test("vendor can't add already added product attribute", { tag: ['@pro', '@vendor'] }, async () => { await vendor.cantAddAlreadyAddedAttribute(productName, data.product.productInfo.attribute.attributeName); + // Verify the error message + const errorMessage = await vendor.getAttributeErrorMessage(); + expect(errorMessage).toBe('This attribute is already exists'); });tests/pw/utils/interfaces.ts (2)
292-294: LGTM! Consider clarifying the duration unit in the property name.The new properties are well-structured and follow the existing naming conventions. However, consider making the time unit more explicit in
relistAuctionDurationInHfor better clarity.- relistAuctionDurationInH: string; + relistAuctionDurationInHours: string;
292-294: Add JSDoc comments to document the new properties.Consider adding JSDoc comments to explain the purpose and expected format of the new auction relisting properties.
+ /** Number of hours to wait before relisting a failed auction */ relistIfFailAfterNHours: string; + /** Number of hours to wait before relisting an unpaid auction */ relistIfNotPaidAfterNHours: string; + /** Duration in hours for the relisted auction */ relistAuctionDurationInH: string;tests/pw/utils/payloads.ts (1)
Line range hint
1110-1147: Consider improving the required fields implementation.The required fields implementation could be enhanced in several ways:
- Document which fields are truly required vs optional
- Consider extracting common fields between full and required implementations
- Add TypeScript types to enforce required fields
Here's a suggested implementation:
// Define the required fields interface interface RequiredAuctionFields { name: string; type: 'auction'; status: 'publish'; purchasable: boolean; categories: Array<any>; meta_data: Array<{ key: string; value: string; }>; } // Extract common auction metadata const getRequiredAuctionMetadata = () => [ { key: '_auction_item_condition', value: 'new', }, { key: '_auction_type', value: 'normal', }, { key: '_auction_start_price', value: '10', }, { key: '_auction_bid_increment', value: '20', }, { key: '_auction_dates_from', value: helpers.currentDateTime, }, { key: '_auction_dates_to', value: helpers.addDays(helpers.currentDateTime, 20, 'full'), } ]; // Update the implementation to use the interface createAuctionProductRequiredFields: (): RequiredAuctionFields => ({ name: `${faker.commerce.productName()}_${faker.string.nanoid(5)} (Auction)`, type: 'auction', status: 'publish', purchasable: true, categories: [{}], meta_data: getRequiredAuctionMetadata() })tests/pw/pages/selectors.ts (5)
Line range hint
482-497: Consider using an enum for menu items.The menu items are currently defined as string literals. Consider using an enum or constant object to make the code more maintainable and type-safe.
+ enum VendorMenuItems { + ACCOUNT_INFO = 'Account Info', + ADDRESS = 'Address', + PAYMENT_OPTIONS = 'Payment Options' + } - accountInfo: '//div[@class="tab-link"]//a[contains(text(), "Account Info")]', - address: '//div[@class="tab-link"]//a[contains(text(), "Address")]', - paymentOptions: '//div[@class="tab-link"]//a[contains(text(), "Payment Options")]', + accountInfo: `//div[@class="tab-link"]//a[contains(text(), "${VendorMenuItems.ACCOUNT_INFO}")]`, + address: `//div[@class="tab-link"]//a[contains(text(), "${VendorMenuItems.ADDRESS}")]`, + paymentOptions: `//div[@class="tab-link"]//a[contains(text(), "${VendorMenuItems.PAYMENT_OPTIONS}")]`,
3847-3849: Remove redundant line breaks.These empty lines between related selector definitions can be removed to improve code readability.
- tagInput: '//input[@class="select2-search__field"]', - - searchedTag: (tagName: string) => `//li[contains(@class,"select2-results__option") and contains(@id, "select2-product_tag-result") and normalize-space(text())="${tagName}"]`, - - nonCreatedTag: (tagName: string) => `//li[@class="select2-results__option select2-results__option--highlighted" and normalize-space(text())="${tagName}"]`, + tagInput: '//input[@class="select2-search__field"]', + searchedTag: (tagName: string) => `//li[contains(@class,"select2-results__option") and contains(@id, "select2-product_tag-result") and normalize-space(text())="${tagName}"]`, + nonCreatedTag: (tagName: string) => `//li[@class="select2-results__option select2-results__option--highlighted" and normalize-space(text())="${tagName}"]`,
6804-6807: Remove redundant empty lines in customer vendor section.The spacing between related selectors is inconsistent. Maintain consistent spacing throughout the code.
- // customer followed store - cVendors: { - noVendorFound: '.dokan-error', - - followedStore: (vendorName: string) => `//div[@class="store-data "]//a[normalize-space()="${vendorName}"]`, + // customer followed store + cVendors: { + noVendorFound: '.dokan-error', + followedStore: (vendorName: string) => `//div[@class="store-data "]//a[normalize-space()="${vendorName}"]`,
6836-6838: Remove redundant empty lines in customer support tickets section.Similar to above, maintain consistent spacing between sections.
- // customer support tickets - cSupportTickets: { - - // menus + // customer support tickets + cSupportTickets: { + // menus
Line range hint
1-7500: Consider splitting this large file into smaller modules.The file is quite large (7000+ lines) which can make it difficult to maintain. Consider splitting it into smaller modules based on functionality (e.g., admin.selectors.ts, vendor.selectors.ts, customer.selectors.ts).
tests/pw/tests/e2e/productsDetailsAuction.spec.ts (1)
258-258: Improve readability by using named destructuringThe destructuring assignment skips multiple positions, which can make the code hard to understand and maintain. Consider returning an object from
createAttributeTermor using named variables to improve readability.Example:
const { attributeTerm: attributeTerm2 } = await apiUtils.createAttributeTerm( payloads.createAttribute(), payloads.createAttributeTerm(), payloads.adminAuth );tests/pw/pages/vendorAuctionsPage.ts (2)
88-90: Add verification after navigation ingoToAuctionProductEditByIdThe method
goToAuctionProductEditByIdnavigates to the auction product edit page but does not verify if the navigation was successful. Consider adding a check to ensure that the page has loaded correctly and that key elements are present.Add a verification step after navigation:
await this.gotoUntilNetworkidle(data.subUrls.frontend.vDashboard.auctionProductEdit(productId)); await this.toBeVisible(auctionProductsVendor.auction.productName);
507-529: Unnecessarydefaultcase in switch statementIn the
addProductOtherOptionsmethod, thedefaultcase in the switch statement does not perform any action. This can be omitted to simplify the code.Remove the unnecessary
defaultcase:switch (choice) { case 'status': await this.selectByValue(productsVendor.otherOptions.productStatus, otherOption.status); break; case 'visibility': await this.selectByValue(productsVendor.otherOptions.visibility, otherOption.visibility); break; - default: - break; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (17)
tests/pw/feature-map/feature-map.yml(4 hunks)tests/pw/package.json(1 hunks)tests/pw/pages/followStorePage.ts(2 hunks)tests/pw/pages/helpPage.ts(1 hunks)tests/pw/pages/productsPage.ts(4 hunks)tests/pw/pages/selectors.ts(27 hunks)tests/pw/pages/vendorAuctionsPage.ts(4 hunks)tests/pw/pages/vendorSettingsPage.ts(2 hunks)tests/pw/tests/e2e/followStore.spec.ts(2 hunks)tests/pw/tests/e2e/liveChat.spec.ts(2 hunks)tests/pw/tests/e2e/liveSearch.spec.ts(1 hunks)tests/pw/tests/e2e/productsDetails.spec.ts(4 hunks)tests/pw/tests/e2e/productsDetailsAuction.spec.ts(1 hunks)tests/pw/utils/dbUtils.ts(1 hunks)tests/pw/utils/interfaces.ts(1 hunks)tests/pw/utils/payloads.ts(1 hunks)tests/pw/utils/testData.ts(3 hunks)
🔇 Additional comments (25)
tests/pw/tests/e2e/liveSearch.spec.ts (2)
1-6: LGTM: Imports are well-organized and complete.
The imports follow a logical order and include all necessary dependencies.
Line range hint 1-46: Verify relevance to PR objectives.
This file contains live search tests, but the PR is titled "Auction product details tests". Could you clarify how these changes relate to auction product testing? Consider adding test cases specifically for searching auction products if that's part of the requirements.
tests/pw/pages/followStorePage.ts (1)
Line range hint 34-44: LGTM! Clean implementation of the followed vendors check.
The method properly handles both scenarios (no vendors and existing vendors) with appropriate assertions.
tests/pw/tests/e2e/followStore.spec.ts (2)
6-6: LGTM: Clean import and environment variable additions.
The additions follow the existing patterns and are necessary for the new test functionality.
Also applies to: 8-8
42-45: Verify complete test coverage for followed vendors functionality.
The new test case adds coverage for viewing followed vendors, but let's ensure we have comprehensive coverage.
✅ Verification successful
Let's examine the actual test coverage by looking at the test files content.
Test coverage for followed vendors functionality is comprehensive
The test suite has thorough coverage across both E2E and API levels:
- E2E tests cover:
- Customer perspective: viewing followed vendors, following/unfollowing from store list and single store pages
- Vendor perspective: viewing followers menu and followers list
- API tests cover:
- Store follow status checks
- Follow/unfollow operations
- Followers retrieval
- Supporting utilities (DB and API) are properly implemented for test setup and verification
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for all followed vendor related test cases
echo "Searching for followed vendor related tests..."
rg -l "follow.*vendor|vendor.*follow" --type ts tests/
Length of output: 353
Script:
#!/bin/bash
# Check content of follow-related test files
echo "=== E2E Test Coverage (followStore.spec.ts) ==="
rg -A 5 "test\(" tests/pw/tests/e2e/followStore.spec.ts
echo -e "\n=== API Test Coverage (followStores.spec.ts) ==="
rg -A 5 "test\(" tests/pw/tests/api/followStores.spec.ts
echo -e "\n=== Available Follow-related DB Utils ==="
rg "follow" tests/pw/utils/dbUtils.ts
echo -e "\n=== Available Follow-related API Utils ==="
rg "follow" tests/pw/utils/apiUtils.ts
Length of output: 5359
tests/pw/package.json (1)
13-15: LGTM! Script naming simplified for better clarity.
The renaming of setup scripts removes the redundant "test:" prefix while maintaining the same functionality. This change improves readability and follows the principle of keeping things simple.
Let's verify that no other scripts depend on the old names:
✅ Verification successful
Script renaming changes are safe to proceed
The comprehensive search across the codebase shows no references to the old script names (test:site:setup, test:env:setup, test:site:reset), and no direct usage of the playwright test commands with these project configurations outside of the package.json file. This confirms that the renaming is safe and won't break any existing dependencies.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any references to the old script names in the codebase
# Test: Search for old script names. Expect: No occurrences of old names in other files.
echo "Checking for references to old script names..."
rg "test:site:setup|test:env:setup|test:site:reset" --type-not json
Length of output: 172
Script:
#!/bin/bash
# Let's try a more comprehensive search approach:
# 1. Search for any references to these scripts in package.json files
echo "Checking package.json files for script references..."
fd "package.json" --exec grep -l "test:site:\|test:env:" {} \;
# 2. Search for npm run commands in all files
echo -e "\nChecking for npm run commands with old script names..."
rg "npm run test:(site|env)" --type-not json
# 3. Search for direct npx playwright commands that might bypass package.json
echo -e "\nChecking for direct playwright test commands..."
rg "npx playwright test.*--project=(local_site_setup|e2e_setup)" --type-not json
Length of output: 546
tests/pw/tests/e2e/liveChat.spec.ts (2)
6-6: LGTM: Import statement is properly placed and utilized.
Line range hint 1-93: Verify PR scope: Live chat tests in auction product details PR.
This file contains live chat functionality tests but is included in a PR titled "Auction product details tests". This seems to be outside the scope of the PR's stated objectives.
Let's verify if there are any auction-related changes in this file:
tests/pw/utils/dbUtils.ts (1)
236-238: LGTM! Well-constructed SQL query.
The SQL query is well-constructed with:
- Prevention of duplicate entries using WHERE NOT EXISTS
- Proper use of parameterized queries for security
- Inclusion of timestamp for audit trail
tests/pw/pages/vendorSettingsPage.ts (2)
240-240: LGTM: Method call updated to match new signature.
The call to liveChatSettings() has been correctly updated to align with the simplified method signature.
431-433: Verify the simplification of live chat settings.
The liveChatSettings method has been simplified to a basic checkbox toggle. Please verify that:
- There are no additional configuration options needed for the live chat feature
- The existing test coverage remains adequate with this simplified implementation
✅ Verification successful
Live chat settings simplification is appropriate
The simplification of liveChatSettings to a basic checkbox toggle is correct because:
- The live chat feature is controlled by a single option in the database (
dokan_live_chat) - Test scenarios only verify basic enable/disable functionality:
- Setting up live chat via database option
- Enabling chat button on vendor page
- The actual chat functionality (iframe, messages) is handled separately in
liveChatPage.ts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for tests that might be affected by the live chat settings change
rg -l "liveChatSettings|liveChat.*vendor" tests/
Length of output: 212
Script:
#!/bin/bash
# Check the implementation of live chat tests and related files
rg -A 10 "liveChatSettings|liveChat.*vendor" tests/pw/tests/e2e/liveChat.spec.ts
# Check the database setup/mock data
rg "liveChat" tests/pw/utils/dbData.ts
# Check the live chat page implementation
rg "liveChat" tests/pw/pages/liveChatPage.ts
# Check the environment setup
rg "liveChat" tests/pw/tests/e2e/_env.setup.ts
Length of output: 2293
tests/pw/tests/e2e/productsDetails.spec.ts (1)
297-299: LGTM!
The test follows the established pattern for testing product options and correctly validates the purchase note update functionality.
tests/pw/feature-map/feature-map.yml (4)
152-153: LGTM: Virtual product options permissions are properly configured.
The new permissions for managing virtual product options are correctly scoped to [lite] version and follow the existing pattern for product management permissions.
166-166: LGTM: Purchase note update permission is properly configured.
The permission for updating product purchase notes is correctly scoped to [lite] version and aligns with existing product management capabilities.
292-292: LGTM: Store terms visibility is properly configured.
The permission for customers to view store terms and conditions is correctly scoped to [lite] version and complements the vendor's existing ability to set terms.
733-733: LGTM: Followed vendors visibility is properly configured.
The permission aligns with the surrounding follow store features and maintains consistency in the feature set.
tests/pw/pages/productsPage.ts (2)
1041-1060: LGTM! Well-structured method implementation.
The addProductVirtualOption method is well-implemented with:
- Clear parameter types and return type
- Proper error handling and state verification
- Good use of conditional logic for DOKAN_PRO features
- Comprehensive assertions to verify the changes
1339-1345: LGTM! Good validation implementation.
The cantAddAlreadyAddedAttribute method is well-implemented with:
- Clear focus on a single validation case
- Good use of assertions to verify the disabled state
- Proper error handling
tests/pw/utils/testData.ts (3)
400-402: LGTM! New auction relisting properties added.
The new properties for auction relisting functionality are well-structured and follow the existing patterns in the file.
1081-1081: LGTM! Auction product edit path added.
The new path function for editing auction products is well-implemented and follows the established patterns in the file.
1005-1005: Verify the impact of the renamed frontend path.
The rename from 'vendors' to 'followingStores' improves clarity. However, ensure that all test files using this path have been updated accordingly.
✅ Verification successful
Let me gather more context about the usage of these paths in the test file.
Let me check if there are any references to this path being used in test files or route configurations.
Let me check the content of the followStorePage.ts to see how the path is being used there.
Path rename is properly implemented across test files
The renamed path from 'vendors' to 'followingStores' is correctly referenced in both test files:
tests/pw/utils/testData.ts: Defines the path constanttests/pw/pages/followStorePage.ts: Uses the path viadata.subUrls.frontend.followingStores
No instances of the old path 'my-account/vendors' were found in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any remaining references to the old path
# Test: Look for any files still using the old 'vendors' path
rg -l "my-account/vendors"
# Test: Verify the new path is used consistently
rg -l "my-account/following"
Length of output: 82
Script:
#!/bin/bash
# Get more context around the usage of both old and new paths
rg -C 3 "my-account/following|my-account/vendors" tests/pw/utils/testData.ts
Length of output: 407
Script:
#!/bin/bash
# Search for any usage of these paths in test files or route configurations
rg -l "followingStores|my-account/following"
# Also check for any potential route handlers or components
ast-grep --pattern 'const $_ = $_("/my-account/following")'
ast-grep --pattern 'path: "/my-account/following"'
Length of output: 222
Script:
#!/bin/bash
# Check how the path is used in the follow store page
rg -C 3 "followingStores|my-account/following" tests/pw/pages/followStorePage.ts
Length of output: 640
tests/pw/pages/selectors.ts (1)
Line range hint 1-3: LGTM! Clear file header with appropriate imports.
The file starts with a clean import statement and follows TypeScript conventions.
tests/pw/tests/e2e/productsDetailsAuction.spec.ts (2)
108-108: Address the TODO: Prepare a product with a cover image
The same TODO as previously noted needs to be addressed here as well.
228-228: Include reference to the known Dokan issue when skipping tests
The same suggestion applies here for including an issue reference in the skip message.
tests/pw/pages/vendorAuctionsPage.ts (1)
365-367: Possible selector inconsistency when removing images
When removing the cover image or gallery images, the selectors used (productsVendor.image.coverImageDiv and productsVendor.image.galleryImageDiv) might not accurately target the correct elements, especially if there are multiple images or dynamic changes in the DOM. Ensure that these selectors uniquely identify the intended elements to avoid flaky tests.
Run the following script to check the uniqueness and consistency of the selectors:
Also applies to: 379-381
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Outside diff range and nitpick comments (26)
tests/pw/pages/helpPage.ts (1)
15-15: Good improvement to test reliability!Adding the 'networkidle' wait condition ensures that all network connections are idle before proceeding with visibility checks, making the test more reliable.
For consistency, consider applying the same pattern to the
adminGetHelpDropdownRenderProperlymethod:- await this.goIfNotThere(data.subUrls.backend.dokan.help); + await this.goIfNotThere(data.subUrls.backend.dokan.help, 'networkidle');tests/pw/tests/e2e/liveSearch.spec.ts (1)
Line range hint
7-23: Consider enhancing test setup robustnessWhile the setup is functional, consider these improvements for better test isolation and error handling:
- Add try-catch blocks in hooks to handle setup/cleanup failures gracefully
- Consider resetting database state before each test
- Add error scenario tests (e.g., database connection failures)
Example enhancement:
test.beforeAll(async ({ browser }) => { + try { const customerContext = await browser.newContext(data.auth.customerAuth); cPage = await customerContext.newPage(); customer = new LiveSearch(cPage); await dbUtils.updateOptionValue('widget_dokna_product_search', dbData.liveSearchWidget); await dbUtils.updateOptionValue('sidebars_widgets', { ...dbData.sidebarWidgets, 'sidebar-1': ['dokna_product_search-2'] }); + } catch (error) { + console.error('Test setup failed:', error); + throw error; + } });tests/pw/pages/followStorePage.ts (1)
Line range hint
34-44: Add JSDoc comment to describe the method's purpose and behavior.The method handles both empty and non-empty states well, but could benefit from documentation explaining its purpose, expected behavior, and return value.
Add a descriptive comment like this:
/** * Verifies the rendering of followed vendors page. * Checks for either "No vendor found" message or the presence of store cards. * @returns {Promise<void>} */tests/pw/tests/e2e/liveChat.spec.ts (1)
Line range hint
33-89: Consider adding additional test scenariosThe test suite covers the main happy paths well, but consider adding these scenarios for more robust coverage:
- Negative test cases:
- Handling invalid/empty messages
- Network disconnection scenarios
- Concurrent chat sessions:
- Multiple customers chatting with same vendor
- Vendor managing multiple chat sessions
- Chat history persistence:
- Verify messages are saved correctly
- Test chat history retrieval
Would you like me to help create these additional test cases?
tests/pw/utils/dbUtils.ts (2)
234-241: The implementation looks good but could benefit from improved type safety and error handling.The function correctly implements the follow vendor functionality with duplicate prevention. However, consider these improvements:
Consider applying these enhancements:
- async followVendor(followerId: string, vendorId: string) { + async followVendor(followerId: string, vendorId: string): Promise<{ affectedRows: number }> { + // Validate inputs + if (!followerId || !vendorId) { + throw new Error('followerId and vendorId are required'); + } + const currentTime = helpers.currentDateTimeFullFormat; const query = `INSERT INTO ${dbPrefix}_dokan_follow_store_followers (vendor_id, follower_id, followed_at) SELECT ?, ?, ? WHERE NOT EXISTS ( SELECT 1 FROM ${dbPrefix}_dokan_follow_store_followers WHERE vendor_id = ? AND follower_id = ? );`; - const res = await dbUtils.dbQuery(query, [vendorId, followerId, currentTime, vendorId, followerId]); + const res = await dbUtils.dbQuery(query, [vendorId, followerId, currentTime, vendorId, followerId]) as { affectedRows: number }; return res; },The changes:
- Add explicit return type for better type safety
- Add input validation to prevent null/empty values
- Add type assertion for the query result
233-233: Add JSDoc documentation for the new method.Add documentation to describe the method's purpose, parameters, and return value:
+ /** + * Creates a follower relationship between a customer and a vendor. + * + * @param followerId - The ID of the customer following the vendor + * @param vendorId - The ID of the vendor being followed + * @returns Promise resolving to the query result with affected rows + * @throws {Error} When followerId or vendorId is empty + */ async followVendor(followerId: string, vendorId: string): Promise<{ affectedRows: number }> {tests/pw/pages/vendorSettingsPage.ts (1)
431-433: Consider enhancing test coverage for live chat settings.The simplified implementation looks good, but consider adding test cases to verify different live chat states and configurations.
Consider expanding the test coverage by adding methods like:
async verifyLiveChatEnabled(): Promise<void> { await this.toBeChecked(settingsVendor.liveChat); } async verifyLiveChatDisabled(): Promise<void> { await this.not.toBeChecked(settingsVendor.liveChat); }tests/pw/tests/e2e/productsDetails.spec.ts (1)
Line range hint
1-541: Consider adding missing test scenarios.The test suite is comprehensive but could benefit from additional test scenarios:
- Test virtual product options with shipping settings
- Test attribute validation with special characters
- Test purchase note with maximum length constraints
Would you like me to help generate these additional test cases?
tests/pw/utils/interfaces.ts (1)
292-294: Consider using number type and consistent naming for hour-based properties.The implementation is functionally correct. However, here are some suggestions for improvement:
- Consider using
numbertype instead ofstringfor hour values to ensure type safety and prevent invalid inputs.- Make property naming more consistent by using either 'Hours' or 'H' suffix across all properties.
- relistIfFailAfterNHours: string; - relistIfNotPaidAfterNHours: string; - relistAuctionDurationInH: string; + relistIfFailAfterHours: number; + relistIfNotPaidAfterHours: number; + relistAuctionDurationHours: number;tests/pw/feature-map/feature-map.yml (1)
Line range hint
1-1000: Consider organizing auction-related permissions.Given that this PR focuses on auction product details tests, I notice that auction-related permissions are scattered across different sections. Consider grouping them under a dedicated "Auction Products" section for better maintainability.
Example structure:
+ - page: 'Auction Products' + features: + admin: + admin can add auction product: true + vendor: + vendor can add auction product: true + vendor can edit auction product: true + vendor can view auction product: true + vendor can't bid own product: true + vendor can search auction product: true + vendor can permanently delete auction product: true + vendor can view auction activity page: true + vendor can filter auction activity: true + vendor can search auction activity: true + customer: + customer can bid auction product: true + customer can buy auction product with buy it now price: falsetests/pw/pages/productsPage.ts (2)
571-573: Fix indentation to use tabs instead of spaces.The indentation uses spaces while the rest of the file uses tabs. Maintain consistent indentation throughout the file.
- if (DOKAN_PRO) { - await this.notToBeVisible(productsVendor.shipping.shippingContainer); - } + if (DOKAN_PRO) { + await this.notToBeVisible(productsVendor.shipping.shippingContainer); + }
1223-1224: Remove extra empty line between methods.A single empty line is sufficient for separating methods.
async addProductShipping(productName: string, shipping: product['productInfo']['shipping']): Promise<void> { // ... method implementation ... } - // remove product shipping async removeProductShipping(productName: string): Promise<void> { // ... method implementation ... }tests/pw/utils/testData.ts (1)
400-402: Consider using numbers instead of strings for numeric values.The auction relist configuration properties use string values for numeric data. Consider using numbers instead to maintain type consistency and prevent potential type coercion issues.
- relistIfFailAfterNHours: '1', - relistIfNotPaidAfterNHours: '2', - relistAuctionDurationInH: '3', + relistIfFailAfterNHours: 1, + relistIfNotPaidAfterNHours: 2, + relistAuctionDurationInH: 3,tests/pw/utils/payloads.ts (1)
Line range hint
1110-1147: Add JSDoc documentation for the required fields function.The required fields implementation is clean, but would benefit from clear documentation about which fields are mandatory vs optional for auction products.
Add JSDoc documentation:
+/** + * Creates the minimum required fields for an auction product. + * @returns {Object} Object containing the minimum required fields for an auction product + * Required fields: + * - name: Product name + * - type: Must be 'auction' + * - status: Product status + * - purchasable: Must be true + * - categories: At least one category + * - meta_data: Must include auction settings (condition, type, pricing, dates) + */ createAuctionProductRequiredFields: () => ({ // ... implementation })tests/pw/pages/selectors.ts (5)
Line range hint
1309-1424: Fix indentation and remove redundant comments in reports section.The reports section has inconsistent indentation and contains redundant comments. Consider fixing the indentation and removing unnecessary comments.
- // reports - reports: { - // menus - menus: { + reports: { + menus: { reports: '//a[contains(@class, "nav-tab") and contains(text(),"Reports")]', allLogs: '//a[contains(@class, "nav-tab") and contains(text(),"All Logs")]', },
Line range hint
482-494: Fix indentation and remove redundant comments in menu manager section.The menu manager section has inconsistent indentation and contains redundant comments.
- // menus - accountInfo: '//div[@class="tab-link"]//a[contains(text(), "Account Info")]', - address: '//div[@class="tab-link"]//a[contains(text(), "Address")]', + accountInfo: '//div[@class="tab-link"]//a[contains(text(), "Account Info")]', + address: '//div[@class="tab-link"]//a[contains(text(), "Address")]',
Line range hint
3672-3679: Fix formatting in tags section.The tags section has inconsistent spacing and contains redundant empty lines.
- tags: { - tagInput: '//input[@class="select2-search__field"]', - searchedTag: (tagName: string) => `//li[@class="select2-results__option select2-results__option--highlighted"]//span[normalize-space(text())="${tagName}"]`, - nonCreatedTag: (tagName: string) => `//li[@class="select2-results__option select2-results__option--highlighted" and normalize-space(text())="${tagName}"]`, - - selectedTags: (tagName: string) => `//li[@class="select2-selection__choice" and contains(., "${tagName}")]`, - removeSelectedTags: (tagName: string) => `//li[@class="select2-selection__choice" and contains(., "${tagName}")]//span[@class="select2-selection__choice__remove"]`, - }, + tags: { + tagInput: '//input[@class="select2-search__field"]', + searchedTag: (tagName: string) => `//li[@class="select2-results__option select2-results__option--highlighted"]//span[normalize-space(text())="${tagName}"]`, + nonCreatedTag: (tagName: string) => `//li[@class="select2-results__option select2-results__option--highlighted" and normalize-space(text())="${tagName}"]`, + selectedTags: (tagName: string) => `//li[@class="select2-selection__choice" and contains(., "${tagName}")]`, + removeSelectedTags: (tagName: string) => `//li[@class="select2-selection__choice" and contains(., "${tagName}")]//span[@class="select2-selection__choice__remove"]`, + },
Line range hint
3896-3897: Fix indentation and remove redundant comments in bookings section.The bookings section has inconsistent indentation and contains redundant comments.
- // bookings - bookings: { + bookings: {
5613-5615: Fix indentation and remove redundant comments in auction section.The auction section has inconsistent indentation and contains redundant comments.
- // auction - vAuction: { - // menus + vAuction: {tests/pw/tests/e2e/productsDetailsAuction.spec.ts (6)
101-105: Address the TODO comment.The test for updating the auction product cover image is implemented correctly. However, there is a TODO comment indicating that a product with a cover image is needed. Ensure that the test setup includes a product with a cover image before running this test.
Do you want me to generate the code to create a product with a cover image or open a GitHub issue to track this task?
107-111: Address the TODO comment.The test for removing the auction product cover image is implemented correctly. However, there is a TODO comment indicating that a product with a cover image is needed. Ensure that the test setup includes a product with a cover image before running this test.
Do you want me to generate the code to create a product with a cover image or open a GitHub issue to track this task?
119-123: Address the TODO comment.The test for updating auction product gallery images is implemented correctly. However, there is a TODO comment indicating that a product with gallery images is needed. Ensure that the test setup includes a product with gallery images before running this test.
Do you want me to generate the code to create a product with gallery images or open a GitHub issue to track this task?
125-129: Address the TODO comment.The test for removing auction product gallery images is implemented correctly. However, there is a TODO comment indicating that a product with gallery images is needed. Ensure that the test setup includes a product with gallery images before running this test.
Do you want me to generate the code to create a product with gallery images or open a GitHub issue to track this task?
157-160: Address the TODO comment.The test for updating auction product downloadable options is implemented correctly. However, there is a TODO comment indicating that a product with a downloadable file is needed. Ensure that the test setup includes a product with a downloadable file before running this test.
Do you want me to generate the code to create a product with a downloadable file or open a GitHub issue to track this task?
162-167: Address the TODO comment.The test for removing the auction product downloadable file is implemented correctly. However, there is a TODO comment indicating that a product with a downloadable file is needed. Ensure that the test setup includes a product with a downloadable file before running this test.
Do you want me to generate the code to create a product with a downloadable file or open a GitHub issue to track this task?
tests/pw/pages/vendorAuctionsPage.ts (1)
569-585: Handle Potential Duplicate Attributes When Adding Product AttributesIn the
addProductAttributemethod, there is no check to prevent adding an attribute that already exists. This may lead to errors or duplicate entries.Consider checking if the attribute already exists before attempting to add it:
+ // Check if the attribute is already added + const attributeExists = await this.isVisible(productsVendor.attribute.savedAttribute(attribute.attributeName)); + if (!attributeExists) { await this.selectByLabel(productsVendor.attribute.customAttribute, attribute.attributeName); await this.clickAndWaitForResponse(data.subUrls.ajax, productsVendor.attribute.addAttribute); + } else { + console.warn(`Attribute ${attribute.attributeName} is already added.`); + } // Continue with adding terms and savingThis ensures the method is idempotent and prevents unnecessary actions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (17)
tests/pw/feature-map/feature-map.yml(4 hunks)tests/pw/package.json(1 hunks)tests/pw/pages/followStorePage.ts(2 hunks)tests/pw/pages/helpPage.ts(1 hunks)tests/pw/pages/productsPage.ts(4 hunks)tests/pw/pages/selectors.ts(27 hunks)tests/pw/pages/vendorAuctionsPage.ts(4 hunks)tests/pw/pages/vendorSettingsPage.ts(2 hunks)tests/pw/tests/e2e/followStore.spec.ts(2 hunks)tests/pw/tests/e2e/liveChat.spec.ts(2 hunks)tests/pw/tests/e2e/liveSearch.spec.ts(1 hunks)tests/pw/tests/e2e/productsDetails.spec.ts(4 hunks)tests/pw/tests/e2e/productsDetailsAuction.spec.ts(1 hunks)tests/pw/utils/dbUtils.ts(1 hunks)tests/pw/utils/interfaces.ts(1 hunks)tests/pw/utils/payloads.ts(1 hunks)tests/pw/utils/testData.ts(3 hunks)
🔇 Additional comments (57)
tests/pw/tests/e2e/liveSearch.spec.ts (2)
1-5: LGTM! Clean import organization
The imports are well-organized and the removal of ApiUtils aligns with e2e testing best practices by focusing on UI interactions rather than direct API calls.
Line range hint 25-45: Enhance test coverage and documentation
The test cases cover basic functionality well, but consider these improvements:
- Add explicit assertions for search results
- Include edge cases (empty results, special characters)
- Add JSDoc comments describing test scenarios and expected outcomes
Example enhancement:
+/**
+ * Tests the live search functionality with suggestion box
+ * Expected behavior:
+ * 1. Search results should appear in suggestion box
+ * 2. Results should match the search term
+ * 3. Results should be limited to the specified category if provided
+ */
test('customer can search product using live search (suggestion box)', { tag: ['@pro', '@customer'] }, async () => {
await customer.searchByLiveSearch(data.predefined.simpleProduct.product1.name);
+ // Add explicit assertions
+ await expect(customer.getSearchResults()).toContain(data.predefined.simpleProduct.product1.name);
});Let's verify the test coverage:
tests/pw/pages/followStorePage.ts (1)
45-49: Verify test coverage for the new method.
Let's ensure the new method is properly utilized in test cases.
✅ Verification successful
Method is properly integrated and tested
The customerViewFollowedVendors method is being used in a dedicated test case that verifies the customer's ability to view followed vendors. The test case:
- Has appropriate tags (
@pro,@customer) - Sets up the test data using
dbUtils.followVendor - Verifies the functionality using the method in question
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of customerViewFollowedVendors in test files
echo "Searching for test cases using customerViewFollowedVendors..."
rg "customerViewFollowedVendors" -t ts tests/pw/tests/
Length of output: 316
Script:
#!/bin/bash
# Let's check the test case context where customerViewFollowedVendors is used
rg "customerViewFollowedVendors" -B 5 -A 5 tests/pw/tests/e2e/followStore.spec.ts
Length of output: 729
tests/pw/tests/e2e/followStore.spec.ts (2)
6-8: LGTM: Clean environment setup changes
The addition of dbUtils import and CUSTOMER_ID environment variable is well-structured and necessary for the new test functionality.
42-45: Consider consistency and cleanup in test implementation
The new test case introduces direct DB manipulation which differs from the pattern used in other tests that utilize apiUtils. This inconsistency might make the test suite harder to maintain.
Consider:
- Using
apiUtils.followStore()for consistency with other tests - Adding cleanup in afterAll/afterEach to ensure test isolation
Let's verify the cleanup patterns in other test files:
Here's a suggested refactor to align with existing patterns:
test('customer can view followed vendors', { tag: ['@pro', '@customer'] }, async () => {
- await dbUtils.followVendor(CUSTOMER_ID, VENDOR_ID);
+ await apiUtils.followStore(VENDOR_ID, payloads.customerAuth);
await customer.customerViewFollowedVendors(data.predefined.vendorStores.vendor1);
});✅ Verification successful
Let me analyze the cleanup patterns in the test files and verify if we need additional information.
This will help us understand:
- The complete context of the test case in question
- Other related test cases in the same file to verify the consistency of follow/unfollow operations
Consider using apiUtils.followStore() for consistency
The test file shows a clear pattern where all other follow/unfollow operations use apiUtils.followStore() and apiUtils.unfollowStore(). Only this single test uses dbUtils.followVendor(). For better maintainability and consistency, use apiUtils.followStore() instead.
Suggested refactor:
test('customer can view followed vendors', { tag: ['@pro', '@customer'] }, async () => {
- await dbUtils.followVendor(CUSTOMER_ID, VENDOR_ID);
+ await apiUtils.followStore(VENDOR_ID, payloads.customerAuth);
await customer.customerViewFollowedVendors(data.predefined.vendorStores.vendor1);
});Note: Test cleanup is already handled in the test file's afterAll hook which calls apiUtils.dispose(), so no additional cleanup is needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for cleanup patterns in other test files
rg -A 5 "afterAll|afterEach" "tests/pw/tests/e2e/"
Length of output: 34691
Script:
#!/bin/bash
# Search for cleanup patterns in followStore.spec.ts
rg -A 10 "test\('customer can view followed vendors" "tests/pw/tests/e2e/followStore.spec.ts"
# Search for other follow/unfollow related tests
rg -A 5 "followStore|followVendor|unfollowStore|unfollowVendor" "tests/pw/tests/e2e/followStore.spec.ts"
Length of output: 3035
tests/pw/package.json (1)
13-15: LGTM! Command naming improvements look good.
The script command changes improve naming consistency by removing redundant "test:" prefixes while maintaining the same functionality. The dependencies are properly configured for the testing requirements.
Let's verify no other scripts are still referencing the old command names:
✅ Verification successful
Command renaming changes are safe to proceed
The verification shows no remaining references to the old command names (test:site:setup, test:env:setup, test:site:reset) in the codebase, confirming that the renaming changes are complete and consistent. The new command names can be safely used without breaking any existing scripts or workflows.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to old command names
rg "test:site:setup|test:env:setup" --type json
Length of output: 49
Script:
#!/bin/bash
# Let's try a broader search to find any potential references to these commands
# First, let's check for any references to these commands in any file
rg -l "test:site:setup|test:env:setup|test:site:reset"
# Also check for any npm script references that might use these commands
rg "npm run.*test:(site|env):(setup|reset)"
Length of output: 102
tests/pw/tests/e2e/liveChat.spec.ts (1)
6-6: LGTM: Clean import statement
The import follows the project's consistent import style and is properly placed with other imports.
tests/pw/utils/dbUtils.ts (1)
234-241: Verify test coverage for the followVendor method.
Let's verify the test integration:
✅ Verification successful
Let me gather more context about the test implementation to ensure proper coverage.
Let me check the customer helper methods to understand the complete test coverage.
Test coverage for followVendor method is comprehensive and well-integrated
The followVendor method is properly tested through:
- Direct usage in
followStore.spec.tsfor verifying followed vendors view - Integration with API-level follow/unfollow operations
- End-to-end tests covering both UI interactions and database operations
- Test scenarios for both following and unfollowing stores from different pages
- Verification of the followed vendors list rendering
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if followVendor is properly tested
# Test 1: Check if followStore.spec.ts exists and uses followVendor
echo "Checking for followStore.spec.ts test file..."
rg -l "followStore\.spec\.ts"
# Test 2: Verify usage of followVendor in tests
echo "Checking usage of followVendor in tests..."
rg "followVendor" "tests/pw/"
Length of output: 443
Script:
#!/bin/bash
# Get the test implementation details
echo "Checking test implementation in followStore.spec.ts..."
rg -A 10 -B 10 "followVendor" "tests/pw/tests/e2e/followStore.spec.ts"
# Check for related test assertions
echo "Checking for related assertions..."
rg "test\(.*follow" "tests/pw/tests/e2e/followStore.spec.ts"
Length of output: 2127
Script:
#!/bin/bash
# Check the customer helper implementation
echo "Checking customer helper methods..."
rg -A 5 "customerViewFollowedVendors|customerFollowedVendorsRenderProperly|followUnfollowStore" "tests/pw/"
# Check the API utils implementation
echo "Checking API utils implementation..."
rg -A 5 "unfollowStore" "tests/pw/"
Length of output: 8920
tests/pw/pages/vendorSettingsPage.ts (1)
240-240: LGTM!
The update to remove the parameter correctly aligns with the liveChatSettings method signature change.
tests/pw/tests/e2e/productsDetails.spec.ts (3)
132-132: LGTM! Proper test setup.
The update to product category style setting is correctly implemented before testing the category functionality.
233-244: LGTM! Well-structured virtual product option tests.
The tests for virtual product options are well-implemented with:
- Proper test data setup using payloads
- Both positive and negative scenarios (add/remove)
- Clear test descriptions
383-386: LGTM! Important validation test added.
Good addition of a negative test case to verify that duplicate attributes cannot be added. This helps ensure data integrity and proper validation.
tests/pw/feature-map/feature-map.yml (3)
152-153: LGTM: Virtual product options permissions are properly configured.
The new permissions align with the product management capabilities and follow the same pattern as other product-related permissions in the file.
733-733: LGTM: Follow Store feature permissions are consistent.
The permission to view followed vendors aligns with the other follow store related permissions (follow/unfollow) defined in the same section.
292-292: Verify the impact of enabling store terms visibility.
The change enables customers to view store terms and conditions, which is good for transparency. However, ensure that vendors have the corresponding capability to set these terms.
✅ Verification successful
Verified: Vendors have the capability to set store terms and conditions
The codebase shows that vendors have full capability to set and manage store terms and conditions:
- The vendor settings form (
templates/settings/store-form.php) includes a dedicated section for terms and conditions with:- A toggle to enable/disable terms display
- A rich text editor to input detailed terms content
- The vendor class (
Vendor.php) includes methods to:- Get terms status via
toc_enabled() - Get terms content via
get_toc() - Set terms via
set_enable_tnc()andset_store_tnc()
- Get terms status via
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that vendors have the capability to set store terms
# and that the terms are properly displayed to customers
# Check if vendors have the capability to set terms and conditions
rg -l "vendor.*terms.*conditions"
# Check for any UI components that render store terms
rg -l "store[-_]terms"
Length of output: 229
Script:
#!/bin/bash
# Let's examine the specific implementations to verify the vendor's capability
# Check the vendor settings implementation
cat includes/Vendor/SettingsApi/Settings/Pages/Store.php
# Check the test implementations for more context
cat tests/pw/tests/e2e/vendorSettings.spec.ts
# Look for terms-related UI components or templates
rg -l "terms.*conditions" --type php
Length of output: 30671
Script:
#!/bin/bash
# Let's check the implementation of terms and conditions display
cat templates/settings/store-form.php
# Check vendor class implementation
cat includes/Vendor/Vendor.php
# Check if there are any frontend templates for terms display
rg -l "store.*terms" --type-add 'template:*.php' --type template
Length of output: 62915
tests/pw/pages/productsPage.ts (2)
1041-1060: LGTM! Well-structured implementation.
The method is well-implemented with proper error handling, assertions, and follows the established patterns in the codebase.
1339-1345: LGTM! Well-implemented negative test case.
The method effectively verifies that duplicate attributes cannot be added, with proper assertions and error handling.
tests/pw/utils/testData.ts (2)
1005-1005: LGTM! Improved naming clarity.
The property rename from 'vendors' to 'followingStores' better describes its purpose and maintains consistency with the feature's functionality.
1081-1081: LGTM! Well-structured URL template.
The auction product edit URL template follows the established pattern and correctly includes the necessary query parameters.
tests/pw/tests/e2e/productsDetailsAuction.spec.ts (36)
1-8: LGTM!
The import statements are correctly defined and follow the proper syntax.
12-37: LGTM!
The test suite setup looks good:
- The
beforeAllhook correctly initializes the vendor context, creates the necessary test products, and sets up the API utilities. - The
afterAllhook ensures proper cleanup by closing the vendor page and disposing of the API utilities.
43-46: LGTM!
The test for updating the auction product title is implemented correctly:
- It creates a new auction product using the API.
- It calls the
addProductTitlemethod with the correct parameters to update the product title.
50-52: LGTM!
The test for updating the auction product category (single) is implemented correctly:
- It calls the
addProductCategorymethod with the correct parameters to update the product category.
54-57: LGTM!
The test for adding multiple auction product categories is implemented correctly:
- It updates the
product_category_styleoption to 'multiple' using theupdateOptionValuemethod. - It calls the
addProductCategorymethod with the correct parameters to add multiple product categories.
59-64: LGTM!
The test for removing multiple auction product categories is implemented correctly:
- It updates the
product_category_styleoption to 'multiple' using theupdateOptionValuemethod. - It creates a new auction product with multiple categories using the API.
- It calls the
removeProductCategorymethod with the correct parameters to remove a specific category from the product.
66-69: LGTM!
The test for adding a multi-step auction product category (last category) is implemented correctly:
- It updates the
product_category_styleoption to 'single' using theupdateOptionValuemethod. - It calls the
addProductCategorymethod with the correct parameters to add the last category from the multi-step categories array.
71-74: LGTM!
The test for adding a multi-step auction product category (any category) is implemented correctly:
- It updates the
dokan_any_category_selectionoption to 'on' using theupdateOptionValuemethod. - It calls the
addProductCategorymethod with the correct parameters to add a specific category from the multi-step categories array.
76-79: LGTM!
The test for verifying that a vendor can't add a multi-step product category (any category) is implemented correctly:
- It updates the
dokan_any_category_selectionoption to 'off' using theupdateOptionValuemethod. - It calls the
cantAddCategorymethod with the correct parameters to verify that the vendor can't add a specific category from the multi-step categories array.
83-85: LGTM!
The test for adding auction product tags is implemented correctly:
- It calls the
addProductTagsmethod with the correct parameters to add tags to the product.
87-89: LGTM!
The test for removing auction product tags is implemented correctly:
- It calls the
removeProductTagsmethod with the correct parameters to remove tags from the product.
91-93: LGTM!
The test for creating auction product tags is implemented correctly:
- It calls the
addProductTagsmethod with the correct parameters to create new tags for the product.
97-99: LGTM!
The test for adding an auction product cover image is implemented correctly:
- It calls the
addProductCoverImagemethod with the correct parameters to add a cover image to the product.
115-117: LGTM!
The test for adding auction product gallery images is implemented correctly:
- It calls the
addProductGalleryImagesmethod with the correct parameters to add gallery images to the product.
133-135: LGTM!
The test for adding an auction product short description is implemented correctly:
- It calls the
addProductShortDescriptionmethod with the correct parameters to add a short description to the product.
137-139: LGTM!
The test for updating the auction product short description is implemented correctly:
- It calls the
addProductShortDescriptionmethod with the correct parameters to update the short description of the product.
141-143: LGTM!
The test for removing the auction product short description is implemented correctly:
- It calls the
addProductShortDescriptionmethod with an empty string to remove the short description of the product.
147-149: LGTM!
The test for updating the auction product description is implemented correctly:
- It calls the
addProductDescriptionmethod with the correct parameters to update the description of the product.
153-155: LGTM!
The test for adding auction product downloadable options is implemented correctly:
- It calls the
addProductDownloadableOptionsmethod with the correct parameters to add downloadable options to the product.
171-174: LGTM!
The test for adding a product virtual option is implemented correctly:
- It creates a new auction product using the API.
- It calls the
addProductVirtualOptionmethod with the correct parameters to add the virtual option to the product.
176-179: LGTM!
The test for removing a product virtual option is implemented correctly:
- It creates a new auction product with the virtual option set to true using the API.
- It calls the
addProductVirtualOptionmethod with the correct parameters to remove the virtual option from the product.
181-183: LGTM!
The test for updating product general options is implemented correctly:
- It calls the
addProductGeneralOptionmethod with the correct parameters to update the general options of the product.
187-189: LGTM!
The test for adding auction product inventory options (SKU) is implemented correctly:
- It calls the
addProductInventorymethod with the correct parameters to add inventory options to the product.
191-193: LGTM!
The test for updating auction product inventory options (SKU) is implemented correctly:
- It calls the
addProductInventorymethod with the correct parameters to update the inventory options of the product.
195-197: LGTM!
The test for removing auction product inventory options (SKU) is implemented correctly:
- It calls the
addProductInventorymethod with an empty string for the SKU to remove the inventory options from the product.
201-203: LGTM!
The test for adding auction product other options (product status) is implemented correctly:
- It calls the
addProductOtherOptionsmethod with the correct parameters to add other options (product status) to the product.
205-207: LGTM!
The test for adding auction product other options (visibility) is implemented correctly:
- It calls the
addProductOtherOptionsmethod with the correct parameters to add other options (visibility) to the product.
211-213: LGTM!
The test for adding auction product shipping is implemented correctly:
- It calls the
addProductShippingmethod with the correct parameters to add shipping options to the product.
215-217: LGTM!
The test for updating auction product shipping is implemented correctly:
- It calls the
addProductShippingmethod with the correct parameters to update the shipping options of the product.
219-221: LGTM!
The test for removing auction product shipping is implemented correctly:
- It calls the
removeProductShippingmethod to remove the shipping options from the product.
223-225: LGTM!
The test for adding auction product tax is implemented correctly:
- It calls the
addProductTaxmethod with the correct parameters to add tax options to the product.
234-236: LGTM!
The test for adding an auction product attribute is implemented correctly:
- It calls the
addProductAttributemethod with the correct parameters to add an attribute to the product.
238-240: LGTM!
The test for verifying that a vendor can't add an already added auction product attribute is implemented correctly:
- It calls the
cantAddAlreadyAddedAttributemethod with the correct parameters to verify that the vendor can't add an attribute that is already added to the product.
243-247: LGTM!
The test for creating an auction product attribute term is implemented correctly:
- It creates a new attribute and attribute term using the API.
- It creates a new auction product using the API.
- It calls the
addProductAttributemethod with the correct parameters to add the newly created attribute term to the product.
249-254: LGTM!
The test for removing an auction product attribute is implemented correctly:
- It creates a new attribute and attribute term using the API.
- It creates a new auction product with the attribute using the API.
- It calls the
removeProductAttributemethod with the correct parameters to remove the attribute from the product.
[approve
227-230: Verify the Dokan issue.
The test for adding auction product tax (with tax class) is skipped due to a Dokan issue. Ensure that the issue is tracked and resolved before enabling this test.
#!/bin/bash
# Description: Verify if the Dokan issue related to adding auction product tax with tax class is resolved.
# Test: Search for the issue in the Dokan repository. Expect: Issue should be resolved or have a clear resolution plan.
gh issue list --repo getdokan/dokan --search "auction product tax with tax class" --limit 1 --json title,state --jq '.[].state'tests/pw/pages/vendorAuctionsPage.ts (2)
88-90: Verify the Existence and Behavior of gotoUntilNetworkidle Method
In the goToAuctionProductEditById method, you're using gotoUntilNetworkidle, which may not be a standard method provided by Playwright or your base classes.
Ensure that gotoUntilNetworkidle is properly defined and behaves as expected. If it's intended to navigate to a URL and wait until the network is idle, confirm that it properly waits for all network requests to complete before proceeding.
If this method does not exist or is not reliable, consider using Playwright's built-in await page.goto(url, { waitUntil: 'networkidle' }); for consistent behavior.
114-114: Confirm Changes to Response Handling in clickAndWaitForResponseAndLoadState Calls
In the addAuctionProduct and editAuctionProduct methods, the explicit status code 302 has been removed from the clickAndWaitForResponseAndLoadState calls.
Ensure that the removal of the hardcoded status code does not affect the functionality of these methods. The method should still correctly handle the page navigation and response without specifying the status code if it's not necessary.
If the status code was important for ensuring the correct response, consider adding it back or implementing logic to handle different possible status codes.
Also applies to: 122-122
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (3)
tests/pw/pages/vendorAuctionsPage.ts (1)
488-493: Consider adding more comprehensive assertions for inventory management.The current implementation only verifies the SKU value. Consider adding assertions for other inventory-related fields and edge cases:
- Verify that the inventory management UI updates correctly
- Check for proper error messages when invalid values are entered
- Verify the interaction between inventory and other product settings
tests/pw/feature-map/feature-map.yml (1)
654-702: Consider standardizing auction product permission naming patterns.While the auction product management permissions are comprehensive, there are some inconsistencies in the naming patterns:
- Some permissions use "auction product" prefix while others use "product" (e.g., line 677-678)
- Some permissions are duplicates of regular product permissions
Consider applying these changes for consistency:
- vendor can add product virtual option: true - vendor can remove product virtual option: true + vendor can add auction product virtual option: true + vendor can remove auction product virtual option: trueAlso, consider consolidating duplicate permissions with regular product permissions to reduce maintenance overhead.
tests/pw/tests/e2e/productsDetailsAuction.spec.ts (1)
244-244: Address the TODO: Refactor the following testsThe comment indicates that the tests below need refactoring. Refactoring these tests could improve code organization, readability, and maintainability. Would you like assistance in refactoring these tests or creating a task to track this effort?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
tests/pw/feature-map/feature-map.yml(5 hunks)tests/pw/pages/selectors.ts(27 hunks)tests/pw/pages/vendorAuctionsPage.ts(4 hunks)tests/pw/tests/e2e/liveSearch.spec.ts(1 hunks)tests/pw/tests/e2e/productsDetailsAuction.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/pw/pages/selectors.ts
- tests/pw/tests/e2e/liveSearch.spec.ts
🔇 Additional comments (4)
tests/pw/pages/vendorAuctionsPage.ts (1)
234-237: Consider using a more robust approach for date input handling.
The current implementation removes the 'readonly' attribute to set dates, which could be fragile if the UI framework changes. Consider using the framework's built-in date picker or dispatching appropriate events instead.
tests/pw/feature-map/feature-map.yml (3)
152-153: LGTM: Virtual product options are properly configured.
The addition of virtual product options for vendors aligns well with the existing product management capabilities and follows the established permission pattern.
292-292: LGTM: Store terms visibility enhancement improves transparency.
Enabling customers to view store terms and conditions is a positive change that enhances transparency and helps with legal compliance.
782-782: LGTM: Follow Store feature properly configured.
Enabling customers to view followed vendors enhances the social proof aspect of the platform and improves vendor discovery.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
tests/pw/pages/productsPage.ts (2)
1042-1060: Consider adding error handling.The implementation is good, but consider adding error handling for edge cases:
- What happens if the save operation fails?
- What if the virtual checkbox is disabled or not found?
async addProductVirtualOption(productName: string, enable: boolean): Promise<void> { await this.goToProductEdit(productName); + try { if (enable) { + await this.toBeEnabled(productsVendor.virtual); await this.check(productsVendor.virtual); } else { + await this.toBeEnabled(productsVendor.virtual); await this.focus(productsVendor.virtual); await this.uncheck(productsVendor.virtual); } await this.saveProduct(); if (enable) { await this.toBeChecked(productsVendor.virtual); if (DOKAN_PRO) { await this.notToBeVisible(productsVendor.shipping.shippingContainer); } } else { await this.notToBeChecked(productsVendor.virtual); } + } catch (error) { + throw new Error(`Failed to ${enable ? 'enable' : 'disable'} virtual option: ${error.message}`); + } }
1340-1345: Enhance error messaging for better UX.Consider adding a descriptive error message when attempting to add a duplicate attribute.
async cantAddAlreadyAddedAttribute(productName: string, attributeName: string): Promise<void> { await this.goToProductEdit(productName); await this.toBeVisible(productsVendor.attribute.savedAttribute(attributeName)); await this.toHaveAttribute(productsVendor.attribute.disabledAttribute(attributeName), 'disabled', 'disabled'); + // Verify error message is displayed + await this.toContainText( + productsVendor.attribute.errorMessage, + `Attribute "${attributeName}" has already been added to this product` + ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
tests/pw/pages/productsPage.ts(6 hunks)tests/pw/pages/vendorAuctionsPage.ts(4 hunks)
🔇 Additional comments (3)
tests/pw/pages/vendorAuctionsPage.ts (1)
9-10: Skip comment generation.
A past review comment already addresses the need to handle undefined DOKAN_PRO environment variable.
tests/pw/pages/productsPage.ts (2)
571-573: Indentation issue already reported.
977-984: Well-structured refactoring!
Good job extracting the gallery image removal logic into a separate method. This improves code reusability and follows the DRY principle.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
tests/pw/tests/e2e/vendorDeliveryTime.spec.ts (1)
Line range hint
55-61: Address test dependencies and side effects.There are multiple issues in this test:
- Incorrect skip implementation similar to the previous test
- The test relies on side effects from previous tests and modifies global settings
This violates test isolation principles and can lead to flaky tests.
Consider these improvements:
- Fix the skip implementation as shown in the previous comment
- Move the settings modification to a
beforeEachhook to ensure a clean state- Add a cleanup in
afterEachto restore the original settingsExample refactor:
+ test.describe('customer store pickup tests', () => { + test.beforeEach(async () => { + // Set up clean state + await dbUtils.setOptionValue(dbData.dokan.optionName.deliveryTime, { + ...dbData.dokan.deliveryTimeSettings, + allow_vendor_override_settings: 'off' + }); + }); + + test.afterEach(async () => { + // Restore original settings + await dbUtils.setOptionValue(dbData.dokan.optionName.deliveryTime, + dbData.dokan.deliveryTimeSettings + ); + }); + + test.skip('customer can buy product with store pickup', { tag: ['@pro', '@customer'] }, async () => { + // TODO: Enable when chart & checkout block PR is merged await customer.addProductToCart(data.predefined.simpleProduct.product1.name, 'single-product'); await customer.placeOrderWithDeliverTimeStorePickup('store-pickup', data.deliveryTime); }); + });tests/pw/utils/testData.ts (1)
400-402: LGTM! Consider adding type annotations for better type safety.The new auction relisting properties are well-organized and follow consistent naming conventions. For better type safety and documentation, consider adding TypeScript interfaces to define the expected types.
interface AuctionConfig { relistIfFailAfterNHours: string; relistIfNotPaidAfterNHours: string; relistAuctionDurationInH: string; // ... other existing properties }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
tests/pw/feature-map/feature-map.yml(6 hunks)tests/pw/pages/selectors.ts(27 hunks)tests/pw/pages/vendorDeliveryTimePage.ts(1 hunks)tests/pw/tests/e2e/vendorDeliveryTime.spec.ts(1 hunks)tests/pw/utils/testData.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/pw/feature-map/feature-map.yml
- tests/pw/pages/selectors.ts
🔇 Additional comments (3)
tests/pw/pages/vendorDeliveryTimePage.ts (2)
105-105: Great type safety improvement!
The method signature now uses a literal union type 'delivery-time' | 'store-pickup' instead of string. This change:
- Prevents passing invalid delivery types
- Improves IDE autocompletion
- Makes the code more maintainable
112-112: Verify the API endpoint change.
The API endpoint has been updated from ajax to api.dokan.deliveryTime. While this change aligns with modern API practices, we should ensure this endpoint is properly configured and accessible.
Let's verify the API endpoint configuration:
Also applies to: 119-119
tests/pw/utils/testData.ts (1)
1005-1005: LGTM! Property rename improves clarity.
Renaming from 'vendors' to 'followingStores' better describes the property's purpose and maintains consistency with the related functionality.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
tests/pw/tests/e2e/vendorSettings.spec.ts (2)
9-9: Add type checking for environment variables.Consider adding type checking and default value handling for the environment variables to improve reliability.
-const { DOKAN_PRO, VENDOR_ID } = process.env; +const DOKAN_PRO = process.env.DOKAN_PRO === 'true'; +const VENDOR_ID = process.env.VENDOR_ID ?? ''; + +if (!VENDOR_ID) { + throw new Error('VENDOR_ID environment variable is required'); +}
Based on the gathered information, I can now provide the final response:
Skipped tests require implementation or documentation
The skipped tests for store banner and profile picture settings have corresponding implementation in
vendorSettingsPage.ts, indicating they are part of the intended functionality. However, they are currently skipped without any explanation. Since these tests are tagged with@liteand@vendor, they appear to be important for vendor functionality testing.
tests/pw/tests/e2e/vendorSettings.spec.ts: Implement or document the reason for skipping:
vendor can set store banner settingsvendor can set store profile picture settings🔗 Analysis chain
Line range hint
51-57: Address skipped tests for store banner and profile picture settings.Two tests are currently skipped:
- 'vendor can set store banner settings'
- 'vendor can set store profile picture settings'
Please either:
- Implement these tests if they are required
- Add a comment explaining why they are skipped
- Remove them if they are no longer needed
Let me check the implementation details in these files to better understand the context.
Let me check if there are any recent changes or issues related to these tests.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if these tests are implemented in other files rg -l "store banner settings|profile picture settings" tests/Length of output: 142
Script:
#!/bin/bash # Check the implementation details rg -A 5 "store banner settings|profile picture settings" tests/Length of output: 1560
Script:
#!/bin/bash # Check git history for these tests git log -p tests/pw/tests/e2e/vendorSettings.spec.ts | grep -A 10 "store banner settings\|profile picture settings"Length of output: 824
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
tests/pw/package.json(2 hunks)tests/pw/tests/e2e/vendorSettings.spec.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/pw/package.json
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
tests/pw/feature-map/feature-map.yml(6 hunks)tests/pw/pages/selectors.ts(27 hunks)tests/pw/tests/e2e/productsDetailsAuction.spec.ts(1 hunks)tests/pw/utils/interfaces.ts(1 hunks)tests/pw/utils/testData.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/pw/pages/selectors.ts
- tests/pw/tests/e2e/productsDetailsAuction.spec.ts
- tests/pw/utils/interfaces.ts
- tests/pw/utils/testData.ts
🔇 Additional comments (4)
tests/pw/feature-map/feature-map.yml (4)
152-153: LGTM: Virtual product options permissions are properly configured.
The new permissions for adding and removing virtual product options align well with existing vendor product management capabilities.
292-292: LGTM: Store terms and conditions visibility enabled for customers.
Enabling customer access to store terms and conditions improves transparency and follows e-commerce best practices.
655-698: LGTM: Comprehensive auction product management permissions.
The permissions provide vendors with a complete set of capabilities to manage auction products, including product details, categories, images, and attributes.
Also applies to: 700-703
727-728: LGTM: Enhanced delivery options for customers.
The enabled permissions for delivery time and store pickup provide customers with flexible delivery options.
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
Title
Detailed Description of the pull request. What was previous behaviour
and what will be changed in this PR.
Before Changes
Describe the issue before changes with screenshots(s).
After Changes
Describe the issue after changes with screenshot(s).
Feature Video (optional)
Link of detailed video if this PR is for a feature.
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests