Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions docs/react-samples/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {UserState} from '@webex/cc-user-state';
function App() {
const [isSdkReady, setIsSdkReady] = useState(false);
const [accessToken, setAccessToken] = useState("");
const [isLoggedIn, setIsLoggedIn] = useState(false);

const webexConfig = {
fedramp: false,
Expand All @@ -16,10 +17,12 @@ function App() {

const onLogin = () => {
console.log('Agent login has been succesful');
setIsLoggedIn(true);
}
Comment thread
mkesavan13 marked this conversation as resolved.

const onLogout = () => {
console.log('Agent logout has been succesful');
setIsLoggedIn(false);
}

return (
Expand All @@ -39,12 +42,13 @@ function App() {
});
}}
>Init Widgets</button>
{/* write code to check if sdk is ready and load components */}
{
isSdkReady && (
<>
<StationLogin onLogin={onLogin} onLogout={onLogout} />
<UserState />
{
isLoggedIn && <UserState />
}
</>
)
}
Expand Down
12 changes: 10 additions & 2 deletions docs/web-component-samples/app.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
const widgetsContainer = document.getElementById('widgets-container');
const accessTokenElem = document.getElementById('access_token_elem');
const widgetsContainer = document.getElementById('widgets-container');
const ccStationLogin = document.getElementById('cc-station-login');
const ccUserState = document.createElement('widget-cc-user-state');

if (!ccStationLogin && !ccUserState) {
console.error('Failed to find the required elements');
}

function switchButtonState(){
const buttonElem = document.querySelector('button');
Expand All @@ -20,16 +25,19 @@ function initWidgets(){
}).then(() => {
ccStationLogin.onLogin = loginSuccess;
ccStationLogin.onLogout = logoutSuccess;
widgetsContainer.classList.remove('disabled');
ccStationLogin.classList.remove('disabled');
}).catch((error) => {
console.error('Failed to initialize widgets:', error);
});
}

function loginSuccess(){
console.log('Agent login has been succesful');
ccUserState.classList.remove('disabled');
Comment on lines 35 to +36

@coderabbitai coderabbitai Bot Dec 5, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Replace console logs with proper logging utility.

Production code should use a proper logging utility instead of console.log/error.

Consider using the logger from webexConfig:

 function loginSuccess(){
-    console.log('Agent login has been succesful');
+    webexConfig.logger.log('Agent login has been succesful');
     ccUserState.classList.remove('disabled');
 }

 function logoutSuccess(){
-    console.log('Agent logout has been succesful');
+    webexConfig.logger.log('Agent logout has been succesful');
     ccUserState.classList.add('disabled');
 }

Also applies to: 35-36

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a sample app change. Not going to do it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@mkesavan13, understood. Since this is a sample app change, it's acceptable to use console.log here. I'll keep that in mind for future reviews.


✏️ Learnings added
Learnt from: mkesavan13
PR: webex/widgets#347
File: docs/web-component-samples/app.js:30-31
Timestamp: 2024-12-06T00:30:30.668Z
Learning: In sample app code within this repository, using `console.log` for logging is acceptable, and replacing it with a proper logging utility is not required.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

widgetsContainer.appendChild(ccUserState);
}

function logoutSuccess(){
console.log('Agent logout has been succesful');
ccUserState.classList.add('disabled');
}
5 changes: 2 additions & 3 deletions docs/web-component-samples/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,8 @@ <h1>Contact Center widgets as web-component</h1>
autocapitalize="off"
/>
<button onclick="initWidgets()" disabled>Init Widgets</button>
<div id="widgets-container" class="disabled">
<widget-cc-station-login id="cc-station-login"></widget-cc-station-login>
<widget-cc-user-state></widget-cc-user-state>
<div id="widgets-container">
<widget-cc-station-login class="disabled" id="cc-station-login"></widget-cc-station-login>
</div>
<script src="dist/bundle.js"></script>
<script src="app.js"></script>
Expand Down
10 changes: 7 additions & 3 deletions packages/contact-center/station-login/src/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ export const useStationLogin = (props: UseStationLoginProps) => {
cc.stationLogin({teamId: team, loginOption: deviceType, dialNumber: dialNumber})
.then((res: StationLoginSuccess) => {
setLoginSuccess(res);
loginCb();
if(loginCb){
Comment thread
Shreyas281299 marked this conversation as resolved.
loginCb();
}
}).catch((error: Error) => {
console.error(error);
setLoginFailure(error);
Expand All @@ -28,8 +30,10 @@ export const useStationLogin = (props: UseStationLoginProps) => {
cc.stationLogout({logoutReason: 'User requested logout'})
.then((res: StationLogoutSuccess) => {
setLogoutSuccess(res);
logoutCb();
}).catch((error: any) => {
if(logoutCb){
logoutCb();
}
}).catch((error: Error) => {
console.error(error);
Comment thread
Shreyas281299 marked this conversation as resolved.
});
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ export interface IStationLoginProps {
/**
* Callback function to be invoked once the agent login is successful
*/
onLogin: () => void;
onLogin?: () => void;

/**
* Callback function to be invoked once the agent login is successful
*/
onLogout: () => void;
onLogout?: () => void;

/**
* Handler to set device type
Expand Down
147 changes: 92 additions & 55 deletions packages/contact-center/station-login/tests/helper.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {renderHook, act} from '@testing-library/react-hooks';
import {renderHook, act, waitFor} from '@testing-library/react';
import {useStationLogin} from '../src/helper';

// Mock webex instance
Expand All @@ -18,6 +18,10 @@ const loginCb = jest.fn();
const logoutCb = jest.fn();

describe('useStationLogin Hook', () => {

afterEach(() => {
jest.clearAllMocks();
});

it('should set loginSuccess on successful login', async () => {
const successResponse = {
Expand Down Expand Up @@ -45,7 +49,7 @@ describe('useStationLogin Hook', () => {

ccMock.stationLogin.mockResolvedValue(successResponse);

const { result, waitForNextUpdate } = renderHook(() =>
const { result } = renderHook(() =>
useStationLogin({cc: ccMock, onLogin: loginCb, onLogout: logoutCb})
);

Expand All @@ -57,25 +61,42 @@ describe('useStationLogin Hook', () => {
result.current.login();
});

await waitForNextUpdate();

expect(ccMock.stationLogin).toHaveBeenCalledWith({
teamId: loginParams.teamId,
loginOption: loginParams.loginOption,
dialNumber: loginParams.dialNumber,
waitFor(() => {
expect(ccMock.stationLogin).toHaveBeenCalledWith({
teamId: loginParams.teamId,
loginOption: loginParams.loginOption,
dialNumber: loginParams.dialNumber,
});
expect(loginCb).toHaveBeenCalledWith();

expect(result.current).toEqual({
name: 'StationLogin',
setDeviceType: expect.any(Function),
setDialNumber: expect.any(Function),
setTeam: expect.any(Function),
login: expect.any(Function),
logout: expect.any(Function),
loginSuccess: successResponse,
loginFailure: undefined,
logoutSuccess: undefined
});
});
expect(loginCb).toHaveBeenCalledWith();

expect(result.current).toEqual({
name: 'StationLogin',
setDeviceType: expect.any(Function),
setDialNumber: expect.any(Function),
setTeam: expect.any(Function),
login: expect.any(Function),
logout: expect.any(Function),
loginSuccess: successResponse,
loginFailure: undefined,
logoutSuccess: undefined
});

it('should not call login callback if not present', async () => {

ccMock.stationLogin.mockResolvedValue({});

const { result } = renderHook(() =>
useStationLogin({cc: ccMock, onLogout: logoutCb})
);

act(() => {
result.current.login();
});

waitFor(() => {
expect(loginCb).not.toHaveBeenCalled();
});
});

Expand All @@ -84,7 +105,7 @@ describe('useStationLogin Hook', () => {
ccMock.stationLogin.mockRejectedValue(errorResponse);

loginCb.mockClear();
const { result, waitForNextUpdate } = renderHook(() =>
const { result } = renderHook(() =>
useStationLogin({cc: ccMock, onLogin: loginCb, onLogout: logoutCb})
);

Expand All @@ -96,26 +117,26 @@ describe('useStationLogin Hook', () => {
result.current.login();
});

await waitForNextUpdate();

expect(ccMock.stationLogin).toHaveBeenCalledWith({
teamId: loginParams.teamId,
loginOption: loginParams.loginOption,
dialNumber: loginParams.dialNumber,
});

expect(loginCb).not.toHaveBeenCalledWith();

expect(result.current).toEqual({
name: 'StationLogin',
setDeviceType: expect.any(Function),
setDialNumber: expect.any(Function),
setTeam: expect.any(Function),
login: expect.any(Function),
logout: expect.any(Function),
loginSuccess: undefined,
loginFailure: errorResponse,
logoutSuccess: undefined
waitFor(() => {
expect(ccMock.stationLogin).toHaveBeenCalledWith({
teamId: loginParams.teamId,
loginOption: loginParams.loginOption,
dialNumber: loginParams.dialNumber,
});
expect(loginCb).not.toHaveBeenCalledWith();

expect(result.current).toEqual({
name: 'StationLogin',
setDeviceType: expect.any(Function),
setDialNumber: expect.any(Function),
setTeam: expect.any(Function),
login: expect.any(Function),
logout: expect.any(Function),
loginSuccess: undefined,
loginFailure: errorResponse,
logoutSuccess: undefined
});
});
});

Expand All @@ -137,30 +158,46 @@ describe('useStationLogin Hook', () => {

ccMock.stationLogout.mockResolvedValue(successResponse);

const {result, waitForNextUpdate} = renderHook(() =>
const {result} = renderHook(() =>
useStationLogin({cc: ccMock, onLogin: loginCb, onLogout: logoutCb})
);

act(() => {
result.current.logout();
});

await waitForNextUpdate();
waitFor(() => {
expect(ccMock.stationLogout).toHaveBeenCalledWith({logoutReason: 'User requested logout'});
expect(logoutCb).toHaveBeenCalledWith();


expect(result.current).toEqual({
name: 'StationLogin',
setDeviceType: expect.any(Function),
setDialNumber: expect.any(Function),
setTeam: expect.any(Function),
login: expect.any(Function),
logout: expect.any(Function),
loginSuccess: undefined,
loginFailure: undefined,
logoutSuccess: successResponse
});
});
});

expect(ccMock.stationLogout).toHaveBeenCalledWith({logoutReason: 'User requested logout'});
expect(logoutCb).toHaveBeenCalledWith();
it('should not call logout callback if not present', async () => {
ccMock.stationLogout.mockResolvedValue({});

const {result} = renderHook(() =>
useStationLogin({cc: ccMock, onLogin: loginCb})
);

act(() => {
result.current.logout();
});

expect(result.current).toEqual({
name: 'StationLogin',
setDeviceType: expect.any(Function),
setDialNumber: expect.any(Function),
setTeam: expect.any(Function),
login: expect.any(Function),
logout: expect.any(Function),
loginSuccess: undefined,
loginFailure: undefined,
logoutSuccess: successResponse
waitFor(() => {
expect(logoutCb).not.toHaveBeenCalled();
});
});
})
2 changes: 0 additions & 2 deletions packages/contact-center/store/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@
},
"jest": {
"testEnvironment": "jsdom",
"//": "We can remove this when we have tests",
"passWithNoTests": true,
"testMatch": [
"**/tests/**/*.ts",
"**/tests/**/*.tsx"
Expand Down
6 changes: 5 additions & 1 deletion packages/contact-center/store/src/store.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import {makeAutoObservable, observable} from 'mobx';
import Webex from 'webex';
import {
AgentLogin,
IContactCenter,
Profile,
Team,
WithWebex,
IdleCode,
InitParams,
IStore
} from './store.types';
Expand All @@ -14,6 +14,8 @@ class Store implements IStore {
teams: Team[] = [];
loginOptions: string[] = [];
cc: IContactCenter;
idleCodes: IdleCode[] = [];
agentId: string = '';

constructor() {
makeAutoObservable(this, {cc: observable.ref});
Expand All @@ -24,6 +26,8 @@ class Store implements IStore {
return this.cc.register().then((response: Profile) => {
this.teams = response.teams;
Comment thread
Shreyas281299 marked this conversation as resolved.
this.loginOptions = response.loginVoiceOptions;
this.idleCodes = response.idleCodes;
this.agentId = response.agentId;
}).catch((error) => {
console.error('Error registering contact center', error);
return Promise.reject(error);
Expand Down
10 changes: 10 additions & 0 deletions packages/contact-center/store/src/store.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,19 @@ interface WithWebexConfig {

type InitParams = WithWebex | WithWebexConfig;

type IdleCode = {
name: string;
id: string;
isSystem: boolean;
isDefault: boolean;
}

interface IStore {
teams: Team[];
loginOptions: string[];
cc: IContactCenter;
idleCodes: IdleCode[];
Comment thread
Shreyas281299 marked this conversation as resolved.
agentId: string;

registerCC(webex: WithWebex['webex']): Promise<Profile>;
init(params: InitParams): Promise<void>;
Expand All @@ -26,6 +35,7 @@ export type {
Team,
AgentLogin,
WithWebex,
IdleCode,
InitParams,
IStore
}
Loading