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

[$500] Unable to mute all sounds on App #36455

Closed
4 of 6 tasks
m-natarajan opened this issue Feb 14, 2024 · 18 comments
Closed
4 of 6 tasks

[$500] Unable to mute all sounds on App #36455

m-natarajan opened this issue Feb 14, 2024 · 18 comments
Assignees

Comments

@m-natarajan
Copy link

m-natarajan commented Feb 14, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: 1.4.41.1
Reproducible in staging?: Y
Reproducible in production?: No (New feature)
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause internal team
Slack conversation:

Action Performed:

  1. Open app on two devices
  2. Go to Preferences > On "Mute all sounds from Expensify".
  3. Send message from one device to another

Expected Result:

There is no audio actually played.

Actual Result:

Done sound. Unable to mute all sounds on App. "Mute all sounds from Expensify" button doesn't function.

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6378819_1707874989063.RPReplay_Final1707855567.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~019ce6ba6ef040e21d
  • Upwork Job ID: 1757587986193575936
  • Last Price Increase: 2024-02-14
@m-natarajan m-natarajan added DeployBlockerCash This issue or pull request should block deployment External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Feb 14, 2024
Copy link

melvin-bot bot commented Feb 14, 2024

Job added to Upwork: https://www.upwork.com/jobs/~019ce6ba6ef040e21d

@melvin-bot melvin-bot bot changed the title Unable to mute all sounds on App [$500] Unable to mute all sounds on App Feb 14, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 14, 2024
Copy link

melvin-bot bot commented Feb 14, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @alitoshmatov (External)

Copy link

melvin-bot bot commented Feb 14, 2024

Triggered auto assignment to @abekkala (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Feb 14, 2024
Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

Copy link

melvin-bot bot commented Feb 14, 2024

Auto-assign attempt failed, all eligible assignees are OOO.

@ikevin127
Copy link
Contributor

ikevin127 commented Feb 14, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

Unable to mute all sounds -> "Mute all sounds from Expensify" toggle doesn't seem to work as expected.

What is the root cause of that problem?

This issue is coming from PR #31055.

When a user on device A toggles ON the "Mute all sounds from Expensify" this sets the isMutedAllSounds to true only locally via:

function setMuteAllSounds(isMutedAllSounds: boolean) {
Onyx.merge(ONYXKEYS.USER, {isMutedAllSounds});
}

When the same user account on device B receives a message, the sounds are still enabled since different device means the local data from device A is not synced with that on device B in real-time -> therefore isMutedAllSounds is false on device B.

This is happening since we're not saving this data on BE in order to sync on all devices via pusher in real-time.

What changes do you think we should make in order to solve the problem?

Implement saving of isMutedAllSounds on BE using specific endpoint in order to sync the data in real-time on all active devices through pusher event.

Similar to what we're doing currently for the "Receive relevant feature updates and Expensify news" setting within the toggle function:

function updateNewsletterSubscription(isSubscribed: boolean) {

using its own unique WRITE_COMMANDS.‎UPDATE_NEWSLETTER_SUBSCRIPTION update endpoint:

API.write(WRITE_COMMANDS.UPDATE_NEWSLETTER_SUBSCRIPTION, parameters, {
optimisticData,
failureData,
});

@jeremy-croff
Copy link
Contributor

jeremy-croff commented Feb 14, 2024

Similar to #35422, but different setting. I think an overal solution could be designed to handle all this use cases.

@ikevin127
Copy link
Contributor

ikevin127 commented Feb 14, 2024

@jeremy-croff Not that similar with the switch toggle based settings for which there's no overall solution because if we take a look at the current logic of other switch toggle based settings from Preferences they seem to have their own update endpoint.

Take for example the "Receive relevant feature updates and Expensify news" setting toggle function:

function updateNewsletterSubscription(isSubscribed: boolean) {

using its own unique WRITE_COMMANDS.‎UPDATE_NEWSLETTER_SUBSCRIPTION update endpoint:

API.write(WRITE_COMMANDS.UPDATE_NEWSLETTER_SUBSCRIPTION, parameters, {
optimisticData,
failureData,
});

@jeremy-croff
Copy link
Contributor

So there are 2 sounds.

  1. native sounds like this bug is showing
  2. app sounds, like we are playing through react-native-sounds library using playSound
    They both fire off when a notification gets received.
    When we mute all sounds, it just silences the app sounds

The notification send to target devices still has a sound in the payload and it allows the device to honor it's own settings.
If the phone is on silence it won't play, otherwise it would.
The BE needs to stop sending a sound file in the notification for this to also stop making a sound there.

@aimane-chnaif
Copy link
Contributor

We're not supporting BE yet. Sound is just local feature for now.
Let's close

@Beamanator Beamanator self-assigned this Feb 14, 2024
@Beamanator
Copy link
Contributor

I'm going to at least mark this as NAB for now, but we're discussing a bit in slack about how to fix / who should fix

@Beamanator Beamanator added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Feb 14, 2024
@kirillzyusko
Copy link
Contributor

Basically @ikevin127 and @aimane-chnaif are right, now this feature is device-level only (so the preference for this setting will not be synchronized across devices).

Also want to point out this comment #31055 (comment) where we agreed that for now it should be device-level feature.

So @Beamanator please decide what should we do next 👀 Thank you!

@Beamanator Beamanator removed External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Bug Something is broken. Auto assigns a BugZero manager. labels Feb 14, 2024
@Beamanator
Copy link
Contributor

Thanks so much @kirillzyusko !

So it sounds like there's nothing to fix here! I'll only keep this open to make sure @abekkala and I can make sure testrail steps are updated so we're not expecting this to happen 🙏

@m-natarajan are there regression tests in testrail that we should change since this issue is not actually a bug?

@abekkala
Copy link
Contributor

I'll await confirmation from @m-natarajan irt TR.

If there are no updates needed, I'll close out

@melvin-bot melvin-bot bot added the Overdue label Feb 19, 2024
Copy link

melvin-bot bot commented Feb 19, 2024

@Beamanator, @abekkala Whoops! This issue is 2 days overdue. Let's get this updated quick!

@Beamanator
Copy link
Contributor

Hmm Maybe we should post in the #qa slack channel to get more 👀 on this @abekkala ? What you think? :D

@melvin-bot melvin-bot bot removed the Overdue label Feb 19, 2024
@abekkala
Copy link
Contributor

@Beamanator yeah, good call. https://expensify.slack.com/archives/C9YU7BX5M/p1708369604811349

@Julesssss
Copy link
Contributor

Let's close this, and we'll add creating testrail cases to the tracking issue

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

No branches or pull requests

9 participants