Skip to content

Commit 6b81bc9

Browse files
amittapalliamittapalli
authored
W-21432256 Address code review comments (#3715)
* W-21432256: address code review comments from PWA team
1 parent 37a7237 commit 6b81bc9

File tree

14 files changed

+174
-151
lines changed

14 files changed

+174
-151
lines changed

‎packages/commerce-sdk-react/CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
## v5.1.0-dev
2-
- Bump commerce-sdk-isomorphic to 5.1.0-unstable-20260226081656
2+
- Bump commerce-sdk-isomorphic to 5.1.0
33
- Add Node 24 support. Drop Node 16 support. [#3652](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/3652)
44
- Add Shopper Consents API support [#3674](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/3674)
55

‎packages/commerce-sdk-react/package-lock.json‎

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/commerce-sdk-react/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
"version": "node ./scripts/version.js"
4141
},
4242
"dependencies": {
43-
"commerce-sdk-isomorphic": "5.1.0-unstable-20260226081656",
43+
"commerce-sdk-isomorphic": "5.1.0",
4444
"js-cookie": "^3.0.1",
4545
"jwt-decode": "^4.0.0"
4646
},

‎packages/pwa-kit-create-app/assets/bootstrap/js/config/default.js.hbs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -158,11 +158,14 @@ module.exports = {
158158
}
159159
},
160160
// Salesforce Payments configuration
161-
// Set enabled to false to disable Salesforce Payments even if the Commerce Cloud instance supports it.
162-
// Example URLs: sdkUrl: 'https://<instance>.unified.demandware.net/on/demandware.static/Sites-Site/-/-/internal/jscript/sfp/v1/sfp.js'
163-
// metadataUrl: 'https://<instance>.unified.demandware.net/on/demandware.static/Sites-Site/-/-/internal/metadata/v1.json'
161+
// Set enabled to true to enable Salesforce Payments (requires the Salesforce Payments feature toggle to be enabled on the Commerce Cloud instance).
162+
// Set enabled to false to disable Salesforce Payments on the storefront (the Commerce Cloud feature toggle is unaffected).
163+
// sdkUrl and metadataUrl are hosted on your Commerce Cloud instance. Replace <hostname> with your instance hostname.
164+
// This may be a demandware.net hostname (e.g., myinstance.unified.demandware.net) or a vanity/custom hostname.
165+
// sdkUrl: 'https://<hostname>/on/demandware.static/Sites-Site/-/-/internal/jscript/sfp/v1/sfp.js'
166+
// metadataUrl: 'https://<hostname>/on/demandware.static/Sites-Site/-/-/internal/metadata/v1.json'
164167
sfPayments: {
165-
enabled: true,
168+
enabled: false,
166169
sdkUrl: '',
167170
metadataUrl: ''
168171
},

‎packages/pwa-kit-create-app/assets/bootstrap/js/overrides/app/ssr.js.hbs‎

Lines changed: 31 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
import crypto from 'crypto'
1111
import express from 'express'
1212
import helmet from 'helmet'
13-
import https from 'https'
1413
import {createRemoteJWKSet as joseCreateRemoteJWKSet, jwtVerify, decodeJwt} from 'jose'
1514
import path from 'path'
1615
import {getRuntime} from '@salesforce/pwa-kit-runtime/ssr/server/express'
@@ -359,7 +358,8 @@ const {handler} = runtime.createHandler(options, (app) => {
359358
'*.stripe.com',
360359
'*.paypal.com',
361360
'*.adyen.com',
362-
'*.google.com',
361+
'pay.google.com',
362+
'www.gstatic.com',
363363
'*.demandware.net' // Used to load a valid payment scripts in test environment
364364
],
365365
'connect-src': [
@@ -375,16 +375,21 @@ const {handler} = runtime.createHandler(options, (app) => {
375375
// Payment gateways
376376
'*.demandware.net', // Used to load a valid payment scripts in test environment
377377
'*.adyen.com',
378-
'*.google.com'
378+
'*.paypal.com',
379+
'pay.google.com',
380+
'payments.google.com',
381+
'google.com',
382+
'www.google.com'
379383
],
380384
'frame-src': [
381385
// Allow frames from Salesforce site.com (Needed for MIAW)
382386
'*.site.com',
383387
// Payment gateways
384388
'*.stripe.com',
385389
'*.paypal.com',
386-
'*.google.com',
387-
'*.adyen.com'
390+
'*.adyen.com',
391+
'payments.google.com',
392+
'pay.google.com'
388393
]
389394
}
390395
}
@@ -450,49 +455,30 @@ const {handler} = runtime.createHandler(options, (app) => {
450455
// Helper function to transform relative icon paths to absolute URLs
451456
function transformIconPaths(data, ecomServerHost) {
452457
const baseUrl = `https://${ecomServerHost}/on/demandware.static/Sites-Site/-/-/internal`
453-
const dataStr = JSON.stringify(data)
454-
// Replace all relative icon paths with absolute URLs
455-
const transformedStr = dataStr.replace(/"src":\s*"\/icons\//g, `"src":"${baseUrl}/icons/`)
456-
return JSON.parse(transformedStr)
458+
const methodTypes = data?.paymentMethodTypes
459+
if (methodTypes) {
460+
for (const method of Object.values(methodTypes)) {
461+
for (const image of method.images ?? []) {
462+
if (image.src?.startsWith('/icons/')) {
463+
image.src = `${baseUrl}${image.src}`
464+
}
465+
}
466+
}
467+
}
468+
return data
457469
}
458-
470+
471+
// Helper function to fetch payment metadata from the Commerce Cloud instance
459472
app.get('/api/payment-metadata', async (req, res) => {
460473
try {
461-
// Parse the URL to extract hostname and path
462-
const url = new URL(config.app.sfPayments.metadataUrl)
463-
// Use Node's https module instead of fetch
464-
const data = await new Promise((resolve, reject) => {
465-
const options = {
466-
hostname: url.hostname,
467-
path: url.pathname,
468-
method: 'GET',
469-
rejectUnauthorized: false, // This bypasses SSL verification
470-
headers: {
471-
Accept: 'application/json'
472-
}
473-
}
474-
475-
const req = https.request(options, (response) => {
476-
let data = ''
477-
response.on('data', (chunk) => {
478-
data += chunk
479-
})
480-
response.on('end', () => {
481-
try {
482-
resolve(JSON.parse(data))
483-
} catch (e) {
484-
reject(e)
485-
}
486-
})
487-
})
488-
489-
req.on('error', reject)
490-
req.end()
474+
const response = await fetch(config.app.sfPayments.metadataUrl, {
475+
headers: { Accept: 'application/json' }
491476
})
492-
493-
// Transform relative icon paths to absolute URLs
494-
const transformedData = transformIconPaths(data, url.hostname)
495-
477+
if (!response.ok) {
478+
throw new Error(`Metadata request failed with status: ${response.status}`)
479+
}
480+
const data = await response.json()
481+
const transformedData = transformIconPaths(data, new URL(config.app.sfPayments.metadataUrl).hostname)
496482
res.setHeader('Access-Control-Allow-Origin', '*')
497483
res.setHeader('Content-Type', 'application/json')
498484
res.json(transformedData)
@@ -503,6 +489,7 @@ const {handler} = runtime.createHandler(options, (app) => {
503489
})
504490
}
505491
})
492+
506493
app.get('*', runtime.render)
507494
})
508495
// SSR requires that we export a single handler function called 'get', that

‎packages/pwa-kit-create-app/assets/templates/@salesforce/retail-react-app/app/ssr.js.hbs‎

Lines changed: 30 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
import crypto from 'crypto'
1111
import express from 'express'
1212
import helmet from 'helmet'
13-
import https from 'https'
1413
import {createRemoteJWKSet as joseCreateRemoteJWKSet, jwtVerify, decodeJwt} from 'jose'
1514
import path from 'path'
1615
import {getRuntime} from '@salesforce/pwa-kit-runtime/ssr/server/express'
@@ -359,7 +358,8 @@ const {handler} = runtime.createHandler(options, (app) => {
359358
'*.stripe.com',
360359
'*.paypal.com',
361360
'*.adyen.com',
362-
'*.google.com',
361+
'pay.google.com',
362+
'www.gstatic.com',
363363
'*.demandware.net' // Used to load a valid payment scripts in test environment
364364
],
365365
'connect-src': [
@@ -375,16 +375,21 @@ const {handler} = runtime.createHandler(options, (app) => {
375375
// Payment gateways
376376
'*.demandware.net', // Used to load a valid payment scripts in test environment
377377
'*.adyen.com',
378-
'*.google.com'
378+
'*.paypal.com',
379+
'pay.google.com',
380+
'payments.google.com',
381+
'google.com',
382+
'www.google.com'
379383
],
380384
'frame-src': [
381385
// Allow frames from Salesforce site.com (Needed for MIAW)
382386
'*.site.com',
383387
// Payment gateways
384388
'*.stripe.com',
385389
'*.paypal.com',
386-
'*.google.com',
387-
'*.adyen.com'
390+
'*.adyen.com',
391+
'payments.google.com',
392+
'pay.google.com'
388393
]
389394
}
390395
}
@@ -450,49 +455,30 @@ const {handler} = runtime.createHandler(options, (app) => {
450455
// Helper function to transform relative icon paths to absolute URLs
451456
function transformIconPaths(data, ecomServerHost) {
452457
const baseUrl = `https://${ecomServerHost}/on/demandware.static/Sites-Site/-/-/internal`
453-
const dataStr = JSON.stringify(data)
454-
// Replace all relative icon paths with absolute URLs
455-
const transformedStr = dataStr.replace(/"src":\s*"\/icons\//g, `"src":"${baseUrl}/icons/`)
456-
return JSON.parse(transformedStr)
458+
const methodTypes = data?.paymentMethodTypes
459+
if (methodTypes) {
460+
for (const method of Object.values(methodTypes)) {
461+
for (const image of method.images ?? []) {
462+
if (image.src?.startsWith('/icons/')) {
463+
image.src = `${baseUrl}${image.src}`
464+
}
465+
}
466+
}
467+
}
468+
return data
457469
}
458-
470+
471+
// Helper function to fetch payment metadata from the Commerce Cloud instance
459472
app.get('/api/payment-metadata', async (req, res) => {
460473
try {
461-
// Parse the URL to extract hostname and path
462-
const url = new URL(config.app.sfPayments.metadataUrl)
463-
// Use Node's https module instead of fetch
464-
const data = await new Promise((resolve, reject) => {
465-
const options = {
466-
hostname: url.hostname,
467-
path: url.pathname,
468-
method: 'GET',
469-
rejectUnauthorized: false, // This bypasses SSL verification
470-
headers: {
471-
Accept: 'application/json'
472-
}
473-
}
474-
475-
const req = https.request(options, (response) => {
476-
let data = ''
477-
response.on('data', (chunk) => {
478-
data += chunk
479-
})
480-
response.on('end', () => {
481-
try {
482-
resolve(JSON.parse(data))
483-
} catch (e) {
484-
reject(e)
485-
}
486-
})
487-
})
488-
489-
req.on('error', reject)
490-
req.end()
474+
const response = await fetch(config.app.sfPayments.metadataUrl, {
475+
headers: { Accept: 'application/json' }
491476
})
492-
493-
// Transform relative icon paths to absolute URLs
494-
const transformedData = transformIconPaths(data, url.hostname)
495-
477+
if (!response.ok) {
478+
throw new Error(`Metadata request failed with status: ${response.status}`)
479+
}
480+
const data = await response.json()
481+
const transformedData = transformIconPaths(data, new URL(config.app.sfPayments.metadataUrl).hostname)
496482
res.setHeader('Access-Control-Allow-Origin', '*')
497483
res.setHeader('Content-Type', 'application/json')
498484
res.json(transformedData)

‎packages/pwa-kit-create-app/assets/templates/@salesforce/retail-react-app/config/default.js.hbs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -158,11 +158,14 @@ module.exports = {
158158
}
159159
},
160160
// Salesforce Payments configuration
161-
// Set enabled to false to disable Salesforce Payments even if the Commerce Cloud instance supports it.
162-
// Example URLs: sdkUrl: 'https://<instance>.unified.demandware.net/on/demandware.static/Sites-Site/-/-/internal/jscript/sfp/v1/sfp.js'
163-
// metadataUrl: 'https://<instance>.unified.demandware.net/on/demandware.static/Sites-Site/-/-/internal/metadata/v1.json'
161+
// Set enabled to true to enable Salesforce Payments (requires the Salesforce Payments feature toggle to be enabled on the Commerce Cloud instance).
162+
// Set enabled to false to disable Salesforce Payments on the storefront (the Commerce Cloud feature toggle is unaffected).
163+
// sdkUrl and metadataUrl are hosted on your Commerce Cloud instance. Replace <hostname> with your instance hostname.
164+
// This may be a demandware.net hostname (e.g., myinstance.unified.demandware.net) or a vanity/custom hostname.
165+
// sdkUrl: 'https://<hostname>/on/demandware.static/Sites-Site/-/-/internal/jscript/sfp/v1/sfp.js'
166+
// metadataUrl: 'https://<hostname>/on/demandware.static/Sites-Site/-/-/internal/metadata/v1.json'
164167
sfPayments: {
165-
enabled: true,
168+
enabled: false,
166169
sdkUrl: '',
167170
metadataUrl: ''
168171
},

‎packages/template-retail-react-app/app/hooks/use-current-basket.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,8 @@ export const useCurrentBasket = ({id = ''} = {}) => {
9595
])
9696

9797
return {
98-
data: currentBasket,
9998
...restOfQuery,
99+
data: currentBasket,
100100
derivedData: {
101101
// Only true if a non-temporary basket exists (temporary baskets are filtered out above)
102102
hasBasket: !!currentBasket,

‎packages/template-retail-react-app/app/hooks/use-sf-payments.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ export const useSFPayments = () => {
4343
}
4444
}, [status.loaded])
4545

46+
const metadataUrl = config?.app?.sfPayments?.metadataUrl
47+
const localEnabled = config?.app?.sfPayments?.enabled ?? true
48+
4649
const {data: serverMetadata, isLoading: serverMetadataLoading} = useQuery({
4750
queryKey: ['payment-metadata'],
4851
queryFn: async () => {
@@ -52,6 +55,9 @@ export const useSFPayments = () => {
5255
}
5356
return await response.json()
5457
},
58+
// Only fetch metadata if metadataUrl is set and sfPayments is enabled,
59+
// prevents any 500 on server side and unnecessary network requests
60+
enabled: localEnabled && !!metadataUrl,
5561
staleTime: 10 * 60 * 1000 // 10 minutes
5662
})
5763

‎packages/template-retail-react-app/app/hooks/use-sf-payments.test.js‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,8 @@ describe('useSFPayments hook', () => {
127127
mockGetConfig.mockReturnValue({
128128
app: {
129129
sfPayments: {
130-
sdkUrl: 'https://test.sfpayments.com/sdk.js'
130+
sdkUrl: 'https://test.sfpayments.com/sdk.js',
131+
metadataUrl: 'https://test.sfpayments.com/metadata'
131132
}
132133
}
133134
})
@@ -298,6 +299,43 @@ describe('useSFPayments hook', () => {
298299
})
299300
})
300301

302+
describe('metadata query guard', () => {
303+
test('does not fetch metadata when metadataUrl is empty', async () => {
304+
mockGetConfig.mockReturnValue({
305+
app: {
306+
sfPayments: {
307+
sdkUrl: 'https://test.sfpayments.com/sdk.js',
308+
metadataUrl: ''
309+
}
310+
}
311+
})
312+
313+
renderWithQueryClient(<TestComponent />)
314+
315+
await waitFor(() => {
316+
expect(mockFetch).not.toHaveBeenCalled()
317+
})
318+
})
319+
320+
test('does not fetch metadata when sfPayments is disabled', async () => {
321+
mockGetConfig.mockReturnValue({
322+
app: {
323+
sfPayments: {
324+
enabled: false,
325+
sdkUrl: 'https://test.sfpayments.com/sdk.js',
326+
metadataUrl: 'https://test.sfpayments.com/metadata'
327+
}
328+
}
329+
})
330+
331+
renderWithQueryClient(<TestComponent />)
332+
333+
await waitFor(() => {
334+
expect(mockFetch).not.toHaveBeenCalled()
335+
})
336+
})
337+
})
338+
301339
describe('confirming basket state', () => {
302340
test('startConfirming updates confirmingBasket', async () => {
303341
let hookData

0 commit comments

Comments
 (0)