[hold for payment 2023-08-30] IOS - Onfido screen present multiple time when app pushed to Background

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Go to setting
2. Go to any workspace
3. Setup Bank account
4. On last step click Save & continue
5. It will present Onfido | Real identity screen
6. Push app in background
7. Again open the app

Expected Result:

Onfido | Real identity screen should be presented only once

Actual Result:

Onfido | Real identity screen presented multiple time when app goes in background and again opened

Workaround:

Unknown

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.42-26
Reproducible in staging?: n/a
Reproducible in production?: n/a
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

Image.from.iOS.1.MP4

Expensify/Expensify Issue URL:
Issue reported by: @DinalJivani
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1689948418893939

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~013b5a7fa2bf8b3fcc
• Upwork Job ID: 1684610566252363776
• Last Price Increase: 2023-07-27
• Automatic offers:
• ginsuma | Contributor | 25908892
• DinalJivani | Reporter | 25908893

[melvin-bot]

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

[joekaufmanexpensify]

I can reproduce this but I think we're using an Onfido SDK for this, so not sure this is something we can fix. Checking.

[joekaufmanexpensify]

Bumped in Slack

[joekaufmanexpensify]

Okay, this is in Onfido's SDK. Which is open source. So we'll leave this open for now, and see if anyone wants to propose an upstream fix. If we don't get any proposals after a bit, we'll close, since not in our codebase.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[joekaufmanexpensify]

Awaiting proposals

[jaskarwal]

Hi @joekaufmanexpensify , I am trying to login but it seems textfield issue while entering code. Cloned from main branch.

[melvin-bot]

📣 @jaskarwal! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[jaskarwal]

Hi @joekaufmanexpensify ,

I am trying to resolve the issue but I am not able to attach to debugger on iOS simulator. Is there any other branch to check ?

[joekaufmanexpensify]

@allroundexperts any thoughts on the above question?

[ginsuma]

Proposal

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

IOS - Onfido screen present multiple time when app pushed to Background

What is the root cause of that problem?

In ReimbursementAccountPage, we are using IS_LOADING_REPORT_DATA as props.isLoadingReportData to detect isLoading here and show components depend on it.

When the app returns from the background, ReconnectApp is sent, and IS_LOADING_REPORT_DATA state is reset. This makes Onfido mounted again and shows one more "Verify your identity" screen.

App/src/libs/actions/App.js

Lines 161 to 168 in efb4ac6

	apiMethod(isReconnecting ? 'ReconnectApp' : 'OpenApp', params, {
	optimisticData: [
	{
	onyxMethod: Onyx.METHOD.MERGE,
	key: ONYXKEYS.IS_LOADING_REPORT_DATA,
	value: true,
	},
	],

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

Creating a ref called isLoadedReportDataRef and using it instead of props.isLoadingReportData to detect isLoading.
This will help us avoid rerendering Onfido.
Code ref:

constructor(props) {
  ...
  this.isLoadedReportDataRef = React.createRef();
  this.isLoadedReportDataRef.current = !props.isLoadingReportData;
}
componentDidUpdate(prevProps) {
  if (prevProps.isLoadingReportData && !this.props.isLoadingReportData) {
      this.isLoadedReportDataRef.current = true;
  }
}
...
const isLoading = !this.isLoadedReportDataRef.current || ...

Result

Untitled.mp4

[joekaufmanexpensify]

Proposal pending review

[allroundexperts]

Hi @jaskarwal!
We usually use Flipper for debugging the native apps.

[allroundexperts]

@ginsuma's proposal has the correct root cause and seems to be working well. Let's go with them!

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

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

Offer link
Upwork job

[ginsuma]

Thanks for accepting my proposal. The PR #24079 is ready for review.
cc: @allroundexperts @NikkiWines

[joekaufmanexpensify]

Looks like this is pending review from @NikkiWines

[allroundexperts]

Update: We decided to disregard our changes on native Android because of an issue in the Onfido Android sdk. More context in the discussion proceeding this #24079 (comment)

[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 @ginsuma got assigned: 2023-08-02 17:17:07 Z
• when the PR got merged: 2023-08-18 15:56:02 UTC
• days elapsed: 11

On to the next one 🚀

[joekaufmanexpensify]

Payment is due on 2023-08-30

[joekaufmanexpensify]

After factoring in the 3 day internal delay to merge this issue, there is no penalty here. So we need to make the following payments:

• @allroundexperts - $1,000 for C+ review. Paid via NewDot.
• @ginsuma - $1,000 for PR. Paid via Upwork.
• @DinalJivani - $250 for reporting the bug. Paid via Upwork.

[allroundexperts]

@joekaufmanexpensify I think the penalty should be revisited here. The reason is that the PR was actually reviewed quickly. However, there was a bug in the Onfido's android SDK due to which we had a lot of back and forth conversations. In the end (after consultation with the internal engineer), we decided not to apply the fix to android.

[joekaufmanexpensify]

Got it. I will review further!

[ginsuma]

@joekaufmanexpensify For further clarification, the internal engineer approved but forgot to merge #24079 (comment). It took us about three days.

[joekaufmanexpensify]

Ah, thanks for flagging that. Yeah, I see @NikkiWines approved on 2023-08-14, but did not merge until 2023-08-17. If we remove those three days then the time here was 8 business days, so no penalty would be applied.

[joekaufmanexpensify]

Just edited the payment message to reflect that there will no penalty here.

[joekaufmanexpensify]

@ginsuma $1,000 sent and contract ended!

[joekaufmanexpensify]

@DinalJivani $250 sent and contract ended!

[joekaufmanexpensify]

@allroundexperts could you please request $1,000 via NewDot and confirm here once complete?

[allroundexperts]

Done!

[joekaufmanexpensify]

Great, thanks for confirming!

[joekaufmanexpensify]

Going to close this one out for now. If your request is not paid within 7 days, please let me know and I'll look into it.

[joekaufmanexpensify]

As an FYI for the individual paying, payment summary message is here.

[JmillsExpensify]

Reviewed the details for @allroundexperts. $1,000 payment approved based on BZ summary.

[kavimuru]

@joekaufmanexpensify @allroundexperts Tester was able to reproduce it today. Should we need a new ticket?

Gbqe4049.1.mp4

[joekaufmanexpensify]

@kavimuru If this is broken again now, wouldn't it be a regression? If so, I'd think we'd fix it on the issue that introduced the regression.