-
Notifications
You must be signed in to change notification settings - Fork 344
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(plugin-meetings): Introduce SDK changes to call WCME and Locus APIs when current user steps away or returns #3942
base: brb-feature
Are you sure you want to change the base?
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With 2 comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good! I left a rather lengthy comment about the timing of sending the "away" state. Ideally, we'd like to send the "away" state as soon as possible, and with how it is right now, I suspect we'd see a noticeable delay.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with Marcin about checking transcoded meetings, though we don't have to do it in this PR (as long as you stick a TODO in there). Left a small question about the event we're emitting but other than that looks good.
packages/@webex/plugin-meetings/test/unit/spec/locus-info/index.js
Outdated
Show resolved
Hide resolved
packages/@webex/plugin-meetings/test/unit/spec/locus-info/index.js
Outdated
Show resolved
Hide resolved
packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js
Outdated
Show resolved
Hide resolved
packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js
Outdated
Show resolved
Hide resolved
packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js
Outdated
Show resolved
Hide resolved
471268c
to
ce7757f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Left a super minor comment and a small question, but none of them are blocking are I've approved it.
COMPLETES # SPARK-559645
This pull request addresses
Introduce SDK changes to call WCME and Locus APIs when the current user steps away or returns.
Related PRs:
by making the following changes
This PR only contains updates for SDK logic for the sender mostly.
Since the PR is still under development with unit tests (https://jira-eng-gpk2.cisco.com/jira/browse/SPARK-574651), it will be presented as a draft.
Also, for now, it will contain local linking
"link:../../../../webrtc-media-core"
for better dx until WCME changes will be ready and release.Also, either this PR or a separate one will be updated logic for test-app.
How to test:
Be right back true / Be right back false
buttons in "Participants, Breakout Sessions, and Events published" section. Check the console with data about Be right back requests (for examplelocus-info:updateSelf->event#SELF_MEETING_BRB_CHANGED
)Change Type
The following scenarios were tested
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.