Add Live search tests #2425
Add Live search tests #2425
Conversation
WalkthroughThis pull request introduces extensive modifications to user permissions and capabilities across various features in 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Outside diff range and nitpick comments (7)
tests/pw/pages/liveSearchPage.ts (1)
9-12: Remove unnecessary constructor.The constructor can be removed as it only calls super without additional initialization. TypeScript will automatically create this constructor.
export class LiveSearch extends BasePage { - constructor(page: Page) { - super(page); - }🧰 Tools
🪛 Biome
[error] 10-12: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
tests/pw/tests/e2e/liveSearch.spec.ts (1)
8-48: Consider adding more test scenarios for comprehensive coverage.The test suite would benefit from additional test cases:
- Empty search results scenario
- Search with special characters
- Error handling cases (e.g., network issues)
- Performance testing with large result sets
Example test case structure:
test('should handle empty search results gracefully', async () => { const nonExistentProduct = 'xyznonexistent123'; const results = await customer.searchByLiveSearch(nonExistentProduct); expect(results).toHaveLength(0); }); test('should handle special characters in search', async () => { const specialCharsProduct = '!@#$%^&*()'; const results = await customer.searchByLiveSearch(specialCharsProduct); // Verify proper escaping and handling });tests/pw/utils/apiEndPoints.ts (2)
631-634: Fix parameter naming inconsistency in getSingleWidgetType.The route parameter uses
widget-typeswhile the function parameter useswidgetId. Consider renaming for consistency:- getSingleWidgetType: (widgetId: string) => `${SERVER_URL}/wp/v2/widget-types/${widgetId}`, + getSingleWidgetType: (widgetTypeId: string) => `${SERVER_URL}/wp/v2/widget-types/${widgetTypeId}`,
635-638: Fix parameter casing inconsistency in sidebar endpoints.The parameter uses camelCase inconsistently. Consider standardizing to
sidebarId:- getSingleSidebar: (sideBarId: string) => `${SERVER_URL}/wp/v2/sidebars/${sideBarId}`, - updateSidebar: (sideBarId: string) => `${SERVER_URL}/wp/v2/sidebars/${sideBarId}`, + getSingleSidebar: (sidebarId: string) => `${SERVER_URL}/wp/v2/sidebars/${sidebarId}`, + updateSidebar: (sidebarId: string) => `${SERVER_URL}/wp/v2/sidebars/${sidebarId}`,tests/pw/feature-map/feature-map.yml (1)
775-776: Fix comment formatting.The comment formatting is inconsistent. The indentation of the commented admin section doesn't align with other sections in the file.
Apply this diff to fix the formatting:
- # admin: - # admin can set Dokan live search settings [duplicate]: true + admin: + #admin can set Dokan live search settings [duplicate]: truetests/pw/utils/dbData.ts (1)
784-785: Document the live search options.Consider adding JSDoc comments to document:
- The purpose of these settings
- Valid values for
live_search_option- Expected structure for
dashboard_menu_manager+/** + * Live search configuration + * @property {('old_live_search'|'suggestion_box')} live_search_option - The type of live search to use + * @property {Array} dashboard_menu_manager - Menu configuration for live search + */ liveSearchSettings: { live_search_option: 'suggestion_box', // old_live_search, suggestion_box dashboard_menu_manager: [], },tests/pw/pages/basePage.ts (1)
302-306: Consider making delay and load state configurable.The implementation is solid, combining load state waiting with text input efficiently. However, consider making the keystroke delay and load state type configurable parameters for better flexibility.
-async typeByPageAndWaitForLoadState(selector: string, text: string, clear = true): Promise<void> { +async typeByPageAndWaitForLoadState( + selector: string, + text: string, + options: { + clear?: boolean; + delay?: number; + state?: 'load' | 'domcontentloaded' | 'networkidle'; + } = {} +): Promise<void> { + const { clear = true, delay = 200, state = 'domcontentloaded' } = options; if (clear) await this.clearInputField(selector); - await Promise.all([this.waitForLoadState(), this.page.locator(selector).pressSequentially(text, { delay: 200 })]); + await Promise.all([ + this.waitForLoadState(state), + this.page.locator(selector).pressSequentially(text, { delay }) + ]); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (10)
tests/pw/feature-map/feature-map.yml(1 hunks)tests/pw/pages/basePage.ts(3 hunks)tests/pw/pages/liveSearchPage.ts(1 hunks)tests/pw/pages/selectors.ts(3 hunks)tests/pw/tests/e2e/liveSearch.spec.ts(1 hunks)tests/pw/tests/e2e/privacyPolicy.spec.ts(1 hunks)tests/pw/utils/apiEndPoints.ts(1 hunks)tests/pw/utils/apiUtils.ts(1 hunks)tests/pw/utils/dbData.ts(3 hunks)tests/pw/utils/testData.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/pw/utils/testData.ts
🧰 Additional context used
🪛 Biome
tests/pw/pages/liveSearchPage.ts
[error] 10-12: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
🔇 Additional comments (12)
tests/pw/pages/liveSearchPage.ts (2)
1-8: LGTM! Well-organized imports and selector definition.
The imports are properly structured, and the selector constant improves code maintainability.
14-29: 🛠️ Refactor suggestion
Add input validation and improve method documentation.
The method implementation looks solid but could benefit from the following improvements:
- Add input validation for the productName parameter
- Document the purpose of typing a space character
- Consider making the delay configurable
- Add JSDoc documentation for better maintainability
Here's a suggested improvement:
+/**
+ * Performs a live search for a product with optional category filtering
+ * @param productName - The name of the product to search for
+ * @param autoload - Whether to use autoload mode (default: false)
+ * @param categoryName - Optional category to filter results
+ * @throws {Error} If productName is empty
+ */
async searchByLiveSearch(productName: string, autoload = false, categoryName?: string) {
+ if (!productName?.trim()) {
+ throw new Error('Product name cannot be empty');
+ }
+
await this.gotoUntilNetworkidle(data.subUrls.frontend.myAccount);
if (!autoload) {Let's verify if similar search implementations exist in the codebase:
tests/pw/tests/e2e/privacyPolicy.spec.ts (2)
Line range hint 1-48: LGTM for the test implementation.
The test suite is well-structured with:
- Clear separation of concerns
- Proper test categorization using tags
- Good coverage of both customer and admin scenarios
- Appropriate use of page object pattern
19-20: Clarify the reason for commenting out widget setup.
The commented-out widget configuration could affect test isolation and the initial state of the tests. If these configurations are no longer needed, they should be removed. If they are still needed but temporarily disabled, please add a comment explaining why and create a tracking issue.
Let's verify if these widget configurations are still required:
tests/pw/tests/e2e/liveSearch.spec.ts (2)
1-7: LGTM! Well-organized imports.
The imports are properly structured and include all necessary dependencies for the test suite.
39-47: 🛠️ Refactor suggestion
Refactor duplicate database configuration.
The last two tests contain duplicate database update code. Consider moving this to a shared setup function.
Extract the database configuration:
+async function enableAutoloadContent() {
+ await dbUtils.updateOptionValue(dbData.dokan.optionName.liveSearch, { live_search_option: 'old_live_search' });
+}
test('customer can search product using live search (autoload content)', { tag: ['@pro', '@customer'] }, async () => {
- await dbUtils.updateOptionValue(dbData.dokan.optionName.liveSearch, { live_search_option: 'old_live_search' });
+ await enableAutoloadContent();
await customer.searchByLiveSearch(data.predefined.simpleProduct.product1.name, true);
});Also, consider adding:
- Data cleanup between tests to ensure test isolation
- Retry logic or wait conditions to handle potential timing issues with search results
✅ Verification successful
Refactoring suggestion is valid, but test isolation is already handled
The suggestion to extract duplicate database configuration is valid and would improve code maintainability. However, the concerns about test isolation and wait conditions are already addressed:
- Test isolation is handled through
afterAllwhich resets the live search settings to their original state - The
searchByLiveSearchimplementation inliveSearchPage.tsincludes proper wait conditions:- Uses
waitForResponsefor AJAX calls - Has visibility checks with
toBeVisible - Includes network idle state checks
- Uses
You can proceed with the refactoring suggestion for the duplicate database configuration, but the additional concerns are already properly handled.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if searchByLiveSearch has proper wait conditions
ast-grep --pattern 'class LiveSearch {
$$$
async searchByLiveSearch($$$) {
$$$
await $$$
$$$
}
$$$
}'
Length of output: 123
Script:
#!/bin/bash
# Let's try to find the searchByLiveSearch method using ripgrep
rg -A 10 "searchByLiveSearch"
# Also search for any test setup/cleanup patterns in the same file
rg -A 5 "beforeAll|beforeEach|afterAll|afterEach" "tests/pw/tests/e2e/liveSearch.spec.ts"
Length of output: 3683
tests/pw/utils/apiEndPoints.ts (1)
624-629: LGTM: Widget endpoints follow REST API best practices.
The implementation provides a complete set of CRUD operations with consistent URL structure and proper parameter typing.
tests/pw/feature-map/feature-map.yml (1)
778-781: LGTM: Live search permissions are properly configured.
The customer permissions for live search features are well-defined and comprehensive, covering both basic and advanced search capabilities:
- Search with suggestion box (with/without category)
- Search with autoload content (with/without category)
All permissions are appropriately set to true to enable these features.
Let's verify if the live search feature is properly integrated with the frontend:
tests/pw/utils/dbData.ts (1)
1304-1353: LGTM! Well-structured widget and sidebar configuration.
The configuration provides a clean separation between:
- Widget definitions
- Sidebar locations
- Widget assignments
- Widget-specific settings
tests/pw/pages/basePage.ts (1)
309-310: LGTM! Good addition of the clear parameter.
The addition of the optional clear parameter provides good flexibility while maintaining backward compatibility.
tests/pw/pages/selectors.ts (2)
2248-2254: LGTM! Live search settings selectors look good.
The selectors for live search settings are well-structured and follow the existing patterns in the codebase.
Line range hint 6902-7758: LGTM! Live search widget selectors are properly implemented.
The selectors for the live search widget functionality are well-organized and follow best practices:
- Clear naming conventions
- Proper use of template literals for dynamic selectors
- Consistent structure with rest of the codebase
| }); | ||
|
|
||
| test.afterAll(async () => { | ||
| await dbUtils.updateOptionValue(dbData.dokan.optionName.privacyPolicy, { enable_privacy: 'on' }); | ||
| await dbUtils.updateOptionValue(dbData.dokan.optionName.appearance, { contact_seller: 'on' }); | ||
| await dbUtils.setOptionValue('sidebars_widgets', dbData.widget.emptySideBarsWidgets); | ||
| await dbUtils.setOptionValue('sidebars_widgets', dbData.emptySideBarsWidgets); // todo: remove in future |
There was a problem hiding this comment.
Address the temporary solution in teardown.
The TODO comment suggests this is a temporary change. This should be properly tracked and resolved to maintain test reliability.
Consider:
- Creating a tracking issue for this temporary solution
- Adding a reference to the tracking issue in the TODO comment
- Documenting why the data structure was changed from
dbData.widget.emptySideBarsWidgetstodbData.emptySideBarsWidgets
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
tests/pw/feature-map/feature-map.yml(1 hunks)tests/pw/pages/basePage.ts(3 hunks)tests/pw/pages/selectors.ts(3 hunks)tests/pw/utils/apiUtils.ts(1 hunks)tests/pw/utils/testData.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/pw/feature-map/feature-map.yml
- tests/pw/utils/testData.ts
🔇 Additional comments (15)
tests/pw/pages/selectors.ts (2)
2248-2254: LGTM! Live search settings selectors are well-structured.
The selectors for live search settings are properly organized and follow consistent naming conventions. The scoping to parent elements helps maintain selector specificity.
7796-7802: LGTM! Live search widget selectors are well-implemented.
The frontend live search widget selectors:
- Use appropriate locator strategies (CSS for static elements, XPath for dynamic content)
- Follow consistent naming conventions
- Properly handle dynamic content with template literals
tests/pw/pages/basePage.ts (3)
314-317: Consider aligning typeAndWaitForLoadState with typeByPageAndWaitForLoadState.
This method has inconsistent behavior compared to typeByPageAndWaitForLoadState. The previous review comment is still applicable.
697-697: Efficient use of fill('') in clearInputField.
Using fill('') is a reliable and concise way to clear input fields. This improves the method's efficiency.
837-840: Great addition of a configurable load state parameter.
The update to selectByValueAndWaitForLoadState by adding the state parameter enhances flexibility and maintains consistency with other methods.
tests/pw/utils/apiUtils.ts (10)
1769-1772: getAllWidgets Method Implementation Looks Good
The getAllWidgets function correctly retrieves all widgets with optional authentication and follows consistent coding patterns.
1775-1778: getSingleWidget Method Implementation Looks Good
The getSingleWidget function properly fetches a single widget by its ID, maintaining consistency with similar methods.
1781-1784: createWidget Method Implementation Looks Good
The createWidget function accurately creates a new widget using the provided payload, aligning with the existing API utility patterns.
1787-1790: updateWidget Method Implementation Looks Good
The updateWidget function effectively updates an existing widget identified by its ID, using the appropriate HTTP method.
1793-1796: Unnecessary Payload in DELETE Request
The deleteWidget method includes a payload parameter and sends it in the DELETE request body. According to HTTP specifications, DELETE requests typically do not include a request body. This may lead to unexpected behavior or compatibility issues.
Consider removing the payload parameter:
-async deleteWidget(widgetId: string, payload: object, auth?: auth): Promise<[APIResponse, responseBody]> {
- const [response, responseBody] = await this.delete(endPoints.wp.deleteWidget(widgetId), { data: payload, headers: auth });
+async deleteWidget(widgetId: string, auth?: auth): Promise<[APIResponse, responseBody]> {
+ const [response, responseBody] = await this.delete(endPoints.wp.deleteWidget(widgetId), { headers: auth });
return [response, responseBody];
}1801-1804: getAllWidgetTypes Method Implementation Looks Good
The getAllWidgetTypes function successfully retrieves all widget types and adheres to the established coding conventions.
1807-1810: Method Naming Consistency: Use Singular Form
The getSingleWidgetTypes method is intended to retrieve a single widget type, but the method name is pluralized. For clarity and consistency, the method name should be getSingleWidgetType.
1815-1818: getAllSidebars Method Implementation Looks Good
The getAllSidebars function correctly retrieves all sidebars, maintaining consistency with other retrieval methods.
1821-1824: getSingleSidebar Method Implementation Looks Good
The getSingleSidebar function properly fetches a sidebar by its ID, following the established patterns.
1827-1830: Inconsistent HTTP Method in updateSidebar
The updateSidebar method uses the POST HTTP method for an update operation. Consistency and RESTful conventions suggest using PUT or PATCH for updates.
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
LiveSearchclass for improved product searching functionality.Bug Fixes
Documentation
dbDataobject for improved widget management.Tests
Chores