[$250] Expense - Mark as unread green line for one transaction is not shown initially

[lanitochka17]

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

---

Version Number: 9.0.47-1 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Issue reported by: Applause - Internal Team

Action Performed:

1. Launch app
2. Tap on a workspace chat
3. Submit an expense
4. Open the expense
5. Select a category
6. For category selected system message is displayed below, mark it as unread
7. Note new green line not displayed
8. Long press the message and open reply in thread
9. Note you can check new green line in thread page
10. Tap from link and now note for category selected system message, green line displayed now

Expected Result:

Mark as unread green line for one transaction must be shown initially before opening reply in thread page

Actual Result:

Mark as unread green line for one transaction is not shown initially before opening reply in thread page

Workaround:

Unknown

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/0b756d09-17e1-43eb-a505-2579e0b9f6be

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021846309887749659812
• Upwork Job ID: 1846309887749659812
• Last Price Increase: 2024-12-03

[melvin-bot[bot]]

Triggered auto assignment to @johncschuster (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.

[lanitochka17]

@johncschuster FYI 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

[melvin-bot[bot]]

@johncschuster Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[truph01]

Edited by proposal-police: This proposal was edited at 2024-10-16 09:27:54 UTC.

Proposal

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

Mark as unread green line for one transaction is not shown initially before opening reply in thread page

What is the root cause of that problem?

When users go to one transaction report, we will open iou report and combine the actions from iou report and transaction thread report

https://github.com/Expensify/App/blob/96ffbfe27c5c1b85b062e6ab50c0adc46b0e94c3/src/pages/home/report/ReportActionsView.tsx#L213

Here is the logic to get unread report action id

https://github.com/Expensify/App/blob/96ffbfe27c5c1b85b062e6ab50c0adc46b0e94c3/src/pages/home/report/ReportActionsList.tsx#L219-L222

we use unreadMarkerTime from report.lastReadTime

https://github.com/Expensify/App/blob/96ffbfe27c5c1b85b062e6ab50c0adc46b0e94c3/src/pages/home/report/ReportActionsList.tsx#L209

The report here is iou report

When users press mark as unread the category selected system message, we call markAsUnread API, this API will update report.lastReadTime, but the updated report is transaction thread report since this message comes from transaction report, not iou report

That why the logic to get unreadMarkerReportActionID is not triggered

There's one more issue when we open iouReport for one transaction:

If users add new messages, they would expect to add them in thread report, but they're actually added in iou report -> That causes the confusion

https://github.com/user-attachments/assets/0a58f75b-567e-4eeb-bcb6-a1cafaa8bda9

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

We should go to the transaction report if there'e one transaction

Update this line to

const transactionThreadReportID = ReportActionsUtils.getOneTransactionThreadReportID(iouReportID ?? '', ReportActionsUtils.getAllReportActions(iouReportID), isOffline);
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(transactionThreadReportID ?? iouReportID));

We also need to update the header to back to ws/1:1 chat

What alternative solutions did you explore? (Optional)

[dukenv0307]

@truph01 Thanks for your proposal. I think we did it on purpose here: https://github.com/Expensify/App/issues/38655

But I'm not sure why we have to do that. Can you elaborate on why we should open the transaction report for one transaction?

[truph01]

@dukenv0307 As you can see here's the expense report with one transaction. It's actually the UI of transaction thread report, so I believe when users add new comments here, they would like to add them in transaction report instead of iou report.

Furthermore, opening transaction report can make our implementation clearer since we're combining 2 reportActions, but just the actions from transaction thread are shown at the beginning -> Users should realize it's the transaction report

[dukenv0307]

That makes sense to me, but I need to hear other thoughts @NikkiWines @shawnborton @dannymcclain

BTW, if we decide to open the thread transaction, we can fix many bugs related to this. For example, this one: https://github.com/Expensify/App/issues/49959 (I'm C+ on that) can be fixed with a minor change

🎀👀🎀 C+ reviewed

[melvin-bot[bot]]

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

[shawnborton]

We don't want to change anything about how the transaction or report is displayed. Let's just try to address the fact that the unread marker isn't showing up as it should. But perhaps I am not understanding correctly, so feel free to rephrase if needed :)

[truph01]

Let's just try to address the fact that the unread marker isn't showing up as it should

That because we're showing the iou report, but the action is transaction report

[thienlnam]

Will let @NikkiWines chime in here, but I am not sure we want to change the view to just the transaction directly. I feel like the intention was to consolidate the one-expense chat experience but anything that happens there is still part of the "report"

[shawnborton]

Yeah, we definitely do not want to change the view. We need to fix the unread marker without changing anything about the view itself.

[truph01]

@shawnborton No, I don't want to change the view itself, just change the reportID in the URL cc @NikkiWines

[truph01]

As you can see in the below video, iou report (first report) has the same view as transaction report (second one). Currently we're showing iou report, so I would like to show transaction report and update the header to be the same as iouReport.

https://github.com/user-attachments/assets/8b4491f7-8662-434d-82a8-2b88d7b986d5

[shawnborton]

Curious what our engineers think about that. I'm mostly just chiming in from a design perspective that we need to keep the view looking exactly the same but add the ability to have the correct unread/new marker here.

[dannymcclain]

Curious what our engineers think about that. I'm mostly just chiming in from a design perspective that we need to keep the view looking exactly the same but add the ability to have the correct unread/new marker here.

+1. It would be great to get some engineering perspectives on this—I feel like there was a fair bit of discussion around how to handle the whole report thread vs transaction thread when we initially implemented this consolidated view.

[NikkiWines]

Furthermore, opening transaction report can make our implementation clearer since we're combining 2 reportActions, but just the actions from transaction thread are shown at the beginning -> Users should realize it's the transaction report

This was originally proposed as a flow for the one-transaction view when we first discussed how to implement it way back when, but ultimately the implementation went a different way and opted to combine the view on the IOU report level instead. I don't think we want to change this now (though I'm not opposed to revisiting the need for this view), and so I don't think changing the reportID is the correct route here.

The intended behavior based on the design brief is that we show the IOU report, and pull in parts of the transaction thread (the ability to select categories, displaying transaction thread report actions, etc.) for a more comprehensive view. However, the location of where those actions occur stay on their requesite reports. If the user changes the category, that's a transaction-level action and the reportAction is ultimately created on the transactionThread. If they comment on the IOU report (even while the view is combined for the one-transaction view) that comment should exist on the IOU report level.

To me, it sounds like we need to check the report.lastReadTime for both the IOU report and transaction thread report to determine if the unread indicator needs to be displayed on the combined view.

[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]]

@johncschuster, @thienlnam, @dukenv0307 Eep! 4 days overdue now. Issues have feelings too...

[dukenv0307]

Waiting for more proposals...

[melvin-bot[bot]]

@johncschuster @thienlnam @dukenv0307 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot[bot]]

@johncschuster, @thienlnam, @dukenv0307 Eep! 4 days overdue now. Issues have feelings too...

[johncschuster]

Raised this in Slack for proposals

[klajdipaja]

Edited by proposal-police: This proposal was edited at 2024-10-29 10:04:29 UTC.

Proposal

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

When we mark unread a message in a transaction the unread marker line does not show up on the message immediately but only after the user leaves the thread and returns to it.

What is the root cause of that problem?

When the user marks as unread a message a backend action is triggered that updates the lastReadTime of the report and we subscribe to the result of that call in this block https://github.com/Expensify/App/blob/2463bf74b4edabdde887636f7874fdbbfcf9eb2c/src/pages/home/report/ReportActionsList.tsx#L270-L283

For transaction threads the reportId that is used for the action is not the one of the parent but the one of the action itself and since we are using reportId of the chat then we are not getting the result of the API call.

The other part of the problem is that the lastReadTime used throughout the component is the one of the chat report which actual latest read time of the user as the user might have read the thread instead of the chat.

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

To fix that we need to add subscriptions for the transaction thread as well:

useEffect(() => {
    const handleUnreadAction = (newLastReadTime: string) => {
        setUnreadMarkerTime(newLastReadTime);
        userActiveSince.current = DateUtils.getDBTime();
    };

    const handleReadNewestAction = (newLastReadTime: string) => {
        setUnreadMarkerTime(newLastReadTime);
    };

    const unreadActionSubscription = DeviceEventEmitter.addListener(`unreadAction_${report.reportID}`, handleUnreadAction);
    const readNewestActionSubscription = DeviceEventEmitter.addListener(`readNewestAction_${report.reportID}`, handleReadNewestAction);

    const unreadActionSubscriptionTransactionThread = DeviceEventEmitter.addListener(`unreadAction_${transactionThreadReport?.reportID}`, handleUnreadAction);
    const readNewestActionSubscriptionTransactionThread = DeviceEventEmitter.addListener(`readNewestAction_${transactionThreadReport?.reportID}`, handleReadNewestAction);

    return () => {
        unreadActionSubscription.remove();
        readNewestActionSubscription.remove();
        unreadActionSubscriptionTransactionThread.remove();
        readNewestActionSubscriptionTransactionThread.remove();
    };
}, [report.reportID, transactionThreadReport?.reportID]);

That would ensure that the unread marker time is updated with the latest action.

What alternative solutions did you explore? (Optional)

[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? 💸

[klajdipaja]

Updated my proposal https://github.com/Expensify/App/issues/50568#issuecomment-2443764382 to refactor the code inside the useEffect according to DRY principle

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[dukenv0307]

@klajdipaja Can you reproduce this issue?