Page MenuHomePhabricator
Differential D7744

[native] Check install referrer on Android
ClosedPublic
Actions

Authored by tomek on May 8 2023, 9:34 AM.
Tags
None
Referenced Files
F3387652: D7744.diff
Fri, Nov 29, 10:32 AM
Unknown Object (File)
Mon, Nov 18, 3:14 PM
Unknown Object (File)
Wed, Nov 13, 4:40 AM
Unknown Object (File)
Thu, Nov 7, 4:45 PM
Unknown Object (File)
Oct 28 2024, 1:27 AM
Unknown Object (File)
Oct 28 2024, 1:27 AM
Unknown Object (File)
Oct 28 2024, 1:26 AM
Unknown Object (File)
Oct 28 2024, 1:26 AM
View All Files
Subscribers
ashoat
atul

Details

Reviewers
bartek
kamil
inka
ashoat
Commits
rCOMMfb20be65e509: [native] Check install referrer on Android
Summary

When a user clicks an invite link and doesn't have the app installed, we redirect him to Play Store from where the app can be installed. This redirection specifies a referrer parameter which can be used to check from where the app was installed. The, when the app is run, we check if referrer was set and if that's the case, we use it just like an invite link.

Depends on D7751

Test Plan

Open an invite link on an emulator without the app installed. Install the app. Run the app - the invitation modal should appear. Close the app and open it - the invitation shouldn't appear. Log out, close the app, open the app, log in - the invitation shouldn't appear.

Diff Detail

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

Event Timeline

tomek created this revision.May 8 2023, 9:34 AM
Herald added subscribers: atul, ashoat. · View Herald TranscriptMay 8 2023, 9:34 AM
Harbormaster returned this revision to the author for changes because remote builds failed.May 8 2023, 9:48 AM
Harbormaster failed remote builds in B19139: Diff 26204!
tomek updated this revision to Diff 26249.May 9 2023, 2:12 AM

Rebase

Harbormaster completed remote builds in B19168: Diff 26249.May 9 2023, 2:30 AM
tomek requested review of this revision.May 9 2023, 2:30 AM
tomek updated this revision to Diff 26268.May 9 2023, 2:55 AM

Use onFirstLaunch effect

tomek edited the summary of this revision. (Show Details)May 9 2023, 2:55 AM
tomek edited the test plan for this revision. (Show Details)
tomek added a parent revision: D7751: [native] Introduce a hook that allows running an effect just once per installation.
tomek removed a parent revision: D7743: [native] Unsubscribe listener on unmount.
Harbormaster completed remote builds in B19186: Diff 26268.May 9 2023, 3:12 AM
bartek accepted this revision.May 9 2023, 4:45 AM
This revision is now accepted and ready to land.May 9 2023, 4:45 AM
ashoat requested changes to this revision.May 9 2023, 9:43 AM
ashoat added inline comments.
native/navigation/invite-link-handler.react.js
44 ↗(On Diff #26268)

You are redefining the effect function here on every invocation, which means it is very possible that this will get called multiple times if InviteLinkHandler is rerendered before the async logic completes (including the AsyncStorage stuff in useOnFirstLaunchEffect, as well as the function passed here)

45 ↗(On Diff #26268)

Why is this IIFE necessary here?

46 ↗(On Diff #26268)

We should "early exit" to reduce indentation here

This revision now requires changes to proceed.May 9 2023, 9:43 AM
tomek added inline comments.May 15 2023, 3:22 AM
native/navigation/invite-link-handler.react.js
44 ↗(On Diff #26268)

That makes sense - going to fix it.

45 ↗(On Diff #26268)

We have to use an async function, but you're right, the fact that the second parameter of useOnFirstLaunchEffect returns mixed, means that we can simplify.

tomek updated this revision to Diff 26461.May 15 2023, 5:05 AM

Refactor the code

Harbormaster completed remote builds in B19336: Diff 26461.May 15 2023, 5:28 AM
ashoat accepted this revision.May 15 2023, 9:46 AM
This revision is now accepted and ready to land.May 15 2023, 9:46 AM
ashoat added inline comments.May 15 2023, 9:46 AM
native/utils/crash-utils.js
15 ↗(On Diff #26461)

Unrelated, but I wonder if we should be clearing SQLite here too

tomek added inline comments.May 16 2023, 3:59 AM
native/utils/crash-utils.js
15 ↗(On Diff #26461)

Yeah, that makes sense - created a task for it https://linear.app/comm/issue/ENG-3898/clear-sqlite-database-after-wipe-and-exit-action

Closed by commit rCOMMfb20be65e509: [native] Check install referrer on Android (authored by tomek). · Explain WhyMay 16 2023, 5:10 AM
This revision was automatically updated to reflect the committed changes.
tomek added a commit: rCOMMfb20be65e509: [native] Check install referrer on Android.

Revision Contents
Changeset List

PathSize
native/
ios/
4 lines
navigation/
24 lines
1 line
5 lines

Diff 26249

View Options

native/ios/Podfile.lock

Loading...
View Options

native/navigation/invite-link-handler.react.js

Loading...
View Options

native/package.json

Loading...
View Options

yarn.lock

Loading...