-
Notifications
You must be signed in to change notification settings - Fork 204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add: add product form test(attribute, bulk discount, geolocation) #2395
Add: add product form test(attribute, bulk discount, geolocation) #2395
Conversation
WalkthroughThe pull request includes significant modifications to the feature map configuration and the Changes
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (6)
tests/pw/tests/e2e/productsDetails.spec.ts (4)
366-371
: Approve test, but address TODO commentThe test case for creating a product attribute term is comprehensive, involving multiple steps to set up the test scenario. However, there's a TODO comment suggesting that this test needs refactoring.
Consider addressing the TODO comment by refactoring this test. Perhaps you could extract the attribute and product creation into a separate setup method to improve readability and maintainability.
388-388
: Implement test for adding already added attributeThere's a TODO comment about testing the scenario where a vendor attempts to add an already added attribute. This is an important edge case that should be covered in your test suite.
Consider implementing this test case to improve the robustness of your attribute management tests. It will help ensure that the system properly handles duplicate attribute additions.
396-398
: Approve test, but consider clarifying method nameThe test case for updating product bulk discount options is structurally sound. However, it uses the same
addProductBulkDiscountOptions
method as the "add" test case.Consider renaming the
addProductBulkDiscountOptions
method tosetProductBulkDiscountOptions
or creating a separateupdateProductBulkDiscountOptions
method to clearly distinguish between adding and updating operations. This will improve the clarity and maintainability of your tests.
406-416
: LGTM: Comprehensive tests for product geolocation, with a suggestionThese test cases effectively cover the CRUD operations for product geolocation. They are well-structured, correctly tagged, and follow the established pattern in the file.
For the update test (lines 410-412), consider renaming the
addProductGeolocation
method tosetProductGeolocation
or creating a separateupdateProductGeolocation
method to clearly distinguish between adding and updating operations. This will improve the clarity and maintainability of your tests.tests/pw/pages/productsPage.ts (2)
1326-1336
: Consider consolidating redundant steps inremoveProductAttributeTerm
.After saving the product, clicking on the saved attribute and checking for the term's non-existence (lines 1334-1335) might be redundant since you've already verified it's not visible before saving. You may consider removing these lines to streamline the code.
Apply this diff to remove redundant steps:
- await this.saveProduct(); - await this.click(productsVendor.attribute.savedAttribute(attribute)); - await this.notToBeVisible(productsVendor.attribute.selectedAttributeTerm(attributeTerm)); + await this.saveProduct();
1351-1356
: Consistency in method naming: Update method comment inremoveProductBulkDiscountOptions
.The comment above the
removeProductBulkDiscountOptions
method incorrectly states "// add product discount options". Updating the comment to reflect the correct action improves code readability.Apply this diff to correct the comment:
- // add product discount options + // remove product bulk discount options
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- tests/pw/feature-map/feature-map.yml (1 hunks)
- tests/pw/pages/productsPage.ts (1 hunks)
- tests/pw/tests/e2e/productsDetails.spec.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (11)
tests/pw/tests/e2e/productsDetails.spec.ts (6)
362-364
: LGTM: New test for adding product attributeThe test case for adding a product attribute is well-structured and follows the existing pattern in the file. It's correctly tagged and named, which will help in test organization and execution.
373-378
: LGTM: Comprehensive test for removing product attributeThis test case effectively covers the removal of a product attribute. It properly sets up the test scenario using API calls and then tests the removal functionality. The test is well-structured and clearly named.
380-386
: LGTM: Detailed test for removing product attribute termThis test case thoroughly covers the removal of a specific product attribute term. It sets up a complex scenario with multiple attribute terms and tests the precise removal of one term. The test is well-structured and named appropriately.
392-394
: LGTM: New test for adding product bulk discount optionsThe test case for adding product bulk discount options is well-structured and follows the existing pattern in the file. It's correctly tagged and named, which will help in test organization and execution.
400-402
: LGTM: Test for removing product bulk discount optionsThis test case effectively covers the removal of product bulk discount options, completing the CRUD operations for this feature. It follows the established pattern in the file and is correctly tagged and named.
359-416
: Overall assessment: Valuable additions to the test suiteThe new tests for product attributes, bulk discount options, and geolocation are well-implemented and significantly enhance the test coverage for these important product features. They follow the established patterns in the file and are correctly tagged for proper categorization.
A few minor suggestions have been made to improve clarity and maintainability:
- Address the TODO comment about refactoring the 'vendor can create product attribute term' test.
- Implement the missing test case for adding an already added attribute.
- Consider renaming some methods to clearly distinguish between add and update operations.
These changes will further improve the robustness and readability of the test suite.
tests/pw/feature-map/feature-map.yml (1)
180-189
: Excellent additions to vendor capabilities!These new permissions significantly enhance the vendor's ability to manage products:
- Product attributes management (lines 180-183)
- Bulk discount options (lines 184-186)
- Product geolocation features (lines 187-189)
These additions align well with the PR objectives and provide vendors with more control over their product listings, potentially improving their ability to create more detailed and attractive product offerings.
tests/pw/pages/productsPage.ts (4)
1316-1323
:removeProductAttribute
method looks good.The method correctly removes the specified attribute from the product and ensures it is no longer visible.
1339-1348
:addProductBulkDiscountOptions
method is well-implemented.The method correctly adds bulk discount options to the product and verifies the values.
1361-1370
: Add verification for location selection inaddProductGeolocation
.After entering the location and simulating the arrow down and enter keys, it's important to verify that the intended location is correctly selected. Consider adding a verification step to ensure that the geolocation matches the expected value.
Modify the code to verify the selected location:
await this.press(data.key.enter); + await this.toHaveValue(productsVendor.geolocation.productLocation, location); await this.saveProduct();
1373-1378
:removeProductGeolocation
method functions correctly.The method appropriately reverts the geolocation to "same as store" by checking the option and verifying it.
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