0

I get a transaction from API service, the condition is if the transaction status is 'pending', keep reloading and subscribing the transaction until the transaction status is 'completed' or 'rejected'. My code only works for the first time, then next time visit, the page is blank but the data still runs in console even though I unsubscribed it.

Here is my code:

export class TransactionSummaryComponent implements OnInit, OnDestroy {

  transaction: Models.Transaction = <Models.Transaction>{};

  cancelling: boolean = false;
  goToPayment: boolean = false;

  private dataRefreshSub: Subscription;
  private subscribeToDataSub: Subscription;
  private timer: Observable<any>;

  constructor(
    private route: ActivatedRoute,
    private router: Router,
    private apiService: ApiService,
    private zone: NgZone,
    @Inject(PLATFORM_ID) private platformId: Object) { }

  ngOnInit() {
    if (isPlatformBrowser(this.platformId)) {
      this.getTransaction();
    }
  }

  getTransaction() {
    this.route.paramMap
    .switchMap((params: ParamMap) => this.apiService.getTransaction(params.get('id')))
    .subscribe((transaction: Models.Transaction) => {
      this.transaction = transaction;

      if (this.transaction.status === 'Pending') {
        this.refreshData();
      }
    });
  }

  refreshData() {
    this.dataRefreshSub = this.route.paramMap
      .switchMap((params: ParamMap) => this.apiService.getTransaction(params.get('id')))
      .subscribe((transaction: Models.Transaction) => {
        this.transaction = transaction;
        this.subscribeToData();
      });
  }

  subscribeToData() {
    this.zone.runOutsideAngular(() => {
      NgZone.assertNotInAngularZone();
      this.timer = Observable.timer(1, 5000);
      this.subscribeToDataSub = this.timer
        .subscribe(() => {
          this.refreshData();
        });
    });
  }

  ngOnDestroy() {
    if (this.dataRefreshSub !== undefined) {
      this.dataRefreshSub.unsubscribe();
    }
    if (this.subscribeToDataSub !== undefined) {
      this.subscribeToDataSub.unsubscribe();
    }
  }
}
Hoàng Nguyễn
  • 1,121
  • 3
  • 33
  • 57
  • 1
    you have several nested subscribes, which is always an anti pattern, you're only unsubscribing from the most recent refresh subscription, you lose the reference to all the other ones. – bryan60 Dec 04 '17 at 14:56
  • And what is the question? What have you tried? What problems are you facing? This is not a "fix my code plz" website.... – olivarra1 Dec 04 '17 at 14:58
  • @bryan60 alright, I got what you meant, I will give a try for unsubscribing all subscriptions – Hoàng Nguyễn Dec 04 '17 at 15:04
  • @olivarra1 well you can write, but I can't read, I still don't get it. At least Bryan understood my question very clearly. I don't even ask you to fix my code, I asked many questions here by explaining problem rather than asking question like 1 + 1 = ? without facing any troublemaker (Maybe you want this). Thank you for downvoting, man up then ;) – Hoàng Nguyễn Dec 04 '17 at 15:10

1 Answers1

1

I couldn't come up with a solution that doesn't use side effects, but I think it may help you. Rxjs has a retry() operator, which will rerun the subscription for you when it throws. So I'd do something like this:

getTransaction() {
    this.route.paramMap
        .switchMap((params: ParamMap) => this.apiService
            .getTransaction(params.get('id'))
            .do(transaction => this.transaction = transaction) // Bad side effect here, I'm not sure how can this be cleaned out.
            .map(transaction => {
                if(transaction.status === 'Pending') {
                    throw 'Pending';
                }
                return transaction;
            })
            // use .retry(N) to retry at most N times. This will infinitely retry
            .retryWhen(errors => errors)
        )
        .subscribe((transaction: Models.Transaction) => {
            // Here transaction will be 'Completed' or 'Rejected'
        });
}

With this you can delete all other subscriptions, in theory.

olivarra1
  • 3,269
  • 3
  • 23
  • 34
  • I haven't tried out your solution but I will. I came up with a solution of refreshing data from service but no subscription by this answer (https://stackoverflow.com/questions/44947551/angular2-4-refresh-data-realtime/44947870#44947870). And as Bryan stated above about my unsubscription issues, then I also updated the unsubscription methods by this answer (https://stackoverflow.com/questions/38008334/angular-rxjs-when-should-i-unsubscribe-from-subscription/41177163#41177163). Thank you anyway. – Hoàng Nguyễn Dec 05 '17 at 07:52
  • Alright, you solution also works fine, I did not see any bad side effect as you stated – Hoàng Nguyễn Dec 05 '17 at 08:03
  • 1
    @HoàngNguyễn That `.do(...)` over there is called a "side effect": Rxjs operators are meant to be pure, so you can know the whole picture by looking at the declaration of the stream. Having something in between that uses `this` is considered a bad practice, and it's good to think of a way to avoid it. If there's no other option it's ok. Many times they are perfectly accepted (like the one on `.switchMap(params => this.apiService.getTransaction(...))`). Don't worry too much on it if you're not into perfect clean code. – olivarra1 Dec 05 '17 at 08:09