Skip to content
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.

Sign up for GitHub

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

Fix locale alias by including locale object in MultiSite Context #716

Merged
merged 8 commits into from
Sep 13, 2022
Merged

Fix locale alias by including locale object in MultiSite Context #716

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
674a514
Fix locale alias
adamraya Sep 12, 2022
6dfb720
Fix tests
adamraya Sep 13, 2022
1d4b5ad
Add test cases with locale alias 'fr'
adamraya Sep 13, 2022
55aa336
Clean up
adamraya Sep 13, 2022
313c32b
Include locale alias in `getPathWithLocale`
adamraya Sep 13, 2022
ab02c6f
Small refactor
adamraya Sep 13, 2022
f70a79c
Use refactor `getSiteByReference` in `TestProviders`
adamraya Sep 13, 2022
265d900
Merge branch 'develop' into fix-locale-alias
adamraya Sep 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/template-retail-react-app/app/components/_app-config/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ AppConfig.restore = (locals = {}) => {
locals.api = new CommerceAPI({...apiConfig, locale: locale.id, currency})
locals.buildUrl = createUrlTemplate(appConfig, site.alias || site.id, locale.id)
locals.site = site
locals.locale = locale.id
locals.locale = locale
}

AppConfig.freeze = () => undefined
Expand Down
6 changes: 3 additions & 3 deletions packages/template-retail-react-app/app/components/link/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ test('renders a link with locale prepended', () => {
delete window.location
window.location = new URL('/us/en-US', 'https://www.example.com')
const {getByText} = renderWithProviders(<Link href="/mypage">My Page</Link>, {
wrapperProps: {locale: 'en-US', siteAlias: 'us', appConfig: mockConfig.app}
wrapperProps: {locale: {id: 'en-US'}, siteAlias: 'us', appConfig: mockConfig.app}
})
expect(getByText(/My Page/i)).toHaveAttribute('href', '/us/en-US/mypage')
})
Expand All @@ -49,7 +49,7 @@ test('renders a link with locale and site as query param', () => {
delete window.location
window.location = new URL('https://www.example.com/women/dresses?site=us&locale=en-US')
const {getByText} = renderWithProviders(<Link href="/mypage">My Page</Link>, {
wrapperProps: {locale: 'en-US', siteAlias: 'us', appConfig: newConfig.app}
wrapperProps: {locale: {id: 'en-US'}, siteAlias: 'us', appConfig: newConfig.app}
})

expect(getByText(/My Page/i)).toHaveAttribute('href', `/mypage?site=us&locale=en-US`)
Expand All @@ -60,7 +60,7 @@ test('accepts `to` prop as well', () => {
delete window.location
window.location = new URL('us/en-US', 'https://www.example.com')
const {getByText} = renderWithProviders(<Link to="/mypage">My Page</Link>, {
wrapperProps: {locale: 'en-US', siteAlias: 'us', appConfig: mockConfig.app}
wrapperProps: {locale: {id: 'en-US'}, siteAlias: 'us', appConfig: mockConfig.app}
})
expect(getByText(/My Page/i)).toHaveAttribute('href', '/us/en-US/mypage')
})
Expand Down
2 changes: 1 addition & 1 deletion packages/template-retail-react-app/app/contexts/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ MultiSiteProvider.propTypes = {
children: PropTypes.node.isRequired,
buildUrl: PropTypes.func,
site: PropTypes.object,
locale: PropTypes.string
locale: PropTypes.object
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/template-retail-react-app/app/hooks/use-multi-site.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const useMultiSite = () => {
return originalFn(
path,
siteRef ? siteRef : site?.alias || site?.id,
localeRef ? localeRef : locale
localeRef ? localeRef : locale?.alias || locale?.id
)
},
[originalFn, site, locale]
Expand Down
2 changes: 1 addition & 1 deletion packages/template-retail-react-app/app/pages/checkout/confirmation.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ afterAll(() => server.close())

test('Navigates to homepage when no order present', async () => {
renderWithProviders(<Confirmation />, {
wrapperProps: {siteAlias: 'uk', locale: 'en-GB'}
wrapperProps: {siteAlias: 'uk', locale: {id: 'en-GB'}}
})
expect(screen.queryByTestId('sf-checkout-confirmation-container')).not.toBeInTheDocument()
await waitFor(() => {
Expand Down
6 changes: 3 additions & 3 deletions packages/template-retail-react-app/app/pages/checkout/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ test('Can proceed through checkout as registered customer', async () => {
// Set the initial browser router path and render our component tree.
window.history.pushState({}, 'Checkout', createPathWithDefaults('/checkout'))
renderWithProviders(<WrappedCheckout history={history} />, {
wrapperProps: {siteAlias: 'uk', locale: 'en-GB', appConfig: mockConfig.app}
wrapperProps: {siteAlias: 'uk', locale: {id: 'en-GB'}, appConfig: mockConfig.app}
})

// Switch to login
Expand Down Expand Up @@ -586,7 +586,7 @@ test('Can edit address during checkout as a registered customer', async () => {
// Set the initial browser router path and render our component tree.
window.history.pushState({}, 'Checkout', createPathWithDefaults('/checkout'))
renderWithProviders(<WrappedCheckout history={history} />, {
wrapperProps: {siteAlias: 'uk', locale: 'en-GB'}
wrapperProps: {siteAlias: 'uk', locale: {id: 'en-GB'}}
})

// Switch to login
Expand Down Expand Up @@ -693,7 +693,7 @@ test('Can add address during checkout as a registered customer', async () => {
// Set the initial browser router path and render our component tree.
window.history.pushState({}, 'Checkout', createPathWithDefaults('/checkout'))
renderWithProviders(<WrappedCheckout history={history} />, {
wrapperProps: {siteAlias: 'us', locale: 'en-US'}
wrapperProps: {siteAlias: 'us', locale: {id: 'en-US'}}
})

// Switch to login
Expand Down
8 changes: 4 additions & 4 deletions packages/template-retail-react-app/app/pages/login/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ test('Allows customer to sign in to their account', async () => {
)
// render our test component
renderWithProviders(<MockedComponent />, {
wrapperProps: {siteAlias: 'uk', locale: 'en-GB', appConfig: mockConfig.app}
wrapperProps: {siteAlias: 'uk', locale: {id: 'en-GB'}, appConfig: mockConfig.app}
})

// enter credentials and submit
Expand All @@ -164,7 +164,7 @@ test('Renders error when given incorrect log in credentials', async () => {

// render our test component
renderWithProviders(<MockedComponent />, {
wrapperProps: {siteAlias: 'uk', locale: 'en-GB', appConfig: mockConfig.app}
wrapperProps: {siteAlias: 'uk', locale: {id: 'en-GB'}, appConfig: mockConfig.app}
})

// enter credentials and submit
Expand All @@ -185,7 +185,7 @@ test('Renders error when given incorrect log in credentials', async () => {
test('should navigate to sign in page when the user clicks Create Account', async () => {
// render our test component
renderWithProviders(<MockedComponent />, {
wrapperProps: {siteAlias: 'uk', locale: 'en-GB', appConfig: mockConfig.app}
wrapperProps: {siteAlias: 'uk', locale: {id: 'en-GB'}, appConfig: mockConfig.app}
})
user.click(screen.getByText(/Create Account/i))

Expand All @@ -196,7 +196,7 @@ test('should navigate to sign in page when the user clicks Create Account', asyn
test('should navigate to reset password page when the user clicks Forgot Password', async () => {
// render our test component
renderWithProviders(<MockedComponent />, {
wrapperProps: {siteAlias: 'uk', locale: 'en-GB', appConfig: mockConfig.app}
wrapperProps: {siteAlias: 'uk', locale: {id: 'en-GB'}, appConfig: mockConfig.app}
})
user.click(screen.getByText(/forgot password/i))

Expand Down
4 changes: 2 additions & 2 deletions packages/template-retail-react-app/app/pages/product-list/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ test('show login modal when an unauthenticated user tries to add an item to wish

test('clicking a filter will change url', async () => {
renderWithProviders(<MockedComponent />, {
wrapperProps: {siteAlias: 'uk', locale: 'en-GB'}
wrapperProps: {siteAlias: 'uk', locale: {id: 'en-GB'}}
})
user.click(screen.getByText(/Beige/i))
await waitFor(() =>
Expand All @@ -184,7 +184,7 @@ test('click on filter All should clear out all the filter in search params', asy
'uk/en-GB/category/mens-clothing-jackets?limit=25&refine=c_refinementColor%3DBeige&sort=best-matches'
)
renderWithProviders(<MockedComponent />, {
wrapperProps: {siteAlias: 'uk', locale: 'en-GB'}
wrapperProps: {siteAlias: 'uk', locale: {id: 'en-GB'}}
})
const clearAllButton = await screen.findAllByText(/Clear All/i)
user.click(clearAllButton[0])
Expand Down
8 changes: 4 additions & 4 deletions packages/template-retail-react-app/app/utils/test-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export const TestProviders = ({
initialBasket = null,
initialCustomer = null,
initialCategories = initialMockCategories,
locale = DEFAULT_LOCALE,
locale = {id: DEFAULT_LOCALE},
messages = fallbackMessages,
appConfig = appDefaultConfig,
siteAlias = DEFAULT_SITE
Expand Down Expand Up @@ -132,10 +132,10 @@ export const TestProviders = ({
return site.alias === siteAlias || site.id === appConfig['site']
}) || getDefaultSite()

const buildUrl = createUrlTemplate(appConfig, site.alias || site.id, locale)
const buildUrl = createUrlTemplate(appConfig, site.alias || site.id, locale.alias || locale.id)

return (
<IntlProvider locale={locale} defaultLocale={DEFAULT_LOCALE} messages={messages}>
<IntlProvider locale={locale.id} defaultLocale={DEFAULT_LOCALE} messages={messages}>
<MultiSiteProvider site={site} locale={locale} buildUrl={buildUrl}>
<CommerceAPIProvider value={api}>
<CategoriesProvider categories={initialCategories}>
Expand Down Expand Up @@ -170,7 +170,7 @@ TestProviders.propTypes = {
initialCategories: PropTypes.element,
initialProductLists: PropTypes.object,
messages: PropTypes.object,
locale: PropTypes.string,
locale: PropTypes.object,
appConfig: PropTypes.object,
siteAlias: PropTypes.string
}
Expand Down
74 changes: 44 additions & 30 deletions packages/template-retail-react-app/app/utils/url.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,10 @@ describe('createUrlTemplate tests', () => {
showDefaults: false
},
site: defaultSiteMock,
locale: isDefaultLocale === 0 ? {id: 'en-GB'} : {id: 'fr-FR'}
locale:
isDefaultLocale === 0
? {id: 'en-GB'}
: {id: 'fr-FR', alias: 'fr'}
})
} else {
cases.push({
Expand All @@ -220,7 +223,10 @@ describe('createUrlTemplate tests', () => {
showDefaults: false
},
site: nonDefaultSiteMock,
locale: isDefaultLocale === 0 ? {id: 'en-US'} : {id: 'fr-FR'}
locale:
isDefaultLocale === 0
? {id: 'en-US'}
: {id: 'fr-FR', alias: 'fr'}
})
}
}
Expand All @@ -236,34 +242,34 @@ describe('createUrlTemplate tests', () => {
? [
`/uk/en-GB${path}`,
`${path}`,
`/fr-FR${path}`,
`/fr${path}`,
`/us${path}`,
`/us/fr-FR${path}`,
`/us/fr${path}`,
`/en-GB${path}?site=uk`,
`${path}`,
`/fr-FR${path}`,
`/fr${path}`,
`${path}?site=us`,
`/fr-FR${path}?site=us`,
`/fr${path}?site=us`,
`/en-GB${path}`,
`${path}`,
`/fr-FR${path}`,
`/fr${path}`,
`${path}`,
`/fr-FR${path}`,
`/fr${path}`,
`/uk${path}?locale=en-GB`,
`${path}`,
`${path}?locale=fr-FR`,
`${path}?locale=fr`,
`/us${path}`,
`/us${path}?locale=fr-FR`,
`/us${path}?locale=fr`,
`${path}?site=uk&locale=en-GB`,
`${path}`,
`${path}?locale=fr-FR`,
`${path}?locale=fr`,
`${path}?site=us`,
`${path}?site=us&locale=fr-FR`,
`${path}?site=us&locale=fr`,
`${path}?locale=en-GB`,
`${path}`,
`${path}?locale=fr-FR`,
`${path}?locale=fr`,
`${path}`,
`${path}?locale=fr-FR`,
`${path}?locale=fr`,
`/uk${path}`,
`${path}`,
`${path}`,
Expand All @@ -283,34 +289,34 @@ describe('createUrlTemplate tests', () => {
: [
`${path}`,
`${path}`,
`/fr-FR${path}`,
`/fr${path}`,
`/us${path}`,
`/us/fr-FR${path}`,
`/us/fr${path}`,
`${path}`,
`${path}`,
`/fr-FR${path}`,
`/fr${path}`,
`${path}?site=us`,
`/fr-FR${path}?site=us`,
`/fr${path}?site=us`,
`${path}`,
`${path}`,
`/fr-FR${path}`,
`/fr${path}`,
`${path}`,
`/fr-FR${path}`,
`/fr${path}`,
`${path}`,
`${path}`,
`${path}?locale=fr-FR`,
`${path}?locale=fr`,
`/us${path}`,
`/us${path}?locale=fr-FR`,
`/us${path}?locale=fr`,
`${path}`,
`${path}`,
`${path}?locale=fr-FR`,
`${path}?locale=fr`,
`${path}?site=us`,
`${path}?site=us&locale=fr-FR`,
`${path}?site=us&locale=fr`,
`${path}`,
`${path}`,
`${path}?locale=fr-FR`,
`${path}?locale=fr`,
`${path}`,
`${path}?locale=fr-FR`,
`${path}?locale=fr`,
`${path}`,
`${path}`,
`${path}`,
Expand All @@ -330,11 +336,19 @@ describe('createUrlTemplate tests', () => {
}
paths.forEach((path) => {
cases.forEach(({urlConfig, site, locale}, index) => {
test(`URL template path:${path}, site:${site.alias}, locale:${
locale.id
test(`URL template path:${path}, site:${site.alias}, locale.id:${locale.id}${
locale?.alias ? `, locale.alias:${locale.alias}` : ''
} and urlConfig:${JSON.stringify(urlConfig)}`, () => {
const buildUrl = createUrlTemplate({url: urlConfig}, site.id, locale.id)
const resultUrl = buildUrl(path, mockConfig.app.siteAliases[site.id], locale.id)
const buildUrl = createUrlTemplate(
{url: urlConfig},
site.id,
locale?.alias || locale?.id
)
const resultUrl = buildUrl(
path,
mockConfig.app.siteAliases[site.id],
locale?.alias || locale?.id
)

expect(resultUrl).toEqual(expectedResults(path)[index])
})
Expand Down