Closed izarutskaya closed 1 week ago
Triggered auto assignment to @iwiznia (DeployBlockerCash
), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.
Triggered auto assignment to @lschurr (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.
:wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:
@lschurr 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.
We think this issue might be related to the #collect project.
Crash issue no longer happens on latest main, but we have another issue is loading infinity
https://github.com/Expensify/App/assets/11959869/e4730322-f5cb-4969-94d7-1e708d1013b8
We have 2 issues:
Issue 1: After delete workspace the function loadOlderChats has trigger call infinity due to the report action is empty and FlatList trigger call onEndReached
Issue 2: At this function we have filter out actionName = 'CREATE' and at this function we also filter out actionName = 'CLOSED' so sortedVisibleReportActions is empty and the UI is missing info
Fix issue 1:
We should prevent trigger call onEndReached={loadOlderChats} when sortedVisibleReportActions
is empty
onEndReached={() => {
if (!sortedVisibleReportActions.length || isLoadingOlderReportActions) {
return;
}
loadOlderChats();
}}
Fix issue 2: We will update reportActions when workspace is archived by get all reportActions
+ const isArchivedRoom = ReportUtils.isArchivedRoom(report);
const reportActions = useMemo(() => {
if (!sortedAllReportActions.length) {
return [];
}
+ const filteredReportActions = ReportActionsUtils.getContinuousReportActionChain(sortedAllReportActions, reportActionIDFromRoute);
+ if (isArchivedRoom) {
+ return sortedAllReportActions;
+ };
+ return filteredReportActions;
}, [isArchivedRoom, reportActionIDFromRoute, sortedAllReportActions]);
Update getContinuousReportActionChain
function when isArchivedRoom
exclude actionName = 'CREATE'
I don't think that solution makes sense. You should be able to go back and load all the older messages from an archived chat and the solution proposed will break that, no?
Did anyone find the actual code that makes it crash?
@izarutskaya why is there no desktop video? Also, desktop can't crash, so not sure what is happening in desktop... Is it also not working on web? If so, can you share console logs?
ok did the reproduction steps and not seeing a crash in web, seeing the loading older action issues described above (this is on staging, not main).
Posted in slack here https://expensify.slack.com/archives/C01GTK53T8Q/p1714747609951879
I can reproduce the same issue of infinite loading in android, but not a crash. Don't have an ios to test the crash there.
Is it also not working on web? If so, can you share console logs?
@iwiznia Here is a current behavior on staging
https://github.com/Expensify/App/assets/11959869/736523d1-2ef1-44fd-a7c6-ca427d552f74
After reload
https://github.com/Expensify/App/assets/11959869/62fe0dc0-8c8c-4e7d-87ea-0d0fbeeebeb8
Here is a behavior on latest main and fix
https://github.com/Expensify/App/assets/11959869/b49d9bd5-0b6f-47d1-9810-4b92ce017ce1
Update another place check condition can fix this issue
@izarutskaya I don't see any email in the issue. You should always include the email and other relevant data in the issue report (such as workspaceID, reportID, etc). That way I can retrieve logs for cases where I can't reproduce the issue like this one.
@suneox did you see this?
I don't think that solution makes sense. You should be able to go back and load all the older messages from an archived chat and the solution proposed will break that, no?
I think this might be fixed by https://github.com/Expensify/App/pull/41558. Would someone be able to test out that PR?
My other theory is that the chat doesn't have a CREATED action as the last one for some reason so we don't know that we are at the end of the chat (see https://github.com/janicduplessis/Expensify-App/blob/10fbd6a53177685702bdfa90e102163506e263cf/src/pages/home/report/ReportActionsView.tsx#L320-L321). I have a proposal to have the backend return whether more data is available but it is more of a medium term fix (https://github.com/Expensify/App/issues/41153).
I don't think that solution makes sense. You should be able to go back and load all the older messages from an archived chat and the solution proposed will break that, no?
@iwiznia I have update proposal to handle load all the older messages and prevent infinity trigger getOlderActions
with demo
- if (isArchivedRoom && filteredReportActions.length === 1 && filteredReportActions[0].actionName === CONST.REPORT.ACTIONS.TYPE.CLOSED) {
I think this part is wrong as it only works if the actions is of length 1 but if the other app adds a comment before deleting the workspace I still see the broken behavior. I also don't quite understand what that code is doing exactly.
I think this part is wrong as it only works if the actions is of length 1 but if the other app adds a comment before deleting the workspace I still see the broken behavior. I also don't quite understand what that code is doing exactly.
This update is specific for this issue to avoid regression, so if we still confuse we can check by condition
if (isArchivedRoom) { return sortedAllReportActions }
GetOlderActions
is called in a loop and new messages are never loaded.
There is a bug in getContinuousReportActionChain
where if there is an optimistic action in the middle of a chat we incorrectly detect a gap in the chat. This causes a smaller array to be returned and then this incorrect array is used for pagination.
New messages loaded by GetOlderActions
are never included in the chat because of the incorrect gap so we keep calling GetOlderActions
in a loop since we never reach the CREATED
action, which is the first message in all chats.
For more context and debugging of this problem you can read through this chat in Slack.
We need to fix getContinuousReportActionChain
to properly ignore optimistic actions since the backend does not know about them and previousReportActionID
will be wrong.
I already have a PR ready for the fix since I think this is a deploy blocker https://github.com/Expensify/App/pull/41644, this is based on @roryabraham 's test fix https://github.com/Expensify/App/pull/41598
@iwiznia Tester login credentials (email) - applausetester+omq76@applause.expensifail.com. Let me please include a console logs..
nice, let's move forward with https://github.com/Expensify/App/pull/41644, that looks correct. I kicked off an AdHoc build of that so we can test.
Verified that this is fixed and we didn't use any C+ services so I'm closing this out
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.70-1 Reproducible in staging?: Y Reproducible in production?: N If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4538783&group_by=cases:section_id&group_order=asc&group_id=306202 Email or phone of affected tester (no customers): applausetester+omq76@applause.expensifail.com Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: Applause-Internal team
Action Performed:
Expected Result:
Error should be displayed
Actual Result:
App crashed when user sent attachment offline, deleted workspace from other device and returned online on main device
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
https://github.com/Expensify/App/assets/115492554/9b5beaad-2f01-4a4e-9393-5f70516d3514
View all open jobs on GitHub