wallet icon indicating copy to clipboard operation
wallet copied to clipboard

feat: make invite referral link short

Open kathaypacific opened this issue 2 years ago • 5 comments

Description

An example of a short link: https://vlra.app/vqT5MqgEVCVnBjJB7

Video of it working on Android: https://user-images.githubusercontent.com/20150449/196137182-731cb918-9880-41e5-94cd-c9d430885f34.mp4

Other changes

N/A

Tested

Manually - ensure that the link can be created / clicked on. On mobile it should take you to the app or app store, on desktop it should take you to the valoraapp.com website (a redirect to the homepage is coming soon)

How others should test

Ensure that the invite link can be shared via the invite screen

Related issues

  • Fixes RET-435

Backwards compatibility

Yes

kathaypacific avatar Oct 14 '22 12:10 kathaypacific

RET-435 [Wallet] Use short links for Invite via Share URL

Currently we're using DynamicLinks' createLink method. It works fine, but results in a long and ugly URL.

Instead we can use the createShortLink method. When the original implementation was being built, we were using createShortLink successfully. However, at some point it stopped working on Android (likely after rebasing from main). Specifically, the call to createShortLink would just hang forever, with no warnings or errors. We switched back to createLink in order to ship the PR.

Some ideas to debug and fix on Android:

  • Might need to go over the Android setup steps again: https://rnfirebase.io/dynamic-links/usage#android-setup
  • Re-visit any Android-specific updates that went into main shortly before the original PR was merged, and see if we can identify which broke the integration.
  • Upgrade the @react-native-firebase/* packages
  • Dig deeper into native and/or network logs to see if any errors pop up

linear[bot] avatar Oct 14 '22 12:10 linear[bot]

Codecov Report

Merging #2996 (3eaf908) into main (0310e7c) will decrease coverage by 0.00%. The diff coverage is 76.92%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2996      +/-   ##
==========================================
- Coverage   80.37%   80.37%   -0.01%     
==========================================
  Files         647      647              
  Lines       23486    23495       +9     
  Branches     4226     4228       +2     
==========================================
+ Hits        18877    18884       +7     
- Misses       4552     4554       +2     
  Partials       57       57              
Impacted Files Coverage Δ
src/firebase/firebase.ts 24.11% <25.00%> (+0.02%) :arrow_up:
src/app/saga.ts 60.11% <100.00%> (+0.91%) :arrow_up:
src/invite/utils.ts 94.59% <100.00%> (+0.15%) :arrow_up:
src/tokens/utils.ts 100.00% <0.00%> (+2.94%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 0310e7c...3eaf908. Read the comment docs.

codecov[bot] avatar Oct 17 '22 09:10 codecov[bot]

An example of a short link I was able to generate on Android: https://vlra.app/vUL2DsRdrt52Xvwd9

Related, how do we want to handle clicking these links from a desktop browser? Right now it leads to a 404.

jeanregisser avatar Oct 17 '22 10:10 jeanregisser

An example of a short link I was able to generate on Android: https://vlra.app/vUL2DsRdrt52Xvwd9

Related, how do we want to handle clicking these links from a desktop browser? Right now it leads to a 404.

yeah good question. i'm not sure. it does lead to a 404 right now but that's not a change due to the short link - the long link also gives a 404. i couldn't find a way to make it redirect to a webpage on a browser since firebase dynamic links seems to not allow you to configure links with dynamic parts :/

do you think this is an important use case for us to cover now? i don't think that many users will try to click this link on a desktop browser...

kathaypacific avatar Oct 17 '22 12:10 kathaypacific

@jeanregisser this is ready for review again :)

kathaypacific avatar Oct 18 '22 11:10 kathaypacific