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

refactor: move deep link prefix to .env file #6008

Merged
merged 5 commits into from
Sep 16, 2024
Merged

Conversation

kathaypacific
Copy link
Collaborator

@kathaypacific kathaypacific commented Sep 16, 2024

Description

Related to making the MS runtime native folders brand agnostic. This PR:

  • add deep link prefix value to .env files
  • read the variable during build and run times
  • update the config name in the src folder

Test plan

CI passes - we have deep link tests there.

Related issues

Backwards compatibility

Y

Network scalability

If a new NetworkId and/or Network are added in the future, the changes in this PR will:

  • Continue to work without code changes, OR trigger a compilation error (guaranteeing we find it when a new network is added)

Copy link

codecov bot commented Sep 16, 2024

Codecov Report

Attention: Patch coverage is 88.23529% with 2 lines in your changes missing coverage. Please review.

Project coverage is 88.62%. Comparing base (7721ad2) to head (045de29).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/fiatExchanges/SimplexScreen.tsx 66.66% 1 Missing ⚠️
src/webview/WebViewScreen.tsx 66.66% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6008      +/-   ##
==========================================
+ Coverage   88.61%   88.62%   +0.01%     
==========================================
  Files         728      726       -2     
  Lines       30546    30466      -80     
  Branches     5224     5216       -8     
==========================================
- Hits        27068    27001      -67     
+ Misses       3435     3422      -13     
  Partials       43       43              
Files with missing lines Coverage Δ
src/app/reducers.ts 25.58% <100.00%> (+1.13%) ⬆️
src/config.ts 96.33% <100.00%> (ø)
src/qrcode/schema.ts 100.00% <100.00%> (ø)
src/qrcode/utils.ts 68.42% <100.00%> (ø)
src/walletConnect/saga.ts 58.25% <100.00%> (ø)
src/walletConnect/walletConnect.ts 100.00% <100.00%> (ø)
src/fiatExchanges/SimplexScreen.tsx 84.61% <66.66%> (-0.20%) ⬇️
src/webview/WebViewScreen.tsx 62.06% <66.66%> (ø)

... and 16 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

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

Copy link
Member

@jeanregisser jeanregisser left a comment

Choose a reason for hiding this comment

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

Great, thanks! 👍

@@ -77,8 +77,9 @@ - (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(
[[CleverTapReactManager sharedInstance] applicationDidLaunchWithOptions:launchOptions];

NSString *env = [RNCConfig envFor:@"FIREBASE_ENABLED"];
NSString *deepLinkPrefix = [[NSBundle mainBundle] objectForInfoDictionaryKey:@"DEEP_LINK_PREFIX"];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
NSString *deepLinkPrefix = [[NSBundle mainBundle] objectForInfoDictionaryKey:@"DEEP_LINK_PREFIX"];
NSString *deepLinkPrefix = [RNCConfig envFor:@"DEEP_LINK_PREFIX"];

.env.alfajores Outdated
@@ -12,3 +12,4 @@ IOS_GOOGLE_SERVICE_PLIST=GoogleService-Info.alfajores.plist
SENTRY_ENABLED=true
AUTH0_DOMAIN=auth.alfajores.valora.xyz
ONBOARDING_FEATURES_ENABLED=CloudBackupSetup,CloudBackupRestore,CloudBackupInOnboarding,EnableBiometry,ProtectWallet
DEEP_LINK_PREFIX=celo
Copy link
Member

@jeanregisser jeanregisser Sep 16, 2024

Choose a reason for hiding this comment

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

What do you think of naming it DEEP_LINK_URL_SCHEME?
It better represents what it is and more closely follows what iOS and Android use for this.

@kathaypacific kathaypacific added this pull request to the merge queue Sep 16, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 16, 2024
@kathaypacific kathaypacific added this pull request to the merge queue Sep 16, 2024
Merged via the queue into main with commit 91d9835 Sep 16, 2024
16 checks passed
@kathaypacific kathaypacific deleted the kathy/deep-link-env branch September 16, 2024 11:15
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.

2 participants