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

feat(voicea-highlight): added-highlights #3807

Draft
wants to merge 3 commits into
base: next
Choose a base branch
from

Conversation

adhmenon
Copy link
Contributor

@adhmenon adhmenon commented Sep 4, 2024

COMPLETES #< SPARK-509136 >

This pull request addresses

  • Added transcription highlighting to the meeting sdk.
  • NOTE - This only works for orgs which have webex ai assistant toggle turned off (new one)

by making the following changes

  • Added code to talk to locus on the old webex assistant so as to enable highlighting.
  • Emmitting a highlight event from meeting sdk with the required object.
  • Added code changes to the sample app to utilise this.
  • Added unit tests.

Screenshot of highlights object

Screenshot 2024-09-04 at 11 21 16 AM

Screenshot of Sample App

Screenshot 2024-09-04 at 11 23 34 AM

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

< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >

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.

@@ -183,6 +184,17 @@ export type CaptionData = {
speaker: string;
};

export type Highlight = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We would need a new type for highlights object.

@@ -365,6 +365,7 @@ export const EVENT_TRIGGERS = {
MEETING_STOPPED_RECEIVING_TRANSCRIPTION: 'meeting:receiveTranscription:stopped',
MEETING_MANUAL_CAPTION_UPDATED: 'meeting:manualCaptionControl:updated',
MEETING_CAPTION_RECEIVED: 'meeting:caption-received',
MEETING_HIGHLIGHT_CREATED: 'meeting:highlight-created',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New event to be emitted from meeting object to the user.

* @public
* @returns {Promise<void>} a promise to open the WebSocket connection
*/
public async toggleHighlighting(activate: boolean, options?: {spokenLanguage?: string}) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rather than a separate stop or start method, we can use toggle which enabled users to toggle the highlighting setting. This way we can also compact all the logic inside of this method.

});
}
} else {
throw new Error('This operation is not allowed in your org. Contact org administrator.');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We throw this error if we find that transcriptions are not enabled. Usually happens if the old webex assistant is not enabled for the org.

@@ -118,3 +121,41 @@ export const processNewCaptions = ({
}
transcriptData.interimCaptions[transcriptId] = interimTranscriptionIds;
};

export const processHighlightCreated = ({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We simply process the highlights voicea gives back and pack it into the meetings object as an array.

@@ -1188,6 +1188,49 @@ describe('plugin-meetings', () => {
});
});

describe('#toggleHighlighting', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added tests to check all the flows (toggling). Had to mock a lot of data.


describe('processHighlightCreated', () => {
beforeEach(() => {
fakeVoiceaPayload = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here too - had to mock a lot of values in order to properly unit test the required method.

@@ -1104,6 +1106,44 @@ async function toggleTranscription(enable = false){
}
}

async function toggleHighlights() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using toggle is an easier way for users to access highlighting.
Here, we check for the button data status and if it is set as 'true', it indicated that it was turned on and hence needs to be switched off - hence the first block is that flow while the second block is the flow for turning on the highlights.

@@ -1144,6 +1184,9 @@ function setTranscriptEvents() {
meeting.on('meeting:receiveTranscription:stopped', () => {
generalToggleTranscription.innerText = "Start Transcription";
generalTranscriptionContent.innerHTML = 'Transcription Content: Webex Assistant must be enabled, check the console!';
generalHighlightTranscription.setAttribute('data-enabled', "false");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMPORTANT - Here we need to disable the transcription button and replace the text as when transcription is stopped highlighting no longer sends events.
Also - this gets triggered when we close the meeting also.

I'm not a 100% sure about this method (open to any suggestions). However, since this works for the sample app - I feel it should work.

@@ -851,13 +851,17 @@ <h2 class="collapsible">
<div class="btn-group u-mb">
<button id="gc-toggle-transcription" type="button"
onclick="toggleTranscription()" class="btn-code" data-enabled="false">Start Transcription</button>
<button id="gc-toggle-highlights" type="button"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Creating new UI for this change.

Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-3807.d3m3l2kee0btzx.amplifyapp.com

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant