refactor: cleanup/remove obsolete/update TODO comments

* removed obsolete styling TODOs
* applied consistent TODO style
* removed leftover footer collapsed code
* removed obsolete/wrong selectOrderAfterRedirectFailed$ effect

BREAKING CHANGES: The TODO code cleanup removed some implementation fixes to work with ICM versions prior to 7.10.39.2.

Co-authored-by: Stefan Hauke <s.hauke@intershop.de>

docs/concepts/project-structure.md

@@ -45,7 +45,6 @@ The `src/app` folder contains all TypeScript code (sources and tests) and HTML t
 
 - `core` contains all configuration and utility code for the main B2C application.
   - `core/models` contains models for all data entities for the B2C store.
-    <!-- TODO: see Data Handling with Mappers. -->
   - `core/utils` contains all utility functions that are used in multiple cases.
   - `core/store` contains the main [State Management](./state-management.md), `core/facades` contains facades for accessing the state in the application.
 - `shell` contains the synchronously loaded application shell (header and footer).

docs/guides/migrations.md

@@ -104,6 +104,7 @@ Obsolete functionality that is no longer needed with the current state of the In
 - removed outdated `kubernetes-deployment` schematic that could be used to create Kubernetes charts, use the official [Intershop PWA Helm Chart repository](https://github.com/intershop/helm-charts/tree/main/charts/pwa) instead
 - removed unused `azure-pipeline` schematic that could be used to create an Azure Pipeline template based on the generated Kubernetes charts for DevOps
 - removed migration scripts that where used for pre PWA 1.0 migration support
+- removed obsolete TODO comments and related logic that handled for example wrong/odd/old REST API quirks
 
 We recommend to use the Action Group Creator to create store actions now.
 Therefore the corresponding store schematic for the action creation has been adapted.

e2e/cypress/e2e/specs/checkout/checkout-payments.b2c.e2e-spec.ts

@@ -64,11 +64,7 @@ describe('Checkout Payment', () => {
         page.paymentInstrument('ISH_DEBIT_TRANSFER').submit();
         page.paymentInstrument('ISH_DEBIT_TRANSFER').formError('holder').log('ish debit transfer');
         page.paymentInstrument('ISH_DEBIT_TRANSFER').formError('holder').should('contain', 'missing');
-        /* TODO: size validator does not display message correctly
-      page
-        .paymentInstrument('ISH_DEBIT_TRANSFER')
-        .formError('IBAN')
-        .should('contain', 'must have a length'); */
+        page.paymentInstrument('ISH_DEBIT_TRANSFER').formError('IBAN').should('contain', 'must have a length');
       });
     });
 

projects/organization-management/src/app/components/cost-center-buyer-edit-dialog/cost-center-buyer-edit-dialog.component.spec.ts

@@ -3,7 +3,6 @@ import { UntypedFormBuilder, UntypedFormGroup } from '@angular/forms';
 import { instance, mock } from 'ts-mockito';
 
 import { AppFacade } from 'ish-core/facades/app.facade';
-import { FormlyTestingModule } from 'ish-shared/formly/dev/testing/formly-testing.module';
 import { SpecialValidators } from 'ish-shared/forms/validators/special-validators';
 
 import { OrganizationManagementFacade } from '../../facades/organization-management.facade';
@@ -24,7 +23,6 @@ describe('Cost Center Buyer Edit Dialog Component', () => {
     organizationManagementFacade = mock(organizationManagementFacade);
 
     await TestBed.configureTestingModule({
-      imports: [FormlyTestingModule],
       providers: [
         { provide: AppFacade, useFactory: () => instance(appFacade) },
         { provide: OrganizationManagementFacade, useFactory: () => instance(organizationManagementFacade) },

projects/organization-management/src/app/pages/user-create/user-create-page.component.ts

@@ -57,7 +57,7 @@ export class UserCreatePageComponent implements OnInit {
       email: formValue.profile.email,
       active: formValue.profile.active,
       phoneHome: formValue.profile.phoneHome,
-      birthday: formValue.profile.birthday === '' ? undefined : formValue.birthday, // TODO: see IS-22276
+      birthday: formValue.profile.birthday === '' ? undefined : formValue.birthday,
       businessPartnerNo: `U${uuid()}`,
       roleIDs: formValue.roleIDs,
       userBudget: formValue.userBudget.currency

projects/organization-management/src/app/services/cost-centers/cost-centers.service.spec.ts

@@ -20,6 +20,7 @@ describe('Cost Centers Service', () => {
     when(apiService.post(anything(), anything())).thenReturn(of({}));
     when(apiService.patch(anything(), anything())).thenReturn(of({}));
     when(apiService.delete(anything())).thenReturn(of({}));
+    when(apiService.resolveLinks()).thenReturn(() => of([]));
 
     TestBed.configureTestingModule({
       providers: [

projects/organization-management/src/app/services/cost-centers/cost-centers.service.ts

@@ -1,13 +1,12 @@
 import { Injectable } from '@angular/core';
 import { Store, select } from '@ngrx/store';
-import { Observable, OperatorFunction, forkJoin, iif, of, throwError } from 'rxjs';
+import { Observable, forkJoin, throwError } from 'rxjs';
 import { concatMap, map, switchMap, take } from 'rxjs/operators';
 
 import { CostCenterData } from 'ish-core/models/cost-center/cost-center.interface';
 import { CostCenterMapper } from 'ish-core/models/cost-center/cost-center.mapper';
 import { CostCenter, CostCenterBase, CostCenterBuyer } from 'ish-core/models/cost-center/cost-center.model';
-import { Link } from 'ish-core/models/link/link.model';
-import { ApiService, AvailableOptions } from 'ish-core/services/api/api.service';
+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';
@@ -27,7 +26,7 @@ export class CostCentersService {
     return this.currentCustomer$.pipe(
       switchMap(customer =>
         this.apiService.get(`customers/${customer.customerNo}/costcenters`).pipe(
-          this.resolveCostCenterLinks<CostCenterData>(),
+          this.apiService.resolveLinks<CostCenterData>(),
           map(ccData => ccData.map(CostCenterMapper.fromData))
         )
       )
@@ -185,26 +184,4 @@ export class CostCentersService {
       )
     );
   }
-
-  /**
-   * ToDo: use resolveLinks of the api.service instead
-   * Temporary fix, it can be removed  after the REST response has been corrected, see #71044
-   */
-  private resolveCostCenterLinks<T>(options?: AvailableOptions): OperatorFunction<Link[], T[]> {
-    return source$ =>
-      source$.pipe(
-        // filter for all real Link elements
-        map(links => (links?.length ? links.filter(el => el?.type === 'Link' && !!el.uri) : [])),
-        // transform Link elements to API Observables
-        map(links =>
-          links.map(item => {
-            // check link uri and prepend server ICM if necessary
-            const uri = item.uri.substring(item.uri.indexOf('/customers'));
-            return this.apiService.get<T>(uri, options);
-          })
-        ),
-        // flatten to API requests O<O<T>[]> -> O<T[]>
-        concatMap(obsArray => iif(() => !!obsArray.length, forkJoin(obsArray), of([])))
-      );
-  }
 }

projects/requisition-management/src/app/store/requisitions/requisitions.effects.ts

@@ -78,7 +78,6 @@ export class RequisitionsEffects {
           .updateRequisitionStatus(payload.requisitionId, payload.status, payload.approvalComment)
           .pipe(
             concatMap(requisition =>
-              /* ToDo: use only relative routes */
               from(this.router.navigate([`/account/requisitions/approver`])).pipe(
                 concatMap(() => {
                   let messageAction;

src/app/core/interceptors/identity-provider.interceptor.ts

@@ -10,7 +10,6 @@ export class IdentityProviderInterceptor implements HttpInterceptor {
   constructor(private identityProviderFactory: IdentityProviderFactory, private appFacade: AppFacade) {}
 
   intercept(req: HttpRequest<unknown>, next: HttpHandler): Observable<HttpEvent<unknown>> {
-    // TODO: check if this works with PROXY_ICM
     if (req.url.startsWith(this.appFacade.icmBaseUrl)) {
       return this.identityProviderFactory.getInstance()?.intercept(req, next) ?? next.handle(req);
     }

src/app/core/models/content-configuration-parameter/content-configuration-parameter.mapper.ts

@@ -31,24 +31,7 @@ export class ContentConfigurationParameterMapper {
     return configurationParameters;
   }
 
-  private resolveStaticURL(value: string): string {
-    if (value.startsWith('http')) {
-      return value;
-    }
-
-    if (!value.includes(':/')) {
-      return value;
-    }
-
-    const split = value.split(':');
-
-    return encodeURI(`${this.staticURL}/${split[0]}/${this.lang}${split[1]}`);
-  }
-
-  /**
-   * TODO: Make this method use name-based plugin mechanism to delegate post processing of
-   * configuration parameter data to specific handler.
-   */
+  // post process the configuration parameter data to apply special handling for specific types
   private postProcessData(data: ContentConfigurationParameterData): string | object | number {
     switch (data.type) {
       case 'bc_pmc:types.pagelet2-ImageFileRef':
@@ -62,4 +45,18 @@ export class ContentConfigurationParameterMapper {
         return data.value;
     }
   }
+
+  // convert ICM file references to full server URLs
+  private resolveStaticURL(value: string): string {
+    if (value.startsWith('http')) {
+      return value;
+    }
+
+    if (!value.includes(':/')) {
+      return value;
+    }
+
+    const split = value.split(':');
+    return encodeURI(`${this.staticURL}/${split[0]}/${this.lang}${split[1]}`);
+  }
 }

src/app/core/models/product-variation/product-variation.helper.ts

@@ -119,7 +119,6 @@ export class ProductVariationHelper {
    *
    * @param option  The select option to check.
    * @returns       Indicates if no perfect match is found.
-   * TODO: Refactor this to a more functional style
    */
   // eslint-disable-next-line complexity
   private static alternativeCombinationCheck(option: VariationSelectOption, product: ProductView): boolean {

src/app/core/services/api/api.service.ts

@@ -81,7 +81,7 @@ export class ApiService {
         ? // captcha V3
           headers.set(ApiService.AUTHORIZATION_HEADER_KEY, `CAPTCHA recaptcha_token=${captcha} action=${captchaAction}`)
         : // captcha V2
-          // TODO: remove second parameter 'foo=bar' that currently only resolves a shortcoming of the server side implementation that still requires two parameters
+          // second parameter 'foo=bar' is only required to resolve a shortcoming of the server side implementation that requires two parameters
           headers.set(ApiService.AUTHORIZATION_HEADER_KEY, `CAPTCHA g-recaptcha-response=${captcha} foo=bar`)
     );
   }

src/app/core/services/payment/payment.service.ts

@@ -364,7 +364,6 @@ export class PaymentService {
         parameters: paymentInstrument.parameters.map(attr => ({ key: attr.name, property: attr.value })),
       };
 
-      // TODO: Replace this PUT request with PATCH request once it is fixed in ICM
       return this.apiService
         .put(`customers/-/payments/${paymentInstrument.id}`, body)
         .pipe(map(() => paymentInstrument));

src/app/core/services/products/products.service.ts

@@ -237,7 +237,6 @@ export class ProductsService {
 
   /**
    * exchange single-return variation products to master products for B2B
-   * TODO: this is a work-around
    */
   private postProcessMasters(products: Partial<Product>[], advancedVariationHandling: boolean): Product[] {
     if (advancedVariationHandling) {

src/app/core/store/customer/basket/basket-validation.effects.ts

@@ -51,7 +51,7 @@ export class BasketValidationEffects {
     },
     [CheckoutStepType.Shipping]: { scopes: ['Shipping'], route: '/checkout/shipping' },
     [CheckoutStepType.Payment]: { scopes: ['Payment'], route: '/checkout/payment' },
-    [CheckoutStepType.Review]: { scopes: ['All', 'CostCenter'], route: '/checkout/review' }, // ToDo: has to be changed if the cost center approval has been implemented
+    [CheckoutStepType.Review]: { scopes: ['All', 'CostCenter'], route: '/checkout/review' },
     [CheckoutStepType.Receipt]: { scopes: ['All'], route: 'auto' }, // targetRoute will be calculated in dependence of the validation result
   };
 

src/app/core/store/customer/orders/orders.effects.spec.ts

@@ -427,22 +427,6 @@ describe('Orders Effects', () => {
     });
   });
 
-  describe('selectOrderAfterRedirectFailed', () => {
-    it('should navigate to /checkout/payment if order creation failed after redirect', done => {
-      actions$ = of(selectOrderAfterRedirectFail(undefined));
-
-      effects.selectOrderAfterRedirectFailed$.subscribe({
-        next: action => {
-          expect(action).toMatchInlineSnapshot(`[Basket Internal] Load Basket`);
-          expect(location.path()).toEqual('/checkout/payment?redirect=failure');
-          done();
-        },
-        error: fail,
-        complete: noop,
-      });
-    });
-  });
-
   describe('setOrderBreadcrumb$', () => {
     beforeEach(fakeAsync(() => {
       store.dispatch(loadOrdersSuccess({ orders }));

src/app/core/store/customer/orders/orders.effects.ts

@@ -225,28 +225,16 @@ export class OrdersEffects {
             if (params.redirect === 'success') {
               return selectOrder({ orderId });
             } else {
+              // cancelled payment
               return loadBasket();
             }
           }),
-          mapErrorToAction(selectOrderAfterRedirectFail) // ToDo: display error message on receipt page
+          mapErrorToAction(selectOrderAfterRedirectFail)
         )
       )
     )
   );
 
-  selectOrderAfterRedirectFailed$ = createEffect(() =>
-    this.actions$.pipe(
-      ofType(selectOrderAfterRedirectFail),
-      concatMap(() =>
-        from(
-          this.router.navigate(['/checkout/payment'], {
-            queryParams: { redirect: 'failure' },
-          })
-        ).pipe(map(() => loadBasket()))
-      )
-    )
-  );
-
   setOrderBreadcrumb$ = createEffect(() =>
     this.actions$.pipe(
       ofType(routerNavigatedAction),

src/app/core/utils/link-parser.ts

@@ -10,14 +10,13 @@ export class LinkParser {
 
       switch (protocol) {
         case 'product':
-          // TODO: use ProductRoutePipe for SEO URLs
+          // product page links
           return `${prefix}/product/${value}`;
         case 'category':
-          // TODO: use CategoryRoutePipe for SEO URLs
+          // category page links
           return `${prefix}/categoryref/${value}${unitName}`;
         case 'page':
-          // CMS managed pages link
-          // TODO: use ContentPageRoutePipe for SEO URLs
+          // content page links
           return `${prefix}/page/${value}`;
         case 'route':
           // direct router links for the PWA

src/app/extensions/order-templates/shared/order-template-preferences-dialog/order-template-preferences-dialog.component.html

@@ -14,7 +14,6 @@ <h2 class="modal-title">{{ modalHeader | translate }}</h2>
 
   <form [formGroup]="orderTemplateForm" (ngSubmit)="submitOrderTemplateForm()">
     <div class="modal-body">
-      <!-- TODO: Show server error -->
       <formly-form [form]="orderTemplateForm" [fields]="fields" [model]="model"></formly-form>
     </div>
 

src/app/extensions/quoting/shared/quote-state/quote-state.component.html

@@ -3,14 +3,6 @@
   <ng-template [ngSwitchCase]="'Submitted'">{{ 'quote.state.submitted' | translate }}</ng-template>
   <ng-template [ngSwitchCase]="'Responded'">{{ 'quote.state.responded' | translate }}</ng-template>
   <ng-template [ngSwitchCase]="'Expired'">{{ 'quote.state.expired' | translate }}</ng-template>
-  <!-- TODO: implement accepted? -->
-  <!-- <ng-template [ngSwitchCase]="7">
-    {{ 'quote.state.accepted' | translate}}
-    <ng-container *ngIf="quote.validToDate < currentDateTime">
-        &nbsp;
-        {{ 'quote.state.expired' | translate }}
-    </ng-container>
-  </ng-template> -->
   <ng-template [ngSwitchCase]="'Rejected'">{{ 'quote.state.rejected' | translate }}</ng-template>
   <ng-template ngSwitchDefault>{{ 'quote.state.unknown' | translate }}</ng-template>
 </ng-container>

src/app/extensions/rating/services/reviews/reviews.service.ts

@@ -1,6 +1,6 @@
 import { Injectable } from '@angular/core';
 import { Observable, throwError } from 'rxjs';
-import { map, tap } from 'rxjs/operators';
+import { map } from 'rxjs/operators';
 
 import { Link } from 'ish-core/models/link/link.model';
 import { ApiService, unpackEnvelope } from 'ish-core/services/api/api.service';
@@ -24,18 +24,9 @@ export class ReviewsService {
       return throwError(() => new Error('getProductReviews() called without sku'));
     }
 
-    let ownReviewId: string;
-
     return this.apiService.get(`products/${sku}/reviews?attrs=own`, { sendSPGID: true }).pipe(
       unpackEnvelope<Link>(),
-      tap(reviewLinks => {
-        ownReviewId = reviewLinks?.find(link =>
-          link.attributes.find(attr => attr.name === 'own' && attr.value === true)
-        )?.itemId;
-      }),
       this.apiService.resolveLinks<ProductReview>(),
-      // ToDo: remove this mapping if the own flag is returned by the REST api
-      map(reviews => reviews.map(review => ({ ...review, own: review.id === ownReviewId }))),
       map(reviews => ProductReviewsMapper.fromData(sku, reviews))
     );
   }

src/app/extensions/rating/shared/product-review-create-dialog/product-review-create-dialog.component.ts

@@ -120,7 +120,7 @@ export class ProductReviewCreateDialogComponent implements OnInit {
     }
   }
 
-  /** ToDo: send the review to the server */
+  /** Send the review to the server */
   submitForm(sku: string) {
     if (this.form.invalid) {
       markAsDirtyRecursive(this.form);

src/app/extensions/recently/shared/recently-viewed/recently-viewed.component.html

@@ -2,7 +2,6 @@
   <div *ngIf="products.length" class="product-list-container" data-testing-id="recently-viewed">
     <h2>{{ 'recentlyViewed.component.heading' | translate }}</h2>
 
-    <!-- TODO: carousel or better clearing after 4 elements with intended CSS classes -->
     <div class="product-list row">
       <div *ngFor="let productSku of products" class="col-6 col-lg-3 product-list-item">
         <ish-product-item ishProductContext [sku]="productSku" displayType="tile"></ish-product-item>

src/app/extensions/wishlists/shared/wishlist-preferences-dialog/wishlist-preferences-dialog.component.html

@@ -14,7 +14,6 @@ <h2 class="modal-title">{{ modalHeader | translate }}</h2>
 
   <form [formGroup]="wishListForm" (ngSubmit)="submitWishlistForm()">
     <div class="modal-body">
-      <!-- TODO: Show server error -->
       <formly-form [form]="wishListForm" [fields]="fields" [model]="model"> </formly-form>
     </div>
 

src/app/pages/account-overview/account-overview/account-overview.component.html

@@ -71,8 +71,12 @@
       </ng-container>
     </ng-container>
 
+    <!-- ishFeature directives are needed for styling purposes here -->
     <ish-lazy-quote-widget *ishFeature="'quoting'" class="col-12 col-md-4"></ish-lazy-quote-widget>
-    <ish-lazy-order-template-widget class="col-12 col-md-4"></ish-lazy-order-template-widget>
+    <ish-lazy-order-template-widget
+      *ishFeature="'orderTemplates'"
+      class="col-12 col-md-4"
+    ></ish-lazy-order-template-widget>
   </div>
 
   <!-- Order Widget - displays latest orders as list -->

src/app/pages/account-overview/account-overview/account-overview.component.spec.ts

@@ -14,7 +14,6 @@ import { ServerSettingPipe } from 'ish-core/pipes/server-setting.pipe';
 import { RoleToggleModule } from 'ish-core/role-toggle.module';
 import { OrderWidgetComponent } from 'ish-shared/components/order/order-widget/order-widget.component';
 
-import { LazyOrderTemplateWidgetComponent } from '../../../extensions/order-templates/exports/lazy-order-template-widget/lazy-order-template-widget.component';
 import { LazyWishlistWidgetComponent } from '../../../extensions/wishlists/exports/lazy-wishlist-widget/lazy-wishlist-widget.component';
 
 import { AccountOverviewComponent } from './account-overview.component';
@@ -34,7 +33,6 @@ describe('Account Overview Component', () => {
         AccountOverviewComponent,
         MockComponent(FaIconComponent),
         MockComponent(LazyBudgetWidgetComponent),
-        MockComponent(LazyOrderTemplateWidgetComponent),
         MockComponent(LazyRequisitionWidgetComponent),
         MockComponent(LazyWishlistWidgetComponent),
         MockComponent(OrderWidgetComponent),