Page MenuHomePhabricator

[native] Refactor the deep links context provider to support both invite links and QR code links
ClosedPublic

Authored by rohan on Aug 30 2023, 8:52 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 26, 2:42 AM
Unknown Object (File)
Sun, Nov 10, 7:21 PM
Unknown Object (File)
Thu, Oct 31, 1:50 PM
Unknown Object (File)
Thu, Oct 31, 1:50 PM
Unknown Object (File)
Thu, Oct 31, 1:50 PM
Unknown Object (File)
Thu, Oct 31, 1:50 PM
Unknown Object (File)
Thu, Oct 31, 1:50 PM
Unknown Object (File)
Thu, Oct 31, 1:46 PM
Subscribers

Details

Summary

This diff handles moving all of the deep link handling logic into the one. The main thing here is we use both parse methods (parseSecretFromInviteLinkURL and parseKeysFromQRCodeURL) on the current link, and then handle what to do based on whether the regex finds a match. We check for the invite link first because it is possible in theory for there to be an invite link like /invite/qr-code/ if the community is called qr-code, but it's not really possible for the QR code link to contain an invite route.

This is not part of a monthly goal, just a follow-up task.

Depends on D9027

Part of ENG-4810

Test Plan

Confirmed the following:

  • While the app is open, click on an invite link and confirm that Comm opens and the invite link modal appears to verify it
  • While the app is closed, click on the invite link and confirm that Comm opens and the invite link modal appears to verify it
  • While the app is not logged in, click on the invite link and confirm that Comm opens and following a successful login, the invite link modal appears to verify it
  • While the app is open, scan the login QR code in my iOS camera and confirm that Comm opens and navigates to the scanner
  • While the app is closed, scan the login QR code in my iOS camera and confirm that Comm opens and then navigates to the scanner
  • While the app is not logged in, scan the login QR code in my iOS camera and confirm that Comm opens, and following a login, navigates to the scanner
  • Made sure that invite links don't open the barcode scanner
  • Made sure that scanning a QR code doesn't prompt any invite link modals

Also following the React Navigation docs, I ran the following:

  • npx uri-scheme open "comm://invite/121313" --ios and confirmed that Comm opened and prompted the invite link modal
  • npx uri-scheme open "comm://qr-code/121313" --ios and confirmed that Comm opened and opened the barcode scanner

Diff Detail

Repository
rCOMM Comm
Branch
handlers
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

rohan edited the test plan for this revision. (Show Details)
Harbormaster returned this revision to the author for changes because remote builds failed.Aug 30 2023, 8:54 AM
Harbormaster failed remote builds in B22271: Diff 30568!
rohan requested review of this revision.Aug 30 2023, 9:06 AM

Shouldn't we delete QRCodeLinkHandler as a part of this diff?

native/navigation/deep-links-context-provider.react.js
98–99 ↗(On Diff #30568)

Might be worth implementing a method that does the whole parsing and returns the result in an explicit way, e.g. something like

(url: string) => { type: 'inviteLink', data: {secret: ...} } | { type: 'qr-code', data: { keys: ...} } | null
102–107 ↗(On Diff #30568)

After this refactoring, this code should be in an invite-link-specific location. Currently, it might be tricky to get it right, but after doing https://linear.app/comm/issue/ENG-4391/display-a-message-if-youre-already-part-of-a-community it should be straightforward.

This revision is now accepted and ready to land.Aug 31 2023, 4:41 AM
In D9028#266436, @tomek wrote:

Shouldn't we delete QRCodeLinkHandler as a part of this diff?

Yeah I could do, I'll abandon D9029 and just delete it here

native/navigation/deep-links-context-provider.react.js
98–99 ↗(On Diff #30568)

Sure that would be good!

102–107 ↗(On Diff #30568)

Yeah that makes sense. Would it be best to leave as-is right now until that task is done?

native/navigation/deep-links-context-provider.react.js
102–107 ↗(On Diff #30568)

Yeah, that would make the most sense. We can also add a note in that task.

rohan requested review of this revision.Aug 31 2023, 8:22 AM

Re-requesting review after addressing feedback

This revision is now accepted and ready to land.Sep 1 2023, 3:04 AM
This revision was landed with ongoing or failed builds.Sep 1 2023, 7:17 AM
This revision was automatically updated to reflect the committed changes.