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

[HOLD for payment 2024-11-18] [HOLD for payment 2024-11-11] [$250] The setting "Mute all sounds from Expensify" keeps turning back off despite turning it on every day. #49087

Closed
1 of 6 tasks
m-natarajan opened this issue Sep 12, 2024 · 88 comments
Assignees
Labels
AutoAssignerNewDotQuality Used to assign quality issues to engineers Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@m-natarajan
Copy link

m-natarajan commented Sep 12, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 9.0.33-1
Reproducible in staging?: Y
Reproducible in production?: Y
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: @flodnv
Slack conversation: https://expensify.slack.com/archives/C05LX9D6E07/p1726129231306989

Action Performed:

  1. Go to Account settings > Preferences
  2. Toggle "Mute all sounds from Expensify" to "ON"
  3. Signout and Singnin

Expected Result:

Preference settings are saved for each platform and remain intact after the user signs out and signs back in.

Actual Result:

Preference settings not saved and toggles back to "OFF"

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

Recording.541.mp4

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021834238152691021896
  • Upwork Job ID: 1834238152691021896
  • Last Price Increase: 2024-10-03
  • Automatic offers:
    • jjcoffee | Reviewer | 104324316
    • c3024 | Contributor | 104324317
Issue OwnerCurrent Issue Owner: @isabelastisser
@m-natarajan m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. AutoAssignerNewDotQuality Used to assign quality issues to engineers labels Sep 12, 2024
Copy link

melvin-bot bot commented Sep 12, 2024

Triggered auto assignment to @johnmlee101 (AutoAssignerNewDotQuality)

Copy link

melvin-bot bot commented Sep 12, 2024

