[$250] Tasks - App goes back to main chat or LHN when editing a task's description

[lanitochka17]

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: 9.0.22-5 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/4872853 Email or phone of affected tester (no customers): gatantm+82@gmail.com Issue reported by: Applause - Internal Team

Action Performed:

1. Go to any chat and create a task
2. Open the task and click on description
3. Make any edits > Save

Expected Result:

The task is saved and user stays on task view

Actual Result:

The user is returned back to the LHN on mobile or the chat that the task was in. The same happens on the name field after the description was edited once

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• [ ] Android: Native
• [ ] Android: mWeb Chrome
• [ ] iOS: Native
• [ ] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/c2ec4dc9-6750-4fc5-82cb-589a0634c0de

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0135f56d92cfd4346d
• Upwork Job ID: 1826020469224260035
• Last Price Increase: 2024-09-10

Issue Owner

Current Issue Owner: @getusha

[melvin-bot[bot]]

Triggered auto assignment to @deetergp (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[github-actions[bot]]

: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:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[lanitochka17]

We think that this bug might be related to #vip-vsp

[daledah]

Edited by proposal-police: This proposal was edited at {your_timestamp_here}.

Proposal

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

The user is returned back to the LHN on mobile or the chat that the task was in. The same happens on the name field after the description was edited once

What is the root cause of that problem?

• When clicking on CTA button in edit description page, we call: https://github.com/Expensify/App/blob/78c20f046be7a307a52050657bc041dd57d2fd53/src/pages/tasks/TaskDescriptionPage.tsx#L55

then: https://github.com/Expensify/App/blob/78c20f046be7a307a52050657bc041dd57d2fd53/src/pages/tasks/TaskDescriptionPage.tsx#L58

then In PR, we call: https://github.com/Expensify/App/blob/78c20f046be7a307a52050657bc041dd57d2fd53/src/pages/home/report/ReportActionsList.tsx#L370

• In above, function Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(report.reportID)) can be called before Navigation.dismissModal(report?.reportID) hence the app go back to main chat.

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

  We can dismiss modal before call editTask in: https://github.com/Expensify/App/blob/78c20f046be7a307a52050657bc041dd57d2fd53/src/pages/tasks/TaskDescriptionPage.tsx#L55

The same should be applied in edit title flow.

What alternative solutions did you explore? (Optional)

In case Task.editTask will be called, we can move logic:

Navigation.dismissModal(report?.reportID);

to below: https://github.com/Expensify/App/blob/6334d36a049d01cfa9a445f3747125d79a00bc98/src/libs/actions/Task.ts#L567

[daledah]

Proposal updated

[bernhardoj]

Proposal

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

The main chat opens after editing task description.

What is the root cause of that problem?

When we edit the task, we notify the report that a new action has been added. https://github.com/Expensify/App/blob/80236f5bd80086950344eb10aa89c174bf7c923e/src/libs/actions/Task.ts#L568-L569

It will trigger scrollToBottomForCurrentUserAction and if hasNewestReportActionRef is false, then we navigate to the self-report to clear the reportActionID params. https://github.com/Expensify/App/blob/80236f5bd80086950344eb10aa89c174bf7c923e/src/pages/home/report/ReportActionsList.tsx#L362-L372

After editing the task, we call dismissModal to close the RHP. https://github.com/Expensify/App/blob/80236f5bd80086950344eb10aa89c174bf7c923e/src/pages/tasks/TaskDescriptionPage.tsx#L55-L58

In dismissModal, if the target report ID is the same as the topmost report ID, which is true in our case, we simply pop the current screen. https://github.com/Expensify/App/blob/80236f5bd80086950344eb10aa89c174bf7c923e/src/libs/Navigation/dismissModalWithReport.ts#L70-L72

So, the first navigate to the self-report already closes the RHP and the dismissModal calls pop which brings us back to the parent report.

Even though this happens after my PR where I add the navigate to self-report logic, but we already guard it with hasNewestReportActionRef condition. So, the real issue here is, why is hasNewestReportActionRef false?

