Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade Onfido native sdk to 7.4.0 + fix allowed documents #14802

Merged
merged 3 commits into from
Mar 2, 2023

Conversation

nkuoch
Copy link
Contributor

@nkuoch nkuoch commented Feb 3, 2023

Details

Fixed Issues

Part of https://github.com/Expensify/Expensify/issues/233363

Tests

Test on all platforms adding a verifying business bank account to a workspace on newDot, using random data.
Onfido flow should:

  • Not show any country selector
  • Allow you to select between passport or driver license

Make sure you can proceed until the bank account has been created (no matter the state)

Offline tests

N/A

QA Steps

Test on all platforms adding a verifying business bank account to a workspace on newDot, using random data.
Onfido flow should:

  • Not show any country selector
  • Allow you to select between passport or driver license

Make sure you can proceed until the bank account has been created (no matter the state)

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android / native
    • Android / Chrome
    • iOS / native
    • iOS / Safari
    • MacOS / Chrome / Safari
    • MacOS / Desktop
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
      • If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
    • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
    • I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
    • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
    • I verified the JSDocs style guidelines (in STYLE.md) were followed
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
  • I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
  • I verified that if a function's arguments changed that all usages have also been updated correctly
  • If a new component is created I verified that:
    • A similar component doesn't exist in the codebase
    • All props are defined accurately and each prop has a /** comment above it */
    • The file is named correctly
    • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
    • The only data being stored in the state is data necessary for rendering and nothing else
    • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
    • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
    • All JSX used for rendering exists in the render method
    • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
  • If any new file was added I verified that:
    • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
  • I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

Web image image image

IMG_6305
IMG_6306
IMG_6307
IMG_6308
IMG_6309
IMG_6310
IMG_6311

image image
Mobile Web - Chrome
Mobile Web - Safari
Screen.Recording.2023-02-20.at.3.20.21.PM.mov
Desktop

Screenshot 2023-02-20 at 3 39 52 PM

iOS
Screen.Recording.2023-02-20.at.3.37.58.PM.mov
Android
Screen.Recording.2023-02-20.at.3.12.32.PM.mov

@nkuoch nkuoch self-assigned this Feb 3, 2023
@nkuoch nkuoch force-pushed the nat-onfidonocross branch 4 times, most recently from ac71a05 to a80059d Compare February 18, 2023 08:16
@nkuoch nkuoch changed the title Dont force Onfido photo on mobile anymore + fix allowed documents Upgrade Onfido native sdk to 7.4.0 + fix allowed documents Feb 18, 2023
@nkuoch nkuoch requested a review from MariaHCD February 18, 2023 08:36
@nkuoch nkuoch marked this pull request as ready for review February 18, 2023 08:52
@nkuoch nkuoch requested a review from a team as a code owner February 18, 2023 08:52
@melvin-bot melvin-bot bot removed the request for review from a team February 18, 2023 08:52
@MelvinBot
Copy link

@Gonals Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

@melvin-bot melvin-bot bot requested a review from Gonals February 18, 2023 08:53
@MariaHCD
Copy link
Contributor

Adding screenshots for all platforms in the OP since @nkuoch is OOO.

I'm running into an error on Android mWeb, I always seem to get this error even though I'm on the latest version of Chrome (110.0.5481.65):

Copy link
Contributor

@MariaHCD MariaHCD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The build fails on Android:

BUILD FAILED in 5m 6s

error Failed to install the app. Make sure you have the Android development environment set up: https://reactnative.dev/docs/environment-setup.
Error: Command failed: ./gradlew app:installDebug -PreactNativeDevServerPort=8081
/Users/mariadcosta/Expensidev/App/android/app/src/debug/AndroidManifest.xml:11:7-34 Error:
        Attribute application@supportsRtl value=(false) from AndroidManifest.xml:11:7-34
        is also present at [com.onfido.sdk.capture:onfido-capture-sdk-core:15.4.0] AndroidManifest.xml:28:18-44 value=(true).
        Suggestion: add 'tools:replace="android:supportsRtl"' to <application> element at AndroidManifest.xml:7:5-13:19 to override.

FAILURE: Build completed with 2 failures.

1: Task failed with an exception.
-----------
* What went wrong:
Execution failed for task ':app:processDebugMainManifest'.
> Manifest merger failed : Attribute application@supportsRtl value=(false) from AndroidManifest.xml:11:7-34
        is also present at [com.onfido.sdk.capture:onfido-capture-sdk-core:15.4.0] AndroidManifest.xml:28:18-44 value=(true).
        Suggestion: add 'tools:replace="android:supportsRtl"' to <application> element at AndroidManifest.xml:7:5-13:19 to override.

Looks like a conflict in the manifest.

Apply this diff worked but I'm not 100% if that's the correct approach:

diff --git a/android/app/src/main/AndroidManifest.xml b/android/app/src/main/AndroidManifest.xml
index 71c3dc6c9a..208076beeb 100644
--- a/android/app/src/main/AndroidManifest.xml
+++ b/android/app/src/main/AndroidManifest.xml
@@ -1,4 +1,5 @@
 <manifest xmlns:android="http://schemas.android.com/apk/res/android"
+  xmlns:tools="http://schemas.android.com/tools"
   package="com.expensify.chat">
 
     <uses-permission android:name="android.permission.INTERNET" />
@@ -15,7 +16,8 @@
       android:roundIcon="@mipmap/ic_launcher_round"
       android:allowBackup="false"
       android:resizeableActivity="false"
-      android:theme="@style/AppTheme">
+      android:theme="@style/AppTheme"
+      tools:replace="android:supportsRtl">
 
       <activity
         android:name=".MainActivity"

@MariaHCD
Copy link
Contributor

MariaHCD commented Feb 20, 2023

Also, Onfido on native Android does not allow selecting the document type, is that expected?

Screen.Recording.2023-02-20.at.3.12.32.PM.mov

Same for iOS native:

Screen.Recording.2023-02-20.at.3.37.58.PM.mov

@MariaHCD
Copy link
Contributor

Got an error when running pod install:

[!] CocoaPods could not find compatible versions for pod "Onfido":
  In snapshot (Podfile.lock):
    Onfido (= 27.4.0)

  In Podfile:
    onfido-react-native-sdk (from `../node_modules/@onfido/react-native-sdk`) was resolved to 7.4.0, which depends on
      Onfido (= 27.4.0)

None of your spec sources contain a spec satisfying the dependencies: `Onfido (= 27.4.0), Onfido (= 27.4.0)`.

You have either:
 * out-of-date source repos which you can update with `pod repo update` or with `pod install --repo-update`.
 * mistyped the name or version.
 * not added the source repo that hosts the Podspec to your Podfile.

Running pod update repo and then pod install fixed it

@nkuoch
Copy link
Contributor Author

nkuoch commented Feb 27, 2023

Also, Onfido on native Android does not allow selecting the document type, is that expected?

Yes, native sdk doesn't allow to specify a list. You can upload any id, and Onfido will just guess what it is.

Copy link
Contributor

@MariaHCD MariaHCD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The podfile check seems to be failing:

💥 lottie-react-native (5.1.4) not found in Podfile.lock. Did you forget to run `npx pod-install`?

@@ -1,4 +1,5 @@
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cristipaval Would you mind checking these changes in this file? I was running into an error when building on android and these changes fixed it: #14802 (review)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, this is fine. The error occurs because a library added as dependency to the App has android:supportsRtl="true" in its AndroidManifext.xml file. Or maybe the library doesn't have that property set at all, which means that the compiler sets the default value which is true. This is in conflict with our App settings (android:supportsRtl="false") and the line added (tools:replace="android:supportsRtl") tells the compiler to set our App's value to that property to all libraries when there is a conflict. This is fine, because the setting disables support for languages which are written from right to left and we don't support those languages yet.

@nkuoch
Copy link
Contributor Author

nkuoch commented Feb 28, 2023

Updated

Copy link
Contributor

@MariaHCD MariaHCD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! @Gonals Would you be able to fill out the reviewer checklist? The screenshots in the OP are mine so it would be good to get this tested by someone else.

@Gonals
Copy link
Contributor

Gonals commented Mar 2, 2023