Triggered auto assignment to @isabelastisser (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Sep 12, 2024
@flodnv flodnv removed their assignment Sep 12, 2024
@flodnv flodnv added the External Added to denote the issue can be worked on by a contributor label Sep 12, 2024
@melvin-bot melvin-bot bot changed the title The setting "Mute all sounds from Expensify" keeps turning back off despite turning it on every day. [$250] The setting "Mute all sounds from Expensify" keeps turning back off despite turning it on every day. Sep 12, 2024
Copy link

melvin-bot bot commented Sep 12, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 12, 2024
Copy link

melvin-bot bot commented Sep 12, 2024

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

@Nodebrute
Copy link
Contributor

Nodebrute commented Sep 12, 2024

Edited by proposal-police: This proposal was edited at 2024-09-12 14:52:57 UTC.

Proposal

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

The setting "Mute all sounds from Expensify" keeps turning back off despite turning it on every day.

What is the root cause of that problem?

When user mute all sounds we only merge it in onyx and no API calls are made to store the preference in backend

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

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

We should do a api call for setMuteAllSounds just like we do for updateNewsletterSubscription

We can also do the same for usestagingserver

What alternative solutions did you explore? (Optional)

@eyramtitiati
Copy link

Problem Statement:

The setting “Mute all sounds from Expensify” keeps turning off, even after being enabled by the user.

Root Cause:

The current implementation merges the isMutedAllSounds only into Onyx (local storage) without making an API call to persist the preference in the backend, causing the setting to reset daily.

Proposed Solution:

We will add an API call in User.ts for the setMuteAllSounds function, similar to how updateNewsletterSubscription operates. This ensures the setting is stored in the backend, maintaining persistence across app sessions.

Alternative Solutions Explored:

  1. Persist Locally with AsyncStorage: Instead of adding a backend call, we could utilize React Native’s AsyncStorage to persist the mute setting locally across sessions. This would reduce dependency on the backend, making it quicker but potentially less reliable across different devices.
  2. Onyx Sync with Backend at App Load: Another approach is to sync Onyx with the backend on app load to ensure that any changes made to settings (like isMutedAllSounds) are reflected in the backend at regular intervals, reducing the number of API calls but maintaining sync reliability.

Copy link

melvin-bot bot commented Sep 12, 2024

📣 @eyramtitiati! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@jjcoffee
Copy link
Contributor

jjcoffee commented Sep 12, 2024

Discussing on Slack as a similar issue was previously closed as expected.

@eyramtitiati
Copy link

Contributor details
Your Expensify account email:eyramtitiati@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~0182773c2654eddb1f?mp_source=share

Copy link

melvin-bot bot commented Sep 12, 2024

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@NaveedShaikh78
Copy link

Proposal

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

The setting "Mute all sounds from Expensify" keeps turning back off despite turning it on every day.

What is the root cause of that problem?

API to update "Mute all sounds from Expensify" is not implemented

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

Need to implement the API and other related changes to respective files
app\src\libs\actions\User.ts

function updateIsMutedAllSounds(isMutedAllSounds: boolean) {
    const optimisticData: OnyxUpdate[] = [
        {
            onyxMethod: Onyx.METHOD.MERGE,
            key: ONYXKEYS.USER,
            value: {isMutedAllSounds: isMutedAllSounds},
        },
    ];
    const failureData: OnyxUpdate[] = [
        {
            onyxMethod: Onyx.METHOD.MERGE,
            key: ONYXKEYS.USER,
            value: {isMutedAllSounds: !isMutedAllSounds},
        },
    ];
                     
    const parameters: UpdateIsMutedAllSoundsParams = {isMutedAllSounds};

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

What alternative solutions did you explore? (Optional)

@jjcoffee
Copy link
Contributor

Coming from the Slack discussion, the repro steps for this issue are wrong. The current thinking is that it happens when the app is updated, not when signing in/out (though it would also happen there, but that's expected since the setting isn't stored at the account-level).

@jjcoffee
Copy link
Contributor

Bumped on Slack.

@c3024
Copy link
Contributor

c3024 commented Sep 16, 2024

Proposal

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

Muted setting resets on updating app.

What is the root cause of that problem?

After updating the app, I think backend sends ONYXKEYS.RESET_REQUIRED as true. So, this here

const KEYS_TO_PRESERVE: OnyxKey[] = [
ONYXKEYS.ACCOUNT,
ONYXKEYS.IS_CHECKING_PUBLIC_ROOM,
ONYXKEYS.IS_LOADING_APP,
ONYXKEYS.IS_SIDEBAR_LOADED,
ONYXKEYS.MODAL,
ONYXKEYS.NETWORK,
ONYXKEYS.SESSION,
ONYXKEYS.SHOULD_SHOW_COMPOSE_INPUT,
ONYXKEYS.NVP_TRY_FOCUS_MODE,
ONYXKEYS.PREFERRED_THEME,
ONYXKEYS.NVP_PREFERRED_LOCALE,
ONYXKEYS.CREDENTIALS,
];
Onyx.connect({
key: ONYXKEYS.RESET_REQUIRED,
callback: (isResetRequired) => {
if (!isResetRequired) {
return;
}
Onyx.clear(KEYS_TO_PRESERVE).then(() => {

retains information only those in the keys to preserve and resets the rest.

As muting preference is within ONYXKEYS.USER it gets reset.

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

I tried the following. Then this muting setting does not get reset.

  1. We can move the muting preference to ONYXKEYS.ACCOUNT because it is already one of the keys in the keys to be preserved.
    (or)
  2. We can add ONYXKEYS.USER to the keys to preserve because it has some other local preferences like clearing focus mode notification etc. so they are not lost when updated,
    (or)
  3. Extract muting sounds to a separate Onyx key.

What alternative solutions did you explore? (Optional)

@jjcoffee
Copy link
Contributor

After updating the app, I think backend sends ONYXKEYS.RESET_REQUIRED as true.

@c3024 Thanks for the proposal! It does sound like a plausible RCA, but I'm wondering if this is true. Based on the comment here it sounds more like this should be triggered fairly rarely (the reports are of it happening "2 or 3 times a week"/"every day"):

App/src/ONYXKEYS.ts

Lines 364 to 365 in 19d037b

/** Indicates whether an forced reset is required. Used in emergency situations where we must completely erase the Onyx data in the client because it is in a bad state. This will clear Oynx data without signing the user out. */
RESET_REQUIRED: 'resetRequired',

Of course it's possible that an "emergency" reset is being triggered more often that we'd think, or even that it's being triggered by mistake on every update.

@johnmlee101 Are you able to check on the BE to see what/if any Onyx keys get set (e.g. RESET_REQUIRED) to require/trigger an update?

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Nov 4, 2024
@melvin-bot melvin-bot bot changed the title [$250] The setting "Mute all sounds from Expensify" keeps turning back off despite turning it on every day. [HOLD for payment 2024-11-11] [$250] The setting "Mute all sounds from Expensify" keeps turning back off despite turning it on every day. Nov 4, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Nov 4, 2024
Copy link

melvin-bot bot commented Nov 4, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

Copy link

melvin-bot bot commented Nov 4, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.56-9 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-11-11. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Nov 4, 2024

@jjcoffee / @c3024 @isabelastisser The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

@mallenexpensify
Copy link
Contributor

QQ, I see the PR hit staging last week. I'm on latest staging, desktop and..

  1. Had sounds muted.
  2. Signed out/in
  3. Sounds weren't muted anymore.

That's the bug I was hoping would be fixed.

I then signed out/in again and had the same results. Any idea why this isn't working?

@mallenexpensify
Copy link
Contributor

mallenexpensify commented Nov 6, 2024

Somethings way broken here.
After the above, when I muted sounds again

  • Went to chat with Deeter
  • Sent him message
  • Sound happened.

Turned mute back on again, followed the above steps and sounds happen on ALL the messages I send 😨

It's happening on production, web/Chrome too.

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Nov 6, 2024
Copy link

melvin-bot bot commented Nov 6, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@jjcoffee
Copy link
Contributor

jjcoffee commented Nov 6, 2024

  1. Sounds weren't muted anymore.

@mallenexpensify Was the mute all sounds setting turned off or just that you heard sounds?

@c3024 has raised a PR with fixes for a couple of areas we missed where the loading of the new setting needed updating, but the toggle itself should have been displaying correctly. The only other thing I can think of is that you will have needed to re-enable the setting once the PR hit staging.

@mallenexpensify
Copy link
Contributor

This is how I have my settings and it's playing sounds when I send a message, it doesn't appear to be when I receive one though.
image

and.. check this weirdness around the 6 second mark. When I got into preferences, you can see the toggle move to the right.

2024-11-06_10-32-55.mp4

@c3024
Copy link
Contributor

c3024 commented Nov 7, 2024

I think that appearance of toggling action happening on visiting the page is not related to the issue of sounds playing even after muting.

PR #52088 fixes the sounds problem.

@isabelastisser
Copy link
Contributor

The payments were processed in Upwork now.

@jjcoffee, do we need a regression test for this?

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Nov 11, 2024
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2024-11-11] [$250] The setting "Mute all sounds from Expensify" keeps turning back off despite turning it on every day. [HOLD for payment 2024-11-18] [HOLD for payment 2024-11-11] [$250] The setting "Mute all sounds from Expensify" keeps turning back off despite turning it on every day. Nov 11, 2024
Copy link

melvin-bot bot commented Nov 11, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Nov 11, 2024
Copy link

melvin-bot bot commented Nov 11, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.59-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-11-18. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Nov 11, 2024

@jjcoffee / @c3024 @isabelastisser The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

@jjcoffee
Copy link
Contributor

jjcoffee commented Nov 12, 2024

BugZero Checklist:

  • [Contributor] Classify the bug:
Bug classification

Source of bug:

  • 1a. Result of the original design (eg. a case wasn't considered)
  • 1b. Mistake during implementation
  • 1c. Backend bug
  • 1z. Other:

Where bug was reported:

  • 2a. Reported on production
  • 2b. Reported on staging (deploy blocker)
  • 2c. Reported on a PR
  • 2z. Other:

Who reported the bug:

  • 3a. Expensify user
  • 3b. Expensify employee
  • 3c. Contributor
  • 3d. QA
  • 3z. Other:
  • [Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

    Link to comment: N/A since we ended up adding a new feature

  • [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

    Link to discussion:

  • [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

  • [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

    Link to issue:

Regression Test Proposal

Test:

  1. Switch the mute all sounds setting in Preferences page to On
  2. Log out and back in
  3. Verify that the mute all sounds setting is still turned on
  4. Navigate to any chat and send a message in it
  5. Verify that no sounds play
  6. [iOS/Android native only] Click on FAB > Submit Expense > Scan
  7. [iOS/Android native only] Take a photo with the camera
    8.[iOS/Android native only] Verify that no sound plays

Do we agree 👍 or 👎

@isabelastisser
Copy link
Contributor

All set!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AutoAssignerNewDotQuality Used to assign quality issues to engineers Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
Development

No branches or pull requests