feat: WxCC SDK Subscribe Notifications and establish Mercury Connection #3909
Conversation
| const handlers = []; | ||
| if (!eventType) { | ||
| return handlers; |
There was a problem hiding this comment.
can you add comments on what is a handler and how it looks like
There was a problem hiding this comment.
example of handlers w.r.t existing mercury
[
{
"name": "processKmsMessageEvent",
"namespace": "encryption"
}
]
There was a problem hiding this comment.
_getEventHandlers(eventType) {
const handlers = [];
if (!eventType) {
return handlers;
}
const [namespace, name] = eventType.split('.');
if (!this.webex[namespace] && !this.webex.internal[namespace]) {
return handlers;
}
const handlerName = camelCase(`process_${name}_event`);
if ((this.webex[namespace] || this.webex.internal[namespace])[handlerName]) {
handlers.push({
name: handlerName,
namespace,
});
}
return handlers;
},
There was a problem hiding this comment.
Please fix the above comment.. looks like half of it is in code while the other half is text.
packages/@webex/internal-plugin-mercury/src/socket/socket-base.js
Outdated
Show resolved
Hide resolved
| * @param {object} body | ||
| * @returns {Promise<void>} | ||
| */ | ||
| register(datachannelUrl, body: object): Promise<void> { |
There was a problem hiding this comment.
should this funciton be called connect , connectDataChannel , connectSocket or something !?
| deviceRegistrationRequired: false, | ||
| }; | ||
|
|
||
| const CCMercury = Mercury.extend({ |
There was a problem hiding this comment.
can you try if something like this can be used
import AmpState from 'ampstate';
// Define the interface for your extended Mercury class
interface ExtendedMercury extends AmpState {
// Add any custom properties or methods you need
customProperty: string;
customMethod(): void;
}
// Create the extended Mercury class
class CCMercury extends AmpState implements ExtendedMercury {
customProperty = 'initial value';
customMethod() {
console.log('Custom method called');
}
}
// Use the extended Mercury class
const mercuryInstance: ExtendedMercury = new CCMercury();
console.log(mercuryInstance.customProperty);
mercuryInstance.customMethod();
There was a problem hiding this comment.
@arun3528 Can you please elaborate more. because in the above mentioned code, existing Mercury is not extended.
We need to extend existing Mercury class.
There was a problem hiding this comment.
As discussed added it as a modern TS class named 'WebSocket'
rsarika
changed the title
| @@ -511,13 +516,15 @@ const Mercury = WebexPlugin.extend({ | |||
| ) | |||
| .then(() => { | |||
| this._emit('event', event.data); | |||
| const [namespace] = data.eventType.split('.'); | |||
| if (data.eventType) { | |||
There was a problem hiding this comment.
just a check for data.eventType to avoid undefined/null error
| import webSocketConfig from './config'; | ||
| import IWebSocket from './IWebSocket'; | ||
|
|
||
| class WebSocket extends (Mercury as any) implements IWebSocket { |
There was a problem hiding this comment.
Why did we need to add as any here? I see mercury.d.ts file is deleted now so was this change done as an alternative for the same
There was a problem hiding this comment.
yes. this is the alternative to d.ts file and to impose type using interface IWebSocket
There was a problem hiding this comment.
Did we try making it work with using as any here for Mercury? I removed it and it gives simple type error for request and connect function which should get resoolved if we add them in the interface I believe. Please try it out and address it before merge
There was a problem hiding this comment.
Did we get chance to try this out ?
| */ | ||
| public unregister(): Promise<void> { | ||
| return this.webSocket.disconnectWebSocket().then(() => { | ||
| this.webSocket.off(EVENT, this.processEvent); |
There was a problem hiding this comment.
Would it make sense to switch off the listener after the disconnect has already happened? This should happened in disconnectWebsocket function itself I believe
There was a problem hiding this comment.
Can we get a response on this also?
| registered = false; | ||
| eventHandlers: Map< | ||
| string, | ||
| {resolve: (data: any) => void; reject: (error: any) => void; timeoutId: NodeJS.Timeout} |
There was a problem hiding this comment.
Any should be avoided here also
| }); | ||
|
|
||
| const result = await promise; | ||
| expect(webSocketMock.subscribeAndConnect).toHaveBeenCalled(); |
There was a problem hiding this comment.
These 2 expect statements can be combined into one and use toHaveBeenCalledOnceWith. Applies everywhere
| import webSocketConfig from './config'; | ||
| import IWebSocket from './IWebSocket'; | ||
|
|
||
| class WebSocket extends (Mercury as any) implements IWebSocket { |
There was a problem hiding this comment.
Did we get chance to try this out ?
| */ | ||
| public unregister(): Promise<void> { | ||
| return this.webSocket.disconnectWebSocket().then(() => { | ||
| this.webSocket.off(EVENT, this.processEvent); |
There was a problem hiding this comment.
Can we get a response on this also?
There was a problem hiding this comment.
Approving with a couple of questions.
- Why am I seeing this error constantly in the console
25 logger.js:396 wx-js-sdk socket,api.wxcc-us1.cisco.com: error while receiving WebSocket message TypeError: Cannot read properties of undefined (reading 'eventType')
- The text box under the webex.cc.register() still says "Not Subscribed" after the subscribe POST works fine. Is that expected?
| @@ -0,0 +1,52 @@ | |||
| import {WebSocketEvent} from '../types'; | |||
|
|
|||
| // ts doc | |||
There was a problem hiding this comment.
Why is this comment needed?
COMPLETES #https://jira-eng-gpk2.cisco.com/jira/browse/SPARK-558547
This pull request addresses
The changes in this PR addresses the code to subscribe notifications and establish mercury connection with CC websocket.
by making the following changes
< DESCRIBE YOUR CHANGES >
Made changes in mercury plugin to accept additional config to skip authentication and ackowledgement.
Added code to call subscribe api and call mercury connection with websocket url.
Created new file MercuryCC.ts to extend Mercury and provide additional config and subscribe notifications.
Change Type
The following scenarios were tested
< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >
Updated samples page with register method.
If we click on register method it call subscribe notification API and gets Welcome Message as part of WebSocket.
We can see Welcome Message and CI-USerId in console logs.
This can be used to fetch agent profile in further PR's
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.