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

Rooms - Notification preference changes back after refresh/reload under Settings #9805

Closed
kbecciv opened this issue Jul 8, 2022 · 10 comments
Assignees

Comments

@kbecciv
Copy link

kbecciv commented Jul 8, 2022

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


Issue was found when executing PR #9501

Action Performed:

  1. Go to URL https://staging.new.expensify.com/
  2. Open an #announce workspace chat
  3. Click on the chat header to go to the details
  4. Go to the settings
  5. Change the notification preferences from any option to mute
  6. Refresh the page

Expected Result:

Сhanging preferences works correctly

Actual Result:

Drop down works inconsistently after changing Daily notifications to Mute under Settings

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Version Number: 1.1.82.5

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers): any

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Bug5640869_Recording__1055.mp4
RPReplay_Final1657295155.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

@melvin-bot
Copy link

melvin-bot bot commented Jul 8, 2022

Triggered auto assignment to @marcochavezf (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@kbecciv kbecciv changed the title Web - Rooms - Drop down works inconsistently after changing Daily notifications to Mute under Settings Web - Rooms - Notification preference changes back after refresh/reload under Settings Jul 8, 2022
@kbecciv kbecciv changed the title Web - Rooms - Notification preference changes back after refresh/reload under Settings Rooms - Notification preference changes back after refresh/reload under Settings Jul 8, 2022
@melvin-bot melvin-bot bot added the Overdue label Jul 11, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jul 11, 2022

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

@marcochavezf
Copy link
Contributor

Hmm I was able to reproduce this issue when I change the notification preference and refresh the browser quickly, but the bug is not happening consistently. Posting a comment in the related PR to have more opinions about this.

@marcaaron
Copy link
Contributor

when I change the notification preference and refresh the browser quickly, but the bug is not happening consistently

Hard to say what is happening without looking at the network traffic.

@marcaaron
Copy link
Contributor

Though it seems pretty consistently broken on my end. Did we check logs for any errors?

@marcaaron
Copy link
Contributor

I think the problem is something to do with this. Must not be sending the notificationPreferences rNVP.

const notificationPreference = lodashGet(report, ['reportNameValuePairs', 'notificationPreferences', currentUserAccountID], CONST.REPORT.NOTIFICATION_PREFERENCE.DAILY);

@marcaaron
Copy link
Contributor

Tried to update a room then looked to see if this rNVP is returning with my accountID and it is not there. Even though the request returned 200

2022-07-11_14-55-12

So something weird is happening on the API side.

@marcaaron
Copy link
Contributor

Here, we are calling setNVP() after returning the onyx data and need to do it before...

https://github.com/Expensify/Web-Expensify/blob/84be775cff58ca3a32dc3d504093cd462fc2a54a/lib/ReportAPI.php#L3553-L3562

@marcochavezf
Copy link
Contributor

I will continue with this issue after https://github.com/Expensify/Web-Expensify/pull/34260 is merged (more context in this comment).

@marcochavezf
Copy link
Contributor

The error seems to be fixed on https://github.com/Expensify/Web-Expensify/pull/34260, I can't reproduce it anymore.

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

3 participants