Potential improvement to issueService

[Eclipse-Dominator]

Investigation done in #124 (duplicate of #100)

During initial loading of any repo,

  startPollIssues() {
    if (this.issuesPollSubscription === undefined) {
      if (this.issues$.getValue().length === 0) {
        this.isLoading.next(true);
      }

      this.issuesPollSubscription = timer(0, IssueService.POLL_INTERVAL)
        .pipe(
          exhaustMap(() => {
            return this.reloadAllIssues().pipe(
              catchError(() => {
                return EMPTY;
              }),
              finalize(() => this.isLoading.next(false))
            );
          })
        )
        .subscribe();
    }
  }

Start poll issue will check if current subscription is empty and afterwards check if there are currently any value in issues$. During a repo change, issue$ is not being cleared causing isLoading to be never set to true. This results in the lack of the loading spinner in the dashboard which observes the isLoading observable. Since issues$ did not reset, it leads to isLoading to be never true and also leads to the persistence of old repo data during the repo change.

This can be solved by calling the method below which resets the current session and sets the necessary variables.

  reset(resetSessionId: boolean) {
    if (resetSessionId) {
      this.sessionId = undefined;
    }

    this.issues = undefined;
    this.issues$.next(new Array<Issue>());

    this.stopPollIssues();
    this.isLoading.complete();
    this.isLoading = new BehaviorSubject<boolean>(false);
  }

This was done in the fix provided in #114, calling reset() method is called when a repository is changed.
However, this method also stops the polling of data. Which may affect currently loading processes in the background.
The saving grace here is that the cardViews which contains IssueDataTable which is responsible for loading the issues list for each user is being regenerated when the assignees change during a repo change. This means that the IssueDataTable class will be recreated allowing polling of data to resume during a repo change.

---

Potential Improvement 1

However, in the event of a statically loaded IssueDataTable, this might not be regenerated which can cause polling of data to stop unnecessarily. Which may or may not cause issues. A potential suggestion is to adopt the idea of a shared pointer similar to C++'s std::shared_ptr where we can track the number of references to start poll issues and reduce it when stopPollIssues() is called. Assuming each element properly calls stop poll issues upon destruction, we can then prevent the case above.

This is only a suggestion for now and it might be an overkill since the class itself can manually call startPollIssues() in the scenario mentioned above.

---

Potential Improvement 2

Another potential issue is the resetting of isLoading. isLoading is set to complete() and then changed its reference to a new observable. This can potentially leads to dangling references where old static references to isLoading might no longer be up to date.

Originally posted by @Eclipse-Dominator in #124 (comment)

What do you guys think of these suggestions?

[nknguyenhc]

Hi, can I check if potential improvement 2 is still relevant? It looks like isLoading is no longer set to complete() as described anywhere in issueService, and it is not set to a new reference, as the implementation of the reset method is already changed.