Add: Form manager tests #2537
Add: Form manager tests #2537
Conversation
WalkthroughThis pull request introduces several enhancements to the testing framework for a marketplace application. The changes span multiple utility files, test specifications, and page objects, with a primary focus on improving product form management, API testing, and UI interaction capabilities. Key additions include a new Changes
Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
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: 2
🧹 Nitpick comments (13)
tests/pw/pages/productFormManagerPage.ts (5)
11-13: Remove the unnecessary constructor.
According to static analysis, the constructor does nothing besides callingsuper(page). You can safely remove it.-export class ProductFormManager extends AdminPage { - constructor(page: Page) { - super(page); - } +export class ProductFormManager extends AdminPage {🧰 Tools
🪛 Biome (1.9.4)
[error] 11-13: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
30-36: Consider renaming or expanding the module toggle logic.
Although named “enableProductFormManagerModule”, this method doesn’t perform an explicit toggle—it only verifies that the Product Form Manager menu is visible. Consider adding an actual toggle or clarifying the method name.
37-42: Same toggle concern in disable method.
Similar to the enable method, “disable” checks for non-visibility of the Product Form Manager menu but doesn’t explicitly disable. Consider providing an actual toggle flow if required, or rename for clarity.
95-110: Toggle logic is fine, but consider simpler checks.
Usingif (status == true)is functional. Optionally, you can simplify toif (status)for readability. Everything else is solid.-if (status == true) { +if (status) {
130-157: Method name comment mismatch.
The docstring says “admin can add custom field,” which is correct, but also appears exactly above other toggle methods. For clarity, consider adjusting those docstrings. Otherwise, this implementation is well-rounded and covers field editing thoroughly.tests/pw/utils/mergeSummaryReport.ts (1)
87-87: Consider making the reports folder path configurable.The hardcoded path
'./all-reports'could be problematic in different environments. Consider using an environment variable or configuration option for flexibility.-const reportsFolder = './all-reports'; +const reportsFolder = process.env.REPORTS_FOLDER || './all-reports';tests/pw/tests/e2e/productFormManager.spec.ts (1)
68-73: Track TODO with an issue.The TODO comment about updating the test for resetting predefined blocks should be tracked in an issue management system.
Would you like me to help create an issue to track this TODO? I can generate a detailed issue description with acceptance criteria.
tests/pw/utils/summaryReporter.ts (1)
Line range hint
100-105: Optimize flaky test handling.The current implementation uses separate push and splice operations. This could be optimized using array filter.
- summary.flaky_tests.push(test.title); - const index = summary.failed_tests.indexOf(test.title); - if (index !== -1) { - summary.failed_tests.splice(index, 1); - } + summary.flaky_tests.push(test.title); + summary.failed_tests = summary.failed_tests.filter(title => title !== test.title);🧰 Tools
🪛 Biome (1.9.4)
[error] 101-101: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
tests/pw/tests/api/admin.spec.ts (1)
98-103: Enhance test coverage for admin settings.While the basic success case is covered, consider adding tests for:
- Invalid section/option parameters
- Missing parameters
- Specific assertions about the expected commission type values
Example additional test:
test('get admin settings handles invalid parameters', { tag: ['@pro'] }, async () => { const [response, responseBody] = await apiUtils.get(endPoints.getAdminSettings, { params: { section: 'invalid_section', option: 'invalid_option' } }); expect(response.ok()).toBeFalsy(); expect(response.status()).toBe(400); });tests/pw/utils/schemas.ts (1)
1649-1651: Consider using a more specific schema definition instead ofz.any().Using
z.any()bypasses type checking and validation. Consider defining a more specific schema that matches the expected admin settings structure to catch potential issues at runtime.- adminSettingsSchema: z.any(), + adminSettingsSchema: z.object({ + // Define specific fields and their types here + // Example: + // setting1: z.string(), + // setting2: z.number(), + // etc. + }),tests/pw/utils/dbData.ts (2)
842-1819: Consider adding TypeScript interfaces for form manager configurations.The product form manager configuration is extensive but lacks type definitions. Adding interfaces would improve maintainability and provide better IDE support.
interface FormManagerField { id: string; type: string; content_class: string; parent_block: string; label: string; placeholder?: string; help_content?: string; visibility: 'on' | 'off'; required: 'on' | 'off'; input_type: string; default: { label: string; visibility: 'on' | 'off'; required: 'on' | 'off'; }; } interface FormManagerBlock { id: string; type: string; block_type: 'predefined' | 'custom'; content_class: string; // ... other properties }
2579-2607: Consider separating test configurations and using UUIDs for identifiers.The custom block configuration uses timestamp-based IDs and is mixed with production code. Consider these improvements:
- Use UUIDs instead of timestamps for guaranteed uniqueness
- Move test-specific configurations to a separate test data file
- dokan_form_customizer_custom_block_1736758186816: { + dokan_form_customizer_custom_block_${uuid()}: {Additionally, consider extracting this test configuration to a separate file like
testFormConfigs.tsto maintain a clear separation between test and production code.tests/pw/utils/testData.ts (1)
Line range hint
2595-2612: Add validation for environment variables and array synchronization.Consider these improvements for the Printful settings:
- Add fallback values or validation for environment variables
PRINTFUL_APP_IDandPRINTFUL_APP_SECRET.- Ensure
optionNamesandoptionValuesarrays maintain the same length to prevent index mismatches.printful: { settingTitle: 'Printful Settings', - clientId: PRINTFUL_APP_ID, - secretKey: PRINTFUL_APP_SECRET, + clientId: PRINTFUL_APP_ID || '', + secretKey: PRINTFUL_APP_SECRET || '', // ... other settings ... + // Validate arrays have same length + validateArrays() { + if (this.optionNames.length !== this.optionValues.length) { + throw new Error('Printful settings: optionNames and optionValues must have the same length'); + } + }, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
tests/pw/pages/basePage.ts(2 hunks)tests/pw/pages/menuManagerPage.ts(6 hunks)tests/pw/pages/productFormManagerPage.ts(1 hunks)tests/pw/pages/selectors.ts(2 hunks)tests/pw/tests/api/admin.spec.ts(1 hunks)tests/pw/tests/e2e/productFormManager.spec.ts(1 hunks)tests/pw/utils/apiEndPoints.ts(1 hunks)tests/pw/utils/dbData.ts(4 hunks)tests/pw/utils/interfaces.ts(1 hunks)tests/pw/utils/mergeCoverageSummary.ts(0 hunks)tests/pw/utils/mergeSummaryReport.ts(1 hunks)tests/pw/utils/payloads.ts(1 hunks)tests/pw/utils/schemas.ts(1 hunks)tests/pw/utils/summaryReporter.ts(1 hunks)tests/pw/utils/testData.ts(2 hunks)
💤 Files with no reviewable changes (1)
- tests/pw/utils/mergeCoverageSummary.ts
👮 Files not reviewed due to content moderation or server errors (4)
- tests/pw/pages/menuManagerPage.ts
- tests/pw/utils/apiEndPoints.ts
- tests/pw/utils/interfaces.ts
- tests/pw/pages/basePage.ts
🧰 Additional context used
🪛 Biome (1.9.4)
tests/pw/pages/productFormManagerPage.ts
[error] 11-13: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (11)
tests/pw/pages/productFormManagerPage.ts (8)
1-5: All good with the imports and type definitions.
The imports (Page,AdminPage,selector,data, andblock,fieldinterfaces) are consistent with the file’s purpose.
15-22: Navigation method looks clean.
The logic to navigate to the Product Form Manager Settings is straightforward and leverages existing helpers effectively.
24-28: Saving and verifying settings is well-handled.
The approach of waiting for the response and checking for the success message is a good practice.
44-50: Reset flow looks good.
CallingresetAll, then saving and verifying vianotToBeVisibleensures the block is reset.
53-83: Comprehensive approach for adding and editing custom blocks.
This method thoroughly covers each aspect of block creation—from labeling to product category selection—and verifies the changes immediately. Good job!
85-92: Proper block deletion logic.
The deletion flow with a confirmation step and final visibility check is well done.
112-128: Same toggle pattern for required fields.
Matches the active status toggle and is consistently implemented.
159-170: Clean field deletion process.
The deletion flow parallels block deletion. The confirm and finalnotToBeVisiblecheck ensures correctness.tests/pw/pages/selectors.ts (1)
Line range hint
2050-2603: Added Product Form Manager Settings SectionThe changes introduce a new section for Product Form Manager settings with comprehensive selectors for managing product form blocks and fields. Here's a detailed breakdown of the key additions:
- Block Management:
blockSection: (blockName: string) => `//h3[contains(@class,'block-header-title') and normalize-space(text())="${blockName}"]/../../../../..`, enableBlock: (blockName: string) => `//h3[contains(@class,'block-header-title') and normalize-space(text())="${blockName}"]/../../../../..//div[contains(@class,'block-header-toggle')]//label[@class="switch tips"]`, editBlock: (blockName: string) => `//h3[contains(@class,'block-header-title') and normalize-space(text())="${blockName}"]/../../../../..//span[normalize-space(text())="Edit Block"]/..`, deleteBlock: (blockName: string) => `//h3[contains(@class,'block-header-title') and normalize-space(text())="${blockName}"]/../../../../..//button[contains(@class,'delete-button')]`,
- Block Contents Configuration:
blockContents: { label: (blockName: string) => `//h3[contains(@class,'block-header-title') and normalize-space(text())="${blockName}"]/../../../../..//input[@id="input-label"]`, description: (blockName: string) => `//h3[contains(@class,'block-header-title') and normalize-space(text())="${blockName}"]/../../../../..//input[@id="input-desc"]`, // Product type and category selectors specificProductTypeDropdown: (blockName: string) => `//h3[contains(@class,'block-header-title') and normalize-space(text())="${blockName}"]/../../../../..//label[normalize-space()="Specific Product Type"]/..//div[@class="multiselect__select"]`, productType: (productType: string) => `//div[@role="combobox" and contains(@class,'multiselect multiselect--active') ]//ul[@role="listbox"]//span[normalize-space(text())="${productType}"]`, // ... more selectors for product categories },
- Field Management:
fieldSection: (blockName: string, fieldName: string) => `//h3[contains(@class,'block-header-title') and normalize-space(text())="${blockName}"]/../../../../..//h3[normalize-space(text())='${fieldName}']/../..`, enableField: (blockName: string, fieldName: string) => `(//h3[contains(@class,'block-header-title') and normalize-space(text())="${blockName}"]/../../../../..//h3[normalize-space(text())='${fieldName}']/../..//span[normalize-space(text())='Enabled']/..//label[@class="switch tips"])[last()]`, requireField: (blockName: string, fieldName: string) => `(//h3[contains(@class,'block-header-title') and normalize-space(text())="${blockName}"]/../../../../..//h3[normalize-space(text())='${fieldName}']/../..//span[normalize-space(text())='Required']/..//label[@class="switch tips"])[last()]`,
- Field Contents Configuration:
fieldContents: { label: (blockName: string, fieldName: string) => `//h3[contains(@class,'block-header-title') and normalize-space(text())="${blockName}"]/../../../../..//h3[normalize-space(text())='${fieldName}']/../../..//div[@class="field-form-control-wrapper"]//input[@id="field-input-label"]`, type: (blockName: string, fieldName: string) => `//h3[contains(@class,'block-header-title') and normalize-space(text())="${blockName}"]/../../../../..//h3[normalize-space(text())='${fieldName}']/../../..//div[@class="field-form-control-wrapper"]//select[@id="field-input-type"]`, // ... more selectors for field configuration },
- Custom Block Creation:
createCustomBlock: '//button[contains(.,"Create Custom Block")]',The implementation follows best practices by:
- Using dynamic XPath selectors for flexible targeting
- Organizing selectors hierarchically based on functionality
- Providing clear and descriptive selector names
- Including selectors for all necessary UI interactions
The implementation is well-structured and comprehensive, providing all necessary selectors for managing product form blocks and fields.
tests/pw/tests/e2e/productFormManager.spec.ts (1)
10-11: Clarify test suite skip strategy.The entire suite is skipped with
test.skip(true, 'feature not merged yet'), but individual tests aren't skipped. This could lead to confusion. Either:
- Remove the suite-level skip if the feature is ready for testing, or
- Skip individual tests that aren't ready yet.
tests/pw/utils/payloads.ts (1)
3644-3644: LGTM! The module ID mapping for product form manager is correctly defined.The addition follows the established pattern for module identifiers and provides a consistent way to reference the product form manager feature in tests.
Let's verify this module ID is used consistently across the test suite:
✅ Verification successful
✓ Module ID is consistently defined across test utilities
The
product_form_customizationmodule ID is properly synchronized between test payload definitions and database fixtures, following the established pattern for product-related modules.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other usages of this module ID to ensure consistency rg -l "product_form_customization" --type tsLength of output: 96
Script:
#!/bin/bash # Check the usage context of the module ID in both files rg "product_form_customization" --type ts -C 3Length of output: 928
1 participant
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
Improvements
API Changes