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
Eligibility API logic #286
Eligibility API logic #286
Changes from all commits
2a07770
f50baa8
e564f3e
193ab5c
de28ce6
fc37fd6
de7bf1a
e0573a1
c0000b4
8a9f7e0
fa4863c
e366890
bfd4749
a3000a1
f63935d
fdc17bf
26b72e4
7fb6765
8daa52b
4198535
c1b1a75
3d41794
7ff6bb1
1a34e59
4a87950
d8b3b76
805304d
8dbf00c
32dddf8
65749c2
fed4f0c
a5aec4c
64c6cee
2408217
3a0c83a
f6de949
2e15a23
2cd8c4b
2ede375
cecce87
3be4ecb
1ee3d2b
65ebc50
cd016e3
c9f14ab
ee7e160
9a6dc5a
54e53c0
df51eb0
8f253cf
644df70
f9f2df3
b422115
33d3d46
fb550e2
19239f9
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.
I don't think we really need this to be in an extension here. Was there a particular reason for using it?
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 decided to put the query in an extension to make the code more readable. I based it on this implementation where the query is in the same method, which makes the method much larger. Basically, it's to separate(ish) responsibilities
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.
We could also throw it in a private method under
check
if we want to separate it but move it out of the extensionThere 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.
👏 Nice docstrings
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 added this enum based on the logic that was previously removed:
paypal-ios/Sources/CorePayments/Networking/GraphQL/FundingEligibilityIntent.swift
Line 1 in 257df31
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 wonder if we only support
CAPTURE
andAUTHORIZE
. Per these PP docs, these are the only 2 you can create an orderID with.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 think it would be OK to just start with those 2. Also needs a docstring since public
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.
🤔 docs says
SALE
andAUTHORIZE
, thanks for your feedbackThere 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.
Updated, I also changed cases to lowercase to be more
Swifty
: 54e53c0