[HOLD for payment 2024-07-10] [$250] Web - Profile - Notify me about new messages disappears from profile page

[kbecciv]

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: 1.4.80-1
Reproducible in staging?: y
Reproducible in production?: y
Issue reported by: Applause - Internal team

Action Performed:

1. Login with a expensifail account
2. Go to FAB> Start chat> Search for a expensifail account and create new chat
3. Click on the header to view user details

Expected Result:

Notify me about new messages option should appear and remain

Actual Result:

Notify me about new messages disappears from profile page

Workaround:

n/a

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

Bug6504314_1717702299783.Recording__3141.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~015aa8cecd7682d626
• Upwork Job ID: 1800065189510532413
• Last Price Increase: 2024-06-10
• Automatic offers:
• DylanDylann | Reviewer | 102711970
• dominictb | Contributor | 102711971

Issue Owner

Current Issue Owner: @VictoriaExpensify

[melvin-bot]

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

[kbecciv]

@VictoriaExpensify 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

[Tony-MK]

Proposal

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

Web - Profile - Notify me about new messages disappears from profile page

What is the root cause of that problem?

In the profile page, we only show the notify me menu item based mainly on the user's report.notificationPreference is CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN

App/src/pages/ProfilePage.tsx

Lines 153 to 155 in 4fcc5a9

	const shouldShowNotificationPreference =
	!isEmptyObject(report) && !isCurrentUser && !!report.notificationPreference && report.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN;
	const notificationPreference = shouldShowNotificationPreference

Hence, if the user is new and the personal details get synced from the backend, the notfiy me menu item disappears because the user has never chatted with him.

App/src/pages/ProfilePage.tsx

Lines 259 to 266 in 4fcc5a9

	{shouldShowNotificationPreference && (
	<MenuItemWithTopDescription
	shouldShowRightIcon
	title={notificationPreference}
	description={translate('notificationPreferencesPage.label')}
	onPress={() => Navigation.navigate(ROUTES.REPORT_SETTINGS_NOTIFICATION_PREFERENCES.getRoute(report.reportID))}
	/>
	)}

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

Therefore, let's remove the condition report.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN to make the shouldShowNotificationPreference boolean is true even if the user's report.notificationPreference is CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN

So something like below should work.

const shouldShowNotificationPreference = !isEmptyObject(report) && !isCurrentUser && !!report.notificationPreference;

[melvin-bot]

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

[melvin-bot]

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

[VictoriaExpensify]

This looks ok to me. I've added the project and the External label

[dominictb]

Proposal

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

Notify me about new messages disappears from profile page

What is the root cause of that problem?

The default notificationPreference of optimistic report is always here but when we create a new DM chat with a new user BE return it as hidden. It leads Notify me about new messages option appears briefly and then disappears after OpenReport API is complete

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

Because a new report always has notificationPreference is hidden by default, we should change the default value of notificationPreference here to CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN

Or we can pass notificationPreference as CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN when we create a new optimistic report here and here

What alternative solutions did you explore? (Optional)

NA

[DylanDylann]

Thanks for your proposals

@Tony-MK Your change may break other places because we need to hide Notify me about new messages option if notificationPreference is "hidden"

@dominictb Your proposal looks promising if we want to hide Notify me about new messages option for new users. Could you explain why we should hide this option for the new user?

Note that: this problem happens with all new users while creating a new chat (not only the expensifail account) because we have an inconsistency between BE and FE about the notificationPreference for the new chat

[dominictb]

Could you explain why we should hide this option for the new user?

@DylanDylann If we create a new chat, the notificationPreference of this report should be hidden by default (it's expected from BE) if we don't send any message in this report. It's the same behavior with iou report or transaction thread report.

[DylanDylann]

if we don't send any message in this report. It's the same behavior with iou report or transaction thread report.

Ah, It makes sense to me

[DylanDylann]

Let's go with @dominictb's proposal as explained here

🎀👀🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @cead22, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[melvin-bot]

📣 @DylanDylann 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @dominictb 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[DylanDylann]

While reviewing PR, I see that we don't always want to hide the notificationPreference of the report. It depends on which report.

1. If the user doesn't exist on the Expensify, the BE return notificationPreference: hidden
2. If the user exists on the Expensify, but the current user has never chatted with them before, the BE returns notificationPreference: always

@dominictb Could you help to confirm that?

[dominictb]

Yep

[DylanDylann]

@cead22 It is a bit tough if we want to mimic the notificationPreference like the back-end in the optimisticData because in the front-end we can't verify whether the user exists on Expensify or not. Which option should we set for notificationPreference in the optimisticData when creating a new chat?

[cead22]

@JmillsExpensify @luacmartins is that the expected behavior, and if so, do you have a suggestion for solving this? Perhaps we pick one of the options to show as the default, show the option disabled at first until we get the value from the server, and then update the control and enable it

[luacmartins]

I don't think we'd always want to set the notification preferences to hidden. It seems like that makes sense if the user doesn't exist, but not otherwise. I'd imagine we could hide that preferences menu until we get a response from the backend?

[DylanDylann]

@luacmartins Should we also hide that preferences menu in group chat until we get a response from the backend? I think it also makes sense

[luacmartins]

If we're seeing the same behavior there, I think we should.

[melvin-bot]

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

[melvin-bot]

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

• fix: default notification to be hidden for new chat #43638

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

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

• @DylanDylann requires payment automatic offer (Reviewer)
• @dominictb requires payment automatic offer (Contributor)

[melvin-bot]

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:

• [@DylanDylann] The PR that introduced the bug has been identified. Link to the PR:
• [@DylanDylann] 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:
• [@DylanDylann] 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:
• [@DylanDylann] Determine if we should create a regression test for this bug.
• [@DylanDylann] 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.
• [@VictoriaExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[DylanDylann]

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:

[@DylanDylann] The PR that introduced the bug has been identified. Link to the PR: NA
[@DylanDylann] 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: NA
[@DylanDylann] 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: NA
[@DylanDylann] Determine if we should create a regression test for this bug. Yes
[@DylanDylann] 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.

Regression Test Proposal

1. Start a new chat/a new group chat
2. Click on the header of the newly created chat
3. Verify that the notify me about new messages option should not appear

Do we agree 👍 or 👎

[melvin-bot]

@cead22, @VictoriaExpensify, @DylanDylann, @dominictb Whoops! This issue is 2 days overdue. Let's get this updated quick!

[cead22]

I'd take this a couple of steps further

1. Start a new chat/a new group chat
2. Click on the header of the newly created chat
3. Verify that the notify me about new messages option should not appear
4. Close the settings pane
5. Send a mesage
6. Click on the header of the created chat
7. Verify that the notify me about new messages option appears

[melvin-bot]

@cead22, @VictoriaExpensify, @DylanDylann, @dominictb Still overdue 6 days?! Let's take care of this!

[cead22]

@VictoriaExpensify all yours

[cead22]

This isn't overdue, just waiting for payment

[VictoriaExpensify]

• Contributor: @dominictb paid $250 via Upwork
• Contributor+: @DylanDylann due $250 via Upwork

Upwork Job Link Here

[VictoriaExpensify]

@dominictb and @DylanDylann have been paid, the contract has been completed, and the Upwork post has been closed.