How to convert nested rxjs subscriptions into maps and maintain nested subscription logic-CodePudding

Problem: I have am trying to implement logic to reload page data. When calling an API to reload a user party, if they are a creditor, I need to call another API to load their bank details. Currently, after reading this and other related posts, I get the concept and need to avoid nested subscriptions, but I am unsure how to refactor my code to abide by best practices. I am essentially trying to chain subscriptions to two APIs.

*Using rxjs 6.6.2

Parent Method that calls reload data

  reloadData(partyId) {
    if(!!partyId) {
      this.pageData = new PartyPageData();
      this.reloadPartyDetails(partyId);
      this.getSupportingCurrencies();
    }
  }

Method to reload party details

  reloadPartyDetails(partyId) {
    this.receiverService.getPartyDetails(partyId).pipe(
        take(1),
        map(response => response.body)
      ).subscribe(response => {
      this.pageData.party = response; 
      this.pageData.action = CONSTANTS.PARTY.ACTION.UPDATE;
      if(this.pageData?.party?.side?.toUpperCase() == CONSTANTS.PARTY.PARTY_SIDE.CREDITOR.toUpperCase()) {
        this.pageData.partySide = CONSTANTS.PARTY.PARTY_SIDE.RECEIVER;
        this.reloadBankDetails(partyId);
      }
      else {
        this.pageData.partySide = CONSTANTS.PARTY.PARTY_SIDE.SENDER;
      }
      this.changeDetector.detectChanges();
    }, error => {
      this.goBack();
    });
  }

Nested method to reload bank details

  reloadBankDetails(partyId) {
    this.receiverService.getAllPartyBankAccounts(partyId).pipe(
        take(1),
        map(response => response.body)
      ).subscribe(response => {
      if(!this.pageData?.bank) {
        this.pageData.bank = new BankDetails(CONSTANTS.PARTY.PARTY_SIDE.CREDITOR);
        this.pageData.bank.financialInstitution = new FinancialInstitution;
      }
      this.pageData.bank.accountName = response?.[0].accountName,
      this.pageData.bank.accountNumber = response?.[0].accountNumber;
      this.pageData.bank.currencyCode = response?.[0].currencyCode;
      this.pageData.bank.preferredFlag = response?.[0].preferred;
      this.pageData.bank.type = response?.[0].accountType;
      this.pageData.bank.memberId = response?.[0].financialInstitution.memberId;
      this.pageData.bank.financialInstitution = this.setFinanicalInstitution(response?.[0].financialInstitution);
      this.pageData.bank.accountId = response?.[0].accountId;
      this.pageData.bank.reEnterAccountNumber = response?.[0].accountNumber;

      this.changeDetector.detectChanges();
    }, error => {
      this.goBack();
    });
  }

I feel as though I am missing a core concept. Perhaps regarding mapping or observables and how to handle them.

I looked at rxjs concat() and concatMap() but i am not sure if/how to use them, or to refactor my code so that i can use them.

When looking at other solutions that refactor to maps, i am unsure how properties get implicitly assigned to their target values. e.g. how in the linked page this.seller and this.rating get assigned.

I tried to refactor the reloadPartyDetails to look more like:

  reloadPartyDetails(partyId) {
    this.receiverService.getPartyDetails(partyId).pipe(
        take(1),
        map(response => response.body),
        mergeMap(response => { // or switchMap() or flatMap()?
      this.pageData.party = response; 
      this.pageData.action = CONSTANTS.PARTY.ACTION.UPDATE;
      if(this.pageData?.party?.side?.toUpperCase() == CONSTANTS.PARTY.PARTY_SIDE.CREDITOR.toUpperCase()) {
        this.pageData.partySide = CONSTANTS.PARTY.PARTY_SIDE.RECEIVER;
        return this.pageData.partySide;
      }
      else {
        this.pageData.partySide = CONSTANTS.PARTY.PARTY_SIDE.SENDER;
      }
      this.changeDetector.detectChanges();
    })).subscribe(partySide => {
      this.reloadBankDetails(partyId);
    });
  }

this example made sense, but I did not understand how to refactor it if the .subscribe() was not empty, as in my case.

But I am not sure how to maintain the internal functionality of reloadBankDetails() I am also not sure whether switchMap(), mergeMap(), flatMap(), or concat() is the appopriate higher order observable mapping method to use.

There is only one nested subscription, that should get called conditionally. But re-writing it this way seems to just perpetuate nested subscriptions.

CodePudding user response：

You're spot on with concatMap, pretending you want to do one api call then another (and that the order matters).

First attempt would look something like this

