From f369ead3327bf14e4b1ff18588434bd749609a52 Mon Sep 17 00:00:00 2001 From: FrancescoMolinaro Date: Mon, 13 Apr 2026 09:33:06 +0200 Subject: [PATCH 1/4] [DURACOM-470] implement detailed error handling for custom url conflicts --- src/app/core/router/utils/dso-route.utils.ts | 4 +- src/app/item-page/item-page-routing-paths.ts | 4 +- .../custom-url-conflict-error.component.html | 23 +++ ...ustom-url-conflict-error.component.spec.ts | 184 ++++++++++++++++++ .../custom-url-conflict-error.component.ts | 99 ++++++++++ .../item-page/simple/item-page.component.html | 6 +- .../item-page/simple/item-page.component.ts | 20 ++ src/assets/i18n/en.json5 | 10 + .../item-page/simple/item-page.component.ts | 2 + 9 files changed, 347 insertions(+), 5 deletions(-) create mode 100644 src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.html create mode 100644 src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.spec.ts create mode 100644 src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.ts diff --git a/src/app/core/router/utils/dso-route.utils.ts b/src/app/core/router/utils/dso-route.utils.ts index fc0f8b4ffda..d92b3dc2750 100644 --- a/src/app/core/router/utils/dso-route.utils.ts +++ b/src/app/core/router/utils/dso-route.utils.ts @@ -29,11 +29,11 @@ export function getCommunityPageRoute(communityId: string) { * Depending on the item's entity type, the route will either start with /items or /entities * @param item The item to retrieve the route for */ -export function getItemPageRoute(item: Item) { +export function getItemPageRoute(item: Item, ignoreCustomUrl = false) { const type = item.firstMetadataValue('dspace.entity.type'); let url = item.uuid; - if (isNotEmpty(item.metadata) && item.hasMetadata('dspace.customurl')) { + if (isNotEmpty(item.metadata) && item.hasMetadata('dspace.customurl') && !ignoreCustomUrl) { url = item.firstMetadataValue('dspace.customurl'); } diff --git a/src/app/item-page/item-page-routing-paths.ts b/src/app/item-page/item-page-routing-paths.ts index a6e8ca98d64..1308ce5ba4e 100644 --- a/src/app/item-page/item-page-routing-paths.ts +++ b/src/app/item-page/item-page-routing-paths.ts @@ -6,8 +6,8 @@ import { import { Item } from '@dspace/core/shared/item.model'; import { URLCombiner } from '@dspace/core/url-combiner/url-combiner'; -export function getItemEditRoute(item: Item) { - return new URLCombiner(getItemPageRoute(item), ITEM_EDIT_PATH).toString(); +export function getItemEditRoute(item: Item, ignoreCustomUrl = false) { + return new URLCombiner(getItemPageRoute(item, ignoreCustomUrl), ITEM_EDIT_PATH).toString(); } export function getItemEditVersionhistoryRoute(item: Item) { diff --git a/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.html b/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.html new file mode 100644 index 00000000000..1f0eb394587 --- /dev/null +++ b/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.html @@ -0,0 +1,23 @@ + +

{{ 'error.custom-url-conflict.title' | translate }}

+

{{ 'error.custom-url-conflict.description' | translate: { customUrl: customUrl } }}

+

{{ 'error.custom-url-conflict.admin-action' | translate }}

+ @let items = (conflictingItems$ | async); + @if (items) { + @if (items.length > 0) { + + } @else { +

{{ 'error.custom-url-conflict.no-items-found' | translate }}

+ } + } @else { + + } + diff --git a/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.spec.ts b/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.spec.ts new file mode 100644 index 00000000000..6b2ea123376 --- /dev/null +++ b/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.spec.ts @@ -0,0 +1,184 @@ +import { Component } from '@angular/core'; +import { + ComponentFixture, + TestBed, + waitForAsync, +} from '@angular/core/testing'; +import { By } from '@angular/platform-browser'; +import { provideNoopAnimations } from '@angular/platform-browser/animations'; +import { provideRouter } from '@angular/router'; +import { DSONameService } from '@dspace/core/breadcrumbs/dso-name.service'; +import { Item } from '@dspace/core/shared/item.model'; +import { SearchObjects } from '@dspace/core/shared/search/models/search-objects.model'; +import { SearchResult } from '@dspace/core/shared/search/models/search-result.model'; +import { DSONameServiceMock } from '@dspace/core/testing/dso-name.service.mock'; +import { TranslateLoaderMock } from '@dspace/core/testing/translate-loader.mock'; +import { URLCombiner } from '@dspace/core/url-combiner/url-combiner'; +import { createSuccessfulRemoteDataObject$ } from '@dspace/core/utilities/remote-data.utils'; +import { + TranslateLoader, + TranslateModule, +} from '@ngx-translate/core'; +import { throwError } from 'rxjs'; + +import { AlertComponent } from '../../../shared/alert/alert.component'; +import { SearchService } from '../../../shared/search/search.service'; +import { getItemEditRoute } from '../../item-page-routing-paths'; +import { CustomUrlConflictErrorComponent } from './custom-url-conflict-error.component'; + +@Component({ + selector: 'ds-alert', + template: '', +}) +class AlertComponentStub {} + +describe('CustomUrlConflictErrorComponent', () => { + let component: CustomUrlConflictErrorComponent; + let fixture: ComponentFixture; + let searchServiceSpy: jasmine.SpyObj; + + const customUrl = 'my-conflicting-url'; + + const mockItem1 = Object.assign(new Item(), { + uuid: 'item-uuid-1', + name: 'Item One', + metadata: {}, + _links: { self: { href: 'item-1-selflink' } }, + }); + + const mockItem2 = Object.assign(new Item(), { + uuid: 'item-uuid-2', + name: 'Item Two', + metadata: {}, + _links: { self: { href: 'item-2-selflink' } }, + }); + + const buildSearchResult = (item: Item): SearchResult => + Object.assign(new SearchResult(), { indexableObject: item }); + + const buildSearchObjects = (items: Item[]): SearchObjects => + Object.assign(new SearchObjects(), { page: items.map(buildSearchResult) }); + + const expectedEditLink = (item: Item) => + new URLCombiner(getItemEditRoute(item, true), 'metadata').toString(); + + const createComponent = () => { + fixture = TestBed.createComponent(CustomUrlConflictErrorComponent); + component = fixture.componentInstance; + component.customUrl = customUrl; + fixture.detectChanges(); + }; + + beforeEach(waitForAsync(() => { + searchServiceSpy = jasmine.createSpyObj('SearchService', ['search']); + // Default: return empty results — overridden per suite below + searchServiceSpy.search.and.returnValue( + createSuccessfulRemoteDataObject$(buildSearchObjects([])), + ); + + TestBed.configureTestingModule({ + imports: [ + CustomUrlConflictErrorComponent, + TranslateModule.forRoot({ + loader: { provide: TranslateLoader, useClass: TranslateLoaderMock }, + }), + ], + providers: [ + provideNoopAnimations(), + provideRouter([]), + { provide: SearchService, useValue: searchServiceSpy }, + { provide: DSONameService, useValue: new DSONameServiceMock() }, + ], + }) + .overrideComponent(CustomUrlConflictErrorComponent, { + remove: { imports: [AlertComponent] }, + add: { imports: [AlertComponentStub] }, + }) + .compileComponents(); + })); + + describe('when search returns matching items', () => { + beforeEach(() => { + searchServiceSpy.search.and.returnValue( + createSuccessfulRemoteDataObject$(buildSearchObjects([mockItem1, mockItem2])), + ); + createComponent(); + }); + + it('should create', () => { + expect(component).toBeTruthy(); + }); + + it('should call SearchService.search with a query containing the custom URL', () => { + expect(searchServiceSpy.search).toHaveBeenCalledOnceWith( + jasmine.objectContaining({ query: `dspace.customurl:${customUrl}` }), + ); + }); + + it('should emit one entry per conflicting item', (done) => { + component.conflictingItems$.subscribe((items) => { + expect(items.length).toBe(2); + done(); + }); + }); + + it('should build the correct metadata edit link for each item', (done) => { + component.conflictingItems$.subscribe((items) => { + expect(items[0].editLink).toBe(expectedEditLink(mockItem1)); + expect(items[1].editLink).toBe(expectedEditLink(mockItem2)); + done(); + }); + }); + + it('should use DSONameService to resolve item names', (done) => { + component.conflictingItems$.subscribe((items) => { + expect(items[0].name).toBe(mockItem1.name); + expect(items[1].name).toBe(mockItem2.name); + done(); + }); + }); + + it('should render one edit link per item in the template', () => { + fixture.detectChanges(); + const links = fixture.debugElement.queryAll(By.css('ul li a')); + expect(links.length).toBe(2); + }); + }); + + describe('when search returns no items', () => { + beforeEach(() => { + searchServiceSpy.search.and.returnValue( + createSuccessfulRemoteDataObject$(buildSearchObjects([])), + ); + createComponent(); + }); + + it('should emit an empty array', (done) => { + component.conflictingItems$.subscribe((items) => { + expect(items.length).toBe(0); + done(); + }); + }); + + it('should show the no-items-found message', () => { + fixture.detectChanges(); + const compiled = fixture.nativeElement as HTMLElement; + expect(compiled.textContent).toContain('error.custom-url-conflict.no-items-found'); + }); + }); + + describe('when search throws an error', () => { + beforeEach(() => { + searchServiceSpy.search.and.returnValue(throwError(() => new Error('Network error'))); + createComponent(); + }); + + it('should emit an empty array on error', (done) => { + component.conflictingItems$.subscribe((items) => { + expect(items).toEqual([]); + done(); + }); + }); + }); +}); + diff --git a/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.ts b/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.ts new file mode 100644 index 00000000000..32bf9f92ec9 --- /dev/null +++ b/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.ts @@ -0,0 +1,99 @@ +import { AsyncPipe } from '@angular/common'; +import { + Component, + Input, + OnInit, +} from '@angular/core'; +import { RouterLink } from '@angular/router'; +import { DSONameService } from '@dspace/core/breadcrumbs/dso-name.service'; +import { RemoteData } from '@dspace/core/data/remote-data'; +import { DSpaceObjectType } from '@dspace/core/shared/dspace-object-type.model'; +import { Item } from '@dspace/core/shared/item.model'; +import { getFirstCompletedRemoteData } from '@dspace/core/shared/operators'; +import { PaginatedSearchOptions } from '@dspace/core/shared/search/models/paginated-search-options.model'; +import { SearchObjects } from '@dspace/core/shared/search/models/search-objects.model'; +import { SearchResult } from '@dspace/core/shared/search/models/search-result.model'; +import { URLCombiner } from '@dspace/core/url-combiner/url-combiner'; +import { TranslateModule } from '@ngx-translate/core'; +import { + Observable, + of, +} from 'rxjs'; +import { + catchError, + map, +} from 'rxjs/operators'; + +import { AlertComponent } from '../../../shared/alert/alert.component'; +import { AlertType } from '../../../shared/alert/alert-type'; +import { ThemedLoadingComponent } from '../../../shared/loading/themed-loading.component'; +import { SearchService } from '../../../shared/search/search.service'; +import { getItemEditRoute } from '../../item-page-routing-paths'; + +/** + * Component shown on the item page when a custom URL lookup returns a 500 error, + * which indicates that multiple items share the same `dspace.customurl` value. + * + * Uses `SearchService.search()` with a `dspace.customurl` filter to find all items + * with the conflicting value, then renders a direct edit link (`/edit-items/:FULL`) + * for each result so administrators can immediately navigate to and fix the affected items. + * + * Usage: placed inside the item page template when `itemRD.statusCode === 500` + * and the resolved route param is not a UUID (i.e. it was a custom URL lookup). + */ +@Component({ + selector: 'ds-custom-url-conflict-error', + templateUrl: './custom-url-conflict-error.component.html', + imports: [ + AlertComponent, + AsyncPipe, + RouterLink, + ThemedLoadingComponent, + TranslateModule, + ], +}) +export class CustomUrlConflictErrorComponent implements OnInit { + + /** The custom URL value that caused the conflict (the route param). */ + @Input() customUrl: string; + + /** AlertType enum reference for the template */ + readonly AlertTypeEnum = AlertType; + + /** + * Observable emitting the list of items that share the conflicting custom URL. + * Each entry contains the item's uuid, display name, and a direct edit link + * pointing to `/edit-items/:FULL`. + */ + conflictingItems$: Observable<{ uuid: string; name: string; editLink: string }[]>; + + constructor( + private searchService: SearchService, + private dsoNameService: DSONameService, + ) {} + + ngOnInit(): void { + const searchOptions = new PaginatedSearchOptions({ + dsoTypes: [DSpaceObjectType.ITEM], + query: `dspace.customurl:${this.customUrl}`, + }); + + this.conflictingItems$ = this.searchService.search(searchOptions).pipe( + getFirstCompletedRemoteData(), + map((rd: RemoteData>) => { + if (rd.hasSucceeded && rd.payload?.page?.length > 0) { + return rd.payload.page.map((result: SearchResult) => { + const item = result.indexableObject; + return { + uuid: item.uuid, + name: this.dsoNameService.getName(item), + editLink: new URLCombiner(getItemEditRoute(item, true), 'metadata').toString(), + }; + }); + } + return []; + }), + catchError(() => of([])), + ); + } +} diff --git a/src/app/item-page/simple/item-page.component.html b/src/app/item-page/simple/item-page.component.html index 2b9fdebec5b..4de3dd56b1c 100644 --- a/src/app/item-page/simple/item-page.component.html +++ b/src/app/item-page/simple/item-page.component.html @@ -18,7 +18,11 @@ } @if (itemRD?.hasFailed) { - + @if ((customUrlConflict$ | async); as conflictUrl) { + + } @else { + + } } @if (itemRD?.isLoading) { diff --git a/src/app/item-page/simple/item-page.component.ts b/src/app/item-page/simple/item-page.component.ts index 8260e2fd8d4..0e1bf923f7f 100644 --- a/src/app/item-page/simple/item-page.component.ts +++ b/src/app/item-page/simple/item-page.component.ts @@ -46,6 +46,7 @@ import { switchMap, take, } from 'rxjs/operators'; +import { validate as uuidValidate } from 'uuid'; import { fadeInOut } from '../../shared/animations/fade'; import { ErrorComponent } from '../../shared/error/error.component'; @@ -56,6 +57,7 @@ import { ThemedItemAlertsComponent } from '../alerts/themed-item-alerts.componen import { ItemVersionsComponent } from '../versions/item-versions.component'; import { ItemVersionsNoticeComponent } from '../versions/notice/item-versions-notice.component'; import { AccessByTokenNotificationComponent } from './access-by-token-notification/access-by-token-notification.component'; +import { CustomUrlConflictErrorComponent } from './custom-url-conflict-error/custom-url-conflict-error.component'; import { NotifyRequestsStatusComponent } from './notify-requests-status/notify-requests-status-component/notify-requests-status.component'; import { QaEventNotificationComponent } from './qa-event-notification/qa-event-notification.component'; @@ -73,6 +75,8 @@ import { QaEventNotificationComponent } from './qa-event-notification/qa-event-n imports: [ AccessByTokenNotificationComponent, AsyncPipe, + CustomUrlConflictErrorComponent, + CustomUrlConflictErrorComponent, ErrorComponent, ItemVersionsComponent, ItemVersionsNoticeComponent, @@ -119,6 +123,12 @@ export class ItemPageComponent implements OnInit, OnDestroy { itemUrl: string; + /** + * When set, indicates that the page failed due to a custom URL conflict (HTTP 500 + non-UUID param). + * Contains the conflicting custom URL value so the error component can build search/edit links. + */ + customUrlConflict$: Observable; + /** * Contains a list of SignpostingLink related to the item */ @@ -162,6 +172,16 @@ export class ItemPageComponent implements OnInit, OnDestroy { this.isAdmin$ = this.authorizationService.isAuthorized(FeatureID.AdministratorOf); + // Detect custom URL conflict: 500 error on a non-UUID route param + this.customUrlConflict$ = this.itemRD$.pipe( + map((itemRD: RemoteData) => { + const routeId = this.route.snapshot.params.id; + if (itemRD?.hasFailed && itemRD.statusCode === 500 && hasValue(routeId) && !uuidValidate(routeId)) { + return routeId as string; + } + return null; + }), + ); } /** diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 32577407f5d..bb2eb12f8bb 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -1983,6 +1983,16 @@ "error.item": "Error fetching item", + "error.custom-url-conflict.title": "Duplicate custom URL detected", + + "error.custom-url-conflict.description": "The custom URL \"{{customUrl}}\" is assigned to more than one item. This causes a conflict and the item cannot be displayed.", + + "error.custom-url-conflict.admin-action": "Please contact your repository administrator. The following affected items need their dspace.customurl metadata fixed so each value is unique:", + + "error.custom-url-conflict.edit-link": "Edit \"{{name}}\" ({{uuid}})", + + "error.custom-url-conflict.no-items-found": "No conflicting items could be retrieved. Please contact your administrator.", + "error.items": "Error fetching items", "error.objects": "Error fetching objects", diff --git a/src/themes/custom/app/item-page/simple/item-page.component.ts b/src/themes/custom/app/item-page/simple/item-page.component.ts index 202338f652b..81c8cd98033 100644 --- a/src/themes/custom/app/item-page/simple/item-page.component.ts +++ b/src/themes/custom/app/item-page/simple/item-page.component.ts @@ -7,6 +7,7 @@ import { TranslateModule } from '@ngx-translate/core'; import { ThemedItemAlertsComponent } from '../../../../../app/item-page/alerts/themed-item-alerts.component'; import { AccessByTokenNotificationComponent } from '../../../../../app/item-page/simple/access-by-token-notification/access-by-token-notification.component'; +import { CustomUrlConflictErrorComponent } from '../../../../../app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component'; import { ItemPageComponent as BaseComponent } from '../../../../../app/item-page/simple/item-page.component'; import { NotifyRequestsStatusComponent } from '../../../../../app/item-page/simple/notify-requests-status/notify-requests-status-component/notify-requests-status.component'; import { QaEventNotificationComponent } from '../../../../../app/item-page/simple/qa-event-notification/qa-event-notification.component'; @@ -29,6 +30,7 @@ import { VarDirective } from '../../../../../app/shared/utils/var.directive'; imports: [ AccessByTokenNotificationComponent, AsyncPipe, + CustomUrlConflictErrorComponent, ErrorComponent, ItemVersionsComponent, ItemVersionsNoticeComponent, From 06e37b736913c52edd04306af0a86000313f7760 Mon Sep 17 00:00:00 2001 From: FrancescoMolinaro Date: Thu, 16 Apr 2026 16:11:18 +0200 Subject: [PATCH 2/4] [DURACOM-470] show errors as text for unauthenticated users --- .../custom-url-conflict-error.component.html | 10 +++- ...ustom-url-conflict-error.component.spec.ts | 57 ++++++++++++++++--- .../custom-url-conflict-error.component.ts | 6 ++ 3 files changed, 63 insertions(+), 10 deletions(-) diff --git a/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.html b/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.html index 1f0eb394587..a79e27bd2ab 100644 --- a/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.html +++ b/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.html @@ -8,9 +8,13 @@

{{ 'error.custom-url-conflict.title' | translate }}

diff --git a/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.spec.ts b/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.spec.ts index 6b2ea123376..fbdc0ef7317 100644 --- a/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.spec.ts +++ b/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.spec.ts @@ -7,6 +7,7 @@ import { import { By } from '@angular/platform-browser'; import { provideNoopAnimations } from '@angular/platform-browser/animations'; import { provideRouter } from '@angular/router'; +import { AuthService } from '@dspace/core/auth/auth.service'; import { DSONameService } from '@dspace/core/breadcrumbs/dso-name.service'; import { Item } from '@dspace/core/shared/item.model'; import { SearchObjects } from '@dspace/core/shared/search/models/search-objects.model'; @@ -19,7 +20,10 @@ import { TranslateLoader, TranslateModule, } from '@ngx-translate/core'; -import { throwError } from 'rxjs'; +import { + of, + throwError, +} from 'rxjs'; import { AlertComponent } from '../../../shared/alert/alert.component'; import { SearchService } from '../../../shared/search/search.service'; @@ -36,6 +40,7 @@ describe('CustomUrlConflictErrorComponent', () => { let component: CustomUrlConflictErrorComponent; let fixture: ComponentFixture; let searchServiceSpy: jasmine.SpyObj; + let authServiceSpy: jasmine.SpyObj; const customUrl = 'my-conflicting-url'; @@ -71,10 +76,11 @@ describe('CustomUrlConflictErrorComponent', () => { beforeEach(waitForAsync(() => { searchServiceSpy = jasmine.createSpyObj('SearchService', ['search']); - // Default: return empty results — overridden per suite below + authServiceSpy = jasmine.createSpyObj('AuthService', ['isAuthenticated']); searchServiceSpy.search.and.returnValue( createSuccessfulRemoteDataObject$(buildSearchObjects([])), ); + authServiceSpy.isAuthenticated.and.returnValue(of(true)); TestBed.configureTestingModule({ imports: [ @@ -88,6 +94,7 @@ describe('CustomUrlConflictErrorComponent', () => { provideRouter([]), { provide: SearchService, useValue: searchServiceSpy }, { provide: DSONameService, useValue: new DSONameServiceMock() }, + { provide: AuthService, useValue: authServiceSpy }, ], }) .overrideComponent(CustomUrlConflictErrorComponent, { @@ -102,20 +109,22 @@ describe('CustomUrlConflictErrorComponent', () => { searchServiceSpy.search.and.returnValue( createSuccessfulRemoteDataObject$(buildSearchObjects([mockItem1, mockItem2])), ); - createComponent(); }); it('should create', () => { + createComponent(); expect(component).toBeTruthy(); }); it('should call SearchService.search with a query containing the custom URL', () => { + createComponent(); expect(searchServiceSpy.search).toHaveBeenCalledOnceWith( jasmine.objectContaining({ query: `dspace.customurl:${customUrl}` }), ); }); it('should emit one entry per conflicting item', (done) => { + createComponent(); component.conflictingItems$.subscribe((items) => { expect(items.length).toBe(2); done(); @@ -123,6 +132,7 @@ describe('CustomUrlConflictErrorComponent', () => { }); it('should build the correct metadata edit link for each item', (done) => { + createComponent(); component.conflictingItems$.subscribe((items) => { expect(items[0].editLink).toBe(expectedEditLink(mockItem1)); expect(items[1].editLink).toBe(expectedEditLink(mockItem2)); @@ -131,6 +141,7 @@ describe('CustomUrlConflictErrorComponent', () => { }); it('should use DSONameService to resolve item names', (done) => { + createComponent(); component.conflictingItems$.subscribe((items) => { expect(items[0].name).toBe(mockItem1.name); expect(items[1].name).toBe(mockItem2.name); @@ -138,10 +149,42 @@ describe('CustomUrlConflictErrorComponent', () => { }); }); - it('should render one edit link per item in the template', () => { - fixture.detectChanges(); - const links = fixture.debugElement.queryAll(By.css('ul li a')); - expect(links.length).toBe(2); + describe('when user is authenticated', () => { + beforeEach(() => { + authServiceSpy.isAuthenticated.and.returnValue(of(true)); + createComponent(); + }); + + it('should render one edit link () per item', () => { + fixture.detectChanges(); + const links = fixture.debugElement.queryAll(By.css('ul li a')); + expect(links.length).toBe(2); + }); + + it('should not render plain text spans for items', () => { + fixture.detectChanges(); + const spans = fixture.debugElement.queryAll(By.css('ul li span')); + expect(spans.length).toBe(0); + }); + }); + + describe('when user is not authenticated', () => { + beforeEach(() => { + authServiceSpy.isAuthenticated.and.returnValue(of(false)); + createComponent(); + }); + + it('should not render any edit links ()', () => { + fixture.detectChanges(); + const links = fixture.debugElement.queryAll(By.css('ul li a')); + expect(links.length).toBe(0); + }); + + it('should render one plain text span per item', () => { + fixture.detectChanges(); + const spans = fixture.debugElement.queryAll(By.css('ul li span')); + expect(spans.length).toBe(2); + }); }); }); diff --git a/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.ts b/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.ts index 32bf9f92ec9..386cc809a53 100644 --- a/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.ts +++ b/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.ts @@ -5,6 +5,7 @@ import { OnInit, } from '@angular/core'; import { RouterLink } from '@angular/router'; +import { AuthService } from '@dspace/core/auth/auth.service'; import { DSONameService } from '@dspace/core/breadcrumbs/dso-name.service'; import { RemoteData } from '@dspace/core/data/remote-data'; import { DSpaceObjectType } from '@dspace/core/shared/dspace-object-type.model'; @@ -67,12 +68,17 @@ export class CustomUrlConflictErrorComponent implements OnInit { */ conflictingItems$: Observable<{ uuid: string; name: string; editLink: string }[]>; + /** Observable emitting whether the current user is authenticated. */ + isAuthenticated$: Observable; + constructor( private searchService: SearchService, private dsoNameService: DSONameService, + private authService: AuthService, ) {} ngOnInit(): void { + this.isAuthenticated$ = this.authService.isAuthenticated(); const searchOptions = new PaginatedSearchOptions({ dsoTypes: [DSpaceObjectType.ITEM], query: `dspace.customurl:${this.customUrl}`, From 4e9b2a6aa9a4bf30c7d8b9ad5b2fa8f234c4bb26 Mon Sep 17 00:00:00 2001 From: FrancescoMolinaro Date: Fri, 17 Apr 2026 13:16:11 +0200 Subject: [PATCH 3/4] [DURACOM-470] add handling for non latin chars in custom url, fix missing UI error in edit mode --- .../breadcrumbs/dso-breadcrumb.resolver.ts | 18 +++++- src/app/core/data/request-error.model.ts | 3 + src/app/core/data/request.actions.ts | 9 ++- src/app/core/data/request.effects.ts | 2 +- src/app/core/data/request.reducer.ts | 1 + .../core/dspace-rest/dspace-rest.service.ts | 1 + src/app/core/router/utils/dso-route.utils.ts | 12 +++- src/app/item-page/item-page.resolver.spec.ts | 58 +++++++++++++++++-- src/app/item-page/item-page.resolver.ts | 37 +++++++++--- src/assets/i18n/en.json5 | 6 ++ 10 files changed, 127 insertions(+), 20 deletions(-) diff --git a/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts b/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts index ad3aec4eca6..80793090212 100644 --- a/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts +++ b/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts @@ -8,9 +8,14 @@ import { map } from 'rxjs/operators'; import { IdentifiableDataService } from '../data/base/identifiable-data.service'; import { ItemDataService } from '../data/item-data.service'; -import { getDSORoute } from '../router/utils/dso-route.utils'; +import { + CUSTOM_URL_VALID_PATTERN, + getDSORoute, + getItemPageRoute, +} from '../router/utils/dso-route.utils'; import { DSpaceObject } from '../shared/dspace-object.model'; import { FollowLinkConfig } from '../shared/follow-link-config.model'; +import { Item } from '../shared/item.model'; import { getFirstCompletedRemoteData, getRemoteDataPayload, @@ -63,7 +68,16 @@ export const DSOBreadcrumbResolverByUuid: (route: ActivatedRouteSnapshot, state: getRemoteDataPayload(), map((object: DSpaceObject) => { if (hasValue(object)) { - return { provider: breadcrumbService, key: object, url: getDSORoute(object) }; + // For items, fall back to UUID-based route if the custom URL contains non-latin characters + let url: string; + if (object instanceof Item && object.hasMetadata('dspace.customurl')) { + const customUrl = object.firstMetadataValue('dspace.customurl'); + const ignoreCustomUrl = !CUSTOM_URL_VALID_PATTERN.test(customUrl); + url = getItemPageRoute(object as Item, ignoreCustomUrl); + } else { + url = getDSORoute(object); + } + return { provider: breadcrumbService, key: object, url }; } else { return undefined; } diff --git a/src/app/core/data/request-error.model.ts b/src/app/core/data/request-error.model.ts index bde0e1ea235..06fbc1d72ec 100644 --- a/src/app/core/data/request-error.model.ts +++ b/src/app/core/data/request-error.model.ts @@ -1,4 +1,7 @@ +import { PathableObjectError } from './response-state.model'; + export class RequestError extends Error { statusCode: number; statusText: string; + errors?: PathableObjectError[]; } diff --git a/src/app/core/data/request.actions.ts b/src/app/core/data/request.actions.ts index 14ae3780004..783b3d9820b 100644 --- a/src/app/core/data/request.actions.ts +++ b/src/app/core/data/request.actions.ts @@ -4,6 +4,7 @@ import { Action } from '@ngrx/store'; import { type } from '../ngrx/type'; import { HALLink } from '../shared/hal-link.model'; import { UnCacheableObject } from '../shared/uncacheable-object.model'; +import { PathableObjectError } from './response-state.model'; import { RestRequest } from './rest-request.model'; /** @@ -103,7 +104,8 @@ export class RequestErrorAction extends RequestUpdateAction { uuid: string, timeCompleted: number, statusCode: number, - errorMessage: string + errorMessage: string, + errors?: PathableObjectError[] }; /** @@ -115,14 +117,17 @@ export class RequestErrorAction extends RequestUpdateAction { * the statusCode of the response * @param errorMessage * the error message in the response + * @param errors + * the list of pathable errors */ - constructor(uuid: string, statusCode: number, errorMessage: string) { + constructor(uuid: string, statusCode: number, errorMessage: string, errors?: PathableObjectError[]) { super(); this.payload = { uuid, timeCompleted: new Date().getTime(), statusCode, errorMessage, + errors, }; } } diff --git a/src/app/core/data/request.effects.ts b/src/app/core/data/request.effects.ts index f65c2b78de9..13eddd5245e 100644 --- a/src/app/core/data/request.effects.ts +++ b/src/app/core/data/request.effects.ts @@ -68,7 +68,7 @@ export class RequestEffects { catchError((error: unknown) => { if (error instanceof RequestError) { // if it's an error returned by the server, complete the request - return [new RequestErrorAction(request.uuid, error.statusCode, error.message)]; + return [new RequestErrorAction(request.uuid, error.statusCode, error.message, error.errors)]; } else { // if it's a client side error, throw it throw error; diff --git a/src/app/core/data/request.reducer.ts b/src/app/core/data/request.reducer.ts index 55c9d323a2d..16883e85105 100644 --- a/src/app/core/data/request.reducer.ts +++ b/src/app/core/data/request.reducer.ts @@ -151,6 +151,7 @@ function completeFailedRequest(storeState: RequestState, action: RequestErrorAct statusCode: action.payload.statusCode, payloadLink: null, errorMessage: action.payload.errorMessage, + errors: action.payload.errors, }, lastUpdated: action.lastUpdated, }), diff --git a/src/app/core/dspace-rest/dspace-rest.service.ts b/src/app/core/dspace-rest/dspace-rest.service.ts index 40acd4062bc..f04ea737cf8 100644 --- a/src/app/core/dspace-rest/dspace-rest.service.ts +++ b/src/app/core/dspace-rest/dspace-rest.service.ts @@ -159,6 +159,7 @@ export class DspaceRestService { error.statusCode = err.status; error.statusText = err.statusText; + error.errors = err?.error || null; return error; } else { diff --git a/src/app/core/router/utils/dso-route.utils.ts b/src/app/core/router/utils/dso-route.utils.ts index d92b3dc2750..f24a7f143a2 100644 --- a/src/app/core/router/utils/dso-route.utils.ts +++ b/src/app/core/router/utils/dso-route.utils.ts @@ -16,6 +16,12 @@ import { getItemModuleRoute, } from '../core-routing-paths'; +/** + * Regex to validate that a custom URL contains only safe latin-compatible characters. + * If the custom URL does not match this pattern, routing falls back to the item UUID. + */ +export const CUSTOM_URL_VALID_PATTERN = /^[.a-zA-Z0-9\-_]+$/; + export function getCollectionPageRoute(collectionId: string) { return new URLCombiner(getCollectionModuleRoute(), collectionId).toString(); } @@ -26,8 +32,10 @@ export function getCommunityPageRoute(communityId: string) { /** * Get the route to an item's page - * Depending on the item's entity type, the route will either start with /items or /entities - * @param item The item to retrieve the route for + * Depending on the item's entity type, the route will either start with /items or /entities. + * + * @param item The item to retrieve the route for + * @param ignoreCustomUrl When true, always use the UUID even if a valid custom URL exists */ export function getItemPageRoute(item: Item, ignoreCustomUrl = false) { const type = item.firstMetadataValue('dspace.entity.type'); diff --git a/src/app/item-page/item-page.resolver.spec.ts b/src/app/item-page/item-page.resolver.spec.ts index 0d2132239f6..1146f73c5ae 100644 --- a/src/app/item-page/item-page.resolver.spec.ts +++ b/src/app/item-page/item-page.resolver.spec.ts @@ -8,6 +8,7 @@ import { HardRedirectService } from '@dspace/core/services/hard-redirect.service import { DSpaceObject } from '@dspace/core/shared/dspace-object.model'; import { MetadataValueFilter } from '@dspace/core/shared/metadata.models'; import { AuthServiceStub } from '@dspace/core/testing/auth-service.stub'; +import { NotificationsServiceStub } from '@dspace/core/testing/notifications-service.stub'; import { createSuccessfulRemoteDataObject$ } from '@dspace/core/utilities/remote-data.utils'; import { first } from 'rxjs/operators'; @@ -35,6 +36,8 @@ describe('itemPageResolver', () => { let authService: AuthServiceStub; let platformId: any; let hardRedirectService: any; + let notificationsService: NotificationsServiceStub; + let translateService: any; const uuid = '1234-65487-12354-1235'; let item: DSpaceObject; @@ -46,6 +49,8 @@ describe('itemPageResolver', () => { hardRedirectService = jasmine.createSpyObj('hardRedirectService', { redirect: {}, }); + notificationsService = new NotificationsServiceStub(); + translateService = { instant: (key: string) => key } as any; item = Object.assign(new DSpaceObject(), { uuid: uuid, firstMetadataValue(_keyOrKeys: string | string[], _valueFilter?: MetadataValueFilter): string { @@ -74,6 +79,8 @@ describe('itemPageResolver', () => { authService, platformId, hardRedirectService, + notificationsService, + translateService, ).pipe(first()) .subscribe( () => { @@ -83,7 +90,7 @@ describe('itemPageResolver', () => { ); }); - it('should not redirect if we’re already on the correct route', (done) => { + it('should not redirect if we\'re already on the correct route', (done) => { spyOn(item, 'firstMetadataValue').and.returnValue(entityType); spyOn(router, 'navigateByUrl').and.callThrough(); @@ -96,6 +103,8 @@ describe('itemPageResolver', () => { authService, platformId, hardRedirectService, + notificationsService, + translateService, ).pipe(first()) .subscribe( () => { @@ -129,6 +138,8 @@ describe('itemPageResolver', () => { let authService: AuthServiceStub; let platformId: any; let hardRedirectService: any; + let notificationsService: NotificationsServiceStub; + let translateService: any; const uuid = '1234-65487-12354-1235'; let item: DSpaceObject; @@ -139,6 +150,8 @@ describe('itemPageResolver', () => { hardRedirectService = jasmine.createSpyObj('hardRedirectService', { redirect: {}, }); + notificationsService = new NotificationsServiceStub(); + translateService = { instant: (key: string) => key } as any; item = Object.assign(new DSpaceObject(), { uuid: uuid, id: uuid, @@ -168,7 +181,7 @@ describe('itemPageResolver', () => { const route = { params: { id: uuid } } as any; const state = { url: `/entities/person/${uuid}` } as any; - resolver(route, state, router, itemService, store, authService, platformId, hardRedirectService) + resolver(route, state, router, itemService, store, authService, platformId, hardRedirectService, notificationsService, translateService) .pipe(first()) .subscribe((rd: any) => { const expectedUrl = `/entities/person/${customUrl}`; @@ -183,7 +196,7 @@ describe('itemPageResolver', () => { const route = { params: { id: customUrl } } as any; const state = { url: `/entities/person/${customUrl}` } as any; - resolver(route, state, router, itemService, store, authService, platformId, hardRedirectService) + resolver(route, state, router, itemService, store, authService, platformId, hardRedirectService, notificationsService, translateService) .pipe(first()) .subscribe((rd: any) => { expect(router.navigateByUrl).not.toHaveBeenCalled(); @@ -197,7 +210,7 @@ describe('itemPageResolver', () => { const route = { params: { id: customUrl } } as any; const state = { url: `/entities/person/${customUrl}/edit` } as any; - resolver(route, state, router, itemService, store, authService, platformId, hardRedirectService) + resolver(route, state, router, itemService, store, authService, platformId, hardRedirectService, notificationsService, translateService) .pipe(first()) .subscribe(() => { expect(router.navigateByUrl).toHaveBeenCalledWith(`/entities/person/${uuid}/edit`); @@ -211,13 +224,48 @@ describe('itemPageResolver', () => { const route = { params: { id: customUrl } } as any; const state = { url: `/entities/person/${customUrl}/full` } as any; - resolver(route, state, router, itemService, store, authService, platformId, hardRedirectService) + resolver(route, state, router, itemService, store, authService, platformId, hardRedirectService, notificationsService, translateService) .pipe(first()) .subscribe(() => { expect(router.navigateByUrl).not.toHaveBeenCalled();; done(); }); }); + + it('should fall back to UUID and show a warning notification when dspace.customurl contains invalid characters', (done) => { + const invalidCustomUrl = 'café-测试'; + const itemWithInvalidUrl = Object.assign(new DSpaceObject(), { + uuid: uuid, + id: uuid, + firstMetadataValue(_keyOrKeys: string | string[], _valueFilter?: MetadataValueFilter): string { + return _keyOrKeys === 'dspace.entity.type' ? 'person' : invalidCustomUrl; + }, + hasMetadata(_keyOrKeys: string | string[], _valueFilter?: MetadataValueFilter): boolean { + return true; + }, + metadata: { 'dspace.customurl': invalidCustomUrl }, + }); + const invalidItemService = { + findByIdOrCustomUrl: (_id: string) => createSuccessfulRemoteDataObject$(itemWithInvalidUrl), + }; + notificationsService = new NotificationsServiceStub(); + translateService = { instant: (key: string) => key } as any; + + spyOn(router, 'navigateByUrl').and.callThrough(); + spyOn(notificationsService, 'warning').and.callThrough(); + + const encodedInvalidUrl = encodeURIComponent(invalidCustomUrl); + const route = { params: { id: invalidCustomUrl } } as any; + const state = { url: `/entities/person/${encodedInvalidUrl}` } as any; + + resolver(route, state, router, invalidItemService, store, authService, platformId, hardRedirectService, notificationsService, translateService) + .pipe(first()) + .subscribe(() => { + expect(router.navigateByUrl).toHaveBeenCalledWith(`/entities/person/${uuid}`); + expect(notificationsService.warning).toHaveBeenCalled(); + done(); + }); + }); }); }); diff --git a/src/app/item-page/item-page.resolver.ts b/src/app/item-page/item-page.resolver.ts index 58f690ae309..84d73950b50 100644 --- a/src/app/item-page/item-page.resolver.ts +++ b/src/app/item-page/item-page.resolver.ts @@ -12,8 +12,13 @@ import { import { AuthService } from '@dspace/core/auth/auth.service'; import { ItemDataService } from '@dspace/core/data/item-data.service'; import { RemoteData } from '@dspace/core/data/remote-data'; +import { NotificationOptions } from '@dspace/core/notification-system/models/notification-options.model'; +import { NotificationsService } from '@dspace/core/notification-system/notifications.service'; import { ResolvedAction } from '@dspace/core/resolving/resolver.actions'; -import { getItemPageRoute } from '@dspace/core/router/utils/dso-route.utils'; +import { + CUSTOM_URL_VALID_PATTERN, + getItemPageRoute, +} from '@dspace/core/router/utils/dso-route.utils'; import { HardRedirectService } from '@dspace/core/services/hard-redirect.service'; import { redirectOn4xx, @@ -26,6 +31,7 @@ import { import { getFirstCompletedRemoteData } from '@dspace/core/shared/operators'; import { hasValue } from '@dspace/shared/utils/empty.util'; import { Store } from '@ngrx/store'; +import { TranslateService } from '@ngx-translate/core'; import { Observable } from 'rxjs'; import { map } from 'rxjs/operators'; @@ -51,6 +57,8 @@ export const itemPageResolver: ResolveFn> = ( authService: AuthService = inject(AuthService), platformId: any = inject(PLATFORM_ID), hardRedirectService: HardRedirectService = inject(HardRedirectService), + notificationsService: NotificationsService = inject(NotificationsService), + translateService: TranslateService = inject(TranslateService), ): Observable> => { const itemRD$ = itemService.findByIdOrCustomUrl( route.params.id, @@ -67,23 +75,36 @@ export const itemPageResolver: ResolveFn> = ( store.dispatch(new ResolvedAction(state.url, itemRD.payload)); }); + return itemRD$.pipe( map((rd: RemoteData) => { if (rd.hasSucceeded && hasValue(rd.payload)) { - let itemRoute; + let itemRoute: string; if (hasValue(rd.payload.metadata) && rd.payload.hasMetadata('dspace.customurl')) { const customUrl = rd.payload.firstMetadataValue('dspace.customurl'); - const isSubPath = !(state.url.endsWith(customUrl) || state.url.endsWith(rd.payload.id) || state.url.endsWith('/full')); - itemRoute = isSubPath ? state.url : router.parseUrl(getItemPageRoute(rd.payload)).toString(); + const isValidCustomUrl = CUSTOM_URL_VALID_PATTERN.test(customUrl); + const decodedStateUrl = decodeURIComponent(state.url); + const isSubPath = !(decodedStateUrl.endsWith(customUrl) || decodedStateUrl.endsWith(rd.payload.id) || decodedStateUrl.endsWith('/full')); + itemRoute = (isSubPath || !isValidCustomUrl) ? state.url : router.parseUrl(getItemPageRoute(rd.payload)).toString(); let newUrl: string; - if (route.params.id !== customUrl && !isSubPath) { - newUrl = itemRoute.replace(route.params.id,rd.payload.firstMetadataValue('dspace.customurl')); - } else if (isSubPath && route.params.id === customUrl) { + if (route.params.id !== customUrl && !isSubPath && isValidCustomUrl) { + newUrl = itemRoute.replace(route.params.id, rd.payload.firstMetadataValue('dspace.customurl')); + } else if ((isSubPath || !isValidCustomUrl) && route.params.id === customUrl) { // In case of a sub path, we need to ensure we navigate to the edit page of the item ID, not the custom URL const itemId = rd.payload.uuid; - newUrl = itemRoute.replace(rd.payload.firstMetadataValue('dspace.customurl'), itemId); + newUrl = decodeURIComponent(itemRoute).replace(customUrl, itemId); + if (!isValidCustomUrl && !isSubPath) { + // Notify the user that custom url won't be used as it is malformed + const notificationOptions = new NotificationOptions(-1, true); + notificationsService.warning( + translateService.instant('item-page.resolver.invalid-custom-url.title'), + translateService.instant('item-page.resolver.invalid-custom-url.message'), + notificationOptions, + ); + } } + if (hasValue(newUrl)) { router.navigateByUrl(newUrl); } diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index bb2eb12f8bb..b64b6ae2bac 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -3138,6 +3138,10 @@ "item.page.filesection.license.bundle": "License bundle", + "item-page.resolver.invalid-custom-url.title": "Invalid custom URL", + + "item-page.resolver.invalid-custom-url.message": "The custom URL of this item contains unsupported characters. The item is being displayed using its UUID.", + "item.page.return": "Back", "item.page.version.create": "Create new version", @@ -5765,6 +5769,8 @@ "submission.sections.general.deposit_error_notice": "There was an issue when submitting the item, please try again later.", + "submission.sections.general.invalid_state_error": "Cannot save current changes, mandatory fields are missing. Please resolve problems and save again later.", + "submission.sections.general.deposit_success_notice": "Submission deposited successfully.", "submission.sections.general.discard_error_notice": "There was an issue when discarding the item, please try again later.", From 7bcdf723dac9dd34e8204ec5c8fc58b95528f0cb Mon Sep 17 00:00:00 2001 From: FrancescoMolinaro Date: Fri, 17 Apr 2026 14:35:19 +0200 Subject: [PATCH 4/4] [DURACOM-470] remove duplicated spy --- src/app/item-page/item-page.resolver.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/app/item-page/item-page.resolver.spec.ts b/src/app/item-page/item-page.resolver.spec.ts index 1146f73c5ae..f9ede807dc0 100644 --- a/src/app/item-page/item-page.resolver.spec.ts +++ b/src/app/item-page/item-page.resolver.spec.ts @@ -252,7 +252,6 @@ describe('itemPageResolver', () => { translateService = { instant: (key: string) => key } as any; spyOn(router, 'navigateByUrl').and.callThrough(); - spyOn(notificationsService, 'warning').and.callThrough(); const encodedInvalidUrl = encodeURIComponent(invalidCustomUrl); const route = { params: { id: invalidCustomUrl } } as any;