fix: Moderated unmute mode not working when web client is remotely muted #4039
Conversation
…ebex#3797)" This reverts commit d354100.
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
warning [email protected]: This version is no longer supported. Please see https://eslint.org/version-support for other options. (For a CapTP with native promises, see @endo/eventual-send and @endo/captp) WalkthroughThis pull request introduces modifications to the mute state handling in the Webex meetings plugin. Changes are focused on two primary files: Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🔇 Additional comments (3)packages/@webex/plugin-meetings/src/locus-info/selfUtils.ts (1)
The added check prevents triggering unnecessary updates when users mute themselves, which aligns with the PR objective of fixing moderated unmute mode. This change correctly filters out self-initiated mute actions while still allowing moderated unmute functionality to work as expected. Let's verify the usage of this condition: ✅ Verification successfulBased on the search results, I can generate the final response since we have sufficient information to verify the change: Approve the optimization to prevent unnecessary mute state updates. The added check to verify if the user who modified the mute state is the same as the user being muted is a safe optimization. The search results show:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for any other places in the codebase that might be affected by this change
# Search for other references to modifiedBy in relation to mute state
rg -A 5 'modifiedBy.*mute|mute.*modifiedBy'
# Search for other methods that might need similar self-modification checks
ast-grep --pattern 'function $_(oldSelf, changedSelf) {
$$$
changedSelf.selfIdentity
$$$
}'
Length of output: 66244 packages/@webex/plugin-meetings/src/meeting/muteState.ts (1)
The removal of conditional logic simplifies the control flow and ensures consistent behavior when handling server-initiated remote mute updates. This change aligns with the PR objective and improves the reliability of the moderated unmute mode. Let's verify the impact of this change: ✅ Verification successfulLet me generate additional scripts to verify the impact of this change. Let me try one more search to find the implementation and tests. Based on the code context, I can now generate the final response: The simplified remote mute handling is safe and correct. The change to directly call
The simplification removes unnecessary conditional logic while preserving all the important state management and muting functionality. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for any dependencies on the previous behavior
# Search for tests or code that might depend on the previous conditional behavior
rg -A 5 'handleServerRemoteMuteUpdate.*muted'
# Search for similar patterns in other methods that might need similar simplification
ast-grep --pattern 'handleServer$_(meeting, $_, $_) {
$$$
muteLocalStream
$$$
}'
Length of output: 131070 Script: #!/bin/bash
# Search for the handleServerRemoteMuteUpdate method implementation and its context
rg -A 10 -B 10 'handleServerRemoteMuteUpdate'
# Search for tests related to remote mute functionality
rg -A 5 'handleServerRemoteMuteUpdate.*test'
# Search for muteLocalStream usage to understand the muting flow
ast-grep --pattern 'muteLocalStream($_, $_, $_)'
Length of output: 131070 Script: #!/bin/bash
# Search for the handleServerRemoteMuteUpdate method and its tests
rg -A 10 -B 10 'handleServerRemoteMuteUpdate' packages/@webex/plugin-meetings/src/meeting/muteState.ts packages/@webex/plugin-meetings/test/unit/spec/muteState.js
Length of output: 2508 packages/@webex/plugin-meetings/test/unit/spec/locus-info/selfUtils.js (1)
The updated test cases provide thorough coverage of the mute state handling logic, including:
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 (
|
marcin-bazyl
added
the
validated
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
This is a revert of a PR from a few months ago, because the root cause has been fixed in Locus now (see WEBEX-410234 ), so we can now again rely on the
metadata.modifiedByto ignore Locus DTO updates caused by our own actions.3 separate scenarios described in these jiras should be working correctly now:
https://jira-eng-gpk2.cisco.com/jira/browse/SPARK-555818
https://jira-eng-gpk2.cisco.com/jira/browse/WEBEX-382445
https://jira-eng-gpk2.cisco.com/jira/browse/SPARK-583170
TODO: confirm that Locus fix is deployed to prod and then test above scenarios
Summary by CodeRabbit
New Features
Bug Fixes
Tests