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

Update the Apple app association file #6385

Merged
merged 3 commits into from
Nov 22, 2021
Merged

Conversation

mountiny
Copy link
Contributor

@mountiny mountiny commented Nov 21, 2021

Details

Apple is deprecating the format of app-association-file we are currently using, therefore we need to change it. The new format is described in this documentation and the applinks property is described more specifically on this page.

The file also needs to be served from .well-known/ directory as described in the docs:
image

Check the linked issue for more details.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/185770

Tests

No local tests.

QA Steps

Once deployed to staging, make sure to have the latest app version and then click this link and make sure the app opens instead of the browser.

Also, put staging.new.expensify.com into this Apple tool https://search.developer.apple.com/appsearch-validation-tool and make sure there are no errors.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Mobile Web

Desktop

iOS

Android

@mountiny mountiny requested a review from Jag96 November 21, 2021 15:38
@mountiny mountiny requested a review from a team as a code owner November 21, 2021 15:38
@mountiny mountiny self-assigned this Nov 21, 2021
@MelvinBot MelvinBot requested review from Dal-Papa and removed request for a team November 21, 2021 15:38
@mountiny
Copy link
Contributor Author

@Dal-Papa We chatted with Joe over DM about reviewing this and he is OOO for a week so we are not in a rush on this. I guess we could even go forward with this one as we have already made these changes in Web-E.

Copy link
Contributor

@Jag96 Jag96 left a comment

Choose a reason for hiding this comment

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

@vitHoracek we were able to test #6366 on staging so I think we can do the same here. Since we can test on staging I don't think we need to hold on this, I'll update the QA section

@Jag96 Jag96 changed the title [NoQA] Update the Apple app association file Update the Apple app association file Nov 22, 2021
@Jag96 Jag96 merged commit 79874d2 into main Nov 22, 2021
@Jag96 Jag96 deleted the vit-updateAppleAssociation branch November 22, 2021 19:49
@mountiny
Copy link
Contributor Author

@Jag96 Ah, that is true. I will test it out when it gets deployed to Staging 🙌

@Jag96
Copy link
Contributor

Jag96 commented Nov 22, 2021

Looks like we missed something here: https://github.com/Expensify/App/runs/4291419056?check_suite_focus=true. ERROR in unable to locate "apple-app-site-association" at "/home/runner/work/App/App/apple-app-site-association"

This file has a mention that we need to update, going to make a quick PR.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @Jag96 in version: 1.1.15-19 🚀

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

@mountiny
Copy link
Contributor Author

@Jag96 ah, good catch, waiting for the E2E to pass to merge the fix!

@mountiny
Copy link
Contributor Author

Testing this on staging and not sure what is the issue. The validator says this:
image

But the deeplink on my phone works as expected, which makes me think it can't be broken.

@marcaaron
Copy link
Contributor

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.1.16-10 🚀

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

@mountiny
Copy link
Contributor Author

@Jag96 Ok, so the apple association validator is still showing error which is confusing to me honestly.
image

For expensify.com, the validator does the same:
image

When I made the changes there, I have left the old format as a fallback as well. At the same time, we know that on both platforms the universal links work so the file cannot be completely wrong. The validator refers to the old archived documentation so maybe it is the validator which is not updated 😵‍💫

@marcaaron
Copy link
Contributor

At the same time, we know that on both platforms the universal links work so the file cannot be completely wrong

If Universal Links work... what are we trying to fix? What are the consequences of not passing the "validator"?

@marcaaron
Copy link
Contributor

FWIW, https://new.expensify.com/.well-known/apple-app-site-association doesn't seem to download a file, but https://new.expensify.com/apple-app-site-association does. 🙃

@mountiny
Copy link
Contributor Author

@marcaaron correct, I got confused, this PR which fixes the location just got deployed to staging #6430 (comment)

So we should try the validator in couple of minutes on staging. However, on expensify.com the validator also does not pass.

I guess technically, the validator should tell us the universal link are not working because the file has a wrong format, however, that is clearly not the case since the OldDot the validator also shows error, but we know the links work for us 🤷‍♂️

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.

5 participants