-
Notifications
You must be signed in to change notification settings - Fork 237
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
Adding imageUrl, upcType, upcCode to PayPalLineItem #871
Changes from 2 commits
bacfe47
940340b
a8fd7ee
d2543d8
b1dbb15
0344fb3
39d3713
5c133de
dc59edf
468888f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -29,6 +29,22 @@ public class PayPalNativeCheckoutLineItem implements Parcelable { | |||||
public static final String KIND_CREDIT = "credit"; | ||||||
public static final String KIND_DEBIT = "debit"; | ||||||
|
||||||
/** | ||||||
* The upc type of PayPal line item. | ||||||
*/ | ||||||
@Retention(RetentionPolicy.SOURCE) | ||||||
@StringDef({PayPalLineItem.UPC_TYPE_A, PayPalLineItem.UPC_TYPE_B, PayPalLineItem.UPC_TYPE_C, PayPalLineItem.UPC_TYPE_D, PayPalLineItem.UPC_TYPE_E, PayPalLineItem.UPC_TYPE_2, PayPalLineItem.UPC_TYPE_5}) | ||||||
@interface PayPalLineItemUpcType { | ||||||
} | ||||||
|
||||||
public static final String UPC_TYPE_A = "UPC-A"; | ||||||
public static final String UPC_TYPE_B = "UPC-B"; | ||||||
public static final String UPC_TYPE_C = "UPC-C"; | ||||||
public static final String UPC_TYPE_D = "UPC-D"; | ||||||
public static final String UPC_TYPE_E = "UPC-E"; | ||||||
public static final String UPC_TYPE_2 = "UPC-2"; | ||||||
public static final String UPC_TYPE_5 = "UPC-5"; | ||||||
|
||||||
private static final String DESCRIPTION_KEY = "description"; | ||||||
private static final String KIND_KEY = "kind"; | ||||||
private static final String NAME_KEY = "name"; | ||||||
|
@@ -37,6 +53,9 @@ public class PayPalNativeCheckoutLineItem implements Parcelable { | |||||
private static final String UNIT_AMOUNT_KEY = "unit_amount"; | ||||||
private static final String UNIT_TAX_AMOUNT_KEY = "unit_tax_amount"; | ||||||
private static final String URL_KEY = "url"; | ||||||
private static final String IMAGE_URL_KEY = "image_url"; | ||||||
private static final String UPC_CODE_KEY = "upc_code"; | ||||||
private static final String UPC_TYPE_KEY = "upc_type"; | ||||||
|
||||||
private String description; | ||||||
private String kind; | ||||||
|
@@ -46,6 +65,9 @@ public class PayPalNativeCheckoutLineItem implements Parcelable { | |||||
private String unitAmount; | ||||||
private String unitTaxAmount; | ||||||
private String url; | ||||||
private String imageUrl; | ||||||
private String upcCode; | ||||||
private String upcType; | ||||||
|
||||||
/** | ||||||
* Constructs a line item for PayPal checkout flows. All parameters are required. | ||||||
|
@@ -137,6 +159,33 @@ public void setUrl(@NonNull String url) { | |||||
this.url = url; | ||||||
} | ||||||
|
||||||
/** | ||||||
* The image URL to product information. | ||||||
* | ||||||
* @param imageURL The image URL with additional information. | ||||||
*/ | ||||||
public void setImageUrl(@NonNull String imageUrl) { | ||||||
this.imageUrl = imageUrl; | ||||||
} | ||||||
|
||||||
/** | ||||||
* UPC code of the item. | ||||||
* | ||||||
* @param upcCode The UPC code. | ||||||
*/ | ||||||
public void setUpcCode(@NonNull String upcCode) { | ||||||
this.upcCode = upcCode; | ||||||
} | ||||||
|
||||||
/** | ||||||
* UPC type of the item. | ||||||
* | ||||||
* @param upcType The UPC type. | ||||||
*/ | ||||||
public void setUpcType(@NonNull String upcType) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added interface type to setter and getter annotations |
||||||
this.upcType = upcType; | ||||||
} | ||||||
|
||||||
@Nullable | ||||||
public String getDescription() { | ||||||
return description; | ||||||
|
@@ -178,6 +227,21 @@ public String getUrl() { | |||||
return url; | ||||||
} | ||||||
|
||||||
@Nullable | ||||||
public String getImageUrl() { | ||||||
return imageUrl; | ||||||
} | ||||||
|
||||||
@Nullable | ||||||
public String getUpcCode() { | ||||||
return upcCode; | ||||||
} | ||||||
|
||||||
@Nullable | ||||||
public String getUpcType() { | ||||||
return upcType; | ||||||
} | ||||||
|
||||||
public JSONObject toJson() { | ||||||
try { | ||||||
return new JSONObject() | ||||||
|
@@ -188,7 +252,10 @@ public JSONObject toJson() { | |||||
.putOpt(QUANTITY_KEY, quantity) | ||||||
.putOpt(UNIT_AMOUNT_KEY, unitAmount) | ||||||
.putOpt(UNIT_TAX_AMOUNT_KEY, unitTaxAmount) | ||||||
.putOpt(URL_KEY, url); | ||||||
.putOpt(URL_KEY, url) | ||||||
.putOpt(IMAGE_UTL_KRY, imageUrl) | ||||||
.putOpt(UPC_CODE_KEY, upcCode) | ||||||
.putOpt(UPC_TYPE_KRY, upcType); | ||||||
} catch (JSONException ignored) { | ||||||
} | ||||||
|
||||||
|
@@ -204,6 +271,9 @@ public JSONObject toJson() { | |||||
unitAmount = in.readString(); | ||||||
unitTaxAmount = in.readString(); | ||||||
url = in.readString(); | ||||||
imageUrl = in.readString(); | ||||||
upcCode = in.readString(); | ||||||
upcType = in.readString(); | ||||||
} | ||||||
|
||||||
public static final Creator<PayPalNativeCheckoutLineItem> CREATOR = new Creator<PayPalNativeCheckoutLineItem>() { | ||||||
|
@@ -233,6 +303,9 @@ public void writeToParcel(Parcel parcel, int i) { | |||||
parcel.writeString(unitAmount); | ||||||
parcel.writeString(unitTaxAmount); | ||||||
parcel.writeString(url); | ||||||
parcel.writeString(imageUrl); | ||||||
parcel.writeString(upcCode); | ||||||
parcel.writeSting(upcType); | ||||||
} | ||||||
|
||||||
} |
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'd actually go for
@Nullable
for these properties to align the getter/setter annotations.upcType: String?
makes sense since it's initiallynull
.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.
Currently all the existing properties have @nonnull for setters and @nullable for getters, as these are not required fields and merchant may or may not use setter methods. We didn't see any issues so far.
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 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 setlineItem.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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated setter method annotation from
@NonNull
to@nullable
to keep it consistent with getter method annotations