Reviewer Checklist

  • I have verified the author checklist is complete (all boxes are checked off).
  • I verified the correct issue is linked in the ### Fixed Issues section above
  • I verified testing steps are clear and they cover the changes made in this PR
    • I verified the steps for local testing are in the Tests section
    • I verified the steps for Staging and/or Production testing are in the QA steps section
    • I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I checked that screenshots or videos are included for tests on all platforms
  • I included screenshots or videos for tests on all platforms
  • I verified tests pass on all platforms & I tested again on:
    • Android / native
    • Android / Chrome
    • iOS / native
    • iOS / Safari
    • MacOS / Chrome / Safari
    • MacOS / Desktop
  • If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
  • I verified proper code patterns were followed (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
    • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
    • I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
    • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
    • I verified the JSDocs style guidelines (in STYLE.md) were followed
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I verified that this PR follows the guidelines as stated in the Review Guidelines
  • I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
  • I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
  • I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
  • If a new component is created I verified that:
    • A similar component doesn't exist in the codebase
    • All props are defined accurately and each prop has a /** comment above it */
    • The file is named correctly
    • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
    • The only data being stored in the state is data necessary for rendering and nothing else
    • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
    • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
    • All JSX used for rendering exists in the render method
    • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
  • If any new file was added I verified that:
    • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
  • I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web
Mobile Web - Chrome
Mobile Web - Safari
Desktop
iOS
Android

@Gonals
Copy link
Contributor

Gonals commented Mar 2, 2023

Seems to work for me!

@nkuoch nkuoch merged commit b6ebac3 into main Mar 2, 2023
@nkuoch nkuoch deleted the nat-onfidonocross branch March 2, 2023 16:22
@OSBotify
Copy link
Contributor

OSBotify commented Mar 2, 2023

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2023

Performance Comparison Report 📊

Significant Changes To Duration

Name Duration
Open Search Page TTI 605.224 ms → 665.755 ms (+60.532 ms, +10.0%) 🔴
Show details
Name Duration
Open Search Page TTI Baseline
Mean: 605.224 ms
Stdev: 19.703 ms (3.3%)
Runs: 573.9649660009891 575.8675539996475 576.3267819993198 578.9891360010952 579.4921070002019 582.9047439992428 589.5991619993001 591.4761560000479 591.6734209991992 591.8563240002841 593.3968509994447 595.0957849994302 595.82466599904 601.9520669989288 602.4517419990152 604.8488779999316 605.0746250003576 606.3193360008299 607.7306319996715 608.3891199994832 609.3279219996184 609.8380939997733 612.3600260000676 613.0005689989775 615.8614499997348 619.6397710014135 620.6249599996954 622.2909760009497 626.0897220000625 634.2141520008445 641.9457600004971 644.1790360007435 649.781128000468

Current
Mean: 665.755 ms
Stdev: 14.743 ms (2.2%)
Runs: 627.48445700109 628.4418130014092 650.7245289999992 651.3721110001206 652.1610920000821 655.5368650015444 656.8067630007863 657.7135419994593 659.4309899993241 662.7293710000813 663.4506020005792 663.5016280002892 664.6779789999127 665.2012130003422 666.015829000622 666.2357590012252 669.1202400010079 669.5049649998546 669.6263839993626 670.0902509987354 672.1335860006511 673.0874020010233 674.9532069992274 677.9975180011243 678.153401998803 678.2456870004535 680.6286209989339 680.8068850003183 687.9314379990101 698.8992519993335

Meaningless Changes To Duration

Show entries
Name Duration
App start runJsBundle 197.733 ms → 201.000 ms (+3.267 ms, +1.7%)
App start TTI 721.060 ms → 722.521 ms (+1.461 ms, ±0.0%)
App start nativeLaunch 19.438 ms → 19.800 ms (+0.363 ms, +1.9%)
App start regularAppStart 0.014 ms → 0.014 ms (-0.000 ms, -0.8%)
Show details
Name Duration
App start runJsBundle Baseline
Mean: 197.733 ms
Stdev: 20.474 ms (10.4%)
Runs: 166 168 169 177 181 182 184 185 185 186 188 189 192 195 195 196 198 198 199 199 200 200 206 208 209 215 235 236 245 246

Current
Mean: 201.000 ms
Stdev: 18.305 ms (9.1%)
Runs: 171 174 180 182 183 186 186 187 187 191 192 193 193 194 194 195 195 197 201 203 203 204 205 214 214 217 224 225 228 232 239 243
App start TTI Baseline
Mean: 721.060 ms
Stdev: 38.223 ms (5.3%)
Runs: 660.4001529999077 672.8983720000833 673.0769260004163 674.6192669998854 675.1413269992918 677.6042739991099 692.7049509994686 693.8434960003942 695.2602869998664 696.0010579992086 698.316081000492 701.1296829991043 701.6914239991456 705.9254669994116 706.6622739993036 709.5617609992623 713.2879319991916 717.84259499982 724.9103480000049 727.6361950002611 727.9077300000936 731.2422880008817 745.2955189999193 747.6068099997938 759.250942999497 762.1522250007838 763.1955660004169 767.5545879993588 776.1001350004226 778.9825049992651 779.2717010006309 816.8316319994628

Current
Mean: 722.521 ms
Stdev: 26.761 ms (3.7%)
Runs: 676.0509350001812 683.3641049992293 686.600476000458 688.9858919996768 692.5321759991348 694.0647730007768 697.4180529993027 697.836595999077 702.9470419995487 705.6905770003796 707.1368189994246 712.3260180000216 714.392595000565 716.4659839998931 717.0856679994613 718.4319930002093 722.7318079993129 725.6554000005126 725.8857070002705 728.0095210000873 728.6459619998932 729.3407630007714 730.4992430005223 742.945320000872 744.5684789996594 751.4347869995981 753.3030529990792 753.7899670004845 756.1411419995129 763.9197330009192 765.5280080009252 786.9274310003966
App start nativeLaunch Baseline
Mean: 19.438 ms
Stdev: 1.478 ms (7.6%)
Runs: 17 17 17 18 18 18 18 18 18 19 19 19 19 19 19 19 19 19 20 20 20 20 20 20 21 21 21 21 22 22 22 22

Current
Mean: 19.800 ms
Stdev: 1.759 ms (8.9%)
Runs: 17 17 18 18 18 18 18 18 19 19 19 19 19 19 20 20 20 20 20 20 20 21 21 21 21 22 22 23 23 24
App start regularAppStart Baseline
Mean: 0.014 ms
Stdev: 0.001 ms (8.5%)
Runs: 0.012329000979661942 0.01269499957561493 0.01269499957561493 0.01326499879360199 0.013345999643206596 0.013346999883651733 0.013387000188231468 0.013427000492811203 0.013468999415636063 0.013469001278281212 0.013549000024795532 0.013671999797224998 0.013672001659870148 0.01371300034224987 0.013754000887274742 0.0138349998742342 0.0139979999512434 0.014079000800848007 0.014159999787807465 0.014444999396800995 0.014607999473810196 0.01489199884235859 0.014973999932408333 0.015015000477433205 0.015300000086426735 0.01534000039100647 0.015421001240611076 0.015544001013040543 0.016112999990582466 0.0168869998306036 0.017822999507188797

Current
Mean: 0.014 ms
Stdev: 0.001 ms (5.0%)
Runs: 0.012736000120639801 0.013345999643206596 0.013346999883651733 0.013346999883651733 0.013590000569820404 0.013590998947620392 0.013590998947620392 0.013591000810265541 0.013630999252200127 0.013671999797224998 0.013672001659870148 0.013711998239159584 0.013712000101804733 0.013916000723838806 0.013916000723838806 0.01407800056040287 0.014322999864816666 0.014364000409841537 0.01440500095486641 0.01448499970138073 0.014526000246405602 0.014527000486850739 0.01476999931037426 0.014810999855399132 0.014810999855399132 0.014812000095844269 0.014932999387383461 0.015137000009417534 0.015543999150395393 0.015786999836564064

@github-actions github-actions bot added the DeployBlockerCash This issue or pull request should block deployment label Mar 2, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2023

@Expensify/mobile-deployers 📣 Please look into this performance regression as it's a deploy blocker.

@OSBotify
Copy link
Contributor

OSBotify commented Mar 2, 2023

🚀 Deployed to staging by https://github.com/nkuoch in version: 1.2.78-0 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@roryabraham
Copy link
Contributor

roryabraham commented Mar 6, 2023

This performance regression appears to be consistent with what we were seeing on all PRs since last week, removing the DeployBlockerCash label

https://expensify.slack.com/archives/C07J32337/p1677708239351139?thread_ts=1677703372.513129&cid=C07J32337

@roryabraham roryabraham removed the DeployBlockerCash This issue or pull request should block deployment label Mar 6, 2023
@OSBotify
Copy link
Contributor

OSBotify commented Mar 6, 2023

🚀 Deployed to production by https://github.com/mountiny in version: 1.2.78-0 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@@ -47,8 +47,8 @@
"@formatjs/intl-numberformat": "^6.2.5",
"@formatjs/intl-pluralrules": "^4.0.13",
"@gorhom/portal": "^1.0.14",
"@oguzhnatly/react-native-image-manipulator": "github:Expensify/react-native-image-manipulator#5cdae3d4455b03a04c57f50be3863e2fe6c92c52",
"@onfido/react-native-sdk": "7.0.1",
"@oguzhnatly/react-native-image-manipulator": "github:Expensify/react-native-image-manipulator#c5f654fc9d0ad7cc5b89d50b34ecf8b0e3f4d050",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nkuoch Was there any reason to revert this to older commit? This introduces regression - #14751

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nkuoch is on leave till June. I'm not sure if there was a specific reason, it might've been something we accidentally overlooked 🤔 Was the solution to the regression to update the hash?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MariaHCD yes, update hash

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the author is on leave. We're hard to verify it. Based on the commit message, I think it's a accident:
278c1fe

Btw, with current package.json file, I can bump @oguzhnatly/react-native-image-manipulator to previous hash without any error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants