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

[ADHOCREQ-106] Redirect to safari for braze links that aren't deep links #1845

Merged
merged 1 commit into from Aug 22, 2023

Conversation

ifosli
Copy link
Contributor

@ifosli ifosli commented Aug 21, 2023

📲 What

We currently try to parse all braze links as deep links. This is a simple change that looks at the link itself to determine if it's a deep link or not. Deep links are handled the same as before; other links will now open in Safari instead of doing nothing.

Note that this does not try to parse the braze info to see if the link was intended to be opened in web or not.

🤔 Why

External links from braze notifications are currently broken; nothing happens if the user tries to click on them.

👀 See

JIRA bug

RPReplay-Final1692715886.mov

✅ Acceptance criteria

  • Current deep links are unchanged
  • Current external urls are unchanged
  • Braze urls that aren't deep links should now open in Safari

⏰ TODO

  • Add screencast

@ifosli ifosli added the WIP label Aug 21, 2023
@codecov
Copy link

codecov bot commented Aug 22, 2023

Codecov Report

Merging #1845 (1c02ede) into main (15d3e2b) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1845   +/-   ##
=======================================
  Coverage   84.54%   84.54%           
=======================================
  Files        1272     1272           
  Lines      115364   115378   +14     
  Branches    30710    30710           
=======================================
+ Hits        97530    97542   +12     
- Misses      16765    16767    +2     
  Partials     1069     1069           
Files Changed Coverage Δ
Kickstarter-iOS/AppDelegateViewModel.swift 93.05% <100.00%> (+0.04%) ⬆️
Kickstarter-iOS/AppDelegateViewModelTests.swift 98.72% <100.00%> (+<0.01%) ⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@msadoon msadoon left a comment

Choose a reason for hiding this comment

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

Yea great quick fix.

@msadoon
Copy link
Contributor

msadoon commented Aug 22, 2023

Just for future, small organizational things - include the milestone in this PR 5.10.0 in this case and update the branch name and pr name with ticket (ie. MBL-999), assign yourself to the ticket.

@ifosli ifosli added this to the release-5.10.0 milestone Aug 22, 2023
@ifosli
Copy link
Contributor Author

ifosli commented Aug 22, 2023

Just for future, small organizational things - include the milestone in this PR 5.10.0 in this case and update the branch name and pr name with ticket (ie. MBL-999), assign yourself to the ticket.

Added the milestone - I didn't do this initially, since I wasn't sure what release this would land in. I didn't think we have a ticket for this, but I guess you're counting ADHOCREQ-106. I'll link it.

@ifosli ifosli changed the title Redirect to safari for braze links that aren't deep links [ADHOCREQ-106] Redirect to safari for braze links that aren't deep links Aug 22, 2023
@ifosli ifosli self-assigned this Aug 22, 2023
Copy link
Contributor

@scottkicks scottkicks left a comment

Choose a reason for hiding this comment

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

@ifosli ifosli merged commit 9a91c6b into main Aug 22, 2023
6 checks passed
@ifosli ifosli deleted the external-links-bug branch August 22, 2023 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants