Support partial payments #310
Merged
Conversation
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
LCOV of commit
|
LCOV of commit
|
LCOV of commit
|
Robert-SD
commented
Nov 21, 2024
There was a problem hiding this comment.
Generated code - no review required.
LCOV of commit
|
Robert-SD
commented
Nov 21, 2024
There was a problem hiding this comment.
Generated code - no review required.
LCOV of commit
|
LCOV of commit
|
LCOV of commit
|
Robert-SD
commented
Nov 21, 2024
ios/Classes/PlatformApi.swift
Outdated
There was a problem hiding this comment.
Generated code - no review required.
LCOV of commit
|
LCOV of commit
|
Robert-SD
force-pushed
the
feature/addPartialPayments
branch
from
d79c350 to
1307490
Compare
LCOV of commit
|
Robert-SD
force-pushed
the
feature/addPartialPayments
branch
from
1307490 to
9cc78f6
Compare
LCOV of commit
|
LCOV of commit
|
atmamont
reviewed
Dec 16, 2024
|
|
||
| private func removeGiftCardPaymentMethods(paymentMethods: PaymentMethods) -> PaymentMethods { | ||
| private func removeGiftCardPaymentMethods(paymentMethods: PaymentMethods, isPartialPaymentSupported: Bool) -> PaymentMethods { | ||
| if isPartialPaymentSupported { |
There was a problem hiding this comment.
Just to be more Swifty it could be guard as a common syntax for early exits.
| @@ -51,12 +51,14 @@ class InstantAdvancedComponent: BaseInstantComponent, InstantComponentProtocol { | |||
| onError(paymentEventDTO: paymentEventDTO) | |||
| case .action: | |||
| onAction(paymentEventDTO: paymentEventDTO) | |||
| case .update: | |||
| return | |||
There was a problem hiding this comment.
Some components silently return for case update. Apple Pay has a dedicated comment for code clarity, just an observation, maybe it is worth to add same comments?
There was a problem hiding this comment.
Good suggestion, will add a comment here as well to enhance code clarity.
android/src/main/kotlin/com/adyen/checkout/flutter/dropIn/DropInPlatformApi.kt
Show resolved
Hide resolved
|
|
||
| val balanceResult = mapToBalanceDropInServiceResult(message.contentIfNotHandled as String) | ||
| lifecycleScope.launch { | ||
| delay(300) |
There was a problem hiding this comment.
Is this delay necessary?
android/src/main/kotlin/com/adyen/checkout/flutter/dropIn/advanced/AdvancedDropInService.kt
Show resolved
Hide resolved
LCOV of commit
|
araratthehero
previously approved these changes
Dec 18, 2024
atmamont
previously approved these changes
Dec 19, 2024
Robert-SD
dismissed stale reviews from atmamont and araratthehero
via
2b05fee
LCOV of commit
|
Robert-SD
commented
Dec 19, 2024
There was a problem hiding this comment.
Generated code. No review required.
descorp
reviewed
Dec 19, 2024
Comment on lines
281
to
283
| guard !isPartialPaymentSupported else { | ||
| return paymentMethods | ||
| } |
There was a problem hiding this comment.
I think this one is hard to read now 😅
Simple IF is better, since guard is for validation or edge-casese and here it is a pure Business logic.
if isPartialPaymentSupported {
return paymentMethods
}
descorp
reviewed
Dec 19, 2024
| @@ -69,7 +69,7 @@ class AdyenCheckoutSession { | |||
| InstantPaymentType.applePay, | |||
| ); | |||
| } else if (configuration is DropInConfiguration) { | |||
| return configuration.toDTO(sdkVersionNumber); | |||
| return configuration.toDTO(sdkVersionNumber, true); | |||
There was a problem hiding this comment.
Maybe make this one a constant? Like:
configuration.toDTO(sdkVersionNumber, alwaysSupportPartialPayments);
LCOV of commit
|
descorp
approved these changes
Dec 19, 2024
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.