[DURACOM-470] add handling for non latin chars in custom url, fix missing UI error in edit mode

Author: FrancescoMolinaro

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

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[];
 }

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,
     };
   }
 }

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;

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,
       }),

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 {

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');

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

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<RemoteData<Item>> = (
   authService: AuthService = inject(AuthService),
   platformId: any = inject(PLATFORM_ID),
   hardRedirectService: HardRedirectService = inject(HardRedirectService),
+  notificationsService: NotificationsService = inject(NotificationsService),
+  translateService: TranslateService = inject(TranslateService),
 ): Observable<RemoteData<Item>> => {
   const itemRD$ = itemService.findByIdOrCustomUrl(
     route.params.id,
@@ -67,23 +75,36 @@ export const itemPageResolver: ResolveFn<RemoteData<Item>> = (
     store.dispatch(new ResolvedAction(state.url, itemRD.payload));
   });
 
+
   return itemRD$.pipe(
     map((rd: RemoteData<Item>) => {
       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);
           }

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.",