Include groupby params in url (CATcher-org#319)

With groupby parameters in the URL, users can easily share the current grouping by using the URL. Let's implement initialization with URL parameters and the ability to set groupby parameters.
NereusWB922 · Mar 29, 2024 · 83dcdae · 83dcdae
1 parent 0867e1c
commit 83dcdae
Show file tree

Hide file tree

Showing 7 changed files with 93 additions and 17 deletions.
diff --git a/src/app/core/services/grouping/grouping-context.service.ts b/src/app/core/services/grouping/grouping-context.service.ts
@@ -1,4 +1,5 @@
 import { Injectable, Injector } from '@angular/core';
+import { ActivatedRoute, Router } from '@angular/router';
 import { BehaviorSubject, Observable } from 'rxjs';
 import { Group } from '../../models/github/group.interface';
 import { Issue } from '../../models/issue.model';
@@ -20,13 +21,14 @@ export const DEFAULT_GROUPBY = GroupBy.Assignee;
   providedIn: 'root'
 })
 export class GroupingContextService {
+  public static readonly GROUP_BY_QUERY_PARAM_KEY = 'groupby';
   private currGroupBySubject: BehaviorSubject<GroupBy>;
   currGroupBy: GroupBy;
   currGroupBy$: Observable<GroupBy>;
 
   private groupingStrategyMap: Map<string, GroupingStrategy>;
 
-  constructor(private injector: Injector) {
+  constructor(private injector: Injector, private route: ActivatedRoute, private router: Router) {
     this.currGroupBy = DEFAULT_GROUPBY;
     this.currGroupBySubject = new BehaviorSubject<GroupBy>(this.currGroupBy);
     this.currGroupBy$ = this.currGroupBySubject.asObservable();
@@ -39,12 +41,34 @@ export class GroupingContextService {
   }
 
   /**
-   * Sets the current grouping type.
-   * @param groupBy - The grouping type to set.
+   * Initializes the service from URL parameters.
+   */
+  initializeFromUrlParams() {
+    const groupByParam = this.route.snapshot.queryParamMap.get(GroupingContextService.GROUP_BY_QUERY_PARAM_KEY);
+
+    if (groupByParam && Object.values(GroupBy).includes(groupByParam as GroupBy)) {
+      this.setCurrentGroupingType(groupByParam as GroupBy);
+    } else {
+      this.setCurrentGroupingType(DEFAULT_GROUPBY);
+    }
+  }
+
+  /**
+   * Sets the current grouping type and updates the corresponding query parameter in the URL.
+   * @param groupBy The grouping type to set.
    */
   setCurrentGroupingType(groupBy: GroupBy): void {
     this.currGroupBy = groupBy;
     this.currGroupBySubject.next(this.currGroupBy);
+
+    this.router.navigate([], {
+      relativeTo: this.route,
+      queryParams: {
+        [GroupingContextService.GROUP_BY_QUERY_PARAM_KEY]: groupBy
+      },
+      queryParamsHandling: 'merge',
+      replaceUrl: true
+    });
   }
 
   /**

diff --git a/src/app/core/services/view.service.ts b/src/app/core/services/view.service.ts
@@ -88,7 +88,8 @@ export class ViewService {
     this.router.navigate(['issuesViewer'], {
       queryParams: {
         [ViewService.REPO_QUERY_PARAM_KEY]: repo.toString()
-      }
+      },
+      queryParamsHandling: 'merge'
     });
   }
 

diff --git a/src/app/issues-viewer/card-view/card-view.component.ts b/src/app/issues-viewer/card-view/card-view.component.ts
@@ -11,7 +11,7 @@ import {
   ViewChild
 } from '@angular/core';
 import { MatPaginator } from '@angular/material/paginator';
-import { Observable } from 'rxjs';
+import { Observable, Subscription } from 'rxjs';
 import { Group } from '../../core/models/github/group.interface';
 import { Issue } from '../../core/models/issue.model';
 import { GroupBy, GroupingContextService } from '../../core/services/grouping/grouping-context.service';
@@ -41,6 +41,10 @@ export class CardViewComponent implements OnInit, AfterViewInit, OnDestroy, Filt
   issues: IssuesDataTable;
   issues$: Observable<Issue[]>;
 
+  private timeoutId: NodeJS.Timeout | null = null;
+  private issuesLengthSubscription: Subscription;
+  private issuesLoadingStateSubscription: Subscription;
+
   isLoading = true;
   issueLength = 0;
 
@@ -62,18 +66,18 @@ export class CardViewComponent implements OnInit, AfterViewInit, OnDestroy, Filt
   }
 
   ngAfterViewInit(): void {
-    setTimeout(() => {
+    this.timeoutId = setTimeout(() => {
       this.issues.loadIssues();
       this.issues$ = this.issues.connect();
 
       // Emit event when issues change
-      this.issues$.subscribe(() => {
+      this.issuesLengthSubscription = this.issues$.subscribe(() => {
         this.issueLength = this.issues.count;
         this.issueLengthChange.emit(this.issueLength);
       });
 
       // Emit event when loading state changes
-      this.issues.isLoading$.subscribe((isLoadingUpdate) => {
+      this.issuesLoadingStateSubscription = this.issues.isLoading$.subscribe((isLoadingUpdate) => {
         this.isLoading = isLoadingUpdate;
       });
     });
@@ -91,9 +95,21 @@ export class CardViewComponent implements OnInit, AfterViewInit, OnDestroy, Filt
   }
 
   ngOnDestroy(): void {
-    setTimeout(() => {
+    if (this.timeoutId) {
+      clearTimeout(this.timeoutId);
+    }
+
+    if (this.issues) {
       this.issues.disconnect();
-    });
+    }
+
+    if (this.issuesLengthSubscription) {
+      this.issuesLengthSubscription.unsubscribe();
+    }
+
+    if (this.issuesLoadingStateSubscription) {
+      this.issuesLoadingStateSubscription.unsubscribe();
+    }
   }
 
   retrieveFilterable(): FilterableSource {

diff --git a/src/app/issues-viewer/issues-viewer.component.ts b/src/app/issues-viewer/issues-viewer.component.ts
@@ -1,5 +1,7 @@
 import { AfterViewInit, Component, OnDestroy, OnInit, QueryList, ViewChildren } from '@angular/core';
+import { NavigationEnd, NavigationStart, Router } from '@angular/router';
 import { BehaviorSubject, of, Subscription } from 'rxjs';
+import { filter } from 'rxjs/operators';
 import { Group } from '../core/models/github/group.interface';
 import { Repo } from '../core/models/repo.model';
 import { ErrorMessageService } from '../core/services/error-message.service';
@@ -28,6 +30,12 @@ export class IssuesViewerComponent implements OnInit, AfterViewInit, OnDestroy {
   /** Observes for any change in the cardviews */
   viewChange: Subscription;
 
+  popStateEventSubscription: Subscription;
+
+  availableGroupsSubscription: Subscription;
+
+  popStateNavigationId: number;
+
   /** Users to show as columns */
   groups: Group[] = [];
 
@@ -44,7 +52,8 @@ export class IssuesViewerComponent implements OnInit, AfterViewInit, OnDestroy {
     public issueService: IssueService,
     public labelService: LabelService,
     public milestoneService: MilestoneService,
-    public groupingContextService: GroupingContextService
+    public groupingContextService: GroupingContextService,
+    private router: Router
   ) {
     this.repoChangeSubscription = this.viewService.repoChanged$.subscribe((newRepo) => {
       this.issueService.reset(false);
@@ -55,10 +64,23 @@ export class IssuesViewerComponent implements OnInit, AfterViewInit, OnDestroy {
     this.groupByChangeSubscription = this.groupingContextService.currGroupBy$.subscribe((newGroupBy) => {
       this.initialize();
     });
+
+    this.popStateEventSubscription = this.router.events
+      .pipe(filter((event) => event instanceof NavigationEnd || event instanceof NavigationStart))
+      .subscribe((event) => {
+        if (event instanceof NavigationStart && event.navigationTrigger === 'popstate') {
+          this.popStateNavigationId = event.id;
+        }
+
+        if (event instanceof NavigationEnd && event.id === this.popStateNavigationId) {
+          this.groupingContextService.initializeFromUrlParams();
+        }
+      });
   }
 
   ngOnInit() {
     this.initialize();
+    this.groupingContextService.initializeFromUrlParams();
   }
 
   ngAfterViewInit(): void {
@@ -68,12 +90,17 @@ export class IssuesViewerComponent implements OnInit, AfterViewInit, OnDestroy {
   ngOnDestroy(): void {
     this.repoChangeSubscription.unsubscribe();
     this.viewChange.unsubscribe();
+    this.popStateEventSubscription.unsubscribe();
   }
 
   /**
    * Fetch and initialize all information from repository to populate Issue Dashboard.
    */
   private initialize() {
+    if (this.availableGroupsSubscription) {
+      this.availableGroupsSubscription.unsubscribe();
+    }
+
     this.checkIfValidRepository().subscribe((isValidRepository) => {
       if (!isValidRepository) {
         throw new Error(ErrorMessageService.repositoryNotPresentMessage());
@@ -84,7 +111,7 @@ export class IssuesViewerComponent implements OnInit, AfterViewInit, OnDestroy {
     this.groups = [];
     this.hiddenGroups = [];
 
-    this.groupingContextService.getGroups().subscribe((x) => (this.groups = x));
+    this.availableGroupsSubscription = this.groupingContextService.getGroups().subscribe((x) => (this.groups = x));
 
     // Fetch issues
     this.issueService.reloadAllIssues();

diff --git a/src/app/shared/issue-tables/IssuesDataTable.ts b/src/app/shared/issue-tables/IssuesDataTable.ts
@@ -39,7 +39,9 @@ export class IssuesDataTable extends DataSource<Issue> implements FilterableSour
   disconnect() {
     this.filterChange.complete();
     this.issuesSubject.complete();
-    this.issueSubscription.unsubscribe();
+    if (this.issueSubscription) {
+      this.issueSubscription.unsubscribe();
+    }
     this.issueService.stopPollIssues();
   }
 

diff --git a/src/app/shared/layout/header.component.ts b/src/app/shared/layout/header.component.ts
@@ -14,6 +14,7 @@ import { ErrorHandlingService } from '../../core/services/error-handling.service
 import { FiltersService } from '../../core/services/filters.service';
 import { GithubService } from '../../core/services/github.service';
 import { GithubEventService } from '../../core/services/githubevent.service';
+import { GroupingContextService } from '../../core/services/grouping/grouping-context.service';
 import { IssueService } from '../../core/services/issue.service';
 import { LabelService } from '../../core/services/label.service';
 import { LoggingService } from '../../core/services/logging.service';
@@ -61,7 +62,8 @@ export class HeaderComponent implements OnInit {
     private githubService: GithubService,
     private dialogService: DialogService,
     private repoSessionStorageService: RepoSessionStorageService,
-    private filtersService: FiltersService
+    private filtersService: FiltersService,
+    private groupingContextService: GroupingContextService
   ) {
     router.events
       .pipe(
@@ -244,6 +246,7 @@ export class HeaderComponent implements OnInit {
       .then(() => {
         this.auth.setTitleWithViewDetail();
         this.currentRepo = newRepoString;
+        this.groupingContextService.reset();
       })
       .catch((error) => {
         this.openChangeRepoDialog();

diff --git a/tests/services/view.service.spec.ts b/tests/services/view.service.spec.ts
@@ -51,7 +51,8 @@ describe('ViewService', () => {
       viewService.setRepository(WATCHER_REPO);
 
       expect(routerSpy.navigate).toHaveBeenCalledWith(['issuesViewer'], {
-        queryParams: { repo: WATCHER_REPO.toString() }
+        queryParams: { repo: WATCHER_REPO.toString() },
+        queryParamsHandling: 'merge'
       });
     });
   });
@@ -88,7 +89,8 @@ describe('ViewService', () => {
       expect(loggingServiceSpy.info).toHaveBeenCalledWith(`ViewService: Changing current repository to '${WATCHER_REPO}'`);
       expect(viewService.currentRepo).toEqual(WATCHER_REPO);
       expect(routerSpy.navigate).toHaveBeenCalledWith(['issuesViewer'], {
-        queryParams: { repo: WATCHER_REPO.toString() }
+        queryParams: { repo: WATCHER_REPO.toString() },
+        queryParamsHandling: 'merge'
       });
       expect(repoUrlCacheServiceSpy.cache).toHaveBeenCalledWith(WATCHER_REPO.toString());
       expect(repoChanged$Spy).toHaveBeenCalledWith(WATCHER_REPO);
@@ -115,7 +117,8 @@ describe('ViewService', () => {
       expect(loggingServiceSpy.info).toHaveBeenCalledWith(`ViewService: Repo is ${WATCHER_REPO}`);
       expect(viewService.currentRepo).toEqual(WATCHER_REPO);
       expect(routerSpy.navigate).toHaveBeenCalledWith(['issuesViewer'], {
-        queryParams: { repo: WATCHER_REPO.toString() }
+        queryParams: { repo: WATCHER_REPO.toString() },
+        queryParamsHandling: 'merge'
       });
       expect(repoSetSourceNext).toHaveBeenCalledWith(true);
     });