[$250] Bug: old message not loading while switching from offline to online reported by @gadhiyamanan

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. go any chat which has lots of messages
2. go to offline
3. go back to online try to load old messages

Expected Result:

messages should load

Actual Result:

old message not loading

Workaround:

unknown

Platform:

Where is this issue occurring?

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

Version Number: 1.2.12-3 Reproducible in staging?: y Reproducible in production?: y Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos:

https://user-images.githubusercontent.com/43996225/195211614-553a84b6-d8c3-4415-9a72-6a51f36585ed.mp4

https://user-images.githubusercontent.com/43996225/195211618-48ca49dc-ca98-4d7f-9955-264b5b5f3ac5.mov

Expensify/Expensify Issue URL: Issue reported by: @gadhiyamanan Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1665499460172419

View all open jobs on GitHub

[melvin-bot[bot]]

Triggered auto assignment to @zanyrenney (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[zanyrenney]

I actually can't even scroll at all to old messages- regardless of being offline > online again when I test this in my own account on STG.

But I can still scroll to old messages when I test this on PROD:

Looks like we are tackling scrolling more generally here: https://github.com/Expensify/App/issues/2545. And it's been reported in Slack as a big complaint here.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

Triggered auto assignment to @slafortune (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[madmax330]

Yep looks like when we get back online we stop calling the command to get more chats

[madmax330]

I can repro this on my account in staging

[tienifr]

Proposal

Problem

• Currently, in https://github.com/Expensify/App/blob/7044ab2b96067081c8f67191e51fa859dba66d91/src/pages/home/report/ReportActionsView.js#L263 when user scroll to get old message, in function loadMoreChats, ReadOldestAction will be called and update isLoadingMoreReportActions=true. After that when API is successfully called or error return by BE, will update isLoadingMoreReportActions=false.

  API.read('ReadOldestAction',
        {
            reportID,
            reportActionsOffset: oldestActionSequenceNumber,
        },
        {
            optimisticData: [{
                onyxMethod: CONST.ONYX.METHOD.MERGE,
                key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
                value: {
                    isLoadingMoreReportActions: true,
                },
            }],
            successData: [{
                onyxMethod: CONST.ONYX.METHOD.MERGE,
                key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
                value: {
                    isLoadingMoreReportActions: false,
                },
            }],
            failureData: [{
                onyxMethod: CONST.ONYX.METHOD.MERGE,
                key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
                value: {
                    isLoadingMoreReportActions: false,
                },
            }],
        });

• In offline mode, ReadOldestAction will be called first and update isLoadingMoreReportActions=true, but API not called => responseData =undefined and in function SaveResponseInOnyx will return void without update Onyx. So isLoadingMoreReportActions still being true, at this case in function loadMoreChats ReadOldestAction won't be called when user back to online and scroll to get old message.

function SaveResponseInOnyx(response, request) {
    return response
        .then((responseData) => {
            const onyxUpdates = [];
            if (!responseData) {
+           we dont't have responseData
                return;
            }
            if (responseData.jsonCode === 200 && request.successData) {
                onyxUpdates.push(...request.successData);
            } else if (responseData.jsonCode !== 200 && request.failureData) {
                onyxUpdates.push(...request.failureData);
            }
           ...
        });
}

Solution

In https://github.com/Expensify/App/blob/7044ab2b96067081c8f67191e51fa859dba66d91/src/libs/actions/Report.js add function resetLoadMoreReportActions to set isLoadingMoreReportActions in this report to false

+ function resetLoadMoreReportActions(reportID) {
+    Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { isLoadingMoreReportActions: false })
+ }

export {
    ...
+   resetLoadMoreReportActions
}

In https://github.com/Expensify/App/blob/7044ab2b96067081c8f67191e51fa859dba66d91/src/pages/home/report/ReportActionsView.js in shouldUpdateComponent lifecycle, when user come back online mode, we'll check this.props.report.isLoadingMoreReportActions, if true we'll change isLoadingMoreReportActions in this report to false.

if (lodashGet(nextProps.network, 'isOffline') !== lodashGet(this.props.network, 'isOffline')) {
+            if (!lodashGet(nextProps.network, 'isOffline') && this.props.report.isLoadingMoreReportActions) {
+                Report.resetLoadMoreReportActions(this.props.report.reportID)
+            }
            return true;
        }

https://user-images.githubusercontent.com/113963320/196002715-acf6db4f-5d44-45ee-9f68-0674d2ea04bb.mp4

[thienlnam]

@tienifr That's a great investigation - thanks for that! That's a well described issue, so we optimistically set isLoadingMoreReportActions to true. ReadOldestAction is not requeued when the user is back online since it's a read request, but when we reconnect, we should be calling Report.reconnect here which should reset our isLoadingReportActions to false after a successful/failed request. At that point, isLoadingMoreReportActions would be updated and then we wouldn't need to it override it here. Could you try to investigate why that isn't happening?

[parasharrajat]

I am preoccupied with some other stuff. @thienlnam Please assign this to someone else. Thanks.

[tienifr]

@thienlnam In here, isLoadingReportActions will be updated but isLoadingMoreReportActions is not updated in Report.openReport and Report.reconnect https://github.com/Expensify/App/blob/e38df100d62d197689bb16888a3ac7859f157681/src/libs/actions/Report.js#L695-L725 https://github.com/Expensify/App/blob/e38df100d62d197689bb16888a3ac7859f157681/src/libs/actions/Report.js#L732-L758

Solution 2

If you want to update isLoadingMoreReportActions in https://github.com/Expensify/App/blob/e38df100d62d197689bb16888a3ac7859f157681/src/pages/home/report/ReportActionsView.js#L163-L175, we must pass isLoadingMoreReportActions: false to value of optimisticData in function openReport and reconnect.

function openReport(reportID) {
    API.write('openReport',
            ...
             optimisticData: [{
                ...
                value: {
                    isLoadingReportActions: true,
+                 isLoadingMoreReportActions: false,
                },
            }],
            ...
}

function reconnect(reportID) {
    API.write('ReconnectToReport',
             ...
             optimisticData: [{
                ...
                value: {
                    isLoadingReportActions: true,
+                 isLoadingMoreReportActions: false,
                },
            }],
            ...
        });
}

Solution 3

A simpler way, https://github.com/Expensify/App/blob/7044ab2b96067081c8f67191e51fa859dba66d91/src/pages/home/report/ReportActionsView.js#L263 in function loadMoreChats if user in offline mode we'll immediately return.

loadMoreChats(){
-       if (this.props.report.isLoadingMoreReportActions) {
+       if (this.props.report.isLoadingMoreReportActions || lodashGet(nextProps.network, 'isOffline')) {
            return;
        }
        ...
}

[thienlnam]

Thanks for looking into that! Let's go with solution 2, that works better with the system we have currently

[melvin-bot[bot]]

📣 @tienifr You have been assigned to this job by @thienlnam! Please apply to this job in Upwork 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 📖

[tienifr]

@thienlnam Could you please send the Upwork job link that I can apply to? thanks

[slafortune]

@tienifr - here ya go! https://www.upwork.com/ab/applicants/1582472084181204992/job-details

[tienifr]

Thanks @slafortune, I applied to the job and the PR will be ready by October 21

[melvin-bot[bot]]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist will need to be completed before the issue can be closed:

• [x] A regression test has been added or updated so that the same bug will not reach production again. Link to the updated test here: https://github.com/Expensify/Expensify/issues/243010
• [x] The PR that introduced the bug has been identified. Link to the PR: Checking [here] - https://github.com/Expensify/App/pull/11439(https://expensify.slack.com/archives/C02NK2DQWUX/p1666968409254259)
• [ ] 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:
• [ ] A discussion in #contributor-plus 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:
• [x] Payment has been made to the issue reporter (if applicable)
• [x] Payment has been made to the contributor that fixed the issue (if applicable)
• [x] Payment has been made to the contributor+ that helped on the issue (if applicable)

[melvin-bot[bot]]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist will need to be completed before the issue can be closed:

• [ ] A regression test has been added or updated so that the same bug will not reach production again. Link to the updated test here:
• [ ] The PR that introduced the bug has been identified. Link to the PR:
• [ ] 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:
• [ ] A discussion in #contributor-plus 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:
• [ ] Payment has been made to the issue reporter (if applicable)
• [ ] Payment has been made to the contributor that fixed the issue (if applicable)
• [ ] Payment has been made to the contributor+ that helped on the issue (if applicable)

[melvin-bot[bot]]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist will need to be completed before the issue can be closed:

• [ ] A regression test has been added or updated so that the same bug will not reach production again. Link to the updated test here:
• [ ] The PR that introduced the bug has been identified. Link to the PR:
• [ ] 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:
• [ ] A discussion in #contributor-plus 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:
• [ ] Payment has been made to the issue reporter (if applicable)
• [ ] Payment has been made to the contributor that fixed the issue (if applicable)
• [ ] Payment has been made to the contributor+ that helped on the issue (if applicable)

[melvin-bot[bot]]

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

• https://github.com/Expensify/App/pull/12126

If no regressions arise, payment will be issued on 2022-11-09. :confetti_ball:

[gadhiyamanan]

@slafortune Upwork job missing for reporting bonus

[tienifr]

hi @slafortune I applied to the job a while ago as in https://github.com/Expensify/App/issues/11737#issuecomment-1285231203 but haven't received the offer for the job yet, can you please help check? thanks

[thienlnam]

cc @slafortune Looks like we're missing some payments here

[slafortune]

Sorry for my delay on this - looking for some help here and will get this done by tomorrow!

[melvin-bot[bot]]

📣 @thesahindia You have been assigned to this job by @slafortune! Please apply to this job in Upwork 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 📖

[slafortune]

@thesahindia @gadhiyamanan I've sent you both an offer for this, let me know once you've accepted that and I'll get this paid out - sorry for my delay!

[gadhiyamanan]

@slafortune accepted 🎉

[thesahindia]

Accepted, thanks!

[slafortune]

@thesahindia I’m trying to wrap up this GH and am looking for the original PR that caused the issue, can you help me out with that?