Search code examples
angulartypescriptrxjsngrxrxjs-pipeable-operators

What is the cleanest way of structuring multiple HTTP calls within one @Effect using ngrx


I am using ngrx and have an @Effect hooked up to a LOAD_CONTRACT action, which then makes 3 HTTP calls to get data. Private variables are used to store the data from each GET, so that at the end the LoadContractSuccessAction can be called with a payload containing the 3 retrieved objects.

My code below works fine, and the error handling works as well... but I feel like there may be a neater or better way of structuring things.

I don't know if all the nesting is necessary, or if things could be flattened out in some way. I also am not sure if using switchMap is the best operator.

Could someone with better knowledge of ngrx best practices comment on how the below could be improved/simplified...?

private clientContractIds: IClientContractIds;
private contract: Contract;
private contractSummaryMonths: ContractSummaryMonth[];
private contractSummaryTotals: ContractSummaryTotals;

// Loads a contract and its summary months and totals.
@Effect()
loadContract$ = this.actions$.pipe(
  ofType(ContractActionTypes.LOAD_CONTRACT),
    map((action: IActionWithPayload<IClientContractIds>) => {
      this.clientContractIds = {
        client_id: action.payload.client_id,
        contract_id: action.payload.contract_id
      };
    }),

    // Get the contract.
    switchMap(() => {
      return this.contractService.getContract$(this.clientContractIds).pipe(
        map(contract => (this.contract = contract)),
        catchError(error => throwError(error)),

        // Get the summary months.
        switchMap(() => {
          return this.contractService
            .getContractSummaryMonths$(this.clientContractIds)
            .pipe(
              map(
                contractSummaryMonths =>
                  (this.contractSummaryMonths = contractSummaryMonths)
              ),
              catchError(error => throwError(error))
            );
        }),

        // Get the summary totals.
        switchMap(() => {
          return this.contractService
            .getContractSummaryTotals$(this.clientContractIds)
            .pipe(
              map(
                contractSummaryTotals =>
                  (this.contractSummaryTotals = contractSummaryTotals)
              ),
              catchError(error => throwError(error))
            );
        }),

        // Call the success action with the payload objects.
        switchMap(() => {
          return of(
            new LoadContractSuccessAction({
              contract: this.contract,
              contractSummaryMonths: this.contractSummaryMonths,
              contractSummryTotals: this.contractSummaryTotals
            })
          );
        })
    );
  }),
  catchError(error => {
    return of(new LoadContractFailAction(error));
  })
);

Solution

  • What I would do is have your loadContract$ effect dispatch one action per http call and have one effect per action doing your http call. Splitting your call makes it easier to understand and debug.

    Its probably not 100% accurate but it can give you a general idea.

    @Effect()
    loadContract$ = this.actions$.pipe(
      ofType(ContractActionTypes.LOAD_CONTRACT),
        switchMap((action: IActionWithPayload<IClientContractIds>) => {
          this.clientContractIds = {
            client_id: action.payload.client_id,
            contract_id: action.payload.contract_id
          };
         return [new action1(this.clientContractIds), new action2(this.clientContractIds), new action3(this.clientContractIds), new action4(this.clientContractIds)]
        })
    )
    
    @Effect()
    action1$ = this.actions$.pipe(
        ofType(ContractActionTypes.action1),
        switchMap((action: ACTION1) => {
            return this.contractService
                .getContractSummaryMonths$(action.payload.clientContractIds)
                .pipe(
                    map(
                        contractSummaryMonths =>
                            (this.contractSummaryMonths = contractSummaryMonths)
                    ),
                    catchError(error => throwError(error))
                );
        }))
    

    I kept your code but instead of assigning private variable, I would dispatch success action for each http call that would set the data in your store using the reducers.