Add notification preference for all the reports and update the design for the group chats #28200

techievivek · 2023-09-25T23:09:09Z

Details

Here are a few things that got updated in this PR.

We updated the details page for the group chat.
We show the settings pages for all the chat reports.
We allow all the chat reports to set the notification preference.

Fixed Issues

$ #27394
PROPOSAL: N/A

Tests

Needs to be tested with this Web-E PR: https://github.com/Expensify/Web-Expensify/pull/38977

Test for DM chat report

Login to newDot as userA on one device/browser.
Login to newDot as userB on another device/browser.
Create a DM chat report with userB.
Write a new message to userB and confirm that they receive a notification.
Update the notification preference as userB to daily and confirm you no longer see the notification.

Test for workspace private/restricted rooms

Login to newDot as userA on one device/browser.
Login to newDot as userB on another device/browser.
Create a new workspace and invite the userB to it.
Write a new message to in any of the private/default/restricted room and confirm that the userB receives a notification.
Update the notification preference as userB to daily and confirm you no longer see the notification.

Test for group DM chat

Login to newDot as userA on one device/browser.
Login to newDot as userB on another device/browser.
Create a group DM chat report with userB and bunch of other users.
Write a new message in the chat and confirm userB received a notification.
Update the notification preference as userB to daily and confirm you no longer see the notification.

Test for workspace public rooms

Login to newDot as userA on one device/browser.
Login to newDot as userB on another device/browser.
Create a new workspace and create a new public room.
As userA, write a few message to the room.
As userB, join the above public room.
As userA, write few more new message and confirm userB don't receive a notification.
Update the notification preference for userB to always and confirm they receive a notification.
Verify that no errors appear in the JS console

Offline tests

QA Steps

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

melvin-bot · 2023-09-27T00:42:08Z

@mollfpr Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

techievivek · 2023-09-27T00:55:20Z

@mollfpr I am also working from Bali 😄. Let me know if the changes look good to you. I will add the videos by then. If you have questions, you can put them here, and we can resolve them quickly. Looking to get this shipped ASAP as it is a wave8 priority.

techievivek · 2023-09-27T00:56:15Z

Added @arosiclair for review as he is assigned to review the web-e changes.

mollfpr · 2023-09-27T01:11:32Z

Take the review now!!!

@techievivek Really?? I didn't see you in the 10am spot in Ubud 😅

techievivek · 2023-09-27T01:12:14Z

@mollfpr I only joined for the last week 😢, so now we are working from Jimbaran.

mollfpr · 2023-09-27T01:13:15Z

@techievivek Is the Web-E PR in staging?

MitchExpensify · 2023-09-27T01:15:50Z

Thank you for prioritizing this @mollfpr, this is an important piece of an external deadline project so your help is super appreciated!

mollfpr

Mostly looks good.

src/pages/ProfilePage.js

mollfpr · 2023-09-27T03:39:36Z

@techievivek The selected option is not changed on the profile page.

Screen.Recording.2023-09-27.at.10.36.04.mov

techievivek · 2023-09-27T03:58:30Z

@mollfpr Aaah sorry the backend changes are not yet live, do you want me to share my ngrok URL so you can test them?

mollfpr · 2023-09-27T04:03:20Z

@techievivek That's would be great!

techievivek · 2023-09-27T04:14:05Z

@mollfpr I am looking at your videos and it looks like this should have worked because it doesn't seem to actually depend on the backend changes. Do you mind sharing the onyx response for the request. Does it include the updated notification preference for the report?

mollfpr · 2023-09-27T04:30:16Z

Tested with your ngrok. The response seems fine, but the selected value doesn't change.

Screen.Recording.2023-09-27.at.11.27.56.mov

mollfpr · 2023-09-27T04:33:38Z

I think the issue is because the page is not re-render after selecting the preference.

src/pages/ProfilePage.js

techievivek · 2023-10-09T07:07:03Z

@mollfpr How is it going? Let me know if you are stuck anywhere or need help with anything, thanks.

mollfpr

Sorry @techievivek I forgot to approve the PR. The changes looks good and it tests well on Web, Desktop and Android. I still uploading the recordings to the checklist.

melvin-bot · 2023-10-09T09:42:42Z

@hayata-suenaga Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

