[QL] Consolidate Return URL Logic #1271
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,84 @@ | ||||||
| import Foundation | ||||||
|
|
||||||
| #if canImport(BraintreeCore) | ||||||
| import BraintreeCore | ||||||
| #endif | ||||||
|
|
||||||
| enum BTPayPalAppSwitchReturnURLState { | ||||||
jaxdesmarais marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| case unknownPath | ||||||
| case succeeded | ||||||
| case canceled | ||||||
|
There was a problem hiding this comment. small nit:
Suggested change
There was a problem hiding this comment. Our SDKs always use the 1 L canceled/cancelation |
||||||
| } | ||||||
|
|
||||||
| /// This class interprets URLs received from the PayPal app via app switch returns. | ||||||
| /// | ||||||
| /// PayPal app switch authorization requests should result in success or user-initiated cancelation. These states are communicated in the url. | ||||||
| struct BTPayPalReturnURL { | ||||||
|
There was a problem hiding this comment. This file was renamed but is showing additional diff. Adding logic for the web flow in the |
||||||
|
|
||||||
| /// The overall status of the app switch - success, cancelation, or an unknown path | ||||||
| var state: BTPayPalAppSwitchReturnURLState = .unknownPath | ||||||
|
|
||||||
| /// Initializes a new `BTPayPalAppSwitchReturnURL` | ||||||
jaxdesmarais marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| /// - Parameter url: an incoming app switch url | ||||||
| init?(_ redirectType: PayPalRedirectType) { | ||||||
| switch redirectType { | ||||||
| case .payPalApp(let url), .webBrowser(let url): | ||||||
| if url.path.contains("success") { | ||||||
| state = .succeeded | ||||||
| } else if url.path.contains("cancel") { | ||||||
| state = .canceled | ||||||
| } else { | ||||||
| state = .unknownPath | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // MARK: - Static Methods | ||||||
|
|
||||||
| /// Evaluates whether the url represents a valid PayPal return URL. | ||||||
| /// - Parameter url: an app switch return URL | ||||||
jaxdesmarais marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| /// - Returns: `true` if the url represents a valid PayPal app switch return | ||||||
jaxdesmarais marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| static func isValid(_ url: URL) -> Bool { | ||||||
| url.scheme == "https" && (url.path.contains("cancel") || url.path.contains("success")) | ||||||
| } | ||||||
|
|
||||||
| static func isValidURLAction(url: URL, linkType: String?) -> Bool { | ||||||
| guard let host = url.host, let scheme = url.scheme, !scheme.isEmpty else { | ||||||
| return false | ||||||
| } | ||||||
|
|
||||||
| var hostAndPath = host | ||||||
| .appending(url.path) | ||||||
| .components(separatedBy: "/") | ||||||
| .dropLast(1) // remove the action (`success`, `cancel`, etc) | ||||||
| .joined(separator: "/") | ||||||
|
|
||||||
| if hostAndPath.count > 0 { | ||||||
| hostAndPath.append("/") | ||||||
| } | ||||||
|
|
||||||
| /// If we are using the deeplink/ASWeb based PayPal flow we want to check that the host and path matches | ||||||
| /// the static callbackURLHostAndPath. For the universal link flow we do not care about this check. | ||||||
| if hostAndPath != BTPayPalRequest.callbackURLHostAndPath && linkType == "deeplink" { | ||||||
|
Comment on lines
+60
to
+62
There was a problem hiding this comment. @agedd I know you mentioned this logic for this specific if block being confusing before. I added the docstrings as well as passing/checking the There was a problem hiding this comment. this looks great! @jaxdesmarais imo definitely a lot more clearer to read :) |
||||||
| return false | ||||||
| } | ||||||
|
|
||||||
| guard let action = action(from: url), | ||||||
| let query = url.query, | ||||||
| query.count > 0, | ||||||
| action.count >= 0, | ||||||
| ["success", "cancel", "authenticate"].contains(action) else { | ||||||
| return false | ||||||
| } | ||||||
|
|
||||||
| return true | ||||||
| } | ||||||
|
|
||||||
| static func action(from url: URL) -> String? { | ||||||
| guard let action = url.lastPathComponent.components(separatedBy: "?").first, !action.isEmpty else { | ||||||
| return url.host | ||||||
| } | ||||||
|
|
||||||
| return action | ||||||
| } | ||||||
| } | ||||||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit for another time - maybe another indicator we should move this to an enum
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I don't think the
BTPayPalApprovalURLParserclass should care if the merchant has opted into universal links or not. It's only determining which URL type it got back from the GW response.The flow logic for falling back to the web flow already is taken care of in
BTPayPalClient.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As of Thursday, we were still getting the
paypalAppApprovalUrlfrom the Gateway regardless of theBTPayPalClient. This may have been because we were working with a mock and an update may have been deployed to just allow our client side handling. Worth digging into but I was still getting this URL with thePPBetaapp uninstalled.