feat: switch to ICM 12 resource ID encoding and provide a feature tog…

…gle for 'legacyEncoding' (#1704) * moved `encodeResourceID` function to `encodeResourceId` method of the `api.service` * introduced `legacyEncoding`feature toggle for ICM 11 and ICM 7.10 compatibility * enabled ICM 12 resource ID encoding by default
intershop · Aug 16, 2024 · f7176c9 · f7176c9
1 parent fff6649
commit f7176c9
Show file tree

Hide file tree

Showing 54 changed files with 437 additions and 318 deletions.
diff --git a/docs/concepts/configuration.md b/docs/concepts/configuration.md
@@ -177,6 +177,8 @@ Of course, the ICM server must supply appropriate REST resources to leverage fun
 | sentry                       | Sentry error tracking and monitoring (additional configuration via `sentryDSN`)                                                                 |
 | tacton                       | Tacton product configuration integration (additional configuration via `tacton` and `dataRetention` configuration options)                      |
 | tracking                     | Google Tag Manager tracking (additional configuration via `gtmToken`)                                                                           |
+| **ICM compatibility**        |                                                                                                                                                 |
+| legacyEncoding               | Use URL resource encoding for ICM 7.10 and ICM 11                                                                                               |
 
 ### Configuring Features
 

diff --git a/docs/guides/migrations.md b/docs/guides/migrations.md
@@ -9,6 +9,18 @@ kb_sync_latest_only
 
 ## From 5.1 to 5.2
 
+> [!NOTE]
+> The Intershop PWA 5.2.0 is implemented and configured to work out-of-the-box with ICM 12.0.0.
+> This distinction is needed since the way encoded resource IDs are processed by ICM was changed in ICM 12.
+>
+> To use the fitting resource ID encoding for ICM 7.10 or ICM 11, the feature toggle `legacyEncoding` needs to be enabled.
+
+The function `encodeResourceID` has been moved to a method `encodeResourceId` of the [`api.service`](../../src/app/core/services/api/api.service.ts) and it is now used to encode all dynamic resource IDs in any REST API call to ICM.
+This was previously only done for the login but is now consistently used for all resource IDs.
+For ICM 7.10 and ICM 11 a duplicated `encodeURIComponent` encoding is applied, for ICM 12 and newer a single `encodeURIComponent` encoding with additional `+` character handling is used.
+To migrate custom code a simple search for `encodeResourceID(` and replace with `this.apiService.encodeResourceId(` should be sufficient.
+Please be aware when migrating that there is an intermediate [commit](https://github.com/intershop/intershop-pwa/commit/3e7c0ae2ff1d6e676f98d7c399b70b505f389e16) for the resource ID encoding in the 5.2 release that was improved with a later [commit](https://github.com/intershop/intershop-pwa/commit/TODO_with_release_creation) to work with the `legacyEncoding` feature toggle for ICM 7.10 and ICM 11 encoding not requiring any code adaptions anymore.
+
 The store action and method `addBasketToNewOrderTemplate` of the OrderTemplatesFacade have been renamed to `createOrderTemplateFromLineItems` and refactored slightly.
 
 The Intershop PWA specific Pipes `sanitize`, `makeHref` and `htmlEncode` were renamed using the common `ish` prefix that is used for the other custom Pipes as well.
@@ -21,12 +33,6 @@ A few Stylelint rules were changed and the `.scss` files were adapted.
 In addition empty comments in `.scss` files are no longer allowed and removed.
 In migration projects either keep the Stylelint rules with the old settings or migrate all your styling files accordingly running `npm run format`.
 
-The method [`encodeResourceID`](../../src/app/core/utils/url-resource-ids.ts) is now used to encode all dynamic resource IDs in any REST API call.
-This was previously only done for the login but is now consistently used for all resource IDs.
-For ICM 7.10 and ICM 11 this is done by default with a duplicated `encodeURIComponent` encoding.
-For ICM 12 and newer this needs to be changed to a single `encodeURIComponent` with additional `+` character handling.
-The ICM 12 handling is already prepared in [`encodeResourceID`](../../src/app/core/utils/url-resource-ids.ts) and needs to be activated in the source code if the Intershop PWA is used together with ICM 12.
-
 With the Intershop PWA version 5.2.0 the rendering of our demo/example view contexts was disabled by default.
 Each integrated view context triggers a REST call that will potentially decrease the SSR rendering performance, something that is not necessary if this feature is not actively used in a PWA project.
 For that reason the examples were commented out in the source code and have to be activated in the project source code if needed.

diff --git a/projects/organization-management/src/app/services/cost-centers/cost-centers.service.spec.ts b/projects/organization-management/src/app/services/cost-centers/cost-centers.service.spec.ts
@@ -7,15 +7,14 @@ import { CostCenterBase, CostCenterBuyer } from 'ish-core/models/cost-center/cos
 import { Customer } from 'ish-core/models/customer/customer.model';
 import { ApiService } from 'ish-core/services/api/api.service';
 import { getLoggedInCustomer } from 'ish-core/store/customer/user';
-import { encodeResourceID } from 'ish-core/utils/url-resource-ids';
 
 import { CostCentersService } from './cost-centers.service';
 
 describe('Cost Centers Service', () => {
   let apiService: ApiService;
   let costCentersService: CostCentersService;
-  const millersEMail = 'pmiller@test.intershop.de';
-  const millersCostcenterEndpoint = `customers/4711/costcenters/123/buyers/${encodeResourceID(millersEMail)}`;
+  const millersEMail = 'pmiller%40test.intershop.de';
+  const millersCostcenterEndpoint = `customers/4711/costcenters/123/buyers/${millersEMail}`;
 
   beforeEach(() => {
     apiService = mock(ApiService);
@@ -24,6 +23,7 @@ describe('Cost Centers Service', () => {
     when(apiService.patch(anything(), anything())).thenReturn(of({}));
     when(apiService.delete(anything())).thenReturn(of({}));
     when(apiService.resolveLinks()).thenReturn(() => of([]));
+    when(apiService.encodeResourceId(anything())).thenCall(id => id);
 
     TestBed.configureTestingModule({
       providers: [

diff --git a/projects/organization-management/src/app/services/cost-centers/cost-centers.service.ts b/projects/organization-management/src/app/services/cost-centers/cost-centers.service.ts
@@ -9,7 +9,6 @@ import { CostCenter, CostCenterBase, CostCenterBuyer } from 'ish-core/models/cos
 import { ApiService } from 'ish-core/services/api/api.service';
 import { getLoggedInCustomer } from 'ish-core/store/customer/user';
 import { whenTruthy } from 'ish-core/utils/operators';
-import { encodeResourceID } from 'ish-core/utils/url-resource-ids';
 
 @Injectable({ providedIn: 'root' })
 export class CostCentersService {
@@ -25,7 +24,7 @@ export class CostCentersService {
   getCostCenters(): Observable<CostCenter[]> {
     return this.currentCustomer$.pipe(
       switchMap(customer =>
-        this.apiService.get(`customers/${encodeResourceID(customer.customerNo)}/costcenters`).pipe(
+        this.apiService.get(`customers/${this.apiService.encodeResourceId(customer.customerNo)}/costcenters`).pipe(
           this.apiService.resolveLinks<CostCenterData>(),
           map(ccData => ccData.map(CostCenterMapper.fromData))
         )
@@ -48,7 +47,9 @@ export class CostCentersService {
       switchMap(customer =>
         this.apiService
           .get<CostCenterData>(
-            `customers/${encodeResourceID(customer.customerNo)}/costcenters/${encodeResourceID(costCenterId)}`
+            `customers/${this.apiService.encodeResourceId(
+              customer.customerNo
+            )}/costcenters/${this.apiService.encodeResourceId(costCenterId)}`
           )
           .pipe(map(CostCenterMapper.fromData))
       )
@@ -69,7 +70,10 @@ export class CostCentersService {
     return this.currentCustomer$.pipe(
       switchMap(customer =>
         this.apiService
-          .post<CostCenterData>(`customers/${encodeResourceID(customer.customerNo)}/costcenters`, costCenter)
+          .post<CostCenterData>(
+            `customers/${this.apiService.encodeResourceId(customer.customerNo)}/costcenters`,
+            costCenter
+          )
           .pipe(concatMap(() => this.getCostCenter(costCenter.costCenterId)))
       )
     );
@@ -90,7 +94,9 @@ export class CostCentersService {
       switchMap(customer =>
         this.apiService
           .patch<CostCenterData>(
-            `customers/${encodeResourceID(customer.customerNo)}/costcenters/${encodeResourceID(costCenter.id)}`,
+            `customers/${this.apiService.encodeResourceId(
+              customer.customerNo
+            )}/costcenters/${this.apiService.encodeResourceId(costCenter.id)}`,
             costCenter
           )
           .pipe(map(CostCenterMapper.fromData))
@@ -110,7 +116,11 @@ export class CostCentersService {
 
     return this.currentCustomer$.pipe(
       switchMap(customer =>
-        this.apiService.delete(`customers/${encodeResourceID(customer.customerNo)}/costcenters/${encodeResourceID(id)}`)
+        this.apiService.delete(
+          `customers/${this.apiService.encodeResourceId(
+            customer.customerNo
+          )}/costcenters/${this.apiService.encodeResourceId(id)}`
+        )
       )
     );
   }
@@ -135,7 +145,9 @@ export class CostCentersService {
         forkJoin(
           buyers.map(buyer =>
             this.apiService.post(
-              `customers/${encodeResourceID(customer.customerNo)}/costcenters/${encodeResourceID(costCenterId)}/buyers`,
+              `customers/${this.apiService.encodeResourceId(
+                customer.customerNo
+              )}/costcenters/${this.apiService.encodeResourceId(costCenterId)}/buyers`,
               buyer
             )
           )
@@ -163,9 +175,11 @@ export class CostCentersService {
       switchMap(customer =>
         this.apiService
           .patch(
-            `customers/${encodeResourceID(customer.customerNo)}/costcenters/${encodeResourceID(
-              costCenterId
-            )}/buyers/${encodeResourceID(buyer.login)}`,
+            `customers/${this.apiService.encodeResourceId(
+              customer.customerNo
+            )}/costcenters/${this.apiService.encodeResourceId(costCenterId)}/buyers/${this.apiService.encodeResourceId(
+              buyer.login
+            )}`,
             buyer
           )
           .pipe(concatMap(() => this.getCostCenter(costCenterId)))
@@ -192,9 +206,11 @@ export class CostCentersService {
       switchMap(customer =>
         this.apiService
           .delete(
-            `customers/${encodeResourceID(customer.customerNo)}/costcenters/${encodeResourceID(
-              costCenterId
-            )}/buyers/${encodeResourceID(login)}`
+            `customers/${this.apiService.encodeResourceId(
+              customer.customerNo
+            )}/costcenters/${this.apiService.encodeResourceId(costCenterId)}/buyers/${this.apiService.encodeResourceId(
+              login
+            )}`
           )
           .pipe(concatMap(() => this.getCostCenter(costCenterId)))
       )

diff --git a/...services/organization-management-breadcrumb/organization-management-breadcrumb.service.ts b/...services/organization-management-breadcrumb/organization-management-breadcrumb.service.ts
@@ -6,7 +6,6 @@ import { map, switchMap, withLatestFrom } from 'rxjs/operators';
 import { AppFacade } from 'ish-core/facades/app.facade';
 import { BreadcrumbItem } from 'ish-core/models/breadcrumb-item/breadcrumb-item.interface';
 import { whenFalsy, whenTruthy } from 'ish-core/utils/operators';
-import { encodeResourceID } from 'ish-core/utils/url-resource-ids';
 
 import { OrganizationManagementFacade } from '../../facades/organization-management.facade';
 
@@ -43,7 +42,7 @@ export class OrganizationManagementBreadcrumbService {
                     { key: 'account.organization.user_management', link: `${prefix}/users` },
                     {
                       text: `${translation} - ${user.firstName} ${user.lastName}`,
-                      link: `${prefix}/users/${encodeResourceID(user.login)}`,
+                      link: `${prefix}/users/${user.login}`,
                     },
                     {
                       key: `account.user.update_${path.substring(path.lastIndexOf('/') + 1)}.heading`,

diff --git a/projects/organization-management/src/app/services/users/users.service.spec.ts b/projects/organization-management/src/app/services/users/users.service.spec.ts
@@ -7,7 +7,6 @@ import { AppFacade } from 'ish-core/facades/app.facade';
 import { Customer } from 'ish-core/models/customer/customer.model';
 import { ApiService } from 'ish-core/services/api/api.service';
 import { getLoggedInCustomer } from 'ish-core/store/customer/user';
-import { encodeResourceID } from 'ish-core/utils/url-resource-ids';
 
 import { B2bRoleData } from '../../models/b2b-role/b2b-role.interface';
 import { B2bUser } from '../../models/b2b-user/b2b-user.model';
@@ -19,8 +18,8 @@ describe('Users Service', () => {
   let apiService: ApiService;
   let usersService: UsersService;
   let appFacade: AppFacade;
-  const millersEMail = 'pmiller@test.intershop.de';
-  const millersEndpoint = `customers/4711/users/${encodeResourceID(millersEMail)}`;
+  const millersEMail = 'pmiller%40test.intershop.de';
+  const millersEndpoint = `customers/4711/users/${millersEMail}`;
 
   beforeEach(() => {
     apiService = mock(ApiService);
@@ -29,6 +28,7 @@ describe('Users Service', () => {
     when(apiService.delete(anything())).thenReturn(of({}));
     when(apiService.post(anyString(), anything())).thenReturn(of({}));
     when(apiService.put(anyString(), anything())).thenReturn(of({}));
+    when(apiService.encodeResourceId(anything())).thenCall(id => id);
     when(appFacade.currentLocale$).thenReturn(of('en_US'));
 
     TestBed.configureTestingModule({

diff --git a/projects/organization-management/src/app/services/users/users.service.ts b/projects/organization-management/src/app/services/users/users.service.ts
@@ -8,7 +8,6 @@ import { PriceHelper } from 'ish-core/models/price/price.helper';
 import { ApiService, unpackEnvelope } from 'ish-core/services/api/api.service';
 import { getLoggedInCustomer } from 'ish-core/store/customer/user';
 import { whenTruthy } from 'ish-core/utils/operators';
-import { encodeResourceID } from 'ish-core/utils/url-resource-ids';
 
 import { B2bRoleData } from '../../models/b2b-role/b2b-role.interface';
 import { B2bRoleMapper } from '../../models/b2b-role/b2b-role.mapper';
@@ -37,7 +36,7 @@ export class UsersService {
     return this.currentCustomer$.pipe(
       switchMap(customer =>
         this.apiService
-          .get(`customers/${encodeResourceID(customer.customerNo)}/users`)
+          .get(`customers/${this.apiService.encodeResourceId(customer.customerNo)}/users`)
           .pipe(unpackEnvelope(), map(B2bUserMapper.fromListData))
       )
     );
@@ -53,7 +52,11 @@ export class UsersService {
     return this.currentCustomer$.pipe(
       switchMap(customer =>
         this.apiService
-          .get(`customers/${encodeResourceID(customer.customerNo)}/users/${encodeResourceID(login)}`)
+          .get(
+            `customers/${this.apiService.encodeResourceId(
+              customer.customerNo
+            )}/users/${this.apiService.encodeResourceId(login)}`
+          )
           .pipe(map(B2bUserMapper.fromData))
       )
     );
@@ -74,7 +77,7 @@ export class UsersService {
       withLatestFrom(this.appFacade.currentLocale$),
       switchMap(([customer, currentLocale]) =>
         this.apiService
-          .post<B2bUser>(`customers/${encodeResourceID(customer.customerNo)}/users`, {
+          .post<B2bUser>(`customers/${this.apiService.encodeResourceId(customer.customerNo)}/users`, {
             elements: [
               {
                 ...customer,
@@ -120,16 +123,21 @@ export class UsersService {
     return this.currentCustomer$.pipe(
       switchMap(customer =>
         this.apiService
-          .put(`customers/${encodeResourceID(customer.customerNo)}/users/${encodeResourceID(user.login)}`, {
-            ...customer,
-            ...user,
-            preferredInvoiceToAddress: { urn: user.preferredInvoiceToAddressUrn },
-            preferredShipToAddress: { urn: user.preferredShipToAddressUrn },
-            preferredPaymentInstrument: { id: user.preferredPaymentInstrumentId },
-            preferredInvoiceToAddressUrn: undefined,
-            preferredShipToAddressUrn: undefined,
-            preferredPaymentInstrumentId: undefined,
-          })
+          .put(
+            `customers/${this.apiService.encodeResourceId(
+              customer.customerNo
+            )}/users/${this.apiService.encodeResourceId(user.login)}`,
+            {
+              ...customer,
+              ...user,
+              preferredInvoiceToAddress: { urn: user.preferredInvoiceToAddressUrn },
+              preferredShipToAddress: { urn: user.preferredShipToAddressUrn },
+              preferredPaymentInstrument: { id: user.preferredPaymentInstrumentId },
+              preferredInvoiceToAddressUrn: undefined,
+              preferredShipToAddressUrn: undefined,
+              preferredPaymentInstrumentId: undefined,
+            }
+          )
           .pipe(map(B2bUserMapper.fromData))
       )
     );
@@ -148,15 +156,19 @@ export class UsersService {
 
     return this.currentCustomer$.pipe(
       switchMap(customer =>
-        this.apiService.delete(`customers/${encodeResourceID(customer.customerNo)}/users/${encodeResourceID(login)}`)
+        this.apiService.delete(
+          `customers/${this.apiService.encodeResourceId(customer.customerNo)}/users/${this.apiService.encodeResourceId(
+            login
+          )}`
+        )
       )
     );
   }
 
   getAvailableRoles(): Observable<B2bRole[]> {
     return this.currentCustomer$.pipe(
       switchMap(customer =>
-        this.apiService.get(`customers/${encodeResourceID(customer.customerNo)}/roles`).pipe(
+        this.apiService.get(`customers/${this.apiService.encodeResourceId(customer.customerNo)}/roles`).pipe(
           unpackEnvelope<B2bRoleData>('userRoles'),
           map(data => this.b2bRoleMapper.fromData(data))
         )
@@ -173,9 +185,14 @@ export class UsersService {
     return this.currentCustomer$.pipe(
       switchMap(customer =>
         this.apiService
-          .put(`customers/${encodeResourceID(customer.customerNo)}/users/${encodeResourceID(login)}/roles`, {
-            userRoles,
-          })
+          .put(
+            `customers/${this.apiService.encodeResourceId(
+              customer.customerNo
+            )}/users/${this.apiService.encodeResourceId(login)}/roles`,
+            {
+              userRoles,
+            }
+          )
           .pipe(
             unpackEnvelope<B2bRoleData>('userRoles'),
             map(data => data.map(r => r.roleID))
@@ -199,7 +216,9 @@ export class UsersService {
     return this.currentCustomer$.pipe(
       switchMap(customer =>
         this.apiService.put<UserBudget>(
-          `customers/${encodeResourceID(customer.customerNo)}/users/${encodeResourceID(login)}/budgets`,
+          `customers/${this.apiService.encodeResourceId(customer.customerNo)}/users/${this.apiService.encodeResourceId(
+            login
+          )}/budgets`,
           budget
         )
       )

diff --git a/projects/requisition-management/src/app/services/requisitions/requisitions.service.spec.ts b/projects/requisition-management/src/app/services/requisitions/requisitions.service.spec.ts
@@ -19,6 +19,8 @@ describe('Requisitions Service', () => {
     apiServiceMock = mock(ApiService);
     userServiceMock = mock(UserService);
     when(apiServiceMock.b2bUserEndpoint()).thenReturn(instance(apiServiceMock));
+    when(apiServiceMock.encodeResourceId(anything())).thenCall(id => id);
+
     TestBed.configureTestingModule({
       providers: [
         { provide: ApiService, useFactory: () => instance(apiServiceMock) },

diff --git a/projects/requisition-management/src/app/services/requisitions/requisitions.service.ts b/projects/requisition-management/src/app/services/requisitions/requisitions.service.ts
@@ -87,7 +87,7 @@ export class RequisitionsService {
 
     return this.apiService
       .b2bUserEndpoint()
-      .get<RequisitionData>(`requisitions/${requisitionId}`, {
+      .get<RequisitionData>(`requisitions/${this.apiService.encodeResourceId(requisitionId)}`, {
         params,
       })
       .pipe(
@@ -129,7 +129,7 @@ export class RequisitionsService {
 
     return this.apiService
       .b2bUserEndpoint()
-      .patch<RequisitionData>(`requisitions/${requisitionId}`, body, {
+      .patch<RequisitionData>(`requisitions/${this.apiService.encodeResourceId(requisitionId)}`, body, {
         params,
       })
       .pipe(concatMap(payload => this.processRequisitionData(payload)));
@@ -149,7 +149,7 @@ export class RequisitionsService {
 
       if (requisitionData.order?.itemId) {
         return this.apiService
-          .get<OrderData>(`orders/${requisitionData.order.itemId}`, {
+          .get<OrderData>(`orders/${this.apiService.encodeResourceId(requisitionData.order.itemId)}`, {
             headers: this.orderHeaders,
             params,
           })

diff --git a/src/app/core/services/address/address.service.spec.ts b/src/app/core/services/address/address.service.spec.ts
@@ -24,6 +24,7 @@ describe('Address Service', () => {
       ],
     });
     when(appFacade.customerRestResource$).thenReturn(of('customers'));
+    when(apiService.encodeResourceId(anything())).thenCall(id => id);
 
     addressService = TestBed.inject(AddressService);
   });