NCPS v2 👾 #2389

spencersablan · 2024-05-09T22:05:29Z

Description

Note

Very sorry this PR became a bit larger than originally expected. I'll try to leave some comments around to point out some of the main parts.

🤝 Support for NCP form fields API v2 - Support for v2 without losing support for v1.
📝 Extensive testing - Added / adjusted a ton of unit tests to account for the new API.
🙅‍♂️ Removal for .setAttribute() - We had merchants concerned with .setAttribute() as it violates some CSP's.
🦆 **Updated some syntax in utils.js to make it slightly more consistent ** - I noticed sometimes we were using function name() {} and sometimes we were using const name = () => {}... TBH, I'd rather use function, but it seems like we use const more, so I went with that. Would love some opinions here 😄

Why are we making these changes? Include references to any related Jira tasks or GitHub Issues

JIRA: DTPPCPSDK-2228 & DTPPCPSDK-2229

Reproduction Steps (if applicable)

Setup

Pull this branch to local
In clientsdknodeweb's package.json, under `proxy`, change `/ncp` to `"https://www.te-ncp-js-sdk.qa.paypal.com"` (this will give you the v2 NCP Form Fields API)
Run CSNW & SCNW using `npm run debug`.

Creating a local no-code integration

Navigate to https://www.msmaster.qa.paypal.com/buttons/smart

Sign in using an msmaster developer account

Build any integration you'd like and copy the code

Add that code to an html file and launch it in your browser.

Screenshots (if applicable)

Important

The styles are a bit off in these screenshots since the NCP team has not included finalized styles in their API response. So ignore the lack of margin between our buttons & their "checkout" button. 😄

Click through to see the different cases:

Prefers only PayPal button

preferences: {  
    button_preferences: [ "paypal" ], 
    eligible_funding_methods: ["paypal",  "paylater" ]
}

Prefers PayPal & Venmo, both are eligible.

preferences: {  
    button_preferences: [ "paypal", "venmo" ], 
    eligible_funding_methods: ["paypal", "venmo", "paylater" ]
}

Prefers PayPal & PayLater, both are eligible.

preferences: {  
    button_preferences: [ "paypal", "paylater" ], 
    eligible_funding_methods: ["paypal", "venmo", "paylater" ]
}

Prefers PayPal & Venmo, Venmo not eligible. (Shows only PayPal)

preferences: {  
    button_preferences: [ "paypal", "venmo" ], 
    eligible_funding_methods: ["paypal",  "paylater" ]
}

Prefers PayPal & default second button.

preferences: {  
    button_preferences: [ "paypal", "venmo" ], 
    eligible_funding_methods: ["paypal",  "venmo", "paylater" ]
}

Prefers PayPal & default second button, but all second buttons fail Buttons().isEligible() check. (Shows only PayPal)

preferences: {  
    button_preferences: [ "paypal", "default" ], 
    eligible_funding_methods: ["paypal",  "paylater" ]
}
... 
Buttons({ fundingSource: "paylater" }).isEligible() -> false 😱

Horizontal layout example (S/O @nikrom17 )

preferences: {  
    button_preferences: [ "paypal", "venmo" ], 
    eligible_funding_methods: ["paypal",  "venmo", "paylater" ]
}

❤️ Thank you!

…ing standalone button

… # in id's

spencersablan

Here's a couple of helpful notes as you review this PR 😄 ⬇️

src/hosted-buttons/index.js

src/hosted-buttons/index.test.js

src/hosted-buttons/utils.js

nikrom17

This is looking pretty good so far 🙌

src/hosted-buttons/utils.js

src/hosted-buttons/index.js

src/hosted-buttons/utils.js

imbrian · 2024-05-16T15:49:08Z

src/hosted-buttons/index.js

@@ -63,30 +68,25 @@ export const getHostedButtonsComponent = (): HostedButtonsComponent => {
        style,
      };

-      if (shouldRenderSDKButtons(fundingSources)) {
+      if (version === "2") {


Do we anticipate additional major API versions frequently needing to maintain backwards compatibility like this?

from what we've been told, this is an evergreen experience so any major changes would be a change in the sdk itself.

This version check is more like a feature-flag so we don't break the current experience while we work on the new one.

Good question! I think once v2 is completely rolled out, we can remove this check and all the v1 handling. I should totally add a comment for this.

sounds good! my concern was maintaining this version-based model of backwards compatibility with future versions, but doesn't sound like an issue here 🚀

src/hosted-buttons/utils.js

imbrian · 2024-05-16T16:13:03Z

src/hosted-buttons/utils.test.js

@@ -602,4 +588,234 @@ test("getElementFromSelector", () => {
  expect(mockQuerySelector).toHaveBeenCalledWith(containerId);
 });

+describe("getButtonPreferences", () => {


nice test coverage

nikrom17

Looks great!

Spencer Sablan added 16 commits May 7, 2024 14:03

Add types

53338ca

Add util functions for getting preferences, applying styles, & render…

9566edd

…ing standalone button

Implement new functions into HostedButtons component

fafc038

Remove deleted util functions & tests

964f9ca

A couple fixes for the linter

b219180

Refactor to make getButtonPreferences more concise

9ff8f1f

Add tests for getButtonPreferences & getDefaultButton

2f8bc7e

Fixup some tests for NCP v1

0f9584d

Fix filter / update types

df0b415

Add tests for renderStandaloneButtons

722ed3a

Add applyContainerStyles tests

96de08d

Add # to id in querySelector

3d576ad

Update tests for HostedButtons v2

88d3ee8

Update getButtonPreferences to not filter out 'default'

9ff3c1c

Update container name / fix id # / add conditional

0dd03d5

Add error handling to getButtonPreferences / fix tests to account for…

8dc54b7

… # in id's

spencersablan marked this pull request as ready for review May 9, 2024 22:58

spencersablan requested a review from a team as a code owner May 9, 2024 22:58

spencersablan commented May 9, 2024

View reviewed changes

src/hosted-buttons/index.js Outdated Show resolved Hide resolved

src/hosted-buttons/index.test.js Show resolved Hide resolved

src/hosted-buttons/utils.js Show resolved Hide resolved

src/hosted-buttons/utils.js Outdated Show resolved Hide resolved

nikrom17 reviewed May 10, 2024

View reviewed changes

Spencer Sablan added 5 commits May 10, 2024 14:32

Code review changes pt I

3e7e516

Code review pt II: log errors when throwing

d599565

Fix error tests / update error names for applyContainerStyles

7da8159

refactor button render to handle default preference separately / tests

c388036

Fix type issue in tests / change test name

96f0d74

imbrian reviewed May 16, 2024

View reviewed changes

nikrom17 approved these changes May 17, 2024

View reviewed changes

Change applyContainerStyles return type

865fc9d

imbrian approved these changes May 21, 2024

View reviewed changes

spencersablan merged commit c849191 into main May 21, 2024
4 checks passed

spencersablan deleted the feature/ncps-preferences branch May 21, 2024 21:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NCPS v2 👾 #2389

NCPS v2 👾 #2389

spencersablan commented May 9, 2024 •

edited

spencersablan left a comment

nikrom17 left a comment

imbrian May 16, 2024

wsbrunson May 16, 2024

spencersablan May 16, 2024

imbrian May 16, 2024

imbrian May 16, 2024

nikrom17 left a comment

NCPS v2 👾 #2389

NCPS v2 👾 #2389

Conversation

spencersablan commented May 9, 2024 • edited

Description

Why are we making these changes? Include references to any related Jira tasks or GitHub Issues

Reproduction Steps (if applicable)

Screenshots (if applicable)

Click through to see the different cases:

spencersablan left a comment

Choose a reason for hiding this comment

nikrom17 left a comment

Choose a reason for hiding this comment

imbrian May 16, 2024

Choose a reason for hiding this comment

wsbrunson May 16, 2024

Choose a reason for hiding this comment

spencersablan May 16, 2024

Choose a reason for hiding this comment

imbrian May 16, 2024

Choose a reason for hiding this comment

imbrian May 16, 2024

Choose a reason for hiding this comment

nikrom17 left a comment

Choose a reason for hiding this comment

spencersablan commented May 9, 2024 •

edited