[PAID] [$250] Room - After rejoining room, notification settings changed to immediately #48694

IuliiaHerets · 2024-09-06T10:16:46Z

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: 9.0.30-7
Reproducible in staging?: Y
Reproducible in production?: Y
Issue reported by: Applause Internal Team

Action Performed:

Go to https://staging.new.expensify.com/home
Via fab, create a room
Tap header -- settings
Note notifications is set "daily"
Tap leave and now tap header -- settings
Now note notifications field missing
Now send a message and join the room
Tap header--settings

Expected Result:

After rejoining room, notification settings must not change to immediately.

Actual Result:

After rejoining room, notification settings changed to immediately.

Workaround:

Unknown

Platforms:

Screenshots/Videos

Bug6594781_1725582608988.on.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~021833657980074790757
Upwork Job ID: 1833657980074790757
Last Price Increase: 2024-10-02
Automatic offers:

shubham1206agra | Contributor | 104297840

Issue Owner

Current Issue Owner: @strepanier03

melvin-bot · 2024-09-06T10:16:55Z

Triggered auto assignment to @strepanier03 (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.

IuliiaHerets · 2024-09-06T10:17:06Z

@strepanier03 FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

Nodebrute · 2024-09-06T11:00:37Z

Edited by proposal-police: This proposal was edited at 2023-10-03T13:22:17Z.

Proposal

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

After rejoining room, notification settings changed to immediately

What is the root cause of that problem?

When we leave room we set the notification preference to "hidden" and when we join room by adding comment we set the notification preference to "always"

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

We should update this to daily because when we create room the notification preference is daily CONST.REPORT.NOTIFICATION_PREFERENCE.DAILY

App/src/libs/actions/Report.ts

Line 495 in 812fc31

    
           optimisticReport.notificationPreference = CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS;

We need to change this in joinroom too. We can also fix this in other places.

What alternative solutions did you explore? (Optional)

dominictb · 2024-09-08T05:28:26Z

Edited by proposal-police: This proposal was edited at {current_timestamp}.

Proposal

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

After rejoining room, notification settings changed to immediately.

What is the root cause of that problem?

In here:

App/src/libs/actions/Report.ts

Line 495 in 812fc31

optimisticReport.notificationPreference = CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS;

and here:

App/src/libs/actions/Report.ts

Line 555 in 812fc31

notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS,

we always set the notificationPreference to always.

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

We should update:

App/src/libs/actions/Report.ts

Line 495 in 812fc31

optimisticReport.notificationPreference = CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS;

to:

        optimisticReport.notificationPreference = CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS;
        if (ReportUtils.isUserCreatedPolicyRoom(report)) {
            optimisticReport.notificationPreference = CONST.REPORT.NOTIFICATION_PREFERENCE.DAILY;
        }

and the same should be applied to:

App/src/libs/actions/Report.ts

Line 555 in 812fc31

notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS,

Additionally:

App/src/pages/workspace/WorkspaceNewRoomPage.tsx

Line 120 in 812fc31

CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS,

should be CONST.REPORT.NOTIFICATION_PREFERENCE.DAILY

What alternative solutions did you explore? (Optional)

melvin-bot · 2024-09-09T18:35:45Z

@strepanier03 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot · 2024-09-11T00:05:06Z

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

melvin-bot · 2024-09-11T00:05:12Z

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

melvin-bot · 2024-09-16T18:21:30Z

@strepanier03, @fedirjh Huh... This is 4 days overdue. Who can take care of this?

melvin-bot · 2024-09-18T16:00:10Z

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot · 2024-09-18T18:07:03Z

@strepanier03, @fedirjh Still overdue 6 days?! Let's take care of this!

fedirjh · 2024-09-18T22:10:58Z

After rejoining room, notification settings must not change to immediately.

cc @strepanier03 @IuliiaHerets What is the expected result? Should this be set to daily?

fedirjh · 2024-09-18T22:13:46Z

@Nodebrute @dominictb I checked the AddComment command and it seems that the backend returns the correct data, why doesn’t this onyx update be applied ? seems like there is a bug with Onyx ?

dominictb · 2024-09-19T18:08:38Z

seems like there is a bug with Onyx

yes, I think so. But I cannot find the RCA.

melvin-bot · 2024-09-20T17:59:42Z

@strepanier03 @fedirjh this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

shubham1206agra · 2024-09-23T14:35:44Z

@puneetlath I think you need to check this.

wildan-m · 2024-09-24T09:05:41Z

Proposal

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

Upon reentering the room, notification settings were changed to immediate.

What is the root cause of that problem?

We are always override the notification to ALWAYS when report not empty and current notification preference is hidden

App/src/libs/actions/Report.ts

Lines 551 to 562 in 1c39bd2

    
           if (shouldUpdateNotificationPrefernece) { 
        
               // optimisticReport.notificationPreference = CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS; 
        
               successData.push({ 
        
                   onyxMethod: Onyx.METHOD.MERGE, 
        
                   key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, 
        
                   value: { 
        
                       participants: { 
        
                           [currentUserAccountID]: {notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS}, 
        
                       }, 
        
                   }, 
        
               }); 
        
           }

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

The change is intended to fix the issue of chats disappearing when creating a chat with an unknown user while offline.

#47450

When the PR was created, the addComment API did not return notificationPreference, so the PR attempted to fill it from the front end. This is detailed in the author's alternative proposal. #44829 (comment)

The addComment API now includes notification preference from the backend. We can revert the unnecessary changes in the PR, particularly the ones related to successData.

App/src/libs/actions/Report.ts

Lines 551 to 562 in 1c39bd2

    
           if (shouldUpdateNotificationPrefernece) { 
        
               // optimisticReport.notificationPreference = CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS; 
        
               successData.push({ 
        
                   onyxMethod: Onyx.METHOD.MERGE, 
        
                   key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, 
        
                   value: { 
        
                       participants: { 
        
                           [currentUserAccountID]: {notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS}, 
        
                       }, 
        
                   }, 
        
               }); 
        
           }

I've noticed that join room intentionally makes the notification to ALWAYS, if that's not intentional we can also adjust that as well to make notification preferences based on BE response

App/src/libs/actions/Report.ts

Lines 2753 to 2766 in 1c39bd2

    
           function joinRoom(report: OnyxEntry<Report>) { 
        
               if (!report) { 
        
                   return; 
        
               } 
        
               updateNotificationPreference( 
        
                   report.reportID, 
        
                   ReportUtils.getReportNotificationPreference(report), 
        
                   CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS, 
        
                   false, 
        
                   report.parentReportID, 
        
                   report.parentReportActionID, 
        
                   report, 
        
               ); 
        
           }

What alternative solutions did you explore? (Optional)

N/A

wildan-m · 2024-09-24T09:10:45Z

Proposal Updated

mention joinRoom adjustment if required

melvin-bot · 2024-10-18T18:47:29Z

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

melvin-bot · 2024-10-18T18:47:34Z

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

Fix join rooms notification preference #50503

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

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

@fedirjh requires payment through NewDot Manual Requests
@shubham1206agra requires payment automatic offer (Contributor)

melvin-bot · 2024-10-18T18:47:35Z

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@fedirjh / @shubham1206agra] The PR that introduced the bug has been identified. Link to the PR:
[@fedirjh / @shubham1206agra] 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:
[@fedirjh / @shubham1206agra] A discussion in #expensify-bugs 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:
[@fedirjh / @shubham1206agra] Determine if we should create a regression test for this bug.
[@fedirjh / @shubham1206agra] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[@strepanier03] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

