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

Ignore project preview deep-links #538

Merged
merged 6 commits into from Jan 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 3 additions & 5 deletions Kickstarter-iOS/ViewModels/AppDelegateViewModel.swift
Expand Up @@ -287,16 +287,13 @@ AppDelegateViewModelOutputs {
let continueUserActivityWithNavigation = continueUserActivity
.filter { $0.activityType == NSUserActivityTypeBrowsingWeb }
.map { activity in (activity, activity.webpageURL.flatMap(Navigation.match)) }
.filter(second >>> isNotNil)

self.continueUserActivityReturnValue <~ continueUserActivityWithNavigation.mapConst(true)
self.continueUserActivityReturnValue <~ continueUserActivityWithNavigation.map(second >>> isNotNil)

let deepLinkUrl = Signal
.merge(
openUrl.map { $0.url },

self.foundRedirectUrlProperty.signal.skipNil(),

continueUserActivity
.filter { $0.activityType == NSUserActivityTypeBrowsingWeb }
.map { $0.webpageURL }
Expand Down Expand Up @@ -407,7 +404,7 @@ AppDelegateViewModelOutputs {
.filter { $0 == .tab(.me) }
.ignoreValues()

self.goToMobileSafari = self.foundRedirectUrlProperty.signal.skipNil()
self.goToMobileSafari = deepLinkUrl
.filter { Navigation.deepLinkMatch($0) == nil }

let projectLink = deepLink
Expand Down Expand Up @@ -603,6 +600,7 @@ AppDelegateViewModelOutputs {
.observeValues { AppEnvironment.current.koala.trackOpenedAppBanner($0) }

continueUserActivityWithNavigation
.filter(second >>> isNotNil)
.map(first)
.observeValues { AppEnvironment.current.koala.trackUserActivity($0) }

Expand Down
33 changes: 33 additions & 0 deletions Kickstarter-iOS/ViewModels/AppDelegateViewModelTests.swift
Expand Up @@ -1365,6 +1365,39 @@ final class AppDelegateViewModelTests: TestCase {
self.goToMobileSafari.assertValues([unrecognizedUrl], "Go to mobile safari for the unrecognized url.")
}

func testEmailDeepLinking_UnrecognizedUrl_ProjectPreview() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

let emailUrl = URL(string: "https://click.e.kickstarter.com/?qs=deadbeef")!

// The application launches.
self.vm.inputs.applicationDidFinishLaunching(application: UIApplication.shared,
launchOptions: [:])

self.findRedirectUrl.assertValues([])
self.presentViewController.assertValueCount(0)
self.goToMobileSafari.assertValues([])

// We deep-link to an email url.
self.vm.inputs.applicationDidEnterBackground()
self.vm.inputs.applicationWillEnterForeground()
let result = self.vm.inputs.applicationOpenUrl(application: UIApplication.shared,
url: emailUrl,
sourceApplication: nil,
annotation: 1)
XCTAssertFalse(result)

self.findRedirectUrl.assertValues([emailUrl], "Ask to find the redirect after open the email url.")
self.presentViewController.assertValues([], "No view controller is presented.")
self.goToMobileSafari.assertValues([], "Do not go to mobile safari")

// We find the redirect to be an unrecognized url (project preview).
let unrecognizedUrl = URL(string: "https://www.kickstarter.com/projects/creator/project?token=4")!
self.vm.inputs.foundRedirectUrl(unrecognizedUrl)

self.findRedirectUrl.assertValues([emailUrl], "Nothing new is emitted.")
self.presentViewController.assertValues([], "Do not present controller since the url was unrecognizable.")
self.goToMobileSafari.assertValues([unrecognizedUrl], "Go to mobile safari for the unrecognized url.")
}

func testOtherEmailDeepLink() {
let emailUrl = URL(string: "https://email.kickstarter.com/mpss/a/b/c/d/e/f/g")!

Expand Down
18 changes: 17 additions & 1 deletion Library/Navigation.swift
Expand Up @@ -14,6 +14,7 @@ public enum Navigation {
case signup
case tab(Tab)
case project(Param, Navigation.Project, refTag: RefTag?)
case projectPreview(Param, Navigation.Project, refTag: RefTag?, token: String)
case user(Param, Navigation.User)

public enum Checkout {
Expand Down Expand Up @@ -91,6 +92,9 @@ public func == (lhs: Navigation, rhs: Navigation) -> Bool {
return lhs == rhs
case let (.project(lhsParam, lhsProject, lhsRefTag), .project(rhsParam, rhsProject, rhsRefTag)):
return lhsParam == rhsParam && lhsProject == rhsProject && lhsRefTag == rhsRefTag
case let (.projectPreview(lhsParam, lhsProject, lhsRefTag, lhsToken),
.projectPreview(rhsParam, rhsProject, rhsRefTag, rhsToken)):
return lhsParam == rhsParam && lhsProject == rhsProject && lhsRefTag == rhsRefTag && lhsToken == rhsToken
case let (.user(lhsParam, lhsUser), .user(rhsParam, rhsUser)):
return lhsParam == rhsParam && lhsUser == rhsUser
default:
Expand Down Expand Up @@ -401,10 +405,22 @@ private func signup(_: RouteParams) -> Decoded<Navigation> {
}

private func project(_ params: RouteParams) -> Decoded<Navigation> {
return curry(Navigation.project)
let projectPreview = curry(Navigation.projectPreview)
<^> params <| "project_param"
<*> .success(.root)
<*> params <|? "ref"
<*> params <| "token"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Treats token as required for a project preview link.


// If we're certain this is not a project preview link, try to decode it as a normal project link.
if case .failure = projectPreview {
return curry(Navigation.project)
<^> params <| "project_param"
<*> .success(.root)
<*> params <|? "ref"
}

// Fail here as we don't currently support project preview links.
return .failure(.custom("Project preview links are unsupported"))
}

private func thanks(_ params: RouteParams) -> Decoded<Navigation> {
Expand Down
6 changes: 5 additions & 1 deletion Library/NavigationTests.swift
Expand Up @@ -4,7 +4,7 @@ import Prelude
import XCTest
@testable import Library

private func KSRAssertMatch(_ expected: Navigation,
private func KSRAssertMatch(_ expected: Navigation?,
_ path: String,
file: StaticString = #file,
line: UInt = #line) {
Expand Down Expand Up @@ -38,6 +38,10 @@ public final class NavigationTests: XCTestCase {
KSRAssertMatch(.project(.slug("project"), .root, refTag: nil),
"/projects/creator/project")

KSRAssertMatch(nil, "/projects/creator/project?token=4")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just proves that we get a nil match on this type of URL.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wonder whether we should also add KSRAssertMatch(nil, "/projects/creator/project?ref=discovery&token=4") for exhausted url example.


KSRAssertMatch(nil, "/projects/creator/project?ref=discovery&token=4")

KSRAssertMatch(.project(.slug("project"), .checkout(1, .thanks(racing: nil)), refTag: nil),
"/projects/creator/project/checkouts/1/thanks")

Expand Down