Merged in task/main-cris/DSC-2050 (pull request DSpace#2638)

[DSC-2050] use queryParam for item reports

Approved-by: Francesco Molinaro

Author: Andrea Barbasso

src/app/shared/metric/metric-embedded/metric-embedded-download/metric-embedded-download.component.spec.ts

@@ -9,6 +9,7 @@ import {
 
 import { APP_CONFIG } from '../../../../../config/app-config.interface';
 import { environment } from '../../../../../environments/environment.test';
+import { Metric } from '../../../../core/shared/metric.model';
 import { metricEmbeddedDownload } from '../../../../cris-layout/cris-layout-matrix/cris-layout-box-container/boxes/metrics/cris-layout-metrics-box.component.spec';
 import { TranslateLoaderMock } from '../../../mocks/translate-loader.mock';
 import { MetricEmbeddedDownloadComponent } from './metric-embedded-download.component';
@@ -42,4 +43,25 @@ describe('MetricEmbeddedDownloadComponent', () => {
   it('should create', () => {
     expect(component).toBeTruthy();
   });
+
+  it('should append reportType to href if href is defined and does not contain query parameters', () => {
+    component.href = 'http://example.com';
+    component.metric = {} as Metric;
+    component.ngOnInit();
+    expect(component.href).toBe('http://example.com?reportType=TotalDownloads');
+  });
+
+  it('should append reportType to href if href is defined and contains query parameters', () => {
+    component.href = 'http://example.com?param=value';
+    component.metric = {} as Metric;
+    component.ngOnInit();
+    expect(component.href).toBe('http://example.com?param=value&reportType=TotalDownloads');
+  });
+
+  it('should not modify href if href is not defined', () => {
+    component.href = undefined;
+    component.metric = {} as Metric;
+    component.ngOnInit();
+    expect(component.href).toBeUndefined();
+  });
 });

src/app/shared/metric/metric-embedded/metric-embedded-download/metric-embedded-download.component.ts

@@ -1,13 +1,15 @@
 import { NgIf } from '@angular/common';
 import {
   Component,
+  OnInit,
   Renderer2,
 } from '@angular/core';
 import { TranslateModule } from '@ngx-translate/core';
 
 import { RedirectWithHrefDirective } from '../../../../directives/redirect/redirect-href.directive';
 import { BaseEmbeddedHtmlMetricComponent } from '../base-embedded-html-metric.component';
 
+export const METRIC_TYPE_DOWNLOAD = 'TotalDownloads';
 @Component({
   selector: 'ds-metric-embedded-download',
   templateUrl: './metric-embedded-download.component.html',
@@ -19,10 +21,17 @@ import { BaseEmbeddedHtmlMetricComponent } from '../base-embedded-html-metric.co
     TranslateModule,
   ],
 })
-export class MetricEmbeddedDownloadComponent extends BaseEmbeddedHtmlMetricComponent {
+export class MetricEmbeddedDownloadComponent extends BaseEmbeddedHtmlMetricComponent implements OnInit {
 
   constructor(protected render: Renderer2) {
     super(render);
   }
 
+  ngOnInit() {
+    super.ngOnInit();
+    if (this.href) {
+      this.href += (this.href.includes('?') ? '&' : '?') + 'reportType=' + METRIC_TYPE_DOWNLOAD;
+    }
+  }
+
 }

src/app/statistics-page/cris-statistics-page/cris-statistics-page.component.spec.ts

@@ -14,11 +14,15 @@ import {
   TranslateLoader,
   TranslateModule,
 } from '@ngx-translate/core';
-import { of as observableOf } from 'rxjs';
+import {
+  of as observableOf,
+  of,
+} from 'rxjs';
 
 import { AuthService } from '../../core/auth/auth.service';
 import { SiteDataService } from '../../core/data/site-data.service';
 import { DSpaceObject } from '../../core/shared/dspace-object.model';
+import { UsageReport } from '../../core/statistics/models/usage-report.model';
 import { StatisticsState } from '../../core/statistics/statistics.reducer';
 import { StatisticsCategoriesDataService } from '../../core/statistics/statistics-categories-data.service';
 import { UsageReportDataService } from '../../core/statistics/usage-report-data.service';
@@ -141,4 +145,71 @@ describe('CrisStatisticsPageComponent', () => {
     expect(renderedCategories[0].nativeElement.classList.contains('active')).toBe(true);
   });
 
+  it('should set selectedReportId to the first report id if no reportType query param is present', () => {
+    const category = { id: 'category1' };
+    const reports = [{ id: 'report1', reportType: 'type1' }, { id: 'report2', reportType: 'type2' }];
+    spyOn(component, 'getReports$').and.returnValue(of(reports as UsageReport[]));
+    spyOn(component, 'getReportId').and.returnValue(of(null));
+    spyOn(component, 'getCategoryId').and.returnValue(of(null));
+    spyOn(component, 'setStatisticsState');
+
+    component.getUserReports(category);
+
+    expect(component.setStatisticsState).toHaveBeenCalledWith('report1', 'category1');
+    expect(component.selectedReportId).toBe('report1');
+  });
+
+  it('should set selectedReportId to the report id matching the reportType query param', () => {
+    const category = { id: 'category1' };
+    const reports = [{ id: 'report1', reportType: 'type1' }, { id: 'report2', reportType: 'type2' }];
+    spyOn(component, 'getReports$').and.returnValue(of(reports as UsageReport[]));
+    spyOn(component, 'getReportId').and.returnValue(of(null));
+    spyOn(component, 'getCategoryId').and.returnValue(of(null));
+    spyOn(component, 'setStatisticsState');
+
+    component.getUserReports(category, 'type2');
+
+    expect(component.setStatisticsState).toHaveBeenCalledWith('report2', 'category1');
+    expect(component.selectedReportId).toBe('report2');
+  });
+
+  it('should set selectedReportId to the first report id if reportType query param does not match any report', () => {
+    const category = { id: 'category1' };
+    const reports = [{ id: 'report1', reportType: 'type1' }, { id: 'report2', reportType: 'type2' }];
+    spyOn(component, 'getReports$').and.returnValue(of(reports as UsageReport[]));
+    spyOn(component, 'getReportId').and.returnValue(of(null));
+    spyOn(component, 'getCategoryId').and.returnValue(of(null));
+    spyOn(component, 'setStatisticsState');
+
+    component.getUserReports(category, 'non_existing_type');
+
+    expect(component.setStatisticsState).toHaveBeenCalledWith('report1', 'category1');
+    expect(component.selectedReportId).toBe('report1');
+  });
+
+  it('should set selectedReportId and categoryId from state if they exist', () => {
+    const category = { id: 'category1' };
+    const reports = [{ id: 'report1', reportType: 'type1' }, { id: 'report2', reportType: 'type2' }];
+    spyOn(component, 'getReports$').and.returnValue(of(reports as UsageReport[]));
+    spyOn(component, 'getReportId').and.returnValue(of('report1'));
+    spyOn(component, 'getCategoryId').and.returnValue(of('category1'));
+    spyOn(component, 'setStatisticsState');
+
+    component.getUserReports(category);
+
+    expect(component.setStatisticsState).toHaveBeenCalledWith('report1', 'category1');
+  });
+
+  it('should handle null category gracefully', () => {
+    spyOn(component, 'getReports$').and.returnValue(of([]));
+    spyOn(component, 'getReportId').and.returnValue(of(null));
+    spyOn(component, 'getCategoryId').and.returnValue(of(null));
+    spyOn(component, 'setStatisticsState');
+
+    component.getUserReports(null);
+
+    expect(component.setStatisticsState).not.toHaveBeenCalled();
+    expect(component.selectedReportId).toBeUndefined();
+  });
+
 });

src/app/statistics-page/cris-statistics-page/cris-statistics-page.component.ts

@@ -239,8 +239,9 @@ export class CrisStatisticsPageComponent implements OnInit, OnDestroy {
   /**
    * Get the user reports for the specific category.
    * @param category the that is being selected
+   * @param reportType
    */
-  getUserReports(category) {
+  getUserReports(category, reportType = this.route?.snapshot?.queryParams?.reportType) {
     this.reports$ =
       of(category)
         .pipe(
@@ -250,8 +251,15 @@ export class CrisStatisticsPageComponent implements OnInit, OnDestroy {
       this.reports$, this.getReportId(), this.getCategoryId(),
     ]).subscribe(([report, reportId, categoryId]) => {
       if (!reportId && !categoryId) {
-        this.setStatisticsState(report[0].id, category.id);
-        this.selectedReportId = report[0].id;
+        let reportToShowId = report[0].id;
+        if (reportType) {
+          const newReport = report.find((r) => r.reportType === reportType)?.id;
+          if (newReport) {
+            reportToShowId = newReport;
+          }
+        }
+        this.setStatisticsState(reportToShowId, category.id);
+        this.selectedReportId = reportToShowId;
       } else {
         this.setStatisticsState(reportId, categoryId);
       }

src/app/statistics-page/cris-statistics-page/statistics-chart/statistics-chart.component.html

@@ -10,7 +10,8 @@ <h2>{{'statistics.reports.title' | translate}}</h2>
         <ul class="nav nav-pills mb-2">
           <li class="nav-item mr-3" *ngFor="let report of (reports | dsFilterMap : false)">
             <a class="nav-link" [ngClass]="{'active' : !!selectedReport && selectedReport.id === report.id}"
-               routerLink="./" (click)="$event.preventDefault();changeReport(report)">
+               href="#"
+               (click)="$event.preventDefault();$event.stopPropagation();changeReport(report)">
               {{'statistics.table.' + categoryType + '.title.' + report.reportType | translate}}
             </a>
           </li>

src/app/statistics-page/cris-statistics-page/statistics-chart/statistics-chart.component.spec.ts

@@ -197,10 +197,67 @@ describe('StatisticsChartComponent', () => {
     expect(de.query(By.css('.container'))).toBeNull();
   });
 
-  it('after reports check if container of pills are truthly', () => {
+  it('after reports check if container of pills are truthy', () => {
     component.reports = reports;
     fixture.detectChanges();
     expect(de.query(By.css('.container'))).toBeTruthy();
   });
 
+  it('should set selectedReport to the report matching selectedReportId', () => {
+    component.reports = reports;
+    component.selectedReportId = '1911e8a4-6939-490c-b58b-a5d70f8d91fb_TotalVisits';
+    component.ngOnInit();
+    expect(component.selectedReport.id).toBe('1911e8a4-6939-490c-b58b-a5d70f8d91fb_TotalVisits');
+  });
+
+  it('should set selectedReport to the first report if selectedReportId does not match any report', () => {
+    component.reports = reports;
+    component.selectedReportId = 'non_existing_id';
+    component.ngOnInit();
+    expect(component.selectedReport.id).toBe('1911e8a4-6939-490c-b58b-a5d70f8d91fb_TotalVisits');
+  });
+
+  it('should emit changeReportEvent with the first report id if selectedReportId does not match any report', () => {
+    spyOn(component.changeReportEvent, 'emit');
+    component.reports = reports;
+    component.selectedReportId = 'non_existing_id';
+    component.ngOnInit();
+    expect(component.changeReportEvent.emit).toHaveBeenCalledWith('1911e8a4-6939-490c-b58b-a5d70f8d91fb_TotalVisits');
+  });
+
+  it('should call addQueryParams with the reportType of the selected report', () => {
+    spyOn(component, 'addQueryParams');
+    component.reports = reports;
+    component.selectedReportId = '1911e8a4-6939-490c-b58b-a5d70f8d91fb_TotalVisits';
+    component.ngOnInit();
+    expect(component.addQueryParams).toHaveBeenCalledWith('TotalVisits');
+  });
+
+  it('should not set selectedReport if reports is undefined', () => {
+    component.reports = undefined;
+    component.ngOnInit();
+    expect(component.selectedReport).toBeUndefined();
+  });
+
+  it('should not set selectedReport if reports is empty', () => {
+    component.reports = [];
+    component.ngOnInit();
+    expect(component.selectedReport).toBeUndefined();
+  });
+
+  it('should update selectedReport and emit changeReportEvent on changeReport', () => {
+    spyOn(component.changeReportEvent, 'emit');
+    const newReport = reports[1];
+    component.changeReport(newReport);
+    expect(component.selectedReport).toBe(newReport);
+    expect(component.changeReportEvent.emit).toHaveBeenCalledWith(newReport.id);
+  });
+
+  it('should call addQueryParams with the reportType of the new report on changeReport', () => {
+    spyOn(component, 'addQueryParams');
+    const newReport = reports[1];
+    component.changeReport(newReport);
+    expect(component.addQueryParams).toHaveBeenCalledWith(newReport.reportType);
+  });
+
 });

src/app/statistics-page/cris-statistics-page/statistics-chart/statistics-chart.component.ts

@@ -1,16 +1,22 @@
 import {
+  Location,
   NgClass,
   NgFor,
   NgIf,
 } from '@angular/common';
 import {
   Component,
   EventEmitter,
+  inject,
   Input,
   OnInit,
   Output,
 } from '@angular/core';
-import { RouterLink } from '@angular/router';
+import {
+  ActivatedRoute,
+  Router,
+  RouterLink,
+} from '@angular/router';
 import { TranslateModule } from '@ngx-translate/core';
 
 import { StatisticsCategory } from '../../../core/statistics/models/statistics-category.model';
@@ -71,6 +77,10 @@ export class StatisticsChartComponent implements OnInit {
    */
   @Output() changeReportEvent = new EventEmitter<string>();
 
+  router = inject(Router);
+  activatedRoute = inject(ActivatedRoute);
+  location = inject(Location);
+
   /**
    * Requests the current set values for this chart
    * If the chart config is open by default OR the chart has at least one value, the chart should be initially expanded
@@ -85,6 +95,7 @@ export class StatisticsChartComponent implements OnInit {
         this.selectedReport = this.reports[0];
         this.changeReportEvent.emit(this.reports[0].id);
       }
+      this.addQueryParams(this.selectedReport.reportType);
     }
   }
 
@@ -95,6 +106,15 @@ export class StatisticsChartComponent implements OnInit {
   changeReport(report) {
     this.selectedReport = report;
     this.changeReportEvent.emit(report.id);
+    this.addQueryParams(report.reportType);
+  }
+
+  addQueryParams(reportType: string) {
+    this.location.go(this.router.createUrlTree([], {
+      queryParams: { reportType },
+      queryParamsHandling: 'merge',
+      relativeTo: this.activatedRoute,
+    }).toString());
   }
 
 }