strepanier03 · 2024-10-25T19:18:19Z

@shubham1206agra @fedirjh - I'm going to be off for the day very soon but I'll check in this weekend to see if the checklist is complete. Once it is I'll do that payment summary.

lakchote · 2024-10-28T08:29:19Z

@shubham1206agra @fedirjh please complete the checklist, thanks!

shubham1206agra · 2024-10-28T08:31:08Z

@fedirjh needs to complete the checklist.

fedirjh · 2024-10-28T12:03:51Z

BugZero Checklist:

Link to the PR: The PR that introduced the changes fix notification preference issue in offline mode #47450
Link to comment: fix notification preference issue in offline mode #47450 (comment)
Link to discussion: N/A
Determine if we should create a regression test for this bug: Yes

Regression Test Proposal

1. Open App
2. Create new room & navigate to it
3. Tap header -> settings
4. Verify notifications is set "daily"
5. Tap leave and now tap header -> settings
6. Verify notifications field missing
7. Send a message and join the room
8. Tap header -> settings
9. Verify that notifications is set "daily"

Do we agree 👍 or 👎

strepanier03 · 2024-10-28T17:04:30Z

Payment Summary

$250 - @fedirjh via Manual Requests in New Expensify
$250 - @shubham1206agra requires payment via Upwork but offer is pending still (automatic offer)

strepanier03 · 2024-10-28T17:05:31Z

@shubham1206agra I'll check back in later today to see if you accepted the offer in Upwork.

@fedirjh - please feel free to request payment in New Expensify. I see your checklist responses and have created the reg test GH.

shubham1206agra · 2024-10-29T03:10:26Z

@strepanier03 Offer accepted

strepanier03 · 2024-10-29T03:41:04Z

All right, I paid out the UPwork job, thanks for accepting @shubham1206agra.

All that let's is @fedirjh to request in New Expensify so I'm closing this GH as completed.

melvin-bot · 2024-10-29T03:41:07Z

@strepanier03 @lakchote Be sure to fill out the Contact List!

garrettmknight · 2024-12-10T12:17:16Z

$250 approved for @fedirjh

IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Sep 6, 2024

melvin-bot bot assigned strepanier03 Sep 6, 2024

melvin-bot bot added the Overdue label Sep 9, 2024

strepanier03 added the External Added to denote the issue can be worked on by a contributor label Sep 11, 2024

melvin-bot bot changed the title ~~Room - After rejoining room, notification settings changed to immediately~~ [$250] Room - After rejoining room, notification settings changed to immediately Sep 11, 2024

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

melvin-bot bot assigned fedirjh Sep 11, 2024

melvin-bot bot removed the Overdue label Sep 11, 2024

