Auction tests #2452
Auction tests #2452
Conversation
WalkthroughThe changes in this pull request primarily involve updates to permissions and functionalities for vendors across various pages, as defined in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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: 1
🧹 Outside diff range and nitpick comments (8)
tests/pw/tests/e2e/vendorAuction.spec.ts (2)
Line range hint
15-29: Improve test isolation by reducing shared stateThe beforeAll hook creates shared state that could lead to test interdependencies:
- The shared
auctionProductNamecould cause flaky tests if modified- The
customer.bidAuctionProductcall might not be relevant for all testsConsider moving product creation and bidding to individual tests where needed:
test.beforeAll(async ({ browser }) => { const adminContext = await browser.newContext(data.auth.adminAuth); aPage = await adminContext.newPage(); admin = new AuctionsPage(aPage); const vendorContext = await browser.newContext(data.auth.vendorAuth); vPage = await vendorContext.newPage(); vendor = new AuctionsPage(vPage); const customerContext = await browser.newContext(data.auth.customerAuth); cPage = await customerContext.newPage(); customer = new AuctionsPage(cPage); apiUtils = new ApiUtils(await request.newContext()); - [, , auctionProductName] = await apiUtils.createProduct(payloads.createAuctionProduct(), payloads.vendorAuth); - await customer.bidAuctionProduct(auctionProductName); });
Line range hint
93-98: Track the skipped "buy it now" test issueThe skipped test indicates a limitation in the API that should be tracked and addressed.
Would you like me to create a GitHub issue to track the API limitation for saving "buy it now" price? The issue would include:
- Description of the current limitation
- Impact on testing coverage
- Proposed solution for the API
tests/pw/tests/e2e/commission.spec.ts (3)
9-9: Consider using a centralized test configurationInstead of directly accessing process.env, consider moving all test configuration including PRODUCT_ID to a dedicated config file. This would make test configuration more maintainable and easier to track.
-const { PRODUCT_ID } = process.env; +import { testConfig } from '@utils/testConfig'; +const { PRODUCT_ID } = testConfig;
Line range hint
85-98: Implement missing subscription product creation functionalityThe subscription product tests are skipped due to missing
createDokanSubscriptionProductfunction. This significantly reduces test coverage for subscription-related features.Would you like me to help create the missing
createDokanSubscriptionProductfunction implementation?
Line range hint
108-109: Create tickets for missing test scenariosThe TODO comments indicate several untested commission-related scenarios for both admin and vendor perspectives. These are critical paths that should be tested:
- Commission visibility in product list
- Commission visibility in order list
- Commission visibility in order details
- Sub-order commission details
- Vendor earning visibility
Would you like me to help create GitHub issues to track these missing test scenarios?
tests/pw/pages/vendorAuctionsPage.ts (2)
154-156: Consider extracting row actions visibility logic to a shared helper method.The pattern of removing the class attribute to handle row actions visibility is repeated across multiple methods. Consider extracting this to a shared helper method like
ensureRowActionsVisible(productName: string)to improve maintainability.+ private async ensureRowActionsVisible(productName: string): Promise<void> { + await this.removeAttribute(auctionProductsVendor.rowActions(productName), 'class'); + await this.hover(auctionProductsVendor.productCell(productName)); + } async duplicateAuctionProduct(productName: string) { await this.searchAuctionProduct(productName); - await this.removeAttribute(auctionProductsVendor.rowActions(productName), 'class'); - await this.hover(auctionProductsVendor.productCell(productName)); + await this.ensureRowActionsVisible(productName); await this.clickAndWaitForResponseAndLoadState(data.subUrls.frontend.vDashboard.auction, auctionProductsVendor.duplicate(productName), 302); await this.toBeVisible(auctionProductsVendor.duplicateSuccessMessage); await this.toBeVisible(auctionProductsVendor.productCell(productName + ' (Copy)')); }
152-152: Add JSDoc documentation for the new method.Consider adding JSDoc documentation to improve code maintainability and IDE support:
+ /** + * Duplicates an auction product and verifies the operation's success. + * @param productName - The name of the product to duplicate + * @throws Will throw an error if the product is not found or duplication fails + */ async duplicateAuctionProduct(productName: string) {tests/pw/pages/selectors.ts (1)
5711-5712: LGTM with a minor suggestion for the success message selector.The selectors for auction product duplication functionality look good and follow the existing patterns. However, consider making the success message selector more specific to avoid potential conflicts.
Consider updating the success message selector to be more specific:
- duplicateSuccessMessage: '//div[contains(@class,"dokan-alert dokan-alert-success")]//strong[normalize-space(text())="Product successfully duplicated"]', + duplicateSuccessMessage: '//div[contains(@class,"dokan-alert dokan-alert-success") and contains(.,"Product successfully duplicated")]//strong[normalize-space(text())="Product successfully duplicated"]',This makes the selector more robust by checking both the container class and message content.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
tests/pw/feature-map/feature-map.yml(2 hunks)tests/pw/pages/selectors.ts(1 hunks)tests/pw/pages/vendorAuctionsPage.ts(1 hunks)tests/pw/tests/e2e/commission.spec.ts(4 hunks)tests/pw/tests/e2e/vendorAuction.spec.ts(1 hunks)
🔇 Additional comments (5)
tests/pw/tests/e2e/commission.spec.ts (2)
Line range hint 27-31: Remove or refactor commented code blocks
The commented code related to DOKAN_PRO functionality should either be:
- Removed if the functionality is no longer needed
- Refactored into a proper conditional implementation
- Moved to a separate test file for pro features
This will improve code maintainability and prevent confusion.
Also applies to: 39-42
52-54:
Address systematically skipped category-based commission tests
Multiple category-based commission tests are skipped across different contexts (setup wizard, selling settings, vendor). This suggests a potential issue with the category-based commission feature implementation.
Please clarify:
- Are these tests temporarily skipped due to known issues?
- Is the category-based commission feature fully implemented?
Also applies to: 64-66, 76-78
✅ Verification successful
Let me try a different approach to gather more context about the category-based commission tests and implementation.
Based on the search results, I can see that the category-based commission feature is fully implemented across the codebase with:
-
Frontend components (Vue.js):
- CategoryBasedCommission.vue component
- Commission settings in admin and setup wizard
- Category commission fields and validation
-
Backend implementation (PHP):
- Commission strategies including CategoryBased class
- Commission settings and models
- Category commission calculations and upgrades
-
Tests:
- Unit tests for category commission functionality
- E2E tests for category commission setup
The skipped tests appear to be temporary while verifying the implementation works correctly, rather than indicating missing functionality.
Tests can be unskipped after verifying category commission setup works as expected
The category-based commission feature is fully implemented but the tests are temporarily skipped to validate the implementation. You can unskip these tests once you've manually verified that:
- Category commission setup works correctly in the Dokan setup wizard
- Commission calculations are accurate for category-based rules
- Changes are properly saved and applied
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for category-based commission implementation
ast-grep --pattern 'specificCategory'
Length of output: 37
Script:
#!/bin/bash
# Search for category-based commission related tests and implementation
rg -i "category.*commission" -A 3
# Also check for specific test files related to commission
fd "commission.*\.(ts|js|php)" --exec cat {}
Length of output: 131124
tests/pw/pages/vendorAuctionsPage.ts (1)
152-160: LGTM! The implementation follows best practices.
The method is well-structured and includes proper verification steps:
- Searches for the product
- Handles potential UI flakiness
- Waits for response and verifies success
- Confirms the duplicated product exists
tests/pw/feature-map/feature-map.yml (2)
687-687: LGTM: Vendor permission for auction product duplication.
The permission flag is correctly configured to allow vendors to duplicate auction products.
763-784:
Review admin and vendor permission alignment.
There appears to be an inconsistency in the permission structure:
- Admin permissions (lines 763-771) are all set to
false - Vendor permissions have mixed values with some set to
true(lines 775-776)
This could create functionality gaps where vendors have permissions that admins don't have. Consider aligning the admin permissions with the vendor permissions.
| test('vendor can duplicate auction product', { tag: ['@pro', '@vendor'] }, async () => { | ||
| const [, , auctionProductName] = await apiUtils.createProduct(payloads.createAuctionProduct(), payloads.vendorAuth); | ||
| await vendor.duplicateAuctionProduct(auctionProductName); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance test verification and cleanup
The test should verify the duplicated product's existence and properties, and ensure proper cleanup. Consider:
- Verifying the duplicated product's attributes match the original
- Adding cleanup for both original and duplicated products
test('vendor can duplicate auction product', { tag: ['@pro', '@vendor'] }, async () => {
const [, , auctionProductName] = await apiUtils.createProduct(payloads.createAuctionProduct(), payloads.vendorAuth);
- await vendor.duplicateAuctionProduct(auctionProductName);
+ const duplicatedProductName = await vendor.duplicateAuctionProduct(auctionProductName);
+ // Verify the duplicated product
+ await vendor.verifyDuplicatedProduct(auctionProductName, duplicatedProductName);
+ // Cleanup
+ await apiUtils.deleteProduct(duplicatedProductName, payloads.vendorAuth);
+ await apiUtils.deleteProduct(auctionProductName, payloads.vendorAuth);
});Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
tests/pw/pages/commissionPage.ts (2)
33-44: Extract wait logic into a reusable methodMultiple instances of fixed waits after form interactions indicate a systemic timing issue.
Consider creating a helper method to handle form interactions consistently:
private async setCommissionValue(selector: string, value: string) { await this.clearAndType(selector, value); await this.page.waitForFunction( ([sel, val]) => { const input = document.querySelector(sel) as HTMLInputElement; return input && input.value === val; }, [selector, value] ); }This would replace the current pattern:
-await this.clearAndType(setupWizardAdmin.categoryPercentage(...), commission.commissionPercentage); -await this.wait(1); +await this.setCommissionValue(setupWizardAdmin.categoryPercentage(...), commission.commissionPercentage);
Line range hint
1-190: Consider implementing a robust test infrastructureThe multiple timing adjustments throughout the file suggest a need for a more systematic approach to handling asynchronous operations in tests.
Consider implementing:
- A custom test fixture that provides standardized waiting and interaction methods
- Page object methods that encapsulate common patterns of form interaction
- Utility functions for handling common timing patterns
- Logging and debugging helpers to identify timing issues
This would help:
- Reduce test flakiness
- Improve maintenance
- Make tests more reliable and faster
- Provide better debugging information
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
tests/pw/pages/commissionPage.ts(3 hunks)tests/pw/pages/selectors.ts(2 hunks)tests/pw/tests/e2e/commission.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/commission.spec.ts
🔇 Additional comments (1)
tests/pw/pages/commissionPage.ts (1)
127-127: LGTM! Good improvement in navigation reliability
Switching to gotoUntilNetworkidle ensures the page is fully loaded before proceeding with interactions, which is a more reliable approach than simple navigation.
Also applies to: 144-144
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
Tests