feat(plugin-cc): add agent logout #2

rarajes2 · 2024-10-29T09:54:16Z

COMPLETES #< SPARK-558556 >

This pull request addresses

Adds the Agent Station Logout feature

by making the following changes

Agent is able to Logout from station.

Change Type

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation update
Tooling change
Internal code refactor

The following scenarios where tested

Tested the station logout on the samples page after logging in with Browser.

I certified that

I have read and followed contributing guidelines
I discussed changes with code owners prior to submitting this pull request
I have not skipped any automated checks
All existing and new tests passed
I have updated the documentation accordingly

Make sure to have followed the contributing guidelines before submitting.

bhabalan · 2024-10-30T04:48:21Z

docs/samples/contact-center/index.html

@@ -112,6 +112,7 @@ <h2 class="collapsible">
                  <input id="dialNumber" name="dialNumber" placeholder="Dial Number" value="" type="text">
                  <button id="loginAgent" disabled class="btn btn-primary my-3" onclick="doAgentLogin()">Login With
                    Selected Team</button>
+                  <button id="logoutAgent" style="display: none;" class="btn btn-primary my-3 ml-2" onclick="logoutAgent()">Logout Agent</button>


Prefer using the already present 'hidden' class instead of display: none

bhabalan · 2024-10-30T04:49:03Z

packages/@webex/plugin-cc/src/cc.ts

+        this.webRTCCalling.deregisterWebCallingLine();
+      }
+
+      return response;


Add a log for the success case

this.agent.stationLogout is already adding the success log. Adding here will add it twice.

@rarajes2 , In that case, let's remove the logs from catch

bhabalan · 2024-10-30T04:50:38Z

packages/@webex/plugin-cc/src/features/Agent.ts

