From 4c0f115ebbc36e6cf5468496cb6745f7a905745f Mon Sep 17 00:00:00 2001 From: "max.kless@googlemail.com" Date: Wed, 23 Jun 2021 16:27:35 +0200 Subject: [PATCH 01/23] makehref changes --- src/app/core/pipes/make-href.pipe.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/app/core/pipes/make-href.pipe.ts b/src/app/core/pipes/make-href.pipe.ts index 5ea64e6ffb..c20097bd60 100644 --- a/src/app/core/pipes/make-href.pipe.ts +++ b/src/app/core/pipes/make-href.pipe.ts @@ -3,7 +3,12 @@ import { Pipe, PipeTransform } from '@angular/core'; @Pipe({ name: 'makeHref', pure: false }) export class MakeHrefPipe implements PipeTransform { - transform(location: LocationStrategy, urlParams: { [key: string]: string }): string { + mappings: Record = { + de_DE: '/de', + en_US: '/en', + fr_FR: '/fr', + }; + transform(location: LocationStrategy, urlParams: Record): string { if (!location || !location.path()) { return 'undefined'; } @@ -25,6 +30,9 @@ export class MakeHrefPipe implements PipeTransform { newUrl += `?${split[1]}`; } + if (urlParams.lang && this.mappings[urlParams.lang]) { + newUrl = newUrl.replace(location.getBaseHref(), this.mappings[urlParams.lang]); + } return newUrl; } } From 13be5fb2f60aab458e34bac8eefd0de8592a505e Mon Sep 17 00:00:00 2001 From: "max.kless@googlemail.com" Date: Wed, 23 Jun 2021 16:27:36 +0200 Subject: [PATCH 02/23] multisite handling in service --- docker-compose.yml | 1 - src/app/core/configurations/injection-keys.ts | 7 ++++ src/app/core/pipes/make-href.pipe.ts | 15 +++---- .../multi-site/multi-site.service.spec.ts | 23 ++++++++++ .../utils/multi-site/multi-site.service.ts | 42 +++++++++++++++++++ src/environments/environment.model.ts | 8 ++++ 6 files changed, 85 insertions(+), 11 deletions(-) create mode 100644 src/app/core/utils/multi-site/multi-site.service.spec.ts create mode 100644 src/app/core/utils/multi-site/multi-site.service.ts diff --git a/docker-compose.yml b/docker-compose.yml index 8123e00aac..30a659ce74 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -43,7 +43,6 @@ services: - baseHref: /de channel: default lang: de_DE - protected: false - baseHref: /fr channel: default lang: fr_FR diff --git a/src/app/core/configurations/injection-keys.ts b/src/app/core/configurations/injection-keys.ts index 6fe9f9745d..0fd8613b07 100644 --- a/src/app/core/configurations/injection-keys.ts +++ b/src/app/core/configurations/injection-keys.ts @@ -58,6 +58,13 @@ export const DATA_RETENTION_POLICY = new InjectionToken('da factory: () => environment.dataRetention, }); +/** + * the locale to multi site configuration map + */ +export const MULTI_SITE_LOCALE_MAP = new InjectionToken | undefined>('multiSiteLocaleMap', { + factory: () => environment.multiSiteLocaleMap, +}); + /* * global definition of the Bootstrap grid system breakpoint widths */ diff --git a/src/app/core/pipes/make-href.pipe.ts b/src/app/core/pipes/make-href.pipe.ts index c20097bd60..9a683a470d 100644 --- a/src/app/core/pipes/make-href.pipe.ts +++ b/src/app/core/pipes/make-href.pipe.ts @@ -1,19 +1,18 @@ import { LocationStrategy } from '@angular/common'; import { Pipe, PipeTransform } from '@angular/core'; +import { MultiSiteService } from 'ish-core/utils/multi-site/multi-site.service'; + @Pipe({ name: 'makeHref', pure: false }) export class MakeHrefPipe implements PipeTransform { - mappings: Record = { - de_DE: '/de', - en_US: '/en', - fr_FR: '/fr', - }; + constructor(private multiSiteService: MultiSiteService) {} transform(location: LocationStrategy, urlParams: Record): string { if (!location || !location.path()) { return 'undefined'; } - const split = location.path().split('?'); + const url = this.multiSiteService.getLangUpdatedUrl(urlParams.lang, location); + const split = url.split('?'); // url without query params let newUrl = split[0]; @@ -29,10 +28,6 @@ export class MakeHrefPipe implements PipeTransform { if (split.length > 1) { newUrl += `?${split[1]}`; } - - if (urlParams.lang && this.mappings[urlParams.lang]) { - newUrl = newUrl.replace(location.getBaseHref(), this.mappings[urlParams.lang]); - } return newUrl; } } diff --git a/src/app/core/utils/multi-site/multi-site.service.spec.ts b/src/app/core/utils/multi-site/multi-site.service.spec.ts new file mode 100644 index 0000000000..4b624638b0 --- /dev/null +++ b/src/app/core/utils/multi-site/multi-site.service.spec.ts @@ -0,0 +1,23 @@ +import { TestBed } from '@angular/core/testing'; +import { instance, mock } from 'ts-mockito'; + +import { ApiService } from 'ish-core/services/api/api.service'; + +import { MultiSiteService } from './multi-site.service'; + +describe('Multi Site Service', () => { + let apiServiceMock: ApiService; + let multiSiteService: MultiSiteService; + + beforeEach(() => { + apiServiceMock = mock(ApiService); + TestBed.configureTestingModule({ + providers: [{ provide: ApiService, useFactory: () => instance(apiServiceMock) }], + }); + multiSiteService = TestBed.inject(MultiSiteService); + }); + + it('should be created', () => { + expect(multiSiteService).toBeTruthy(); + }); +}); diff --git a/src/app/core/utils/multi-site/multi-site.service.ts b/src/app/core/utils/multi-site/multi-site.service.ts new file mode 100644 index 0000000000..42c5f8cbd4 --- /dev/null +++ b/src/app/core/utils/multi-site/multi-site.service.ts @@ -0,0 +1,42 @@ +import { LocationStrategy } from '@angular/common'; +import { Inject, Injectable } from '@angular/core'; + +import { MULTI_SITE_LOCALE_MAP } from 'ish-core/configurations/injection-keys'; + +export type MultiSiteLocaleMap = Record | undefined; + +@Injectable({ providedIn: 'root' }) +export class MultiSiteService { + constructor(@Inject(MULTI_SITE_LOCALE_MAP) private multiSiteLocaleMap: MultiSiteLocaleMap) {} + + /** + * returns the current url, modified to fit the locale parameter if the environment parameter "multiSiteLocaleMap" is set + * @param locale the locale which the new url should fit + * @param location the current loation + * @returns + */ + getLangUpdatedUrl(locale: string, location: LocationStrategy): string { + let newUrl = location.path(); + /** + * only replace lang parameter if: + * - multiSiteLocaleMap environment var is declared (set to undefined to skip this behaviour) + * - current baseHref is part of the map (so the empty "/" baseHref would not be replaced) + * - multiSiteLocaleMap contains target locale + */ + if ( + this.multiSiteLocaleMap && + Object.keys(this.multiSiteLocaleMap).includes(location.getBaseHref()) && + localeMapHasLangString(locale, this.multiSiteLocaleMap) + ) { + newUrl = newUrl.replace(location.getBaseHref(), this.multiSiteLocaleMap[locale]); + } + return newUrl; + } +} + +function localeMapHasLangString( + lang: string, + multiSiteLocaleMap: MultiSiteLocaleMap +): multiSiteLocaleMap is Record { + return lang && multiSiteLocaleMap[lang] && typeof multiSiteLocaleMap[lang] === 'string'; +} diff --git a/src/environments/environment.model.ts b/src/environments/environment.model.ts index d6601ee0fe..74086f09fb 100644 --- a/src/environments/environment.model.ts +++ b/src/environments/environment.model.ts @@ -85,6 +85,7 @@ export interface Environment { // configuration of the available locales - hard coded for now locales: Locale[]; + multiSiteLocaleMap: Record | undefined; // configuration of the styling theme ('default' if not configured) // format: 'themeName|themeColor' e.g. theme: 'blue|688dc3', @@ -138,6 +139,13 @@ export const ENVIRONMENT_DEFAULTS: Environment = { { lang: 'de_DE', currency: 'EUR', value: 'de', displayName: 'German', displayLong: 'German (Germany)' }, { lang: 'fr_FR', currency: 'EUR', value: 'fr', displayName: 'French', displayLong: 'French (France)' }, ], + // tslint:disable: use-camel-case-environment-properties + multiSiteLocaleMap: { + en_US: '/en', + de_DE: '/de', + fr_FR: '/fr', + }, + // tslint:enable: use-camel-case-environment-properties cookieConsentOptions: { options: { required: { From 5d1f1ebccb22630c84e4a83367dca382ecce8eba Mon Sep 17 00:00:00 2001 From: "max.kless@googlemail.com" Date: Wed, 23 Jun 2021 16:27:36 +0200 Subject: [PATCH 03/23] docs: multi site update --- docs/guides/multi-site-configurations.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/docs/guides/multi-site-configurations.md b/docs/guides/multi-site-configurations.md index dcd6dfdb2e..e30f0dbd36 100644 --- a/docs/guides/multi-site-configurations.md +++ b/docs/guides/multi-site-configurations.md @@ -7,6 +7,17 @@ kb_sync_latest_only # Multi Site Configurations +- [Multi Site Configurations](#multi-site-configurations) + - [Syntax](#syntax) + - [Examples](#examples) + - [One domain, One Channel, Multiple Locales](#one-domain-one-channel-multiple-locales) + - [Multiple Domains, Multiple Channels, Multiple Locales](#multiple-domains-multiple-channels-multiple-locales) + - [Multiple Subdomains, Multiple channels, Multiple Locales](#multiple-subdomains-multiple-channels-multiple-locales) + - [Extended Example with Many Different Configurations](#extended-example-with-many-different-configurations) + - [Extended Example with two domains, one with basic auth (except /fr), the other without](#extended-example-with-two-domains-one-with-basic-auth-except-fr-the-other-without) + - [Integrate your multi-site configuration with the language switch](#integrate-your-multi-site-configuration-with-the-language-switch) +- [Further References](#further-references) + As explained in [Multi-Site Handling](../concepts/multi-site-handling.md), the PWA supports dynamic configurations of a single PWA deployment. This guide explains the YAML syntax used to define a configuration and provides some common configuration examples, mainly focusing on different setups for handling locales and channels. For more information about how the YAML configuration is processed, refer to [Multi-Site Handling](../concepts/multi-site-handling.md) and [Building and Running NGINX Docker Image | Multi-Site](../guides/nginx-startup.md#Multi-Site). @@ -205,6 +216,16 @@ ca.+\.com: lang: en_US ``` +## Integrate your multi-site configuration with the language switch + +To construct new multi-site URLs when switching between languages, the PWA uses the `multi-site.service.ts`. The `getLangUpdatedUrl` is called with the desired locale string and the current location. From this it constructs a new URL, conforming to our multi-site setup (see [One domain, one channel, multiple locales](#one-domain-one-channel-multiple-locales)). + +In case you want to disable this functionality, simply override the default environment variable `multiSiteLocaleMap` with `undefined`. + +In case you want to extend this functionality to work with more locales, extend the default environment variable `multiSiteLocaleSetup` with your additional locales. + +In case you want to transfer this functionality to work with your specific multi-site setup, override the `multi-site.service.ts` and provide an implementation that conforms to your setup. + # Further References - [Guide - Building and Running nginx Docker Image](../guides/nginx-startup.md) From 4f46a129e7143e1edcfe3ae8ccc9a6c13b03f0d9 Mon Sep 17 00:00:00 2001 From: "max.kless@googlemail.com" Date: Wed, 23 Jun 2021 16:27:36 +0200 Subject: [PATCH 04/23] feat: multi tests and bugfix --- src/app/core/pipes/make-href.pipe.spec.ts | 10 +++++- src/app/core/pipes/make-href.pipe.ts | 6 ++-- .../multi-site/multi-site.service.spec.ts | 33 ++++++++++++++++--- .../utils/multi-site/multi-site.service.ts | 2 +- 4 files changed, 42 insertions(+), 9 deletions(-) diff --git a/src/app/core/pipes/make-href.pipe.spec.ts b/src/app/core/pipes/make-href.pipe.spec.ts index 45c9eb8ed2..01cca75b07 100644 --- a/src/app/core/pipes/make-href.pipe.spec.ts +++ b/src/app/core/pipes/make-href.pipe.spec.ts @@ -1,16 +1,24 @@ import { LocationStrategy } from '@angular/common'; import { TestBed } from '@angular/core/testing'; +import { anything, instance, mock, when } from 'ts-mockito'; + +import { MultiSiteService } from 'ish-core/utils/multi-site/multi-site.service'; import { MakeHrefPipe } from './make-href.pipe'; describe('Make Href Pipe', () => { let makeHrefPipe: MakeHrefPipe; + let multiSiteService: MultiSiteService; beforeEach(() => { + multiSiteService = mock(MultiSiteService); TestBed.configureTestingModule({ - providers: [MakeHrefPipe], + providers: [MakeHrefPipe, { provide: MultiSiteService, useFactory: () => instance(multiSiteService) }], }); makeHrefPipe = TestBed.inject(MakeHrefPipe); + when(multiSiteService.getLangUpdatedUrl(anything(), anything())).thenCall((_: string, location: LocationStrategy) => + location.path() + ); }); it('should be created', () => { diff --git a/src/app/core/pipes/make-href.pipe.ts b/src/app/core/pipes/make-href.pipe.ts index 9a683a470d..d1204bcf1d 100644 --- a/src/app/core/pipes/make-href.pipe.ts +++ b/src/app/core/pipes/make-href.pipe.ts @@ -11,14 +11,16 @@ export class MakeHrefPipe implements PipeTransform { return 'undefined'; } - const url = this.multiSiteService.getLangUpdatedUrl(urlParams.lang, location); - const split = url.split('?'); + const split = location.path().split('?'); // url without query params let newUrl = split[0]; // add supplied url params if (urlParams) { + if (urlParams.lang) { + newUrl = this.multiSiteService.getLangUpdatedUrl(urlParams.lang, location); + } newUrl += Object.keys(urlParams) .map(k => `;${k}=${urlParams[k]}`) .join(''); diff --git a/src/app/core/utils/multi-site/multi-site.service.spec.ts b/src/app/core/utils/multi-site/multi-site.service.spec.ts index 4b624638b0..5cbe85e01c 100644 --- a/src/app/core/utils/multi-site/multi-site.service.spec.ts +++ b/src/app/core/utils/multi-site/multi-site.service.spec.ts @@ -1,23 +1,46 @@ +import { LocationStrategy } from '@angular/common'; import { TestBed } from '@angular/core/testing'; -import { instance, mock } from 'ts-mockito'; +import { instance, mock, when } from 'ts-mockito'; -import { ApiService } from 'ish-core/services/api/api.service'; +import { MULTI_SITE_LOCALE_MAP } from 'ish-core/configurations/injection-keys'; import { MultiSiteService } from './multi-site.service'; +const testLocaleMap = { + de_DE: '/de', + en_US: '/en', +}; + describe('Multi Site Service', () => { - let apiServiceMock: ApiService; + let mockLocation: LocationStrategy; + let location: LocationStrategy; let multiSiteService: MultiSiteService; beforeEach(() => { - apiServiceMock = mock(ApiService); TestBed.configureTestingModule({ - providers: [{ provide: ApiService, useFactory: () => instance(apiServiceMock) }], + providers: [{ provide: MULTI_SITE_LOCALE_MAP, useFactory: () => testLocaleMap }], }); multiSiteService = TestBed.inject(MultiSiteService); + mockLocation = mock(LocationStrategy); + location = instance(mockLocation); }); it('should be created', () => { expect(multiSiteService).toBeTruthy(); }); + it('should return url unchanged if no language baseHref exists', () => { + when(mockLocation.getBaseHref).thenReturn(() => '/'); + when(mockLocation.path).thenReturn(() => '/testpath'); + expect(multiSiteService.getLangUpdatedUrl('de_DE', location)).toMatchInlineSnapshot(`"/testpath"`); + }); + it('should return with new url if language baseHref exists and language is valid', () => { + when(mockLocation.getBaseHref).thenReturn(() => '/de'); + when(mockLocation.path).thenReturn(() => '/de/testpath'); + expect(multiSiteService.getLangUpdatedUrl('en_US', location)).toMatchInlineSnapshot(`"/en/testpath"`); + }); + it('should return url unchanged if language baseHref exists but language is invalid', () => { + when(mockLocation.getBaseHref).thenReturn(() => '/de'); + when(mockLocation.path).thenReturn(() => '/de/testpath'); + expect(multiSiteService.getLangUpdatedUrl('xy_XY', location)).toMatchInlineSnapshot(`"/de/testpath"`); + }); }); diff --git a/src/app/core/utils/multi-site/multi-site.service.ts b/src/app/core/utils/multi-site/multi-site.service.ts index 42c5f8cbd4..f55411d55b 100644 --- a/src/app/core/utils/multi-site/multi-site.service.ts +++ b/src/app/core/utils/multi-site/multi-site.service.ts @@ -25,7 +25,7 @@ export class MultiSiteService { */ if ( this.multiSiteLocaleMap && - Object.keys(this.multiSiteLocaleMap).includes(location.getBaseHref()) && + Object.values(this.multiSiteLocaleMap).includes(location.getBaseHref()) && localeMapHasLangString(locale, this.multiSiteLocaleMap) ) { newUrl = newUrl.replace(location.getBaseHref(), this.multiSiteLocaleMap[locale]); From 5fb69bbf0526634ac5a9eee43c87ba08d98732b1 Mon Sep 17 00:00:00 2001 From: "max.kless@googlemail.com" Date: Wed, 23 Jun 2021 16:27:37 +0200 Subject: [PATCH 05/23] docs: fix formatting --- docs/guides/multi-site-configurations.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/guides/multi-site-configurations.md b/docs/guides/multi-site-configurations.md index e30f0dbd36..5dfa63e2b6 100644 --- a/docs/guides/multi-site-configurations.md +++ b/docs/guides/multi-site-configurations.md @@ -218,7 +218,9 @@ ca.+\.com: ## Integrate your multi-site configuration with the language switch -To construct new multi-site URLs when switching between languages, the PWA uses the `multi-site.service.ts`. The `getLangUpdatedUrl` is called with the desired locale string and the current location. From this it constructs a new URL, conforming to our multi-site setup (see [One domain, one channel, multiple locales](#one-domain-one-channel-multiple-locales)). +To construct new multi-site URLs when switching between languages, the PWA uses the `multi-site.service.ts`. +The `getLangUpdatedUrl` is called with the desired locale string and the current location. +From this it constructs a new URL, conforming to our multi-site setup (see [One domain, one channel, multiple locales](#one-domain-one-channel-multiple-locales)). In case you want to disable this functionality, simply override the default environment variable `multiSiteLocaleMap` with `undefined`. From 1df96a6d3feb19a545e085158b97985335df4add Mon Sep 17 00:00:00 2001 From: "max.kless@googlemail.com" Date: Wed, 23 Jun 2021 16:27:37 +0200 Subject: [PATCH 06/23] fix: use multi site locale map type --- src/environments/environment.model.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/environments/environment.model.ts b/src/environments/environment.model.ts index 74086f09fb..fa41430a1f 100644 --- a/src/environments/environment.model.ts +++ b/src/environments/environment.model.ts @@ -3,6 +3,7 @@ import { CookieConsentOptions } from 'ish-core/models/cookies/cookies.model'; import { Locale } from 'ish-core/models/locale/locale.model'; import { DeviceType, ViewType } from 'ish-core/models/viewtype/viewtype.types'; import { DataRetentionPolicy } from 'ish-core/utils/meta-reducers'; +import { MultiSiteLocaleMap } from 'ish-core/utils/multi-site/multi-site.service'; import { TactonConfig } from '../app/extensions/tacton/models/tacton-config/tacton-config.model'; @@ -85,7 +86,7 @@ export interface Environment { // configuration of the available locales - hard coded for now locales: Locale[]; - multiSiteLocaleMap: Record | undefined; + multiSiteLocaleMap: MultiSiteLocaleMap; // configuration of the styling theme ('default' if not configured) // format: 'themeName|themeColor' e.g. theme: 'blue|688dc3', From 99633640e67684c23cb11152b7ca2f3ee4c66bc7 Mon Sep 17 00:00:00 2001 From: "max.kless@googlemail.com" Date: Wed, 23 Jun 2021 16:27:37 +0200 Subject: [PATCH 07/23] fix: remove dead code --- .../core/utils/multi-site/multi-site.service.spec.ts | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/app/core/utils/multi-site/multi-site.service.spec.ts b/src/app/core/utils/multi-site/multi-site.service.spec.ts index 5cbe85e01c..f27a70d2f0 100644 --- a/src/app/core/utils/multi-site/multi-site.service.spec.ts +++ b/src/app/core/utils/multi-site/multi-site.service.spec.ts @@ -2,24 +2,15 @@ import { LocationStrategy } from '@angular/common'; import { TestBed } from '@angular/core/testing'; import { instance, mock, when } from 'ts-mockito'; -import { MULTI_SITE_LOCALE_MAP } from 'ish-core/configurations/injection-keys'; - import { MultiSiteService } from './multi-site.service'; -const testLocaleMap = { - de_DE: '/de', - en_US: '/en', -}; - describe('Multi Site Service', () => { let mockLocation: LocationStrategy; let location: LocationStrategy; let multiSiteService: MultiSiteService; beforeEach(() => { - TestBed.configureTestingModule({ - providers: [{ provide: MULTI_SITE_LOCALE_MAP, useFactory: () => testLocaleMap }], - }); + TestBed.configureTestingModule({}); multiSiteService = TestBed.inject(MultiSiteService); mockLocation = mock(LocationStrategy); location = instance(mockLocation); From 2c4b90a21518a0c29d4e558053973c268c67cbcb Mon Sep 17 00:00:00 2001 From: "max.kless@googlemail.com" Date: Wed, 23 Jun 2021 16:27:38 +0200 Subject: [PATCH 08/23] test: add makehref test --- src/app/core/pipes/make-href.pipe.spec.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/app/core/pipes/make-href.pipe.spec.ts b/src/app/core/pipes/make-href.pipe.spec.ts index 01cca75b07..3dc62cb64a 100644 --- a/src/app/core/pipes/make-href.pipe.spec.ts +++ b/src/app/core/pipes/make-href.pipe.spec.ts @@ -1,6 +1,6 @@ import { LocationStrategy } from '@angular/common'; import { TestBed } from '@angular/core/testing'; -import { anything, instance, mock, when } from 'ts-mockito'; +import { anything, instance, mock, verify, when } from 'ts-mockito'; import { MultiSiteService } from 'ish-core/utils/multi-site/multi-site.service'; @@ -38,4 +38,9 @@ describe('Make Href Pipe', () => { ])(`should transform "%s" with %j to "%s"`, (url, params, expected) => { expect(makeHrefPipe.transform({ path: () => url } as LocationStrategy, params)).toEqual(expected); }); + + it('should call the multiSiteService if lang parameter exists', () => { + makeHrefPipe.transform({ path: () => '/de/test' } as LocationStrategy, { lang: 'en_US' }); + verify(multiSiteService.getLangUpdatedUrl(anything(), anything())).once(); + }); }); From 282fd60eac46c01d70e3638722a05fa442cc2b3d Mon Sep 17 00:00:00 2001 From: "max.kless@googlemail.com" Date: Wed, 23 Jun 2021 16:27:38 +0200 Subject: [PATCH 09/23] fix: refactoring after bugfix --- docs/guides/multi-site-configurations.md | 2 +- src/app/core/pipes/make-href.pipe.spec.ts | 12 +++++++----- src/app/core/pipes/make-href.pipe.ts | 2 +- .../multi-site/multi-site.service.spec.ts | 18 +++--------------- .../utils/multi-site/multi-site.service.ts | 9 ++++----- 5 files changed, 16 insertions(+), 27 deletions(-) diff --git a/docs/guides/multi-site-configurations.md b/docs/guides/multi-site-configurations.md index 5dfa63e2b6..b1decba9c6 100644 --- a/docs/guides/multi-site-configurations.md +++ b/docs/guides/multi-site-configurations.md @@ -219,7 +219,7 @@ ca.+\.com: ## Integrate your multi-site configuration with the language switch To construct new multi-site URLs when switching between languages, the PWA uses the `multi-site.service.ts`. -The `getLangUpdatedUrl` is called with the desired locale string and the current location. +The `getLangUpdatedUrl` is called with the desired locale string, current url and current baseHref. From this it constructs a new URL, conforming to our multi-site setup (see [One domain, one channel, multiple locales](#one-domain-one-channel-multiple-locales)). In case you want to disable this functionality, simply override the default environment variable `multiSiteLocaleMap` with `undefined`. diff --git a/src/app/core/pipes/make-href.pipe.spec.ts b/src/app/core/pipes/make-href.pipe.spec.ts index 3dc62cb64a..8113af700e 100644 --- a/src/app/core/pipes/make-href.pipe.spec.ts +++ b/src/app/core/pipes/make-href.pipe.spec.ts @@ -16,8 +16,8 @@ describe('Make Href Pipe', () => { providers: [MakeHrefPipe, { provide: MultiSiteService, useFactory: () => instance(multiSiteService) }], }); makeHrefPipe = TestBed.inject(MakeHrefPipe); - when(multiSiteService.getLangUpdatedUrl(anything(), anything())).thenCall((_: string, location: LocationStrategy) => - location.path() + when(multiSiteService.getLangUpdatedUrl(anything(), anything(), anything())).thenCall( + (url: string, _: LocationStrategy) => url ); }); @@ -36,11 +36,13 @@ describe('Make Href Pipe', () => { ['/test?query=q', { foo: 'bar' }, '/test;foo=bar?query=q'], ['/test?query=q', { foo: 'bar', marco: 'polo' }, '/test;foo=bar;marco=polo?query=q'], ])(`should transform "%s" with %j to "%s"`, (url, params, expected) => { - expect(makeHrefPipe.transform({ path: () => url } as LocationStrategy, params)).toEqual(expected); + expect(makeHrefPipe.transform({ path: () => url, getBaseHref: () => '/' } as LocationStrategy, params)).toEqual( + expected + ); }); it('should call the multiSiteService if lang parameter exists', () => { - makeHrefPipe.transform({ path: () => '/de/test' } as LocationStrategy, { lang: 'en_US' }); - verify(multiSiteService.getLangUpdatedUrl(anything(), anything())).once(); + makeHrefPipe.transform({ path: () => '/de/test', getBaseHref: () => '/de' } as LocationStrategy, { lang: 'en_US' }); + verify(multiSiteService.getLangUpdatedUrl(anything(), anything(), anything())).once(); }); }); diff --git a/src/app/core/pipes/make-href.pipe.ts b/src/app/core/pipes/make-href.pipe.ts index d1204bcf1d..fa49f53f7f 100644 --- a/src/app/core/pipes/make-href.pipe.ts +++ b/src/app/core/pipes/make-href.pipe.ts @@ -19,7 +19,7 @@ export class MakeHrefPipe implements PipeTransform { // add supplied url params if (urlParams) { if (urlParams.lang) { - newUrl = this.multiSiteService.getLangUpdatedUrl(urlParams.lang, location); + newUrl = this.multiSiteService.getLangUpdatedUrl(urlParams.lang, newUrl, location.getBaseHref()); } newUrl += Object.keys(urlParams) .map(k => `;${k}=${urlParams[k]}`) diff --git a/src/app/core/utils/multi-site/multi-site.service.spec.ts b/src/app/core/utils/multi-site/multi-site.service.spec.ts index f27a70d2f0..d49e555563 100644 --- a/src/app/core/utils/multi-site/multi-site.service.spec.ts +++ b/src/app/core/utils/multi-site/multi-site.service.spec.ts @@ -1,37 +1,25 @@ -import { LocationStrategy } from '@angular/common'; import { TestBed } from '@angular/core/testing'; -import { instance, mock, when } from 'ts-mockito'; import { MultiSiteService } from './multi-site.service'; describe('Multi Site Service', () => { - let mockLocation: LocationStrategy; - let location: LocationStrategy; let multiSiteService: MultiSiteService; beforeEach(() => { TestBed.configureTestingModule({}); multiSiteService = TestBed.inject(MultiSiteService); - mockLocation = mock(LocationStrategy); - location = instance(mockLocation); }); it('should be created', () => { expect(multiSiteService).toBeTruthy(); }); it('should return url unchanged if no language baseHref exists', () => { - when(mockLocation.getBaseHref).thenReturn(() => '/'); - when(mockLocation.path).thenReturn(() => '/testpath'); - expect(multiSiteService.getLangUpdatedUrl('de_DE', location)).toMatchInlineSnapshot(`"/testpath"`); + expect(multiSiteService.getLangUpdatedUrl('de_DE', '/testpath', '/')).toMatchInlineSnapshot(`"/testpath"`); }); it('should return with new url if language baseHref exists and language is valid', () => { - when(mockLocation.getBaseHref).thenReturn(() => '/de'); - when(mockLocation.path).thenReturn(() => '/de/testpath'); - expect(multiSiteService.getLangUpdatedUrl('en_US', location)).toMatchInlineSnapshot(`"/en/testpath"`); + expect(multiSiteService.getLangUpdatedUrl('en_US', '/de/testpath', '/de')).toMatchInlineSnapshot(`"/en/testpath"`); }); it('should return url unchanged if language baseHref exists but language is invalid', () => { - when(mockLocation.getBaseHref).thenReturn(() => '/de'); - when(mockLocation.path).thenReturn(() => '/de/testpath'); - expect(multiSiteService.getLangUpdatedUrl('xy_XY', location)).toMatchInlineSnapshot(`"/de/testpath"`); + expect(multiSiteService.getLangUpdatedUrl('xy_XY', '/de/testpath', '/de')).toMatchInlineSnapshot(`"/de/testpath"`); }); }); diff --git a/src/app/core/utils/multi-site/multi-site.service.ts b/src/app/core/utils/multi-site/multi-site.service.ts index f55411d55b..19499edec9 100644 --- a/src/app/core/utils/multi-site/multi-site.service.ts +++ b/src/app/core/utils/multi-site/multi-site.service.ts @@ -1,4 +1,3 @@ -import { LocationStrategy } from '@angular/common'; import { Inject, Injectable } from '@angular/core'; import { MULTI_SITE_LOCALE_MAP } from 'ish-core/configurations/injection-keys'; @@ -15,8 +14,8 @@ export class MultiSiteService { * @param location the current loation * @returns */ - getLangUpdatedUrl(locale: string, location: LocationStrategy): string { - let newUrl = location.path(); + getLangUpdatedUrl(locale: string, url: string, baseHref: string): string { + let newUrl = url; /** * only replace lang parameter if: * - multiSiteLocaleMap environment var is declared (set to undefined to skip this behaviour) @@ -25,10 +24,10 @@ export class MultiSiteService { */ if ( this.multiSiteLocaleMap && - Object.values(this.multiSiteLocaleMap).includes(location.getBaseHref()) && + Object.values(this.multiSiteLocaleMap).includes(baseHref) && localeMapHasLangString(locale, this.multiSiteLocaleMap) ) { - newUrl = newUrl.replace(location.getBaseHref(), this.multiSiteLocaleMap[locale]); + newUrl = newUrl.replace(baseHref, this.multiSiteLocaleMap[locale]); } return newUrl; } From 81c741d7d9e07020c6bf2cf0b0b723aa63c9a591 Mon Sep 17 00:00:00 2001 From: "max.kless@googlemail.com" Date: Wed, 23 Jun 2021 16:27:39 +0200 Subject: [PATCH 10/23] refactor: environment property and review fixes --- docs/guides/multi-site-configurations.md | 2 +- src/app/core/configurations/injection-keys.ts | 7 --- src/app/core/pipes/make-href.pipe.ts | 52 +++++++++------- .../multi-site/multi-site.service.spec.ts | 39 +++++++++--- .../utils/multi-site/multi-site.service.ts | 60 ++++++++++++------- .../language-switch.component.html | 2 +- src/environments/environment.model.ts | 1 - .../useCamelCaseEnvironmentPropertiesRule.ts | 2 +- 8 files changed, 104 insertions(+), 61 deletions(-) diff --git a/docs/guides/multi-site-configurations.md b/docs/guides/multi-site-configurations.md index b1decba9c6..9a2fbbf4c8 100644 --- a/docs/guides/multi-site-configurations.md +++ b/docs/guides/multi-site-configurations.md @@ -224,7 +224,7 @@ From this it constructs a new URL, conforming to our multi-site setup (see [One In case you want to disable this functionality, simply override the default environment variable `multiSiteLocaleMap` with `undefined`. -In case you want to extend this functionality to work with more locales, extend the default environment variable `multiSiteLocaleSetup` with your additional locales. +In case you want to extend this functionality to work with more locales, extend the default environment variable `multiSiteLocaleMap` with your additional locales. In case you want to transfer this functionality to work with your specific multi-site setup, override the `multi-site.service.ts` and provide an implementation that conforms to your setup. diff --git a/src/app/core/configurations/injection-keys.ts b/src/app/core/configurations/injection-keys.ts index 0fd8613b07..6fe9f9745d 100644 --- a/src/app/core/configurations/injection-keys.ts +++ b/src/app/core/configurations/injection-keys.ts @@ -58,13 +58,6 @@ export const DATA_RETENTION_POLICY = new InjectionToken('da factory: () => environment.dataRetention, }); -/** - * the locale to multi site configuration map - */ -export const MULTI_SITE_LOCALE_MAP = new InjectionToken | undefined>('multiSiteLocaleMap', { - factory: () => environment.multiSiteLocaleMap, -}); - /* * global definition of the Bootstrap grid system breakpoint widths */ diff --git a/src/app/core/pipes/make-href.pipe.ts b/src/app/core/pipes/make-href.pipe.ts index fa49f53f7f..bc84ae058c 100644 --- a/src/app/core/pipes/make-href.pipe.ts +++ b/src/app/core/pipes/make-href.pipe.ts @@ -1,35 +1,45 @@ import { LocationStrategy } from '@angular/common'; import { Pipe, PipeTransform } from '@angular/core'; +import { Observable, of } from 'rxjs'; +import { map, switchMap } from 'rxjs/operators'; +import { omit } from 'ish-core/utils/functions'; import { MultiSiteService } from 'ish-core/utils/multi-site/multi-site.service'; @Pipe({ name: 'makeHref', pure: false }) export class MakeHrefPipe implements PipeTransform { constructor(private multiSiteService: MultiSiteService) {} - transform(location: LocationStrategy, urlParams: Record): string { + transform(location: LocationStrategy, urlParams: Record): Observable { if (!location || !location.path()) { - return 'undefined'; + return of('undefined'); } - const split = location.path().split('?'); + return of(location.path().split('?')).pipe( + switchMap(split => { + // url without query params + const newUrl = split[0]; - // url without query params - let newUrl = split[0]; - - // add supplied url params - if (urlParams) { - if (urlParams.lang) { - newUrl = this.multiSiteService.getLangUpdatedUrl(urlParams.lang, newUrl, location.getBaseHref()); - } - newUrl += Object.keys(urlParams) - .map(k => `;${k}=${urlParams[k]}`) - .join(''); - } - - // add query params at the end - if (split.length > 1) { - newUrl += `?${split[1]}`; - } - return newUrl; + if (urlParams) { + if (urlParams.lang) { + return this.multiSiteService.getLangUpdatedUrl(urlParams.lang, newUrl, location.getBaseHref()).pipe( + map(modifiedUrl => { + const modifiedUrlParams = modifiedUrl === newUrl ? urlParams : omit(urlParams, 'lang'); + return appendUrlParams(modifiedUrl, modifiedUrlParams, split?.[1]); + }) + ); + } else { + return of(newUrl).pipe(map(url => appendUrlParams(url, urlParams, split?.[1]))); + } + } + }) + ); } } + +function appendUrlParams(url: string, urlParams: Record, queryParams: string | undefined): string { + return ` + ${url}${Object.keys(urlParams) + .map(k => `;${k}=${urlParams[k]}`) + .join('')}${queryParams ? `?${queryParams}` : ''} + `; +} diff --git a/src/app/core/utils/multi-site/multi-site.service.spec.ts b/src/app/core/utils/multi-site/multi-site.service.spec.ts index d49e555563..b42045761f 100644 --- a/src/app/core/utils/multi-site/multi-site.service.spec.ts +++ b/src/app/core/utils/multi-site/multi-site.service.spec.ts @@ -1,25 +1,50 @@ import { TestBed } from '@angular/core/testing'; +import { of } from 'rxjs'; +import { anything, instance, mock, when } from 'ts-mockito'; + +import { StatePropertiesService } from 'ish-core/utils/state-transfer/state-properties.service'; import { MultiSiteService } from './multi-site.service'; describe('Multi Site Service', () => { let multiSiteService: MultiSiteService; + let statePropertiesService: StatePropertiesService; beforeEach(() => { - TestBed.configureTestingModule({}); + statePropertiesService = mock(StatePropertiesService); + TestBed.configureTestingModule({ + providers: [{ provide: StatePropertiesService, useFactory: () => instance(statePropertiesService) }], + }); multiSiteService = TestBed.inject(MultiSiteService); + + when(statePropertiesService.getStateOrEnvOrDefault(anything(), anything())).thenReturn( + of({ + en_US: '/en', + de_DE: '/de', + fr_FR: '/fr', + }) + ); }); it('should be created', () => { expect(multiSiteService).toBeTruthy(); }); - it('should return url unchanged if no language baseHref exists', () => { - expect(multiSiteService.getLangUpdatedUrl('de_DE', '/testpath', '/')).toMatchInlineSnapshot(`"/testpath"`); + it('should return url unchanged if no language baseHref exists', done => { + multiSiteService.getLangUpdatedUrl('de_DE', '/testpath', '/').subscribe(url => { + expect(url).toMatchInlineSnapshot(`"/testpath"`); + done(); + }); }); - it('should return with new url if language baseHref exists and language is valid', () => { - expect(multiSiteService.getLangUpdatedUrl('en_US', '/de/testpath', '/de')).toMatchInlineSnapshot(`"/en/testpath"`); + it('should return with new url if language baseHref exists and language is valid', done => { + multiSiteService.getLangUpdatedUrl('en_US', '/de/testpath', '/de').subscribe(url => { + expect(url).toMatchInlineSnapshot(`"/en/testpath"`); + done(); + }); }); - it('should return url unchanged if language baseHref exists but language is invalid', () => { - expect(multiSiteService.getLangUpdatedUrl('xy_XY', '/de/testpath', '/de')).toMatchInlineSnapshot(`"/de/testpath"`); + it('should return url unchanged if language baseHref exists but language is invalid', done => { + multiSiteService.getLangUpdatedUrl('xy_XY', '/de/testpath', '/de').subscribe(url => { + expect(url).toMatchInlineSnapshot(`"/de/testpath"`); + done(); + }); }); }); diff --git a/src/app/core/utils/multi-site/multi-site.service.ts b/src/app/core/utils/multi-site/multi-site.service.ts index 19499edec9..12e71de5ba 100644 --- a/src/app/core/utils/multi-site/multi-site.service.ts +++ b/src/app/core/utils/multi-site/multi-site.service.ts @@ -1,35 +1,51 @@ -import { Inject, Injectable } from '@angular/core'; +import { Injectable, OnDestroy } from '@angular/core'; +import { Observable, Subject } from 'rxjs'; +import { map, take, takeUntil } from 'rxjs/operators'; -import { MULTI_SITE_LOCALE_MAP } from 'ish-core/configurations/injection-keys'; +import { StatePropertiesService } from 'ish-core/utils/state-transfer/state-properties.service'; export type MultiSiteLocaleMap = Record | undefined; @Injectable({ providedIn: 'root' }) -export class MultiSiteService { - constructor(@Inject(MULTI_SITE_LOCALE_MAP) private multiSiteLocaleMap: MultiSiteLocaleMap) {} +export class MultiSiteService implements OnDestroy { + private destroy$ = new Subject(); + constructor(private stateProperties: StatePropertiesService) {} /** * returns the current url, modified to fit the locale parameter if the environment parameter "multiSiteLocaleMap" is set * @param locale the locale which the new url should fit - * @param location the current loation - * @returns + * @param location the current location + * @returns the current url, modified to fit the locale parameter */ - getLangUpdatedUrl(locale: string, url: string, baseHref: string): string { - let newUrl = url; - /** - * only replace lang parameter if: - * - multiSiteLocaleMap environment var is declared (set to undefined to skip this behaviour) - * - current baseHref is part of the map (so the empty "/" baseHref would not be replaced) - * - multiSiteLocaleMap contains target locale - */ - if ( - this.multiSiteLocaleMap && - Object.values(this.multiSiteLocaleMap).includes(baseHref) && - localeMapHasLangString(locale, this.multiSiteLocaleMap) - ) { - newUrl = newUrl.replace(baseHref, this.multiSiteLocaleMap[locale]); - } - return newUrl; + getLangUpdatedUrl(locale: string, url: string, baseHref: string): Observable { + return this.stateProperties + .getStateOrEnvOrDefault>('MULTI_SITE_LOCALE_MAP', 'multiSiteLocaleMap') + .pipe( + take(1), + map(multiSiteLocaleMap => { + let newUrl = url; + /** + * only replace lang parameter if: + * - multiSiteLocaleMap environment var is declared (set to undefined to skip this behaviour) + * - current baseHref is part of the map (so the empty "/" baseHref would not be replaced) + * - multiSiteLocaleMap contains target locale + */ + if ( + multiSiteLocaleMap && + Object.values(multiSiteLocaleMap).includes(baseHref) && + localeMapHasLangString(locale, multiSiteLocaleMap) + ) { + newUrl = newUrl.replace(baseHref, multiSiteLocaleMap[locale]); + } + return newUrl; + }), + takeUntil(this.destroy$) + ); + } + + ngOnDestroy(): void { + this.destroy$.next(); + this.destroy$.complete(); } } diff --git a/src/app/shell/header/language-switch/language-switch.component.html b/src/app/shell/header/language-switch/language-switch.component.html index e970e37f51..42b98918be 100644 --- a/src/app/shell/header/language-switch/language-switch.component.html +++ b/src/app/shell/header/language-switch/language-switch.component.html @@ -17,7 +17,7 @@ diff --git a/src/environments/environment.model.ts b/src/environments/environment.model.ts index fa41430a1f..ef01eb773f 100644 --- a/src/environments/environment.model.ts +++ b/src/environments/environment.model.ts @@ -140,7 +140,6 @@ export const ENVIRONMENT_DEFAULTS: Environment = { { lang: 'de_DE', currency: 'EUR', value: 'de', displayName: 'German', displayLong: 'German (Germany)' }, { lang: 'fr_FR', currency: 'EUR', value: 'fr', displayName: 'French', displayLong: 'French (France)' }, ], - // tslint:disable: use-camel-case-environment-properties multiSiteLocaleMap: { en_US: '/en', de_DE: '/de', diff --git a/tslint-rules/src/useCamelCaseEnvironmentPropertiesRule.ts b/tslint-rules/src/useCamelCaseEnvironmentPropertiesRule.ts index 277b6b5589..931626134f 100644 --- a/tslint-rules/src/useCamelCaseEnvironmentPropertiesRule.ts +++ b/tslint-rules/src/useCamelCaseEnvironmentPropertiesRule.ts @@ -10,7 +10,7 @@ export class Rule extends Lint.Rules.AbstractRule { return this.applyWithFunction(sourceFile, ctx => { tsquery(sourceFile, 'PropertyAssignment').forEach((token: PropertyAssignment) => { const identifier = token.name.getText(); - if (!identifier.match(/^[a-z][A-Za-z0-9]*$/)) { + if (!identifier.match(/^([a-z][A-Za-z0-9]*|[a-z][a-z]_[A-Z][A-Z])$/)) { ctx.addFailureAtNode(token, `Property ${token.getText()} is not camelCase formatted.`); } }); From 628f24e04aadd08709a26ca86a6d6e9d72c5ff32 Mon Sep 17 00:00:00 2001 From: "max.kless@googlemail.com" Date: Wed, 23 Jun 2021 17:42:24 +0200 Subject: [PATCH 11/23] test: test fixes --- package-lock.json | 30 ++++++++++++++++++++++- src/app/core/pipes/make-href.pipe.spec.ts | 27 ++++++++++++-------- src/app/core/pipes/make-href.pipe.ts | 14 +++++++---- 3 files changed, 55 insertions(+), 16 deletions(-) diff --git a/package-lock.json b/package-lock.json index 2d11d098ae..fff16688bb 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4798,6 +4798,16 @@ "integrity": "sha512-jDctJ/IVQbZoJykoeHbhXpOlNBqGNcwXJKJog42E5HDPUwQTSdjCHdihjj0DlnheQ7blbT6dHOafNAiS8ooQKA==", "dev": true }, + "bindings": { + "version": "1.5.0", + "resolved": "https://registry.npmjs.org/bindings/-/bindings-1.5.0.tgz", + "integrity": "sha512-p2q/t/mhvuOj/UeLlV6566GD/guowlr0hHxClI0W9m7MWYkL1F0hLo+0Aexs9HSPCtR1SXQ0TD3MMKrXZajbiQ==", + "dev": true, + "optional": true, + "requires": { + "file-uri-to-path": "1.0.0" + } + }, "bintrees": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/bintrees/-/bintrees-1.0.1.tgz", @@ -8809,6 +8819,13 @@ "flat-cache": "^3.0.4" } }, + "file-uri-to-path": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/file-uri-to-path/-/file-uri-to-path-1.0.0.tgz", + "integrity": "sha512-0Zt+s3L7Vf1biwWZ29aARiVYLx7iMGnEUl9x33fbB/j3jR81u/O2LbqK+Bm1CDSNDKVtJ/YjwY7TUd5SkeLQLw==", + "dev": true, + "optional": true + }, "fill-range": { "version": "7.0.1", "resolved": "https://registry.npmjs.org/fill-range/-/fill-range-7.0.1.tgz", @@ -14293,6 +14310,13 @@ "integrity": "sha512-nnbWWOkoWyUsTjKrhgD0dcz22mdkSnpYqbEjIm2nhwhuxlSkpywJmBo8h0ZqJdkp73mb90SssHkN4rsRaBAfAA==", "dev": true }, + "nan": { + "version": "2.14.2", + "resolved": "https://registry.npmjs.org/nan/-/nan-2.14.2.tgz", + "integrity": "sha512-M2ufzIiINKCuDfBSAUr1vWQ+vuVcA9kqx8JJUsbQi6yf1uGRyb7HfpdfUr5qLXf3B/t8dPvcjhKMmlfnP47EzQ==", + "dev": true, + "optional": true + }, "nanoid": { "version": "3.1.23", "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.1.23.tgz", @@ -21918,7 +21942,11 @@ "resolved": "https://registry.npmjs.org/fsevents/-/fsevents-1.2.13.tgz", "integrity": "sha512-oWb1Z6mkHIskLzEJ/XWX0srkpkTQ7vaopMQkyaEIoq0fmtFVxOthb8cCxeT+p3ynTdkk/RZwbgG4brR5BeWECw==", "dev": true, - "optional": true + "optional": true, + "requires": { + "bindings": "^1.5.0", + "nan": "^2.12.1" + } }, "glob-parent": { "version": "3.1.0", diff --git a/src/app/core/pipes/make-href.pipe.spec.ts b/src/app/core/pipes/make-href.pipe.spec.ts index 8113af700e..cd0547c152 100644 --- a/src/app/core/pipes/make-href.pipe.spec.ts +++ b/src/app/core/pipes/make-href.pipe.spec.ts @@ -1,5 +1,6 @@ import { LocationStrategy } from '@angular/common'; import { TestBed } from '@angular/core/testing'; +import { of } from 'rxjs'; import { anything, instance, mock, verify, when } from 'ts-mockito'; import { MultiSiteService } from 'ish-core/utils/multi-site/multi-site.service'; @@ -17,15 +18,16 @@ describe('Make Href Pipe', () => { }); makeHrefPipe = TestBed.inject(MakeHrefPipe); when(multiSiteService.getLangUpdatedUrl(anything(), anything(), anything())).thenCall( - (url: string, _: LocationStrategy) => url + (url: string, _: LocationStrategy) => of(url) ); }); it('should be created', () => { expect(makeHrefPipe).toBeTruthy(); }); - - it.each([ + // workaround for https://github.com/DefinitelyTyped/DefinitelyTyped/issues/34617 + // tslint:disable-next-line: no-any + it.each([ [undefined, undefined, 'undefined'], ['/test', undefined, '/test'], ['/test', {}, '/test'], @@ -35,14 +37,19 @@ describe('Make Href Pipe', () => { ['/test?query=q', {}, '/test?query=q'], ['/test?query=q', { foo: 'bar' }, '/test;foo=bar?query=q'], ['/test?query=q', { foo: 'bar', marco: 'polo' }, '/test;foo=bar;marco=polo?query=q'], - ])(`should transform "%s" with %j to "%s"`, (url, params, expected) => { - expect(makeHrefPipe.transform({ path: () => url, getBaseHref: () => '/' } as LocationStrategy, params)).toEqual( - expected - ); + ])(`should transform "%s" with %j to "%s"`, (url, params, expected, done: jest.DoneCallback) => { + makeHrefPipe.transform({ path: () => url, getBaseHref: () => '/' } as LocationStrategy, params).subscribe(res => { + expect(res).toEqual(expected); + done(); + }); }); - it('should call the multiSiteService if lang parameter exists', () => { - makeHrefPipe.transform({ path: () => '/de/test', getBaseHref: () => '/de' } as LocationStrategy, { lang: 'en_US' }); - verify(multiSiteService.getLangUpdatedUrl(anything(), anything(), anything())).once(); + it('should call the multiSiteService if lang parameter exists', done => { + makeHrefPipe + .transform({ path: () => '/de/test', getBaseHref: () => '/de' } as LocationStrategy, { lang: 'en_US' }) + .subscribe(() => { + verify(multiSiteService.getLangUpdatedUrl(anything(), anything(), anything())).once(); + done(); + }); }); }); diff --git a/src/app/core/pipes/make-href.pipe.ts b/src/app/core/pipes/make-href.pipe.ts index bc84ae058c..8a6c795751 100644 --- a/src/app/core/pipes/make-href.pipe.ts +++ b/src/app/core/pipes/make-href.pipe.ts @@ -30,6 +30,8 @@ export class MakeHrefPipe implements PipeTransform { } else { return of(newUrl).pipe(map(url => appendUrlParams(url, urlParams, split?.[1]))); } + } else { + return of(appendUrlParams(newUrl, undefined, split?.[1])); } }) ); @@ -37,9 +39,11 @@ export class MakeHrefPipe implements PipeTransform { } function appendUrlParams(url: string, urlParams: Record, queryParams: string | undefined): string { - return ` - ${url}${Object.keys(urlParams) - .map(k => `;${k}=${urlParams[k]}`) - .join('')}${queryParams ? `?${queryParams}` : ''} - `; + return `${url}${ + urlParams + ? Object.keys(urlParams) + .map(k => `;${k}=${urlParams[k]}`) + .join('') + : '' + }${queryParams ? `?${queryParams}` : ''}`; } From 14c828528732ff691ad7e0e6c0fa350a6456effe Mon Sep 17 00:00:00 2001 From: "max.kless@googlemail.com" Date: Wed, 23 Jun 2021 18:00:11 +0200 Subject: [PATCH 12/23] test: fix language test --- .../language-switch.component.spec.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/app/shell/header/language-switch/language-switch.component.spec.ts b/src/app/shell/header/language-switch/language-switch.component.spec.ts index 6f8b85fca8..8b0492605f 100644 --- a/src/app/shell/header/language-switch/language-switch.component.spec.ts +++ b/src/app/shell/header/language-switch/language-switch.component.spec.ts @@ -3,7 +3,7 @@ import { Router } from '@angular/router'; import { RouterTestingModule } from '@angular/router/testing'; import { FaIconComponent } from '@fortawesome/angular-fontawesome'; import { NgbDropdownModule } from '@ng-bootstrap/ng-bootstrap'; -import { MockComponent } from 'ng-mocks'; +import { MockComponent, MockPipe } from 'ng-mocks'; import { of } from 'rxjs'; import { instance, mock, when } from 'ts-mockito'; @@ -18,6 +18,7 @@ describe('Language Switch Component', () => { let fixture: ComponentFixture; let element: HTMLElement; let appFacade: AppFacade; + const locales = [ { lang: 'en_US', value: 'en', displayName: 'English' }, { lang: 'de_DE', value: 'de', displayName: 'Deutsch' }, @@ -28,7 +29,11 @@ describe('Language Switch Component', () => { appFacade = mock(AppFacade); await TestBed.configureTestingModule({ - declarations: [LanguageSwitchComponent, MakeHrefPipe, MockComponent(FaIconComponent)], + declarations: [ + LanguageSwitchComponent, + MockComponent(FaIconComponent), + MockPipe(MakeHrefPipe, (_, urlParams) => of(urlParams.lang)), + ], imports: [NgbDropdownModule, RouterTestingModule], providers: [{ provide: AppFacade, useFactory: () => instance(appFacade) }], }).compileComponents(); @@ -57,8 +62,8 @@ describe('Language Switch Component', () => { expect(element.querySelectorAll('li')).toHaveLength(2); expect(element.querySelectorAll('[href]')).toMatchInlineSnapshot(` NodeList [ - English , - Fran¢aise , + English , + Fran¢aise , ] `); expect(element.querySelector('.language-switch-current-selection').textContent).toMatchInlineSnapshot(`"de"`); From 3ee8b054b6b75c35603cf269686b247dfa6ae2a2 Mon Sep 17 00:00:00 2001 From: "max.kless@googlemail.com" Date: Thu, 24 Jun 2021 09:12:56 +0200 Subject: [PATCH 13/23] fix: remove destroy$ --- .../core/utils/multi-site/multi-site.service.ts | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/app/core/utils/multi-site/multi-site.service.ts b/src/app/core/utils/multi-site/multi-site.service.ts index 12e71de5ba..b17bbca596 100644 --- a/src/app/core/utils/multi-site/multi-site.service.ts +++ b/src/app/core/utils/multi-site/multi-site.service.ts @@ -1,14 +1,13 @@ -import { Injectable, OnDestroy } from '@angular/core'; -import { Observable, Subject } from 'rxjs'; -import { map, take, takeUntil } from 'rxjs/operators'; +import { Injectable } from '@angular/core'; +import { Observable } from 'rxjs'; +import { map, take } from 'rxjs/operators'; import { StatePropertiesService } from 'ish-core/utils/state-transfer/state-properties.service'; export type MultiSiteLocaleMap = Record | undefined; @Injectable({ providedIn: 'root' }) -export class MultiSiteService implements OnDestroy { - private destroy$ = new Subject(); +export class MultiSiteService { constructor(private stateProperties: StatePropertiesService) {} /** @@ -38,15 +37,9 @@ export class MultiSiteService implements OnDestroy { newUrl = newUrl.replace(baseHref, multiSiteLocaleMap[locale]); } return newUrl; - }), - takeUntil(this.destroy$) + }) ); } - - ngOnDestroy(): void { - this.destroy$.next(); - this.destroy$.complete(); - } } function localeMapHasLangString( From 6fd3d9b1938640a03e4df854d2a3df952cc14d91 Mon Sep 17 00:00:00 2001 From: "max.kless@googlemail.com" Date: Thu, 24 Jun 2021 17:25:01 +0200 Subject: [PATCH 14/23] review fixes --- docs/guides/ssr-startup.md | 37 ++++++++++--------- .../utils/multi-site/multi-site.service.ts | 5 ++- .../language-switch.component.html | 2 +- src/environments/environment.model.ts | 1 - 4 files changed, 23 insertions(+), 22 deletions(-) diff --git a/docs/guides/ssr-startup.md b/docs/guides/ssr-startup.md index d8ac0ba03f..03ebd65b7b 100644 --- a/docs/guides/ssr-startup.md +++ b/docs/guides/ssr-startup.md @@ -34,24 +34,25 @@ If the format is _switch_, the property is switched on by supplying `on`, `1`, ` All parameters are **case sensitive**. Make sure to use them as written in the table below. -| | parameter | format | comment | -| ------------------- | --------------------- | -------------------- | ------------------------------------------------------------ | -| **SSR Specific** | PORT | number | Port for running the application | -| | SSL | any | Enables SSL/TLS | -| **General** | ICM_BASE_URL | string | Sets the base URL for the ICM | -| | ICM_CHANNEL | string | Overrides the default channel | -| | ICM_APPLICATION | string | Overrides the default application | -| | FEATURES | comma-separated list | Overrides active features | -| | THEME | string | Overrides the default theme | -| **Debug** :warning: | TRUST_ICM | any | Use this if ICM is deployed with an insecure certificate | -| | LOGGING | switch | Enables extra log output | -| **Hybrid Approach** | SSR_HYBRID | any | Enables running PWA and ICM in [Hybrid Mode][concept-hybrid] | -| | PROXY_ICM | any \| URL | Proxy ICM via `/INTERSHOP` (enabled if SSR_HYBRID is active) | -| **Third party** | GTM_TOKEN | string | Token for Google Tag Manager | -| | SENTRY_DSN | string | Sentry DSN URL for using Sentry Error Monitor | -| | PROMETHEUS | switch | Exposes Prometheus metrics | -| | ICM_IDENTITY_PROVIDER | string | ID of Identity Provider for [SSO][concept-sso] | -| | IDENTITY_PROVIDERS | JSON | Configuration of Identity Providers for [SSO][concept-sso] | +| | parameter | format | comment | +| ------------------- | --------------------- | -------------------- | -------------------------------------------------------------------------------------------- | +| **SSR Specific** | PORT | number | Port for running the application | +| | SSL | any | Enables SSL/TLS | +| **General** | ICM_BASE_URL | string | Sets the base URL for the ICM | +| | ICM_CHANNEL | string | Overrides the default channel | +| | ICM_APPLICATION | string | Overrides the default application | +| | FEATURES | comma-separated list | Overrides active features | +| | THEME | string | Overrides the default theme | +| | MULTI_SITE_LOCALE_MAP | JSON | Used to map locales to [url modification parameters](../guides/multi-site-configurations.md) | +| **Debug** :warning: | TRUST_ICM | any | Use this if ICM is deployed with an insecure certificate | +| | LOGGING | switch | Enables extra log output | +| **Hybrid Approach** | SSR_HYBRID | any | Enables running PWA and ICM in [Hybrid Mode][concept-hybrid] | +| | PROXY_ICM | any \| URL | Proxy ICM via `/INTERSHOP` (enabled if SSR_HYBRID is active) | +| **Third party** | GTM_TOKEN | string | Token for Google Tag Manager | +| | SENTRY_DSN | string | Sentry DSN URL for using Sentry Error Monitor | +| | PROMETHEUS | switch | Exposes Prometheus metrics | +| | ICM_IDENTITY_PROVIDER | string | ID of Identity Provider for [SSO][concept-sso] | +| | IDENTITY_PROVIDERS | JSON | Configuration of Identity Providers for [SSO][concept-sso] | ## Running with https diff --git a/src/app/core/utils/multi-site/multi-site.service.ts b/src/app/core/utils/multi-site/multi-site.service.ts index b17bbca596..85a5f55752 100644 --- a/src/app/core/utils/multi-site/multi-site.service.ts +++ b/src/app/core/utils/multi-site/multi-site.service.ts @@ -13,8 +13,9 @@ export class MultiSiteService { /** * returns the current url, modified to fit the locale parameter if the environment parameter "multiSiteLocaleMap" is set * @param locale the locale which the new url should fit - * @param location the current location - * @returns the current url, modified to fit the locale parameter + * @param url the current url + * @param baseHref the current baseHref which needs to be replaced + * @returns the modified url */ getLangUpdatedUrl(locale: string, url: string, baseHref: string): Observable { return this.stateProperties diff --git a/src/app/shell/header/language-switch/language-switch.component.html b/src/app/shell/header/language-switch/language-switch.component.html index 42b98918be..27b62d184d 100644 --- a/src/app/shell/header/language-switch/language-switch.component.html +++ b/src/app/shell/header/language-switch/language-switch.component.html @@ -17,7 +17,7 @@ diff --git a/src/environments/environment.model.ts b/src/environments/environment.model.ts index ef01eb773f..9ea9bd8a8f 100644 --- a/src/environments/environment.model.ts +++ b/src/environments/environment.model.ts @@ -145,7 +145,6 @@ export const ENVIRONMENT_DEFAULTS: Environment = { de_DE: '/de', fr_FR: '/fr', }, - // tslint:enable: use-camel-case-environment-properties cookieConsentOptions: { options: { required: { From 0c154104debb06971c0010fa99272de98f507cf1 Mon Sep 17 00:00:00 2001 From: "max.kless@googlemail.com" Date: Thu, 24 Jun 2021 18:13:53 +0200 Subject: [PATCH 15/23] chore: remove unnecessary package lock changes --- package-lock.json | 30 +----------------------------- 1 file changed, 1 insertion(+), 29 deletions(-) diff --git a/package-lock.json b/package-lock.json index fff16688bb..2d11d098ae 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4798,16 +4798,6 @@ "integrity": "sha512-jDctJ/IVQbZoJykoeHbhXpOlNBqGNcwXJKJog42E5HDPUwQTSdjCHdihjj0DlnheQ7blbT6dHOafNAiS8ooQKA==", "dev": true }, - "bindings": { - "version": "1.5.0", - "resolved": "https://registry.npmjs.org/bindings/-/bindings-1.5.0.tgz", - "integrity": "sha512-p2q/t/mhvuOj/UeLlV6566GD/guowlr0hHxClI0W9m7MWYkL1F0hLo+0Aexs9HSPCtR1SXQ0TD3MMKrXZajbiQ==", - "dev": true, - "optional": true, - "requires": { - "file-uri-to-path": "1.0.0" - } - }, "bintrees": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/bintrees/-/bintrees-1.0.1.tgz", @@ -8819,13 +8809,6 @@ "flat-cache": "^3.0.4" } }, - "file-uri-to-path": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/file-uri-to-path/-/file-uri-to-path-1.0.0.tgz", - "integrity": "sha512-0Zt+s3L7Vf1biwWZ29aARiVYLx7iMGnEUl9x33fbB/j3jR81u/O2LbqK+Bm1CDSNDKVtJ/YjwY7TUd5SkeLQLw==", - "dev": true, - "optional": true - }, "fill-range": { "version": "7.0.1", "resolved": "https://registry.npmjs.org/fill-range/-/fill-range-7.0.1.tgz", @@ -14310,13 +14293,6 @@ "integrity": "sha512-nnbWWOkoWyUsTjKrhgD0dcz22mdkSnpYqbEjIm2nhwhuxlSkpywJmBo8h0ZqJdkp73mb90SssHkN4rsRaBAfAA==", "dev": true }, - "nan": { - "version": "2.14.2", - "resolved": "https://registry.npmjs.org/nan/-/nan-2.14.2.tgz", - "integrity": "sha512-M2ufzIiINKCuDfBSAUr1vWQ+vuVcA9kqx8JJUsbQi6yf1uGRyb7HfpdfUr5qLXf3B/t8dPvcjhKMmlfnP47EzQ==", - "dev": true, - "optional": true - }, "nanoid": { "version": "3.1.23", "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.1.23.tgz", @@ -21942,11 +21918,7 @@ "resolved": "https://registry.npmjs.org/fsevents/-/fsevents-1.2.13.tgz", "integrity": "sha512-oWb1Z6mkHIskLzEJ/XWX0srkpkTQ7vaopMQkyaEIoq0fmtFVxOthb8cCxeT+p3ynTdkk/RZwbgG4brR5BeWECw==", "dev": true, - "optional": true, - "requires": { - "bindings": "^1.5.0", - "nan": "^2.12.1" - } + "optional": true }, "glob-parent": { "version": "3.1.0", From 8b7b69610bd90458c2a94adf3c4c49cf9a922893 Mon Sep 17 00:00:00 2001 From: Stefan Hauke Date: Fri, 25 Jun 2021 11:56:45 +0200 Subject: [PATCH 16/23] fixup! review fixes --- src/environments/environment.model.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/environments/environment.model.ts b/src/environments/environment.model.ts index 9ea9bd8a8f..801ad2e1d7 100644 --- a/src/environments/environment.model.ts +++ b/src/environments/environment.model.ts @@ -84,8 +84,10 @@ export interface Environment { defaultLocale?: string; - // configuration of the available locales - hard coded for now + // configuration of the possible locales (filtered by the locales activated by the server) locales: Locale[]; + + // multi-site URLs to locales mapping ('undefined' if mapping should not be used) multiSiteLocaleMap: MultiSiteLocaleMap; // configuration of the styling theme ('default' if not configured) From 6c98b1a040fdc59be4c2e66536cb3022a042fc30 Mon Sep 17 00:00:00 2001 From: Stefan Hauke Date: Fri, 25 Jun 2021 11:58:36 +0200 Subject: [PATCH 17/23] We need a working MULTI_SITE_LOCALE_MAP example that can be here as commented example (this combination is not yet working) --- docker-compose.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 30a659ce74..9ccd48a9f9 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -18,6 +18,7 @@ services: # - PROXY_ICM=true - TRUST_ICM=true # - PROMETHEUS=on + - MULTI_SITE_LOCALE_MAP={"en_US":"/enEN","de_DE":"/deDE","fr_FR":"/frFR"} nginx: build: nginx depends_on: @@ -37,13 +38,13 @@ services: # - 1.2.3.4 MULTI_CHANNEL: | .+: - - baseHref: /en + - baseHref: /enEN channel: default lang: en_US - - baseHref: /de + - baseHref: /deDE channel: default lang: de_DE - - baseHref: /fr + - baseHref: /frFR channel: default lang: fr_FR From f228bb1111bc9b17c4c9b9f3e648ecb94af275ba Mon Sep 17 00:00:00 2001 From: "max.kless@googlemail.com" Date: Mon, 28 Jun 2021 16:35:51 +0200 Subject: [PATCH 18/23] fix: transfer multiSiteLocaleMap works --- docker-compose.yml | 8 ++-- docs/guides/multi-site-configurations.md | 6 ++- .../configuration/configuration.effects.ts | 23 +++++++++ .../configuration/configuration.reducer.ts | 2 + .../configuration/configuration.selectors.ts | 5 ++ .../utils/multi-site/multi-site.service.ts | 48 +++++++++---------- 6 files changed, 62 insertions(+), 30 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 9ccd48a9f9..ab4d145991 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -18,7 +18,7 @@ services: # - PROXY_ICM=true - TRUST_ICM=true # - PROMETHEUS=on - - MULTI_SITE_LOCALE_MAP={"en_US":"/enEN","de_DE":"/deDE","fr_FR":"/frFR"} + # - MULTI_SITE_LOCALE_MAP={"en_US":"/en","de_DE":"/de","fr_FR":"/fr"} nginx: build: nginx depends_on: @@ -38,13 +38,13 @@ services: # - 1.2.3.4 MULTI_CHANNEL: | .+: - - baseHref: /enEN + - baseHref: /en channel: default lang: en_US - - baseHref: /deDE + - baseHref: /de channel: default lang: de_DE - - baseHref: /frFR + - baseHref: /fr channel: default lang: fr_FR diff --git a/docs/guides/multi-site-configurations.md b/docs/guides/multi-site-configurations.md index 9a2fbbf4c8..54c55284f2 100644 --- a/docs/guides/multi-site-configurations.md +++ b/docs/guides/multi-site-configurations.md @@ -222,11 +222,13 @@ To construct new multi-site URLs when switching between languages, the PWA uses The `getLangUpdatedUrl` is called with the desired locale string, current url and current baseHref. From this it constructs a new URL, conforming to our multi-site setup (see [One domain, one channel, multiple locales](#one-domain-one-channel-multiple-locales)). -In case you want to disable this functionality, simply override the default environment variable `multiSiteLocaleMap` with `undefined`. +To control the transformation of urls, the `multiSiteLocaleMap` environment variable is used. Depending on your needs, `multiSiteLocaleMap` can be set in either the `environment.ts` or as an environment variable (`MULTI_SITE_LOCALE_MAP`). See [`docker-compose.yml`](../../docker-compose.yml) for a commented out example or [`environment.model.ts`](../../src/environments/environment.model.ts) for the default value. + +In case you want to disable this functionality, simply override the default environment variable `multiSiteLocaleMap` with `undefined` or `MULTI_SITE_LOCALE_MAP` with `false`. In case you want to extend this functionality to work with more locales, extend the default environment variable `multiSiteLocaleMap` with your additional locales. -In case you want to transfer this functionality to work with your specific multi-site setup, override the `multi-site.service.ts` and provide an implementation that conforms to your setup. +In case you want to transfer this functionality to work with your specific multi-site setup, override the `multi-site.service.ts` and provide an implementation that conforms to your setup (as well as configuring the environment variable for your specific use case). # Further References diff --git a/src/app/core/store/core/configuration/configuration.effects.ts b/src/app/core/store/core/configuration/configuration.effects.ts index 085fba519b..4464d38e5d 100644 --- a/src/app/core/store/core/configuration/configuration.effects.ts +++ b/src/app/core/store/core/configuration/configuration.effects.ts @@ -177,4 +177,27 @@ export class ConfigurationEffects { ) ) ); + + transferMultiSiteLocaleMap$ = createEffect(() => + iif( + () => isPlatformServer(this.platformId), + defer(() => + this.actions$.pipe( + ofType(ROOT_EFFECTS_INIT), + withLatestFrom( + this.stateProperties.getStateOrEnvOrDefault | string | false>( + 'MULTI_SITE_LOCALE_MAP', + 'multiSiteLocaleMap' + ) + ), + take(1), + map(([, multiSiteLocaleMap]) => (multiSiteLocaleMap === false ? undefined : multiSiteLocaleMap)), + map(multiSiteLocaleMap => + typeof multiSiteLocaleMap === 'string' ? JSON.parse(multiSiteLocaleMap) : multiSiteLocaleMap + ), + map(multiSiteLocaleMap => applyConfiguration({ multiSiteLocaleMap })) + ) + ) + ) + ); } diff --git a/src/app/core/store/core/configuration/configuration.reducer.ts b/src/app/core/store/core/configuration/configuration.reducer.ts index 67e4469038..8342cef8e5 100644 --- a/src/app/core/store/core/configuration/configuration.reducer.ts +++ b/src/app/core/store/core/configuration/configuration.reducer.ts @@ -27,6 +27,7 @@ export interface ConfigurationState { locales?: Locale[]; lang?: string; serverTranslations: { [lang: string]: Translations }; + multiSiteLocaleMap: Record; // not synced via state transfer _deviceType?: DeviceType; } @@ -43,6 +44,7 @@ const initialState: ConfigurationState = { locales: environment.locales, lang: undefined, serverTranslations: {}, + multiSiteLocaleMap: {}, _deviceType: environment.defaultDeviceType, }; diff --git a/src/app/core/store/core/configuration/configuration.selectors.ts b/src/app/core/store/core/configuration/configuration.selectors.ts index 23932ecf7f..83e505c098 100644 --- a/src/app/core/store/core/configuration/configuration.selectors.ts +++ b/src/app/core/store/core/configuration/configuration.selectors.ts @@ -94,3 +94,8 @@ export const getServerTranslations = (lang: string) => export const getSpecificServerTranslation = (lang: string, key: string) => createSelector(getServerTranslations(lang), translations => translations?.[key]); + +export const getMultiSiteLocaleMap = createSelector( + getConfigurationState, + (state: ConfigurationState) => state.multiSiteLocaleMap +); diff --git a/src/app/core/utils/multi-site/multi-site.service.ts b/src/app/core/utils/multi-site/multi-site.service.ts index 85a5f55752..f8017bdfe4 100644 --- a/src/app/core/utils/multi-site/multi-site.service.ts +++ b/src/app/core/utils/multi-site/multi-site.service.ts @@ -2,13 +2,14 @@ import { Injectable } from '@angular/core'; import { Observable } from 'rxjs'; import { map, take } from 'rxjs/operators'; -import { StatePropertiesService } from 'ish-core/utils/state-transfer/state-properties.service'; +import { select, Store } from '@ngrx/store'; +import { getMultiSiteLocaleMap } from 'ish-core/store/core/configuration'; export type MultiSiteLocaleMap = Record | undefined; @Injectable({ providedIn: 'root' }) export class MultiSiteService { - constructor(private stateProperties: StatePropertiesService) {} + constructor(private store: Store) {} /** * returns the current url, modified to fit the locale parameter if the environment parameter "multiSiteLocaleMap" is set @@ -18,28 +19,27 @@ export class MultiSiteService { * @returns the modified url */ getLangUpdatedUrl(locale: string, url: string, baseHref: string): Observable { - return this.stateProperties - .getStateOrEnvOrDefault>('MULTI_SITE_LOCALE_MAP', 'multiSiteLocaleMap') - .pipe( - take(1), - map(multiSiteLocaleMap => { - let newUrl = url; - /** - * only replace lang parameter if: - * - multiSiteLocaleMap environment var is declared (set to undefined to skip this behaviour) - * - current baseHref is part of the map (so the empty "/" baseHref would not be replaced) - * - multiSiteLocaleMap contains target locale - */ - if ( - multiSiteLocaleMap && - Object.values(multiSiteLocaleMap).includes(baseHref) && - localeMapHasLangString(locale, multiSiteLocaleMap) - ) { - newUrl = newUrl.replace(baseHref, multiSiteLocaleMap[locale]); - } - return newUrl; - }) - ); + return this.store.pipe( + select(getMultiSiteLocaleMap), + take(1), + map(multiSiteLocaleMap => { + let newUrl = url; + /** + * only replace lang parameter if: + * - multiSiteLocaleMap environment var is declared (set to undefined to skip this behaviour) + * - current baseHref is part of the map (so the empty "/" baseHref would not be replaced) + * - multiSiteLocaleMap contains target locale + */ + if ( + multiSiteLocaleMap && + Object.values(multiSiteLocaleMap).includes(baseHref) && + localeMapHasLangString(locale, multiSiteLocaleMap) + ) { + newUrl = newUrl.replace(baseHref, multiSiteLocaleMap[locale]); + } + return newUrl; + }) + ); } } From f8f75fce0f9d4e046e407fe90d1b220704291f05 Mon Sep 17 00:00:00 2001 From: "max.kless@googlemail.com" Date: Mon, 28 Jun 2021 16:36:57 +0200 Subject: [PATCH 19/23] test: fix tests --- .../multi-site/multi-site.service.spec.ts | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/app/core/utils/multi-site/multi-site.service.spec.ts b/src/app/core/utils/multi-site/multi-site.service.spec.ts index b42045761f..09e2ee48a6 100644 --- a/src/app/core/utils/multi-site/multi-site.service.spec.ts +++ b/src/app/core/utils/multi-site/multi-site.service.spec.ts @@ -1,29 +1,24 @@ import { TestBed } from '@angular/core/testing'; -import { of } from 'rxjs'; -import { anything, instance, mock, when } from 'ts-mockito'; +import { provideMockStore } from '@ngrx/store/testing'; -import { StatePropertiesService } from 'ish-core/utils/state-transfer/state-properties.service'; +import { getMultiSiteLocaleMap } from 'ish-core/store/core/configuration'; import { MultiSiteService } from './multi-site.service'; +const multiSiteLocaleMap = { + en_US: '/en', + de_DE: '/de', + fr_FR: '/fr', +}; + describe('Multi Site Service', () => { let multiSiteService: MultiSiteService; - let statePropertiesService: StatePropertiesService; beforeEach(() => { - statePropertiesService = mock(StatePropertiesService); TestBed.configureTestingModule({ - providers: [{ provide: StatePropertiesService, useFactory: () => instance(statePropertiesService) }], + providers: [provideMockStore({ selectors: [{ selector: getMultiSiteLocaleMap, value: multiSiteLocaleMap }] })], }); multiSiteService = TestBed.inject(MultiSiteService); - - when(statePropertiesService.getStateOrEnvOrDefault(anything(), anything())).thenReturn( - of({ - en_US: '/en', - de_DE: '/de', - fr_FR: '/fr', - }) - ); }); it('should be created', () => { From 1406993cce2893959b76058a34e2f698d6159520 Mon Sep 17 00:00:00 2001 From: "max.kless@googlemail.com" Date: Mon, 28 Jun 2021 16:42:06 +0200 Subject: [PATCH 20/23] docs: adapt ssr docs --- docs/guides/ssr-startup.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guides/ssr-startup.md b/docs/guides/ssr-startup.md index 03ebd65b7b..5bc96a4a30 100644 --- a/docs/guides/ssr-startup.md +++ b/docs/guides/ssr-startup.md @@ -43,7 +43,7 @@ Make sure to use them as written in the table below. | | ICM_APPLICATION | string | Overrides the default application | | | FEATURES | comma-separated list | Overrides active features | | | THEME | string | Overrides the default theme | -| | MULTI_SITE_LOCALE_MAP | JSON | Used to map locales to [url modification parameters](../guides/multi-site-configurations.md) | +| | MULTI_SITE_LOCALE_MAP | JSON \| false | Used to map locales to [url modification parameters](../guides/multi-site-configurations.md) | | **Debug** :warning: | TRUST_ICM | any | Use this if ICM is deployed with an insecure certificate | | | LOGGING | switch | Enables extra log output | | **Hybrid Approach** | SSR_HYBRID | any | Enables running PWA and ICM in [Hybrid Mode][concept-hybrid] | From 27f9877ab63443a385d7b217ee52e72e22ed95f7 Mon Sep 17 00:00:00 2001 From: "max.kless@googlemail.com" Date: Mon, 28 Jun 2021 16:52:30 +0200 Subject: [PATCH 21/23] docs: spell check --- docs/guides/multi-site-configurations.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/guides/multi-site-configurations.md b/docs/guides/multi-site-configurations.md index 54c55284f2..0fa0318b44 100644 --- a/docs/guides/multi-site-configurations.md +++ b/docs/guides/multi-site-configurations.md @@ -46,7 +46,7 @@ All other properties are optional: - **features**: Comma-separated list of activated features - **lang**: The default language as defined in the Angular CLI environment - **theme**: The theme used for the channel (format: `(|)?`) -- **protected**: Selectively unprotect a given domain and/or baseHref. Only applies in combination with globally activated nginx basic authentication. +- **protected**: Selectively disable protection of a given domain and/or baseHref. Only applies in combination with globally activated nginx basic authentication. Dynamically directing the PWA to different ICM installations can be done by using: @@ -222,7 +222,9 @@ To construct new multi-site URLs when switching between languages, the PWA uses The `getLangUpdatedUrl` is called with the desired locale string, current url and current baseHref. From this it constructs a new URL, conforming to our multi-site setup (see [One domain, one channel, multiple locales](#one-domain-one-channel-multiple-locales)). -To control the transformation of urls, the `multiSiteLocaleMap` environment variable is used. Depending on your needs, `multiSiteLocaleMap` can be set in either the `environment.ts` or as an environment variable (`MULTI_SITE_LOCALE_MAP`). See [`docker-compose.yml`](../../docker-compose.yml) for a commented out example or [`environment.model.ts`](../../src/environments/environment.model.ts) for the default value. +To control the transformation of urls, the `multiSiteLocaleMap` environment variable is used. +Depending on your needs, `multiSiteLocaleMap` can be set in either the `environment.ts` or as an environment variable (`MULTI_SITE_LOCALE_MAP`). +See [`docker-compose.yml`](../../docker-compose.yml) for a commented out example or [`environment.model.ts`](../../src/environments/environment.model.ts) for the default value. In case you want to disable this functionality, simply override the default environment variable `multiSiteLocaleMap` with `undefined` or `MULTI_SITE_LOCALE_MAP` with `false`. From 41f25f9b7e68d0abb016ce3cf0d142ca1e0702d8 Mon Sep 17 00:00:00 2001 From: "max.kless@googlemail.com" Date: Mon, 28 Jun 2021 17:08:13 +0200 Subject: [PATCH 22/23] fix: lint fix --- src/app/core/utils/multi-site/multi-site.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/core/utils/multi-site/multi-site.service.ts b/src/app/core/utils/multi-site/multi-site.service.ts index f8017bdfe4..0b3603d178 100644 --- a/src/app/core/utils/multi-site/multi-site.service.ts +++ b/src/app/core/utils/multi-site/multi-site.service.ts @@ -1,8 +1,8 @@ import { Injectable } from '@angular/core'; +import { Store, select } from '@ngrx/store'; import { Observable } from 'rxjs'; import { map, take } from 'rxjs/operators'; -import { select, Store } from '@ngrx/store'; import { getMultiSiteLocaleMap } from 'ish-core/store/core/configuration'; export type MultiSiteLocaleMap = Record | undefined; From f061dacf3c472d35b0e2e40c65ab6b35c198bd00 Mon Sep 17 00:00:00 2001 From: "max.kless@googlemail.com" Date: Thu, 1 Jul 2021 10:01:41 +0200 Subject: [PATCH 23/23] refactor: merge config effects --- .../configuration.effects.spec.ts | 2 +- .../configuration/configuration.effects.ts | 40 +++++++------------ 2 files changed, 16 insertions(+), 26 deletions(-) diff --git a/src/app/core/store/core/configuration/configuration.effects.spec.ts b/src/app/core/store/core/configuration/configuration.effects.spec.ts index 03864c88f7..2e8bc5387c 100644 --- a/src/app/core/store/core/configuration/configuration.effects.spec.ts +++ b/src/app/core/store/core/configuration/configuration.effects.spec.ts @@ -45,7 +45,7 @@ describe('Configuration Effects', () => { testComplete$.pipe(take(2)).subscribe({ complete: done }); - effects.setInitialRestEndpoint$.subscribe( + effects.transferEnvironmentProperties$.subscribe( data => { expect(data.type).toEqual(applyConfiguration.type); testComplete$.next(); diff --git a/src/app/core/store/core/configuration/configuration.effects.ts b/src/app/core/store/core/configuration/configuration.effects.ts index 4464d38e5d..e0d9d190f1 100644 --- a/src/app/core/store/core/configuration/configuration.effects.ts +++ b/src/app/core/store/core/configuration/configuration.effects.ts @@ -79,7 +79,7 @@ export class ConfigurationEffects { }); } - setInitialRestEndpoint$ = createEffect(() => + transferEnvironmentProperties$ = createEffect(() => iif( () => !this.transferState.hasKey(NGRX_STATE_SK), this.actions$.pipe( @@ -100,7 +100,18 @@ export class ConfigurationEffects { .pipe(map(x => x || 'ICM')), this.stateProperties .getStateOrEnvOrDefault('IDENTITY_PROVIDERS', 'identityProviders') - .pipe(map(config => (typeof config === 'string' ? JSON.parse(config) : config))) + .pipe(map(config => (typeof config === 'string' ? JSON.parse(config) : config))), + this.stateProperties + .getStateOrEnvOrDefault | string | false>( + 'MULTI_SITE_LOCALE_MAP', + 'multiSiteLocaleMap' + ) + .pipe( + map(multiSiteLocaleMap => (multiSiteLocaleMap === false ? undefined : multiSiteLocaleMap)), + map(multiSiteLocaleMap => + typeof multiSiteLocaleMap === 'string' ? JSON.parse(multiSiteLocaleMap) : multiSiteLocaleMap + ) + ) ), map( ([ @@ -114,6 +125,7 @@ export class ConfigurationEffects { theme, identityProvider, identityProviders, + multiSiteLocaleMap, ]) => applyConfiguration({ baseURL, @@ -125,6 +137,7 @@ export class ConfigurationEffects { theme, identityProvider, identityProviders, + multiSiteLocaleMap, }) ) ) @@ -177,27 +190,4 @@ export class ConfigurationEffects { ) ) ); - - transferMultiSiteLocaleMap$ = createEffect(() => - iif( - () => isPlatformServer(this.platformId), - defer(() => - this.actions$.pipe( - ofType(ROOT_EFFECTS_INIT), - withLatestFrom( - this.stateProperties.getStateOrEnvOrDefault | string | false>( - 'MULTI_SITE_LOCALE_MAP', - 'multiSiteLocaleMap' - ) - ), - take(1), - map(([, multiSiteLocaleMap]) => (multiSiteLocaleMap === false ? undefined : multiSiteLocaleMap)), - map(multiSiteLocaleMap => - typeof multiSiteLocaleMap === 'string' ? JSON.parse(multiSiteLocaleMap) : multiSiteLocaleMap - ), - map(multiSiteLocaleMap => applyConfiguration({ multiSiteLocaleMap })) - ) - ) - ) - ); }