reloadPartyDetails(partyId) {
  this.receiverService.getPartyDetails(partyId).pipe(
    take(1),
    map(response => response.body),
    concatMap(response => {
      let shouldFetchBankDetails = false;
      this.pageData.party = response;
      this.pageData.action = CONSTANTS.PARTY.ACTION.UPDATE;
      if (this.pageData?.party?.side?.toUpperCase() == CONSTANTS.PARTY.PARTY_SIDE.CREDITOR.toUpperCase()) {
        this.pageData.partySide = CONSTANTS.PARTY.PARTY_SIDE.RECEIVER;

        shouldFetchBankDetails = true;

      }
      else {
        this.pageData.partySide = CONSTANTS.PARTY.PARTY_SIDE.SENDER;
      }
      this.changeDetector.detectChanges();

      if (shouldFetchBankDetails) {
        // concatMap expects you to return an observable
        return this.reloadBankDetails(partyId);
      } else {
        return of(); // from rxjs
      }
    }),
    catchError(error => {
      this.goBack();
    })
  ).subscribe(response => {

  }, error => {
    this.goBack();
  });
}

reloadBankDetails(partyId) {
  // we now return this api call, which allows us to subscribe once
  return this.receiverService.getAllPartyBankAccounts(partyId).pipe(
    take(1),
    map(response => response.body),
    tap(response => {
      if (!this.pageData?.bank) {
        this.pageData.bank = new BankDetails(CONSTANTS.PARTY.PARTY_SIDE.CREDITOR);
        this.pageData.bank.financialInstitution = new FinancialInstitution;
      }
      this.pageData.bank.accountName = response?.[0].accountName,
        this.pageData.bank.accountNumber = response?.[0].accountNumber;
      this.pageData.bank.currencyCode = response?.[0].currencyCode;
      this.pageData.bank.preferredFlag = response?.[0].preferred;
      this.pageData.bank.type = response?.[0].accountType;
      this.pageData.bank.memberId = response?.[0].financialInstitution.memberId;
      this.pageData.bank.financialInstitution = this.setFinanicalInstitution(response?.[0].financialInstitution);
      this.pageData.bank.accountId = response?.[0].accountId;
      this.pageData.bank.reEnterAccountNumber = response?.[0].accountNumber;

      this.changeDetector.detectChanges();
    }),
    catchError(error => {
      this.goBack();
    })
  );
  }
}

High level, we're using the operators to do the code instead of doing it all inside of the subscribe. The difference between map and concatMap is that concatMap expects you to return another observable - in this case any api call is fine. The tap operator is what we use when we're setting code, but not setting code for the next observable.

Now, I notice you have some detectChanges in there. This indicates you have a component that is set to changeDetection: ChangeDetectionStrategy.OnPush and when your results come in the component isn't refreshing. If this isn't true then you likely don't need the detectChanges.

Step 2 (or 3) of refactoring this is to have a vm in the html that is being used by the async pipe. This lets the html listen to when new values arrive and then use the OnPush way of telling the component to redraw. The async pipe also replaces your subscribe.

Something like

<div *ngIf="vm$ | async as vm">
  ...
</div>

private reload$ = new Subject<number>();

vm$ = this.reload$.pipe(
  startWith(this.startingPartyId),
  concatMap((partyId)=>{
    this.receiverService.getPartyDetails(partyId).pipe(
      take(1),
      map(response => response.body),
      concatMap(response => {
        ... apply data to your local variable
        if (shouldFetchBankDetails) {
          return this.reloadBankDetails(partyId);
        } else {
          return of(); // from rxjs
        }
      }),
  })

Then your reload data looks something like

reloadData(partyId) {
  if(!!partyId) {
    this.pageData = new PartyPageData();
    this.reload$(partyId);
    this.getSupportingCurrencies();
  }
}

The next refactor I would do is to make the observable return the pageData variable (that it would create inside the concatMap) so that the vm (view model) holds all of the view state. Something like

vm$ = this.reload$.pipe(
  startWith(this.startingPartyId),
  concatMap((partyId)=>{
    // create page data
    const pageData = new PartyPageData();
    this.receiverService.getPartyDetails(partyId).pipe(
      take(1),
      map(response => response.body),
      concatMap(response => {
        ... apply data to your local variable
        if (shouldFetchBankDetails) {
          return this.reloadBankDetails(partyId, pageData).pipe(map(()=> pageData));
        } else {
          return of(pageData); // from rxjs
        }
      }),
  })

Hopefully these examples help - feel free to ask more on it. I answered something almost similar here Also, I still recommend anything by Deborah Kurata, she does a good job of explaining rxjs.