+
+    try {
+      const response = await this.agentService.stationLogout({
+        logoutReason,


The options param can be passed as is to agentService without the need for destructuring

bhabalan · 2024-10-30T04:52:47Z

packages/@webex/plugin-cc/src/services/AgentService.ts

+        eventType: 'Logout',
+        success: ['AgentLogoutSuccess'],
+        failure: ['AgentLogoutFailed'],


These strings can be constants

bhabalan · 2024-10-30T04:54:01Z

packages/@webex/plugin-cc/src/services/types.ts

@@ -236,3 +236,16 @@ export type SubscribeResponse = {
  };
  message: string | null;
 };
+
+export interface LogoutSuccess {
+  eventType: 'AgentDesktopMessage';


Use a constant for this string

bhabalan · 2024-10-30T04:54:06Z

packages/@webex/plugin-cc/src/services/types.ts

+  subStatus: string;
+  loggedOutBy?: string;
+  roles?: string[];
+  type: 'AgentLogoutSuccess';


Use a constant for this string

bhabalan · 2024-10-30T08:49:02Z

packages/@webex/plugin-cc/src/features/Agent.ts

+
+      return response;
+    } catch (error) {
+      return Promise.reject(error);


Add log for failures

Kesari3008 · 2024-11-04T05:09:50Z

packages/@webex/plugin-cc/src/services/constants.ts

+export const AgentDesktopMessage = 'AgentDesktopMessage';
+
+export const LOGOUT_EVENT = 'Logout';
+export const AgentLogoutSuccessEvent = 'AgentLogoutSuccess';


Constant names should be in caps

Kesari3008 · 2024-11-04T05:18:00Z

packages/@webex/plugin-cc/src/services/AgentService.ts

@@ -56,4 +65,28 @@ export default class AgentService {
      return Promise.reject(error);
    }
  }
+
+  public async stationLogout(options: {logoutReason: string}): Promise<LogoutSuccess> {


The return value should be generic right, either the promise resolves or rejects indicating success or failure

Kesari3008 · 2024-11-04T05:28:39Z

packages/@webex/plugin-cc/src/features/Agent.ts

+
+  public async stationLogout(options: {logoutReason: string}): Promise<LogoutSuccess> {
+    try {
+      const response = await this.agentService.stationLogout(options);


If 4xx or 5xx failure occurs, don't we need to handle that and reject promise

Updated the code, now all the logic is in the service, this function simply calls it and returns. It's just one liner now

Kesari3008 · 2024-11-04T05:29:25Z

packages/@webex/plugin-cc/src/features/Agent.ts

+  public async stationLogout(options: {logoutReason: string}): Promise<LogoutSuccess> {
+    try {
+      const response = await this.agentService.stationLogout(options);
+      this.webex.logger.log('Logout API SUCCESS');


It's better to say Logout Success and Logout Failure. We don't need to add API in the log messages.

Kesari3008 · 2024-11-04T05:30:01Z

packages/@webex/plugin-cc/src/features/Agent.ts

@@ -34,4 +34,15 @@ export default class Agent {
      return Promise.reject(new Error('Error while performing agent login', error));
    }
  }
+
+  public async stationLogout(options: {logoutReason: string}): Promise<LogoutSuccess> {


Let's call it data instead of options

Kesari3008 · 2024-11-04T05:32:46Z

packages/@webex/plugin-cc/test/unit/spec/features/Agent.ts

+
+  describe('#stationLogout', () => {


We should have tests added for changes in the cc.ts file as well and the same goes for AgentService.ts. And we should cover the failure response test case also

Added tests for cc and AgentService. Removed tests from Agent as it's one liner now.

Kesari3008 · 2024-11-04T05:33:52Z

Build seems to be failing with below error:
[@webex/plugin-cc]: src/WebRTCCalling.ts(8,8): error TS2307: Cannot find module '@webex/calling' or its corresponding type declarations.

Is this being taken care in any of the PRs?

Please add manual testing details in the PR description

Kesari3008

Approving with few comments. Please ensure to address these comments before merge

Even when logout was successful, timeout error is printing. Please fix this in sample app

Kesari3008 · 2024-11-07T14:32:21Z

packages/@webex/plugin-cc/src/services/constants.ts

+export const AgentDesktopMessage = 'AgentDesktopMessage';
+
+export const LOGOUT_EVENT = 'Logout';
+export const AGENT_LOGOUT_SUCCESS_EVENT = 'AgentLogoutSuccess';


These could be moved within CC_EVENTS itself but check with Ravi if we would even need CC_EVENTS enum since we are moving the aqm code

Kesari3008 · 2024-11-07T14:41:17Z

packages/@webex/plugin-cc/src/services/AgentService.ts

+
+      return response;
+    } catch (error) {
+      this.webex.logger.error(`Station logout failed: ${error}`);


Would we need to have a try-catch block here? I believe we can just return response as it is that we get from httpRequest.sendRequestWithEvent()

Kesari3008 · 2024-11-07T14:42:31Z

packages/@webex/plugin-cc/src/cc.ts

+   * @returns Promise<LogoutSuccess>
+   * @throws Error
+   */
+  public async stationLogout(data: {logoutReason: string}): Promise<StationLogoutResponse> {


Let's create a type for {logoutReason: string} as Logout similar and use that eveyrwhere

Kesari3008 · 2024-11-07T14:43:16Z

packages/@webex/plugin-cc/src/cc.ts

+      }
+
+      return response;
+    } catch (error) {


Would we need this catch block? I believe return response should suffice

rarajes2 · 2024-11-12T13:02:20Z

Closing as this PR changes has been taken in Ravi's PR.

* feat(cc-sdk): ported-code-for-health-check * feat(cc-sdk): fixed-broken-tests * feat(cc-sdk): fixed-blob-issue * feat(cc-sdk): merged-with-ravi-pr * feat(cc-sdk): re-added-cc-code * feat(cc-sdk): added-logger-proxy * feat(cc-sdk): added-config-values * feat(cc-sdk): added-connection-instance * feat(cc-sdk): added-event-listeners-instead-of-signals * feat(cc-sdk): fixed-broken-tests * feat(cc-sdk): fixed-test-coverage

rarajes2 force-pushed the station-logout-cc branch from 740a4f0 to 57015ae Compare October 29, 2024 13:00

bhabalan reviewed Oct 30, 2024

View reviewed changes

rarajes2 force-pushed the station-logout-cc branch 2 times, most recently from d09dc17 to e8da707 Compare October 30, 2024 08:39

rarajes2 added the validated label Oct 30, 2024

bhabalan reviewed Oct 30, 2024

View reviewed changes

bhabalan approved these changes Oct 30, 2024

View reviewed changes

arun3528 force-pushed the AgentLogin branch 2 times, most recently from 2ece117 to ac5285e Compare October 31, 2024 13:30

Kesari3008 suggested changes Nov 4, 2024

View reviewed changes

rarajes2 force-pushed the station-logout-cc branch from e8da707 to 1ec7373 Compare November 5, 2024 03:19

feat(plugin-cc): add agent logout

038b4bb

rarajes2 force-pushed the station-logout-cc branch from 1ec7373 to 61f790d Compare November 7, 2024 06:43

fix(plugin-cc): address review comments

f951248

rarajes2 force-pushed the station-logout-cc branch from 61f790d to f951248 Compare November 7, 2024 06:58

Kesari3008 approved these changes Nov 7, 2024

View reviewed changes

rarajes2 closed this Nov 12, 2024

feat(plugin-cc): add agent logout #2

feat(plugin-cc): add agent logout #2

Conversation

rarajes2 commented Oct 29, 2024 • edited Loading

COMPLETES #< SPARK-558556 >

This pull request addresses

by making the following changes

Change Type

The following scenarios where tested

I certified that

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kesari3008 commented Nov 4, 2024 • edited Loading

Kesari3008 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rarajes2 commented Nov 12, 2024

rarajes2 commented Oct 29, 2024 •

edited

Loading

Kesari3008 commented Nov 4, 2024 •

edited

Loading

Kesari3008 left a comment •

edited

Loading