The issue is both FE and BE. hasNewestReportAction compares the last report action created with the report lastVisibleActionCreated. https://github.com/Expensify/App/blob/80236f5bd80086950344eb10aa89c174bf7c923e/src/pages/home/report/ReportActionsList.tsx#L295

In FE, we optimistically add the update task description system message, but we don't update the lastVisibleActionCreated. https://github.com/Expensify/App/blob/80236f5bd80086950344eb10aa89c174bf7c923e/src/libs/actions/Task.ts#L505-L524

For the BE issue, looks like the BE doesn't consider the update task system message created as the report lastVisibleActionCreated.

From the screenshot above, we can see that after the EditTask is completed, there are 2 reports onyx data. The first one contains the correct lastVisibleActionCreated but the second one contains the wrong lastVisbibleActionCreated (it contains the report CREATED action created value).

This makes hasNewestReportActionRef always false even if we fix it optimistically because when scrollToBottomForCurrentUserAction is called, the optimistic data is not merged yet, so hasNewestReportActionRef is still false because the report lastVisibleActionCreated from the BE is wrong.

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

The fix must be done for both BE and FE. The BE needs to return the correct lastVisibleActionCreated. In FE, we can optimistically update the report lastVisibleActionCreated in editTask to be the same as the edit task system message created. buildOptimisticEditedTaskFieldReportAction will accept a created param to be used as the created value.

const created = DateUtils.getDBTime();
const editTaskReportAction = ReportUtils.buildOptimisticEditedTaskFieldReportAction({title, description}, created);
const optimisticData: OnyxUpdate[] = [
    {
        onyxMethod: Onyx.METHOD.MERGE,
        key: `${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`,
        value: {
            ...,
            lastVisibleActionCreated: created,
        },
    },
];

[tsa321]

Edited by proposal-police: This proposal was edited at 2023-10-01T00:00:00Z.

Proposal

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

Tasks - App goes back to main chat or LHN when editing a task's description

What is the root cause of that problem?

When user edit task, dismissModal will be executed:

https://github.com/Expensify/App/blob/6334d36a049d01cfa9a445f3747125d79a00bc98/src/pages/tasks/TaskDescriptionPage.tsx#L50-L58

and when before calling API write of edit_task:

https://github.com/Expensify/App/blob/6334d36a049d01cfa9a445f3747125d79a00bc98/src/libs/actions/Task.ts#L567-L568

Report.notifyNewAction will be executed first and will execute it's subcribtion function :

https://github.com/Expensify/App/blob/6334d36a049d01cfa9a445f3747125d79a00bc98/src/pages/home/report/ReportActionsList.tsx#L362-L372

which will navigate to the report or scroll to bottom of the report. The navigate is executed first which will make the report screen go back to previous report.

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

If the expected solution is to not scroll to bottom after editing the task, we could remove the Report.notifyNewAction after the API.WRITE EDIT_TASK. This could be used for other similar issues.

What alternative solutions did you explore? (1)

If the expected solution involves scrolling to the bottom, we can move the Report.notifyNewAction from Task.ts to TaskDescriptionPage, placing it below Navigation.dismissModal (ensure to check all usages of the editTaskLibrary function).

If scrolling is only necessary when the page report action list does not have the hasNewestReportAction, we can modify the scrollToBottomForCurrentUserAction function so that it does not scroll when there is a hasNewestReportAction. Alternatively, we could pass a parameter to scrollToBottomForCurrentUserAction to determine whether it should scroll when there is no newestReportAction.

What alternative solutions did you explore? (Optional)

