Fix Deep Links not working on Android 12+ #10128

tomivs · 2022-07-27T03:19:56Z

Details

Upgrading targetSdkVersion, a couple npm packages and AndroidManifest.xml in order to make Deep Links work on Android 12+.

Fixed Issues

$ #9628

Tests

Log in to the app on Android 12
Open web browser
Click on links that start with "new.expensify.com/..."
Click on links to Statements page "new.expensify.com/statements/..."

NOTE: It's necessary to host the file "new.expensify.com/.well-known/assetlinks.json" for new.expensify... links to work (as well as staging).

Verify that no errors appear in the JS console

PR Review Checklist

Contributor (PR Author) Checklist

PR Reviewer Checklist

The Contributor+ will copy/paste it into a new comment and complete it after the author checklist is completed

QA Steps

Log in to the app on Android 12
Open web browser
Click on links that start with "new.expensify.com/..."
Click on links to Statements page "new.expensify.com/statements/..."

NOTE: It's necessary to host the file "new.expensify.com/.well-known/assetlinks.json" for new.expensify... links to work (as well as staging).

Verify that no errors appear in the JS console

Screenshots

Web

WebDeepLinks_AirshipiOS_16_4.MOV

Mobile Web

MobileWebDeepLinks_AirshipiOS_16_4.MOV

Desktop

DesktopDeepLinks_AirshipiOS_16_4.MOV

iOS

iOSDeepLinks_AirshipiOS_16_4_0.mp4

Android

deep-links-android-12.mp4

parasharrajat · 2022-07-27T08:20:15Z

Correct all the links in the description. It is new.expensify.com not news.expensify.com.

tomivs · 2022-07-27T09:23:35Z

Correct all the links in the description. It is new.expensify.com not news.expensify.com.

Oops. Just did it.

parasharrajat · 2022-07-27T11:35:12Z

Did you test it thoroughly? Video does not show click on expensify links.

tomivs · 2022-07-27T14:55:49Z

Did you test it thoroughly? Video does not show click on expensify links.

Yes. I tested it out using my own domain.

The reason why I didn't show it using Expensify links is because they're not verified (therefore they won't work). We must host the https://new.expensify.com/.well-known/assetlinks.json in order to verify the links for new.expensify.com domain (same for staging domain).

I hosted that file using my domain and it worked. Moreover, I checked the links using adb shell pm get-app-links com.expensify.chat and they showed up —which they didn't before— of course saying that they were not verified.

parasharrajat · 2022-07-27T20:13:58Z

Ok, Then please create the assetlinks.json file and put it in https://github.com/Expensify/App/tree/main/.well-known. Then add the necessary config to allow it to be deployed on a web server. I think you can follow apple-app-site-association for the same.

Let us know what should be done internally.

parasharrajat · 2022-07-27T21:00:26Z

android/app/src/main/AndroidManifest.xml

@@ -29,23 +29,24 @@
      <activity
        android:name=".bootsplash.BootSplashActivity"
        android:theme="@style/BootTheme"
-        android:launchMode="singleTask">
+        android:launchMode="singleTask"
+        android:exported="true">


Suggested change

android:exported="true">

android:exported="false">

The value for android:exported must be set to true, otherwise the links won't open the app.

According to Android docs:

If the app component includes the LAUNCHER category, set android:exported to true. In most other cases, set android:exported to false.

Our app component includes the LAUNCHER category

https://github.com/tomivs/App/blob/e55f74de55f087e1b0fd77afdc1c6d2356ccb0b2/android/app/src/main/AndroidManifest.xml#L37

Ahh! I see.

tomivs · 2022-07-27T21:45:29Z

Ok, Then please create the assetlinks.json file and put it in https://github.com/Expensify/App/tree/main/.well-known. Then add the necessary config to allow it to be deployed on a web server. I think you can follow apple-app-site-association for the same.

Let us know what should be done internally.

Just added the file and config for deployment.

For QA: this can only be tested after deployment, since it requires the assetlinks.json file to be hosted in the server.

parasharrajat · 2022-07-27T22:15:49Z

.well-known/assetlinks.json

+    "target": {
+      "namespace": "android_app",
+      "package_name": "com.expensify.chat",
+      "sha256_cert_fingerprints": ["2E:65:6F:1C:34:F5:7E:BF:FC:C0:2D:A3:14:0E:83:FE:61:51:F2:9B:5D:59:58:61:C4:4D:A9:99:0C:CA:F4:8E"]


Is this correct? Or we have to change it. The gpg key in the codebase is encrypted if you used that to get the signature.

I extracted it from the New Expensify's APK from the Play Store, so it should probably work.

@AndrewGable Could you please verify this? https://developer.android.com/training/app-links/verify-site-associations#web-assoc .

@AndrewGable might be a bit slow to respond this week… He's getting married 😅 💍

Awesome, definitely he won't be able to respond. In that case, @deetergp Could you please check the signature? https://developer.android.com/training/app-links/verify-site-associations#web-assoc has details to get the signature.

Yes.. Please let's double check this..

Verified in Google Play store and it is correct!

Thanks @luacmartins

parasharrajat

Add working app videos for each platform.

tomivs · 2022-08-03T19:14:11Z

Add working app videos for each platform.

The Deep Link feature is only for the native mobile apps (Android & iOS). Nonetheless, I can add some videos just showing the app working for other platforms.

parasharrajat · 2022-08-04T03:09:41Z

ios/NewExpensify.xcodeproj/project.pbxproj

@@ -960,7 +960,6 @@
 				LD_RUNPATH_SEARCH_PATHS = "/usr/lib/swift $(inherited)";
 				LIBRARY_SEARCH_PATHS = (
 					"\"$(TOOLCHAIN_DIR)/usr/lib/swift/$(PLATFORM_NAME)\"",
-					"\"$(TOOLCHAIN_DIR)/usr/lib/swift-5.0/$(PLATFORM_NAME)\"",


Because keeping that path after updating the npm package urbanairship-react-native causes a compilation error for iOS. This was commented in this issue.

I think keeping only the line above should be enough.

Also, I had updated to [email protected] and not to the latest version to avoid incompatibilities. However, apparently we could (and should) update to a higher version — in this case [email protected].

One thing to keep in mind is that we use the macOS-11 image to build the iOS app.

App/.github/workflows/platformDeploy.yml

Line 131 in 06b378d

runs-on: macos-11

IMO, macOS_11 does have Xcode 13 so it should be fine. But we should be careful.

tomivs · 2022-08-04T05:52:02Z

ios/NewExpensify.xcodeproj/project.pbxproj

-				"${PODS_ROOT}/Airship/Airship/AirshipMessageCenter/Resources/UAMessageCenterActions.plist",
-				"${PODS_ROOT}/Airship/Airship/AirshipMessageCenter/Resources/UAMessageCenterListCell.xib",
-				"${PODS_ROOT}/Airship/Airship/AirshipMessageCenter/Resources/UAMessageCenterPlaceholderIcon.png",
-				"${PODS_ROOT}/Airship/Airship/AirshipMessageCenter/Resources/UAInbox.xcdatamodeld",


Resource bundle changes.

Reference.

tomivs · 2022-08-04T09:53:01Z

Add working app videos for each platform.

Just added the working videos for each platform.

tomivs · 2022-08-05T08:53:18Z

I noticed that @janicduplessis is already doing some of the work for me in #9841.

Specifically:

Upgrading targetSdkVersion to 31
Updating @react-native-async-storage/async-storage and urbanairship-react-native npm packages
Updating Podfile.lock and project.pbxproj

Therefore I should probably drop my some of my changes and only:

Update AndroidManifest.xml
Add assetlinks.json to .well-known folder and set up deployment for the aforementioned file

parasharrajat · 2022-08-05T19:27:57Z

We can but relying on that PR is not a very safe choice. I would still prefer to upgrade separately. It's fine if that PR overwrites your changes.

tomivs · 2022-08-08T14:48:00Z

We can but relying on that PR is not a very safe choice. I would still prefer to upgrade separately. It's fine if that PR overwrites your changes.

Got it. The PR we mentioned was already merged so, should we stick with my changes (which are using a RN version that is older than the current one in main)?

parasharrajat · 2022-08-08T15:59:44Z

Please merge main into this and retest.

This reverts commit 6a1741f.

This reverts commit 0d21e9e.

This reverts commit 33ef541.

This reverts commit 921bf0c.

This reverts commit fa5ccf8.

This reverts commit 15f6dc4.

This reverts commit 0d21e9e.

tomivs · 2022-08-11T09:01:21Z

Please merge main into this and retest.

Merged main and tested. Working as expected.

parasharrajat · 2022-08-11T13:55:01Z

android/build.gradle

-        compileSdkVersion = 31
-        targetSdkVersion = 31
+        compileSdkVersion = 30
+        targetSdkVersion = 30


I thought we need 31 for Android 12

targetSdkVersion = 31 is necessary and is already in main. It seems like here you're suggesting to change it to 30?

my bad this is an old commit. All good on this.

parasharrajat

I am 100% with the changes here so I am approving this. But I am not able to test these changes exactly.

I can build the app with local signature and test domain using a local server but not with the correct config which can only be tested if I password to that signing key.

cc: @deetergp

PR 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 verified tests pass on all platforms & I tested again on:
- iOS / native
- Android / native
- iOS / Safari
- Android / Chrome
- MacOS / Chrome
- 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 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 was added in all src/languages/* files
- I verified any copy / text that was added to the app is correct English and approved by marketing by tagging the marketing team 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 */
- Any functional components have the displayName property
- 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 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.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

🎀 👀 🎀 C+ reviewed

mvtglobally · 2022-08-18T04:30:43Z

@tomivs @parasharrajat @AndrewGable Can you please share couple links we should validate for sure as part of this PR.

tomivs · 2022-08-19T23:37:43Z

@mvtglobally this PR must be QA tested after deployment, because it requires a file to be uploaded to the server.

Nonetheless, here are some links to test:

You could also visit https://tomivs.com/linki/ and click the links directly from there.

tomivs added 3 commits July 27, 2022 04:55

feat: upgrading targetSdkVersion

6a1741f

feat: updating npm pkgs for new targetSdkVersion

0d21e9e

feat: android:exported for activities

33ef541

tomivs requested a review from a team as a code owner July 27, 2022 03:19

melvin-bot bot requested review from deetergp and parasharrajat and removed request for a team July 27, 2022 03:19

parasharrajat suggested changes Jul 27, 2022

View reviewed changes

tomivs added 2 commits July 27, 2022 23:19

chore: adds assetlinks.json for link verification

b1ccedf

chore: webpack config to deploy assetlinks.json

e55f74d

parasharrajat reviewed Jul 27, 2022

View reviewed changes

parasharrajat suggested changes Jul 31, 2022

View reviewed changes

fix: removing old Swift reference

921bf0c

parasharrajat reviewed Aug 4, 2022

View reviewed changes

tomivs added 2 commits August 4, 2022 07:21

fix: removing old Swift reference 2

fa5ccf8

feat: updating to [email protected]

15f6dc4

tomivs commented Aug 4, 2022

View reviewed changes

Revert "feat: upgrading targetSdkVersion"

93e92b1

This reverts commit 6a1741f.

tomivs and others added 8 commits August 11, 2022 10:20

Revert "feat: updating npm pkgs for new targetSdkVersion"

f5ecc63

This reverts commit 0d21e9e.

Revert "feat: android:exported for activities"

c0844c0

This reverts commit 33ef541.

Revert "fix: removing old Swift reference"

01b905e

This reverts commit 921bf0c.

Revert "fix: removing old Swift reference 2"

2fb9853

This reverts commit fa5ccf8.

Revert "feat: updating to [email protected]"

99b0890

This reverts commit 15f6dc4.

Revert "feat: updating npm pkgs for new targetSdkVersion"

c8aa9fa

This reverts commit 0d21e9e.

Merge branch 'Expensify:main' into tomivs-deep-links-android-12

9904c07

fix: set up AndroidManifest.xml for Android 12+

6399644

parasharrajat suggested changes Aug 11, 2022

View reviewed changes

Merge branch 'Expensify:main' into tomivs-deep-links-android-12

695862c

parasharrajat approved these changes Aug 11, 2022

View reviewed changes

tomivs added 2 commits August 11, 2022 13:17

Merge branch 'Expensify:main' into tomivs-deep-links-android-12

1b4fa01

Merge branch 'Expensify:main' into tomivs-deep-links-android-12

3e01ca4

deetergp approved these changes Aug 15, 2022

View reviewed changes

deetergp merged commit 82dc866 into Expensify:main Aug 15, 2022

OSBotify mentioned this pull request Aug 15, 2022

Deploy Checklist: New Expensify 2022-08-04 #10253

Closed

melvin-bot bot mentioned this pull request Aug 23, 2022

[HOLD for payment 2022-08-30] [$250] Android - Deep Links not working on Android 12+@tomivs #9628

Closed

Fix Deep Links not working on Android 12+ #10128

Fix Deep Links not working on Android 12+ #10128

Conversation

tomivs commented Jul 27, 2022 • edited Loading

Details

Fixed Issues

Tests

PR Review Checklist

Contributor (PR Author) Checklist

PR Reviewer Checklist

QA Steps

Screenshots

Web

Mobile Web

Desktop

iOS

Android

parasharrajat commented Jul 27, 2022

tomivs commented Jul 27, 2022

parasharrajat commented Jul 27, 2022

tomivs commented Jul 27, 2022

parasharrajat commented Jul 27, 2022 • edited Loading

Choose a reason for hiding this comment

tomivs Jul 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomivs commented Jul 27, 2022 • edited Loading

parasharrajat Jul 27, 2022 • edited Loading

Choose a reason for hiding this comment

tomivs Jul 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parasharrajat left a comment

Choose a reason for hiding this comment

tomivs commented Aug 3, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomivs Aug 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomivs commented Aug 4, 2022

tomivs commented Aug 5, 2022

parasharrajat commented Aug 5, 2022

tomivs commented Aug 8, 2022

parasharrajat commented Aug 8, 2022

tomivs commented Aug 11, 2022

Choose a reason for hiding this comment

tomivs Aug 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parasharrajat left a comment

Choose a reason for hiding this comment

PR Reviewer Checklist

mvtglobally commented Aug 18, 2022

tomivs commented Aug 19, 2022

tomivs commented Jul 27, 2022 •

edited

Loading

parasharrajat commented Jul 27, 2022 •

edited

Loading

tomivs Jul 27, 2022 •

edited

Loading

tomivs commented Jul 27, 2022 •

edited

Loading

parasharrajat Jul 27, 2022 •

edited

Loading

tomivs Jul 27, 2022 •

edited

Loading

tomivs Aug 4, 2022 •

edited

Loading

tomivs Aug 11, 2022 •

edited

Loading