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 fc0f8b4ffda..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,14 +32,16 @@ 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) { +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/item-page.resolver.spec.ts b/src/app/item-page/item-page.resolver.spec.ts index 0d2132239f6..f9ede807dc0 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,47 @@ 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(); + + 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/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..a79e27bd2ab --- /dev/null +++ b/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.html @@ -0,0 +1,27 @@ + +

{{ '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..fbdc0ef7317 --- /dev/null +++ b/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.spec.ts @@ -0,0 +1,227 @@ +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 { 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'; +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 { + of, + 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; + let authServiceSpy: 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']); + authServiceSpy = jasmine.createSpyObj('AuthService', ['isAuthenticated']); + searchServiceSpy.search.and.returnValue( + createSuccessfulRemoteDataObject$(buildSearchObjects([])), + ); + authServiceSpy.isAuthenticated.and.returnValue(of(true)); + + TestBed.configureTestingModule({ + imports: [ + CustomUrlConflictErrorComponent, + TranslateModule.forRoot({ + loader: { provide: TranslateLoader, useClass: TranslateLoaderMock }, + }), + ], + providers: [ + provideNoopAnimations(), + provideRouter([]), + { provide: SearchService, useValue: searchServiceSpy }, + { provide: DSONameService, useValue: new DSONameServiceMock() }, + { provide: AuthService, useValue: authServiceSpy }, + ], + }) + .overrideComponent(CustomUrlConflictErrorComponent, { + remove: { imports: [AlertComponent] }, + add: { imports: [AlertComponentStub] }, + }) + .compileComponents(); + })); + + describe('when search returns matching items', () => { + beforeEach(() => { + searchServiceSpy.search.and.returnValue( + createSuccessfulRemoteDataObject$(buildSearchObjects([mockItem1, mockItem2])), + ); + }); + + 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(); + }); + }); + + 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)); + done(); + }); + }); + + 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); + done(); + }); + }); + + 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); + }); + }); + }); + + 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..386cc809a53 --- /dev/null +++ b/src/app/item-page/simple/custom-url-conflict-error/custom-url-conflict-error.component.ts @@ -0,0 +1,105 @@ +import { AsyncPipe } from '@angular/common'; +import { + Component, + Input, + 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'; +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 }[]>; + + /** 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}`, + }); + + 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..b64b6ae2bac 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", @@ -3128,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", @@ -5755,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.", 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,