techievivek · 2023-10-09T12:01:29Z

@arosiclair Can you please give this a review so we can get this merged today.

techievivek · 2023-10-10T05:45:54Z

🤦 We are once again in the conflict zone, let me resolve them quickly. @mollfpr can you please add the ios recordings for any single test(just for DM).

mollfpr · 2023-10-10T05:49:09Z

@techievivek Sure!

hayata-suenaga · 2023-10-10T05:57:26Z

@hayata-suenaga Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it.

I think the checklist is already done by @mollfpr. @arosiclair is assigned as an internal engineer. melvin, why did you assign me???

Anyway, I'm unassigning myself

arosiclair · 2023-10-10T08:30:38Z

src/pages/ProfilePage.js

+            key: ONYXKEYS.SESSION,
+        },
+    }),
+    // eslint-disable-next-line rulesdir/no-multiple-onyx-in-file


Not sure I understand that comment, but it sounds like there's urgency behind this so I won't be a blocker. However, can we create a follow up issue to clean this up and remove the need for a second Onyx subscription? I'm not sure exactly how it should be cleaned up but it definitely doesn't need to be like this. Thanks!

OSBotify · 2023-10-10T09:19:36Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

techievivek · 2023-10-10T09:34:21Z

@arosiclair Created one here for the cleanup/improvements: #29165

OSBotify · 2023-10-11T05:09:23Z

🚀 Deployed to staging by https://github.com/techievivek in version: 1.3.81-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

aimane-chnaif · 2023-10-11T09:57:49Z

src/pages/ProfilePage.js

+                if (Number(session.accountID) === accountID || Session.isAnonymousUser()) {
+                    return null;
+                }
+                return `${ONYXKEYS.COLLECTION.REPORT}${lodashGet(ReportUtils.getChatByParticipants([accountID]), 'reportID', '')}`;


This caused regression when user opens new user profile link first time.
This happens because report_ returning all reports.
We should fix this by setting report_0 or report_null or null

Suggested change

if (Number(session.accountID) === accountID || Session.isAnonymousUser()) {

return null;

}

return `${ONYXKEYS.COLLECTION.REPORT}${lodashGet(ReportUtils.getChatByParticipants([accountID]), 'reportID', '')}`;

const reportID = lodashGet(ReportUtils.getChatByParticipants([accountID]), 'reportID', '');

if (Number(session.accountID) === accountID || Session.isAnonymousUser() || !reportID) {

return null;

}

return `${ONYXKEYS.COLLECTION.REPORT}${reportID}`;

Right now, app crashes and will be a deploy blocker soon

raised PR for the quick fix

situchan · 2023-10-11T20:38:56Z

src/pages/ReportDetailsPage.js

