[$500] IOU - User is brought to main chat and duplicate error in report when paying same request twice

[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: 1.4.20-2
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:**Applause - Internal Team
Slack conversation:

Action Performed:

Precondition: User A owes User B money

1. Log in as User A on two devices
2. On the first device, go to IOU report page and disconnect network
3. Pay the amount
4. On the second device, without disconnecting network, pay the request
5. On the first device, reconnect network
6. Click on the IOU preview

Expected Result:

In Step 5, user will remain on the IOU report
In Step 6, the error message will not appear twice

Actual Result:

In Step 5, user is redirected to the main chat after reconnecting network
In Step 6, the error message is duplicated -
"'Unexpected error, please try again later. An unexpected error occurred. Please try again.'

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

Add any screenshot/video evidence

Bug6329854_1704114912943.20231229_091649.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01b20888f83f6a41ed
• Upwork Job ID: 1741889910953496576
• Last Price Increase: 2024-01-22
• Automatic offers:
• akinwale | Reviewer | 28121718
• dukenv0307 | Contributor | 28121719

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[dukenv0307]

Proposal

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

1. User is redirected to the main chat after reconnecting network: Can reproduce this bug on the latest main.

2. The error message is duplicated
   "'Unexpected error, please try again later. An unexpected error occurred. Please try again.'

What is the root cause of that problem?

In FE, we generate an error in failureData and then when pay money request fails, BE returns another error.

App/src/libs/actions/IOU.js

Line 2858 in 320ff54

errors: ErrorUtils.getMicroSecondOnyxError('iou.error.other'),

So we have two errors for this action.

App/src/components/DotIndicatorMessage.tsx

Line 73 in 320ff54

{uniqueMessages.map((message, i) =>

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

In DotIndicatorMessage, we can only display the latest error message by using ErrorUtils.getLatestErrorField function.

App/src/components/DotIndicatorMessage.tsx

Line 73 in 320ff54

{uniqueMessages.map((message, i) =>

What alternative solutions did you explore? (Optional)

If we only want to display one error message for ReportActionItem, we can pass errors as ErrorUtils.getLatestErrorField(this.props.errors) here

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

Line 693 in 320ff54

errors={props.action.errors}

[melvin-bot]

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

[jacobkim9881]

To my endpoint, in step 5, a user(me) remains on the IOU report but the bug is reproducible on step 6.

[melvin-bot]

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

[jacobkim9881]

I wonder two errors should not be shown on report screen? 2 messages explain same but occur by different reasons.

[melvin-bot]

@anmurali, @0xmiroslav 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[0xmiros]

There are 2 bugs stated in OP. No one proposed the correct root cause for the first bug

[jacobkim9881]

So there are 2 errors. One is IOU error and the other unexpected error from backend. These could occur in each case. But it looks same.

[aim-salam]

Proposal

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

What is the root cause of that problem?

When the first device offline and try to pay,
Report preview action try to load locally, but it's failure and this condition happen

App/src/libs/actions/IOU.js

Lines 2886 to 2894 in fe49e4f

	failureData.push({
	onyxMethod: Onyx.METHOD.MERGE,
	key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReport.reportID}`,
	value: {
	[optimisticReportPreviewAction.reportActionID]: {
	created: optimisticReportPreviewAction.created,
	},
	},
	});

So, there is 2 errors in failureData

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

We are able to delete the condition. but it may affect other experience.

So, we manage by show only default item on failureData

App/src/components/DotIndicatorMessage.tsx

Lines 73 to 101 in fe49e4f

	{uniqueMessages.map((message, i) =>
	isReceiptError(message) ? (
	<PressableWithoutFeedback
	accessibilityLabel={Localize.translateLocal('iou.error.saveFileMessage')}
	key={i}
	accessibilityRole={CONST.ACCESSIBILITY_ROLE.LINK}
	onPress={() => {
	fileDownload(message.source, message.filename);
	}}
	>
	<Text
	key={i}
	style={styles.offlineFeedback.text}
	>
	<Text style={[styles.optionAlternateText, styles.textLabelSupporting]}>{Localize.translateLocal('iou.error.receiptFailureMessage')}</Text>
	<Text style={[styles.optionAlternateText, styles.textLabelSupporting, styles.link]}>{Localize.translateLocal('iou.error.saveFileMessage')}</Text>
	<Text style={[styles.optionAlternateText, styles.textLabelSupporting]}>{Localize.translateLocal('iou.error.loseFileMessage')}</Text>
	</Text>
	</PressableWithoutFeedback>
	) : (
	<Text
	// eslint-disable-next-line react/no-array-index-key
	key={i}
	style={[StyleUtils.getDotIndicatorTextStyles(isErrorMessage), textStyles]}
	>
	{message}
	</Text>
	),
	)}

+       const message = uniqueMessages[0]

-             
+                    {isReceiptError(message) ? (
                        <PressableWithoutFeedback
                            accessibilityLabel={Localize.translateLocal('iou.error.saveFileMessage')}
-                         
                            accessibilityRole={CONST.ACCESSIBILITY_ROLE.LINK}
                            onPress={() => {
                                fileDownload(message.source, message.filename);
                            }}
                        >
                            <Text
-                             
                                style={styles.offlineFeedback.text}
                            >
                                <Text style={[StyleUtils.getDotIndicatorTextStyles(isErrorMessage)]}>{Localize.translateLocal('iou.error.receiptFailureMessage')}</Text>
                                <Text style={[StyleUtils.getDotIndicatorTextStyles(isErrorMessage), styles.link]}>{Localize.translateLocal('iou.error.saveFileMessage')}</Text>
                                <Text style={[StyleUtils.getDotIndicatorTextStyles(isErrorMessage)]}>{Localize.translateLocal('iou.error.loseFileMessage')}</Text>
                            </Text>
                        </PressableWithoutFeedback>
                    ) : (
                        <Text
                            // eslint-disable-next-line react/no-array-index-key
-
                            style={[StyleUtils.getDotIndicatorTextStyles(isErrorMessage), textStyles]}
                        >
                            {message}
                        </Text>
                    ),
+                  }
-                )}

Screen.Recording.2024-01-10.at.9.42.09.AM.mov

What alternative solutions did you explore? (Optional)

[jacobkim9881]

I agree that single error shows from same reason. But as I know the other error occurs for unknown reason from backend. And IOU error only in IOU cases explain general issue. IOU error can't be distinguished from the unknown error for now.

[anmurali]

@0xmiroslav bumping you for review of the above comments/ revised proposal.

[dukenv0307]

@0xmiroslav

There are 2 bugs stated in OP. No one proposed the correct root cause for the first bug

I can only reproduce the second bug

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

@anmurali @0xmiroslav 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]

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

[mallenexpensify]

@akinwale reassigning, please take over as C+. If you don't have bandwidth, unassign yourself. Thanks

[akinwale]

Taking over as C+ here.

@anmurali I was able to reproduce, but I got the repeated error messages as in the OP. It looks like the two messages in your test are different however, so that could probably be treated as a separate unrelated issue. What do you think?

[akinwale]

It makes sense to display the latest error message in this instance if there happen to be multiple, so we can move forward with @dukenv0307's proposal.

🎀👀🎀 C+ reviewed.

[melvin-bot]

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

[rlinoz]

I really can't reproduce this issue, neither in staging or dev...

Am I missing something @akinwale?

Screen.Recording.2024-01-24.at.16.23.36.mov

[akinwale]

I really can't reproduce this issue, neither in staging or dev...

Am I missing something @akinwale?

After forcing offline, settle the IOU (pay elsewhere) on the session that was forced offline, and then settle on the other online session before putting the offline session back online.

[aimane-chnaif]

This is edge case and not worth fixing IMO. Similar issues (requiring 2 devices) were closed as won't fix.

[rlinoz]

Oh I missed the part of needing 2 devices, thanks

[rlinoz]

Since this looks like a problem of how we are displaying error messages and not really about requiring 2 devices I think this one is worth fixing.

I am assigning @dukenv0307 then, thanks!

[melvin-bot]

📣 @akinwale 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @dukenv0307 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[dukenv0307]

@akinwale The PR is ready for review.

[akinwale]

Looks like automation missed this one. It was deployed to production on 2024-02-05, so the payment due date is 2024-02-12.

cc @anmurali

[akinwale]

• [@akinwale] The PR that introduced the bug has been identified. Link to the PR:
• [@akinwale] 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:
• [@akinwale] 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:

Not a regression.

• [@akinwale] Determine if we should create a regression test for this bug.
• [@akinwale] 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.

Regression Test Steps

1. Login to Expensify as user A on two devices
2. Send a money request from user B to user A
3. On the first device, go offline
4. Settle the request from user B on the offline device
5. On the second device, settle the request from user B without going offline
6. On the first device, go online
7. Verify that the message is not duplicated when the error message appears on the request.

Do we agree 👍 or 👎?

[rlinoz]

Honestly I think this is not an impactful bug, and needing "two" devices with the same account makes it tricky to test, so I think we can skip adding regression tests.

[anmurali]

Works for me. @akinwale and @dukenv0307 have been paid.