Skip to content

Commit

Permalink
Merged in task/dspace-cris-2023_02_x/DSC-2050 (pull request DSpace#2636)
Browse files Browse the repository at this point in the history
[DSC-2050] use queryParam for item reports

Approved-by: Francesco Molinaro
  • Loading branch information
Andrea Barbasso authored and FrancescoMolinaro committed Dec 16, 2024
2 parents b9c2c2b + 7f87b2b commit 8c72f12
Show file tree
Hide file tree
Showing 7 changed files with 189 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { TranslateLoaderMock } from '../../../mocks/translate-loader.mock';
import {
metricEmbeddedDownload
} from '../../../../cris-layout/cris-layout-matrix/cris-layout-box-container/boxes/metrics/cris-layout-metrics-box.component.spec';
import { Metric } from '../../../../core/shared/metric.model';

describe('MetricEmbeddedDownloadComponent', () => {
let component: MetricEmbeddedDownloadComponent;
Expand Down Expand Up @@ -34,4 +35,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();
});
});
Original file line number Diff line number Diff line change
@@ -1,15 +1,23 @@
import { Component, Renderer2 } from '@angular/core';
import { Component, OnInit, Renderer2 } from '@angular/core';
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',
styleUrls: ['./metric-embedded-download.component.scss', '../../metric-loader/base-metric.component.scss']
})
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;
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { ActivatedRoute } from '@angular/router';
import { DebugElement, NO_ERRORS_SCHEMA } from '@angular/core';
import { By } from '@angular/platform-browser';

import { of as observableOf } from 'rxjs';
import { of, of as observableOf } from 'rxjs';
import { TranslateLoader, TranslateModule } from '@ngx-translate/core';

import { CrisStatisticsPageComponent } from './cris-statistics-page.component';
Expand All @@ -23,6 +23,7 @@ import { TranslateLoaderMock } from '../../shared/mocks/translate-loader.mock';

import { provideMockStore } from '@ngrx/store/testing';
import { StatisticsState } from '../../core/statistics/statistics.reducer';
import { UsageReport } from '../../core/statistics/models/usage-report.model';

describe('CrisStatisticsPageComponent', () => {

Expand Down Expand Up @@ -141,4 +142,71 @@ describe('CrisStatisticsPageComponent', () => {
fixture.detectChanges();
});

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();
});

});
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,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(
Expand All @@ -189,8 +190,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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,10 +189,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);
});

});
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { Component, EventEmitter, Input, OnInit, Output } from '@angular/core';
import { Component, EventEmitter, inject, Input, OnInit, Output } from '@angular/core';

import { UsageReport } from '../../../core/statistics/models/usage-report.model';
import { StatisticsCategory } from '../../../core/statistics/models/statistics-category.model';
import { ActivatedRoute, Router } from '@angular/router';
import { Location } from '@angular/common';

@Component({
selector: 'ds-statistics-chart',
Expand Down Expand Up @@ -44,6 +46,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
Expand All @@ -58,6 +64,7 @@ export class StatisticsChartComponent implements OnInit {
this.selectedReport = this.reports[0];
this.changeReportEvent.emit(this.reports[0].id);
}
this.addQueryParams(this.selectedReport.reportType);
}
}

Expand All @@ -68,6 +75,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());
}

}

0 comments on commit 8c72f12

Please sign in to comment.