Appearance settings tests #2458
Conversation
WalkthroughThe pull request introduces several modifications across multiple files, primarily focusing on enhancing environment configurations, particularly with the addition of reCAPTCHA keys and a new Mapbox variable. The workflow for E2E API tests has been updated to include these new environment variables. Additionally, the feature permissions have been revised to shift control towards admin functionalities, particularly in managing store appearance and vendor capabilities. New methods and properties have been introduced in various classes and files to support these changes, while existing structures remain largely intact. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 6
🧹 Outside diff range and nitpick comments (14)
tests/pw/types/environment.d.ts (1)
37-38: Add JSDoc comments to document the reCAPTCHA environment variables.While the type definitions are correct, adding JSDoc comments would improve maintainability by documenting the purpose and expected format of these credentials.
+ /** Google reCAPTCHA v2 site key for the frontend */ RECAPTCHA_SITE_KEY: string; + /** Google reCAPTCHA v2 secret key for backend verification */ RECAPTCHA_SECRET_KEY: string;tests/pw/.env.example (2)
1-20: Consider adding validation instructions for API keys.While the configuration template is clear, it would be helpful to add comments about how to validate these API keys, especially for the newly added reCAPTCHA configuration.
Add validation instructions like:
RECAPTCHA_SITE_KEY=recaptcha_site_key [reCAPTCHA site key] RECAPTCHA_SECRET_KEY=recaptcha_secret_key [reCAPTCHA secret key] +# Validate reCAPTCHA keys at: https://www.google.com/recaptcha/admin
1-20: Consider grouping related API configurations.The configuration could be more organized by grouping related API configurations together (e.g., grouping authentication-related configs like reCAPTCHA with other security settings).
Consider reorganizing the plugin configuration section to group related settings:
# Plugin Configuration +# User Authentication ADMIN=John_Doe [Admin username] ADMIN_PASSWORD=AdminPass123 [Password for the admin account] VENDOR=David_Johnson [Vendor username] VENDOR2=jhonathon_Smith [Vendor username] CUSTOMER=Michael_Williams [Customer username] USER_PASSWORD=Passw0rd123 [Password for all other users] + +# Security & Authentication APIs +RECAPTCHA_SITE_KEY=recaptcha_site_key [reCAPTCHA site key] +RECAPTCHA_SECRET_KEY=recaptcha_secret_key [reCAPTCHA secret key] + +# Dokan Configuration DOKAN_PRO=true [Dokan pro active status] LICENSE_KEY=license_key [Dokan License key] + +# Integration APIs GMAP=map_key [Google Maps API key] TALKJS_APP_ID=talkjs_app_id [TalkJS App ID] TALKJS_APP_SECRET=talkjs_app_secret [TalkJS App Secret] VONAGE_API_KEY=vonage_key [Vonage SMS API key] VONAGE_API_SECRET=vonage_secret [Vonage SMS API secret] FB_APP_ID=facebook_app_id [Facebook App ID] FB_APP_SECRET=facebook_app_secret [Facebook App secret] PRINTFUL_APP_ID=printful_app_id [Printful App ID] PRINTFUL_APP_SECRET=printful_app_secret [Printful App secret] -RECAPTCHA_SITE_KEY=recaptcha_site_key [reCAPTCHA site key] -RECAPTCHA_SECRET_KEY=recaptcha_secret_key [reCAPTCHA secret key]tests/pw/pages/basePage.ts (2)
1484-1488: Add JSDoc documentation for better maintainability.The implementation looks good but would benefit from JSDoc documentation describing the method's purpose, parameters, and return type.
Add this documentation above the method:
+/** + * Asserts that an element exists in the DOM. + * @param selector - The selector to locate the element + * @throws {Error} When the element does not exist + */ async toExists(selector: string) {
1607-1611: Add JSDoc documentation for better maintainability.The implementation looks good but would benefit from JSDoc documentation describing the method's purpose, parameters, and return type.
Add this documentation above the method:
+/** + * Asserts that an element does not exist in the DOM. + * @param selector - The selector to locate the element + * @throws {Error} When the element exists + */ async notToExists(selector: string) {tests/pw/feature-map/feature-map.yml (1)
297-317: LGTM! Well-structured feature flags for store appearance settings.The feature flags are well-organized and follow a consistent pattern. Each setting has both enable and disable capabilities, providing complete control over the features.
Consider the following architectural improvements:
- Group related settings together (e.g., all map-related settings, all template-related settings) by reordering the flags
- Add validation rules for mutually exclusive settings (e.g., map API source can't be both Google Maps and Mapbox)
- Consider adding default value indicators in the comments to improve maintainability
tests/pw/utils/payloads.ts (3)
3953-3996: Consider improving time handling in store operation hoursThe 24/7 operation configuration using "12:00 am" to "11:59 pm" could lead to edge cases:
- Potential issues during midnight transition
- Possible timezone-related problems
- Missing validation for opening time being before closing time
Consider these improvements:
storeOpenClose: { store_open_close: { enabled: 'yes', time: { monday: { status: 'open', - opening_time: ['12:00 am'], - closing_time: ['11:59 pm'], + opening_time: ['00:00'], + closing_time: ['24:00'], }, // Apply similar changes to other days }, open_notice: 'Store is open', close_notice: 'Store is closed', }, }
3723-3723: Remove unnecessary blank lineThe blank line affects code readability and should be removed.
3802-3802: Remove unnecessary blank linesMultiple blank lines affect code readability and should be removed.
Also applies to: 3887-3887
tests/pw/pages/storeAppearance.ts (3)
75-80: Address the TODO comment insetBannerSizeThere is a
//todocomment indicating that more assertions should be added to fully verify the banner size settings. Consider adding these to enhance test coverage.Would you like assistance in implementing additional assertions for this method?
83-91: Address the TODO comment inviewStoreSideBarFromThemeThere's a
// todo: add more assertionscomment in theelseblock. Adding these assertions will improve the test's effectiveness in verifying the store sidebar's absence.Would you like assistance in adding additional assertions?
105-111: Use consistent existence checks inviewFontAwesomeLibraryIn
viewFontAwesomeLibrary, you usetoExistsandnotToExiststo check for the presence of elements. For consistency and clarity, consider usingtoBeVisibleandnotToBeVisible, unless specifically checking for element existence without considering visibility.Alternatively, if the intention is to check for the presence of the library in the page's resources, consider clarifying this in the method's comments.
tests/pw/tests/e2e/storeAppearance.spec.ts (2)
37-37: Provide a justification for skipping the Google reCAPTCHA testThe test case is skipped using
test.skipin line 37 without an explanation. Including a comment detailing why the test is skipped (e.g., pending implementation, environmental constraints) will improve maintainability and clarity.Apply this diff to add a comment:
test.skip(`admin can ${status} Google reCAPTCHA validation`, { tag: ['@lite', '@admin'] }, async () => { + // Skipping due to reCAPTCHA keys not being available in the test environment. await dbUtils.updateOptionValue(dbData.dokan.optionName.appearance, { recaptcha_enable_status: status === 'enable' ? 'on' : 'off' }); await admin.viewGoogleRecaptcha(status as 'enable' | 'disable', data.predefined.vendorStores.vendor1); });
65-65: Add assertions to verify banner size settingThe test on line 65 sets the banner size using
admin.setBannerSize(data.storeBanner);but does not include assertions to confirm the change. Consider adding verification steps to ensure the banner size is updated correctly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (13)
.github/workflows/e2e_api_tests.yml(1 hunks)tests/pw/.env.example(1 hunks)tests/pw/README.MD(1 hunks)tests/pw/feature-map/feature-map.yml(2 hunks)tests/pw/pages/basePage.ts(2 hunks)tests/pw/pages/selectors.ts(4 hunks)tests/pw/pages/storeAppearance.ts(1 hunks)tests/pw/tests/e2e/_auth.setup.ts(2 hunks)tests/pw/tests/e2e/storeAppearance.spec.ts(1 hunks)tests/pw/types/environment.d.ts(1 hunks)tests/pw/utils/dbData.ts(3 hunks)tests/pw/utils/payloads.ts(11 hunks)tests/pw/utils/testData.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/pw/utils/testData.ts
🔇 Additional comments (25)
tests/pw/types/environment.d.ts (1)
37-38: Ensure secure handling of reCAPTCHA credentials.
These are sensitive credentials that should be properly secured:
- Ensure these values are not committed to version control
- Verify that the
.env.examplefile contains placeholder values - Update documentation to guide developers on obtaining and configuring these keys
✅ Verification successful
reCAPTCHA credentials are properly secured with room for documentation improvement
The verification shows good security practices in place:
.env.examplecontains appropriate placeholder values- No
.envfiles are committed to the repository - No hardcoded reCAPTCHA keys found in the codebase
- Basic documentation exists in
tests/pw/README.MD
However, the documentation could be enhanced with:
- Instructions for obtaining reCAPTCHA keys from Google
- Steps for proper configuration in the testing environment
- Security best practices for handling these credentials
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify proper handling of sensitive credentials
# Check if .env.example exists and contains placeholder values
echo "Checking .env.example for reCAPTCHA placeholders..."
if [ -f "tests/pw/.env.example" ]; then
rg "RECAPTCHA_(SITE|SECRET)_KEY=[^']" "tests/pw/.env.example"
fi
# Check if .env is properly gitignored
echo "Verifying .env is gitignored..."
if [ -f ".gitignore" ]; then
rg "tests/pw/\.env$" ".gitignore"
fi
# Check documentation for setup instructions
echo "Checking for setup documentation..."
if [ -f "tests/pw/README.MD" ]; then
rg -A 5 "RECAPTCHA" "tests/pw/README.MD"
fi
Length of output: 1021
Script:
#!/bin/bash
# Let's check if there are any committed .env files or if these keys appear in any committed files
# Check for any committed .env files
echo "Checking for committed .env files..."
fd -H "\.env$" --exclude ".env.example"
# Search for potential hardcoded reCAPTCHA keys in the codebase
echo "Checking for potential hardcoded reCAPTCHA keys..."
rg -g '!{*.example,*.sample}' "RECAPTCHA_(SITE|SECRET)_KEY\s*=\s*['\"]([^'\"]+)['\"]"
# Check if there's any documentation about reCAPTCHA setup in other locations
echo "Checking for additional documentation..."
fd -t f -e md -e MD | xargs rg -l "reCAPTCHA"
Length of output: 483
tests/pw/.env.example (2)
1-20: LGTM! Well-structured plugin configuration section.
The plugin configuration section maintains consistent formatting with clear inline comments. Each sensitive credential and API key uses appropriate placeholder values.
19-20: Verify reCAPTCHA configuration in related test files.
The addition of reCAPTCHA environment variables is well-documented. However, we should ensure that these variables are properly utilized in the test implementations.
tests/pw/tests/e2e/_auth.setup.ts (1)
Line range hint 1-5: Verify required payload properties exist
The new changes depend on payloads.storeOpenClose and payloads.createStoreReview. Let's ensure these are properly defined in the payloads file.
✅ Verification successful
Required payload properties are properly defined and used
The verification confirms that:
payloads.storeOpenCloseexists with required properties (enabled, time, monday status)payloads.createStoreReviewexists with required properties (title, content, rating)- Both payloads are correctly used in the test file for API calls to update store and create reviews
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if required payload properties exist in payloads.ts
rg -A 5 "storeOpenClose|createStoreReview" tests/pw/utils/payloads.ts
Length of output: 393
Script:
#!/bin/bash
# Let's check the actual usage of these payloads in the test file
rg -A 5 "storeOpenClose|createStoreReview" tests/pw/tests/e2e/_auth.setup.ts
Length of output: 967
tests/pw/README.MD (3)
131-131: LGTM: Spelling correction for LICENSE_KEY
The spelling correction from licence_key to license_key improves consistency with standard American English spelling conventions.
124-142: Documentation is well-structured and comprehensive
The environment variables section is well-organized with:
- Clear variable names
- Descriptive comments in square brackets
- Logical grouping of related configurations
- Consistent formatting throughout
141-142: Verify reCAPTCHA configuration documentation
The addition of reCAPTCHA configuration variables is well-documented with clear descriptions. However, let's ensure these variables are properly utilized in the codebase.
✅ Verification successful
Let me gather more specific information about the reCAPTCHA implementation to verify its proper usage.
Based on the results, I can now provide a final verification response:
reCAPTCHA configuration is properly integrated in the codebase
The reCAPTCHA configuration variables are well-documented and properly utilized:
- Environment variables are correctly typed in
types/environment.d.ts - Configuration is properly set in
dbData.tsunder appearance settings - reCAPTCHA token validation is implemented in the contact seller form handler in
Ajax.php - Hidden input field for reCAPTCHA token exists in the store contact form template
- The variables are included in the
.env.examplefile
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of reCAPTCHA environment variables in the codebase
# Search for reCAPTCHA configuration usage
echo "Searching for reCAPTCHA environment variable usage..."
rg -l "RECAPTCHA_(SITE|SECRET)_KEY"
# Search for reCAPTCHA implementation
echo "Searching for reCAPTCHA implementation..."
rg -l "recaptcha"
# Check if the variables are included in the example environment file
echo "Checking .env.example file..."
cat tests/pw/.env.example
Length of output: 3090
Script:
#!/bin/bash
# Check the type definitions and actual usage of reCAPTCHA variables
echo "Checking environment type definitions..."
cat tests/pw/types/environment.d.ts
echo -e "\nChecking reCAPTCHA implementation in database utilities..."
cat tests/pw/utils/dbData.ts
echo -e "\nChecking reCAPTCHA implementation in store contact form..."
cat templates/widgets/store-contact-form.php
echo -e "\nChecking reCAPTCHA implementation in Ajax handler..."
cat includes/Ajax.php
Length of output: 99825
.github/workflows/e2e_api_tests.yml (1)
49-49: Verify MAPBOX secret configuration.
The addition of the MAPBOX environment variable looks good. However, ensure that:
- The MAPBOX secret is configured in GitHub repository settings
- The secret contains a valid Mapbox API key
tests/pw/utils/dbData.ts (1)
223-224: LGTM! Well-documented configuration options.
The added comments clearly document the valid values for each setting, improving code maintainability and developer experience.
Also applies to: 232-232, 236-236, 247-247
tests/pw/feature-map/feature-map.yml (3)
302-305: Verify reCAPTCHA and contact form dependencies.
The reCAPTCHA and contact form settings might require additional configurations.
Let's verify the related configuration files:
✅ Verification successful
Let me check the actual implementation details of these configurations.
Dependencies are properly configured and integrated
The verification shows that both reCAPTCHA and contact form features are well-integrated with proper configurations:
- reCAPTCHA v3 integration is complete with:
- Site key and Secret key configuration fields
- Hidden token field in the contact form
- Documentation links for setup guidance
- Store contact form is properly implemented with:
- Form template with required fields
- Privacy policy integration
- reCAPTCHA token integration
- Proper nonce verification
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for reCAPTCHA and contact form configurations
# Search for reCAPTCHA configuration
echo "Searching for reCAPTCHA configuration..."
rg -l "recaptcha|captcha" --type yaml --type php
# Search for contact form templates
echo "Searching for contact form templates..."
rg -l "contact.*form|store.*contact" --type php --type html --type twig
Length of output: 851
Script:
#!/bin/bash
# Check reCAPTCHA implementation
echo "=== ReCAPTCHA Implementation ==="
rg -A 5 "recaptcha|captcha" includes/Admin/Settings.php
echo "\n=== Contact Form Widget Implementation ==="
cat templates/widgets/store-contact-form.php
echo "\n=== Store Contact Form Settings ==="
rg -A 5 "contact.*form|store.*contact" includes/Admin/Settings.php
Length of output: 5189
297-301: Verify map API source configuration dependencies.
The map-related settings might have dependencies on API keys and configurations.
Let's verify the related configuration files:
✅ Verification successful
Map API source configuration dependencies are properly handled
The codebase shows a well-structured implementation of map API configurations:
- Map API source selection (Google Maps/Mapbox) is managed through settings with proper conditional handling
- API keys are stored securely using WordPress options:
- Google Maps:
gmap_api_keyindokan_appearanceoptions - Mapbox:
mapbox_access_tokenindokan_appearanceoptions
- Google Maps:
- Required scripts and styles are loaded conditionally based on the selected provider
- API key validation is implemented through
dokan_has_map_api_key()
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for map API configuration files and settings
# Search for map-related configuration files
echo "Searching for map configuration files..."
rg -l "map.*api|google.*maps|mapbox" --type yaml --type php
# Search for API key configurations
echo "Searching for API key configurations..."
rg "GOOGLE_MAPS_API_KEY|MAPBOX_ACCESS_TOKEN" --type yaml --type php
Length of output: 945
Script:
#!/bin/bash
# Let's examine the content of relevant files to understand the map API configuration dependencies
# Check store-form.php for map settings
echo "=== Checking store settings form ==="
rg -A 5 "map.*api|google.*maps|mapbox" templates/settings/store-form.php
# Check Assets.php for API key handling
echo -e "\n=== Checking Assets.php for API configurations ==="
rg -A 5 "map.*api|google.*maps|mapbox" includes/Assets.php
# Check Admin/Settings.php for map settings
echo -e "\n=== Checking Admin Settings for map configurations ==="
rg -A 5 "map.*api|google.*maps|mapbox" includes/Admin/Settings.php
# Check for environment variables or constants
echo -e "\n=== Checking for map API constants or configurations ==="
rg "define.*(?i)(GOOGLE_MAPS_API|MAPBOX.*TOKEN)" includes/
Length of output: 5353
306-309: Ensure store header templates are properly defined.
The store header template settings reference four different layouts.
Let's verify the template files exist:
✅ Verification successful
Let's verify the actual content of these files to check if they define the mentioned layouts.
Store header templates are properly defined and implemented.
The verification shows that all four layouts (default, layout1, layout2, and layout3) are properly defined in the codebase:
- The templates are defined in the admin settings (
includes/Admin/Settings.php) with corresponding image assets - The template implementation exists in
templates/store-header.phpwith proper conditional logic for each layout - The system correctly handles different layouts with specific styling and structure variations (e.g., profile image shapes, banner handling)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for store header template files
# Search for store header template files
echo "Searching for store header templates..."
rg -l "store.*header.*template|store.*header.*layout" --type php --type html --type twig
Length of output: 318
Script:
#!/bin/bash
# Check content of relevant files for layout definitions
echo "=== Checking templates/store-header.php ==="
rg -A 5 "layout[1-3]|default" templates/store-header.php
echo -e "\n=== Checking templates/settings/store-form.php ==="
rg -A 5 "layout[1-3]|default" templates/settings/store-form.php
echo -e "\n=== Checking includes/Admin/Settings.php ==="
rg -A 5 "layout[1-3]|default" includes/Admin/Settings.php
echo -e "\n=== Checking for template related functions ==="
ast-grep --pattern 'function $FUNC($$$) {
$$$
$TEMPLATE
$$$
}' --lang php
Length of output: 17424
tests/pw/pages/selectors.ts (4)
6025-6025: LGTM! Added banner help text selector.
The addition of the banner help text selector improves test coverage for the store banner UI component.
7384-7396: LGTM! Improved profile info selectors organization.
The changes enhance selector specificity and maintainability by:
- Using more semantic class names
- Grouping related selectors logically
- Following consistent naming conventions
7528-7529: LGTM! Added store sidebar selector.
The addition of the store sidebar widget area selector follows established patterns and uses semantic class names.
7540-7551: LGTM! Added comprehensive store-related selectors.
The additions enhance test coverage by adding selectors for:
- Store contact form
- Store map
- Store open/close time
- Google reCAPTCHA
- Font Awesome library
The selectors follow consistent naming conventions and use semantic class names.
tests/pw/pages/storeAppearance.ts (7)
15-16: Method gotoSingleStore is implemented correctly
The method correctly navigates to the single store page using the provided store name.
39-47: Method viewGoogleRecaptcha is implemented correctly
The method correctly checks the visibility of Google reCAPTCHA based on the status parameter.
50-57: Method viewStoreContactFormOnStoreSidebar is implemented correctly
The method appropriately checks the visibility of the store contact form based on the status parameter.
60-63: Method viewStoreHeaderTemplate is implemented correctly
The method correctly verifies the class of the store profile info box based on the selected template.
66-73: Method viewStoreOpenCloseTimeOnStoreSidebar is implemented correctly
The method accurately checks the visibility of the store open-close time based on the status parameter.
94-102: Method viewVendorInfoOnSingleStorePage is implemented correctly
The method appropriately checks the visibility of vendor information elements based on the status parameter.
30-37: Validate API parameter handling in viewMapAPISource
Currently, the method assumes that the api parameter will only be 'Google Maps' or 'Mapbox'. Consider adding input validation or a default case to handle unexpected values, preventing potential runtime errors.
Run the following script to ensure that viewMapAPISource is called with valid parameters elsewhere in the codebase:
tests/pw/tests/e2e/storeAppearance.spec.ts (2)
70-70: Verify logic of 'enable_theme_store_sidebar' option
In line 70, the enable_theme_store_sidebar option is set as follows:
enable_theme_store_sidebar: status === 'enable' ? 'off' : 'on'This means when status is 'enable', the option is set to 'off', and vice versa. This might be counterintuitive. Please verify if this logic correctly reflects the intended behavior.
77-77: Confirm correctness of 'hide_vendor_info' settings
In line 77, the hide_vendor_info is assigned based on the status:
hide_vendor_info: status === 'enable' ? { email: '', phone: '', address: '' } : { email: 'email', phone: 'phone', address: 'address' }Assigning empty strings when status is 'enable' may not effectively hide vendor information. Please check if this implementation achieves the desired effect of showing or hiding vendor info.
tests/pw/tests/e2e/_auth.setup.ts
Outdated
| // add open-close time | ||
| await apiUtils.updateStore(sellerId, payloads.storeOpenClose, payloads.adminAuth); | ||
| // add review | ||
| await apiUtils.createStoreReview(sellerId, { ...payloads.createStoreReview, rating: 5 }, payloads.customerAuth); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reduce code duplication by extracting vendor setup logic
The vendor setup logic is duplicated between vendor1 and vendor2. This violates the DRY principle and makes maintenance harder.
Consider extracting the common setup logic into a helper function:
async function setupVendorStore(apiUtils: ApiUtils, createStorePayload: any, adminAuth: any, customerAuth: any): Promise<string> {
const [, sellerId] = await apiUtils.createStore(createStorePayload, adminAuth, true);
// Setup store details
const storeUpdate = await apiUtils.updateStore(sellerId, payloads.storeOpenClose, adminAuth);
expect(storeUpdate, 'Store update failed').toBeTruthy();
// Add initial review
const review = await apiUtils.createStoreReview(
sellerId,
{ ...payloads.createStoreReview, rating: payloads.testRatings.default },
customerAuth
);
expect(review, 'Review creation failed').toBeTruthy();
return sellerId;
}Then use it in both setup blocks:
setup('add vendor1', { tag: ['@lite'] }, async () => {
const sellerId = await setupVendorStore(
apiUtils,
payloads.createStore1,
payloads.adminAuth,
payloads.customerAuth
);
helpers.createEnvVar('VENDOR_ID', sellerId);
});
setup('add vendor2', { tag: ['@lite'] }, async () => {
const sellerId = await setupVendorStore(
apiUtils,
payloads.createStore2,
payloads.adminAuth,
payloads.customerAuth
);
helpers.createEnvVar('VENDOR2_ID', sellerId);
});
tests/pw/tests/e2e/_auth.setup.ts
Outdated
| // add open-close time | ||
| await apiUtils.updateStore(sellerId, payloads.storeOpenClose, payloads.adminAuth); | ||
| // add review | ||
| await apiUtils.createStoreReview(sellerId, { ...payloads.createStoreReview, rating: 5 }, payloads.customerAuth); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider improving test data variability and error handling
The current implementation has several areas for improvement:
- Using a fixed rating of 5 limits test coverage. Consider parameterizing the rating or testing different values.
- Missing assertions for successful store update and review creation.
- This code pattern is duplicated for vendor2 setup.
Consider refactoring to:
- // add open-close time
- await apiUtils.updateStore(sellerId, payloads.storeOpenClose, payloads.adminAuth);
- // add review
- await apiUtils.createStoreReview(sellerId, { ...payloads.createStoreReview, rating: 5 }, payloads.customerAuth);
+ // Setup store details
+ const storeUpdate = await apiUtils.updateStore(sellerId, payloads.storeOpenClose, payloads.adminAuth);
+ expect(storeUpdate, 'Store update failed').toBeTruthy();
+
+ // Add initial review
+ const review = await apiUtils.createStoreReview(
+ sellerId,
+ { ...payloads.createStoreReview, rating: payloads.testRatings.default },
+ payloads.customerAuth
+ );
+ expect(review, 'Review creation failed').toBeTruthy();Committable suggestion skipped: line range outside the PR's diff.
| @@ -46,6 +46,7 @@ env: | |||
| CUSTOMER2: customer2 | |||
| USER_PASSWORD: 01dokan01 | |||
| GMAP: ${{secrets.GMAP}} | |||
| MAPBOX: ${{secrets.MAPBOX}} | |||
There was a problem hiding this comment.
💡 Codebase verification
Document the MAPBOX environment variable in the test documentation
The MAPBOX environment variable is used in the E2E API tests workflow but lacks proper documentation. Based on the changelog entries, it's used as an alternative to Google Maps for address functionality.
- Add documentation for MAPBOX in
tests/pw/README.MDto explain:- Its purpose as a Mapbox API key for address functionality
- How to obtain it
- Required format
- Consider adding it to the environment setup section in
tests/pw/README.MDalongside other test environment variables
🔗 Analysis chain
Document the MAPBOX environment variable.
Since this is a new configuration requirement, consider:
- Adding it to the repository's documentation
- Including it in any environment setup guides
- Documenting its purpose and required format
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if MAPBOX is documented in README or environment setup files
# Search for MAPBOX documentation in markdown files
echo "Searching for MAPBOX documentation in markdown files..."
rg -i "mapbox" -g "*.md"
# Search for MAPBOX in environment example files
echo "Searching for MAPBOX in environment files..."
rg -i "mapbox" -g "*.env*"
Length of output: 603
Script:
#!/bin/bash
# Let's check for any environment setup or configuration documentation in docs directory
echo "Searching for documentation files..."
fd -e md -e txt -e env.example
# Let's also check for any setup or configuration instructions in package files
echo "Searching for package files..."
fd package.json
# Let's examine the content of any found package.json for scripts or documentation
echo "Checking package.json content..."
fd package.json -x cat {}
# Let's check the workflow file itself for any inline documentation
echo "Checking workflow file..."
cat .github/workflows/e2e_api_tests.yml
Length of output: 15201
| recaptcha_enable_status: 'off', | ||
| recaptcha_site_key: RECAPTCHA_SITE_KEY ?? '', | ||
| recaptcha_secret_key: RECAPTCHA_SECRET_KEY ?? '', |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for reCAPTCHA environment variables.
While using environment variables for sensitive keys is good practice, consider adding validation to ensure the keys are properly set when reCAPTCHA is enabled.
- recaptcha_enable_status: 'off',
- recaptcha_site_key: RECAPTCHA_SITE_KEY ?? '',
- recaptcha_secret_key: RECAPTCHA_SECRET_KEY ?? '',
+ recaptcha_enable_status: 'off',
+ recaptcha_site_key: validateRecaptchaKey(RECAPTCHA_SITE_KEY),
+ recaptcha_secret_key: validateRecaptchaKey(RECAPTCHA_SECRET_KEY),Add a validation helper:
function validateRecaptchaKey(key: string | undefined): string {
if (!key && process.env.NODE_ENV === 'production') {
console.warn('reCAPTCHA key not set in production environment');
}
return key ?? '';
}| async viewStoreMapOnStoreSidebar(status: 'enable' | 'disable', storeName: string): Promise<void> { | ||
| await this.gotoSingleStore(storeName); | ||
| if (status === 'enable') { | ||
| await this.toBeVisible(singleStoreCustomer.storeMap.storeMap); | ||
| } else { | ||
| await this.notToBeVisible(singleStoreCustomer.storeMap.storeMap); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider refactoring repetitive code in visibility checks
Multiple methods, such as viewStoreMapOnStoreSidebar, use similar patterns to check the visibility of elements based on the status parameter. To improve maintainability and reduce code duplication, consider creating a helper method for these checks.
Example refactoring:
private async checkVisibility(status: 'enable' | 'disable', element: string): Promise<void> {
if (status === 'enable') {
await this.toBeVisible(element);
} else {
await this.notToBeVisible(element);
}
}Update your method:
async viewStoreMapOnStoreSidebar(status: 'enable' | 'disable', storeName: string): Promise<void> {
await this.gotoSingleStore(storeName);
- if (status === 'enable') {
- await this.toBeVisible(singleStoreCustomer.storeMap.storeMap);
- } else {
- await this.notToBeVisible(singleStoreCustomer.storeMap.storeMap);
- }
+ await this.checkVisibility(status, singleStoreCustomer.storeMap.storeMap);
}| const customerContext = await browser.newContext(data.auth.adminAuth); | ||
| aPage = await customerContext.newPage(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Rename 'customerContext' to 'adminContext' for clarity
The variable customerContext is initialized with admin authentication (data.auth.adminAuth), indicating it represents an admin context rather than a customer context. Renaming it to adminContext will enhance readability and reduce confusion.
Apply this diff to rename the variable:
- const customerContext = await browser.newContext(data.auth.adminAuth);
- aPage = await customerContext.newPage();
+ const adminContext = await browser.newContext(data.auth.adminAuth);
+ aPage = await adminContext.newPage();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const customerContext = await browser.newContext(data.auth.adminAuth); | |
| aPage = await customerContext.newPage(); | |
| const adminContext = await browser.newContext(data.auth.adminAuth); | |
| aPage = await adminContext.newPage(); |
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
RECAPTCHA_SITE_KEYandRECAPTCHA_SECRET_KEY.Store Appearancemanagement with new functionalities for admins, including visibility controls for maps, contact forms, and vendor information.BasePageclass.Bug Fixes
LICENSE_KEYin configuration files.Documentation
Tests