Ignore project preview deep-links #538
Conversation
| @@ -1365,6 +1365,39 @@ final class AppDelegateViewModelTests: TestCase { | |||
| self.goToMobileSafari.assertValues([unrecognizedUrl], "Go to mobile safari for the unrecognized url.") | |||
| } | |||
|
|
|||
| func testEmailDeepLinking_UnrecognizedUrl_ProjectPreview() { | |||
There was a problem hiding this comment.
This test is essentially the same as the one above it but it explicitly uses a known project preview link (containing token query param) to test that it will be ignored.
| @@ -38,6 +38,8 @@ public final class NavigationTests: XCTestCase { | |||
| KSRAssertMatch(.project(.slug("project"), .root, refTag: nil), | |||
| "/projects/creator/project") | |||
|
|
|||
| KSRAssertMatch(nil, "/projects/creator/project?token=4") | |||
There was a problem hiding this comment.
Just proves that we get a nil match on this type of URL.
There was a problem hiding this comment.
Wonder whether we should also add KSRAssertMatch(nil, "/projects/creator/project?ref=discovery&token=4") for exhausted url example.
| <^> params <| "project_param" | ||
| <*> .success(.root) | ||
| <*> params <|? "ref" | ||
| <*> params <| "token" |
There was a problem hiding this comment.
Treats token as required for a project preview link.
justinswart
changed the title
| @@ -407,8 +404,11 @@ AppDelegateViewModelOutputs { | |||
| .filter { $0 == .tab(.me) } | |||
| .ignoreValues() | |||
|
|
|||
| self.goToMobileSafari = self.foundRedirectUrlProperty.signal.skipNil() | |||
| .filter { Navigation.deepLinkMatch($0) == nil } | |||
| self.goToMobileSafari = Signal.merge( | |||
There was a problem hiding this comment.
I think you might be able to just use deeplinkFromUrl.skipNil() here. Just wanted to make a note of that in case I forget later 😄
justinswart
changed the title
justinswart
force-pushed
the
ignore-project-preview-deeplinks
branch
2 times, most recently
from
9765033
to
8a35671
Compare
justinswart
force-pushed
the
ignore-project-preview-deeplinks
branch
from
a314a93
to
75cfd98
Compare
There was a problem hiding this comment.
Tested on the following links and it worked!
Opens mobile Safari
https://www.kickstarter.com/projects/963657391/956205078?ref=519010&token=9638e5ba
Opens mobile Safari
https://www.kickstarter.com/projects/963657391/956205078?token=9638e5ba
Opens mobile Safari
https://www.kickstarter.com/projects/963657391/17931492?ref=4oy4j7&token=4ba1e68a
Opens discover screen with proper sort order
https://www.kickstarter.com/discover?sort=newest
|
Thanks for digging in and testing! |
📲 What
Ignores project preview links as these should be handled by mobile safari and not by the app itself.
🤔 Why
We don't currently support project preview links in the app but because their URL structure matches a route in our
Navigationenum the app attempts to open them. This fails and the user is stuck in the app when they should be redirected back out to mobile safari.🛠 How
projectPreviewcase toNavigationso that we can be sure that a given URL is in fact a preview URL..failure, thereby using the existing functionality to ignore unrecognized URLs.👀 See
https://trello.com/c/4GS7SNZ8
✅ Acceptance criteria