From 573a1da59bfcc784501d306304aabf2214e26d6e Mon Sep 17 00:00:00 2001 From: ResuBaka Date: Sat, 10 Aug 2019 15:47:16 +0200 Subject: [PATCH 1/3] Added new tests and fixed a potential small problem in adjustMultistoreApiUrl The problem could have been that we send the storeCode query parameter two times and now it gets removed bore we add a new one when needed. --- core/lib/multistore.ts | 29 +++- core/lib/test/unit/multistore.spec.ts | 200 +++++++++++++++++++++++++- 2 files changed, 222 insertions(+), 7 deletions(-) diff --git a/core/lib/multistore.ts b/core/lib/multistore.ts index 6876989e50..5bc1e7def4 100644 --- a/core/lib/multistore.ts +++ b/core/lib/multistore.ts @@ -164,11 +164,32 @@ export function removeStoreCodeFromRoute (matchedRouteOrUrl: LocalizedRoute | st } } +function removeURLQueryParameter (url, parameter) { + // prefer to use l.search if you have a location/link object + var urlparts = url.split('?'); + if (urlparts.length >= 2) { + var prefix = encodeURIComponent(parameter) + '='; + var pars = urlparts[1].split(/[&;]/g); + + // reverse iteration as may be destructive + for (var i = pars.length; i-- > 0;) { + // idiom for string.startsWith + if (pars[i].lastIndexOf(prefix, 0) !== -1) { + pars.splice(i, 1); + } + } + + return urlparts[0] + (pars.length > 0 ? '?' + pars.join('&') : ''); + } + return url; +} + export function adjustMultistoreApiUrl (url: string): string { - const storeView = currentStoreView() - if (storeView.storeCode) { + const { storeCode } = currentStoreView() + if (storeCode) { + url = removeURLQueryParameter(url, 'storeCode') const urlSep = (url.indexOf('?') > 0) ? '&' : '?' - url += urlSep + 'storeCode=' + storeView.storeCode + url += `${urlSep}storeCode=${storeCode}` } return url } @@ -177,7 +198,7 @@ export function localizedDispatcherRoute (routeObj: LocalizedRoute | string, sto const appendStoreCodePrefix = config.storeViews[storeCode] ? config.storeViews[storeCode].appendStoreCode : false if (typeof routeObj === 'string') { - return appendStoreCodePrefix ? '/' + storeCode + routeObj : routeObj + return appendStoreCodePrefix ? `/${storeCode}${routeObj}` : routeObj } if (routeObj && routeObj.fullPath) { // case of using dispatcher diff --git a/core/lib/test/unit/multistore.spec.ts b/core/lib/test/unit/multistore.spec.ts index ff0745f6f0..76ce896e85 100644 --- a/core/lib/test/unit/multistore.spec.ts +++ b/core/lib/test/unit/multistore.spec.ts @@ -1,6 +1,15 @@ -import { storeCodeFromRoute, prepareStoreView } from '@vue-storefront/core/lib/multistore' +import { + storeCodeFromRoute, + prepareStoreView, + adjustMultistoreApiUrl, + localizedDispatcherRoute, + LocalizedRoute, + setupMultistoreRoutes +} from '@vue-storefront/core/lib/multistore' import config from 'config' import rootStore from '@vue-storefront/core/store'; +import VueRouter, { RouteConfig } from 'vue-router' +import { RouterManager } from '@vue-storefront/core/lib/router-manager' jest.mock('@vue-storefront/core/app', () => ({ createApp: jest.fn() })) jest.mock('../../../store', () => ({})) @@ -10,9 +19,10 @@ jest.mock('@vue-storefront/core/hooks', () => ({ coreHooksExecutors: { beforeStoreViewChange: jest.fn(args => args), afterStoreViewChange: jest.fn(args => args) }})) -jest.mock('query-string', () => jest.fn()) jest.mock('@vue-storefront/core/lib/router-manager', () => ({ - RouterManager: {} + RouterManager: { + addRoutes: jest.fn() + } })) jest.mock('@vue-storefront/core/lib/logger', () => ({ Logger: {} @@ -20,6 +30,7 @@ jest.mock('@vue-storefront/core/lib/logger', () => ({ jest.mock('config', () => ({})) describe('Multistore', () => { + let router beforeEach(() => { jest.clearAllMocks(); (rootStore as any).state = {}; @@ -393,4 +404,187 @@ describe('Multistore', () => { }) }) }) + + describe('adjustMultistoreApiUrl', () => { + it('returns URL /test without storeCode as parameter', () => { + rootStore.state.storeView = { + storeCode: null + } + + expect(adjustMultistoreApiUrl('/test')).toStrictEqual('/test') + }) + + it('returns URL /test with storeCode de as parameter', () => { + rootStore.state.storeView = { + storeCode: 'de' + } + + expect(adjustMultistoreApiUrl('/test')).toStrictEqual('/test?storeCode=de') + }) + + it('returns URL /test?a=b with storeCode de as parameter and current parameters from the URL', () => { + rootStore.state.storeView = { + storeCode: 'de' + } + + expect(adjustMultistoreApiUrl('/test?a=b')).toStrictEqual('/test?a=b&storeCode=de') + }) + + it('returns URL /test?a=b&storeCode=de with added storeCode at as parameter and removes previous storeCode parameter', () => { + rootStore.state.storeView = { + storeCode: 'at' + } + + expect(adjustMultistoreApiUrl('/test?a=b&storeCode=de')).toStrictEqual('/test?a=b&storeCode=at') + }) + it('returns URL /test?storeCode=de with changed storeCode at as parameter', () => { + rootStore.state.storeView = { + storeCode: 'at' + } + + expect(adjustMultistoreApiUrl('/test?storeCode=de')).toStrictEqual('/test?storeCode=at') + }) + }) + + describe('localizedDispatcherRoute', () => { + it('URL /test stays the same', () => { + config.storeViews = {} + + expect(localizedDispatcherRoute('/test', 'de')).toStrictEqual('/test') + }) + + it('URL /test starts with /de', () => { + config.storeViews = { + de: { + appendStoreCode: true + } + } + + expect(localizedDispatcherRoute('/test', 'de')).toStrictEqual('/de/test') + }) + + it('URL /test?a=b&b=a stays the same', () => { + config.storeViews = {} + + expect(localizedDispatcherRoute('/test?a=b&b=a', 'de')).toStrictEqual('/test?a=b&b=a') + }) + + it('URL /test?a=b&b=a starts with /de', () => { + config.storeViews = { + de: { + appendStoreCode: true + } + } + + expect(localizedDispatcherRoute('/test?a=b&b=a', 'de')).toStrictEqual('/de/test?a=b&b=a') + }) + + it('URL with LocalizedRoute object with fullPath test gets prefixed with /de', () => { + config.storeViews = {} + + const LocalizedRoute: LocalizedRoute = { + fullPath: 'test' + } + + expect(localizedDispatcherRoute(LocalizedRoute, 'de')).toStrictEqual('/test') + }) + + it('URL with LocalizedRoute object with fullPath and parameter test stays the same', () => { + config.storeViews = {} + + const LocalizedRoute: LocalizedRoute = { + fullPath: 'test', + params: { + a: 'b', + b: 'a' + } + } + + expect(localizedDispatcherRoute(LocalizedRoute, 'de')).toStrictEqual('/test?a=b&b=a') + }) + + it('URL with LocalizedRoute object with fullPath test gets prefixed with /de', () => { + config.storeViews = { + de: { + appendStoreCode: true + } + } + + const LocalizedRoute: LocalizedRoute = { + fullPath: 'test' + } + + expect(localizedDispatcherRoute(LocalizedRoute, 'de')).toStrictEqual('/de/test') + }) + + it('URL with LocalizedRoute object with fullPath test and params gets prefixed with /de', () => { + config.storeViews = { + de: { + appendStoreCode: true + } + } + + const LocalizedRoute: LocalizedRoute = { + fullPath: 'test', + params: { + a: 'b', + b: 'a' + } + } + + expect(localizedDispatcherRoute(LocalizedRoute, 'de')).toStrictEqual('/de/test?a=b&b=a') + }) + }) + + describe('setupMultistoreRoutes', () => { + it('Add new routes for each store in mapStoreUrlsFor', () => { + config.storeViews = { + 'de': { + appendStoreCode: true + }, + mapStoreUrlsFor: [ + 'de' + ], + multistore: true + } + + const routeConfig: RouteConfig[] = [ + { + path: 'test' + }, + { + path: 'test2' + } + ] + + const vueRouter = {} + + setupMultistoreRoutes(config, (vueRouter as VueRouter), routeConfig) + + expect(RouterManager.addRoutes).toBeCalledTimes(1) + }) + + it('Do nothing as mapStoreUrlsFor is empty', () => { + config.storeViews = { + 'de': { + }, + mapStoreUrlsFor: [] + } + + const routeConfig: RouteConfig[] = [ + { + path: 'test' + }, + { + path: 'test2' + } + ] + + const vueRouter = {} + + setupMultistoreRoutes(config, (vueRouter as VueRouter), routeConfig) + + expect(RouterManager.addRoutes).toBeCalledTimes(0) + }) + }) }) From 12a377a7ccfb380295217c7ea1d07c0597c3dc88 Mon Sep 17 00:00:00 2001 From: ResuBaka Date: Sat, 10 Aug 2019 15:50:05 +0200 Subject: [PATCH 2/3] Added test in multistore.spec.ts for multiple storeCode as parameter When that happens there should only be one returned when we need to set a new storeCode parameter. --- core/lib/test/unit/multistore.spec.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/core/lib/test/unit/multistore.spec.ts b/core/lib/test/unit/multistore.spec.ts index 76ce896e85..39f362f184 100644 --- a/core/lib/test/unit/multistore.spec.ts +++ b/core/lib/test/unit/multistore.spec.ts @@ -437,6 +437,7 @@ describe('Multistore', () => { expect(adjustMultistoreApiUrl('/test?a=b&storeCode=de')).toStrictEqual('/test?a=b&storeCode=at') }) + it('returns URL /test?storeCode=de with changed storeCode at as parameter', () => { rootStore.state.storeView = { storeCode: 'at' @@ -444,6 +445,14 @@ describe('Multistore', () => { expect(adjustMultistoreApiUrl('/test?storeCode=de')).toStrictEqual('/test?storeCode=at') }) + + it('returns URL /test?storeCode=de with changed storeCode at as parameter', () => { + rootStore.state.storeView = { + storeCode: 'at' + } + + expect(adjustMultistoreApiUrl('/test?storeCode=de&storeCode=de')).toStrictEqual('/test?storeCode=at') + }) }) describe('localizedDispatcherRoute', () => { From 395d921dd6aa1d17259265eefc44a5a7c6c67cdf Mon Sep 17 00:00:00 2001 From: ResuBaka Date: Sat, 10 Aug 2019 15:56:59 +0200 Subject: [PATCH 3/3] updated CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba0da384cc..599f206dea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added loading product attributes (`entities.productListWithChildren.includeFields`) on category page - @andrzejewsky (#3220) - Added config to set Cache-Control header for static assets based on mime type - @phoenix-bjoern (#3268) - Added test:unit:watch with a workaround of a jest problem with template strings - @resubaka (#3351) +- Added test to multistore.ts so it is nearly fully unit tested - @resubaka (#3352) ### Fixed