Skip to content
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 live chat module tests #2426

Merged

Conversation

shashwatahalder01
Copy link
Contributor

@shashwatahalder01 shashwatahalder01 commented Nov 1, 2024

All Submissions:

  • My code follow the WordPress' coding standards
  • My code satisfies feature requirements
  • My code is tested
  • My code passes the PHPCS tests
  • My code has proper inline documentation
  • I've included related pull request(s) (optional)
  • I've included developer documentation (optional)
  • I've added proper labels to this pull request

Changes proposed in this Pull Request:

Related Pull Request(s)

  • Full PR Link

Closes

  • Closes #

How to test the changes in this Pull Request:

  • Steps or issue link

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:

  • Code is not following code style guidelines
  • Bad naming: make sure you would understand your code if you read it a few months from now.
  • KISS: Keep it simple, Sweetie (not stupid!).
  • DRY: Don't Repeat Yourself.
  • Code that is not readable: too many nested 'if's are a bad sign.
  • Performance issues
  • Complicated constructions that need refactoring or comments: code should almost always be self-explanatory.
  • Grammar errors.

FOR PR REVIEWER ONLY:

As a reviewer, your feedback should be focused on the idea, not the person. Seek to understand, be respectful, and focus on constructive dialog.

As a contributor, your responsibility is to learn from suggestions and iterate your pull request should it be needed based on feedback. Seek to collaborate and produce the best possible contribution to the greater whole.

  • Correct — Does the change do what it’s supposed to? ie: code 100% fulfilling the requirements?
  • Secure — Would a nefarious party find some way to exploit this change? ie: everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities?
  • Readable — Will your future self be able to understand this change months down the road?
  • Elegant — Does the change fit aesthetically within the overall style and architecture?

Summary by CodeRabbit

  • New Features

    • Enhanced permissions for managing tax and shipping features for admins.
    • Expanded capabilities for vendor registration and account management.
    • Improved customer functionalities for registration, login, and detail management.
    • Added live chat functionalities, allowing customers to interact with vendors more effectively.
    • New settings for managing live chat features in the Dokan application.
  • Bug Fixes

    • Adjusted visibility checks for chat functionalities on product and vendor pages.
  • Tests

    • Introduced end-to-end tests for live chat features covering admin, vendor, and customer roles.
    • Added test cases for setting live chat settings for both admin and vendor roles.
  • Documentation

    • Updated environment variable definitions to include TALKJS_APP_ID and TALKJS_APP_SECRET.

Copy link
Contributor

coderabbitai bot commented Nov 1, 2024

Walkthrough

The changes in this pull request involve extensive updates to feature permissions across multiple roles, particularly focusing on the live chat functionality. The feature-map.yml file has been modified to restrict plugin management while enhancing capabilities for tax, shipping, and live chat features for admins, vendors, and customers. Additionally, new methods have been introduced in various classes to support live chat interactions, and several tests have been added to validate these functionalities within the Playwright testing framework.

Changes

File Path Change Summary
tests/pw/feature-map/feature-map.yml Updated permissions for "Plugins" (false), "Tax" (true), "Shipping" (true), "Live Chat" (various true settings), and other pages reflecting enhanced functionalities.
tests/pw/pages/basePage.ts Added clickFrameSelectorAndWaitForResponse and toBeVisibleFrameLocator methods to enhance frame interaction capabilities.
tests/pw/pages/liveChatPage.ts Introduced LiveChatPage class with methods for navigating store/product pages, managing vendor inbox, and sending messages.
tests/pw/pages/selectors.ts Updated selectors to use XPath for live chat and refined other selectors for better specificity.
tests/pw/pages/settingsPage.ts Added setDokanLiveChatSettings method to configure live chat settings.
tests/pw/pages/singleProductPage.ts Modified singleProductRenderProperly to include chatNow in visibility checks.
tests/pw/pages/singleStorePage.ts Updated singleStoreRenderProperly to include chatNow in visibility checks.
tests/pw/pages/storeSupportsPage.ts Minor comment update in sendMessageToSupportTicket method.
tests/pw/pages/vendorSettingsPage.ts Added liveChatSettings method and modified setStoreSettings to handle live chat settings.
tests/pw/tests/e2e/_env.setup.ts Introduced test case for setting live chat settings in the admin setup.
tests/pw/tests/e2e/liveChat.spec.ts New test suite for live chat feature covering admin, vendor, and customer interactions.
tests/pw/tests/e2e/settings.spec.ts Added test case for admin setting live chat settings.
tests/pw/tests/e2e/vendorSettings.spec.ts Added test case for vendor setting live chat settings.
tests/pw/types/environment.d.ts Added TALKJS_APP_ID and TALKJS_APP_SECRET properties to ProcessEnv interface.
tests/pw/utils/dbData.ts Updated dbData to include new environment variables and configurations for live chat settings.
tests/pw/utils/interfaces.ts Added liveChat property to vendor interface and defined a new liveChat interface in dokanSettings.
tests/pw/utils/testData.ts Integrated new environment variables and settings for live chat in dokanSettings.
tests/pw/pages/productsPage.ts Modified viewProduct method to include chatNow in destructured productDetails.

Possibly related PRs

Suggested labels

QA approved

🐇 In the meadow where changes bloom,
A chat now brightens every room.
With settings set and roles defined,
The vendors and customers intertwined.
Permissions shift, new paths we tread,
In this lively chat, let joy be spread! 🌼


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 17

🧹 Outside diff range and nitpick comments (16)
tests/pw/types/environment.d.ts (1)

33-34: Add JSDoc comments to document the TalkJS credentials.

Consider adding JSDoc comments to provide more context about these environment variables.

+            /** TalkJS Application ID for chat integration */
             TALKJS_APP_ID: string;
+            /** TalkJS Application Secret for secure API calls */
             TALKJS_APP_SECRET: string;
tests/pw/tests/e2e/liveChat.spec.ts (3)

7-8: Add environment variable validation

Consider adding validation for the VENDOR_ID environment variable to fail fast if it's not properly configured.

+if (!VENDOR_ID) {
+    throw new Error('VENDOR_ID environment variable is required for live chat tests');
+}

14-24: Add error handling for database operations

The database update in beforeAll should include error handling to ensure the test environment is properly configured.

-        await dbUtils.updateUserMeta(VENDOR_ID, 'dokan_profile_settings', { live_chat: 'yes' });
+        try {
+            await dbUtils.updateUserMeta(VENDOR_ID, 'dokan_profile_settings', { live_chat: 'yes' });
+        } catch (error) {
+            console.error('Failed to enable live chat for vendor:', error);
+            throw error;
+        }

1-77: Consider implementing shared test utilities

The test suite would benefit from:

  1. A shared test fixture for common setup/teardown operations
  2. Helper functions for message verification
  3. Constants for test timeouts and retry policies
  4. Custom test assertions for chat-specific validations

This would improve maintainability and reduce code duplication across tests.

tests/pw/pages/singleStorePage.ts (2)

33-35: LGTM! Consider adding JSDoc for the chatNow element.

The addition of the chatNow element to visibility checks is correct and aligns with the live chat module testing objectives.

Consider adding a JSDoc comment above the method to document the purpose of each tab element, including the new chatNow element:

/**
 * Verifies the proper rendering of a single store page
 * @param storeName - The name of the store to verify
 * 
 * Pro features include:
 * - chatNow: TalkJS integration for live chat functionality
 * - toc: Terms and conditions tab
 * ...
 */

Line range hint 19-63: Consider breaking down the singleStoreRenderProperly method.

The method handles multiple concerns (store profile, tabs, search, products, social icons). Consider breaking it down into smaller, focused methods for better maintainability and easier testing:

class SingleStorePage extends CustomerPage {
    // ... other code ...

    async singleStoreRenderProperly(storeName: string) {
        await this.gotoSingleStore(storeName);
        await this.verifyStoreProfile();
        await this.verifyStoreTabs();
        await this.verifySearchElements();
        await this.verifyProducts();
        await this.verifySocialIcons();
    }

    private async verifyStoreProfile() {
        const { verifiedIcon, verifiedIconByIcon, euComplianceData, ...storeProfile } = singleStoreCustomer.storeProfile;
        await this.multipleElementVisible(storeProfile);
        
        if (DOKAN_PRO) {
            await this.multipleElementVisible(singleStoreCustomer.storeProfile.euComplianceData);
        }
    }

