[$500] Chat - Map request position is displayed wrong in workspace

[kbecciv]

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: 1.3.84.0
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
Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1697195763235749

Action Performed:

1. Open the app
2. Open any workspace chat which already has 1 map request pending
3. Send some messages
4. Click on green plus, request money->distance
5. Select any start and finish distance and click Next
6. Select workspace report open in step 2 and complete the process
7. It will open the workspace chat, wait until map is loaded and observe that map request position switches back to previous position instead of displaying as latest message

Expected Result:

Map request should be displayed as latest message in workspace report if no messages are sent after raising the request

Actual Result:

Map request is not displayed as latest message in workspace report even if no messages are sent after raising the request (position issue is rectified on revisit)

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

windows.chrome.map.request.sequence.issue.mp4

mac.chrome.map.request.sequence.issue.mov

Recording.4989.mp4

MacOS: Desktop

mac.desktop.map.request.sequence.issue.mov

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01753ab0226b6e4e39
• Upwork Job ID: 1713204618313187328
• Last Price Increase: 2023-10-14

Issue Owner

Current Issue Owner: @roryabraham

[melvin-bot]

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

[melvin-bot]

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

[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

[melvin-bot]

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

[paultsimura]

This is a BE issue.
The CreateDistanceRequest API response holds an invalid previousReportActionID for the merge operation on the reportActions_{report} key. It should hold a reference to the very last action in the report, but it holds a reference to the last action before the previous money request.

[melvin-bot]

Current assignee @situchan is eligible for the Internal assigner, not assigning anyone new.

[melvin-bot]

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

[jliexpensify]

Hi @roryabraham - could you confirm this is an Internal issue based off this? Thanks in advance!

[Victor-Nyagudi]

I also uncovered signs of a back end issue.

If you inspect the reportActions prop passed to ReportScreen, you'll notice that the report preview is initially at the right place (first in the array) but jumps to the position after the most recently sent comment(s) .

Furthermore, the created property of the report preview's object is set to a time before the latest comment, possibly referencing the previous distance request instead of the previous message.

Take a look.

Video of report preview jumping to different spot

expensify-report-screen-props.mov

It's worth noting that reloading the page or switching to a different tab and back renders the map position in the right place, and this is most likely (as far as I can tell) due to OpenReport API call happening and we receive the correct report preview timestamp.

I suspect createDistanceRequest API call is culprit here because if you repeat the reproduction steps offline, the distance request is rendered at the correct position, even if you do go back online. Additionally, from what I could gather when looking at Onyx, the correct created timestamp is stored in local storage for the new distance request.

App/src/libs/actions/IOU.js

Line 617 in 75a2cc9

function createDistanceRequest(report, participant, comment, created, transactionID, category, tag, amount, currency, merchant, billable) {

Take a look at what happens offline.

Video of trying to reproduce the bug while offline

expensify-testing-mapbox-offline.mov

Speaking of the created timestamp, it's worth noting that the report preview's timestamp is updated to the most recent distance request, however, the latest distance request is manually merged in getMoneyRequestInformation instead of using Onyx.

Take a look at the comment before this happens.

App/src/libs/actions/IOU.js

Lines 509 to 520 in 75a2cc9

	// If there is an existing transaction (which is the case for distance requests), then the data from the existing transaction
	// needs to be manually merged into the optimistic transaction. This is because buildOnyxDataForMoneyRequest() uses `Onyx.set()` for the transaction
	// data. This is a big can of worms to change it to `Onyx.merge()` as explored in https://expensify.slack.com/archives/C05DWUDHVK7/p1692139468252109.
	// I want to clean this up at some point, but it's possible this will live in the code for a while so I've created https://github.com/Expensify/App/issues/25417
	// to remind me to do this.
	const existingTransaction = existingTransactionID && TransactionUtils.getTransaction(existingTransactionID);
	if (existingTransaction) {
	optimisticTransaction = {
	...optimisticTransaction,
	...existingTransaction,
	};
	}

I tried setting the created property of optimisticTransaction back to the optimistic version instead of the existing transaction out of curiosity, and the latest distance request did render after the most recent message as intended but as a standalone money request instead of as part of the report preview.

optimisticTransaction = {
    ...optimisticTransaction,
    ..existingTransaction,
    created: optimisticTransaction.created // Added 
};

In conclusion, I think it would be good to not only check on what's going on in the back end but to also see if what @tgolen was working on in this issue could make a difference.

[roryabraham]

Working on reproducing this but currently wrestling with my dev environment after updating to Sonoma

[melvin-bot]

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

[Christinadobrzyn]

hi there @roryabraham or @situchan - could you confirm if this is related or will be fixed with this? #30147

[melvin-bot]

@jliexpensify @roryabraham @situchan this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[jliexpensify]

Being worked on!

[jliexpensify]

Not overdue! @roryabraham should we make this a Weekly instead?

[melvin-bot]

@jliexpensify @roryabraham @situchan this issue is now 4 weeks old and preventing us from maintaining WAQ. This should now be your highest priority. Please post below what your plan is to get a PR in review ASAP. Thanks!

[jliexpensify]

Not overdue

[melvin-bot]

@jliexpensify, @roryabraham, @situchan 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[jliexpensify]

Not overdue

[situchan]

Same

[roryabraham]

I must apologize for the lack of communication on this issue. The PR I was working on to fix this was deployed to production about a month ago, but the issue persists. Going to bring this to slack to discuss

[roryabraham]

Actually, found a simple solution to fix this, so opened a PR: https://github.com/Expensify/Web-Expensify/pull/40008

[jliexpensify]

Going to open an Upworks job to sort out Reporting payment for @dhanashree-sawant

https://www.upwork.com/jobs/~011e1da39bdff2560d

[dhanashree-sawant]

Thanks @jliexpensify, I have accepted the offer.

[jliexpensify]

Paid and job closed!

[jliexpensify]

Deployed to prod as of last week so going to close this out. Rory - feel ree to re-open if you think we need to keep tabs on this GH.