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.
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
Cleanup App Switch #1381
Cleanup App Switch #1381
Changes from all commits
b479e9b
407e758
d2dce53
7d21b2c
fbd98a8
a3ce334
231e0b0
e4f4484
893d736
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
👍
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.
For EditFI work, we are not using
payPalAppApprovalUrl
because app switching is not in scope yet for that work stream. But if the gateway returns this value as well as approvalURL ingenerate_edit_fi_url
response, using this Parser we would getpayPalAppRedirectURL
instead of theapprovalURL
. For right now for the draft on a branch (editFI-gateway-endpoints), I just forced the web Url fetching by passing in linkType as nil (or could have passed in "deepLink" ). I guess I can use something else to get the webURL.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.
Is the gateway returning a
payPalAppApprovalUrl
for the edit FI endpoints? Because they should not be and we should raise that if so. This was only ever added because when we were working off of stage we always got apayPalAppApprovalUrl
regardless of eligibility and was just never cleaned up. See old convo here: #1271 (comment). I tested the experience in this PR and no longer see this behavior with the app uninstalled.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.
The Gateway specs on Confluence show: Response - agreementSetup {tokenId, paypalAppApprovalUrl, approvalUrl}
as response for edit FI url endpoint. Maybe the doc is not updated or maybe it's supposed to return just approval url. I will check with you and Gateway team tomorrow.
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.
Okay, then removing the link type will not impact that flow and we should not need to pass in a link type, it will always fail this first check for a
payPalAppApprovalUrl
and move on to the next block which includes the web URL.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.
I've posted the question on the collaboration channel so we can clarify with them.
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.
Fwiw we also have to opt in to the app switch flow for the Gateway to return that property, so I would expect it only exists to future proof this feature. We need to pass them everything here to say the merchant opted into app switch which we won't be doing for Edit FI currently:
braintree_ios/Sources/BraintreePayPal/BTPayPalVaultRequest.swift
Lines 56 to 64 in 38297e4
The gateway only returns a
payPalAppApprovalUrl
if those properties are passed AND the merchant is eligible on the gateway side.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.
Great! thank you for the clarification. I do want to make sure that this is true for editFI endpoint as well since there are no details given in the specs. I want to clarify that
payPalAppApprovalUrl
will return a nil value or not exist at all forgenerate_edit_fi_url
endpoint if these parameters are not passed. And these parameters are not mentioned forgenerate_edi_fi_url
endpoint.What you are saying makes sense though.
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.
For sure! Will be good to clarify but shouldn't block this PR. If the gateway does something weird we can make the necessary updates in the Edit FI branch.