    private async verifyStoreTabs() {
        if (DOKAN_PRO) {
            const { toc, chatNow, ...storeTabs } = singleStoreCustomer.storeTabs;
            await this.multipleElementVisible(storeTabs);
        } else {
            await this.toBeVisible(singleStoreCustomer.storeTabs.products);
        }
    }

    // ... similar methods for other sections ...
}
tests/pw/tests/e2e/vendorSettings.spec.ts (1)

Line range hint 1-150: Consider improving test file organization.

While the new test is well-implemented, here are some suggestions to improve the overall test file structure:

  1. Group related tests using nested test.describe blocks
  2. Move cleanup code to test.afterEach hooks
  3. Document or address skipped tests

Here's a suggested structure:

test.describe('Vendor settings test', () => {
    // ... setup code ...

    test.describe('Menu Page Visibility', () => {
        test('vendor can view store settings menu page', ...);
        test('vendor can view Shipstation settings menu page', ...);
        // ... other menu visibility tests ...
    });

    test.describe('Store Settings', () => {
        test.afterEach(async () => {
            // Move cleanup code here
            await dbUtils.updateOptionValue(dbData.dokan.optionName.selling, {
                catalog_mode_hide_add_to_cart_button: 'off',
                catalog_mode_hide_product_price: 'off'
            });
        });

        test('vendor can set store banner settings', ...);
        test('vendor can set store profile picture settings', ...);
        // ... other store settings tests ...
    });

    test.describe('Advanced Settings', () => {
        test('vendor can set live chat settings', ...);
        test('vendor can set shipStation settings', ...);
        // ... other advanced settings tests ...
    });
});
tests/pw/tests/e2e/settings.spec.ts (1)

Line range hint 1-150: Consider adding cleanup for live chat settings.

Several tests in the suite (e.g., email verification, vendor subscription) include cleanup steps to reset settings after the test. Consider adding similar cleanup for live chat settings to ensure test isolation.

Example cleanup pattern:

test('admin can set Dokan live chat settings', { tag: ['@pro', '@admin'] }, async () => {
    await admin.setDokanLiveChatSettings(data.dokanSettings.liveChat);
    
    // reset settings
    await dbUtils.setOptionValue(dbData.dokan.optionName.liveChat, dbData.dokan.liveChatSettings);
});
.github/workflows/e2e_api_tests.yml (1)

Line range hint 1-201: Architecture recommendation for test organization.

The workflow is well-structured, but as you're adding live chat module tests, consider organizing the tests into separate jobs or steps for better isolation and parallel execution capabilities. This would allow for:

  • Independent execution of live chat tests
  • Faster feedback for specific test suites
  • Better resource utilization

For example, you could structure the jobs as:

jobs:
  core-tests:
    # existing tests job
  live-chat-tests:
    needs: core-tests
    # new job specifically for live chat module tests
tests/pw/tests/e2e/_env.setup.ts (1)

236-238: Consider enhancing the live chat settings test with verification and documentation.

The implementation follows the established pattern and integrates well with the existing test suite. However, consider the following improvements:

  1. Add verification to ensure settings were applied correctly
  2. Document the expected format of liveChatSettings
  3. Add error handling for invalid settings data

Consider enhancing the test with verification:

 setup('admin set dokan live chat settings', { tag: ['@pro'] }, async () => {
     await dbUtils.setOptionValue(dbData.dokan.optionName.liveChat, dbData.dokan.liveChatSettings);
+    // Verify settings were applied
+    const savedSettings = await dbUtils.getOptionValue(dbData.dokan.optionName.liveChat);
+    expect(savedSettings).toEqual(dbData.dokan.liveChatSettings);
 });

Also, consider adding JSDoc documentation above the test case to describe the expected settings format and any external dependencies (e.g., TalkJS configuration).

tests/pw/pages/settingsPage.ts (1)

468-490: LGTM! Consider adding error handling for invalid credentials.

The implementation follows the established patterns and includes proper verification steps. However, it might be beneficial to add error handling for cases where TalkJS credentials are invalid.

Consider adding a try-catch block to handle potential API errors:

 async setDokanLiveChatSettings(liveChat: dokanSettings['liveChat']) {
     await this.goToDokanSettings();
     await this.click(settingsAdmin.menus.liveChat);
 
     // liveChat Settings
     await this.enableSwitcher(settingsAdmin.liveChat.enableLiveChat);
     await this.click(settingsAdmin.liveChat.chatProvider(liveChat.chatProvider));
-    await this.clearAndType(settingsAdmin.liveChat.talkJsAppId, liveChat.talkJsAppId);
-    await this.clearAndType(settingsAdmin.liveChat.talkJsAppSecret, liveChat.talkJsAppSecret);
+    try {
+        await this.clearAndType(settingsAdmin.liveChat.talkJsAppId, liveChat.talkJsAppId);
+        await this.clearAndType(settingsAdmin.liveChat.talkJsAppSecret, liveChat.talkJsAppSecret);
+    } catch (error) {
+        throw new Error(`Failed to set TalkJS credentials: ${error.message}`);
+    }
     await this.enableSwitcher(settingsAdmin.liveChat.chatButtonOnVendorPage);
     await this.selectByValue(settingsAdmin.liveChat.chatButtonOnProductPage, liveChat.chatButtonPosition);
tests/pw/utils/interfaces.ts (1)

1728-1737: Consider adding JSDoc comments to document the liveChat settings interface.

The interface is well-structured, but adding JSDoc comments would improve maintainability by documenting the purpose of each property.

 // Rma Settings
+/**
+ * Live Chat Settings Interface
+ * @property settingTitle - Title displayed in the settings UI
+ * @property chatProvider - Selected chat service provider
+ * @property talkJsAppId - TalkJS application ID for integration
+ * @property talkJsAppSecret - TalkJS application secret for authentication
+ * @property chatButtonPosition - Position of the chat button in the UI
+ * @property saveSuccessMessage - Success message shown after saving settings
+ */
 liveChat: {
     settingTitle: string;
     chatProvider: string;
     talkJsAppId: string;
     talkJsAppSecret: string;
     chatButtonPosition: string;
     saveSuccessMessage: string;
 };
tests/pw/feature-map/feature-map.yml (1)

766-771: Verify duplicate comment removal.

The commented line # admin can set Dokan live chat settings [duplicate]: true indicates a duplicate test case. Consider removing this line since the actual test cases that follow are more specific and comprehensive.

The granular test cases for chat button placement look good, covering all necessary scenarios:

  • Vendor page toggle
  • Product page placement (above tab and inside tab)
  • Disable functionality
tests/pw/pages/basePage.ts (2)

980-984: Add JSDoc documentation for better API understanding.

The method implementation is correct, but would benefit from documentation explaining:

  • Purpose and use cases
  • Parameter descriptions
  • Return value details
  • Example usage

Add JSDoc documentation above the method:

+/**
+ * Clicks an element within an iframe and waits for a specific response.
+ * @param frame - The iframe selector
+ * @param subUrl - The URL substring to match in the response
+ * @param frameSelector - The element selector within the iframe
+ * @param code - The expected response status code (default: 200)
+ * @returns Promise resolving to the matched Response
+ * @example
+ * const response = await page.clickFrameSelectorAndWaitForResponse('#chat-frame', '/api/send', '.send-button');
+ */
 async clickFrameSelectorAndWaitForResponse(frame: string, subUrl: string, frameSelector: string, code = 200): Promise<Response> {

1562-1566: Add JSDoc documentation for better API understanding.

The method implementation is correct, but would benefit from documentation explaining:

  • Purpose and use cases
  • Parameter descriptions
  • Options details
  • Example usage

Add JSDoc documentation above the method:

+/**
+ * Asserts that an element within an iframe is visible.
+ * @param frame - The iframe selector
+ * @param frameSelector - The element selector within the iframe
+ * @param options - Playwright visibility assertion options
+ * @example
+ * await page.toBeVisibleFrameLocator('#chat-frame', '.chat-window', { timeout: 5000 });
+ */
 async toBeVisibleFrameLocator(frame: string, frameSelector: string, options?: { timeout?: number; visible?: boolean } | undefined) {
tests/pw/pages/selectors.ts (1)

Line range hint 5914-5940: Consider adding type safety for chat message handling.

The chat message selectors could benefit from additional type safety and validation.

Consider adding validation for chat messages:

 vInbox: {
     chatPersons: 'div#hub',
     chatPerson: (personName: string) => `//div[@class="ConversationListItem__conversation-name" and normalize-space(text())="${personName}"]/../../..`,
+    // Add validation for message length and content
+    validateMessage: (message: string) => {
+        const MAX_LENGTH = 1000;
+        return message.length <= MAX_LENGTH;
+    },
     chatBox: 'div#chat-box',
     chatTextBox: '//div[@role="textbox"]',
     sendButton: 'button.send-button',
     sentMessage: (message: string) => `//div[@id="chat-box"]//span[@class="EntityTreeRenderer" and normalize-space(text())="${message}"]`
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between f31aa2c and 79424a0.

📒 Files selected for processing (18)
  • .github/workflows/e2e_api_tests.yml (1 hunks)
  • tests/pw/feature-map/feature-map.yml (2 hunks)
  • tests/pw/pages/basePage.ts (2 hunks)
  • tests/pw/pages/liveChatPage.ts (1 hunks)
  • tests/pw/pages/selectors.ts (12 hunks)
  • tests/pw/pages/settingsPage.ts (1 hunks)
  • tests/pw/pages/singleProductPage.ts (1 hunks)
  • tests/pw/pages/singleStorePage.ts (1 hunks)
  • tests/pw/pages/storeSupportsPage.ts (1 hunks)
  • tests/pw/pages/vendorSettingsPage.ts (2 hunks)
  • tests/pw/tests/e2e/_env.setup.ts (1 hunks)
  • tests/pw/tests/e2e/liveChat.spec.ts (1 hunks)
  • tests/pw/tests/e2e/settings.spec.ts (1 hunks)
  • tests/pw/tests/e2e/vendorSettings.spec.ts (1 hunks)
  • tests/pw/types/environment.d.ts (1 hunks)
  • tests/pw/utils/dbData.ts (3 hunks)
  • tests/pw/utils/interfaces.ts (2 hunks)
  • tests/pw/utils/testData.ts (7 hunks)
✅ Files skipped from review due to trivial changes (1)
  • tests/pw/pages/storeSupportsPage.ts
🧰 Additional context used
🪛 Biome
tests/pw/pages/liveChatPage.ts

[error] 14-16: This constructor is unnecessary.

Unsafe fix: Remove the unnecessary constructor.

(lint/complexity/noUselessConstructor)

🔇 Additional comments (17)
tests/pw/types/environment.d.ts (1)

33-34: LGTM! Ensure these credentials are properly secured.

The type definitions for TalkJS credentials are correctly typed and follow the established naming convention. Since these are sensitive credentials, ensure they are:

  • Stored securely in CI/CD secrets
  • Not exposed in logs or error messages
  • Added to the relevant documentation for local development setup

Let's verify the secure handling of these credentials in the CI workflow:

✅ Verification successful

Credentials are properly secured using GitHub Secrets ✅

The TalkJS credentials are correctly configured as GitHub Secrets in the e2e_api_tests.yml workflow:

  • TALKJS_APP_ID: ${{secrets.TALKJS_APP_ID}}
  • TALKJS_APP_SECRET: ${{secrets.TALKJS_APP_SECRET}}

This follows security best practices by not exposing sensitive values in the workflow file.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if TalkJS credentials are properly configured as secrets in GitHub Actions

# Test: Verify credentials are set as secrets in workflows
rg -l 'TALKJS_(APP_ID|APP_SECRET)' .github/workflows/

Length of output: 91


Script:

#!/bin/bash
# Check how TalkJS credentials are used in the workflow file
rg -A 5 -B 5 'TALKJS_(APP_ID|APP_SECRET)' .github/workflows/e2e_api_tests.yml

Length of output: 425

tests/pw/pages/singleProductPage.ts (2)

24-24: LGTM: Chat visibility check properly integrated

The addition of chatNow to the destructured elements is consistent with the PR's objective of adding live chat module tests. The visibility check will be performed as part of the multipleElementVisible call on the next line.


Line range hint 24-85: Verify chat-related selectors

The test relies on selectors defined in the imported selector object. Let's verify that the chat-related selectors are properly defined.

✅ Verification successful

The search results show that the chatNow selector is properly defined in selectors.ts with the value 'button.dokan-live-chat' and is correctly destructured in both singleStorePage.ts and singleProductPage.ts. Let me verify if this selector is actually used in the test code.


Chat-related selectors are properly defined and used

The verification shows that:

  • The chatNow selector is properly defined in selectors.ts as 'button.dokan-live-chat'
  • It's correctly destructured in the test file
  • The selector is actively used in liveChatPage.ts for visibility checks and interactions
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for chat-related selector definitions
rg -A 2 "chatNow.*[:=]" --type ts

Length of output: 1083


Script:

#!/bin/bash
# Search for usage of chatNow in test files
rg -A 2 "\.chatNow\b" --type ts

Length of output: 1556

tests/pw/tests/e2e/vendorSettings.spec.ts (1)

109-111: LGTM! The test case follows established patterns.

The new test case for live chat settings is well-structured and consistent with other vendor settings tests in the file. It properly uses the '@Pro' and '@Vendor' tags and follows the same pattern of using setStoreSettings.

Let's verify the dependencies for this test:

✅ Verification successful

Let me gather more specific information about the implementation and test data.


The test case is properly integrated with all required dependencies

The verification confirms that:

  • The liveChatSettings method exists in VendorSettingsPage and is called correctly from setStoreSettings
  • Test data includes proper liveChat configuration with all necessary fields (settingTitle, chatProvider, talkJsAppId, etc.)
  • TalkJS environment variables are properly configured in the CI workflow
  • The test follows the same pattern as other settings tests and uses the established infrastructure
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the existence of required methods and test data for live chat settings

# Test 1: Check if liveChatSettings method exists in VendorSettingsPage
ast-grep --pattern 'liveChatSettings($$$) {
  $$$
}'

# Test 2: Check if vendor test data includes liveChat configuration
rg -l "liveChat" --type ts

# Test 3: Check if TalkJS environment variables are properly configured in the workflow
rg "TALKJS_" .github/workflows/

Length of output: 701


Script:

#!/bin/bash
# Check the implementation of setStoreSettings method and liveChat related code
ast-grep --pattern 'setStoreSettings($$$) {
  $$$
}'

# Check test data structure for liveChat
rg "liveChat" tests/pw/utils/testData.ts -A 5

# Check if there are any specific selectors or elements for liveChat settings
rg "liveChat" tests/pw/pages/vendorSettingsPage.ts -A 5

Length of output: 1245

tests/pw/tests/e2e/settings.spec.ts (1)

107-109: LGTM! Well-structured test case addition.

The new test case follows the established pattern and maintains consistency with other settings tests in the suite.

Let's verify the required test dependencies:

✅ Verification successful

✓ Test dependencies verified and properly implemented

The verification confirms that:

  • The setDokanLiveChatSettings method is properly implemented in settingsPage.ts with all required functionality
  • Test data structure in testData.ts contains all necessary fields used by the implementation
  • The test follows the same pattern as other settings tests in the suite
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the existence of required test data and methods

# Test 1: Check if liveChat settings exist in test data
rg "liveChat.*[:=]" -g "**/*testData.ts"

# Test 2: Verify setDokanLiveChatSettings method implementation
ast-grep --pattern 'setDokanLiveChatSettings($$$) {
  $$$
}'

Length of output: 203


Script:

#!/bin/bash
# Let's check the actual implementation of setDokanLiveChatSettings and its test data structure

# Check the implementation
rg "setDokanLiveChatSettings" -A 10

# Check the test data structure
rg -A 5 "liveChat: \{" tests/pw/utils/testData.ts

Length of output: 2446

.github/workflows/e2e_api_tests.yml (1)

49-50: LGTM! Proper secret management for TalkJS credentials.

The TalkJS environment variables are correctly added and properly use GitHub secrets for secure credential management.

Let's verify that these secrets are properly configured in the repository:

tests/pw/pages/vendorSettingsPage.ts (2)

239-242: LGTM! The live chat case integration looks good.

The new case follows the established pattern and properly delegates to the liveChatSettings method.


430-434: Consider adding test scenarios for live chat settings.

To ensure robust testing of the live chat feature, consider adding test cases for:

  1. Enabling/disabling live chat
  2. Configuration changes (if supported)
  3. Error scenarios (e.g., invalid configuration)
  4. Integration with other vendor settings

Let's verify if there are any existing test cases:

tests/pw/utils/interfaces.ts (1)

818-821: LGTM! The vendor liveChat interface is well-structured.

The interface addition is clean and follows TypeScript conventions.

tests/pw/utils/dbData.ts (2)

1-1: LGTM! Environment variables properly configured.

The addition of TalkJS credentials as environment variables follows security best practices for handling sensitive configuration data.


901-902: LGTM! Live chat configuration is properly structured.

The live chat settings are well-configured with:

  • Feature enabled
  • TalkJS provider selected
  • Credentials properly sourced from environment variables

Also applies to: 904-905

tests/pw/utils/testData.ts (4)

1013-1016: LGTM: WooCommerce AJAX endpoints are correctly defined.

The cart operation endpoints follow WooCommerce's standard naming conventions and are properly prefixed.


1033-1033: LGTM: TalkJS API endpoint is correctly defined.

The endpoint correctly points to TalkJS's API domain for live chat integration.


1077-1085: LGTM: Live chat menu items and settings are properly configured.

The implementation includes:

  • Dashboard menu items for inbox and store support
  • Comprehensive live chat settings with provider selection, credentials, and display options

Also applies to: 2349-2358


2503-2503: LGTM: Random nano ID generator function is properly implemented.

The function uses faker's nanoid method consistently with other ID generation utilities in the file.

tests/pw/pages/selectors.ts (2)

Line range hint 2420-7828: Verify the impact of live chat integration across the codebase.

The live chat implementation spans multiple sections and may affect other parts of the application.

Let's verify the integration points:


7820-7828: 🛠️ Refactor suggestion

Consider improving iframe selector specificity.

The live chat iframe selector could be more specific to avoid potential conflicts.

Consider making the iframe selector more specific:

 cLiveChat: {
-    liveChatIframe: '(//iframe[@name="____talkjs__chat__ui_internal"])[last()]',
+    liveChatIframe: '(//div[contains(@id, "__talkjs_popup_container") and not(@style="display: none;")]//iframe[@name="____talkjs__chat__ui_internal"])[last()]',
     liveChatLauncher: 'a#__talkjs_launcher',
     chatBox: 'div#chat-box',
     chatTextBox: '//div[@role="textbox"]',
     sendButton: 'button.send-button',
     sentMessage: (message: string) => `//span[@class="EntityTreeRenderer" and normalize-space(text())="${message}"]`
 }

Likely invalid or redundant comment.

tests/pw/tests/e2e/liveChat.spec.ts Show resolved Hide resolved
tests/pw/tests/e2e/liveChat.spec.ts Outdated Show resolved Hide resolved
tests/pw/tests/e2e/liveChat.spec.ts Show resolved Hide resolved
tests/pw/tests/e2e/liveChat.spec.ts Show resolved Hide resolved
tests/pw/pages/singleProductPage.ts Show resolved Hide resolved
tests/pw/pages/liveChatPage.ts Outdated Show resolved Hide resolved
tests/pw/pages/liveChatPage.ts Outdated Show resolved Hide resolved
tests/pw/pages/liveChatPage.ts Outdated Show resolved Hide resolved
tests/pw/pages/liveChatPage.ts Outdated Show resolved Hide resolved
tests/pw/pages/liveChatPage.ts Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Outside diff range and nitpick comments (3)
tests/pw/pages/liveChatPage.ts (1)

18-24: Consider documenting the different load states.

The navigation methods are well-implemented. Consider adding a brief comment explaining why different load states are used ('networkidle' for store and 'domcontentloaded' for product) to help future maintainers.

tests/pw/pages/productsPage.ts (2)

Line range hint 1-1000: Consider splitting this class into smaller, more focused classes.

The ProductsPage class is quite large and handles multiple responsibilities (admin products, vendor products, product options, etc.). Consider refactoring into smaller, more focused classes:

  1. Split into separate classes:

    • AdminProductsPage
    • VendorProductsPage
    • ProductOptionsPage
  2. Extract common validation patterns into a shared base class or utility methods.

This would improve maintainability, testability, and adherence to the Single Responsibility Principle.

Example structure:

// adminProductsPage.ts
export class AdminProductsPage extends BasePage {
    // Admin-specific product management methods
}

// vendorProductsPage.ts
export class VendorProductsPage extends BasePage {
    // Vendor-specific product management methods
}

// productOptionsPage.ts
export class ProductOptionsPage extends BasePage {
    // Product options management methods
}

// productValidations.ts
export class ProductValidations {
    // Shared validation methods
}

Line range hint 1-1000: Enhance error handling for critical operations.

The code would benefit from more robust error handling, especially for critical operations like file uploads, network requests, and state-dependent UI interactions.

Consider implementing:

  1. Wrapper methods for critical operations:
private async safeUpload(selector: string, filePath: string): Promise<void> {
    try {
        await this.uploadFile(selector, filePath);
    } catch (error) {
        throw new Error(`Failed to upload file: ${error.message}`);
    }
}
  1. Retry mechanisms for flaky UI interactions:
private async retryClick(selector: string, maxAttempts = 3): Promise<void> {
    let lastError;
    for (let i = 0; i < maxAttempts; i++) {
        try {
            await this.click(selector);
            return;
        } catch (error) {
            lastError = error;
            await this.page.waitForTimeout(1000); // Wait before retry
        }
    }
    throw new Error(`Failed after ${maxAttempts} attempts: ${lastError.message}`);
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 79424a0 and 74793f1.

📒 Files selected for processing (4)
  • tests/pw/pages/liveChatPage.ts (1 hunks)
  • tests/pw/pages/productsPage.ts (1 hunks)
  • tests/pw/pages/selectors.ts (12 hunks)
  • tests/pw/tests/e2e/liveChat.spec.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/pw/tests/e2e/liveChat.spec.ts
🧰 Additional context used
🪛 Biome
tests/pw/pages/liveChatPage.ts

[error] 14-16: This constructor is unnecessary.

Unsafe fix: Remove the unnecessary constructor.

(lint/complexity/noUselessConstructor)

🔇 Additional comments (5)
tests/pw/pages/liveChatPage.ts (1)

1-12: LGTM! Well-organized imports and selectors.

The imports are properly structured and the selectors are well-organized into logical groups.

tests/pw/pages/productsPage.ts (1)

683-683: LGTM: Clean addition of chat feature selector.

The addition of the chatNow property to the destructuring pattern is well-structured and maintains consistency with the existing code pattern.

tests/pw/pages/selectors.ts (3)

2420-2432: LGTM! Live chat settings selectors are well organized and comprehensive.

The selectors cover all necessary elements for configuring live chat including enable/disable toggle, provider selection, and provider-specific settings like Facebook Messenger color, TalkJS credentials and WhatsApp configuration.


7819-7827: LGTM! Customer chat interface selectors are robust and handle dynamic scenarios.

The selectors properly handle:

  • Multiple chat instances through dynamic iframe selection
  • Chat launcher and text input elements
  • Message verification with text matching

6071-6072: LGTM! Simple and clear selector for vendor live chat toggle.

The selector is appropriately placed under vendor settings section.

tests/pw/pages/liveChatPage.ts Show resolved Hide resolved
Comment on lines +51 to +58
async sendMessageToVendor(storename: string, message: string): Promise<void> {
await this.gotoSingleStore(storename, true);
await this.click(singleStoreCustomer.storeTabs.chatNow);
await this.toBeVisible(liveChatCustomer.liveChatIframe);
await this.typeFrameSelector(liveChatCustomer.liveChatIframe, liveChatCustomer.chatTextBox, message);
await this.clickFrameSelectorAndWaitForResponse(liveChatCustomer.liveChatIframe, data.subUrls.frontend.talkjs, liveChatCustomer.sendButton);
await this.toBeVisibleFrameLocator(liveChatCustomer.liveChatIframe, liveChatCustomer.sentMessage(message));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add navigation validation before chat interactions.

Consider adding a validation step after navigation to ensure the store page is loaded correctly before proceeding with chat interactions.

async sendMessageToVendor(storename: string, message: string): Promise<void> {
    await this.gotoSingleStore(storename, true);
    // Add validation
    await this.toBeVisible(singleStoreCustomer.storeContent);  // Add appropriate selector
    await this.click(singleStoreCustomer.storeTabs.chatNow);
    // ... rest of the method
}

tests/pw/pages/liveChatPage.ts Show resolved Hide resolved
@shashwatahalder01 shashwatahalder01 added the QA approved This PR is approved by the QA team label Nov 1, 2024
@shashwatahalder01 shashwatahalder01 merged commit d96222c into getdokan:develop Nov 1, 2024
2 checks passed
@shashwatahalder01 shashwatahalder01 deleted the livechatModule branch November 1, 2024 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QA approved This PR is approved by the QA team Test Automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant