[HOLD for payment 2023-09-29] [$1000] Time of User A doesn't appear for User B in IOU or Task room.

[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. Open 2 Acounts User A and User B
2. Login as User A click The FAB and Request money from User B.
3. Click in the IOU message to open it in a thread
4. Go to User B and Click on the IOU message to open it in a thread
5. To make sure, start sending messages between the two users ( Optional )

Expected Result:

User B should be able to see the time of User A ( who request the money ) inside the IOU room.

Actual Result:

User B Can't see the time of User A ( Who request the money ) inside IOU room
NOTICE: this happens also in the task room

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.50.3
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Screencast.from.01-08-23.18_50_26.webm

Recording.5765.mp4

Expensify/Expensify Issue URL:
Issue reported by: @Ahmed-Abdella
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1690906316722259

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e1a50a5c3e2c06ad
• Upwork Job ID: 1688633263346528256
• Last Price Increase: 2023-09-01
• Automatic offers:
• huzaifa-99 | Contributor | 26515322
• Ahmed-Abdella | Reporter | 26515324

[melvin-bot]

Triggered auto assignment to @conorpendergrast (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[conorpendergrast]

Reproduced for Tasks and Requests

[melvin-bot]

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

[melvin-bot]

Current assignee @conorpendergrast is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[pradeepmdk]

Proposal

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

The time of User A doesn't appear for User B in IOU or Task room

What is the root cause of that problem?

in the command=OpenReport API getting the wrong data

in the second image in participantAccountIDs [email protected] is missing instead its giving current logged user data only there.

when i am checking this openreport API when ownerEmail: "__fake__" data is coming correctly

because we are checking this participantAccountIDs should not be an currentUserPersonalDetails.accountID so that we can show another user time here

App/src/pages/home/report/ReportActionCompose.js

Lines 977 to 981 in 9811ed5

	const reportParticipants = _.without(lodashGet(this.props.report, 'participantAccountIDs', []), this.props.currentUserPersonalDetails.accountID);
	const participantsWithoutExpensifyAccountIDs = _.difference(reportParticipants, CONST.EXPENSIFY_ACCOUNT_IDS);
	const reportRecipient = this.props.personalDetails[participantsWithoutExpensifyAccountIDs[0]];
	const shouldShowReportRecipientLocalTime =
	ReportUtils.canShowReportRecipientLocalTime(this.props.personalDetails, this.props.report, this.props.currentUserPersonalDetails.accountID) && !this.props.isComposerFullSize;

App/src/pages/home/report/ReportActionCompose.js

Line 1020 in 9811ed5

{shouldShowReportRecipientLocalTime && hasReportRecipient && <ParticipantLocalTime participant={reportRecipient} />}

same issue for Task

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

need to fix on the backend side

[GItGudRatio]

Proposal

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

Time of User A doesn't appear for User B in IOU or Task room.

What is the root cause of that problem?

To check if the report time should be shown, the ReportUtils.canShowReportRecipientLocalTime function is used. The main logic that is responsible for this issue is:

App/src/libs/ReportUtils.js

Lines 813 to 817 in 9811ed5

	function canShowReportRecipientLocalTime(personalDetails, report, accountID) {
	const reportParticipants = _.without(lodashGet(report, 'participantAccountIDs', []), accountID);
	const participantsWithoutExpensifyAccountIDs = _.difference(reportParticipants, CONST.EXPENSIFY_ACCOUNT_IDS);
	const hasMultipleParticipants = participantsWithoutExpensifyAccountIDs.length > 1;
	const reportRecipient = personalDetails[participantsWithoutExpensifyAccountIDs[0]];

When a new task/IOU is created, the participantAccountIDs only includes the ID of the person from whom the money is requested. When that user visits the report, their account ID is removed, thus not showing the time at all.

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

In the case of IOU request money and Tasks, their can only be two users. To accommodate for the sender of the IOU/task, we need to add the ownerAccountID to the array reportParticipants. Thus, we simply need to add a check if ReportUtils.isIOUReport(report) || isTaskReport(report)) is true, then add the report.ownerAccountID to the reportParticipants array.

The same fix needs to be applied here as well:

App/src/pages/home/report/ReportActionCompose.js

Lines 976 to 979 in 9811ed5

	render() {
	const reportParticipants = _.without(lodashGet(this.props.report, 'participantAccountIDs', []), this.props.currentUserPersonalDetails.accountID);
	const participantsWithoutExpensifyAccountIDs = _.difference(reportParticipants, CONST.EXPENSIFY_ACCOUNT_IDS);
	const reportRecipient = this.props.personalDetails[participantsWithoutExpensifyAccountIDs[0]];

What alternative solutions did you explore? (Optional)

N/A

[ArekChr]

This issue is duplicated #23836

[conorpendergrast]

Oh nice, great catch. Thanks @ArekChr. Closing as a duplicate of #23836

[Ahmed-Abdella]

@ArekChr @conorpendergrast I don't think s, they are two different cases. This issue is that the Sender locat time doesn't appear for the Recipient. The other issue (#23836) is the other way around, the Recipient Local Time does not appear for the sender.
And there is ahuge difference between the two cases, as in the other issue the Recipient Local Time does appear for the sender after refresh the page, not like this issue, it never appear.

Screencast.from.08-08-23.13.29.54.webm

Compare this video with the first one above you can see the difference.

[huzaifa-99]

Proposal

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

We want the local time to correctly show local time on both the sender and receiver side in threads

What is the root cause of that problem?

We handle participant ids differently in the case of task and money request reports. For these types of reports, we only include the assignee/requestee id in the participant ids, the assigner/requester id gets stored in the onwerAccountID key of the same report object.
For normal threads, we don't set the participant ids until the receiver responds and we fetch the report data again.

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

We can update the way we calculate report participant's here and here to include cases for task/money/simple-thread report types.
For task and money request reports, the report.participantAccountIDs array would have the id of the assignee/requestee on both the assigner/requester and assignee/requestee side, we already filter the current user account id, so we can add the concat the report owner id with participants ids and then filter, something like this

// pseudocode
const initialIDs = lodashGet(report, 'participantAccountIDs', []);
const updatedIDs = (isTaskReport(report) || isMoneyRequestReport(report)) ? _.union(initialIDs, [report.ownerAccountID]) : initialIDs
const reportParticipants = _.without(updatedIDs, currentUserAccountID)

and in the case of simple threads, we can get the parent report participants (we could also update the participant list upon thread creation)

// pseudocode
report = isThread(report) && !(isTaskReport(report) || isMoneyRequestReport(report)) ? lodashGet(allReports, [`${ONYXKEYS.COLLECTION.REPORT}${report.parentReportID}`]) : report

and abstract all of this logic into ReportUtils.

// pseudocode
function getReportParticipantIDs(report, currentUserAccountID){
    const updatedReport = isThread(report) && !(isTaskReport(report) || isMoneyRequestReport(report)) ? lodashGet(allReports, [`${ONYXKEYS.COLLECTION.REPORT}${report.parentReportID}`]) : report
    const initialIDs = lodashGet(updatedReport, 'participantAccountIDs', []);
    const updatedIDs = (isTaskReport(updatedReport) || isMoneyRequestReport(updatedReport)) ? _.union(initialIDs, [updatedReport.ownerAccountID]) : initialIDs
    return _.without(updatedIDs, currentUserAccountID);
}

I am not sure if we want to show the local time in case of workspace threads, if we don't we can add !isWorkspaceThread(report) in the updatedReport condition

What alternative solutions did you explore? (Optional)

We could also update how we set the participant ids for money/task and simple threads, but this would require some decent refactoring.

[mountiny]

@huzaifa-99 I think the problem with these is that in any chat except of DM there can be more participants than just 2. Maybe just IOU could be two. In Task report, you can invite people in threads you can invite people, workspace and expenses can have bunch of people too.

[huzaifa-99]

Thank you for having a look. The canShowReportRecipientLocalTime handles multiple participants case and if there are multiple it won't show local time which seems correct. I think the issue here seems to be that if there are 2 users, User A will see the local time of User B, but User B won't be able to see local time of User A (because of the difference in the way we store iou/task manager/owner and participants ids).

cc: @mountiny

[mountiny]

@conorpendergrast i think this issue is not exactly duplicate of the other one (that is linked to the problem with known and unknown users - secure login)

@ArekChr do you agree? I think we should reopen this

[ArekChr]

@mountiny I looked it up, and I think so too. Let's reopen this issue.

[ArekChr]

I am not sure if we want to show the local time in case of workspace threads, if we don't we can add

@huzaifa-99 Fixing this issue shouldn't change behavior of other features.

[ArekChr]

@huzaifa-99 Why using isMoneyRequestReport? Are we going to include expense reports, not only IOU and tasks?

[huzaifa-99]

@huzaifa-99 Fixing this issue shouldn't change behavior of other features.

Sure, we can then follow the existing behaviour for workspace threads i.e not show local time there.

@huzaifa-99 Why using isMoneyRequestReport? Are we going to include expense reports, not only IOU and tasks?

I believe we should because that too can have 2 users.

In general, if there are 2 or more report participants then we are already hiding the local time here in that case so I think it's fine to include expense reports. However if we want to only include IOU reports then we can use isIOUReport instead of isMoneyRequestReport.

Thoughts @ArekChr?

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

Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:

• when @huzaifa-99 got assigned: 2023-09-05 21:19:37 Z
• when the PR got merged: 2023-09-19 23:23:50 UTC
• days elapsed: 10

On to the next one 🚀

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

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

• Show local time in money request and 1:1 chat threads #26845

If no regressions arise, payment will be issued on 2023-09-29. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @ArekChr does not require payment (Contractor)
• @huzaifa-99 requires payment offer (Contributor)
• @Ahmed-Abdella requires payment offer (Reporter)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

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

• [@ArekChr] The PR that introduced the bug has been identified. Link to the PR:
• [@ArekChr] 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:
• [@ArekChr] A discussion in #expensify-bugs 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:
• [@ArekChr] Determine if we should create a regression test for this bug.
• [@ArekChr] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@johncschuster] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[huzaifa-99]

@johncschuster bumping for payment process.

[ArekChr]

• Link to the PR: No bug identified.
• Link to comment: n/a
• Link to discussion: n/a
• Determine if we should create a regression test for this bug: I don’t think we need to add a regression test here.

[johncschuster]

Thanks for the bump, @huzaifa-99! It looks like the offer is still pending in Upwork. Can you accept that?

[johncschuster]

I've issued payment for @Ahmed-Abdella. I'll get you as soon as you ping me, @huzaifa-99!

[huzaifa-99]

Offer accepted @johncschuster.

[melvin-bot]

@johncschuster, @ArekChr, @roryabraham, @huzaifa-99 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[huzaifa-99]

Bump @johncschuster

[johncschuster]

Thanks for the bump, @huzaifa-99! I've just issued payment