strepanier03 added this to [#whatsnext] #vip-vsb Sep 11, 2024

melvin-bot bot added the Overdue label Sep 16, 2024

melvin-bot bot removed the Overdue label Sep 18, 2024

melvin-bot bot added the Overdue label Sep 23, 2024

melvin-bot bot added the Weekly KSv2 label Oct 14, 2024

mallenexpensify removed this from [#whatsnext] #vip-vsb Oct 17, 2024

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Oct 18, 2024

melvin-bot bot changed the title ~~[$250] Room - After rejoining room, notification settings changed to immediately~~ [HOLD for payment 2024-10-25] [$250] Room - After rejoining room, notification settings changed to immediately Oct 18, 2024

melvin-bot bot removed the Reviewing Has a PR in review label Oct 18, 2024

strepanier03 added Daily KSv2 and removed Weekly KSv2 labels Oct 25, 2024

melvin-bot bot added Daily KSv2 Monthly KSv2 and removed Daily KSv2 labels Oct 25, 2024

melvin-bot bot added the Overdue label Oct 28, 2024

fedirjh mentioned this issue Oct 28, 2024

fix notification preference issue in offline mode #47450

Merged

50 tasks

melvin-bot bot removed the Overdue label Oct 28, 2024

strepanier03 closed this as completed Oct 29, 2024

strepanier03 changed the title ~~[HOLD for payment 2024-10-25] [$250] Room - After rejoining room, notification settings changed to immediately~~ [PAID] [$250] Room - After rejoining room, notification settings changed to immediately Oct 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PAID] [$250] Room - After rejoining room, notification settings changed to immediately #48694

[PAID] [$250] Room - After rejoining room, notification settings changed to immediately #48694

IuliiaHerets commented Sep 6, 2024 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Sep 6, 2024

IuliiaHerets commented Sep 6, 2024

Nodebrute commented Sep 6, 2024 •

edited by github-actions bot

Loading

dominictb commented Sep 8, 2024 •

edited by github-actions bot

Loading

melvin-bot bot commented Sep 9, 2024

melvin-bot bot commented Sep 11, 2024

melvin-bot bot commented Sep 11, 2024

melvin-bot bot commented Sep 16, 2024

melvin-bot bot commented Sep 18, 2024

melvin-bot bot commented Sep 18, 2024

fedirjh commented Sep 18, 2024

fedirjh commented Sep 18, 2024

dominictb commented Sep 19, 2024

melvin-bot bot commented Sep 20, 2024

shubham1206agra commented Sep 23, 2024

wildan-m commented Sep 24, 2024 •

edited

Loading

wildan-m commented Sep 24, 2024

melvin-bot bot commented Oct 18, 2024

melvin-bot bot commented Oct 18, 2024

melvin-bot bot commented Oct 18, 2024

strepanier03 commented Oct 25, 2024

lakchote commented Oct 28, 2024

shubham1206agra commented Oct 28, 2024

fedirjh commented Oct 28, 2024

strepanier03 commented Oct 28, 2024 •

edited by garrettmknight

Loading

strepanier03 commented Oct 28, 2024

shubham1206agra commented Oct 29, 2024

strepanier03 commented Oct 29, 2024

melvin-bot bot commented Oct 29, 2024

garrettmknight commented Dec 10, 2024

[PAID] [$250] Room - After rejoining room, notification settings changed to immediately #48694

[PAID] [$250] Room - After rejoining room, notification settings changed to immediately #48694

Comments

IuliiaHerets commented Sep 6, 2024 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Sep 6, 2024

IuliiaHerets commented Sep 6, 2024

Nodebrute commented Sep 6, 2024 • edited by github-actions bot Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

dominictb commented Sep 8, 2024 • edited by github-actions bot Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

melvin-bot bot commented Sep 9, 2024

melvin-bot bot commented Sep 11, 2024

melvin-bot bot commented Sep 11, 2024

melvin-bot bot commented Sep 16, 2024

melvin-bot bot commented Sep 18, 2024

melvin-bot bot commented Sep 18, 2024

fedirjh commented Sep 18, 2024

fedirjh commented Sep 18, 2024

dominictb commented Sep 19, 2024

melvin-bot bot commented Sep 20, 2024

shubham1206agra commented Sep 23, 2024

wildan-m commented Sep 24, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

wildan-m commented Sep 24, 2024

melvin-bot bot commented Oct 18, 2024

melvin-bot bot commented Oct 18, 2024

melvin-bot bot commented Oct 18, 2024

strepanier03 commented Oct 25, 2024

lakchote commented Oct 28, 2024

shubham1206agra commented Oct 28, 2024

fedirjh commented Oct 28, 2024

BugZero Checklist:

strepanier03 commented Oct 28, 2024 • edited by garrettmknight Loading

Payment Summary

strepanier03 commented Oct 28, 2024

shubham1206agra commented Oct 29, 2024

strepanier03 commented Oct 29, 2024

melvin-bot bot commented Oct 29, 2024

garrettmknight commented Dec 10, 2024

IuliiaHerets commented Sep 6, 2024 •

edited by melvin-bot bot

Loading

Nodebrute commented Sep 6, 2024 •

edited by github-actions bot

Loading

dominictb commented Sep 8, 2024 •

edited by github-actions bot

Loading

wildan-m commented Sep 24, 2024 •

edited

Loading

strepanier03 commented Oct 28, 2024 •

edited by garrettmknight

Loading