To fix the hasNewestReportAction bug (I think it is different bug), we could use add this code in ReportActionList

    const newestVisibleReportAction = sortedVisibleReportActions[0];
    const newestPendingActionTime = useRef(report.lastVisibleActionCreated);
    const newCreatedUserActions = useMemo(() => {
        const previousLastReportAction = ReportActionsUtils.getReportAction(report.reportID, previousLastIndex.current);
        const previousCreatedDate = new Date(previousLastReportAction.created);
        const previousIndex = sortedVisibleReportActions.findIndex((action) => new Date(action.created) < previousCreatedDate);
        const addedActions = sortedVisibleReportActions.slice(0, previousIndex.current === -1 ? -1 : previousIndex - 1);

        const newCreatedActions = addedActions.filter((action) => (action.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD) && (new Date(action.created) > new Date(newestPendingActionTime.current)));
        if (newCreatedActions.length > 0) {
            newestPendingActionTime.current = newCreatedActions.reduce((a, b) => Math.max(new Date(a.created), new Date(b.created))).created;
        }
        return newCreatedActions;
   },   [lastActionIndex]);

    const hasNewestReportAction = (newCreatedUserActions.length > 0) || (new Date(newestVisibleReportAction?.created) >= new Date(report.lastVisibleActionCreated));

[deetergp]

I think this blocker may be a result of this PR from yesterday's staging deploy https://github.com/Expensify/App/pull/47209

[bernhardoj]

It's actually happening after my PR https://github.com/Expensify/App/pull/46724, but the root cause is actually an existing issue coming from both BE and FE optimistic data issues which is why I posted a proposal for this one.

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~0135f56d92cfd4346d

[melvin-bot[bot]]

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

[nkdengineer]

Proposal

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

The user is returned back to the LHN on mobile or the chat that the task was in. The same happens on the name field after the description was edited once

What is the root cause of that problem?

When we edit the task title, we will dismiss the modal

https://github.com/Expensify/App/blob/685bb4c7bbaa2ef332896de5cace193edb9c6a51/src/pages/tasks/TaskTitlePage.tsx#L55

But before that, we have a logic to that will trigger scrollToBottomForCurrentUserAction here https://github.com/Expensify/App/blob/685bb4c7bbaa2ef332896de5cace193edb9c6a51/src/libs/actions/Task.ts#L568

In the normal case, we will scroll to the bottom but in this case hasNewestReportActionRef is false because we don't update lastVisibleActionCreated of the task report in optimistic data, and BE also returns the wrong created of the task report then we will navigate to the task report before we dismiss the modal. These actions are called almost simultaneously when the task report is removed from the stack navigator.

It doesn't happen when we update the assignee because we wrap the dismiss logic in InteractionManager.runAfterInteractions. But when we edit the assignee, lastVisibleActionCreated is also not updated in optimistic data https://github.com/Expensify/App/blob/685bb4c7bbaa2ef332896de5cace193edb9c6a51/src/pages/tasks/TaskAssigneeSelectorModal.tsx#L200-L204

https://github.com/user-attachments/assets/a11b32be-40b2-48c9-bb83-e4eae74e9212

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

On the FE side, we should update lastVisibleActionCreated of task report when we edit the task title, and description here. We can simply update this to editTaskReportAction.created instead of adding new created param

lastVisibleActionCreated: editTaskReportAction.created

https://github.com/Expensify/App/blob/685bb4c7bbaa2ef332896de5cace193edb9c6a51/src/libs/actions/Task.ts#L513-L515

and the same when editing the task assignee here

lastVisibleActionCreated: editTaskReportAction.created,

https://github.com/Expensify/App/blob/685bb4c7bbaa2ef332896de5cace193edb9c6a51/src/libs/actions/Task.ts#L578

OPTIONAL: We can also wrap dismiss modal in InteractionManager.runAfterInteractions as we do for task assignee page

https://github.com/Expensify/App/blob/685bb4c7bbaa2ef332896de5cace193edb9c6a51/src/pages/tasks/TaskTitlePage.tsx#L55

What alternative solutions did you explore? (Optional)

NA

[deetergp]

@getusha We've got a good number of proposals here for you to review 🎉

[getusha]

Reviewing, i will update today

[getusha]

@nkdengineer your proposal looks pretty much alike @bernhardoj's except a very small detail.

[getusha]

@bernhardoj can we test this without the BE change?

[tsa321]

@getusha I just want to confirm the expected behavior. Do we need to scroll to the bottom each time a user edits a task (title, description)? It will be really annoying if the user wants to edit multiple fields. I remember an issue or conversation (but I forgot which one) that we shouldn’t scroll to the bottom after editing a transaction in the transaction thread. Should we seek the UX designer's opinion on this?

https://github.com/user-attachments/assets/819a0493-d699-438f-96c2-a70d9a9ec5ae

[tsa321]

@getusha also, fixing the lastVisibleActionCreated or hasNewestReportAction(I agree that it is a bug) doesn't resolve this issue for some comment linking navigation. If the task report has many comments and the user clicks on a comment link to jump to the top of the comments, the latest message won't be displayed, causing hasNewestReportAction to be false and reproducing the issue. For example:

https://github.com/user-attachments/assets/edab6d9b-c370-4749-8024-2249ccbb2c2a

[tsa321]

@getusha Additionally, I have updated my proposal on alternative solution to fix the hasNewestReportAction bug without requiring backend changes, but fixing this still reproduce the bug like I mentioned above.

[bernhardoj]

can we test this without the BE change?

@getusha We can do it while offline, but the first edit will still navigate to the main chat because the current report lastReadTime and lastVisbibleActionCreated (from OpenReport) are still not matched. 2nd edit and onward won't navigate to the main chat anymore.

[nkdengineer]

@nkdengineer your proposal looks pretty much alike @bernhardoj's except a very small detail.

@getusha

1. My proposal mentioned why this bug only happens when we change the title or description but doesn't when we change the assignee

2. I also mentioned that lastVisibleActionCreated is not updated when we change the assignee. That can prevent the chat from scrolling to the bottom when we change the assignee as expected behavior here.

3. The way I update the lastVisibleActionCreated in optimistic data is also different

[abzokhattab]

Proposal

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

App returns to Expensify chat after renaming onboarding task more than once

What is the root cause of that problem?

the Navigation.dismissModal function makes the app returns to the previous page https://github.com/Expensify/App/blob/4ffbd422296227ae003bfe0b5e584188bae5d68d/src/pages/tasks/TaskTitlePage.tsx#L55

https://github.com/Expensify/App/blob/5c6d2dd487aa8e1d25ec55ee6f64bdf7961c52ca/src/pages/tasks/TaskDescriptionPage.tsx#L57

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

the Navigation.dismissModal is not needed since it will automatically dismiss the modal on save we we can remove that line in both files

What alternative solutions did you explore? (Optional)

NA

[melvin-bot[bot]]

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

[melvin-bot[bot]]

@deetergp, @getusha Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot[bot]]

@deetergp, @getusha 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[melvin-bot[bot]]

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

[getusha]

I'll update today

[melvin-bot[bot]]

@deetergp, @getusha Huh... This is 4 days overdue. Who can take care of this?

[getusha]

This fell through the cracks, reviewing it today

[getusha]

@nkdengineer do you have any insights on why we need Navigation.dismissModal(report?.reportID);? I am inclined to just remove it but would like to explore the root cause.

[nkdengineer]

@getusha As with other flows, we need Navigation.dismissModal(report?.reportID) to dismiss the modal and go back to the current report after we complete and call the API. The current problem in this case is hasNewestReportAction is wrong when we edit task title/description then this logic here is called unexpectedly which causes this bug. It also causes another bug that the chat doesn't scroll to the bottom if we edit the assignee the second time. So I don't think we should remove Navigation.dismissModal(report?.reportID) logic. Instead we should update optimistic data to make hasNewestReportAction is correct.

[tsa321]

@getusha Maybe we need opinion from @Expensify/design about the expected behavior here.

Do we need to scroll to the bottom each time a user edits a task (title, description)? It will be really annoying if the user wants to edit multiple fields.

https://github.com/user-attachments/assets/819a0493-d699-438f-96c2-a70d9a9ec5ae

[melvin-bot[bot]]

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

[melvin-bot[bot]]

@deetergp, @getusha Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!