[$500] Profile - Notification preference displayed when no comment in report

[kbecciv]

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.3.24
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: Applause - Internal Team
Slack conversation:

Issue found when executing PR #33387

Action Performed:

1. Launch app
2. Tap fab
3. Tap start chat
4. Select a new user with whom no previous chat history
5. Tap on header

Expected Result:

In profile page, notification preference must not be displayed when there is no comment in report.

Actual Result:

In profile page, notification preference section is displayed when there is no comment/chat in report.

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

Bug6336979_1704824035182.az_recorder_20240109_190136.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01cb16322001847cf2
• Upwork Job ID: 1744792621694226432
• Last Price Increase: 2024-01-16

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[paultsimura]

Proposal

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

The notification preference is visible on the profile with no previous conversations.

What is the root cause of that problem?

When deciding whether to show the notification preference, we only check it to be not hidden, and the report object to be not empty:

App/src/pages/ProfilePage.js

Line 137 in ffbf5da

const shouldShowNotificationPreference = !_.isEmpty(props.report) && props.report.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN;

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

We should explicitly check if the chat is not empty, similar to how we do it on the HeaderView:

const lastVisibleMessage = ReportActionsUtils.getLastVisibleMessage(props.report.reportID);
const isEmptyChat = !props.report.lastMessageText && !props.report.lastMessageTranslationKey && !lastVisibleMessage.lastMessageText && !lastVisibleMessage.lastMessageTranslationKey;
const shouldShowNotificationPreference = !_.isEmpty(props.report) && !isEmptyChat && props.report.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN;

App/src/pages/ProfilePage.js

Line 137 in ffbf5da

const shouldShowNotificationPreference = !_.isEmpty(props.report) && props.report.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN;

We also make similar calculations here, we can move this calculation into a separate util function:

App/src/libs/ReportUtils.ts

Lines 3479 to 3480 in c0c4d64

	const lastVisibleMessage = ReportActionsUtils.getLastVisibleMessage(report.reportID);
	const isEmptyChat = !report.lastMessageText && !report.lastMessageTranslationKey && !lastVisibleMessage.lastMessageText && !lastVisibleMessage.lastMessageTranslationKey;

What alternative solutions did you explore? (Optional)

[dragnoir]

Proposal

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

Notification preference displayed when no comment in report

What is the root cause of that problem?

The profile page is not checking:

• if the chat is empty
• if there's no old deleted conversation

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

1- We need to check if the chat is empty same as in HeaderView

App/src/pages/home/HeaderView.js

Line 114 in 76e5b1c

const isEmptyChat = !props.report.lastMessageText && !props.report.lastMessageTranslationKey && !lastVisibleMessage.lastMessageText && !lastVisibleMessage.lastMessageTranslationKey;

2- We need to check if there was a discussion before

function  areDatesEqual(lastReadTime, lastVisibleActionCreated) {
  // Removing the milliseconds part from the date-time strings
  const  lastReadTimeWithoutMillis  =  lastReadTime.substring(0, 19);
  const  lastVisibleActionCreatedWithoutMillis  =  lastVisibleActionCreated.substring(0, 19);
  
  // Creating Date objects
  const  lastReadDate  =  new  Date(lastReadTimeWithoutMillis);
  const  lastVisibleActionCreatedDate  =  new  Date(lastVisibleActionCreatedWithoutMillis);
  
  // Returning true if dates are equal, false otherwise
  return  lastReadDate.getTime() ===  lastVisibleActionCreatedDate.getTime();
}

const  result  =  areDatesEqual(props.report.lastReadTime, props.report.lastVisibleActionCreated);

3- Add to shouldShowNotificationPreference those 2 checks to make sure we can display the notifications setting.

App/src/pages/ProfilePage.js

Line 137 in ad583bb

const shouldShowNotificationPreference = !_.isEmpty(props.report) && props.report.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN;

Explaining:

The bug reported here is for the 1st time conversation, but I noticed if there's an old conversation and it's deleted, checking only if the chat is empty will not allow editing notifications settings.
And I think if someone receive a lot of messages (with lots of notifications) then those messages got deleted by the sender, I think the receiver still get the right to mute (or edit) notification.

I am not sure what's the righ way to do this (checking if there's an old deleted conversation) but I noticed there's a difference between lastReadTime and lastVisibleActionCreated in report details. I will try to ask the team on Slack about this.

Video of how deleted messages can lead to empty chat:

14.mp4

What alternative solutions did you explore? (Optional)

Add an utility on ReportUtils to handle this case.

[Victor-Nyagudi]

I'm a little confused about the expected result here.

Could someone kindly explain how showing the notification preference when no messages have been sent is a bad thing? Does someone have to chat first before you can adjust the preference?

[dragnoir]

@Victor-Nyagudi check #33387

[paultsimura]

And I think if someone receive a lot of messages (with lots of notifications) then those messages got deleted by the sender, I think the receiver still get the right to mute (or edit) notification.

At first, I was thinking about this scenario as well, but it looks super unlikely considering the sender will have to remove each message manually. Moreover, if the chat report is empty, it disappears from LHN in both "Last seen" and "Focus" modes, so in such a case the recipient will have to go to the search, type in the sender's details, and mute them.
Therefore, making such a calculation for a super edge case each time a profile page is opened looked like an overkill for me.

[dragnoir]

@paultsimura Let's agree to disagree :)

I was thinking about this scenario as well

It's not mentioned in your proposal even in the alternative solution

it looks super unlikely considering the sender will have to remove each message manually

it can be just one msg, mistake, wrong file..

it disappears from LHN

You can see on the video I attached, it doesn't disappear. The chat is open, msg removed and user (receiver) is not able to mute cayse msg deleted.

such a calculation

I don't see any calculation, it's just a condition.

super edge case

This is how great apps are built. Most of the issues areb super edge case but we enroll to make the app perfect.

Thanks for the details, I have confidance that the C+ will take the right decision.

[sonialiap]

@akinwale what do you think of the above proposals?

[melvin-bot]

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

[akinwale]

@dragnoir I reviewed both proposals, and the expected behaviour here is that there shouldn't be a notification preference displayed whenever the report is empty, so it doesn't matter if there were previous chats that have been deleted. As long as the report is empty, the preference should not be displayed.

After reviewing the proposals, we can move forward with @paultsimura's proposal.

🎀👀🎀 C+ reviewed.

[melvin-bot]

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

[Beamanator]

Hmm @srikarparsi do you mind taking over this one since it has a lot to do with https://github.com/Expensify/Expensify/issues/330506, and specifically your changes in these two PRs?

• https://github.com/Expensify/Auth/pull/9531
• Dont show hidden notif pref in profile page #33387

[dragnoir]

@akinwale I don't agree with you. For an app like expensify, perfection and focus on details is very important. The video I posted show the UX issue when someone is spamming then removing messages. Even when the app and the chat is open.

I totally repect you decision but I don't agree with you. It will be another issue opened later for sure.

[srikarparsi]

Hey! I don't believe this is reproducible anymore since the Auth PR hit production yesterday. This makes sure that new DMs are set to hidden by default and the App PR ensures we don't show the hidden notification preference in the settings pane.

I believe we should be able to close this out?

[paultsimura]

That's sad but seems like you're right @srikarparsi, I cannot reproduce either 🙂

[Beamanator]

That's great! Happy to close this out

[dragnoir]

@paultsimura that was a nice debate, thx for the spirit :)

[paultsimura]

@paultsimura that was a nice debate, thx for the spirit :)

You too, man 💪