From 8977bb6dfe5a7e8a4a5e8c49106600d593902df9 Mon Sep 17 00:00:00 2001 From: milanmajchrak Date: Fri, 19 Jun 2026 11:01:08 +0200 Subject: [PATCH 1/9] UoE/Add pagination to statistics tables (#726) The statistics tables (e.g. the repository-wide "Total visits" report) rendered every point in a single, ungoverned table and, combined with the backend cap of 10 items, users could only ever see the first 10 datasets. Add client-side pagination to the shared StatisticsTableComponent so that all reports paginate their points and every dataset can be browsed page by page. - Render only the current page of points (default 10 per page). - Show an ngb-pagination control when there are more points than fit on a page. - Add the `statistics.table.pagination.label` i18n key for accessibility. Co-Authored-By: Claude Opus 4.8 --- .../statistics-table.component.html | 16 +++++- .../statistics-table.component.spec.ts | 57 +++++++++++++++++++ .../statistics-table.component.ts | 37 +++++++++++- src/assets/i18n/en.json5 | 2 + 4 files changed, 110 insertions(+), 2 deletions(-) diff --git a/src/app/statistics-page/statistics-table/statistics-table.component.html b/src/app/statistics-page/statistics-table/statistics-table.component.html index efa9ce43d99..d6f24bd4371 100644 --- a/src/app/statistics-page/statistics-table/statistics-table.component.html +++ b/src/app/statistics-page/statistics-table/statistics-table.component.html @@ -18,7 +18,7 @@

- {{ getLabel(point) | async }} @@ -33,4 +33,18 @@

+
+ + +
+ diff --git a/src/app/statistics-page/statistics-table/statistics-table.component.spec.ts b/src/app/statistics-page/statistics-table/statistics-table.component.spec.ts index 105a7623d6b..7673507b294 100644 --- a/src/app/statistics-page/statistics-table/statistics-table.component.spec.ts +++ b/src/app/statistics-page/statistics-table/statistics-table.component.spec.ts @@ -96,5 +96,62 @@ describe('StatisticsTableComponent', () => { expect(de.query(By.css('td.item_2-downloads-data')).nativeElement.innerText) .toEqual('8'); }); + + it('should not display a pagination control when all points fit on a single page', () => { + expect(de.query(By.css('ngb-pagination'))).toBeNull(); + }); + }); + + describe('when the report has more points than the page size', () => { + + const numberOfPoints = 25; + + beforeEach(() => { + const points = []; + for (let i = 0; i < numberOfPoints; i++) { + points.push({ + id: `item_${i}`, + label: `item_${i}`, + values: { + views: i, + }, + }); + } + component.report = Object.assign(new UsageReport(), { points }); + component.ngOnInit(); + fixture.detectChanges(); + }); + + it('should only render the first page of points', () => { + expect(de.queryAll(By.css('[data-test="statistics-label"]')).length) + .toEqual(component.pageSize); + expect(de.query(By.css('td.item_0-views-data'))).toBeTruthy(); + expect(de.query(By.css('td.item_10-views-data'))).toBeNull(); + }); + + it('should display a pagination control', () => { + expect(de.query(By.css('ngb-pagination'))).toBeTruthy(); + }); + + it('should render the next page of points when the page changes', () => { + component.onPageChange(2); + fixture.detectChanges(); + + expect(de.query(By.css('td.item_0-views-data'))).toBeNull(); + expect(de.query(By.css('td.item_10-views-data'))).toBeTruthy(); + expect(de.queryAll(By.css('[data-test="statistics-label"]')).length) + .toEqual(component.pageSize); + }); + + it('should render the remaining points on the last page', () => { + const lastPage = Math.ceil(numberOfPoints / component.pageSize); + component.onPageChange(lastPage); + fixture.detectChanges(); + + const remaining = numberOfPoints - (lastPage - 1) * component.pageSize; + expect(de.queryAll(By.css('[data-test="statistics-label"]')).length) + .toEqual(remaining); + expect(de.query(By.css(`td.item_${numberOfPoints - 1}-views-data`))).toBeTruthy(); + }); }); }); diff --git a/src/app/statistics-page/statistics-table/statistics-table.component.ts b/src/app/statistics-page/statistics-table/statistics-table.component.ts index cd717305681..26ad6d0821f 100644 --- a/src/app/statistics-page/statistics-table/statistics-table.component.ts +++ b/src/app/statistics-page/statistics-table/statistics-table.component.ts @@ -8,6 +8,7 @@ import { Input, OnInit, } from '@angular/core'; +import { NgbPaginationModule } from '@ng-bootstrap/ng-bootstrap'; import { TranslateModule, TranslateService, @@ -38,7 +39,7 @@ import { isEmpty } from '../../shared/empty.util'; templateUrl: './statistics-table.component.html', styleUrls: ['./statistics-table.component.scss'], standalone: true, - imports: [NgIf, NgFor, AsyncPipe, TranslateModule], + imports: [NgIf, NgFor, AsyncPipe, TranslateModule, NgbPaginationModule], }) export class StatisticsTableComponent implements OnInit { @@ -48,6 +49,17 @@ export class StatisticsTableComponent implements OnInit { @Input() report: UsageReport; + /** + * The number of points (e.g. datasets, countries, cities) to show per page. + */ + @Input() + pageSize = 10; + + /** + * The currently displayed page (1-based, as expected by ngb-pagination). + */ + currentPage = 1; + /** * Boolean indicating whether the usage report has data */ @@ -73,6 +85,29 @@ export class StatisticsTableComponent implements OnInit { } } + /** + * The points to render for the currently selected page. + */ + get paginatedPoints(): Point[] { + const start = (this.currentPage - 1) * this.pageSize; + return this.report.points.slice(start, start + this.pageSize); + } + + /** + * Whether a pagination control is needed, i.e. there are more points than fit on a single page. + */ + get showPagination(): boolean { + return this.report.points.length > this.pageSize; + } + + /** + * Switch to the given page. + * @param page the 1-based page number to display + */ + onPageChange(page: number): void { + this.currentPage = page; + } + /** * Get the row label to display for a statistics point. * @param point the statistics point to get the label for diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 04d7384174b..4ad7e9578ed 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -4794,6 +4794,8 @@ "statistics.table.no-data": "No data available", + "statistics.table.pagination.label": "Statistics table pagination", + "statistics.table.title.TotalVisits": "Total visits", "statistics.table.title.TotalVisitsPerMonth": "Total visits per month", From 6c2518cc53fc82cfee7dbc6b6775b0d6d2a9fbe7 Mon Sep 17 00:00:00 2001 From: milanmajchrak Date: Tue, 23 Jun 2026 11:13:41 +0200 Subject: [PATCH 2/9] UoE/Use standard ds-pagination for statistics tables (#726) Replace the raw ngb-pagination control with DSpace's standard PaginationComponent (ds-pagination) for consistency with the rest of the UI. Each statistics report table now paginates its points independently via a unique pagination id, the current page is driven through PaginationService, and the standard "Now showing X - Y of Z" detail and page-size selector are shown. - StatisticsTableComponent: build PaginationComponentOptions and derive the current page of points from PaginationService.getCurrentPagination(). - Template: wrap the table in and iterate the paged points. - Remove the custom statistics.table.pagination.label i18n key (ds-pagination provides its own accessible labels). - Provide a mocked PaginationService in the statistics page specs. Co-Authored-By: Claude Opus 4.8 --- ...llection-statistics-page.component.spec.ts | 2 + ...ommunity-statistics-page.component.spec.ts | 2 + .../item-statistics-page.component.spec.ts | 2 + .../site-statistics-page.component.spec.ts | 2 + .../statistics-table.component.html | 75 +++++++++---------- .../statistics-table.component.spec.ts | 67 ++++++++++++++--- .../statistics-table.component.ts | 55 +++++++------- src/assets/i18n/en.json5 | 2 - 8 files changed, 126 insertions(+), 81 deletions(-) diff --git a/src/app/statistics-page/collection-statistics-page/collection-statistics-page.component.spec.ts b/src/app/statistics-page/collection-statistics-page/collection-statistics-page.component.spec.ts index c78fef0521e..08c7c7c6b8c 100644 --- a/src/app/statistics-page/collection-statistics-page/collection-statistics-page.component.spec.ts +++ b/src/app/statistics-page/collection-statistics-page/collection-statistics-page.component.spec.ts @@ -16,6 +16,7 @@ import { of as observableOf } from 'rxjs'; import { AuthService } from '../../core/auth/auth.service'; import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; import { DSpaceObjectDataService } from '../../core/data/dspace-object-data.service'; +import { PaginationService } from '../../core/pagination/pagination.service'; import { Collection } from '../../core/shared/collection.model'; import { UsageReport } from '../../core/statistics/models/usage-report.model'; import { UsageReportDataService } from '../../core/statistics/usage-report-data.service'; @@ -82,6 +83,7 @@ describe('CollectionStatisticsPageComponent', () => { { provide: DSpaceObjectDataService, useValue: {} }, { provide: DSONameService, useValue: nameService }, { provide: AuthService, useValue: authService }, + { provide: PaginationService, useValue: { getCurrentPagination: () => observableOf({ currentPage: 1, pageSize: 10 }) } }, ], }) .compileComponents(); diff --git a/src/app/statistics-page/community-statistics-page/community-statistics-page.component.spec.ts b/src/app/statistics-page/community-statistics-page/community-statistics-page.component.spec.ts index e29e37880f4..8ce3eb9cb1c 100644 --- a/src/app/statistics-page/community-statistics-page/community-statistics-page.component.spec.ts +++ b/src/app/statistics-page/community-statistics-page/community-statistics-page.component.spec.ts @@ -16,6 +16,7 @@ import { of as observableOf } from 'rxjs'; import { AuthService } from '../../core/auth/auth.service'; import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; import { DSpaceObjectDataService } from '../../core/data/dspace-object-data.service'; +import { PaginationService } from '../../core/pagination/pagination.service'; import { Community } from '../../core/shared/community.model'; import { UsageReport } from '../../core/statistics/models/usage-report.model'; import { UsageReportDataService } from '../../core/statistics/usage-report-data.service'; @@ -82,6 +83,7 @@ describe('CommunityStatisticsPageComponent', () => { { provide: DSpaceObjectDataService, useValue: {} }, { provide: DSONameService, useValue: nameService }, { provide: AuthService, useValue: authService }, + { provide: PaginationService, useValue: { getCurrentPagination: () => observableOf({ currentPage: 1, pageSize: 10 }) } }, ], }) .compileComponents(); diff --git a/src/app/statistics-page/item-statistics-page/item-statistics-page.component.spec.ts b/src/app/statistics-page/item-statistics-page/item-statistics-page.component.spec.ts index f5f3361cce1..133e98acfc3 100644 --- a/src/app/statistics-page/item-statistics-page/item-statistics-page.component.spec.ts +++ b/src/app/statistics-page/item-statistics-page/item-statistics-page.component.spec.ts @@ -16,6 +16,7 @@ import { of as observableOf } from 'rxjs'; import { AuthService } from '../../core/auth/auth.service'; import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; import { DSpaceObjectDataService } from '../../core/data/dspace-object-data.service'; +import { PaginationService } from '../../core/pagination/pagination.service'; import { Item } from '../../core/shared/item.model'; import { UsageReport } from '../../core/statistics/models/usage-report.model'; import { UsageReportDataService } from '../../core/statistics/usage-report-data.service'; @@ -82,6 +83,7 @@ describe('ItemStatisticsPageComponent', () => { { provide: DSpaceObjectDataService, useValue: {} }, { provide: DSONameService, useValue: nameService }, { provide: AuthService, useValue: authService }, + { provide: PaginationService, useValue: { getCurrentPagination: () => observableOf({ currentPage: 1, pageSize: 10 }) } }, ], }) .compileComponents(); diff --git a/src/app/statistics-page/site-statistics-page/site-statistics-page.component.spec.ts b/src/app/statistics-page/site-statistics-page/site-statistics-page.component.spec.ts index 9b23afdd74c..a7c5192fe88 100644 --- a/src/app/statistics-page/site-statistics-page/site-statistics-page.component.spec.ts +++ b/src/app/statistics-page/site-statistics-page/site-statistics-page.component.spec.ts @@ -17,6 +17,7 @@ import { AuthService } from '../../core/auth/auth.service'; import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; import { DSpaceObjectDataService } from '../../core/data/dspace-object-data.service'; import { SiteDataService } from '../../core/data/site-data.service'; +import { PaginationService } from '../../core/pagination/pagination.service'; import { Site } from '../../core/shared/site.model'; import { UsageReport } from '../../core/statistics/models/usage-report.model'; import { UsageReportDataService } from '../../core/statistics/usage-report-data.service'; @@ -83,6 +84,7 @@ describe('SiteStatisticsPageComponent', () => { { provide: DSONameService, useValue: nameService }, { provide: SiteDataService, useValue: siteService }, { provide: AuthService, useValue: authService }, + { provide: PaginationService, useValue: { getCurrentPagination: () => observableOf({ currentPage: 1, pageSize: 10 }) } }, ], }) .compileComponents(); diff --git a/src/app/statistics-page/statistics-table/statistics-table.component.html b/src/app/statistics-page/statistics-table/statistics-table.component.html index d6f24bd4371..879678049f2 100644 --- a/src/app/statistics-page/statistics-table/statistics-table.component.html +++ b/src/app/statistics-page/statistics-table/statistics-table.component.html @@ -5,46 +5,39 @@

{{ 'statistics.table.title.' + report.reportType | translate }}

- - - - - - - - - - - - - - - - -
- {{ header }} -
- {{ getLabel(point) | async }} - - {{ point.values[header] }} -
- -
- - -
+ + + + + + + + + + + + + + + + + + +
+ {{ header }} +
+ {{ getLabel(point) | async }} + + {{ point.values[header] }} +
+ + diff --git a/src/app/statistics-page/statistics-table/statistics-table.component.spec.ts b/src/app/statistics-page/statistics-table/statistics-table.component.spec.ts index 7673507b294..e00b82478ca 100644 --- a/src/app/statistics-page/statistics-table/statistics-table.component.spec.ts +++ b/src/app/statistics-page/statistics-table/statistics-table.component.spec.ts @@ -1,4 +1,8 @@ -import { DebugElement } from '@angular/core'; +import { + Component, + DebugElement, + Input, +} from '@angular/core'; import { ComponentFixture, TestBed, @@ -6,19 +10,50 @@ import { } from '@angular/core/testing'; import { By } from '@angular/platform-browser'; import { TranslateModule } from '@ngx-translate/core'; +import { BehaviorSubject } from 'rxjs'; import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; import { DSpaceObjectDataService } from '../../core/data/dspace-object-data.service'; +import { PaginationService } from '../../core/pagination/pagination.service'; import { UsageReport } from '../../core/statistics/models/usage-report.model'; +import { PaginationComponent } from '../../shared/pagination/pagination.component'; +import { PaginationComponentOptions } from '../../shared/pagination/pagination-component-options.model'; import { StatisticsTableComponent } from './statistics-table.component'; +/** + * Lightweight stand-in for ds-pagination so the table can be tested in isolation; the current page is + * driven through the (mocked) PaginationService, exactly as the real component does via the URL. + */ +@Component({ + selector: 'ds-pagination', + standalone: true, + template: '', +}) +class MockPaginationComponent { + @Input() paginationOptions: PaginationComponentOptions; + @Input() collectionSize: number; + @Input() hideSortOptions: boolean; + @Input() retainScrollPosition: boolean; +} + describe('StatisticsTableComponent', () => { let component: StatisticsTableComponent; let de: DebugElement; let fixture: ComponentFixture; + let currentPagination$: BehaviorSubject; + + const paginationService = { + getCurrentPagination: (_id: string, _options: PaginationComponentOptions) => currentPagination$.asObservable(), + }; + + const setPage = (currentPage: number, pageSize = 10) => { + currentPagination$.next(Object.assign(new PaginationComponentOptions(), { currentPage, pageSize })); + }; beforeEach(waitForAsync(() => { + currentPagination$ = new BehaviorSubject(Object.assign(new PaginationComponentOptions(), { currentPage: 1, pageSize: 10 })); + TestBed.configureTestingModule({ imports: [ TranslateModule.forRoot(), @@ -27,8 +62,13 @@ describe('StatisticsTableComponent', () => { providers: [ { provide: DSpaceObjectDataService, useValue: {} }, { provide: DSONameService, useValue: {} }, + { provide: PaginationService, useValue: paginationService }, ], }) + .overrideComponent(StatisticsTableComponent, { + remove: { imports: [PaginationComponent] }, + add: { imports: [MockPaginationComponent] }, + }) .compileComponents(); })); @@ -51,6 +91,10 @@ describe('StatisticsTableComponent', () => { it ('should not display a table', () => { expect(de.query(By.css('table'))).toBeNull(); }); + + it('should not display a pagination control', () => { + expect(de.query(By.directive(MockPaginationComponent))).toBeNull(); + }); }); describe('when the storage report has data', () => { @@ -97,8 +141,10 @@ describe('StatisticsTableComponent', () => { .toEqual('8'); }); - it('should not display a pagination control when all points fit on a single page', () => { - expect(de.query(By.css('ngb-pagination'))).toBeNull(); + it('should wrap the table in a ds-pagination control with the report size', () => { + const pagination = de.query(By.directive(MockPaginationComponent)); + expect(pagination).toBeTruthy(); + expect(pagination.componentInstance.collectionSize).toEqual(2); }); }); @@ -122,6 +168,11 @@ describe('StatisticsTableComponent', () => { fixture.detectChanges(); }); + it('should pass the full report size to the pagination control', () => { + expect(de.query(By.directive(MockPaginationComponent)).componentInstance.collectionSize) + .toEqual(numberOfPoints); + }); + it('should only render the first page of points', () => { expect(de.queryAll(By.css('[data-test="statistics-label"]')).length) .toEqual(component.pageSize); @@ -129,12 +180,8 @@ describe('StatisticsTableComponent', () => { expect(de.query(By.css('td.item_10-views-data'))).toBeNull(); }); - it('should display a pagination control', () => { - expect(de.query(By.css('ngb-pagination'))).toBeTruthy(); - }); - - it('should render the next page of points when the page changes', () => { - component.onPageChange(2); + it('should render the next page of points when the current page changes', () => { + setPage(2); fixture.detectChanges(); expect(de.query(By.css('td.item_0-views-data'))).toBeNull(); @@ -145,7 +192,7 @@ describe('StatisticsTableComponent', () => { it('should render the remaining points on the last page', () => { const lastPage = Math.ceil(numberOfPoints / component.pageSize); - component.onPageChange(lastPage); + setPage(lastPage); fixture.detectChanges(); const remaining = numberOfPoints - (lastPage - 1) * component.pageSize; diff --git a/src/app/statistics-page/statistics-table/statistics-table.component.ts b/src/app/statistics-page/statistics-table/statistics-table.component.ts index 26ad6d0821f..7c42a2fa18d 100644 --- a/src/app/statistics-page/statistics-table/statistics-table.component.ts +++ b/src/app/statistics-page/statistics-table/statistics-table.component.ts @@ -8,7 +8,6 @@ import { Input, OnInit, } from '@angular/core'; -import { NgbPaginationModule } from '@ng-bootstrap/ng-bootstrap'; import { TranslateModule, TranslateService, @@ -21,6 +20,7 @@ import { map } from 'rxjs/operators'; import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; import { DSpaceObjectDataService } from '../../core/data/dspace-object-data.service'; +import { PaginationService } from '../../core/pagination/pagination.service'; import { getFinishedRemoteData, getRemoteDataPayload, @@ -30,6 +30,8 @@ import { UsageReport, } from '../../core/statistics/models/usage-report.model'; import { isEmpty } from '../../shared/empty.util'; +import { PaginationComponent } from '../../shared/pagination/pagination.component'; +import { PaginationComponentOptions } from '../../shared/pagination/pagination-component-options.model'; /** * Component representing a statistics table for a given usage report. @@ -39,7 +41,7 @@ import { isEmpty } from '../../shared/empty.util'; templateUrl: './statistics-table.component.html', styleUrls: ['./statistics-table.component.scss'], standalone: true, - imports: [NgIf, NgFor, AsyncPipe, TranslateModule, NgbPaginationModule], + imports: [NgIf, NgFor, AsyncPipe, TranslateModule, PaginationComponent], }) export class StatisticsTableComponent implements OnInit { @@ -55,11 +57,6 @@ export class StatisticsTableComponent implements OnInit { @Input() pageSize = 10; - /** - * The currently displayed page (1-based, as expected by ngb-pagination). - */ - currentPage = 1; - /** * Boolean indicating whether the usage report has data */ @@ -70,9 +67,20 @@ export class StatisticsTableComponent implements OnInit { */ headers: string[]; + /** + * Configuration for the {@link PaginationComponent} (ds-pagination) used to page through the points. + */ + paginationOptions: PaginationComponentOptions; + + /** + * The points to render for the currently selected page. + */ + paginatedPoints$: Observable; + constructor( protected dsoService: DSpaceObjectDataService, protected nameService: DSONameService, + protected paginationService: PaginationService, private translateService: TranslateService, ) { @@ -83,29 +91,20 @@ export class StatisticsTableComponent implements OnInit { if (this.hasData) { this.headers = Object.keys(this.report.points[0].values); } - } - /** - * The points to render for the currently selected page. - */ - get paginatedPoints(): Point[] { - const start = (this.currentPage - 1) * this.pageSize; - return this.report.points.slice(start, start + this.pageSize); - } - - /** - * Whether a pagination control is needed, i.e. there are more points than fit on a single page. - */ - get showPagination(): boolean { - return this.report.points.length > this.pageSize; - } + this.paginationOptions = Object.assign(new PaginationComponentOptions(), { + // Unique per report so multiple tables on one statistics page paginate independently + id: `stats-${this.report.reportType}`, + pageSize: this.pageSize, + currentPage: 1, + }); - /** - * Switch to the given page. - * @param page the 1-based page number to display - */ - onPageChange(page: number): void { - this.currentPage = page; + this.paginatedPoints$ = this.paginationService.getCurrentPagination(this.paginationOptions.id, this.paginationOptions).pipe( + map((pagination) => { + const start = (pagination.currentPage - 1) * pagination.pageSize; + return this.report.points.slice(start, start + pagination.pageSize); + }), + ); } /** diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 4ad7e9578ed..04d7384174b 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -4794,8 +4794,6 @@ "statistics.table.no-data": "No data available", - "statistics.table.pagination.label": "Statistics table pagination", - "statistics.table.title.TotalVisits": "Total visits", "statistics.table.title.TotalVisitsPerMonth": "Total visits per month", From 11211c4ef1b72986450da932b0b218f4ef57166a Mon Sep 17 00:00:00 2001 From: milanmajchrak Date: Tue, 23 Jun 2026 15:32:13 +0200 Subject: [PATCH 3/9] UoE/Address Copilot review on statistics pagination (#726) - Make the ds-pagination id unique per scope (use report.id, i.e. `_`) so navigating between scopes that share a report type (e.g. TotalVisits) no longer reuses a stale page from the URL. - Hide the page-size gear so pageSize behaves as a fixed, input-driven size. - Hide the pagination detail/bar for single-page reports, so the pagination UI only appears when there are more points than fit on one page. Co-Authored-By: Claude Opus 4.8 --- .../statistics-table/statistics-table.component.html | 2 ++ .../statistics-table/statistics-table.component.spec.ts | 2 ++ .../statistics-table/statistics-table.component.ts | 5 +++-- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/app/statistics-page/statistics-table/statistics-table.component.html b/src/app/statistics-page/statistics-table/statistics-table.component.html index 879678049f2..7124c09eaf8 100644 --- a/src/app/statistics-page/statistics-table/statistics-table.component.html +++ b/src/app/statistics-page/statistics-table/statistics-table.component.html @@ -7,6 +7,8 @@

diff --git a/src/app/statistics-page/statistics-table/statistics-table.component.spec.ts b/src/app/statistics-page/statistics-table/statistics-table.component.spec.ts index e00b82478ca..c4ee0ea5522 100644 --- a/src/app/statistics-page/statistics-table/statistics-table.component.spec.ts +++ b/src/app/statistics-page/statistics-table/statistics-table.component.spec.ts @@ -32,6 +32,8 @@ import { StatisticsTableComponent } from './statistics-table.component'; class MockPaginationComponent { @Input() paginationOptions: PaginationComponentOptions; @Input() collectionSize: number; + @Input() hideGear: boolean; + @Input() hidePaginationDetail: boolean; @Input() hideSortOptions: boolean; @Input() retainScrollPosition: boolean; } diff --git a/src/app/statistics-page/statistics-table/statistics-table.component.ts b/src/app/statistics-page/statistics-table/statistics-table.component.ts index 7c42a2fa18d..7cb5fd66c98 100644 --- a/src/app/statistics-page/statistics-table/statistics-table.component.ts +++ b/src/app/statistics-page/statistics-table/statistics-table.component.ts @@ -93,8 +93,9 @@ export class StatisticsTableComponent implements OnInit { } this.paginationOptions = Object.assign(new PaginationComponentOptions(), { - // Unique per report so multiple tables on one statistics page paginate independently - id: `stats-${this.report.reportType}`, + // Unique per report AND scope so multiple tables paginate independently and a report doesn't pick up + // another scope's page from the URL. report.id is `_`, e.g. `_TotalVisits`. + id: `stats-${this.report.id}`, pageSize: this.pageSize, currentPage: 1, }); From f9fdfba988e2c3f3de9ca0dc71ec576477f841c1 Mon Sep 17 00:00:00 2001 From: milanmajchrak Date: Thu, 25 Jun 2026 11:08:58 +0200 Subject: [PATCH 4/9] UoE/Add results-per-page selector to statistics tables (#726) Re-enable the standard ds-pagination "results per page" selector (gear) so users can choose how many datasets to show per page (10/20/40/60/80/100), reusing the built-in PaginationComponent control. The gear and pagination detail are shown only when a report has more rows than fit on one page; pageSize remains the default. The page slicing already honours the selected size. Co-Authored-By: Claude Opus 4.8 --- .../statistics-table/statistics-table.component.html | 2 +- .../statistics-table.component.spec.ts | 10 ++++++++++ .../statistics-table/statistics-table.component.ts | 2 ++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/app/statistics-page/statistics-table/statistics-table.component.html b/src/app/statistics-page/statistics-table/statistics-table.component.html index 7124c09eaf8..43c2702acdd 100644 --- a/src/app/statistics-page/statistics-table/statistics-table.component.html +++ b/src/app/statistics-page/statistics-table/statistics-table.component.html @@ -7,7 +7,7 @@

diff --git a/src/app/statistics-page/statistics-table/statistics-table.component.spec.ts b/src/app/statistics-page/statistics-table/statistics-table.component.spec.ts index c4ee0ea5522..82d70c02759 100644 --- a/src/app/statistics-page/statistics-table/statistics-table.component.spec.ts +++ b/src/app/statistics-page/statistics-table/statistics-table.component.spec.ts @@ -148,6 +148,10 @@ describe('StatisticsTableComponent', () => { expect(pagination).toBeTruthy(); expect(pagination.componentInstance.collectionSize).toEqual(2); }); + + it('should hide the page-size selector when everything fits on a single page', () => { + expect(de.query(By.directive(MockPaginationComponent)).componentInstance.hideGear).toBeTrue(); + }); }); describe('when the report has more points than the page size', () => { @@ -175,6 +179,12 @@ describe('StatisticsTableComponent', () => { .toEqual(numberOfPoints); }); + it('should offer the page-size selector with its options when there is more than one page', () => { + const pagination = de.query(By.directive(MockPaginationComponent)).componentInstance; + expect(pagination.hideGear).toBeFalse(); + expect(pagination.paginationOptions.pageSizeOptions).toEqual([10, 20, 40, 60, 80, 100]); + }); + it('should only render the first page of points', () => { expect(de.queryAll(By.css('[data-test="statistics-label"]')).length) .toEqual(component.pageSize); diff --git a/src/app/statistics-page/statistics-table/statistics-table.component.ts b/src/app/statistics-page/statistics-table/statistics-table.component.ts index 7cb5fd66c98..cb5f1c7fe12 100644 --- a/src/app/statistics-page/statistics-table/statistics-table.component.ts +++ b/src/app/statistics-page/statistics-table/statistics-table.component.ts @@ -96,7 +96,9 @@ export class StatisticsTableComponent implements OnInit { // Unique per report AND scope so multiple tables paginate independently and a report doesn't pick up // another scope's page from the URL. report.id is `_`, e.g. `_TotalVisits`. id: `stats-${this.report.id}`, + // pageSize is the default; users can change it via the ds-pagination "results per page" selector. pageSize: this.pageSize, + pageSizeOptions: [10, 20, 40, 60, 80, 100], currentPage: 1, }); From 75f166e5b71d66b32861feb2a4a989e70151aebb Mon Sep 17 00:00:00 2001 From: milanmajchrak Date: Thu, 25 Jun 2026 11:12:03 +0200 Subject: [PATCH 5/9] UoE/Test that a larger selected page size renders more rows (#726) Co-Authored-By: Claude Opus 4.8 --- .../statistics-table/statistics-table.component.spec.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/app/statistics-page/statistics-table/statistics-table.component.spec.ts b/src/app/statistics-page/statistics-table/statistics-table.component.spec.ts index 82d70c02759..d8e010ecc05 100644 --- a/src/app/statistics-page/statistics-table/statistics-table.component.spec.ts +++ b/src/app/statistics-page/statistics-table/statistics-table.component.spec.ts @@ -185,6 +185,14 @@ describe('StatisticsTableComponent', () => { expect(pagination.paginationOptions.pageSizeOptions).toEqual([10, 20, 40, 60, 80, 100]); }); + it('should render more rows when a larger page size is selected', () => { + setPage(1, 20); + fixture.detectChanges(); + + expect(de.queryAll(By.css('[data-test="statistics-label"]')).length).toEqual(20); + expect(de.query(By.css('td.item_19-views-data'))).toBeTruthy(); + }); + it('should only render the first page of points', () => { expect(de.queryAll(By.css('[data-test="statistics-label"]')).length) .toEqual(component.pageSize); From a078905c30162804e80fa70f36da74ca18485b1e Mon Sep 17 00:00:00 2001 From: milanmajchrak <90026355+milanmajchrak@users.noreply.github.com> Date: Fri, 26 Jun 2026 09:57:27 +0200 Subject: [PATCH 6/9] UoE/Add pagination to statistics tables (#726) UoE/Add pagination to statistics tables (#726) --- .../statistics-table/statistics-table.component.scss | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/app/statistics-page/statistics-table/statistics-table.component.scss b/src/app/statistics-page/statistics-table/statistics-table.component.scss index 4e173c040ab..e3e68b5fc46 100644 --- a/src/app/statistics-page/statistics-table/statistics-table.component.scss +++ b/src/app/statistics-page/statistics-table/statistics-table.component.scss @@ -2,6 +2,12 @@ th, td { padding: 0.5rem; } +// Space below the pagination top bar (results-per-page gear + "showing" detail) +// so it isn't glued to the table above it. +:host ::ng-deep ds-pagination .pagination-masked.top { + margin-bottom: 0.75rem; +} + td { width: 50px; max-width: 50px; From c9c39a06bfb8d300bee9418bd3a9a6d7c53e3c1c Mon Sep 17 00:00:00 2001 From: milanmajchrak Date: Tue, 30 Jun 2026 10:11:00 +0200 Subject: [PATCH 7/9] UoE/Remove debug console output and avoidable failed requests Make the running app look professional by removing development console logging and eliminating avoidable failed network requests. Each issue was reproduced against the live dev and production instances and traced to its source. Console noise removed (fired during normal browser use): - "Bitstream: " logged once per file on every item page - "Environment extended with app config" on every page load - debug logs across the submission, upload, access-control and datashare services, the submission footer and the section container Avoidable failed requests eliminated (the request is now avoided, since a browser always logs a network 404/401 that JS cannot suppress): - 404 google.analytics.key on every page -> gated behind a new info.enableGoogleAnalytics flag (default false; DataShare does not use GA) - 404 bulkedit.export.max.items on /search -> only requested once the admin export button is shown - 401 qualityassurancesources on item pages for anonymous users -> gated on the CanSeeQA authorization inside the QA component - 404 /favicon.ico -> the DataShare favicon is served from the web root Specs updated for the new GA flag and CanSeeQA gate (with added coverage). Co-Authored-By: Claude Opus 4.8 --- angular.json | 7 ++- config/config.example.yml | 4 ++ ...bmission-form-section-container.service.ts | 1 - .../datashare/datashare-submission.service.ts | 2 - .../file-section/file-section.component.ts | 8 ---- .../qa-event-notification.component.spec.ts | 32 ++++++++++++- .../qa-event-notification.component.ts | 48 +++++++++++++------ ...access-control-form-container.component.ts | 9 +--- .../bulk-access-control.service.ts | 2 - .../cookies/browser-klaro.service.spec.ts | 22 +++++++++ .../shared/cookies/browser-klaro.service.ts | 13 +++-- .../search-export-csv.component.ts | 25 ++++++++-- .../google-analytics.service.spec.ts | 27 +++++++++++ .../statistics/google-analytics.service.ts | 8 ++++ .../submission-form-footer.component.ts | 6 --- .../container/section-container.component.ts | 2 - .../upload/section-upload.component.ts | 4 -- src/config/config.util.ts | 1 - src/config/default-app-config.ts | 4 ++ src/config/info-config.interface.ts | 8 ++++ src/environments/environment.test.ts | 1 + 21 files changed, 176 insertions(+), 58 deletions(-) diff --git a/angular.json b/angular.json index 26b42f19709..a032f797530 100644 --- a/angular.json +++ b/angular.json @@ -40,7 +40,12 @@ "aot": true, "assets": [ "src/assets", - "src/robots.txt" + "src/robots.txt", + { + "glob": "favicon.ico", + "input": "src/themes/datashare/assets/images", + "output": "/" + } ], "styles": [ "src/styles/startup.scss", diff --git a/config/config.example.yml b/config/config.example.yml index 8ea58b96e40..154b0dea47a 100644 --- a/config/config.example.yml +++ b/config/config.example.yml @@ -465,6 +465,10 @@ info: enableEndUserAgreement: true enablePrivacyStatement: true enableCOARNotifySupport: true + # Only enable Google Analytics on installations that configure a `google.analytics.key` + # property on the backend. When false (default) the UI does not probe for that property, + # avoiding a 404 request on every page. + enableGoogleAnalytics: false # Whether to enable Markdown (https://commonmark.org/) and MathJax (https://www.mathjax.org/) # display in supported metadata fields. By default, only dc.description.abstract is supported. diff --git a/src/app/datashare/datashare-submission-form-section-container.service.ts b/src/app/datashare/datashare-submission-form-section-container.service.ts index 90c6a9e8720..4963dd6f3e5 100644 --- a/src/app/datashare/datashare-submission-form-section-container.service.ts +++ b/src/app/datashare/datashare-submission-form-section-container.service.ts @@ -19,7 +19,6 @@ export class DatashareSubmissionFormSectionContainerService { * @param id The ID of the panel to open, or null to close all panels */ setOpenPanelId(id: string | null): void { - console.log('Setting open panel ID to:', id); this.openPanelId.set(id); } } diff --git a/src/app/datashare/datashare-submission.service.ts b/src/app/datashare/datashare-submission.service.ts index e6e7de66b4b..63a4d10484d 100644 --- a/src/app/datashare/datashare-submission.service.ts +++ b/src/app/datashare/datashare-submission.service.ts @@ -23,14 +23,12 @@ export class DatashareSubmissionService { constructor(private notificationsService: NotificationsService, private translate: TranslateService, ) { - console.log('DatashareSubmissionService created'); } /** * Update the deposit button visibility state */ updatehasUploadFilesErrors(show: boolean): void { - console.log('Updating hasUploadFilesErrors to:', show); this._hasUploadFilesErrorsSignal.set(show); } diff --git a/src/app/item-page/simple/field-components/file-section/file-section.component.ts b/src/app/item-page/simple/field-components/file-section/file-section.component.ts index 6d630ff70f5..09131f6d997 100644 --- a/src/app/item-page/simple/field-components/file-section/file-section.component.ts +++ b/src/app/item-page/simple/field-components/file-section/file-section.component.ts @@ -154,10 +154,6 @@ export class FileSectionComponent implements OnInit { this.notificationsService.error(this.translateService.get('file-section.error.header'), `${bitstreamsRD.statusCode} ${bitstreamsRD.errorMessage}`); } else if (hasValue(bitstreamsRD.payload)) { const current: Bitstream[] = this.bitstreams$.getValue(); - // For debugging. - bitstreamsRD.payload.page.forEach(bitstream => { - console.log('Bitstream:', bitstream); - }); this.bitstreams$.next([...current, ...bitstreamsRD.payload.page]); this.isLoading = false; this.isLastPage = this.currentPage === bitstreamsRD.payload.totalPages; @@ -169,10 +165,6 @@ export class FileSectionComponent implements OnInit { this.notificationsService.error(this.translateService.get('file-section.error.header'), `${licenseRD.statusCode} ${licenseRD.errorMessage}`); } else if (hasValue(licenseRD.payload)) { const updated: Bitstream[] = this.bitstreams$.getValue(); - // For debugging. - licenseRD.payload.page.forEach(bitstream => { - console.log('Bitstream:', bitstream); - }); this.bitstreams$.next([...updated, ...licenseRD.payload.page]); } this.isLoading = false; diff --git a/src/app/item-page/simple/qa-event-notification/qa-event-notification.component.spec.ts b/src/app/item-page/simple/qa-event-notification/qa-event-notification.component.spec.ts index 16141b0359b..d007ffd5907 100644 --- a/src/app/item-page/simple/qa-event-notification/qa-event-notification.component.spec.ts +++ b/src/app/item-page/simple/qa-event-notification/qa-event-notification.component.spec.ts @@ -13,6 +13,7 @@ import { SplitPipe } from 'src/app/shared/utils/split.pipe'; import { APP_DATA_SERVICES_MAP } from '../../../../config/app-config.interface'; import { RemoteDataBuildService } from '../../../core/cache/builders/remote-data-build.service'; import { ObjectCacheService } from '../../../core/cache/object-cache.service'; +import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service'; import { RequestService } from '../../../core/data/request.service'; import { QualityAssuranceSourceObject } from '../../../core/notifications/qa/models/quality-assurance-source.model'; import { QualityAssuranceSourceDataService } from '../../../core/notifications/qa/source/quality-assurance-source-data.service'; @@ -28,6 +29,7 @@ describe('QaEventNotificationComponent', () => { let component: QaEventNotificationComponent; let fixture: ComponentFixture; let qualityAssuranceSourceDataServiceStub: any; + let authorizationServiceStub: any; const obj = Object.assign(new QualityAssuranceSourceObject(), { id: 'sourceName:target', @@ -41,7 +43,10 @@ describe('QaEventNotificationComponent', () => { beforeEach(async () => { qualityAssuranceSourceDataServiceStub = { - getSourcesByTarget: () => objPL, + getSourcesByTarget: jasmine.createSpy('getSourcesByTarget').and.returnValue(objPL), + }; + authorizationServiceStub = { + isAuthorized: jasmine.createSpy('isAuthorized').and.returnValue(of(true)), }; await TestBed.configureTestingModule({ imports: [CommonModule, TranslateModule.forRoot(), QaEventNotificationComponent, SplitPipe], @@ -49,6 +54,7 @@ describe('QaEventNotificationComponent', () => { { provide: APP_DATA_SERVICES_MAP, useValue: {} }, { provide: ActivatedRoute, useValue: new ActivatedRouteStub() }, { provide: QualityAssuranceSourceDataService, useValue: qualityAssuranceSourceDataServiceStub }, + { provide: AuthorizationDataService, useValue: authorizationServiceStub }, { provide: RequestService, useValue: {} }, { provide: NotificationsService, useValue: {} }, { provide: HALEndpointService, useValue: new HALEndpointServiceStub('test') }, @@ -57,6 +63,12 @@ describe('QaEventNotificationComponent', () => { provideMockStore({}), ], }) + // The component declares QualityAssuranceSourceDataService in its own `providers`, + // which would otherwise shadow the stub above with a real instance. Override it so + // the component uses the stub we can assert against. + .overrideComponent(QaEventNotificationComponent, { + set: { providers: [{ provide: QualityAssuranceSourceDataService, useValue: qualityAssuranceSourceDataServiceStub }] }, + }) .compileComponents(); fixture = TestBed.createComponent(QaEventNotificationComponent); component = fixture.componentInstance; @@ -78,4 +90,22 @@ describe('QaEventNotificationComponent', () => { const route = component.getQualityAssuranceRoute(); expect(route).toBe('/notifications/quality-assurance'); }); + + it('should request QA sources when the user is authorized to see QA', () => { + authorizationServiceStub.isAuthorized.and.returnValue(of(true)); + qualityAssuranceSourceDataServiceStub.getSourcesByTarget.calls.reset(); + let result: QualityAssuranceSourceObject[]; + component.getQualityAssuranceSources$().subscribe((sources) => result = sources); + expect(qualityAssuranceSourceDataServiceStub.getSourcesByTarget).toHaveBeenCalled(); + expect(result).toEqual([obj]); + }); + + it('should NOT request QA sources when the user is not authorized', () => { + authorizationServiceStub.isAuthorized.and.returnValue(of(false)); + qualityAssuranceSourceDataServiceStub.getSourcesByTarget.calls.reset(); + let result: QualityAssuranceSourceObject[]; + component.getQualityAssuranceSources$().subscribe((sources) => result = sources); + expect(qualityAssuranceSourceDataServiceStub.getSourcesByTarget).not.toHaveBeenCalled(); + expect(result).toEqual([]); + }); }); diff --git a/src/app/item-page/simple/qa-event-notification/qa-event-notification.component.ts b/src/app/item-page/simple/qa-event-notification/qa-event-notification.component.ts index a428f7feb4f..bef2d7adb99 100644 --- a/src/app/item-page/simple/qa-event-notification/qa-event-notification.component.ts +++ b/src/app/item-page/simple/qa-event-notification/qa-event-notification.component.ts @@ -12,14 +12,20 @@ import { } from '@angular/core'; import { RouterLink } from '@angular/router'; import { TranslateModule } from '@ngx-translate/core'; -import { Observable } from 'rxjs'; +import { + Observable, + of as observableOf, +} from 'rxjs'; import { catchError, map, + switchMap, } from 'rxjs/operators'; import { getNotificatioQualityAssuranceRoute } from '../../../admin/admin-routing-paths'; import { RequestParam } from '../../../core/cache/models/request-param.model'; +import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service'; +import { FeatureID } from '../../../core/data/feature-authorization/feature-id'; import { FindListOptions } from '../../../core/data/find-list-options.model'; import { PaginatedList } from '../../../core/data/paginated-list.model'; import { RemoteData } from '../../../core/data/remote-data'; @@ -61,6 +67,7 @@ export class QaEventNotificationComponent implements OnChanges { constructor( private qualityAssuranceSourceDataService: QualityAssuranceSourceDataService, + private authorizationService: AuthorizationDataService, ) {} /** @@ -77,20 +84,31 @@ export class QaEventNotificationComponent implements OnChanges { * Note: sourceId is composed as: id: "sourceName:" */ getQualityAssuranceSources$(): Observable { - const findListTopicOptions: FindListOptions = { - searchParams: [new RequestParam('target', this.item.uuid)], - }; - return this.qualityAssuranceSourceDataService.getSourcesByTarget(findListTopicOptions, false) - .pipe( - getFirstCompletedRemoteData(), - map((data: RemoteData>) => { - if (data.hasSucceeded) { - return data.payload.page; - } - return []; - }), - catchError(() => []), - ); + // Quality Assurance sources are only available to authorized users. Checking the + // authorization first avoids issuing the `qualityassurancesources/search/byTarget` + // request for anonymous/unauthorized users, which would otherwise return a 401 that + // shows up as a failed request in the browser. Authorized users keep the same behavior. + return this.authorizationService.isAuthorized(FeatureID.CanSeeQA).pipe( + switchMap((canSeeQA: boolean) => { + if (!canSeeQA) { + return observableOf([] as QualityAssuranceSourceObject[]); + } + const findListTopicOptions: FindListOptions = { + searchParams: [new RequestParam('target', this.item.uuid)], + }; + return this.qualityAssuranceSourceDataService.getSourcesByTarget(findListTopicOptions, false) + .pipe( + getFirstCompletedRemoteData(), + map((data: RemoteData>) => { + if (data.hasSucceeded) { + return data.payload.page; + } + return []; + }), + ); + }), + catchError(() => observableOf([] as QualityAssuranceSourceObject[])), + ); } /** diff --git a/src/app/shared/access-control-form-container/access-control-form-container.component.ts b/src/app/shared/access-control-form-container/access-control-form-container.component.ts index bb84aee35d9..d3fde59bcac 100644 --- a/src/app/shared/access-control-form-container/access-control-form-container.component.ts +++ b/src/app/shared/access-control-form-container/access-control-form-container.component.ts @@ -98,11 +98,6 @@ export class AccessControlFormContainerComponent impleme * Will be used from a parent component to read the value of the form */ getFormValue() { - console.log({ - bitstream: this.bitstreamAccessCmp.getValue(), - item: this.itemAccessCmp.getValue(), - state: this.state, - }); return { bitstream: this.bitstreamAccessCmp.getValue(), item: this.itemAccessCmp.getValue(), @@ -137,9 +132,7 @@ export class AccessControlFormContainerComponent impleme this.bulkAccessControlService.executeScript( [ this.itemRD.payload.uuid ], file, - ).pipe(take(1)).subscribe((res) => { - console.log('success', res); - }); + ).pipe(take(1)).subscribe(); } /** diff --git a/src/app/shared/access-control-form-container/bulk-access-control.service.ts b/src/app/shared/access-control-form-container/bulk-access-control.service.ts index f3c284e3580..b4a4a91dc70 100644 --- a/src/app/shared/access-control-form-container/bulk-access-control.service.ts +++ b/src/app/shared/access-control-form-container/bulk-access-control.service.ts @@ -59,8 +59,6 @@ export class BulkAccessControlService { * @param file */ executeScript(uuids: string[], file: File): Observable { - console.log('execute', { uuids, file }); - const params: ProcessParameter[] = [ { name: '-f', value: file.name }, ]; diff --git a/src/app/shared/cookies/browser-klaro.service.spec.ts b/src/app/shared/cookies/browser-klaro.service.spec.ts index 5cdd5275cd3..9b31e853333 100644 --- a/src/app/shared/cookies/browser-klaro.service.spec.ts +++ b/src/app/shared/cookies/browser-klaro.service.spec.ts @@ -6,6 +6,7 @@ import cloneDeep from 'lodash/cloneDeep'; import { of as observableOf } from 'rxjs'; import { TestScheduler } from 'rxjs/testing'; +import { environment } from '../../../environments/environment'; import { AuthService } from '../../core/auth/auth.service'; import { RestResponse } from '../../core/cache/response.models'; import { ConfigurationDataService } from '../../core/data/configuration-data.service'; @@ -310,6 +311,9 @@ describe('BrowserKlaroService', () => { let GOOGLE_ANALYTICS_KEY; let REGISTRATION_VERIFICATION_ENABLED_KEY; beforeEach(() => { + // The Google Analytics key is only probed when GA is enabled for the installation; + // these tests exercise that probe-and-filter path. + environment.info.enableGoogleAnalytics = true; GOOGLE_ANALYTICS_KEY = clone((service as any).GOOGLE_ANALYTICS_KEY); REGISTRATION_VERIFICATION_ENABLED_KEY = clone((service as any).REGISTRATION_VERIFICATION_ENABLED_KEY); spyOn((service as any), 'getUser$').and.returnValue(observableOf(user)); @@ -320,6 +324,10 @@ describe('BrowserKlaroService', () => { configurationDataService.findByPropertyName = findByPropertyName; }); + afterEach(() => { + environment.info.enableGoogleAnalytics = false; + }); + it('should not filter googleAnalytics when servicesToHide are empty', () => { const filteredConfig = (service as any).filterConfigServices([]); expect(filteredConfig).toContain(jasmine.objectContaining({ name: googleAnalytics })); @@ -401,5 +409,19 @@ describe('BrowserKlaroService', () => { service.initialize(); expect(service.klaroConfig.services).not.toContain(jasmine.objectContaining({ name: googleAnalytics })); }); + it('should hide googleAnalytics without requesting the key when GA is disabled', () => { + environment.info.enableGoogleAnalytics = false; + const findByPropertyNameSpy = jasmine.createSpy('findByPropertyName').and.returnValue( + createSuccessfulRemoteDataObject$({ + ...new ConfigurationProperty(), + name: trackingIdTestValue, + values: ['false'], + }), + ); + configurationDataService.findByPropertyName = findByPropertyNameSpy; + service.initialize(); + expect(findByPropertyNameSpy).not.toHaveBeenCalledWith(GOOGLE_ANALYTICS_KEY); + expect(service.klaroConfig.services).not.toContain(jasmine.objectContaining({ name: googleAnalytics })); + }); }); }); diff --git a/src/app/shared/cookies/browser-klaro.service.ts b/src/app/shared/cookies/browser-klaro.service.ts index ea660d9dd6d..d9b0000b32e 100644 --- a/src/app/shared/cookies/browser-klaro.service.ts +++ b/src/app/shared/cookies/browser-klaro.service.ts @@ -114,10 +114,15 @@ export class BrowserKlaroService extends KlaroService { this.klaroConfig.translations.zy.consentNotice.description = 'cookies.consent.content-notice.description.no-privacy'; } - const hideGoogleAnalytics$ = this.configService.findByPropertyName(this.GOOGLE_ANALYTICS_KEY).pipe( - getFirstCompletedRemoteData(), - map(remoteData => !remoteData.hasSucceeded || !remoteData.payload || isEmpty(remoteData.payload.values)), - ); + // Only probe the backend for the Google Analytics key when GA is enabled for this + // installation. When it is disabled we hide the GA service from the consent UI without + // making a request that would otherwise 404 (the `google.analytics.key` property is unset). + const hideGoogleAnalytics$ = environment.info?.enableGoogleAnalytics + ? this.configService.findByPropertyName(this.GOOGLE_ANALYTICS_KEY).pipe( + getFirstCompletedRemoteData(), + map(remoteData => !remoteData.hasSucceeded || !remoteData.payload || isEmpty(remoteData.payload.values)), + ) + : observableOf(true); const hideRegistrationVerification$ = this.configService.findByPropertyName(this.REGISTRATION_VERIFICATION_ENABLED_KEY).pipe( getFirstCompletedRemoteData(), diff --git a/src/app/shared/search/search-export-csv/search-export-csv.component.ts b/src/app/shared/search/search-export-csv/search-export-csv.component.ts index 2727c6cce7c..a4baf28839d 100644 --- a/src/app/shared/search/search-export-csv/search-export-csv.component.ts +++ b/src/app/shared/search/search-export-csv/search-export-csv.component.ts @@ -19,6 +19,7 @@ import { Observable } from 'rxjs'; import { filter, map, + shareReplay, startWith, switchMap, } from 'rxjs/operators'; @@ -92,16 +93,34 @@ export class SearchExportCsvComponent implements OnInit, OnChanges { switchMap(() => this.scriptDataService.scriptWithNameExistsAndCanExecute('metadata-export-search')), map((canExecute: boolean) => canExecute), startWith(false), + shareReplay({ bufferSize: 1, refCount: true }), ); - this.shouldShowWarning$ = this.itemExceeds(); + this.shouldShowWarning$ = this.buildShouldShowWarning$(); } ngOnChanges(changes: SimpleChanges): void { - if (changes.total) { - this.shouldShowWarning$ = this.itemExceeds(); + // ngOnChanges runs before ngOnInit, so shouldShowButton$ may not exist on the first + // change; in that case ngOnInit builds the warning observable with the current total. + if (changes.total && hasValue(this.shouldShowButton$)) { + this.shouldShowWarning$ = this.buildShouldShowWarning$(); } } + /** + * Build the observable that decides whether to show the export-limit warning. + * The backend `bulkedit.export.max.items` property is only requested once the export + * button is shown (i.e. the user is an administrator who can run the export script), so + * anonymous and non-admin users never trigger that request (which would otherwise 404 + * when the property is unset). + */ + private buildShouldShowWarning$(): Observable { + return this.shouldShowButton$.pipe( + filter((canShow: boolean) => canShow), + switchMap(() => this.itemExceeds()), + startWith(false), + ); + } + /** * Checks if the export limit has been exceeded and updates the tooltip accordingly */ diff --git a/src/app/statistics/google-analytics.service.spec.ts b/src/app/statistics/google-analytics.service.spec.ts index eb35750c4ca..63c4b969f14 100644 --- a/src/app/statistics/google-analytics.service.spec.ts +++ b/src/app/statistics/google-analytics.service.spec.ts @@ -4,6 +4,7 @@ import { } from 'angulartics2'; import { of } from 'rxjs'; +import { environment } from '../../environments/environment'; import { ConfigurationDataService } from '../core/data/configuration-data.service'; import { ConfigurationProperty } from '../core/shared/configuration-property.model'; import { KlaroService } from '../shared/cookies/klaro.service'; @@ -40,6 +41,10 @@ describe('GoogleAnalyticsService', () => { }); beforeEach(() => { + // These tests exercise the active Google Analytics tracking path, which is only + // reached when GA is enabled for the installation. + environment.info.enableGoogleAnalytics = true; + googleAnalyticsSpy = jasmine.createSpyObj('Angulartics2GoogleAnalytics', [ 'startTracking', ]); @@ -80,11 +85,33 @@ describe('GoogleAnalyticsService', () => { service = new GoogleAnalyticsService(googleAnalyticsSpy, googleTagManagerSpy, klaroServiceSpy, configSpy, documentSpy ); }); + afterEach(() => { + environment.info.enableGoogleAnalytics = false; + }); + it('should be created', () => { expect(service).toBeTruthy(); }); describe('addTrackingIdToPage()', () => { + describe('when Google Analytics is disabled by config', () => { + beforeEach(() => { + environment.info.enableGoogleAnalytics = false; + }); + + it(`should NOT request the ${trackingIdProp} property`, () => { + service.addTrackingIdToPage(); + expect(configSpy.findByPropertyName).not.toHaveBeenCalled(); + }); + + it('should NOT add a script or start tracking', () => { + service.addTrackingIdToPage(); + expect(bodyElementSpy.appendChild).toHaveBeenCalledTimes(0); + expect(googleAnalyticsSpy.startTracking).toHaveBeenCalledTimes(0); + expect(googleTagManagerSpy.startTracking).toHaveBeenCalledTimes(0); + }); + }); + it(`should request the ${trackingIdProp} property`, () => { service.addTrackingIdToPage(); expect(configSpy.findByPropertyName).toHaveBeenCalledTimes(1); diff --git a/src/app/statistics/google-analytics.service.ts b/src/app/statistics/google-analytics.service.ts index 508297c3b9a..d1c3499c3a2 100644 --- a/src/app/statistics/google-analytics.service.ts +++ b/src/app/statistics/google-analytics.service.ts @@ -9,6 +9,7 @@ import { } from 'angulartics2'; import { combineLatest } from 'rxjs'; +import { environment } from '../../environments/environment'; import { ConfigurationDataService } from '../core/data/configuration-data.service'; import { getFirstCompletedRemoteData } from '../core/shared/operators'; import { KlaroService } from '../shared/cookies/klaro.service'; @@ -38,6 +39,13 @@ export class GoogleAnalyticsService { * page and starts tracking. */ addTrackingIdToPage(): void { + // Skip Google Analytics entirely when it is not enabled for this installation. + // This avoids a 404 request to the `google.analytics.key` configuration property + // (and any tracking setup) on installations that do not use Google Analytics. + if (!environment.info?.enableGoogleAnalytics) { + return; + } + const googleKey$ = this.configService.findByPropertyName('google.analytics.key').pipe( getFirstCompletedRemoteData(), ); diff --git a/src/app/submission/form/footer/submission-form-footer.component.ts b/src/app/submission/form/footer/submission-form-footer.component.ts index 375f9bfd1a3..ffbc8d5ca97 100644 --- a/src/app/submission/form/footer/submission-form-footer.component.ts +++ b/src/app/submission/form/footer/submission-form-footer.component.ts @@ -92,8 +92,6 @@ export class SubmissionFormFooterComponent implements OnChanges { private restService: SubmissionRestService, private submissionService: SubmissionService, private datashareSubmissionService: DatashareSubmissionService) { - // Debug: Log the signal value changes - console.log('Footer component created, initial signal value:', this.hasUploadFileErrorsSignal()); } // DATASHARE - End @@ -110,10 +108,6 @@ export class SubmissionFormFooterComponent implements OnChanges { this.processingDepositStatus = this.submissionService.getSubmissionDepositProcessingStatus(this.submissionId); this.showDepositAndDiscard = observableOf(this.submissionService.getSubmissionScope() === SubmissionScopeType.WorkspaceItem); this.hasUnsavedModification = this.submissionService.hasUnsavedModification(); - // DATASHARE - Start - // Debug: Log signal value on changes - console.log('Footer ngOnChanges, signal value:', this.hasUploadFileErrorsSignal()); - // DATASHARE - End } } diff --git a/src/app/submission/sections/container/section-container.component.ts b/src/app/submission/sections/container/section-container.component.ts index eb4e49885be..cd7b7327f56 100644 --- a/src/app/submission/sections/container/section-container.component.ts +++ b/src/app/submission/sections/container/section-container.component.ts @@ -133,7 +133,6 @@ export class SubmissionSectionContainerComponent implements OnInit { * @returns {string | null} The ID of the currently open panel, or null if no panel is open */ get openPanelId() { - console.log('Open panel ID:', this.datashareSubmissionFormSectionContainerService.openPanelId()); return this.datashareSubmissionFormSectionContainerService.openPanelId(); } @@ -155,7 +154,6 @@ export class SubmissionSectionContainerComponent implements OnInit { */ isSectionOpen(): boolean { const openPanelId = this.datashareSubmissionFormSectionContainerService.openPanelId(); - console.log('Checking if section is open:', this.sectionData.id === openPanelId); return this.sectionData.id === openPanelId; } // DATASHARE - end diff --git a/src/app/submission/sections/upload/section-upload.component.ts b/src/app/submission/sections/upload/section-upload.component.ts index efbf83280cd..dc7cfd724df 100644 --- a/src/app/submission/sections/upload/section-upload.component.ts +++ b/src/app/submission/sections/upload/section-upload.component.ts @@ -268,10 +268,6 @@ export class SubmissionSectionUploadComponent extends SectionModelComponent { // IMPORTANT: Update the shared service signal const hasDuplicates = this.datashareSubmissionService.getDuplicateFileNames(this.fileNames).length > 0; - console.log('File names:', this.fileNames); - console.log('Has duplicates:', hasDuplicates); - console.log('Should show deposit button:', !hasDuplicates); - console.log('this.isTotalUploadedFilesSizeExceeded: ', this.isTotalUploadedFilesSizeExceeded); // Update the service signal this.datashareSubmissionService.updatehasUploadFilesErrors(!hasDuplicates || this.isTotalUploadedFilesSizeExceeded); diff --git a/src/config/config.util.ts b/src/config/config.util.ts index 005606e70ff..08a63db82b9 100644 --- a/src/config/config.util.ts +++ b/src/config/config.util.ts @@ -17,7 +17,6 @@ import { */ const extendEnvironmentWithAppConfig = (env: any, appConfig: AppConfig): void => { mergeConfig(env, appConfig); - console.log(`Environment extended with app config`); }; /** diff --git a/src/config/default-app-config.ts b/src/config/default-app-config.ts index fb4f5a427a1..fcc8eb1b86c 100644 --- a/src/config/default-app-config.ts +++ b/src/config/default-app-config.ts @@ -483,6 +483,10 @@ export class DefaultAppConfig implements AppConfig { enablePrivacyStatement: true, enableCOARNotifySupport: true, enableCookieConsentPopup: true, + // Disabled by default: when no Google Analytics key is configured on the backend, + // probing for it only produces a 404 on every page. Enable this on installations + // that actually configure a Google Analytics tracking id. + enableGoogleAnalytics: false, }; // Whether to enable Markdown (https://commonmark.org/) and MathJax (https://www.mathjax.org/) diff --git a/src/config/info-config.interface.ts b/src/config/info-config.interface.ts index 0adf559a481..bcf7b5d7ab9 100644 --- a/src/config/info-config.interface.ts +++ b/src/config/info-config.interface.ts @@ -5,4 +5,12 @@ export interface InfoConfig extends Config { enablePrivacyStatement: boolean; enableCOARNotifySupport: boolean; enableCookieConsentPopup: boolean; + /** + * When false, the UI will not probe the backend for the `google.analytics.key` + * property and will not attempt to load Google Analytics. Set this to true only + * on installations that actually configure a Google Analytics tracking id on the + * backend. Keeping it false avoids a 404 request to + * `/server/api/config/properties/google.analytics.key` on every page. + */ + enableGoogleAnalytics: boolean; } diff --git a/src/environments/environment.test.ts b/src/environments/environment.test.ts index 89be0daf6ae..6c09521da74 100644 --- a/src/environments/environment.test.ts +++ b/src/environments/environment.test.ts @@ -342,6 +342,7 @@ export const environment: BuildConfig = { enablePrivacyStatement: true, enableCOARNotifySupport: true, enableCookieConsentPopup: true, + enableGoogleAnalytics: false, }, markdown: { enabled: false, From 7dbe95622dcc63e6d0ecb2a98599580eb022426b Mon Sep 17 00:00:00 2001 From: milanmajchrak Date: Tue, 30 Jun 2026 11:04:32 +0200 Subject: [PATCH 8/9] test(search-export-csv): assert bulkedit.export.max.items is only probed when the export button is shown Locks in the fix from this PR so a regression that reintroduces the per-page 404 (probing the config property for non-admin/anonymous users) is caught. Addresses Copilot review feedback. Co-Authored-By: Claude Opus 4.8 --- .../search-export-csv.component.spec.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/app/shared/search/search-export-csv/search-export-csv.component.spec.ts b/src/app/shared/search/search-export-csv/search-export-csv.component.spec.ts index f0672637123..7fd98a15a56 100644 --- a/src/app/shared/search/search-export-csv/search-export-csv.component.spec.ts +++ b/src/app/shared/search/search-export-csv/search-export-csv.component.spec.ts @@ -140,6 +140,27 @@ describe('SearchExportCsvComponent', () => { }); }); }); + describe('bulkedit.export.max.items probe', () => { + beforeEach(waitForAsync(() => { + initBeforeEachAsync(); + })); + + it('should NOT probe bulkedit.export.max.items when the export button is not shown', () => { + // Non-admin: the button (and therefore the export-limit warning) is never rendered, + // so the config property must not be requested - this is what avoids the per-page 404. + (authorizationDataService.isAuthorized as jasmine.Spy).and.returnValue(observableOf(false)); + (configurationDataService.findByPropertyName as jasmine.Spy).calls.reset(); + initBeforeEach(); + expect(configurationDataService.findByPropertyName).not.toHaveBeenCalledWith('bulkedit.export.max.items'); + }); + + it('should probe bulkedit.export.max.items once the export button is shown', () => { + // Admin with the export script available: the warning is evaluated, so the limit is fetched. + (configurationDataService.findByPropertyName as jasmine.Spy).calls.reset(); + initBeforeEach(); + expect(configurationDataService.findByPropertyName).toHaveBeenCalledWith('bulkedit.export.max.items'); + }); + }); describe('export', () => { beforeEach(waitForAsync(() => { initBeforeEachAsync(); From 5584f09b1964624c511620c551845c49c115a39a Mon Sep 17 00:00:00 2001 From: milanmajchrak <90026355+milanmajchrak@users.noreply.github.com> Date: Tue, 30 Jun 2026 18:47:28 +0200 Subject: [PATCH 9/9] UoE/Fixed `Datashare` typo to `DataShare` UoE/Fixed `Datashare` typo to `DataShare` --- src/themes/datashare/app/footer/footer.component.html | 4 ++-- .../accessibility-statement.component.html | 4 ++-- src/themes/datashare/app/navbar/navbar.component.html | 2 +- .../app/shared/search-form/search-form.component.html | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/themes/datashare/app/footer/footer.component.html b/src/themes/datashare/app/footer/footer.component.html index 2a817b3973d..1ba6fa1669d 100644 --- a/src/themes/datashare/app/footer/footer.component.html +++ b/src/themes/datashare/app/footer/footer.component.html @@ -29,11 +29,11 @@