Add: add product form tests (download options, inventory) #2389
Conversation
WalkthroughThe pull request introduces significant changes to the feature map configuration and related functionalities for managing vendor capabilities in product management. Key modifications include adjustments to permissions for various user roles across multiple pages, such as disabling certain admin capabilities and enhancing vendor functionalities. New methods for handling downloadable options and inventory management have been added 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: 7
🧹 Outside diff range and nitpick comments (13)
tests/pw/tests/e2e/vendorSettings.spec.ts (3)
9-10: Consider adding a default value or validation for VENDOR_IDWhile it's good practice to use environment variables, it's important to handle cases where the variable might not be set. Consider adding a default value or validation to ensure the tests don't fail unexpectedly if
VENDOR_IDis not provided in the environment.You could modify the line as follows:
const { VENDOR_ID = 'default_vendor_id' } = process.env;Or add validation:
const { VENDOR_ID } = process.env; if (!VENDOR_ID) { throw new Error('VENDOR_ID environment variable is not set'); }
26-26: Approved: Good addition to cleanup processThe addition of setting user metadata in the
afterAllhook is a good practice for cleaning up after tests. It ensures that the RMA settings are reset to a known state after the tests run.For improved clarity, consider adding a comment explaining the purpose of this line:
// Reset RMA settings to default test data after all tests await dbUtils.setUserMeta(VENDOR_ID, '_dokan_rma_settings', dbData.testData.dokan.rmaSettings, true);
Line range hint
1-138: Summary of changes and suggestions for improvementThe changes in this file are focused on expanding the test coverage for vendor settings, particularly around RMA settings, store banner, and profile picture settings. Here's a summary of the review:
- The addition of the
VENDOR_IDenvironment variable is good, but consider adding a default value or validation.- The new line in the
afterAllhook for setting user metadata is a good practice for test cleanup.- Two new tests for store banner and profile picture settings have been added but are currently skipped. These should either be implemented, removed, or have TODO comments explaining their status.
To improve the overall quality and maintainability of the test suite:
- Ensure all environment variables are properly validated or have default values.
- Add comments to explain the purpose of cleanup operations in the
afterAllhook.- Implement or provide clear explanations for skipped tests.
- Consider grouping related tests (e.g., all store settings tests) using
test.describeblocks for better organization.These changes will enhance the robustness and clarity of the test suite, making it easier to maintain and extend in the future.
tests/pw/tests/e2e/productsDetails.spec.ts (7)
217-220: LGTM: Good test for adding downloadable options.This test case appropriately checks if a vendor can add downloadable options to a product. Using
productName1, which was created with only required fields, is a good approach to test this feature in isolation.Consider adding an assertion to verify that the downloadable options were successfully added. For example:
const updatedProduct = await vendor.getProductDetails(productName1); expect(updatedProduct.downloadableOptions).toEqual(data.product.productInfo.downloadableOptions);
221-225: Improve test setup for updating downloadable options.The test case for updating downloadable options is appropriate. However, the TODO comment suggests that the test setup might not be ideal.
Consider the following improvements:
- Create a product with downloadable options in the
beforeAllhook or at the beginning of this test.- Update the existing downloadable options instead of adding new ones.
- Add an assertion to verify that the options were successfully updated.
Example:
test('vendor can update product downloadable options', { tag: ['@lite', '@vendor'] }, async () => { const initialOptions = { ... }; // Initial downloadable options await vendor.addProductDownloadableOptions(productName, initialOptions); const updatedOptions = data.product.productInfo.downloadableOptions; await vendor.addProductDownloadableOptions(productName, updatedOptions); const updatedProduct = await vendor.getProductDetails(productName); expect(updatedProduct.downloadableOptions).toEqual(updatedOptions); });
226-231: Good test case, but needs verification step.The test case for removing a downloadable file is well-structured. Adding the downloadable options before removal is a good workaround for the TODO comment.
Consider the following improvements:
- Add an assertion to verify that the downloadable file was actually removed.
- Use a separate variable for the updated options to improve readability.
Example:
test('vendor can remove product downloadable file', { tag: ['@lite', '@vendor'] }, async () => { await vendor.addProductDownloadableOptions(productName, data.product.productInfo.downloadableOptions); const updatedOptions = { ...data.product.productInfo.downloadableOptions, downloadLimit: '', downloadExpiry: '' }; await vendor.removeDownloadableFile(productName, updatedOptions); const updatedProduct = await vendor.getProductDetails(productName); expect(updatedProduct.downloadableOptions.files).toHaveLength(0); });
234-244: Good SKU tests, but consider improving consistency.The test cases for adding, updating, and removing SKU are appropriate. However, using different products for different operations might lead to inconsistent test states.
Consider the following improvements:
- Use the same product for all SKU operations to ensure consistency.
- Add assertions to verify the SKU changes after each operation.
Example:
test('vendor can manage product inventory options (SKU)', { tag: ['@lite', '@vendor'] }, async () => { const product = productName1; const sku = data.product.productInfo.inventory().sku; // Add SKU await vendor.addProductInventory(product, { sku }, 'sku'); let updatedProduct = await vendor.getProductDetails(product); expect(updatedProduct.sku).toBe(sku); // Update SKU const newSku = 'updated-' + sku; await vendor.addProductInventory(product, { sku: newSku }, 'sku'); updatedProduct = await vendor.getProductDetails(product); expect(updatedProduct.sku).toBe(newSku); // Remove SKU await vendor.addProductInventory(product, { sku: '' }, 'sku'); updatedProduct = await vendor.getProductDetails(product); expect(updatedProduct.sku).toBe(''); });This approach combines the three tests into one, ensuring consistency and reducing test execution time.
246-252: LGTM: Good tests for stock status and stock management.The test cases for adding stock status and stock management options are well-structured and use the same product (
productName1) for consistency.Consider adding assertions to verify that the stock status and stock management options were successfully added. For example:
test('vendor can add product inventory options (stock status)', { tag: ['@lite', '@vendor'] }, async () => { const inventoryOptions = data.product.productInfo.inventory(); await vendor.addProductInventory(productName1, inventoryOptions, 'stock-status'); const updatedProduct = await vendor.getProductDetails(productName1); expect(updatedProduct.stockStatus).toBe(inventoryOptions.stockStatus); }); test('vendor can add product inventory options (stock management)', { tag: ['@lite', '@vendor'] }, async () => { const inventoryOptions = data.product.productInfo.inventory(); await vendor.addProductInventory(productName1, inventoryOptions, 'stock-management'); const updatedProduct = await vendor.getProductDetails(productName1); expect(updatedProduct.manageStock).toBe(inventoryOptions.manageStock); expect(updatedProduct.stockQuantity).toBe(inventoryOptions.stockQuantity); });
254-260: Good tests, but improve consistency and add verifications.The test cases for updating and removing stock management options are appropriate. However, using different products for these operations might lead to inconsistent test states.
Consider the following improvements:
- Use the same product for both update and remove operations.
- Add assertions to verify the changes after each operation.
- Combine update and remove tests into a single test for better flow and consistency.
Example:
test('vendor can update and remove product inventory options (stock management)', { tag: ['@lite', '@vendor'] }, async () => { const product = productName1; const initialOptions = data.product.productInfo.inventory(); // Update stock management await vendor.addProductInventory(product, initialOptions, 'stock-management'); let updatedProduct = await vendor.getProductDetails(product); expect(updatedProduct.manageStock).toBe(initialOptions.manageStock); expect(updatedProduct.stockQuantity).toBe(initialOptions.stockQuantity); // Remove stock management await vendor.removeProductInventory(product); updatedProduct = await vendor.getProductDetails(product); expect(updatedProduct.manageStock).toBe(false); expect(updatedProduct.stockQuantity).toBeNull(); });This approach ensures consistency, verifies changes, and provides a clear flow of operations in a single test.
262-268: Good tests for single quantity option, but improve consistency and add verifications.The test cases for adding and removing the "allow single quantity" option are appropriate. However, using different products for these operations might lead to inconsistent test states.
Consider the following improvements:
- Use the same product for both add and remove operations.
- Add assertions to verify the changes after each operation.
- Combine add and remove tests into a single test for better flow and consistency.
Example:
test('vendor can add and remove product inventory options (allow single quantity)', { tag: ['@lite', '@vendor'] }, async () => { const product = productName1; // Add single quantity option const addOptions = data.product.productInfo.inventory(); await vendor.addProductInventory(product, addOptions, 'one-quantity'); let updatedProduct = await vendor.getProductDetails(product); expect(updatedProduct.soldIndividually).toBe(true); // Remove single quantity option const removeOptions = { ...addOptions, oneQuantity: false }; await vendor.addProductInventory(product, removeOptions, 'one-quantity'); updatedProduct = await vendor.getProductDetails(product); expect(updatedProduct.soldIndividually).toBe(false); });This approach ensures consistency, verifies changes, and provides a clear flow of operations in a single test.
tests/pw/feature-map/feature-map.yml (2)
Line range hint
1037-1038: Approved: Improved log filtering for admins.The addition of filtering capabilities for logs by store name and order status is a valuable improvement for admins. This feature will:
- Enhance the ability to troubleshoot issues specific to certain stores
- Allow for better tracking of order statuses across the marketplace
- Improve overall marketplace management efficiency
Consider adding a date range filter in the future to further enhance the log analysis capabilities.
To implement a date range filter, you could add the following line:
admin can filter all logs by date range: true
Line range hint
1186-1188: Approved: Enhanced store review system for customers.The addition of store review capabilities for customers is a great improvement. This feature:
- Allows customers to provide feedback on their shopping experiences
- Enables customers to edit their reviews, promoting accuracy
- Lets customers view their own reviews, enhancing transparency
To further improve this system, consider implementing a verification system to ensure reviews are from confirmed buyers.
To implement a verified buyer review system, you could add the following line:
customer can leave verified buyer review: trueAdditionally, to ensure the integrity of the review system, it might be beneficial to add a cooldown period for editing reviews. This can be implemented by adding:
customer can edit store review within cooldown period: truetests/pw/pages/productsPage.ts (1)
1096-1101: Confirm inventory is disabled after removalAfter unchecking
enableStockManagementin theremoveProductInventorymethod, it might be beneficial to verify that inventory fields related to stock management are no longer visible or are reset to default values.Consider adding assertions to confirm inventory fields are reset:
await this.notToBeChecked(productsVendor.inventory.enableStockManagement); + await this.toBeVisible(productsVendor.inventory.stockStatus); + await this.toHaveSelectedValue(productsVendor.inventory.stockStatus, 'instock'); + // Additional assertions if necessary
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- tests/pw/feature-map/feature-map.yml (1 hunks)
- tests/pw/pages/productsPage.ts (2 hunks)
- tests/pw/tests/e2e/productsDetails.spec.ts (1 hunks)
- tests/pw/tests/e2e/vendorSettings.spec.ts (3 hunks)
🔇 Additional comments (6)
tests/pw/tests/e2e/productsDetails.spec.ts (2)
215-216: LGTM: Clear section comment.The comment clearly indicates the start of downloadable options tests, which helps in organizing and understanding the test suite structure.
232-233: LGTM: Clear section comment.The comment clearly indicates the start of inventory options tests, which helps in organizing and understanding the test suite structure.
tests/pw/feature-map/feature-map.yml (3)
149-160: LGTM! Enhanced vendor capabilities for product management.These additions significantly improve vendor control over product options, particularly for downloadable products and inventory management. The new features allow vendors to:
- Manage downloadable product options
- Handle inventory options including SKU and stock management
- Control single quantity allowance for products
These enhancements will likely lead to more flexible and detailed product listings, benefiting both vendors and customers.
Line range hint
1-1188: Overall assessment: Significant improvements to marketplace functionality.The changes in this feature map file represent a substantial enhancement to the Dokan marketplace system:
- Vendor capabilities have been expanded, particularly in product management and refund handling.
- Customer engagement has been improved through the enhanced store review system.
- Admin tools have been strengthened with better log filtering options.
These changes collectively contribute to a more robust, user-friendly, and efficient marketplace ecosystem. They balance the needs of vendors, customers, and administrators, which should lead to improved satisfaction for all stakeholders.
To ensure these features are properly implemented and tested, consider creating comprehensive test cases that cover all new functionalities.
To verify the implementation of these new features, please run the following script:
#!/bin/bash # Search for new feature implementations in the codebase rg --type php 'function.*(downloadable|inventory|refund|filter_logs|store_review)' -g '!tests/*'
Line range hint
1026-1027: Approved: Enhanced refund capabilities for vendors.The addition of full and partial refund capabilities for vendors is a significant improvement. This feature:
- Gives vendors more flexibility in handling customer issues
- Can potentially improve customer satisfaction
- Aligns with common e-commerce practices
Ensure that there are appropriate safeguards and monitoring in place to prevent misuse of this feature.
To verify the implementation of these refund capabilities, please run the following script:
tests/pw/pages/productsPage.ts (1)
922-922: Ensure consistent focus management after removing tagsThe addition of
await this.press('Escape');helps shift the focus away from the removed tag element. This can prevent potential issues where the focus remains on a non-existent element, which might cause unintended side effects in the UI interaction.
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