perf: optimize getting order lists (#1565)

BREAKING CHANGE: orders API call now uses additionally the `limit` parameter (requires ICM 11.6), the deprecated `page[limit]` parameter will be removed soon

docs/guides/migrations.md

@@ -10,6 +10,7 @@ kb_sync_latest_only
 ## From 5.0 to 5.1
 
 The OrderListComponent is strictly presentational, components using it have to supply the data.
+The getOrders method of the OrderService doesn't fetch all related order data by default, you can provide an additional parameter to include the data you need.
 
 ## From 4.2 to 5.0
 

src/app/core/facades/account.facade.ts

@@ -11,6 +11,7 @@ import { PasswordReminderUpdate } from 'ish-core/models/password-reminder-update
 import { PasswordReminder } from 'ish-core/models/password-reminder/password-reminder.model';
 import { PaymentInstrument } from 'ish-core/models/payment-instrument/payment-instrument.model';
 import { User } from 'ish-core/models/user/user.model';
+import { OrderListQuery } from 'ish-core/services/order/order.service';
 import { MessagesPayloadType } from 'ish-core/store/core/messages';
 import {
   createCustomerAddress,
@@ -163,8 +164,8 @@ export class AccountFacade {
 
   // ORDERS
 
-  orders$(amount: number) {
-    this.store.dispatch(loadOrders({ amount }));
+  orders$(query?: OrderListQuery) {
+    this.store.dispatch(loadOrders({ query: query || { limit: 30 } }));
     return this.store.pipe(select(getOrders));
   }
 

src/app/core/services/order/order.service.spec.ts

@@ -7,11 +7,11 @@ import { anything, capture, instance, mock, verify, when } from 'ts-mockito';
 
 import { OrderBaseData } from 'ish-core/models/order/order.interface';
 import { Order } from 'ish-core/models/order/order.model';
-import { ApiService } from 'ish-core/services/api/api.service';
+import { ApiService, AvailableOptions } from 'ish-core/services/api/api.service';
 import { getCurrentLocale } from 'ish-core/store/core/configuration';
 import { BasketMockData } from 'ish-core/utils/dev/basket-mock-data';
 
-import { OrderService } from './order.service';
+import { OrderListQuery, OrderService, orderListQueryToHttpParams } from './order.service';
 
 describe('Order Service', () => {
   let orderService: OrderService;
@@ -75,13 +75,26 @@ describe('Order Service', () => {
     it("should get orders when 'getOrders' is called with amount", done => {
       when(apiService.get(anything(), anything())).thenReturn(of([]));
 
-      orderService.getOrders(10).subscribe(() => {
-        verify(apiService.get(`orders?page[limit]=10`, anything())).once();
+      orderService.getOrders({ limit: 30 }).subscribe(() => {
+        verify(apiService.get('orders', anything())).once();
+        const options: AvailableOptions = capture(apiService.get).last()[1];
+        expect(options.params?.toString()).toMatchInlineSnapshot(`"limit=30&page%5Blimit%5D=30"`);
         done();
       });
     });
   });
 
+  describe('orderListQueryToHttpParams', () => {
+    it.each([
+      [{ limit: 10 }, 'limit=10'],
+      [{ limit: 10, include: ['commonShipToAddress'] }, 'limit=10&include=commonShipToAddress'],
+      [{ limit: 30, include: ['discounts', 'payments'] }, 'limit=30&include=discounts,payments'],
+    ] as [OrderListQuery, string][])('should convert %j to %s', (query, expected) => {
+      const params = orderListQueryToHttpParams(query);
+      expect(params.toString()).toEqual(expected);
+    });
+  });
+
   it("should get an order when 'getOrder' is called", done => {
     when(apiService.get(anything(), anything())).thenReturn(of({ data: {} }));
 

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

@@ -22,6 +22,30 @@ type OrderIncludeType =
   | 'payments_paymentMethod'
   | 'payments_paymentInstrument';
 
+export interface OrderListQuery {
+  limit: number;
+  include?: OrderIncludeType[];
+}
+
+export function orderListQueryToHttpParams(query: OrderListQuery): HttpParams {
+  return Object.entries(query).reduce(
+    (acc, [key, value]: [keyof OrderListQuery, OrderListQuery[keyof OrderListQuery]]) => {
+      if (Array.isArray(value)) {
+        if (key === 'include') {
+          return acc.set(key, value.join(','));
+        } else {
+          return value.reduce((acc, value) => acc.append(key, value.toString()), acc);
+        }
+      } else if (value !== undefined) {
+        return acc.set(key, value.toString());
+      } else {
+        return acc;
+      }
+    },
+    new HttpParams()
+  );
+}
+
 /**
  * The Order Service handles the interaction with the REST API concerning orders.
  */
@@ -117,14 +141,18 @@ export class OrderService {
   /**
    * Gets the orders of the logged-in user
    *
-   * @param amount The count of items which should be fetched.
-   * @returns      A list of the user's orders
+   * @param query   Additional query parameters
+   *                - the number of items that should be fetched
+   *                - which data should be included.
+   * @returns       A list of the user's orders
    */
-  getOrders(amount: number): Observable<Order[]> {
-    const params = new HttpParams().set('include', this.allOrderIncludes.join());
+  getOrders(query: OrderListQuery): Observable<Order[]> {
+    let params = orderListQueryToHttpParams(query);
+    // for 7.10 compliance - ToDo: will be removed in PWA 6.0
+    params = params.set('page[limit]', query.limit);
 
     return this.apiService
-      .get<OrderData>(`orders?page[limit]=${amount}`, {
+      .get<OrderData>('orders', {
         headers: this.orderHeaders,
         params,
       })

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

@@ -2,6 +2,7 @@ import { Params } from '@angular/router';
 import { createAction } from '@ngrx/store';
 
 import { Order } from 'ish-core/models/order/order.model';
+import { OrderListQuery } from 'ish-core/services/order/order.service';
 import { httpError, payload } from 'ish-core/utils/ngrx-creators';
 
 export const createOrder = createAction('[Orders] Create Order');
@@ -10,7 +11,7 @@ export const createOrderFail = createAction('[Orders API] Create Order Fail', ht
 
 export const createOrderSuccess = createAction('[Orders API] Create Order Success', payload<{ order: Order }>());
 
-export const loadOrders = createAction('[Orders Internal] Load Orders', payload<{ amount: number }>());
+export const loadOrders = createAction('[Orders Internal] Load Orders', payload<{ query: OrderListQuery }>());
 
 export const loadOrdersFail = createAction('[Orders API] Load Orders Fail', httpError());
 

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

@@ -8,7 +8,7 @@ import { TranslateModule } from '@ngx-translate/core';
 import { cold, hot } from 'jasmine-marbles';
 import { Observable, noop, of, throwError } from 'rxjs';
 import { toArray } from 'rxjs/operators';
-import { anyNumber, anyString, anything, instance, mock, verify, when } from 'ts-mockito';
+import { anyString, anything, instance, mock, verify, when } from 'ts-mockito';
 
 import { Basket } from 'ish-core/models/basket/basket.model';
 import { Customer } from 'ish-core/models/customer/customer.model';
@@ -53,7 +53,7 @@ describe('Orders Effects', () => {
 
   beforeEach(() => {
     orderServiceMock = mock(OrderService);
-    when(orderServiceMock.getOrders(anyNumber())).thenReturn(of(orders));
+    when(orderServiceMock.getOrders(anything())).thenReturn(of(orders));
     when(orderServiceMock.getOrder(anyString())).thenReturn(of(order));
     when(orderServiceMock.getOrderByToken(anyString(), anyString())).thenReturn(of(order));
 
@@ -198,17 +198,17 @@ describe('Orders Effects', () => {
 
   describe('loadOrders$', () => {
     it('should call the orderService for loadOrders', done => {
-      const action = loadOrders({ amount: 30 });
+      const action = loadOrders({ query: { limit: 30 } });
       actions$ = of(action);
 
       effects.loadOrders$.subscribe(() => {
-        verify(orderServiceMock.getOrders(30)).once();
+        verify(orderServiceMock.getOrders(anything())).once();
         done();
       });
     });
 
     it('should load all orders of a user and dispatch a LoadOrdersSuccess action', () => {
-      const action = loadOrders({ amount: 30 });
+      const action = loadOrders({ query: { limit: 30 } });
       const completion = loadOrdersSuccess({ orders });
       actions$ = hot('-a-a-a', { a: action });
       const expected$ = cold('-c-c-c', { c: completion });
@@ -217,9 +217,9 @@ describe('Orders Effects', () => {
     });
 
     it('should dispatch a LoadOrdersFail action if a load error occurs', () => {
-      when(orderServiceMock.getOrders(anyNumber())).thenReturn(throwError(() => makeHttpError({ message: 'error' })));
+      when(orderServiceMock.getOrders(anything())).thenReturn(throwError(() => makeHttpError({ message: 'error' })));
 
-      const action = loadOrders({ amount: 30 });
+      const action = loadOrders({ query: { limit: 30 } });
       const completion = loadOrdersFail({ error: makeHttpError({ message: 'error' }) });
       actions$ = hot('-a-a-a', { a: action });
       const expected$ = cold('-c-c-c', { c: completion });

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

@@ -6,16 +6,7 @@ import { Store, select } from '@ngrx/store';
 import { TranslateService } from '@ngx-translate/core';
 import { isEqual } from 'lodash-es';
 import { EMPTY, from, merge, race } from 'rxjs';
-import {
-  concatMap,
-  distinctUntilChanged,
-  filter,
-  map,
-  mergeMap,
-  switchMap,
-  take,
-  withLatestFrom,
-} from 'rxjs/operators';
+import { concatMap, distinctUntilChanged, filter, map, mergeMap, switchMap, take } from 'rxjs/operators';
 
 import { OrderService } from 'ish-core/services/order/order.service';
 import { ofUrl, selectQueryParam, selectQueryParams, selectRouteParam } from 'ish-core/store/core/router';
@@ -39,7 +30,7 @@ import {
   selectOrderAfterRedirect,
   selectOrderAfterRedirectFail,
 } from './orders.actions';
-import { getOrder, getSelectedOrder, getSelectedOrderId } from './orders.selectors';
+import { getOrder, getSelectedOrder } from './orders.selectors';
 
 @Injectable()
 export class OrdersEffects {
@@ -122,9 +113,9 @@ export class OrdersEffects {
   loadOrders$ = createEffect(() =>
     this.actions$.pipe(
       ofType(loadOrders),
-      mapToPayloadProperty('amount'),
-      switchMap(amount =>
-        this.orderService.getOrders(amount).pipe(
+      mapToPayloadProperty('query'),
+      switchMap(query =>
+        this.orderService.getOrders(query).pipe(
           map(orders => loadOrdersSuccess({ orders })),
           mapErrorToAction(loadOrdersFail)
         )
@@ -182,12 +173,7 @@ export class OrdersEffects {
     merge(
       this.store.pipe(ofUrl(/^\/account\/orders.*/), select(selectRouteParam('orderId'))),
       this.store.pipe(ofUrl(/^\/checkout\/receipt/), select(selectQueryParam('orderId')))
-    ).pipe(
-      withLatestFrom(this.store.pipe(select(getSelectedOrderId))),
-      filter(([fromAction, selectedOrderId]) => fromAction && fromAction !== selectedOrderId),
-      map(([orderId]) => orderId),
-      map(orderId => selectOrder({ orderId }))
-    )
+    ).pipe(map(orderId => selectOrder({ orderId })))
   );
 
   /**

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

@@ -81,7 +81,7 @@ describe('Orders Selectors', () => {
 
   describe('loading orders', () => {
     beforeEach(() => {
-      store$.dispatch(loadOrders({ amount: 30 }));
+      store$.dispatch(loadOrders({ query: { limit: 30 } }));
       store$.dispatch(loadProductSuccess({ product: { sku: 'sku' } as Product }));
     });
 

src/app/pages/account-order-history/account-order-history-page.component.ts

@@ -19,7 +19,7 @@ export class AccountOrderHistoryPageComponent implements OnInit {
   constructor(private accountfacade: AccountFacade) {}
 
   ngOnInit(): void {
-    this.orders$ = this.accountfacade.orders$(30);
+    this.orders$ = this.accountfacade.orders$({ limit: 30, include: ['commonShipToAddress'] });
     this.ordersLoading$ = this.accountfacade.ordersLoading$;
   }
 }

src/app/shared/components/order/order-widget/order-widget.component.ts

@@ -22,7 +22,7 @@ export class OrderWidgetComponent implements OnInit {
   constructor(private accountfacade: AccountFacade) {}
 
   ngOnInit(): void {
-    this.orders$ = this.accountfacade.orders$(5);
+    this.orders$ = this.accountfacade.orders$({ limit: 5 });
     this.ordersLoading$ = this.accountfacade.ordersLoading$;
   }
 }