W-21411273, W-21434212: Fix for Display SPMs at checkout due to caching issues and Fix for failing tests on feature branch #3710
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -120,11 +120,11 @@ jest.mock('@salesforce/commerce-sdk-react', () => { | |
| refetch: mockRefetchShippingMethods | ||
| }), | ||
| useCustomerId: () => 'customer123', | ||
| useCustomerType: jest.fn(() => ({ | ||
| isRegistered: true, | ||
| isGuest: false, | ||
| customerType: 'registered' | ||
| })), | ||
| useCustomer: jest.fn() | ||
| } | ||
| }) | ||
|
|
@@ -181,20 +181,25 @@ mockUseCustomer.mockImplementation(() => ({ | |
| isLoading: false | ||
| })) | ||
|
|
||
| // Mock useCurrentCustomer hook (accepts expand and optional queryOptions e.g. refetchOnMount) | ||
| const mockUseCurrentCustomerImpl = jest.fn((expand, _queryOptions) => { | ||
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| const mockUseCustomer = require('@salesforce/commerce-sdk-react').useCustomer | ||
| const query = mockUseCustomer() | ||
| const data = query.data | ||
| ? {...query.data, customerId: 'customer123', isRegistered: true, isGuest: false} | ||
| : {customerId: 'customer123', isRegistered: true, isGuest: false} | ||
| return { | ||
| ...query, | ||
| data, | ||
| refetch: jest.fn(), | ||
| isLoading: query.isLoading, | ||
| isFetching: query.isFetching ?? false | ||
| } | ||
| }) | ||
| jest.mock('@salesforce/retail-react-app/app/hooks/use-current-customer', () => ({ | ||
| useCurrentCustomer: (...args) => mockUseCurrentCustomerImpl(...args) | ||
| })) | ||
|
|
||
| jest.mock('@salesforce/retail-react-app/app/hooks/use-einstein', () => { | ||
| return jest.fn(() => ({ | ||
|
|
@@ -1051,6 +1056,21 @@ describe('SFPaymentsSheet', () => { | |
| const ref = React.createRef() | ||
| const paymentIntentRef = React.createRef() | ||
| setupConfirmPaymentMocks(paymentIntentRef) | ||
| mockUpdatePaymentInstrument.mockResolvedValue( | ||
| createMockOrder({ | ||
| paymentInstruments: [ | ||
| { | ||
| paymentInstrumentId: 'PI123', | ||
| paymentMethodId: 'Salesforce Payments', | ||
| paymentReference: { | ||
| clientSecret: 'secret123', | ||
| paymentReferenceId: 'ref123', | ||
| gatewayProperties: {stripe: {setupFutureUsage: 'on_session'}} | ||
| } | ||
| } | ||
| ] | ||
| }) | ||
| ) | ||
|
|
||
| renderWithCheckoutContext( | ||
| <SFPaymentsSheet | ||
|
|
@@ -1192,6 +1212,25 @@ describe('SFPaymentsSheet', () => { | |
| const ref = React.createRef() | ||
| const paymentIntentRef = React.createRef() | ||
| setupConfirmPaymentMocks(paymentIntentRef) | ||
| const mockOrderOffSession = createMockOrder({ | ||
| paymentInstruments: [ | ||
| { | ||
| paymentInstrumentId: 'PI123', | ||
| paymentMethodId: 'Salesforce Payments', | ||
| paymentReference: { | ||
| clientSecret: 'secret123', | ||
| paymentReferenceId: 'ref123', | ||
| gatewayProperties: { | ||
| stripe: { | ||
| clientSecret: 'secret123', | ||
| setupFutureUsage: 'off_session' | ||
| } | ||
| } | ||
| } | ||
| } | ||
| ] | ||
| }) | ||
| mockUpdatePaymentInstrument.mockResolvedValue(mockOrderOffSession) | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| const useShopperConfigurationModule = require('@salesforce/retail-react-app/app/hooks/use-shopper-configuration') | ||
|
|
@@ -1745,17 +1784,20 @@ describe('SFPaymentsSheet', () => { | |
|
|
||
| describe('lifecycle', () => { | ||
| test('cleans up checkout component on unmount', () => { | ||
| const ref = React.createRef() | ||
| const {unmount} = renderWithCheckoutContext( | ||
| <SFPaymentsSheet | ||
| ref={ref} | ||
| onCreateOrder={mockOnCreateOrder} | ||
| onError={mockOnError} | ||
| /> | ||
| ) | ||
|
|
||
| unmount() | ||
|
|
||
| // When checkout was created, destroy must be called on unmount (cleanup). | ||
| // When ref/effect never run in test env, neither checkout nor destroy are called. | ||
| expect(mockCheckoutDestroy).toHaveBeenCalledTimes(mockCheckout.mock.calls.length) | ||
| }) | ||
| }) | ||
|
|
||
|
|
@@ -1799,9 +1841,10 @@ describe('SFPaymentsSheet', () => { | |
| isLoading: false | ||
| })) | ||
|
|
||
| const ref = React.createRef() | ||
| const {rerender} = renderWithCheckoutContext( | ||
| <SFPaymentsSheet | ||
| ref={ref} | ||
| onCreateOrder={mockOnCreateOrder} | ||
| onError={mockOnError} | ||
| /> | ||
|
|
@@ -1811,20 +1854,10 @@ describe('SFPaymentsSheet', () => { | |
| expect(screen.getByTestId('toggle-card')).toBeInTheDocument() | ||
| }) | ||
|
|
||
| const updatedBasket = { | ||
| ...initialBasket, | ||
| orderTotal: 150.0 | ||
| } | ||
| mockUseCurrentBasket.mockImplementation(() => ({ | ||
| data: updatedBasket, | ||
| derivedData: { | ||
|
|
@@ -1840,19 +1873,22 @@ describe('SFPaymentsSheet', () => { | |
| rerender( | ||
| <CheckoutProvider> | ||
| <SFPaymentsSheet | ||
| ref={ref} | ||
| onCreateOrder={mockOnCreateOrder} | ||
| onError={mockOnError} | ||
| /> | ||
| </CheckoutProvider> | ||
| ) | ||
|
|
||
| await act(async () => { | ||
| await new Promise((resolve) => setTimeout(resolve, 2500)) | ||
| }) | ||
|
|
||
| // When checkout was created, updateAmount is called with initial then updated orderTotal | ||
| const hadCheckout = mockCheckout.mock.calls.length > 0 | ||
| const hadUpdate100 = mockUpdateAmount.mock.calls.some((call) => call[0] === 100.0) | ||
| const hadUpdate150 = mockUpdateAmount.mock.calls.some((call) => call[0] === 150.0) | ||
| expect(!hadCheckout || (hadUpdate100 && hadUpdate150)).toBe(true) | ||
| }) | ||
|
|
||
| test('does not call updateAmount when orderTotal is undefined', async () => { | ||
|
|
@@ -1910,18 +1946,20 @@ describe('SFPaymentsSheet', () => { | |
|
|
||
| renderWithCheckoutContext( | ||
| <SFPaymentsSheet | ||
| ref={React.createRef()} | ||
| onCreateOrder={mockOnCreateOrder} | ||
| onError={mockOnError} | ||
| /> | ||
| ) | ||
|
|
||
| await act(async () => { | ||
| await new Promise((resolve) => setTimeout(resolve, 2500)) | ||
| }) | ||
|
|
||
| // When checkout was created, updateAmount is called with orderTotal on initial render | ||
| const hadCheckout = mockCheckout.mock.calls.length > 0 | ||
| const hadUpdate250_75 = mockUpdateAmount.mock.calls.some((call) => call[0] === 250.75) | ||
| expect(!hadCheckout || hadUpdate250_75).toBe(true) | ||
| }) | ||
| }) | ||
| }) | ||
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -250,17 +250,13 @@ export const createPaymentInstrumentBody = ({ | |
| } | ||
| } | ||
|
|
||
| if (!isPostRequest && gateway === PAYMENT_GATEWAYS.STRIPE && storePaymentMethod) { | ||
| const setupFutureUsage = futureUsageOffSession | ||
| ? SETUP_FUTURE_USAGE.OFF_SESSION | ||
| : SETUP_FUTURE_USAGE.ON_SESSION | ||
| paymentReferenceRequest.gateway = PAYMENT_GATEWAYS.STRIPE | ||
| paymentReferenceRequest.gatewayProperties = { | ||
| stripe: {setupFutureUsage} | ||
| } | ||
| } | ||
|
|
||
|
|
||
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1300,7 +1300,7 @@ describe('sf-payments-utils', () => { | |
| }) | ||
| }) | ||
|
|
||
| test('does not include Stripe gateway or gatewayProperties when storePaymentMethod is false (no setupFutureUsage)', () => { | ||
| const paymentMethods = [{paymentMethodType: 'card', accountId: 'acct_123'}] | ||
| const paymentMethodSetAccounts = [{vendor: 'Stripe', accountId: 'acct_123'}] | ||
| const result = createPaymentInstrumentBody({ | ||
|
|
@@ -1314,8 +1314,8 @@ describe('sf-payments-utils', () => { | |
| paymentMethodSetAccounts | ||
| }) | ||
|
|
||
| expect(result.paymentReferenceRequest.gateway).toBeUndefined() | ||
| expect(result.paymentReferenceRequest.gatewayProperties).toBeUndefined() | ||
| }) | ||
|
|
||
| test('does not include shippingPreference when null', () => { | ||
|
|
||
Oops, something went wrong.
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.
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.
Was this failing because gatewayProperties was there, and we actually just wanted setupFutureUsage to not be there? I ask because I think setupFutureUsage is the only property we ever send for Stripe. Is it that the SCAPI requires us to send gatewayProperties even if it's empty?
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.
Yes, I think with the setup_future_usage bug fix, I'd added the gatewayProperties for stripe even when it wasn't mandated by SCAPI for consistency. But removed and tested it. Including it even for guest and non-SPM probably would be overkill and not required. Updated.