diff --git a/frontend/src/app/core/state/attachments/attachments.service.spec.ts b/frontend/src/app/core/state/attachments/attachments.service.spec.ts index 7764f52b5e7..fa2855659b1 100644 --- a/frontend/src/app/core/state/attachments/attachments.service.spec.ts +++ b/frontend/src/app/core/state/attachments/attachments.service.spec.ts @@ -27,11 +27,19 @@ //++ import { TestBed } from '@angular/core/testing'; -import { provideHttpClient, withInterceptorsFromDi, withXhr } from '@angular/common/http'; +import { + HttpErrorResponse, + provideHttpClient, + withInterceptorsFromDi, + withXhr, +} from '@angular/common/http'; import { provideHttpClientTesting } from '@angular/common/http/testing'; +import { Observable, throwError } from 'rxjs'; import { States } from 'core-app/core/states/states.service'; import { ConfigurationService } from 'core-app/core/config/configuration.service'; import { OpUploadService } from 'core-app/core/upload/upload.service'; +import { ToastService } from 'core-app/shared/components/toaster/toast.service'; +import { OpenprojectHalModule } from 'core-app/features/hal/openproject-hal.module'; import { AttachmentsResourceService } from './attachments.service'; describe('AttachmentsResourceService', () => { @@ -52,3 +60,66 @@ describe('AttachmentsResourceService', () => { expect(TestBed.inject(AttachmentsResourceService)).toBeTruthy(); }); }); + +describe('AttachmentsResourceService upload error handling', () => { + const halMessage = "The file was rejected by an automatic filter. 'image/webp' is not allowed for upload."; + + function setupWithUploadError(uploadObservable:Observable) { + TestBed.configureTestingModule({ + imports: [OpenprojectHalModule], + providers: [ + AttachmentsResourceService, + { provide: States, useValue: new States() }, + { provide: ConfigurationService, useValue: {} }, + { provide: OpUploadService, useValue: { upload: () => [uploadObservable] } }, + provideHttpClient(withXhr(), withInterceptorsFromDi()), + provideHttpClientTesting(), + ], + }); + } + + it('re-emits the HAL error message as a plain Error when the upload fails with a HAL error body', (done) => { + const httpError = new HttpErrorResponse({ + status: 422, + error: { + _type: 'Error', + errorIdentifier: 'urn:openproject-org:api:v3:errors:PropertyConstraintViolation', + message: halMessage, + }, + }); + + setupWithUploadError(throwError(() => httpError)); + + const service = TestBed.inject(AttachmentsResourceService); + spyOn(TestBed.inject(ToastService), 'addUpload').and.returnValue({ message: '', type: 'upload' }); + + service.addAttachments('key', '/api/v3/attachments', [{ file: new File([], 'test.webp') }]) + .subscribe({ + next: () => done.fail('Expected an error, not a value'), + error: (err:Error) => { + expect(err).toBeInstanceOf(Error); + expect(err.message).toEqual(halMessage); + done(); + }, + }); + }); + + it('re-emits the raw HTTP message as a plain Error when no HAL body is present', (done) => { + const httpError = new HttpErrorResponse({ status: 500, statusText: 'Internal Server Error' }); + + setupWithUploadError(throwError(() => httpError)); + + const service = TestBed.inject(AttachmentsResourceService); + spyOn(TestBed.inject(ToastService), 'addUpload').and.returnValue({ message: '', type: 'upload' }); + + service.addAttachments('key', '/api/v3/attachments', [{ file: new File([], 'test.webp') }]) + .subscribe({ + next: () => done.fail('Expected an error, not a value'), + error: (err:Error) => { + expect(err).toBeInstanceOf(Error); + expect(err.message).toBeTruthy(); + done(); + }, + }); + }); +}); diff --git a/frontend/src/app/core/state/attachments/attachments.service.ts b/frontend/src/app/core/state/attachments/attachments.service.ts index 1d6e3fa1173..e47c6285957 100644 --- a/frontend/src/app/core/state/attachments/attachments.service.ts +++ b/frontend/src/app/core/state/attachments/attachments.service.ts @@ -33,7 +33,7 @@ import { HttpHeaders, } from '@angular/common/http'; import { applyTransaction } from '@datorama/akita'; -import { Observable } from 'rxjs'; +import { Observable, throwError } from 'rxjs'; import { catchError, map, @@ -50,6 +50,7 @@ import { IUploadFile, OpUploadService, } from 'core-app/core/upload/upload.service'; +import { isHalError } from 'core-app/features/hal/resources/error-resource'; import { removeEntityFromCollectionAndState } from 'core-app/core/state/resource-store'; import { ResourceStore, @@ -162,6 +163,10 @@ export class AttachmentsResourceService extends ResourceStoreService response.body) .filter((body) => body !== null)), + catchError((error:HttpErrorResponse) => { + const message = isHalError(error.error) ? error.error.message : error.message; + return throwError(() => new Error(message)); + }), ); } diff --git a/frontend/src/app/shared/components/toaster/toast.component.html b/frontend/src/app/shared/components/toaster/toast.component.html index 03603a68d4a..75c89e07100 100644 --- a/frontend/src/app/shared/components/toaster/toast.component.html +++ b/frontend/src/app/shared/components/toaster/toast.component.html @@ -45,7 +45,7 @@ + (uploadError)="onUploadError($event)" /> } } diff --git a/frontend/src/app/shared/components/toaster/toast.component.spec.ts b/frontend/src/app/shared/components/toaster/toast.component.spec.ts new file mode 100644 index 00000000000..d17dda43b7b --- /dev/null +++ b/frontend/src/app/shared/components/toaster/toast.component.spec.ts @@ -0,0 +1,84 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { TestBed } from '@angular/core/testing'; +import { HttpErrorResponse, provideHttpClient, withInterceptorsFromDi, withXhr } from '@angular/common/http'; +import { provideHttpClientTesting } from '@angular/common/http/testing'; +import { I18nService } from 'core-app/core/i18n/i18n.service'; +import { ToastComponent } from 'core-app/shared/components/toaster/toast.component'; +import { IToast, ToastService } from 'core-app/shared/components/toaster/toast.service'; +import { ConfigurationService } from 'core-app/core/config/configuration.service'; +import { OpenprojectHalModule } from 'core-app/features/hal/openproject-hal.module'; + +describe('ToastComponent', () => { + let component:ToastComponent; + let toastService:ToastService; + + const mockToast:IToast = { message: 'Uploading...', type: 'upload' }; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [OpenprojectHalModule], + providers: [ + ToastComponent, + { provide: ConfigurationService, useValue: { autoHidePopups: () => false } }, + I18nService, + ToastService, + provideHttpClient(withXhr(), withInterceptorsFromDi()), + provideHttpClientTesting(), + ], + }).compileComponents(); + + component = TestBed.inject(ToastComponent); + toastService = TestBed.inject(ToastService); + + component.toast = mockToast; + }); + + describe('#onUploadError', () => { + it('calls toastService.addError with the HttpErrorResponse', () => { + const error = new HttpErrorResponse({ status: 422, error: { message: 'image/webp is not allowed' } }); + spyOn(toastService, 'addError'); + spyOn(toastService, 'remove'); + + component.onUploadError(error); + + expect(toastService.addError).toHaveBeenCalledWith(error); + }); + + it('removes the upload toast after showing the error', () => { + const error = new HttpErrorResponse({ status: 422, error: { message: 'image/webp is not allowed' } }); + spyOn(toastService, 'addError'); + spyOn(toastService, 'remove'); + + component.onUploadError(error); + + expect(toastService.remove).toHaveBeenCalledWith(mockToast); + }); + }); +}); diff --git a/frontend/src/app/shared/components/toaster/toast.component.ts b/frontend/src/app/shared/components/toaster/toast.component.ts index 30924c5e4c5..3f895c67bfc 100644 --- a/frontend/src/app/shared/components/toaster/toast.component.ts +++ b/frontend/src/app/shared/components/toaster/toast.component.ts @@ -36,6 +36,7 @@ import { timeout, } from 'rxjs/operators'; import { take } from 'rxjs/internal/operators/take'; +import { HttpErrorResponse } from '@angular/common/http'; import { I18nService } from 'core-app/core/i18n/i18n.service'; import { IToast, ToastService, ToastType } from 'core-app/shared/components/toaster/toast.service'; @@ -121,6 +122,11 @@ export class ToastComponent implements OnInit { this.uploadCount += 1; } + public onUploadError(error:HttpErrorResponse):void { + this.toastService.addError(error); + this.remove(); + } + public get uploadText():string { const count = (this.data as unknown[]).length; return this.I18n.t('js.label_upload_counter', { done: this.uploadCount, count });