diff --git a/src/app/core/entity-components/entity-form/entity-form/entity-form.component.spec.ts b/src/app/core/entity-components/entity-form/entity-form/entity-form.component.spec.ts index f057bdb18c..5bdd579aa2 100644 --- a/src/app/core/entity-components/entity-form/entity-form/entity-form.component.spec.ts +++ b/src/app/core/entity-components/entity-form/entity-form/entity-form.component.spec.ts @@ -166,4 +166,31 @@ describe("EntityFormComponent", () => { expect(component.entity.name).toBe("Changed Name"); expect(component.form.get("name")).toHaveValue("Changed Name"); }); + + it("should align form with entity if canceled and notify about click", () => { + const child = new Child(); + child.name = "test child"; + component.entity = child; + component.form.enable(); + component.form.get("name").setValue("other name"); + let cancelEmitted = false; + component.cancel.subscribe(() => (cancelEmitted = true)); + + component.cancelClicked(); + + expect(component.form.disabled).toBeTrue(); + expect(component.form.get("name")).toHaveValue("test child"); + expect(cancelEmitted).toBeTrue(); + }); + + it("should also reset form values which where not set before", () => { + component.entity = new Child(); + component.ngOnInit(); + component.form.enable(); + + component.form.get("name").setValue("my name"); + component.cancelClicked(); + + expect(component.form.get("name")).toHaveValue(null); + }); }); diff --git a/src/app/core/entity-components/entity-form/entity-form/entity-form.component.ts b/src/app/core/entity-components/entity-form/entity-form/entity-form.component.ts index 1e2a0cc354..3a5fe39330 100644 --- a/src/app/core/entity-components/entity-form/entity-form/entity-form.component.ts +++ b/src/app/core/entity-components/entity-form/entity-form/entity-form.component.ts @@ -76,6 +76,7 @@ export class EntityFormComponent implements OnInit { form: EntityForm; private saveInProgress = false; + private initialFormValues: any; constructor( private entityFormService: EntityFormService, @@ -86,8 +87,8 @@ export class EntityFormComponent implements OnInit { ngOnInit() { this.buildFormConfig(); - if (this.editing) { - this.form.enable(); + if (!this.editing) { + this.form.disable(); } this.entityMapper .receiveUpdates(this.entity.getConstructor()) @@ -96,7 +97,7 @@ export class EntityFormComponent implements OnInit { } private async applyChanges(entity) { - if (this.saveInProgress) { + if (this.saveInProgress || this.formIsUpToDate(entity)) { // this is the component that currently saves the values -> no need to apply changes. return; } @@ -107,7 +108,7 @@ export class EntityFormComponent implements OnInit { $localize`Local changes are in conflict with updated values synced from the server. Do you want the local changes to be overwritten with the latest values?` )) ) { - this.buildFormConfig(entity); + this.resetForm(entity); } } @@ -129,21 +130,35 @@ export class EntityFormComponent implements OnInit { cancelClicked() { this.cancel.emit(); - this.buildFormConfig(); + this.resetForm(); + this.form.disable(); } - private buildFormConfig(entity = this.entity) { + private buildFormConfig() { const flattenedFormFields = new Array().concat( ...this._columns ); this.entityFormService.extendFormFieldConfig( flattenedFormFields, - entity.getConstructor() + this.entity.getConstructor() ); this.form = this.entityFormService.createFormGroup( flattenedFormFields, - entity + this.entity + ); + this.initialFormValues = this.form.getRawValue(); + } + + private resetForm(entity = this.entity) { + // Patch form with values from the entity + this.form.patchValue(Object.assign(this.initialFormValues, entity)); + this.form.markAsPristine(); + } + + private formIsUpToDate(entity: T): boolean { + return Object.entries(this.form.getRawValue()).every( + ([key, value]) => + entity[key] === value || (entity[key] === undefined && value === null) ); - this.form.disable(); } } diff --git a/src/app/features/file/couchdb-file.service.spec.ts b/src/app/features/file/couchdb-file.service.spec.ts index c59275d7d6..9bd55eaed6 100644 --- a/src/app/features/file/couchdb-file.service.spec.ts +++ b/src/app/features/file/couchdb-file.service.spec.ts @@ -9,7 +9,14 @@ import { HttpStatusCode, } from "@angular/common/http"; import { MatDialog } from "@angular/material/dialog"; -import { EMPTY, of, Subject, throwError } from "rxjs"; +import { + BehaviorSubject, + EMPTY, + firstValueFrom, + of, + Subject, + throwError, +} from "rxjs"; import { ShowFileComponent } from "./show-file/show-file.component"; import { Entity } from "../../core/entity/model/entity"; import { EntityMapperService } from "../../core/entity/entity-mapper.service"; @@ -26,7 +33,6 @@ describe("CouchdbFileService", () => { let service: CouchdbFileService; let mockHttp: jasmine.SpyObj; let mockDialog: jasmine.SpyObj; - let mockEntityMapper: jasmine.SpyObj; let mockSnackbar: jasmine.SpyObj; let dismiss: jasmine.Spy; let updates: Subject>; @@ -36,8 +42,6 @@ describe("CouchdbFileService", () => { mockHttp = jasmine.createSpyObj(["get", "put", "delete"]); mockDialog = jasmine.createSpyObj(["open"]); updates = new Subject(); - mockEntityMapper = jasmine.createSpyObj(["save", "receiveUpdates"]); - mockEntityMapper.receiveUpdates.and.returnValue(updates); mockSnackbar = jasmine.createSpyObj(["openFromComponent"]); dismiss = jasmine.createSpy(); mockSnackbar.openFromComponent.and.returnValue({ dismiss } as any); @@ -49,7 +53,10 @@ describe("CouchdbFileService", () => { { provide: HttpClient, useValue: mockHttp }, { provide: MatSnackBar, useValue: mockSnackbar }, { provide: MatDialog, useValue: mockDialog }, - { provide: EntityMapperService, useValue: mockEntityMapper }, + { + provide: EntityMapperService, + useValue: { receiveUpdates: () => updates }, + }, { provide: EntityRegistry, useValue: entityRegistry }, ], }); @@ -60,7 +67,7 @@ describe("CouchdbFileService", () => { expect(service).toBeTruthy(); }); - it("should add a attachment to a existing document and update the entity", () => { + it("should add a attachment to a existing document", () => { mockHttp.get.and.returnValue(of({ _rev: "test_rev" })); mockHttp.put.and.returnValue(of({ ok: true })); const file = { type: "image/png", name: "file.name" } as File; @@ -78,8 +85,6 @@ describe("CouchdbFileService", () => { jasmine.anything(), jasmine.anything() ); - expect(entity["testProp"]).toBe("file.name"); - expect(mockEntityMapper.save).toHaveBeenCalledWith(entity); }); it("should create attachment document if it does not exist yet", (done) => { @@ -102,8 +107,6 @@ describe("CouchdbFileService", () => { jasmine.anything(), jasmine.anything() ); - expect(entity["testProp"]).toBe("file.name"); - expect(mockEntityMapper.save).toHaveBeenCalledWith(entity); done(); }); }); @@ -124,12 +127,10 @@ describe("CouchdbFileService", () => { }); }); - it("should remove a file using the latest rev and update the entity", () => { + it("should remove a file using the latest rev", () => { mockHttp.get.and.returnValue(of({ _rev: "test_rev" })); mockHttp.delete.and.returnValue(of({ ok: true })); const entity = new Entity("testId"); - const prop = "testProp"; - entity[prop] = "test.file"; service.removeFile(entity, "testProp").subscribe(); @@ -141,8 +142,6 @@ describe("CouchdbFileService", () => { `${attachmentUrlPrefix}/Entity:testId/testProp?rev=test_rev` ) ); - expect(entity[prop]).toBe(undefined); - expect(mockEntityMapper.save).toHaveBeenCalledWith(entity); }); it("should show progress while downloading a file", () => { @@ -196,19 +195,84 @@ describe("CouchdbFileService", () => { })); it("should not fail if to-be-removed file reference could not be found", () => { - const entity = new Entity(); - entity["testProp"] = "some.file"; mockHttp.get.and.returnValue( throwError( () => new HttpErrorResponse({ status: HttpStatusCode.NotFound }) ) ); - service.removeFile(entity, "testProp").subscribe({ - error: () => fail("Removing file should not fail"), - }); + return expectAsync( + firstValueFrom(service.removeFile(new Entity(), "testProp")) + ).toBeResolved(); + }); + + it("should wait for previous request to finish before starting a new one", () => { + const firstPut = new BehaviorSubject({ ok: true }); + const secondPut = new BehaviorSubject({ ok: true }); + const thirdPut = new BehaviorSubject({ ok: true }); + const file1 = { type: "image/png", name: "file1.name" } as File; + const file2 = { type: "image/png", name: "file2.name" } as File; + const file3 = { type: "image/png", name: "file3.name" } as File; + const entity = new Entity("testId"); + mockHttp.get.and.returnValues( + of({ _rev: "1-rev" }), + of({ _rev: "2-rev" }), + of({ _rev: "3-rev" }) + ); + mockHttp.put.and.returnValues(firstPut, secondPut, thirdPut); + + let file1Done = false; + let file2Done = false; + let file3Done = false; + service + .uploadFile(file1, entity, "prop1") + .subscribe({ complete: () => (file1Done = true) }); + service + .uploadFile(file2, entity, "prop2") + .subscribe({ complete: () => (file2Done = true) }); + service + .uploadFile(file3, entity, "prop3") + .subscribe({ complete: () => (file3Done = true) }); + + expect(firstPut.observed).toBeTrue(); + expect(secondPut.observed).toBeFalse(); + expect(mockHttp.put).toHaveBeenCalledTimes(1); + expect(mockHttp.put).toHaveBeenCalledWith( + jasmine.stringContaining( + `${attachmentUrlPrefix}/Entity:testId/prop1?rev=1-rev` + ), + jasmine.anything(), + jasmine.anything() + ); - expect(entity["testProp"]).toBeUndefined(); - expect(mockEntityMapper.save).toHaveBeenCalledWith(entity); + firstPut.complete(); + + expect(file1Done).toBeTrue(); + expect(file2Done).toBeFalse(); + expect(file3Done).toBeFalse(); + expect(secondPut.observed).toBeTrue(); + expect(thirdPut.observed).toBeFalse(); + expect(mockHttp.put).toHaveBeenCalledTimes(2); + expect(mockHttp.put).toHaveBeenCalledWith( + jasmine.stringContaining( + `${attachmentUrlPrefix}/Entity:testId/prop2?rev=2-rev` + ), + jasmine.anything(), + jasmine.anything() + ); + + secondPut.complete(); + + expect(file2Done).toBeTrue(); + expect(file3Done).toBeFalse(); + expect(thirdPut.observed).toBeTrue(); + expect(mockHttp.put).toHaveBeenCalledTimes(3); + expect(mockHttp.put).toHaveBeenCalledWith( + jasmine.stringContaining( + `${attachmentUrlPrefix}/Entity:testId/prop3?rev=3-rev` + ), + jasmine.anything(), + jasmine.anything() + ); }); }); diff --git a/src/app/features/file/couchdb-file.service.ts b/src/app/features/file/couchdb-file.service.ts index 98afe4babc..73a408c14e 100644 --- a/src/app/features/file/couchdb-file.service.ts +++ b/src/app/features/file/couchdb-file.service.ts @@ -12,7 +12,6 @@ import { catchError, concatMap, filter, - finalize, map, shareReplay, } from "rxjs/operators"; @@ -26,6 +25,7 @@ import { MatSnackBar } from "@angular/material/snack-bar"; import { ProgressComponent } from "./progress/progress.component"; import { EntityRegistry } from "../../core/entity/database-entity.decorator"; import { LoggingService } from "../../core/logging/logging.service"; +import { ObservableQueue } from "./observable-queue/observable-queue"; /** * Stores the files in the CouchDB. @@ -35,6 +35,7 @@ import { LoggingService } from "../../core/logging/logging.service"; @Injectable() export class CouchdbFileService extends FileService { private attachmentsUrl = `${AppSettings.DB_PROXY_PREFIX}/${AppSettings.DB_NAME}-attachments`; + private requestQueue = new ObservableQueue(); constructor( private http: HttpClient, @@ -48,9 +49,17 @@ export class CouchdbFileService extends FileService { } uploadFile(file: File, entity: Entity, property: string): Observable { + const obs = this.requestQueue.add( + this.runFileUpload(file, entity, property) + ); + this.reportProgress($localize`Uploading "${file.name}"`, obs); + return obs; + } + + private runFileUpload(file: File, entity: Entity, property: string) { const blob = new Blob([file]); const attachmentPath = `${this.attachmentsUrl}/${entity.getId(true)}`; - const obs = this.getAttachmentsDocument(attachmentPath).pipe( + return this.getAttachmentsDocument(attachmentPath).pipe( concatMap(({ _rev }) => this.http.put(`${attachmentPath}/${property}?rev=${_rev}`, blob, { headers: { "Content-Type": file.type, "ngsw-bypass": "" }, @@ -58,15 +67,9 @@ export class CouchdbFileService extends FileService { observe: "events", }) ), - finalize(() => { - entity[property] = file.name; - this.entityMapper.save(entity); - }), // prevent http request to be executed multiple times (whenever .subscribe is called) shareReplay() ); - this.reportProgress($localize`Uploading "${file.name}"`, obs); - return obs; } private getAttachmentsDocument( @@ -85,6 +88,10 @@ export class CouchdbFileService extends FileService { } removeFile(entity: Entity, property: string) { + return this.requestQueue.add(this.runFileRemoval(entity, property)); + } + + private runFileRemoval(entity: Entity, property: string) { const attachmentPath = `${this.attachmentsUrl}/${entity.getId(true)}`; return this.http.get<{ _rev: string }>(attachmentPath).pipe( concatMap(({ _rev }) => @@ -96,15 +103,15 @@ export class CouchdbFileService extends FileService { } else { throw err; } - }), - finalize(() => { - entity[property] = undefined; - this.entityMapper.save(entity); }) ); } removeAllFiles(entity: Entity): Observable { + return this.requestQueue.add(this.runAllFilesRemoval(entity)); + } + + private runAllFilesRemoval(entity: Entity) { const attachmentPath = `${this.attachmentsUrl}/${entity.getId(true)}`; return this.http .get<{ _rev: string }>(attachmentPath) diff --git a/src/app/features/file/edit-file/edit-file.component.spec.ts b/src/app/features/file/edit-file/edit-file.component.spec.ts index d96da7ec4e..d21cb12bfa 100644 --- a/src/app/features/file/edit-file/edit-file.component.spec.ts +++ b/src/app/features/file/edit-file/edit-file.component.spec.ts @@ -1,28 +1,24 @@ -import { - ComponentFixture, - fakeAsync, - TestBed, - tick, -} from "@angular/core/testing"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; import { EditFileComponent } from "./edit-file.component"; import { FileModule } from "../file.module"; import { AlertService } from "../../../core/alerts/alert.service"; -import { ConfirmationDialogService } from "../../../core/confirmation-dialog/confirmation-dialog.service"; import { FormControl } from "@angular/forms"; import { FontAwesomeTestingModule } from "@fortawesome/angular-fontawesome/testing"; -import { of, throwError } from "rxjs"; +import { of, Subject } from "rxjs"; import { Entity } from "app/core/entity/model/entity"; import { NoopAnimationsModule } from "@angular/platform-browser/animations"; import { FileService } from "../file.service"; import { EntitySchemaService } from "../../../core/entity/schema/entity-schema.service"; +import { EntityMapperService } from "../../../core/entity/entity-mapper.service"; describe("EditFileComponent", () => { let component: EditFileComponent; let fixture: ComponentFixture; let mockFileService: jasmine.SpyObj; - let mockConfirmationDialog: jasmine.SpyObj; let mockAlertService: jasmine.SpyObj; + let mockEntityMapper: jasmine.SpyObj; + const file = { name: "test.file" } as File; const fileEvent = { target: { files: [file] } }; @@ -32,110 +28,252 @@ describe("EditFileComponent", () => { "showFile", "removeFile", ]); - mockConfirmationDialog = jasmine.createSpyObj(["getConfirmation"]); mockAlertService = jasmine.createSpyObj(["addDanger", "addInfo"]); + mockEntityMapper = jasmine.createSpyObj(["save"]); await TestBed.configureTestingModule({ imports: [FileModule, FontAwesomeTestingModule, NoopAnimationsModule], providers: [ EntitySchemaService, { provide: AlertService, useValue: mockAlertService }, { provide: FileService, useValue: mockFileService }, - { - provide: ConfirmationDialogService, - useValue: mockConfirmationDialog, - }, + { provide: EntityMapperService, useValue: mockEntityMapper }, ], }).compileComponents(); fixture = TestBed.createComponent(EditFileComponent); component = fixture.componentInstance; - component.formControl = new FormControl(""); - component.entity = new Entity(); - component.formControlName = "testProp"; - fixture.detectChanges(); }); it("should create", () => { expect(component).toBeTruthy(); }); - it("update the form after successfully uploading the file", () => { + it("should not upload a file that was selected but the process was canceled", () => { + setupComponent(); + component.formControl.enable(); + + component.onFileSelected(fileEvent); + expect(component.formControl).toHaveValue(file.name); + cancelForm(); + + expect(component.formControl).toHaveValue(null); + expect(mockFileService.uploadFile).not.toHaveBeenCalled(); + }); + + it("should not remove or upload a file if a file was selected and then removed and then canceled", () => { + setupComponent(); + component.formControl.enable(); + + component.onFileSelected(fileEvent); + expect(component.formControl).toHaveValue(file.name); + + component.delete(); + + cancelForm(); + + expect(component.formControl).toHaveValue(null); + expect(mockFileService.uploadFile).not.toHaveBeenCalled(); + expect(mockFileService.removeFile).not.toHaveBeenCalled(); + }); + + it("should not remove or upload a file if a file was selected, replaced and canceled", () => { + setupComponent(); + component.formControl.enable(); + + component.onFileSelected(fileEvent); + expect(component.formControl).toHaveValue(file.name); + + const otherFileEvent = { target: { files: [{ name: "other.file" }] } }; + component.onFileSelected(otherFileEvent); + expect(component.formControl).toHaveValue("other.file"); + + cancelForm(); + + expect(component.formControl).toHaveValue(null); + expect(mockFileService.uploadFile).not.toHaveBeenCalled(); + expect(mockFileService.removeFile).not.toHaveBeenCalled(); + }); + + it("should upload a file if a new file was selected and the form saved", () => { + setupComponent(); mockFileService.uploadFile.and.returnValue(of({ ok: true })); + component.formControl.enable(); component.onFileSelected(fileEvent); + expect(component.formControl).toHaveValue(file.name); + + component.formControl.disable(); + expect(component.formControl).toHaveValue(file.name); expect(mockFileService.uploadFile).toHaveBeenCalledWith( file, component.entity, component.formControlName ); - expect(component.formControl).toHaveValue(file.name); }); - it("should not replace existing file if user declines", fakeAsync(() => { - mockFileService.uploadFile.and.returnValue(of({ ok: true })); - mockConfirmationDialog.getConfirmation.and.resolveTo(false); - component.formControl.setValue("existing.file"); + it("should not remove or upload a file if a file was selected, then removed and then saved", () => { + setupComponent(); + component.formControl.enable(); component.onFileSelected(fileEvent); - tick(); + expect(component.formControl).toHaveValue(file.name); + + component.delete(); - expect(mockConfirmationDialog.getConfirmation).toHaveBeenCalled(); + component.formControl.disable(); + + expect(component.formControl).toHaveValue(null); expect(mockFileService.uploadFile).not.toHaveBeenCalled(); - })); + expect(mockFileService.removeFile).not.toHaveBeenCalled(); + }); - it("should show upload errors as an alert", () => { - mockFileService.uploadFile.and.returnValue(throwError(() => new Error())); + it("should only upload the last file if a file was selected and then replaced", () => { + setupComponent(); + mockFileService.uploadFile.and.returnValue(of({ ok: true })); + component.formControl.enable(); component.onFileSelected(fileEvent); + expect(component.formControl).toHaveValue(file.name); - expect(mockAlertService.addDanger).toHaveBeenCalled(); - expect(component.formControl).not.toHaveValue(file.name); - }); + const otherFile = { name: "other.file" } as File; + const otherFileEvent = { target: { files: [otherFile] } }; + component.onFileSelected(otherFileEvent); + expect(component.formControl).toHaveValue(otherFile.name); - it("should show a file when clicking on the form element", () => { component.formControl.disable(); - component.formControl.setValue("existing.file"); - - component.formClicked(); - expect(mockFileService.showFile).toHaveBeenCalledWith( + expect(component.formControl).toHaveValue(otherFile.name); + expect(mockFileService.removeFile).not.toHaveBeenCalled(); + expect(mockFileService.uploadFile).toHaveBeenCalledWith( + otherFile, component.entity, component.formControlName ); }); - it("should not open a file if no value is set", () => { - component.fileClicked(); + it("should not remove a file if the file input was cleared but then canceled", () => { + setupComponent(file.name); + component.formControl.enable(); - expect(mockFileService.showFile).not.toHaveBeenCalled(); + component.delete(); + expect(component.formControl).toHaveValue(null); + + cancelForm(); + + expect(component.formControl).toHaveValue(file.name); + expect(mockFileService.removeFile).not.toHaveBeenCalled(); }); - it("should reset the form control after a file has been deleted", fakeAsync(() => { - mockConfirmationDialog.getConfirmation.and.resolveTo(true); + it("should not remove and upload a file if the file was replaced but then canceled", () => { + setupComponent(file.name); + component.formControl.enable(); + + const otherFileEvent = { target: { files: [{ name: "other.file" }] } }; + component.onFileSelected(otherFileEvent); + expect(component.formControl).toHaveValue("other.file"); + + cancelForm(); + + expect(component.formControl).toHaveValue(file.name); + expect(mockFileService.removeFile).not.toHaveBeenCalled(); + expect(mockFileService.uploadFile).not.toHaveBeenCalled(); + }); + + it("should remove a file if the file input was cleared and saved", () => { mockFileService.removeFile.and.returnValue(of({ ok: true })); - component.formControl.setValue("existing.file"); + setupComponent(file.name); + component.formControl.enable(); component.delete(); - tick(); + expect(component.formControl).toHaveValue(null); + component.formControl.disable(); + + expect(component.formControl).toHaveValue(null); expect(mockFileService.removeFile).toHaveBeenCalledWith( component.entity, component.formControlName ); - expect(component.formControl).toHaveValue(undefined); expect(mockAlertService.addInfo).toHaveBeenCalled(); - })); + }); - it("should not remove a file if the user declines", fakeAsync(() => { - mockConfirmationDialog.getConfirmation.and.resolveTo(false); - component.formControl.setValue("existing.file"); + it("should upload the new file if the file was replaced and then saved", () => { + mockFileService.uploadFile.and.returnValue(of({ ok: true })); + setupComponent(file.name); + component.formControl.enable(); - component.delete(); - tick(); + const otherFile = { name: "other.file" } as File; + const otherFileEvent = { target: { files: [otherFile] } }; + component.onFileSelected(otherFileEvent); + + component.formControl.disable(); + expect(component.formControl).toHaveValue(otherFile.name); expect(mockFileService.removeFile).not.toHaveBeenCalled(); - expect(component.formControl).toHaveValue("existing.file"); - expect(mockAlertService.addInfo).not.toHaveBeenCalled(); - })); + expect(mockFileService.uploadFile).toHaveBeenCalledWith( + otherFile, + component.entity, + component.formControlName + ); + }); + + it("should show upload errors as an alert and reset entity", () => { + setupComponent("old.file"); + const subject = new Subject(); + mockFileService.uploadFile.and.returnValue(subject); + component.formControl.enable(); + + component.onFileSelected(fileEvent); + + component.entity[component.formControlName] = file.name; + component.formControl.disable(); + + expect(component.formControl).toHaveValue(file.name); + expect(component.entity[component.formControlName]).toBe(file.name); + + subject.error(new Error()); + + expect(mockAlertService.addDanger).toHaveBeenCalled(); + expect(component.formControl).toHaveValue("old.file"); + expect(component.entity[component.formControlName]).toBe("old.file"); + expect(mockEntityMapper.save).toHaveBeenCalledWith(component.entity); + }); + + it("should show a file when clicking on the form element", () => { + setupComponent("existing.file"); + + component.formClicked(); + + expect(mockFileService.showFile).toHaveBeenCalledWith( + component.entity, + component.formControlName + ); + }); + + it("should not open a file if no value is set", () => { + setupComponent(); + component.fileClicked(); + + expect(mockFileService.showFile).not.toHaveBeenCalled(); + }); + + let initialValue: string; + + function setupComponent(value = null) { + initialValue = value; + component.onInitFromDynamicConfig({ + formControl: new FormControl(initialValue), + entity: Object.assign(new Entity(), { testProp: initialValue }), + formFieldConfig: { id: "testProp" }, + propertySchema: undefined, + }); + component.formControl.disable(); + fixture.detectChanges(); + } + + function cancelForm() { + component.formControl.setValue(initialValue); + component.formControl.disable(); + } }); diff --git a/src/app/features/file/edit-file/edit-file.component.ts b/src/app/features/file/edit-file/edit-file.component.ts index f427f8883c..9e0edb189d 100644 --- a/src/app/features/file/edit-file/edit-file.component.ts +++ b/src/app/features/file/edit-file/edit-file.component.ts @@ -1,10 +1,14 @@ import { Component, ElementRef, ViewChild } from "@angular/core"; -import { EditComponent } from "../../../core/entity-components/entity-utils/dynamic-form-components/edit-component"; +import { + EditComponent, + EditPropertyConfig, +} from "../../../core/entity-components/entity-utils/dynamic-form-components/edit-component"; import { DynamicComponent } from "../../../core/view/dynamic-components/dynamic-component.decorator"; import { AlertService } from "../../../core/alerts/alert.service"; import { LoggingService } from "../../../core/logging/logging.service"; -import { ConfirmationDialogService } from "../../../core/confirmation-dialog/confirmation-dialog.service"; import { FileService } from "../file.service"; +import { distinctUntilChanged, filter } from "rxjs/operators"; +import { EntityMapperService } from "../../../core/entity/entity-mapper.service"; /** * This component should be used as a `editComponent` when a property should store files. @@ -18,43 +22,67 @@ import { FileService } from "../file.service"; }) export class EditFileComponent extends EditComponent { @ViewChild("fileUpload") fileInput: ElementRef; + private selectedFile: File; + private removeClicked = false; + private initialValue: string; constructor( private fileService: FileService, private alertService: AlertService, private logger: LoggingService, - private confirmationDialog: ConfirmationDialogService + private entityMapper: EntityMapperService ) { super(); } + onInitFromDynamicConfig(config: EditPropertyConfig) { + super.onInitFromDynamicConfig(config); + this.initialValue = this.formControl.value; + this.formControl.statusChanges + .pipe( + distinctUntilChanged(), + filter((change) => change === "DISABLED") + ) + .subscribe(() => { + if ( + this.selectedFile && + this.selectedFile.name === this.formControl.value + ) { + this.uploadFile(this.selectedFile); + } else if (this.removeClicked && !this.formControl.value) { + this.removeFile(); + } + }); + } + async onFileSelected(event) { const file: File = event.target.files[0]; // directly reset input so subsequent selections with the same name also trigger the change event this.fileInput.nativeElement.value = ""; + this.selectedFile = file; + this.formControl.setValue(file.name); + } - if (this.formControl.value) { - const shouldReplace = await this.confirmationDialog.getConfirmation( - $localize`Replacing file`, - $localize`Do you want to replace the file "${this.formControl.value}" with "${file.name}"?` - ); - if (!shouldReplace) { - return; - } - } - + private uploadFile(file: File) { // The maximum file size which can be processed by CouchDB before a timeout is around 200mb this.fileService .uploadFile(file, this.entity, this.formControlName) .subscribe({ error: (err) => this.handleError(err), - complete: () => this.formControl.setValue(file.name), + complete: () => { + this.initialValue = this.formControl.value; + this.selectedFile = undefined; + }, }); } private handleError(err) { this.logger.error("Failed uploading file: " + JSON.stringify(err)); this.alertService.addDanger("Could not upload file, please try again."); + // Reset entity to how it was before + this.entity[this.formControlName] = this.initialValue; + this.formControl.setValue(this.initialValue); + return this.entityMapper.save(this.entity); } formClicked() { @@ -69,19 +97,22 @@ export class EditFileComponent extends EditComponent { } } - async delete() { - const shouldDelete = await this.confirmationDialog.getConfirmation( - $localize`Deleting file`, - $localize`Do you want to delete the file "${this.formControl.value}"?` - ); - if (!shouldDelete) { - return; - } + delete() { + this.formControl.setValue(null); + this.selectedFile = undefined; + // remove is only necessary if an initial value was set + this.removeClicked = !!this.initialValue; + } + + private removeFile() { this.fileService .removeFile(this.entity, this.formControlName) .subscribe(() => { - this.formControl.setValue(undefined); - this.alertService.addInfo($localize`:Message for user:File deleted`); + this.alertService.addInfo( + $localize`:Message for user:File "${this.initialValue}" deleted` + ); + this.initialValue = undefined; + this.removeClicked = false; }); } } diff --git a/src/app/features/file/file.service.ts b/src/app/features/file/file.service.ts index 6cbfefe455..f75f28c1eb 100644 --- a/src/app/features/file/file.service.ts +++ b/src/app/features/file/file.service.ts @@ -12,9 +12,9 @@ import { LoggingService } from "../../core/logging/logging.service"; */ export abstract class FileService { protected constructor( - protected entityMapper: EntityMapperService, - protected entities: EntityRegistry, - protected logger: LoggingService + private entityMapper: EntityMapperService, + private entities: EntityRegistry, + private logger: LoggingService ) { // TODO maybe registration is to late (only when component is rendered) this.deleteFilesOfDeletedEntities(); @@ -59,7 +59,7 @@ export abstract class FileService { } /** - * Removes a file and updates the entity to reflect this change + * Removes the file * @param entity * @param property of the entity which points to a file */ @@ -79,7 +79,7 @@ export abstract class FileService { abstract showFile(entity: Entity, property: string): void; /** - * Uploads the file and stores the information on `entity[property]` + * Uploads the file * @param file to be uploaded * @param entity * @param property where the information about the file should be stored diff --git a/src/app/features/file/mock-file.service.spec.ts b/src/app/features/file/mock-file.service.spec.ts index c5ba2b6066..e7a864f771 100644 --- a/src/app/features/file/mock-file.service.spec.ts +++ b/src/app/features/file/mock-file.service.spec.ts @@ -11,15 +11,15 @@ import { describe("MockFileService", () => { let service: MockFileService; - let mockEntityMapper: jasmine.SpyObj; beforeEach(() => { - mockEntityMapper = jasmine.createSpyObj(["save", "receiveUpdates"]); - mockEntityMapper.receiveUpdates.and.returnValue(NEVER); TestBed.configureTestingModule({ providers: [ MockFileService, - { provide: EntityMapperService, useValue: mockEntityMapper }, + { + provide: EntityMapperService, + useValue: { receiveUpdates: () => NEVER }, + }, { provide: EntityRegistry, useValue: entityRegistry }, ], }); @@ -39,9 +39,7 @@ describe("MockFileService", () => { await firstValueFrom(service.uploadFile(file, entity, prop)); - expect(entity[prop]).toBe(file.name); expect(URL.createObjectURL).toHaveBeenCalledWith(file); - expect(mockEntityMapper.save).toHaveBeenCalledWith(entity); service.showFile(entity, prop); @@ -51,11 +49,9 @@ describe("MockFileService", () => { it("should remove a file from a entity", async () => { const entity = new Entity(); const prop = "testProp"; - entity[prop] = "test.file"; - await firstValueFrom(service.removeFile(entity, prop)); + const res = await firstValueFrom(service.removeFile(entity, prop)); - expect(entity[prop]).toBe(undefined); - expect(mockEntityMapper.save).toHaveBeenCalledWith(entity); + expect(res).toEqual({ ok: true }); }); }); diff --git a/src/app/features/file/mock-file.service.ts b/src/app/features/file/mock-file.service.ts index 370b78ab30..b6f8716258 100644 --- a/src/app/features/file/mock-file.service.ts +++ b/src/app/features/file/mock-file.service.ts @@ -24,9 +24,8 @@ export class MockFileService extends FileService { } removeFile(entity: Entity, property: string): Observable { - this.fileMap.delete(entity[property]); - entity[property] = undefined; - return of(this.entityMapper.save(entity)); + this.fileMap.delete(entity + property); + return of({ ok: true }); } removeAllFiles(entity: Entity): Observable { @@ -34,13 +33,12 @@ export class MockFileService extends FileService { } showFile(entity: Entity, property: string): void { - window.open(this.fileMap.get(entity[property]), "_blank"); + window.open(this.fileMap.get(entity + property), "_blank"); } uploadFile(file: File, entity: Entity, property: string): Observable { const fileURL = URL.createObjectURL(file); - this.fileMap.set(file.name, fileURL); - entity[property] = file.name; - return of(this.entityMapper.save(entity)); + this.fileMap.set(entity + property, fileURL); + return of({ ok: true }); } } diff --git a/src/app/features/file/observable-queue/observable-queue.spec.ts b/src/app/features/file/observable-queue/observable-queue.spec.ts new file mode 100644 index 0000000000..1547685b45 --- /dev/null +++ b/src/app/features/file/observable-queue/observable-queue.spec.ts @@ -0,0 +1,166 @@ +import { ObservableQueue } from "./observable-queue"; +import { BehaviorSubject, of, Subject } from "rxjs"; +import { tap } from "rxjs/operators"; + +describe("ObservableQueue", () => { + let queue: ObservableQueue; + + beforeEach(() => { + queue = new ObservableQueue(); + }); + + it("should only start a new job after the previous one is done", () => { + const job1 = new Subject(); + const job2 = new Subject(); + const job3 = new BehaviorSubject(1); + let job1Done = false; + let job2Done = false; + let job3Done = false; + queue.add(job1).subscribe(() => (job1Done = true)); + queue.add(job2).subscribe(() => (job2Done = true)); + queue.add(job3).subscribe(() => (job3Done = true)); + + expect(job1.observed).toBeTrue(); + expect(job1Done).toBeFalse(); + expect(job2.observed).toBeFalse(); + expect(job2Done).toBeFalse(); + expect(job3.observed).toBeFalse(); + expect(job3Done).toBeFalse(); + + job1.next(undefined); + job1.complete(); + + expect(job1Done).toBeTrue(); + expect(job2.observed).toBeTrue(); + expect(job2Done).toBeFalse(); + expect(job3.observed).toBeFalse(); + expect(job3Done).toBeFalse(); + + job2.next(undefined); + job2.complete(); + + expect(job1Done).toBeTrue(); + expect(job2Done).toBeTrue(); + // Job 3 is executed once 2 is done + expect(job3Done).toBeTrue(); + }); + + it("should directly run a job if the previous one is already done", () => { + let job1done = false; + queue.add(of(1)).subscribe(() => (job1done = true)); + + expect(job1done).toBeTrue(); + + let job2done = true; + queue.add(of(1)).subscribe(() => (job2done = true)); + expect(job2done).toBeTrue(); + }); + + it("should not run the same observable multiple times", () => { + let job1Calls = 0; + const job1 = of(1).pipe(tap(() => job1Calls++)); + + queue.add(job1).subscribe(); + expect(job1Calls).toBe(1); + + queue.add(of(2)).subscribe(); + queue.add(of(3)).subscribe(); + expect(job1Calls).toBe(1); + }); + + it("should continue running observables if one throws an error", () => { + const errorJob = new Subject(); + const normalJob = new Subject(); + let errorJobDone = false; + let errorJobFailed = false; + let normalJobDone = false; + let normalJobFailed = false; + + queue.add(errorJob).subscribe({ + next: () => (errorJobDone = true), + error: () => (errorJobFailed = true), + }); + queue.add(normalJob).subscribe({ + next: () => (normalJobDone = true), + error: () => (normalJobFailed = true), + }); + + expect(errorJob.observed).toBeTrue(); + expect(normalJob.observed).toBeFalse(); + expect(errorJobFailed).toBeFalse(); + expect(errorJobDone).toBeFalse(); + expect(normalJobFailed).toBeFalse(); + expect(normalJobDone).toBeFalse(); + + errorJob.error(new Error()); + + expect(normalJob.observed).toBeTrue(); + expect(errorJobFailed).toBeTrue(); + expect(errorJobDone).toBeFalse(); + expect(normalJobFailed).toBeFalse(); + expect(normalJobDone).toBeFalse(); + + normalJob.next(undefined); + normalJob.complete(); + + expect(errorJobFailed).toBeTrue(); + expect(errorJobDone).toBeFalse(); + expect(normalJobFailed).toBeFalse(); + expect(normalJobDone).toBeTrue(); + }); + + it("should return the result of the correct observable", () => { + const job1 = new Subject(); + const job2 = of("job2"); + let res1; + let res2; + queue.add(job1).subscribe((res) => (res1 = res)); + queue.add(job2).subscribe((res) => (res2 = res)); + + expect(res1).toBeUndefined(); + expect(res1).toBeUndefined(); + + job1.next({ value: "job1 res" }); + job1.complete(); + + expect(res1).toEqual({ value: "job1 res" }); + expect(res2).toBe("job2"); + }); + + it("should only run next job if previous completes", () => { + const job1 = new Subject(); + const job2 = new Subject(); + let job1res: string; + let job1done = false; + let job2done = false; + queue.add(job1).subscribe({ + next: (res) => (job1res = res), + complete: () => (job1done = true), + }); + queue.add(job2).subscribe({ complete: () => (job2done = true) }); + + expect(job1.observed).toBeTrue(); + expect(job2.observed).toBeFalse(); + expect(job1done).toBeFalse(); + expect(job2done).toBeFalse(); + + job1.next("some"); + expect(job1res).toBe("some"); + job1.next("values"); + expect(job1res).toBe("values"); + job1.next("emitted"); + expect(job1res).toBe("emitted"); + + expect(job1.observed).toBeTrue(); + expect(job2.observed).toBeFalse(); + expect(job1done).toBeFalse(); + expect(job2done).toBeFalse(); + + job1.complete(); + + expect(job1res).toBe("emitted"); + expect(job2.observed).toBeTrue(); + expect(job1done).toBeTrue(); + expect(job2done).toBeFalse(); + }); +}); diff --git a/src/app/features/file/observable-queue/observable-queue.ts b/src/app/features/file/observable-queue/observable-queue.ts new file mode 100644 index 0000000000..3a9906b9d0 --- /dev/null +++ b/src/app/features/file/observable-queue/observable-queue.ts @@ -0,0 +1,18 @@ +import { last, Observable, of } from "rxjs"; +import { catchError, concatMap, shareReplay } from "rxjs/operators"; + +export class ObservableQueue { + private jobQueue: Observable = of(undefined); + + add(obs: Observable): Observable { + const newJob = this.jobQueue.pipe( + concatMap(() => obs), + shareReplay() + ); + this.jobQueue = newJob.pipe( + last(), + catchError(() => of(undefined)) + ); + return newJob; + } +}