Allow App to Navigate to Last Visited Room On visiting with HOME url #6978

K4tsuki · 2022-01-03T03:41:10Z

Fixing navigate to last visited room feature when user access App using HOME url
If onyx local data is exist, App will navigate to last visited room on that device.
If onyx is not exist in the device, App will use server data, hence will open last visited room by other device.

Details

Revert change on Remove unnecessary calls to updateLastRead #4078 because this feature (navigate to last visited room) depend on lastVisitedTimestamp and it is set by updateLastReadActionID. [Instructed]
Remove unnecessary updateLastReadActionID on create new message [Instructed]
Remove updateLastReadActionID on keyboardDidShow ( not instructed. I have tested it on android: updateLastReadActionID is being called each time user open keyboard. if user open a room updateLastReadActionID is called automatically. When he is on top scroll (for example reading old message) then new message coming, updateLastReadActionID is also called (If updateLastReadActionID not called here, then updateLastReadActionID is necessary when he open keyboard). So this updateLastReadActionID when keyboard open up is unnecessary.

Fixed Issues

$ #6474

Tests

Open web new expensify
Navigate between rooms. Remember last time visited room.
Close tab
Open web new expensify on new tab with HOME url
App should navigate tor last visited room
############
Create new message
When creating new message, API Report_UpdateLastRead is called only twice. (on developer tools)
############
On android When keyboard show up on report action view, Report_UpdateLastRead is not called unnecesarrily.

QA Steps

Open web new expensify
Navigate between rooms. Remember last time visited room.
Close tab
Open web new expensify on new tab with HOME url
App should navigate tor last visited room
###############
All scenarios should be tested when messages are marked read. When we switch between reports, When we resize the window on Desktop from mobile to Web, etc...
############
Create new message
When creating new message, API Report_UpdateLastRead is called only twice. (on developer tools)
############
On android When keyboard show up on report action view, Report_UpdateLastRead is not called unnecessarily.

Tested On

Screenshots

Web

last_visit.mp4

Mobile Web

Desktop

iOS

Android

…nnecesary updateLastRead

parasharrajat · 2022-01-03T09:30:25Z

@K4tsuki Could you please add videos for other platforms as well?

marcaaron

Looks good. There are just a couple of style inconsistencies that need to be fixed.

src/pages/home/report/ReportActionsView.js

parasharrajat

Works great.
Fixes:

Current issue.
Last visited chat issue on the SearchPage.

🎀 👀 🎀 C+ reviewed

parasharrajat · 2022-01-04T13:14:12Z

src/pages/home/report/ReportActionsView.js

-            this.scrollToListBottom();
+            ReportScrollManager.scrollToBottom();


Just noticed, Why aren't we calling scrollToBottomAndUpdateLastRead here?

This is explained in the issue description:

Remove updateLastReadActionID on keyboardDidShow ( not instructed. I have tested it on android: updateLastReadActionID is being called each time user open keyboard. if user open a room and he is on a room, updateLastReadActionID is called automatically, whether he is viewing old message (scroll at the top) or not. So, when scroll at the top and user open keyboard updateLastReadActionID is unnecessary. )

I think the assumption is that we are already updating the last read when the report opens and so it is unnecessary to do it again when the keyboard opens. Feels fine to me but maybe there's some reason why we are setting it when the keyboard opens? I would guess the original intention is to just scroll to the bottom. But the excess calls to the API are because those two behaviors were stuck together before.

I don't remember why it was added so it seems fine. But let's add proper tests so that QA can test the unread/read behavior properly.

All scenarios should be tested when messages are marked read.

When we switch between reports.

When we resize the window on Desktop from mobile to Web.

etc...

CC @K4tsuki See ⬆️

marcaaron · 2022-01-11T00:41:11Z

I think we are working on getting the tests passing. Is this safe to merge before then? cc @roryabraham @AndrewGable

roryabraham · 2022-01-11T02:01:20Z

Yes, all the standard non-broken Jest tests passed. This is safe to merge with regard to unit tests.

K4tsuki · 2022-01-11T07:44:47Z

@parasharrajat I have updated the QA steps.

botify · 2022-01-11T16:16:51Z

@marcaaron looks like this was merged without passing tests. Please add a note explaining why this was done and remove the Emergency label if this is not an emergency.

marcaaron · 2022-01-11T16:17:18Z

Not an Emergency. See Rory's comment above.

OSBotify · 2022-01-11T16:17:44Z

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

OSBotify · 2022-01-12T05:54:11Z

🚀 Deployed to staging by @marcaaron in version: 1.1.27-2 🚀

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

OSBotify · 2022-01-12T11:32:19Z

🚀 Deployed to staging by @marcaaron in version: 1.1.27-3 🚀

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

OSBotify · 2022-01-12T18:54:22Z

🚀 Deployed to staging by @marcaaron in version: 1.1.27-3 🚀

platform	result
🤖 android 🤖	skipped 🚫
🖥 desktop 🖥	skipped 🚫
🍎 iOS 🍎	skipped 🚫
🕸 web 🕸	skipped 🚫

mvtglobally · 2022-01-14T03:43:25Z

As per steps this PR needs URL change. QA validated on Web (PASS) and It can't be QA-ed on Android app

parasharrajat · 2022-01-14T04:30:19Z

You can check this on Search Page.

Open any chat.
Confirm that it is visible on the top on search Page.

@mvtglobally

OSBotify · 2022-01-14T19:59:39Z

🚀 Deployed to production by @Jag96 in version: 1.1.29-5 🚀

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

Allow updateLastRead on same sequence number as previous and remove u…

931f6a6

…nnecesary updateLastRead

K4tsuki requested a review from a team as a code owner January 3, 2022 03:41

MelvinBot requested review from marcaaron and parasharrajat and removed request for a team January 3, 2022 03:41

marcaaron requested changes Jan 3, 2022

View reviewed changes

src/pages/home/report/ReportActionsView.js Outdated Show resolved Hide resolved

src/pages/home/report/ReportActionsView.js Outdated Show resolved Hide resolved

marcaaron assigned K4tsuki Jan 3, 2022

Missing semicolons

3438d86

marcaaron reviewed Jan 4, 2022

View reviewed changes

src/pages/home/report/ReportActionsView.js Outdated Show resolved Hide resolved

fixing javascript style

20c53ce

parasharrajat approved these changes Jan 4, 2022

View reviewed changes

parasharrajat suggested changes Jan 4, 2022

View reviewed changes

marcaaron approved these changes Jan 4, 2022

View reviewed changes

retrigger checks

b7e77f7

parasharrajat approved these changes Jan 11, 2022

View reviewed changes

marcaaron merged commit 913bd46 into Expensify:main Jan 11, 2022

botify added the Emergency label Jan 11, 2022

marcaaron removed the Emergency label Jan 11, 2022

OSBotify mentioned this pull request Jan 12, 2022

Deploy Checklist: New Expensify 2022-01-12 #7144

Closed

54 tasks

botify mentioned this pull request Jan 14, 2022

[HOLD for payment 2022-01-21] User is not navigated last active conversation after loading Home URL - Reported by: @parasharrajat #6474

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow App to Navigate to Last Visited Room On visiting with HOME url #6978

Allow App to Navigate to Last Visited Room On visiting with HOME url #6978

K4tsuki commented Jan 3, 2022 •

edited

Loading

parasharrajat commented Jan 3, 2022

marcaaron left a comment

parasharrajat left a comment •

edited

Loading

parasharrajat Jan 4, 2022 •

edited

Loading

marcaaron Jan 4, 2022

parasharrajat Jan 4, 2022 •

edited

Loading

parasharrajat Jan 11, 2022

marcaaron commented Jan 11, 2022

roryabraham commented Jan 11, 2022

K4tsuki commented Jan 11, 2022 •

edited

Loading

botify commented Jan 11, 2022

marcaaron commented Jan 11, 2022

OSBotify commented Jan 11, 2022

OSBotify commented Jan 12, 2022

OSBotify commented Jan 12, 2022

OSBotify commented Jan 12, 2022

mvtglobally commented Jan 14, 2022

parasharrajat commented Jan 14, 2022

OSBotify commented Jan 14, 2022

		this.scrollToListBottom();
		ReportScrollManager.scrollToBottom();

Allow App to Navigate to Last Visited Room On visiting with HOME url #6978

Allow App to Navigate to Last Visited Room On visiting with HOME url #6978

Conversation

K4tsuki commented Jan 3, 2022 • edited Loading

Details

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

parasharrajat commented Jan 3, 2022

marcaaron left a comment

Choose a reason for hiding this comment

parasharrajat left a comment • edited Loading

Choose a reason for hiding this comment

parasharrajat Jan 4, 2022 • edited Loading

Choose a reason for hiding this comment

marcaaron Jan 4, 2022

Choose a reason for hiding this comment

parasharrajat Jan 4, 2022 • edited Loading

Choose a reason for hiding this comment

parasharrajat Jan 11, 2022

Choose a reason for hiding this comment

marcaaron commented Jan 11, 2022

roryabraham commented Jan 11, 2022

K4tsuki commented Jan 11, 2022 • edited Loading

botify commented Jan 11, 2022

marcaaron commented Jan 11, 2022

OSBotify commented Jan 11, 2022

OSBotify commented Jan 12, 2022

OSBotify commented Jan 12, 2022

OSBotify commented Jan 12, 2022

mvtglobally commented Jan 14, 2022

parasharrajat commented Jan 14, 2022

OSBotify commented Jan 14, 2022

K4tsuki commented Jan 3, 2022 •

edited

Loading

parasharrajat left a comment •

edited

Loading

parasharrajat Jan 4, 2022 •

edited

Loading

parasharrajat Jan 4, 2022 •

edited

Loading

K4tsuki commented Jan 11, 2022 •

edited

Loading