@@ -61,8 +61,7 @@ const defaultProps = {
 function ReportDetailsPage(props) {
    const policy = useMemo(() => props.policies[`${ONYXKEYS.COLLECTION.POLICY}${props.report.policyID}`], [props.policies, props.report.policyID]);
    const isPolicyAdmin = useMemo(() => PolicyUtils.isPolicyAdmin(policy), [policy]);
-    const shouldDisableSettings = useMemo(() => ReportUtils.shouldDisableSettings(props.report), [props.report]);
-    const shouldUseFullTitle = !shouldDisableSettings || ReportUtils.isTaskReport(props.report);
+    const shouldUseFullTitle = ReportUtils.isTaskReport(props.report);


Is there reason for removing !shouldDisableSettings || condition here?
This caused regression - #29321

Edit: Ah I think I got answer here

OSBotify · 2023-10-12T04:50:54Z

🚀 Deployed to staging by https://github.com/techievivek in version: 1.3.83-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

s77rt · 2023-10-16T21:51:37Z

src/pages/settings/Report/ReportSettingsPage.js

-    const shouldDisableSettings = _.isEmpty(report) || ReportUtils.shouldDisableSettings(report) || ReportUtils.isArchivedRoom(report);
+    const shouldDisableSettings = _.isEmpty(report) || ReportUtils.isArchivedRoom(report);


After enabling settings for all the reports and revealing access to the notification preference page. We had to test the offline behavior for the newly enabled reports. Coming from #29334 this caused a regression where the creation of a new task missed the optimistic prop notificationPreference.

shubham1206agra · 2023-11-07T02:06:17Z

src/libs/ReportUtils.js

-    if (isChatRoom(report) || isPolicyExpenseChat(report) || isChatThread(report) || isTaskReport(report) || isMoneyRequestReport(report)) {
-        Navigation.navigate(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(report.reportID));
-        return;
-    }
-    if (participantAccountIDs.length === 1) {
+    if (isDM(report) && participantAccountIDs.length === 1) {


This created a regression which is fixed in #30182

shubham1206agra · 2024-03-07T12:11:08Z

src/pages/ProfilePage.js

+                if (Number(session.accountID) === accountID || Session.isAnonymousUser()) {
+                    return null;
+                }
+                return `${ONYXKEYS.COLLECTION.REPORT}${lodashGet(ReportUtils.getChatByParticipants([accountID]), 'reportID', '')}`;


You haven't taken into account the dependency of getChatByParticipants on reports which caused #35654

update the design for group chats

32df987

techievivek self-assigned this Sep 25, 2023

techievivek added 3 commits September 27, 2023 07:07

Merge branch 'main' into techievivek_add_notif_pref_all

78bcb7e

fix profile page

d122a0d

updated details page to also count for group chats

c5946e3

techievivek marked this pull request as ready for review September 27, 2023 00:42

techievivek requested a review from a team as a code owner September 27, 2023 00:42

melvin-bot bot requested review from mollfpr and removed request for a team September 27, 2023 00:42

fix the spacing

cf82041

techievivek requested a review from arosiclair September 27, 2023 00:55

use prettier

083c924

techievivek mentioned this pull request Sep 27, 2023

[HOLD for payment 2023-10-23] [HOLD for payment 2023-10-23] [HOLD for payment 2023-10-20] [HOLD for payment 2023-10-20] Wave8: Add notification preferences to all report types #27394

Closed

mollfpr reviewed Sep 27, 2023

View reviewed changes

src/pages/ProfilePage.js Outdated Show resolved Hide resolved

mollfpr reviewed Sep 27, 2023

View reviewed changes

src/pages/ProfilePage.js Show resolved Hide resolved

get chat report for profile page

994140d

techievivek requested a review from mollfpr September 27, 2023 07:44

mollfpr approved these changes Oct 9, 2023

View reviewed changes

melvin-bot bot requested a review from hayata-suenaga October 9, 2023 09:42

resolve conflicts

ac77a36

hayata-suenaga removed their request for review October 10, 2023 05:57

arosiclair approved these changes Oct 10, 2023

View reviewed changes

techievivek merged commit a63f853 into main Oct 10, 2023
16 checks passed

techievivek deleted the techievivek_add_notif_pref_all branch October 10, 2023 09:12

techievivek mentioned this pull request Oct 10, 2023

[HOLD for payment 2023-11-17] [$250] Clean the usage of double onyx call on profile page #29165

Closed

OSBotify mentioned this pull request Oct 11, 2023

Deploy Checklist: New Expensify 2023-10-11 #29252

Closed

aimane-chnaif reviewed Oct 11, 2023

View reviewed changes

aimane-chnaif mentioned this pull request Oct 11, 2023

[CP Staging] fix crash when first open contact profile page #29277

Merged

58 tasks

ginsuma mentioned this pull request Oct 11, 2023

[HOLD for payment 2023-10-25] [$250] Workspace - Display name for #announce room members in details page is cut off #29321

Closed

6 tasks

Pujan92 mentioned this pull request Oct 11, 2023

[HOLD for payment 2023-10-23] [HOLD for payment 2023-10-23] [HOLD for payment 2023-10-20] [HOLD for payment 2023-10-20] [$500] DEV : App crash when visiting Task's settings for the first time #29334

Closed

6 tasks

situchan reviewed Oct 11, 2023

View reviewed changes

OSBotify mentioned this pull request Oct 12, 2023

Deploy Checklist: New Expensify 2023-10-12 #29417

Closed

97 tasks

s77rt reviewed Oct 16, 2023

View reviewed changes

shubham1206agra reviewed Nov 7, 2023

View reviewed changes

shubham1206agra mentioned this pull request Nov 7, 2023

[HOLD for payment 2023-11-09] [$500] Task - Task assignee profile opens when clicking on the header of task report #30182

Closed

6 tasks

shubham1206agra reviewed Mar 7, 2024

View reviewed changes

shubham1206agra mentioned this pull request Mar 7, 2024

[Awaiting C+ for payment][$500] Profile - Private notes doesn't appear on profile when navigate via Share code link #35654

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add notification preference for all the reports and update the design for the group chats #28200

Add notification preference for all the reports and update the design for the group chats #28200

techievivek commented Sep 25, 2023 •

edited

Loading

melvin-bot bot commented Sep 27, 2023

techievivek commented Sep 27, 2023 •

edited

Loading

techievivek commented Sep 27, 2023

mollfpr commented Sep 27, 2023

techievivek commented Sep 27, 2023

mollfpr commented Sep 27, 2023

MitchExpensify commented Sep 27, 2023

mollfpr left a comment

mollfpr commented Sep 27, 2023

techievivek commented Sep 27, 2023

mollfpr commented Sep 27, 2023

techievivek commented Sep 27, 2023 •

edited

Loading

mollfpr commented Sep 27, 2023

mollfpr commented Sep 27, 2023

techievivek commented Oct 9, 2023

mollfpr left a comment

melvin-bot bot commented Oct 9, 2023

techievivek commented Oct 9, 2023

techievivek commented Oct 10, 2023

mollfpr commented Oct 10, 2023

hayata-suenaga commented Oct 10, 2023

arosiclair Oct 10, 2023

OSBotify commented Oct 10, 2023

techievivek commented Oct 10, 2023

OSBotify commented Oct 11, 2023

aimane-chnaif Oct 11, 2023

aimane-chnaif Oct 11, 2023

situchan Oct 11, 2023 •

edited

Loading

OSBotify commented Oct 12, 2023

s77rt Oct 16, 2023

shubham1206agra Nov 7, 2023

shubham1206agra Mar 7, 2024

		const shouldDisableSettings = _.isEmpty(report) \|\| ReportUtils.shouldDisableSettings(report) \|\| ReportUtils.isArchivedRoom(report);
		const shouldDisableSettings = _.isEmpty(report) \|\| ReportUtils.isArchivedRoom(report);

Add notification preference for all the reports and update the design for the group chats #28200

Add notification preference for all the reports and update the design for the group chats #28200

Conversation

techievivek commented Sep 25, 2023 • edited Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

melvin-bot bot commented Sep 27, 2023

techievivek commented Sep 27, 2023 • edited Loading

techievivek commented Sep 27, 2023

mollfpr commented Sep 27, 2023

techievivek commented Sep 27, 2023

mollfpr commented Sep 27, 2023

MitchExpensify commented Sep 27, 2023

mollfpr left a comment

Choose a reason for hiding this comment

mollfpr commented Sep 27, 2023

techievivek commented Sep 27, 2023

mollfpr commented Sep 27, 2023

techievivek commented Sep 27, 2023 • edited Loading

mollfpr commented Sep 27, 2023

mollfpr commented Sep 27, 2023

techievivek commented Oct 9, 2023

mollfpr left a comment

Choose a reason for hiding this comment

melvin-bot bot commented Oct 9, 2023

techievivek commented Oct 9, 2023

techievivek commented Oct 10, 2023

mollfpr commented Oct 10, 2023

hayata-suenaga commented Oct 10, 2023

arosiclair Oct 10, 2023

Choose a reason for hiding this comment

OSBotify commented Oct 10, 2023

techievivek commented Oct 10, 2023

OSBotify commented Oct 11, 2023

aimane-chnaif Oct 11, 2023

Choose a reason for hiding this comment

aimane-chnaif Oct 11, 2023

Choose a reason for hiding this comment

situchan Oct 11, 2023 • edited Loading

Choose a reason for hiding this comment

OSBotify commented Oct 12, 2023

s77rt Oct 16, 2023

Choose a reason for hiding this comment

shubham1206agra Nov 7, 2023

Choose a reason for hiding this comment

shubham1206agra Mar 7, 2024

Choose a reason for hiding this comment

techievivek commented Sep 25, 2023 •

edited

Loading

techievivek commented Sep 27, 2023 •

edited

Loading

techievivek commented Sep 27, 2023 •

edited

Loading

situchan Oct 11, 2023 •

edited

Loading