From 34a5484a9e60781fe2b1c82daab485367af90e87 Mon Sep 17 00:00:00 2001 From: Romuald Caplier Date: Fri, 27 Sep 2024 16:22:45 +0200 Subject: [PATCH] fix(me): Fixed overview and online resources upload when identical file check if file name already exist and rename if that s the case before uploading --- .../gn4/platform/gn4-platform.service.spec.ts | 22 +++-- .../lib/gn4/platform/gn4-platform.service.ts | 87 ++++++++++++------- .../src/lib/platform.service.interface.ts | 4 +- ...ld-online-link-resources.component.spec.ts | 24 ++++- ...m-field-online-link-resources.component.ts | 11 +-- .../form-field-online-resources.component.ts | 8 +- .../form-field-overviews.component.spec.ts | 16 +++- .../form-field-overviews.component.ts | 7 +- libs/util/shared/src/lib/utils/index.ts | 1 + .../lib/utils/no-duplicate-file-name.spec.ts | 45 ++++++++++ .../src/lib/utils/no-duplicate-file-name.ts | 23 +++++ 11 files changed, 192 insertions(+), 56 deletions(-) create mode 100644 libs/util/shared/src/lib/utils/no-duplicate-file-name.spec.ts create mode 100644 libs/util/shared/src/lib/utils/no-duplicate-file-name.ts diff --git a/libs/api/repository/src/lib/gn4/platform/gn4-platform.service.spec.ts b/libs/api/repository/src/lib/gn4/platform/gn4-platform.service.spec.ts index e39699206b..8b76b0452a 100644 --- a/libs/api/repository/src/lib/gn4/platform/gn4-platform.service.spec.ts +++ b/libs/api/repository/src/lib/gn4/platform/gn4-platform.service.spec.ts @@ -18,10 +18,7 @@ import { someUserFeedbacksFixture, userFeedbackFixture, } from '@geonetwork-ui/common/fixtures' -import { - HttpClientTestingModule, - HttpTestingController, -} from '@angular/common/http/testing' +import { HttpClientTestingModule } from '@angular/common/http/testing' import { HttpClient, HttpEventType } from '@angular/common/http' let geonetworkVersion: string @@ -187,7 +184,11 @@ class RecordsApiServiceMock { }, ]) ) - putResource = jest.fn(() => of(undefined)) + putResource = jest.fn(() => + of({ + type: HttpEventType.UploadProgress, + }) + ) } class UserfeedbackApiServiceMock { @@ -257,7 +258,6 @@ describe('Gn4PlatformService', () => { registriesApiService = TestBed.inject(RegistriesApiService) userFeedbackApiService = TestBed.inject(UserfeedbackApiService as any) recordsApiService = TestBed.inject(RecordsApiService) - TestBed.inject(HttpTestingController) }) it('creates', () => { @@ -775,7 +775,8 @@ describe('Gn4PlatformService', () => { file = new File([''], 'filename') }) it('calls api service', async () => { - service.attachFileToRecord('12345', file) + await firstValueFrom(service.attachFileToRecord('12345', file)) + expect(recordsApiService.getAllResources).toHaveBeenCalledWith('12345') expect(recordsApiService.putResource).toHaveBeenCalledWith( '12345', file, @@ -785,6 +786,13 @@ describe('Gn4PlatformService', () => { true ) }) + it('disambiguates file name if an identical file already exists', async () => { + file = new File([''], 'doge.jpg') + await firstValueFrom(service.attachFileToRecord('12345', file)) + const fileSent = (recordsApiService.putResource as jest.Mock).mock + .calls[0][1] + expect(fileSent.name).not.toEqual('doge.jpg') + }) it('handles progress event', () => { ;(recordsApiService.putResource as jest.Mock).mockReturnValue( of({ diff --git a/libs/api/repository/src/lib/gn4/platform/gn4-platform.service.ts b/libs/api/repository/src/lib/gn4/platform/gn4-platform.service.ts index 09de034fd9..24f3ef0085 100644 --- a/libs/api/repository/src/lib/gn4/platform/gn4-platform.service.ts +++ b/libs/api/repository/src/lib/gn4/platform/gn4-platform.service.ts @@ -1,5 +1,5 @@ import { Injectable } from '@angular/core' -import { combineLatest, Observable, of, switchMap } from 'rxjs' +import { combineLatest, Observable, of, switchMap, throwError } from 'rxjs' import { catchError, filter, map, shareReplay, tap } from 'rxjs/operators' import { MeApiService, @@ -10,7 +10,10 @@ import { UserfeedbackApiService, UsersApiService, } from '@geonetwork-ui/data-access/gn4' -import { PlatformServiceInterface } from '@geonetwork-ui/common/domain/platform.service.interface' +import { + PlatformServiceInterface, + UploadEvent, +} from '@geonetwork-ui/common/domain/platform.service.interface' import { UserModel } from '@geonetwork-ui/common/domain/model/user/user.model' import { Keyword, @@ -26,6 +29,7 @@ import { ThesaurusApiResponse, } from '@geonetwork-ui/api/metadata-converter' import { KeywordType } from '@geonetwork-ui/common/domain/model/thesaurus' +import { noDuplicateFileName } from '@geonetwork-ui/util/shared' const minApiVersion = '4.2.2' @@ -288,34 +292,57 @@ export class Gn4PlatformService implements PlatformServiceInterface { ) } - attachFileToRecord(recordUuid: string, file: File) { + attachFileToRecord(recordUuid: string, file: File): Observable { let sizeBytes = -1 - return this.recordsApiService - .putResource(recordUuid, file, 'public', undefined, 'events', true) - .pipe( - map((event) => { - if (event.type === HttpEventType.UploadProgress) { - sizeBytes = event.total - return { - type: 'progress', - progress: event.total - ? Math.round((100 * event.loaded) / event.total) - : 0, - } as const - } - if (event.type === HttpEventType.Response) { - return { - type: 'success', - attachment: { - url: new URL(event.body.url), - fileName: event.body.filename, - }, - sizeBytes, - } as const - } - return undefined - }), - filter((event) => !!event) - ) + + // Check if the resource already exists on the server and rename it if that's the case + return this.getRecordAttachments(recordUuid).pipe( + map((recordAttachement) => recordAttachement.map((r) => r.fileName)), + switchMap((fileNames) => { + const fileName = noDuplicateFileName(file.name, fileNames) + + const fileCopy = new File([file], fileName, { type: file.type }) + + return this.recordsApiService + .putResource( + recordUuid, + fileCopy, + 'public', + undefined, + 'events', + true + ) + .pipe( + map((event) => { + if (event.type === HttpEventType.UploadProgress) { + sizeBytes = event.total + return { + type: 'progress', + progress: event.total + ? Math.round((100 * event.loaded) / event.total) + : 0, + } as UploadEvent + } + if (event.type === HttpEventType.Response) { + return { + type: 'success', + attachment: { + url: new URL(event.body.url), + fileName: event.body.filename, + }, + sizeBytes, + } as UploadEvent + } + return undefined + }), + filter((event) => !!event) + ) + }), + catchError((error) => { + return throwError( + () => new Error(error.error?.message ?? error.message) + ) + }) + ) } } diff --git a/libs/common/domain/src/lib/platform.service.interface.ts b/libs/common/domain/src/lib/platform.service.interface.ts index 2f13a6d7a1..008c0f24bc 100644 --- a/libs/common/domain/src/lib/platform.service.interface.ts +++ b/libs/common/domain/src/lib/platform.service.interface.ts @@ -4,11 +4,11 @@ import type { Organization } from './model/record/organization.model' import { Keyword, UserFeedback } from './model/record' import { KeywordType } from './model/thesaurus' -interface RecordAttachment { +export interface RecordAttachment { url: URL fileName: string } -type UploadEvent = +export type UploadEvent = | { type: 'progress' progress: number // in percent diff --git a/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-link-resources/form-field-online-link-resources.component.spec.ts b/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-link-resources/form-field-online-link-resources.component.spec.ts index 76fd1b4afb..ce65f266c4 100644 --- a/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-link-resources/form-field-online-link-resources.component.spec.ts +++ b/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-link-resources/form-field-online-link-resources.component.spec.ts @@ -3,19 +3,32 @@ import { FormFieldOnlineLinkResourcesComponent } from './form-field-online-link- import { aSetOfLinksFixture } from '@geonetwork-ui/common/fixtures' import { MockBuilder, MockProvider } from 'ng-mocks' import { TranslateModule } from '@ngx-translate/core' -import { PlatformServiceInterface } from '@geonetwork-ui/common/domain/platform.service.interface' +import { + PlatformServiceInterface, + RecordAttachment, +} from '@geonetwork-ui/common/domain/platform.service.interface' import { NotificationsService } from '@geonetwork-ui/feature/notifications' -import { Subject } from 'rxjs' +import { BehaviorSubject, Subject } from 'rxjs' import { MatDialog, MatDialogRef } from '@angular/material/dialog' import { OnlineLinkResource } from '@geonetwork-ui/common/domain/model/record' import { ModalDialogComponent } from '@geonetwork-ui/ui/layout' +import { ChangeDetectorRef } from '@angular/core' let uploadSubject: Subject + +const recordAttachments = new BehaviorSubject([ + { + url: new URL('https://www.fakedomain.com/test.txt'), + fileName: 'test.txt', + }, +]) + class PlatformServiceInterfaceMock { attachFileToRecord = jest.fn(() => { uploadSubject = new Subject() return uploadSubject }) + getRecordAttachments = jest.fn(() => recordAttachments) } export class MatDialogMock { _subject = new Subject() @@ -44,8 +57,13 @@ describe('FormFieldOnlineLinkResourcesComponent', () => { PlatformServiceInterfaceMock, 'useClass' ), - MockProvider(NotificationsService), + MockProvider(NotificationsService, { + showNotification: jest.fn(), + }), MockProvider(MatDialogRef), + MockProvider(ChangeDetectorRef, { + detectChanges: jest.fn(), + }), MockProvider(MatDialog, MatDialogMock, 'useClass'), ], }).compileComponents() diff --git a/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-link-resources/form-field-online-link-resources.component.ts b/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-link-resources/form-field-online-link-resources.component.ts index a274dbc852..3000324226 100644 --- a/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-link-resources/form-field-online-link-resources.component.ts +++ b/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-link-resources/form-field-online-link-resources.component.ts @@ -63,7 +63,7 @@ export class FormFieldOnlineLinkResourcesComponent { private allResources: OnlineResource[] = [] linkResources: OnlineLinkResource[] = [] - uploadProgress = undefined + uploadProgress?: number = undefined uploadSubscription: Subscription = null protected MAX_UPLOAD_SIZE_MB = MAX_UPLOAD_SIZE_MB @@ -96,7 +96,7 @@ export class FormFieldOnlineLinkResourcesComponent { this.valueChange.emit([...this.allResources, newResource]) } }, - error: (error: Error) => this.handleError(error.message), + error: (error: Error) => this.handleError(error), }) } @@ -117,7 +117,7 @@ export class FormFieldOnlineLinkResourcesComponent { } this.valueChange.emit([...this.allResources, newLink]) } catch (e) { - this.handleError((e as Error).message) + this.handleError(e as Error) } } @@ -134,8 +134,9 @@ export class FormFieldOnlineLinkResourcesComponent { this.openEditDialog(resource, index) } - private handleError(error: string) { + private handleError(error: Error) { this.uploadProgress = undefined + this.cd.detectChanges() this.notificationsService.showNotification({ type: 'error', title: this.translateService.instant( @@ -143,7 +144,7 @@ export class FormFieldOnlineLinkResourcesComponent { ), text: `${this.translateService.instant( 'editor.record.onlineResourceError.body' - )} ${error}`, + )} ${error.message}`, closeMessage: this.translateService.instant( 'editor.record.onlineResourceError.closeMessage' ), diff --git a/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-resources/form-field-online-resources.component.ts b/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-resources/form-field-online-resources.component.ts index afec5556fb..6079095018 100644 --- a/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-resources/form-field-online-resources.component.ts +++ b/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-resources/form-field-online-resources.component.ts @@ -133,7 +133,7 @@ export class FormFieldOnlineResourcesComponent { this.valueChange.emit([...this.allResources, newResource]) } }, - error: (error: Error) => this.handleError(error.message), + error: (error: Error) => this.handleError(error), }) } @@ -154,7 +154,7 @@ export class FormFieldOnlineResourcesComponent { } this.valueChange.emit([...this.allResources, newLink]) } catch (e) { - this.handleError((e as Error).message) + this.handleError(e as Error) } } @@ -190,7 +190,7 @@ export class FormFieldOnlineResourcesComponent { this.openEditDialog(resource, index) } - private handleError(error: string) { + private handleError(error: Error) { this.uploadProgress = undefined this.notificationsService.showNotification({ type: 'error', @@ -199,7 +199,7 @@ export class FormFieldOnlineResourcesComponent { ), text: `${this.translateService.instant( 'editor.record.onlineResourceError.body' - )} ${error}`, + )} ${error.message}`, closeMessage: this.translateService.instant( 'editor.record.onlineResourceError.closeMessage' ), diff --git a/libs/feature/editor/src/lib/components/record-form/form-field/form-field-overviews/form-field-overviews.component.spec.ts b/libs/feature/editor/src/lib/components/record-form/form-field/form-field-overviews/form-field-overviews.component.spec.ts index d6b43d368f..a18dab7012 100644 --- a/libs/feature/editor/src/lib/components/record-form/form-field/form-field-overviews/form-field-overviews.component.spec.ts +++ b/libs/feature/editor/src/lib/components/record-form/form-field/form-field-overviews/form-field-overviews.component.spec.ts @@ -1,17 +1,29 @@ import { ComponentFixture, TestBed } from '@angular/core/testing' import { TranslateModule } from '@ngx-translate/core' import { FormFieldOverviewsComponent } from './form-field-overviews.component' -import { Subject } from 'rxjs' +import { BehaviorSubject, Subject } from 'rxjs' import { NotificationsService } from '@geonetwork-ui/feature/notifications' import { MockBuilder, MockProvider } from 'ng-mocks' -import { PlatformServiceInterface } from '@geonetwork-ui/common/domain/platform.service.interface' +import { + PlatformServiceInterface, + RecordAttachment, +} from '@geonetwork-ui/common/domain/platform.service.interface' let uploadSubject: Subject + +const recordAttachments = new BehaviorSubject([ + { + url: new URL('https://www.fakedomain.com/test.txt'), + fileName: 'test.txt', + }, +]) + class PlatformServiceInterfaceMock { attachFileToRecord = jest.fn(() => { uploadSubject = new Subject() return uploadSubject }) + getRecordAttachments = jest.fn(() => recordAttachments) } describe('FormFieldOverviewsComponent', () => { diff --git a/libs/feature/editor/src/lib/components/record-form/form-field/form-field-overviews/form-field-overviews.component.ts b/libs/feature/editor/src/lib/components/record-form/form-field/form-field-overviews/form-field-overviews.component.ts index 8110002b21..ae56250a87 100644 --- a/libs/feature/editor/src/lib/components/record-form/form-field/form-field-overviews/form-field-overviews.component.ts +++ b/libs/feature/editor/src/lib/components/record-form/form-field/form-field-overviews/form-field-overviews.component.ts @@ -68,7 +68,7 @@ export class FormFieldOverviewsComponent { }) } }, - error: this.errorHandle, + error: (error: Error) => this.handleError(error), }) } @@ -87,7 +87,7 @@ export class FormFieldOverviewsComponent { description: filename, }) } catch (e) { - this.errorHandle(e) + this.handleError(e as Error) } } @@ -106,8 +106,9 @@ export class FormFieldOverviewsComponent { this.valueChange.emit(overView ? [overView] : []) } - private errorHandle = (error) => { + private handleError = (error: Error) => { this.uploadProgress = undefined + this.cd.markForCheck() this.notificationsService.showNotification({ type: 'error', title: this.translateService.instant('editor.record.resourceError.title'), diff --git a/libs/util/shared/src/lib/utils/index.ts b/libs/util/shared/src/lib/utils/index.ts index d3bfe45bdb..301dce35e5 100644 --- a/libs/util/shared/src/lib/utils/index.ts +++ b/libs/util/shared/src/lib/utils/index.ts @@ -3,6 +3,7 @@ export * from './event' export * from './fuzzy-filter' export * from './geojson' export * from './image-resize' +export * from './no-duplicate-file-name' export * from './parse' export * from './remove-whitespace' export * from './sort-by' diff --git a/libs/util/shared/src/lib/utils/no-duplicate-file-name.spec.ts b/libs/util/shared/src/lib/utils/no-duplicate-file-name.spec.ts new file mode 100644 index 0000000000..c173ff72e7 --- /dev/null +++ b/libs/util/shared/src/lib/utils/no-duplicate-file-name.spec.ts @@ -0,0 +1,45 @@ +import { noDuplicateFileName } from './no-duplicate-file-name' + +describe('noDuplicateFileName', () => { + it('should return the original file name if it does not exist in the fileNameList', () => { + const fileName = 'testfile.txt' + const fileNameList = ['otherfile.txt', 'anotherfile.txt'] + + const result = noDuplicateFileName(fileName, fileNameList) + + expect(result).toBe(fileName) + }) + + it('should return a new file name with a timestamp if the file name exists in the fileNameList', () => { + const fileName = 'testfile.txt' + const fileNameList = ['testfile.txt', 'otherfile.txt'] + + const result = noDuplicateFileName(fileName, fileNameList) + + // Check if the new file name starts with the base name and contains a timestamp + const regex = /testfile_\d+\.txt/ + expect(result).toMatch(regex) + }) + + it('should handle file names without an extension', () => { + const fileName = 'testfile' + const fileNameList = ['testfile'] + + const result = noDuplicateFileName(fileName, fileNameList) + + // Check if the new file name has a timestamp appended with no extension + const regex = /testfile_\d+/ + expect(result).toMatch(regex) + }) + + it('should handle file names with multiple dots correctly', () => { + const fileName = 'test.file.name.txt' + const fileNameList = ['test.file.name.txt'] + + const result = noDuplicateFileName(fileName, fileNameList) + + // Check if the new file name with multiple dots contains a timestamp + const regex = /test\.file\.name_\d+\.txt/ + expect(result).toMatch(regex) + }) +}) diff --git a/libs/util/shared/src/lib/utils/no-duplicate-file-name.ts b/libs/util/shared/src/lib/utils/no-duplicate-file-name.ts new file mode 100644 index 0000000000..a026156779 --- /dev/null +++ b/libs/util/shared/src/lib/utils/no-duplicate-file-name.ts @@ -0,0 +1,23 @@ +export function noDuplicateFileName( + fileName: string, + fileNameList: string[] +): string { + if (fileNameList.includes(fileName)) { + const fileNameParts = fileName.split('.') + let extension = '' + let baseName = fileName + + if (fileNameParts.length > 1) { + extension = fileNameParts.pop() as string + baseName = fileNameParts.join('.') + } + + if (extension) { + fileName = `${baseName}_${Date.now()}.${extension}` + } else { + fileName = `${baseName}_${Date.now()}` + } + } + + return fileName +}