Skip to content
This repository has been archived by the owner on May 24, 2023. It is now read-only.

Add support for prefix productName for paypal item #166

Merged
merged 16 commits into from
Mar 30, 2020
Merged

Conversation

JudeNiroshan
Copy link
Contributor

Description

New support for prefix the product name in paypal is supported. This can be enabled/disabled from a new attribute introduced in application.yml

@JudeNiroshan
Copy link
Contributor Author

Need to add the new property of prefixProductNameWithAttr to the test application.yml file

@JudeNiroshan
Copy link
Contributor Author

@lojzatran Can you provide me the encrypted application.yml that is being used for travisCI builds? I need to add the new property that I introduced into it. 😁

@lojzatran
Copy link
Contributor

@lojzatran Can you provide me the encrypted application.yml that is being used for travisCI builds? I need to add the new property that I introduced into it. 😁

Do you mean this file? https://github.com/commercetools/commercetools-paypal-plus-integration/blob/master/travis-build/configuration/travis-build-settings.sh.enc

docs/MigrationGuide.md Outdated Show resolved Hide resolved
docs/MigrationGuide.md Outdated Show resolved Hide resolved
@@ -240,27 +255,22 @@ protected Details getTransactionDetails(@Nonnull CtpPaymentWithCart paymentWithC
protected Stream<Item> mapCustomLineItemToPaypalPlusItem(@Nonnull CustomLineItem customLineItem, @Nonnull List<Locale> locales) {
if (customLineItem.getDiscountedPricePerQuantity().size() > 0) {
return customLineItem.getDiscountedPricePerQuantity().stream()
.map(dlipfq -> createPaypalPlusItem(customLineItem.getName(), locales, dlipfq));
.map(dlipfq -> createPaypalPlusItem(customLineItem.getName().get(locales), dlipfq));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you pass here not a concrete locale but a list of locales which locale will it take? Also, why do you pass a list here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was the previous implementation to use a List<Locale>. In this case, the first matching locale will be used.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does it match exactly or it just takes the first available locale?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should take by default always a language defined as languageCode custom field on the payment (it is a required field for the integration: https://github.com/commercetools/commercetools-paypal-plus-integration) and only fall back to current behaviour if no values for languageCode found or languageCode is not defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JudeNiroshan JudeNiroshan requested a review from butenkor March 24, 2020 13:07
@JudeNiroshan JudeNiroshan removed the request for review from lojzatran March 24, 2020 14:20
Copy link

@ahmetoz ahmetoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after a quick review, looks OK to me, please double-check the missing things.

@JudeNiroshan JudeNiroshan merged commit 7b0e803 into master Mar 30, 2020
@JudeNiroshan JudeNiroshan deleted the ZEG-690 branch March 30, 2020 10:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants