Skip to content

Commit

Permalink
fix(file attachments): avoid errors while offline
Browse files Browse the repository at this point in the history
by disabling UI and offering better error messages

see #2450
  • Loading branch information
sleidig committed Aug 3, 2024
1 parent a70251f commit 840cffd
Show file tree
Hide file tree
Showing 13 changed files with 237 additions and 64 deletions.
12 changes: 12 additions & 0 deletions src/app/core/session/not-available-offline.error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
* Custom error indicating that some functionality (like file access) is not at the moment because the app is offline
* and that feature is not supported in offline mode.
*/
export class NotAvailableOfflineError extends Error {
/**
* @param feature The functionality that was attempted but is not available offline.
*/
constructor(feature: string) {
super("Functionality not available offline: " + feature);
}
}
29 changes: 29 additions & 0 deletions src/app/features/file/couchdb-file.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ import { map } from "rxjs/operators";
import { DomSanitizer, SafeUrl } from "@angular/platform-browser";
import { SyncService } from "../../core/database/sync.service";
import { environment } from "../../../environments/environment";
import { NAVIGATOR_TOKEN } from "../../utils/di-tokens";
import { NotAvailableOfflineError } from "../../core/session/not-available-offline.error";

describe("CouchdbFileService", () => {
let service: CouchdbFileService;
Expand All @@ -43,6 +45,7 @@ describe("CouchdbFileService", () => {
let dismiss: jasmine.Spy;
let updates: Subject<UpdatedEntity<Entity>>;
const attachmentUrlPrefix = `${environment.DB_PROXY_PREFIX}/${environment.DB_NAME}-attachments`;
let mockNavigator;

beforeEach(() => {
mockHttp = jasmine.createSpyObj(["get", "put", "delete"]);
Expand All @@ -57,6 +60,8 @@ describe("CouchdbFileService", () => {
dataType: FileDatatype.dataType,
});

mockNavigator = { onLine: true };

TestBed.configureTestingModule({
providers: [
CouchdbFileService,
Expand All @@ -79,6 +84,7 @@ describe("CouchdbFileService", () => {
},
},
{ provide: SyncService, useValue: mockSyncService },
{ provide: NAVIGATOR_TOKEN, useValue: mockNavigator },
],
});
service = TestBed.inject(CouchdbFileService);
Expand Down Expand Up @@ -156,6 +162,29 @@ describe("CouchdbFileService", () => {
});
});

it("should throw NotAvailableOffline error for uploadFile if offline (and not make requests)", (done) => {
mockNavigator.onLine = false;

service.uploadFile(null, new Entity("testId"), "testProp").subscribe({
error: (err) => {
expect(err).toBeInstanceOf(NotAvailableOfflineError);
expect(mockHttp.put).not.toHaveBeenCalled();
done();
},
});
});
it("should throw NotAvailableOffline error for removeFile if offline (and not make requests)", (done) => {
mockNavigator.onLine = false;

service.removeFile(new Entity("testId"), "testProp").subscribe({
error: (err) => {
expect(err).toBeInstanceOf(NotAvailableOfflineError);
expect(mockHttp.delete).not.toHaveBeenCalled();
done();
},
});
});

it("should remove a file using the latest rev", () => {
mockHttp.get.and.returnValue(of({ _rev: "test_rev" }));
mockHttp.delete.and.returnValue(of({ ok: true }));
Expand Down
19 changes: 14 additions & 5 deletions src/app/features/file/couchdb-file.service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Injectable } from "@angular/core";
import { Inject, Injectable } from "@angular/core";
import {
HttpClient,
HttpEvent,
Expand All @@ -16,7 +16,7 @@ import {
shareReplay,
tap,
} from "rxjs/operators";
import { from, Observable, of } from "rxjs";
import { from, Observable, of, throwError } from "rxjs";
import { MatDialog } from "@angular/material/dialog";
import { ShowFileComponent } from "./show-file/show-file.component";
import { Entity } from "../../core/entity/model/entity";
Expand All @@ -32,6 +32,8 @@ import { SyncStateSubject } from "../../core/session/session-type";
import { SyncService } from "../../core/database/sync.service";
import { SyncState } from "../../core/session/session-states/sync-state.enum";
import { environment } from "../../../environments/environment";
import { NAVIGATOR_TOKEN } from "../../utils/di-tokens";
import { NotAvailableOfflineError } from "../../core/session/not-available-offline.error";

/**
* Stores the files in the CouchDB.
Expand All @@ -54,11 +56,16 @@ export class CouchdbFileService extends FileService {
entityMapper: EntityMapperService,
entities: EntityRegistry,
syncState: SyncStateSubject,
@Inject(NAVIGATOR_TOKEN) private navigator: Navigator,
) {
super(entityMapper, entities, syncState);
}

uploadFile(file: File, entity: Entity, property: string): Observable<any> {
if (!this.navigator.onLine) {
return throwError(() => new NotAvailableOfflineError("File Attachments"));
}

const obs = this.requestQueue.add(
this.runFileUpload(file, entity, property),
);
Expand Down Expand Up @@ -112,6 +119,10 @@ export class CouchdbFileService extends FileService {
}

removeFile(entity: Entity, property: string) {
if (!this.navigator.onLine) {
return throwError(() => new NotAvailableOfflineError("File Attachments"));
}

return this.requestQueue.add(this.runFileRemoval(entity, property));
}

Expand Down Expand Up @@ -185,9 +196,7 @@ export class CouchdbFileService extends FileService {
.pipe(
map((blob) => URL.createObjectURL(blob)),
catchError((err) => {
Logging.warn(
`Could not load file (${entity?.getId()} . ${property}): ${err}`,
);
Logging.warn("Could not load file", entity?.getId(), property, err);

if (throwErrors) {
throw err;
Expand Down
10 changes: 9 additions & 1 deletion src/app/features/file/edit-file/edit-file.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

<mat-form-field
[ngClass]="{ clickable: formControl.value }"
(click)="formClicked()"
(click)="navigator.onLine ? formClicked() : null"
>
<mat-label>{{ label }}</mat-label>
<input
Expand All @@ -21,6 +21,7 @@
matTooltip="Show file"
[matTooltipDisabled]="!(initialValue && formControl.value === initialValue)"
/>

<button
*ngIf="formControl.value && formControl.enabled"
type="button"
Expand All @@ -29,6 +30,7 @@
(click)="delete(); $event.stopPropagation()"
i18n-mattooltip="Tooltip remove file"
matTooltip="Remove file"
[disabled]="!navigator.onLine"
>
<fa-icon icon="xmark"></fa-icon>
</button>
Expand All @@ -40,9 +42,15 @@
(click)="fileUpload.click(); $event.stopPropagation()"
i18n-matTooltip="Tooltip upload file button"
matTooltip="Upload file"
[disabled]="!navigator.onLine"
>
<fa-icon icon="upload"></fa-icon>
</button>

@if (!navigator.onLine && formControl.enabled) {
<mat-hint i18n>Changes to files are not possible offline.</mat-hint>
}

<mat-error>
<app-error-hint [form]="formControl"></app-error-hint>
</mat-error>
Expand Down
30 changes: 25 additions & 5 deletions src/app/features/file/edit-file/edit-file.component.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { ComponentFixture, TestBed } from "@angular/core/testing";
import {
ComponentFixture,
fakeAsync,
TestBed,
tick,
} from "@angular/core/testing";

import { EditFileComponent } from "./edit-file.component";
import { AlertService } from "../../../core/alerts/alert.service";
Expand All @@ -10,6 +15,7 @@ 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/entity-mapper.service";
import { NAVIGATOR_TOKEN } from "../../../utils/di-tokens";

describe("EditFileComponent", () => {
let component: EditFileComponent;
Expand All @@ -27,7 +33,7 @@ describe("EditFileComponent", () => {
"removeFile",
]);
mockAlertService = jasmine.createSpyObj(["addDanger", "addInfo"]);
mockEntityMapper = jasmine.createSpyObj(["save"]);
mockEntityMapper = jasmine.createSpyObj(["save", "load"]);
await TestBed.configureTestingModule({
imports: [
EditFileComponent,
Expand All @@ -39,6 +45,7 @@ describe("EditFileComponent", () => {
{ provide: AlertService, useValue: mockAlertService },
{ provide: FileService, useValue: mockFileService },
{ provide: EntityMapperService, useValue: mockEntityMapper },
{ provide: NAVIGATOR_TOKEN, useValue: { onLine: true } },
],
}).compileComponents();

Expand Down Expand Up @@ -218,8 +225,14 @@ describe("EditFileComponent", () => {
);
});

it("should show upload errors as an alert and reset entity", () => {
it("should show upload errors as an alert and reset entity", fakeAsync(() => {
setupComponent("old.file");
mockEntityMapper.load.and.resolveTo(
Object.assign(new Entity(component.entity.getId()), {
_rev: "2",
testProp: "new.file",
}),
);
const subject = new Subject();
mockFileService.uploadFile.and.returnValue(subject);
component.formControl.enable();
Expand All @@ -233,12 +246,19 @@ describe("EditFileComponent", () => {
expect(component.entity[component.formControlName]).toBe(file.name);

subject.error(new Error());
tick();

expect(mockAlertService.addDanger).toHaveBeenCalled();
expect(component.formControl).toHaveValue("old.file");
expect(component.entity[component.formControlName]).toBe("old.file");
expect(mockEntityMapper.save).toHaveBeenCalledWith(component.entity);
});
expect(mockEntityMapper.save).toHaveBeenCalledWith(
jasmine.objectContaining({
_id: component.entity["_id"],
_rev: "2",
testProp: "old.file",
}),
);
}));

it("should show a file when clicking on the form element", () => {
setupComponent("existing.file");
Expand Down
44 changes: 35 additions & 9 deletions src/app/features/file/edit-file/edit-file.component.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { Component, ElementRef, OnInit, ViewChild } from "@angular/core";
import {
Component,
ElementRef,
Inject,
OnInit,
ViewChild,
} from "@angular/core";
import { EditComponent } from "../../../core/entity/default-datatype/edit-component";
import { DynamicComponent } from "../../../core/config/dynamic-components/dynamic-component.decorator";
import { AlertService } from "../../../core/alerts/alert.service";
Expand All @@ -14,6 +20,8 @@ import { MatTooltipModule } from "@angular/material/tooltip";
import { MatButtonModule } from "@angular/material/button";
import { FontAwesomeModule } from "@fortawesome/angular-fontawesome";
import { ErrorHintComponent } from "../../../core/common-components/error-hint/error-hint.component";
import { NotAvailableOfflineError } from "../../../core/session/not-available-offline.error";
import { NAVIGATOR_TOKEN } from "../../../utils/di-tokens";

/**
* This component should be used as a `editComponent` when a property should store files.
Expand Down Expand Up @@ -47,6 +55,7 @@ export class EditFileComponent extends EditComponent<string> implements OnInit {
protected fileService: FileService,
private alertService: AlertService,
private entityMapper: EntityMapperService,
@Inject(NAVIGATOR_TOKEN) protected navigator: Navigator,
) {
super();
}
Expand Down Expand Up @@ -99,18 +108,34 @@ export class EditFileComponent extends EditComponent<string> implements OnInit {
}

private handleError(err) {
Logging.error("Failed uploading file: " + JSON.stringify(err));

let errorMessage = $localize`:File Upload Error Message:Failed uploading file. Please try again.`;
let errorMessage: string;
if (err?.status === 413) {
errorMessage = $localize`:File Upload Error Message:File too large. Usually files up to 5 MB are supported.`;
} else if (err instanceof NotAvailableOfflineError) {
errorMessage = $localize`:File Upload Error Message:Changes to file attachments are not available offline.`;
} else {
Logging.error("Failed to update file: " + JSON.stringify(err));
errorMessage = $localize`:File Upload Error Message:Failed to update file attachment. Please try again.`;
}
this.alertService.addDanger(errorMessage);

return this.revertEntityChanges();
}

private async revertEntityChanges() {
// ensure we have latest _rev of entity
this.entity = await this.entityMapper.load(
this.entity.getConstructor(),
this.entity.getId(),
);

// Reset entity to how it was before
this.entity[this.formControlName] = this.initialValue;
this.formControl.setValue(this.initialValue);
return this.entityMapper.save(this.entity);

await this.entityMapper.save(this.entity);

this.resetFile();
}

formClicked() {
Expand All @@ -136,15 +161,16 @@ export class EditFileComponent extends EditComponent<string> implements OnInit {
}

protected deleteExistingFile() {
this.fileService
.removeFile(this.entity, this.formControlName)
.subscribe(() => {
this.fileService.removeFile(this.entity, this.formControlName).subscribe({
error: (err) => this.handleError(err),
complete: () => {
this.alertService.addInfo(
$localize`:Message for user:File "${this.initialValue}" deleted`,
);
this.initialValue = undefined;
this.removeClicked = false;
});
},
});
}

protected resetFile() {
Expand Down
10 changes: 9 additions & 1 deletion src/app/features/file/edit-photo/edit-photo.component.html
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
<img [src]="imgPath" alt="Image" class="image" (click)="openPopup()" />

<div class="img-controls">
<div
class="img-controls"
[matTooltipDisabled]="!(formControl.enabled && !navigator.onLine)"
matTooltip="Changes to files are not possible offline."
i18n-matTooltip
>
<label class="img-label">{{ label }}</label>

<button
Expand All @@ -10,6 +15,7 @@
(click)="delete()"
i18n-mattooltip="Tooltip remove file"
matTooltip="Remove file"
[disabled]="!navigator.onLine"
>
<fa-icon icon="xmark"></fa-icon>
</button>
Expand All @@ -20,9 +26,11 @@
(click)="fileUpload.click()"
i18n-matTooltip="Tooltip upload file button"
matTooltip="Upload file"
[disabled]="!navigator.onLine"
>
<fa-icon icon="upload"></fa-icon>
</button>

<input
type="file"
style="display: none"
Expand Down
2 changes: 2 additions & 0 deletions src/app/features/file/edit-photo/edit-photo.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { EntitySchemaService } from "../../../core/entity/schema/entity-schema.s
import { FormControl } from "@angular/forms";
import { Entity } from "../../../core/entity/model/entity";
import { MatDialog } from "@angular/material/dialog";
import { NAVIGATOR_TOKEN } from "../../../utils/di-tokens";

describe("EditPhotoComponent", () => {
let component: EditPhotoComponent;
Expand Down Expand Up @@ -42,6 +43,7 @@ describe("EditPhotoComponent", () => {
{ provide: FileService, useValue: mockFileService },
{ provide: EntityMapperService, useValue: mockEntityMapper },
{ provide: MatDialog, useValue: mockDialog },
{ provide: NAVIGATOR_TOKEN, useValue: { onLine: true } },
],
}).compileComponents();

Expand Down
Loading

0 comments on commit 840cffd

Please sign in to comment.