fix: Incorrect/unclear Ckeditor error when uploading unsupported webp image in work package description (WP STC-645)

This commit is contained in:
Tomas Hykel
2026-06-13 17:58:34 +00:00
parent 8d1cbe8000
commit 37d8b52095
5 changed files with 169 additions and 3 deletions
@@ -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<never>) {
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();
},
});
});
});
@@ -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<IAttachment
responses
.map((response) => response.body)
.filter((body) => body !== null)),
catchError((error:HttpErrorResponse) => {
const message = isHalError(error.error) ? error.error.message : error.message;
return throwError(() => new Error(message));
}),
);
}
@@ -45,7 +45,7 @@
<op-toasters-upload-progress [file]="upload[0]"
[upload]="upload[1]"
(uploadSuccess)="onUploadSuccess()"
(uploadError)="remove()" />
(uploadError)="onUploadError($event)" />
}
</ul>
}
@@ -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);
});
});
});
@@ -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 });