-
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 tests (gallery, description) #2385
Add: add product form tests (gallery, description) #2385
Conversation
WalkthroughThe changes in this pull request introduce significant enhancements to the functionality available for vendors and customers across multiple pages. Key updates include the ability for vendors to manage product images and descriptions on the "Products" page, register with address fields on the "Vendor" page, and manage order statuses and payment methods on the "Orders" and "Payments" pages. Additionally, the "Store Support" page has been improved for better support ticket management, and new compliance features have been added for both vendors and customers. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 0
🧹 Outside diff range and nitpick comments (12)
tests/pw/tests/e2e/productsDetails.spec.ts (4)
159-175
: Improve test data setup for cover image testsThe cover image tests look good overall, but there are a few areas for improvement:
The TODO comments indicate that some tests require products with existing cover images. Consider setting up these products in the
beforeAll
hook to ensure proper test isolation.The update and remove tests are currently using the same product (
productName
). For better test isolation, consider creating separate products for each test case.The
addProductCoverImage
method is called twice in the update test. It might be more clear to rename the second call to something likeupdateProductCoverImage
for better readability.Here's a suggested refactor for the
beforeAll
hook to set up the required test data:test.beforeAll(async ({ browser }) => { // ... existing setup code ... // Set up products for cover image tests const [, , productWithCoverImage] = await apiUtils.createProductWc({ ...payloads.createProductAllFields(), images: [{ src: 'path/to/cover/image.jpg' }] }, payloads.vendorAuth); // ... rest of the setup code ... });Then update the test cases to use the appropriate products:
test('vendor can update product cover image', { tag: ['@lite', '@vendor'] }, async () => { await vendor.updateProductCoverImage(productWithCoverImage, data.product.productInfo.images.cover); }); test('vendor can remove product cover image', { tag: ['@lite', '@vendor'] }, async () => { await vendor.removeProductCoverImage(productWithCoverImage); });
177-193
: Improve test data setup for gallery image testsThe gallery image tests are structured well, but there are similar improvements to be made as with the cover image tests:
Address the TODO comments by setting up products with existing gallery images in the
beforeAll
hook.Create separate products for each test case to ensure better test isolation.
Consider renaming the
addProductGalleryImages
method in the update test toupdateProductGalleryImages
for clarity.Here's a suggested refactor for the
beforeAll
hook to set up the required test data:test.beforeAll(async ({ browser }) => { // ... existing setup code ... // Set up products for gallery image tests const [, , productWithGalleryImages] = await apiUtils.createProductWc({ ...payloads.createProductAllFields(), images: [ { src: 'path/to/gallery/image1.jpg' }, { src: 'path/to/gallery/image2.jpg' } ] }, payloads.vendorAuth); // ... rest of the setup code ... });Then update the test cases to use the appropriate products:
test('vendor can update product gallery image', { tag: ['@lite', '@vendor'] }, async () => { await vendor.updateProductGalleryImages(productWithGalleryImages, data.product.productInfo.images.gallery); }); test('vendor can remove product gallery image', { tag: ['@lite', '@vendor'] }, async () => { await vendor.removeProductGalleryImages(productWithGalleryImages); });
195-207
: Consider using a dedicated method for removing short descriptionThe short description tests look good overall. They follow a consistent structure and use different products for adding and updating/removing, which is good for test isolation.
However, for the remove test, using an empty string to remove the description might not be the most explicit way to represent removal.
Consider creating a dedicated method in the
ProductsPage
class for removing the short description. This would make the test more explicit and easier to understand:test('vendor can remove product short description', { tag: ['@lite', '@vendor'] }, async () => { await vendor.removeProductShortDescription(productName); });Then in the
ProductsPage
class:async removeProductShortDescription(productName: string) { // Implementation to remove the short description // This could involve clearing the field or using a specific UI action for removal }This approach makes the intention of the test clearer and separates the concept of "removing" from "setting to an empty string".
209-213
: Add tests for adding and removing product descriptionsThe test for updating the product description looks good. However, to ensure complete coverage of the product description functionality, consider adding tests for adding and removing product descriptions as well.
Here are suggestions for additional tests:
test('vendor can add product description', { tag: ['@lite', '@vendor'] }, async () => { await vendor.addProductDescription(productName1, data.product.productInfo.description.description); }); test('vendor can remove product description', { tag: ['@lite', '@vendor'] }, async () => { await vendor.removeProductDescription(productName); });Implement the
removeProductDescription
method in theProductsPage
class similar to the suggestion for the short description:async removeProductDescription(productName: string) { // Implementation to remove the product description // This could involve clearing the field or using a specific UI action for removal }These additional tests will provide more comprehensive coverage of the product description functionality.
tests/pw/feature-map/feature-map.yml (2)
Line range hint
1-1037
: Suggestion: Conduct a broader review of feature dependenciesWhile the changes are localized to the "Products" page for vendor capabilities, it's important to ensure that these new features are well-integrated with the rest of the system. Consider reviewing the following areas:
- Admin capabilities: Ensure that admins have corresponding capabilities to manage or oversee these new vendor features.
- Customer-facing features: Check if any updates are needed for how customers view or interact with these new product elements.
- API endpoints: If applicable, verify that API endpoints are updated to support these new features.
- Documentation: Ensure that user documentation and API documentation are updated to reflect these new capabilities.
To assist with this broader review, you may want to run the following script to identify potential areas that might need attention:
#!/bin/bash # Description: Identify potential areas that might need updates based on the new features # Check for related features in other sections echo "Checking for related features in other sections:" rg --type yaml "\b(cover image|gallery image|short description|description)\b" | grep -v "vendor can" # Check for potential API-related entries echo "Checking for potential API-related entries:" rg --type yaml "\bAPI\b" # Check for documentation-related entries echo "Checking for documentation-related entries:" rg --type yaml "\bdocumentation\b"
Line range hint
1-1037
: Overall assessment: Changes look good, broader integration review recommendedThe additions to vendor capabilities for product management are well-defined and enhance the functionality of the system. The changes themselves look good and are approved. However, to ensure a smooth integration of these new features, a broader review of the system's dependencies and related functionalities is recommended.
tests/pw/pages/productsPage.ts (6)
931-945
: LGTM! Consider adding error handling.The
addProductCoverImage
function is well-structured and follows a logical flow. It correctly handles the option to remove a previous cover image before adding a new one. The use of regex to verify the presence of a non-empty src attribute is a good approach.Consider adding error handling to manage potential failures in image upload or saving process. For example:
try { await this.uploadMedia(coverImage); await this.saveProduct(); // Verification steps... } catch (error) { console.error('Failed to add product cover image:', error); // Handle the error appropriately }
947-955
: LGTM! Consider adding error handling.The
removeProductCoverImage
function is well-implemented and follows a logical sequence of actions. The verification step correctly checks for an empty src attribute after removal.Similar to the previous function, consider adding error handling to manage potential failures in the image removal or saving process. For example:
try { await this.click(productsVendor.image.removeFeatureImage); await this.saveProduct(); // Verification steps... } catch (error) { console.error('Failed to remove product cover image:', error); // Handle the error appropriately }
957-976
: LGTM! Consider adding error handling and optimizing multiple uploads.The
addProductGalleryImages
function is well-implemented, efficiently handling multiple image uploads and the option to remove previous images. The verification step correctly checks the count of uploaded images.
- Consider adding error handling to manage potential failures in image upload or saving process, especially important for multiple uploads:
try { for (const galleryImage of galleryImages) { await this.click(productsVendor.image.gallery); await this.uploadMedia(galleryImage); } await this.saveProduct(); // Verification step... } catch (error) { console.error('Failed to add product gallery images:', error); // Handle the error appropriately }
- For performance optimization, consider implementing batch uploads if the API supports it, rather than uploading images one by one:
async uploadMediaBatch(images: string[]): Promise<void> { // Implement batch upload logic here } // In the function: await this.uploadMediaBatch(galleryImages);This could significantly improve performance when uploading multiple images.
978-988
: LGTM! Consider adding error handling and enhancing functionality.The
removeProductGalleryImages
function efficiently removes all gallery images from a product. The verification step correctly checks for zero uploaded images after removal.
- Add error handling to manage potential failures in the image removal or saving process:
try { const imageCount = await this.getElementCount(productsVendor.image.uploadedGalleryImage); for (let i = 0; i < imageCount; i++) { await this.hover(productsVendor.image.galleryImageDiv); await this.click(productsVendor.image.removeGalleryImage); } await this.saveProduct(); // Verification step... } catch (error) { console.error('Failed to remove product gallery images:', error); // Handle the error appropriately }
- Consider enhancing the function to allow removal of specific images:
async removeProductGalleryImages(productName: string, imagesToRemove?: number[]): Promise<void> { await this.goToProductEdit(productName); const imageCount = await this.getElementCount(productsVendor.image.uploadedGalleryImage); if (imagesToRemove) { for (const index of imagesToRemove) { if (index < imageCount) { await this.hover(productsVendor.image.galleryImageDiv(index)); await this.click(productsVendor.image.removeGalleryImage(index)); } } } else { // Existing logic to remove all images } await this.saveProduct(); await this.toHaveCount(productsVendor.image.uploadedGalleryImage, imagesToRemove ? imageCount - imagesToRemove.length : 0); }This enhancement would provide more flexibility in managing gallery images.
990-996
: LGTM! Consider adding error handling.The
addProductShortDescription
function is well-implemented, correctly interacting with the description iframe and verifying the added content.Consider adding error handling to manage potential failures in the description update or saving process:
async addProductShortDescription(productName: string, shortDescription: string): Promise<void> { try { await this.goToProductEdit(productName); await this.typeFrameSelector(productsVendor.shortDescription.shortDescriptionIframe, productsVendor.shortDescription.shortDescriptionHtmlBody, shortDescription); await this.saveProduct(); await this.toContainTextFrameLocator(productsVendor.shortDescription.shortDescriptionIframe, productsVendor.shortDescription.shortDescriptionHtmlBody, shortDescription); } catch (error) { console.error('Failed to add product short description:', error); // Handle the error appropriately } }This will help in catching and handling any unexpected errors during the process.
998-1004
: LGTM! Consider adding error handling and refactoring.The
addProductDescription
function is well-implemented, correctly interacting with the description iframe and verifying the added content.
- Add error handling to manage potential failures:
async addProductDescription(productName: string, description: string): Promise<void> { try { await this.goToProductEdit(productName); await this.typeFrameSelector(productsVendor.description.descriptionIframe, productsVendor.description.descriptionHtmlBody, description); await this.saveProduct(); await this.toContainTextFrameLocator(productsVendor.description.descriptionIframe, productsVendor.description.descriptionHtmlBody, description); } catch (error) { console.error('Failed to add product description:', error); // Handle the error appropriately } }
- Consider refactoring
addProductShortDescription
andaddProductDescription
into a single, more generic function:async addProductTextContent(productName: string, content: string, contentType: 'shortDescription' | 'description'): Promise<void> { try { await this.goToProductEdit(productName); const selector = contentType === 'shortDescription' ? productsVendor.shortDescription : productsVendor.description; await this.typeFrameSelector(selector.descriptionIframe, selector.descriptionHtmlBody, content); await this.saveProduct(); await this.toContainTextFrameLocator(selector.descriptionIframe, selector.descriptionHtmlBody, content); } catch (error) { console.error(`Failed to add product ${contentType}:`, error); // Handle the error appropriately } }This refactoring would reduce code duplication and make the function more flexible for future use.
📜 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 comments (2)
tests/pw/tests/e2e/productsDetails.spec.ts (1)
159-214
: LGTM with minor suggestions for improvementThe new test cases for product cover images, gallery images, short descriptions, and full descriptions are well-structured and significantly improve the test coverage for product details functionality. They follow consistent patterns and use appropriate tag annotations.
A few minor suggestions have been made to enhance test isolation, clarity, and completeness:
- Improve test data setup for cover and gallery image tests to ensure proper test isolation.
- Consider using dedicated methods for removing short descriptions and full descriptions.
- Add tests for adding and removing full product descriptions.
These suggestions are relatively minor and do not detract from the overall quality of the new tests. Great job on expanding the test coverage!
tests/pw/feature-map/feature-map.yml (1)
139-148
: New vendor capabilities added for product managementThe following new features have been added for vendors on the "Products" page:
- Adding, updating, and removing product cover images
- Adding, updating, and removing product gallery images
- Adding, updating, and removing product short descriptions
- Updating product descriptions
These additions enhance the vendors' ability to manage their product listings more comprehensively.
However, to ensure consistency and completeness:
Let's verify if these new features are properly integrated with the rest of the system:
✅ Verification successful
New vendor capabilities are properly integrated without inconsistencies
The added vendor capabilities for product management do not have any associated TODO comments, missing admin capabilities, or customer-facing features related to cover image, gallery image, short description, or description functionalities.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any inconsistencies or missing implementations related to the new features # Test 1: Check if there are any TODO comments related to these new features echo "Checking for TODO comments related to new features:" rg --type yaml "TODO.*\b(cover image|gallery image|short description|description)\b" # Test 2: Check if there are any corresponding admin capabilities for these new vendor features echo "Checking for corresponding admin capabilities:" rg --type yaml "admin can.*\b(cover image|gallery image|short description|description)\b" # Test 3: Check if there are any customer-facing features related to these new capabilities echo "Checking for related customer-facing features:" rg --type yaml "customer can.*\b(cover image|gallery image|short description|description)\b"Length of output: 592
Script:
#!/bin/bash # Description: Verify rg is functioning and searching YAML files correctly echo "Listing all YAML files to ensure they are being searched:" fd --type f --extension yamlLength of output: 155
Script:
#!/bin/bash # Description: Expanded search for TODO comments related to new features with different patterns echo "Checking for TODO comments with various patterns related to new features:" rg --type yaml -i "TODO.*\b(cover image|gallery image|short description|description)\b" rg --type yaml -i "FIXME.*\b(cover image|gallery image|short description|description)\b" rg --type yaml -i "# TODO.*\b(cover image|gallery image|short description|description)\b"Length of output: 429
Script:
#!/bin/bash # Description: Expanded search for admin and customer capabilities with broader patterns echo "Checking for corresponding admin capabilities with broader patterns:" rg --type yaml "admin.*\b(can|cannot)\b.*\b(cover image|gallery image|short description|description)\b" echo "Checking for related customer-facing features with broader patterns:" rg --type yaml "customer.*\b(can|cannot)\b.*\b(cover image|gallery image|short description|description)\b"Length of output: 508
Script:
#!/bin/bash # Description: Search for YAML files with both .yaml and .yml extensions echo "Listing all YAML files with .yaml and .yml extensions:" fd --type f --extension yaml --extension ymlLength of output: 202
Script:
#!/bin/bash # Description: Verify the existence of the specific feature-map.yml file echo "Checking for the existence of tests/pw/feature-map/feature-map.yml:" if [ -f "tests/pw/feature-map/feature-map.yml" ]; then echo "File tests/pw/feature-map/feature-map.yml exists." else echo "File tests/pw/feature-map/feature-map.yml does not exist." fiLength of output: 303
Script:
#!/bin/bash # Description: Re-run expanded searches for TODO comments, admin capabilities, and customer-facing features with correct YAML file detection echo "Rechecking for TODO comments related to new features:" rg --type yaml -i "TODO.*\b(cover image|gallery image|short description|description)\b" rg --type yaml -i "FIXME.*\b(cover image|gallery image|short description|description)\b" rg --type yaml -i "# TODO.*\b(cover image|gallery image|short description|description)\b" echo "Rechecking for corresponding admin capabilities with broader patterns:" rg --type yaml "admin.*\b(can|cannot)\b.*\b(cover image|gallery image|short description|description)\b" echo "Rechecking for related customer-facing features with broader patterns:" rg --type yaml "customer.*\b(can|cannot)\b.*\b(cover image|gallery image|short description|description)\b"Length of output: 906
* Add: add product form tests (getdokan#2385) * Add: add vendor payment tests * Update: update feature map * Update: remove test.only
* Add: add product form tests (getdokan#2385) * Add: add product form tests (download options, inventory) * Fix : revert unwanted changes * Add & Refactor vendor payment tests (getdokan#2387) * Add: add vendor payment tests * Update: update feature map * Update: remove test.only * Add $data parameter to dokan_update_vendor hook (getdokan#2386) * Add $data parameter to dokan_update_vendor hook - Added $data array as second parameter to dokan_update_vendor action hook - Updated doc block for dokan_update_vendor hook to reflect new parameter - Added doc block for dokan_before_update_vendor hook * update: hook doc blocks * update:hook metadata with since attribute * chore: Release Version 3.12.4 --------- Co-authored-by: Al Amin Ahamed <[email protected]> Co-authored-by: Shazahanul Islam Shohag <[email protected]>
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
Tests