Adding imageUrl, upcType, upcCode to PayPalLineItem #871
Conversation
PayPal/src/main/java/com/braintreepayments/api/PayPalLineItem.java
Outdated
Show resolved
Hide resolved
| * | ||
| * @param upcType The UPC type. | ||
| */ | ||
| public void setUpcType(@NonNull String upcType) { |
There was a problem hiding this comment.
| public void setUpcType(@NonNull String upcType) { | |
| public void setUpcType(@Nullable String upcType) { |
I'd actually go for @Nullable for these properties to align the getter/setter annotations. upcType: String? makes sense since it's initially null.
There was a problem hiding this comment.
I think that must have been an oversight when those properties were added. When these annotations don't match up, the Kotlin property setters don't seem to work as expected (ex: lineItem.upcType = "some type" won't work in Kotlin, they would have to set lineItem.setUpcType("some type")), which isn't the ideal Kotlin integration experience. Although I'm not certain if newer versions of Kotlin have resolved this issue.
There was a problem hiding this comment.
Oh cool yeah if it does work in newer versions of Kotlin i'm 👍 . We should check though. The way it's written now makes total sense to me, but I have seen Kotlin get weird when the annotations aren't aligned.
There was a problem hiding this comment.
I just confirmed with the v5 branch that uses the latest Kotlin that this issue still exists 😞 - I created a ticket to update the annotations in v5 where they don't currently align, but we should add these new properties with matching annotations.
There was a problem hiding this comment.
Updated setter method annotation from @NonNull to @nullable to keep it consistent with getter method annotations
vankads
changed the title
vankads
changed the title
CHANGELOG.md
Outdated
| * BraintreePayPal | ||
| * Add following properties to PayPalLineItem | ||
| * imageUrl | ||
| * upcCode | ||
| * upcType |
There was a problem hiding this comment.
| * BraintreePayPal | |
| * Add following properties to PayPalLineItem | |
| * imageUrl | |
| * upcCode | |
| * upcType | |
| * PayPal | |
| * Add `imageUrl`, `upcCode`, and `upcType` to `PayPalLineItem` |
Nitpick but keeps the changelog a little more concise
| * | ||
| * @param upcType The UPC type. | ||
| */ | ||
| public void setUpcType(@NonNull String upcType) { |
There was a problem hiding this comment.
| public void setUpcType(@NonNull String upcType) { | |
| public void setUpcType(@NonNull @PayPalLineItemUpcType String upcType) { |
This type should be added here and in the getter right (and in doc string too probably)? Or is that interface used somewhere else?
There was a problem hiding this comment.
Added interface type to setter and getter annotations
* v5: Remove LocalPaymentBrowserSwitchRequest (#900) Remove SEPADirectDebitBrowserSwitchRequest and Kotlin Conversion (#901) Remove PayPalBrowserSwitchRequest and Kotlin Conversion (#899) Update LocalPaymentLauncher for Browser Switch v3 (#897) Drop -m drop FPTI v1/tracking/batch/events URL (#896) PayPalLauncher with v3 BrowserSwitchClient (#883) SEPADirectDebitLauncher with v3 BrowserSwitchClient (#893) Prepare for development Release 4.41.0 Update Venmo demo to send MULTI_USE when vault is enabled (#890) BraintreeClient Method Cleanup (#887) Update PULL_REQUEST_TEMPLATE.MD Adding imageUrl, upcType, upcCode to PayPalLineItem (#871) Update mxo 1.2.1 (#881) Update README.md # Conflicts: # LocalPayment/src/main/java/com/braintreepayments/api/LocalPaymentClient.java
Thank you for your contribution to Braintree.
Summary of changes
Checklist
Authors