From ce53bab698307dc066040171f4fceb0b024d237a Mon Sep 17 00:00:00 2001 From: Nessar Date: Fri, 24 Jul 2026 19:40:24 +0200 Subject: [PATCH] Harden validation error recovery --- .../web/common/RentalApiExceptionHandler.java | 32 ++++++++++++++++ .../adapter/in/web/RentalWebAdaptersTest.java | 38 ++++++++++++++++++- .../in/web/lease/LeaseAmendmentWebTest.java | 2 + .../lease-clause-editor.ts | 3 +- .../lease-end-notice-card.spec.ts | 19 ++++++++++ .../lease-end-notice-card.ts | 2 + .../app/components/lease-form/lease-form.ts | 1 + .../private-area-editor.spec.ts | 3 ++ .../private-area-editor.ts | 1 + .../src/app/services/workspace-resource.ts | 6 ++- .../app/services/workspace.service.spec.ts | 30 ++++++++++++++- 11 files changed, 130 insertions(+), 7 deletions(-) diff --git a/backend/src/main/java/fr/nessar/lmnporganizer/rental/adapter/in/web/common/RentalApiExceptionHandler.java b/backend/src/main/java/fr/nessar/lmnporganizer/rental/adapter/in/web/common/RentalApiExceptionHandler.java index af58262..0c6621a 100644 --- a/backend/src/main/java/fr/nessar/lmnporganizer/rental/adapter/in/web/common/RentalApiExceptionHandler.java +++ b/backend/src/main/java/fr/nessar/lmnporganizer/rental/adapter/in/web/common/RentalApiExceptionHandler.java @@ -19,10 +19,14 @@ import org.slf4j.LoggerFactory; import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; +import org.springframework.http.converter.HttpMessageNotReadableException; import org.springframework.web.bind.MethodArgumentNotValidException; +import org.springframework.web.bind.MissingServletRequestParameterException; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.RestControllerAdvice; +import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException; import org.springframework.web.multipart.MaxUploadSizeExceededException; +import org.springframework.web.multipart.support.MissingServletRequestPartException; @RestControllerAdvice(basePackages = "fr.nessar.lmnporganizer.rental.adapter.in.web") public class RentalApiExceptionHandler { @@ -93,6 +97,27 @@ ResponseEntity invalidRequest(MethodArgumentNotValidExceptio .toList())); } +@ExceptionHandler(HttpMessageNotReadableException.class) +ResponseEntity unreadableRequest() { + return validation("request", "malformed"); +} + +@ExceptionHandler(MethodArgumentTypeMismatchException.class) +ResponseEntity invalidParameter(MethodArgumentTypeMismatchException exception) { + return validation(exception.getName(), "invalid"); +} + +@ExceptionHandler(MissingServletRequestParameterException.class) +ResponseEntity missingParameter( + MissingServletRequestParameterException exception) { + return validation(exception.getParameterName(), "required"); +} + +@ExceptionHandler(MissingServletRequestPartException.class) +ResponseEntity missingPart(MissingServletRequestPartException exception) { + return validation(exception.getRequestPartName(), "required"); +} + @ExceptionHandler(Exception.class) ResponseEntity unexpected(Exception exception) { LOGGER.error("Unhandled rental API exception", exception); @@ -107,6 +132,13 @@ private static ResponseEntity error(HttpStatus status, String message) return ResponseEntity.status(status).body(new ApiError(message)); } +private static ResponseEntity validation(String field, String code) { + return ResponseEntity.badRequest() + .body( + new ValidationApiError( + "validation_failed", java.util.List.of(new FieldValidationError(field, code)))); +} + record ApiError(String message) {} record CodedApiError(String code, String message) {} diff --git a/backend/src/test/java/fr/nessar/lmnporganizer/rental/adapter/in/web/RentalWebAdaptersTest.java b/backend/src/test/java/fr/nessar/lmnporganizer/rental/adapter/in/web/RentalWebAdaptersTest.java index 03ce96b..64fc14f 100644 --- a/backend/src/test/java/fr/nessar/lmnporganizer/rental/adapter/in/web/RentalWebAdaptersTest.java +++ b/backend/src/test/java/fr/nessar/lmnporganizer/rental/adapter/in/web/RentalWebAdaptersTest.java @@ -2,6 +2,7 @@ package fr.nessar.lmnporganizer.rental.adapter.in.web; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.never; @@ -31,6 +32,7 @@ import fr.nessar.lmnporganizer.rental.application.apartment.ApartmentCatalogServ import fr.nessar.lmnporganizer.rental.application.landlord.LandlordAccountService; import fr.nessar.lmnporganizer.rental.application.lease.AmendLeaseService; import fr.nessar.lmnporganizer.rental.application.lease.GenerateLeaseDocumentService; +import fr.nessar.lmnporganizer.rental.application.lease.LeaseClauseCatalog; import fr.nessar.lmnporganizer.rental.application.lease.LeaseDraftService; import fr.nessar.lmnporganizer.rental.application.rent.AllocateRentPaymentService; import fr.nessar.lmnporganizer.rental.application.rent.GeneratePaymentReceiptService; @@ -126,6 +128,7 @@ private static final RentPaymentRefundId REFUND_ID = new RentPaymentRefundId("re @MockitoBean private LeaseDraftService leaseDrafts; @MockitoBean private AmendLeaseService amendments; @MockitoBean private GenerateLeaseDocumentService leaseDocuments; +@MockitoBean private LeaseClauseCatalog leaseClauseCatalog; @MockitoBean private RentPaymentHistoryService paymentHistory; @MockitoBean private RecordRentPaymentService paymentRecording; @MockitoBean private AllocateRentPaymentService paymentAllocation; @@ -136,9 +139,17 @@ private static final RentPaymentRefundId REFUND_ID = new RentPaymentRefundId("re @BeforeEach void authenticateLandlordAndFixTime() { + Instant now = Instant.parse("2026-03-01T12:00:00Z"); when(accounts.identify(any())).thenReturn(landlord()); - when(clock.instant()).thenReturn(Instant.parse("2026-03-01T12:00:00Z")); + when(clock.instant()).thenReturn(now); when(clock.getZone()).thenReturn(ZoneOffset.UTC); + when(clock.withZone(any())).thenReturn(Clock.fixed(now, ZoneOffset.UTC)); + LeaseClauseCatalog clauses = + LeaseClauseCatalog.load(getClass().getResourceAsStream("/lease-clauses.yml")); + when(leaseClauseCatalog.available(anyString(), any())) + .thenAnswer( + invocation -> + clauses.available(invocation.getArgument(0), invocation.getArgument(1))); } @Test @@ -161,6 +172,25 @@ void protectedRentalMutationsRejectMissingCsrfTokens() throws Exception { .andExpect(status().isForbidden()); } +@Test +void malformedJsonIsAStableClientValidationFailure() throws Exception { + mockMvc + .perform( + post("/api/apartments") + .with(asLandlord()) + .with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content( + """ + {"name":"Studio","address":"12 rue A","purchasePrice":"not-a-number", + "description":"","privateAreas":[]} + """)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value("validation_failed")) + .andExpect(jsonPath("$.errors[0].field").value("request")) + .andExpect(jsonPath("$.errors[0].code").value("malformed")); +} + @Test void oidcClaimsScopeApartmentCreationAndLookupAndResponsesAreMapped() throws Exception { Apartment apartment = @@ -413,7 +443,11 @@ void leaseDraftRequestsRejectAnImpossibleAnnualRevisionDay() throws Exception { "referenceYear":2026,"seriesId":"001515333"}}} """)) .andExpect(status().isBadRequest()) - .andExpect(jsonPath("$.message").value("Le jour annuel de révision IRL est invalide.")); + .andExpect(jsonPath("$.code").value("validation_failed")) + .andExpect( + jsonPath("$.errors[0].field") + .value("legalTerms.rentRevision.annualRevisionDay")) + .andExpect(jsonPath("$.errors[0].code").value("invalid_date")); } @Test diff --git a/backend/src/test/java/fr/nessar/lmnporganizer/rental/adapter/in/web/lease/LeaseAmendmentWebTest.java b/backend/src/test/java/fr/nessar/lmnporganizer/rental/adapter/in/web/lease/LeaseAmendmentWebTest.java index b747a50..f23743f 100644 --- a/backend/src/test/java/fr/nessar/lmnporganizer/rental/adapter/in/web/lease/LeaseAmendmentWebTest.java +++ b/backend/src/test/java/fr/nessar/lmnporganizer/rental/adapter/in/web/lease/LeaseAmendmentWebTest.java @@ -16,6 +16,7 @@ import fr.nessar.lmnporganizer.rental.adapter.in.web.common.RentalApiExceptionHa import fr.nessar.lmnporganizer.rental.application.landlord.LandlordAccountService; import fr.nessar.lmnporganizer.rental.application.lease.AmendLeaseService; import fr.nessar.lmnporganizer.rental.application.lease.GenerateLeaseDocumentService; +import fr.nessar.lmnporganizer.rental.application.lease.LeaseClauseCatalog; import fr.nessar.lmnporganizer.rental.application.lease.LeaseDraftService; import fr.nessar.lmnporganizer.rental.domain.landlord.ExternalIdentity; import fr.nessar.lmnporganizer.rental.domain.landlord.Landlord; @@ -43,6 +44,7 @@ class LeaseAmendmentWebTest { @MockitoBean LeaseDraftService drafts; @MockitoBean AmendLeaseService amendments; @MockitoBean GenerateLeaseDocumentService documents; +@MockitoBean LeaseClauseCatalog clauseCatalog; @MockitoBean Clock clock; @BeforeEach diff --git a/frontend/src/app/components/lease-clause-editor/lease-clause-editor.ts b/frontend/src/app/components/lease-clause-editor/lease-clause-editor.ts index 6f88d12..d09fef5 100644 --- a/frontend/src/app/components/lease-clause-editor/lease-clause-editor.ts +++ b/frontend/src/app/components/lease-clause-editor/lease-clause-editor.ts @@ -131,7 +131,7 @@ export class LeaseClauseEditor { this.emit( selected ? [...clauses, { catalogId: id, parameters: {}, customText: '' }] : clauses, ); - this.fieldChanged.emit('specialClauses[' + index + '].catalogId'); + this.fieldChanged.emit(selected ? 'specialClauses[' + index + '].catalogId' : 'specialClauses['); } parameterValue(catalogId: string, parameterName: string): string { @@ -178,6 +178,7 @@ export class LeaseClauseEditor { removeClause(index: number): void { this.emit(this.specialClauses().filter((_, clauseIndex) => clauseIndex !== index)); + this.fieldChanged.emit('specialClauses['); } private load(): void { diff --git a/frontend/src/app/components/lease-end-notice-card/lease-end-notice-card.spec.ts b/frontend/src/app/components/lease-end-notice-card/lease-end-notice-card.spec.ts index e43266a..a085f36 100644 --- a/frontend/src/app/components/lease-end-notice-card/lease-end-notice-card.spec.ts +++ b/frontend/src/app/components/lease-end-notice-card/lease-end-notice-card.spec.ts @@ -110,4 +110,23 @@ describe('LeaseEndNoticeCard', () => { }); expect(downloadEvidence).toHaveBeenCalledWith('lease-1', 'notice-1', 1, 'preuve.jpg'); }); + + it('clears a stale evidence error when its file or reason is corrected', () => { + const fixture = createFixture(); + const input = document.createElement('input'); + const evidence = new File(['new'], 'preuve.txt', { type: 'text/plain' }); + Object.defineProperty(input, 'files', { value: { item: () => evidence } }); + fixture.componentInstance.evidenceError.set('Sélectionnez une preuve à conserver.'); + + fixture.componentInstance.selectEvidence({ target: input } as unknown as Event); + + expect(fixture.componentInstance.evidenceError()).toBe(''); + fixture.componentInstance.evidenceError.set( + 'Indiquez la raison de cette nouvelle version de preuve.', + ); + const reason = document.createElement('input'); + reason.value = 'Version lisible'; + fixture.componentInstance.updateEvidenceReason({ target: reason } as unknown as Event); + expect(fixture.componentInstance.evidenceError()).toBe(''); + }); }); diff --git a/frontend/src/app/components/lease-end-notice-card/lease-end-notice-card.ts b/frontend/src/app/components/lease-end-notice-card/lease-end-notice-card.ts index fd83551..78502ac 100644 --- a/frontend/src/app/components/lease-end-notice-card/lease-end-notice-card.ts +++ b/frontend/src/app/components/lease-end-notice-card/lease-end-notice-card.ts @@ -114,12 +114,14 @@ export class LeaseEndNoticeCard { this.evidence.set( input instanceof HTMLInputElement ? (input.files?.item(0) ?? null) : null, ); + this.evidenceError.set(''); if (input instanceof HTMLInputElement) input.value = ''; } updateEvidenceReason(event: Event): void { const input = event.target; this.evidenceReason.set(input instanceof HTMLInputElement ? input.value : ''); + this.evidenceError.set(''); } submitWithdrawal(): void { diff --git a/frontend/src/app/components/lease-form/lease-form.ts b/frontend/src/app/components/lease-form/lease-form.ts index 79c8789..db1b93a 100644 --- a/frontend/src/app/components/lease-form/lease-form.ts +++ b/frontend/src/app/components/lease-form/lease-form.ts @@ -202,6 +202,7 @@ export class LeaseForm { this.guarantors.update((guarantors) => guarantors.filter((_, itemIndex) => itemIndex !== index), ); + this.fieldWasChanged('guarantors['); } updateGuarantor(index: number, guarantor: Guarantor): void { diff --git a/frontend/src/app/components/private-area-editor/private-area-editor.spec.ts b/frontend/src/app/components/private-area-editor/private-area-editor.spec.ts index aaffab0..7905aa3 100644 --- a/frontend/src/app/components/private-area-editor/private-area-editor.spec.ts +++ b/frontend/src/app/components/private-area-editor/private-area-editor.spec.ts @@ -103,6 +103,8 @@ describe('PrivateAreaEditor', () => { ]); const changed = vi.fn(); fixture.componentInstance.areasChanged.subscribe(changed); + const fieldChanged = vi.fn(); + fixture.componentInstance.fieldChanged.subscribe(fieldChanged); const family = document.createElement('select'); family.append(new Option('Annexe', 'ANNEXE')); family.value = 'ANNEXE'; @@ -122,5 +124,6 @@ describe('PrivateAreaEditor', () => { ); expect(changed.mock.calls[1][0][0]).toEqual(expect.objectContaining({ label: 'Cave' })); expect(changed.mock.calls[2][0]).toEqual([]); + expect(fieldChanged).toHaveBeenCalledWith('privateAreas['); }); }); diff --git a/frontend/src/app/components/private-area-editor/private-area-editor.ts b/frontend/src/app/components/private-area-editor/private-area-editor.ts index b5c4f80..177fa0f 100644 --- a/frontend/src/app/components/private-area-editor/private-area-editor.ts +++ b/frontend/src/app/components/private-area-editor/private-area-editor.ts @@ -104,6 +104,7 @@ export class PrivateAreaEditor { removeArea(id: string): void { this.areasChanged.emit(this.areas().filter((area) => area.id !== id)); + this.fieldChanged.emit('privateAreas['); } validate(): boolean { diff --git a/frontend/src/app/services/workspace-resource.ts b/frontend/src/app/services/workspace-resource.ts index a228d51..f82876f 100644 --- a/frontend/src/app/services/workspace-resource.ts +++ b/frontend/src/app/services/workspace-resource.ts @@ -108,8 +108,10 @@ export class WorkspaceResource { clearSaveFieldError(key: string, field: string): void { const fieldErrors = this.saveFieldErrors()[key]; - if (!fieldErrors?.some((error) => error.field === field)) return; - const remaining = fieldErrors.filter((error) => error.field !== field); + const matches = (candidate: string) => + field.endsWith('[') ? candidate.startsWith(field) : candidate === field; + if (!fieldErrors?.some((error) => matches(error.field))) return; + const remaining = fieldErrors.filter((error) => !matches(error.field)); this.saveFieldErrors.update((errors) => remaining.length ? { ...errors, [key]: remaining } : withoutRecordKey(errors, key), ); diff --git a/frontend/src/app/services/workspace.service.spec.ts b/frontend/src/app/services/workspace.service.spec.ts index 53159a7..936fd8e 100644 --- a/frontend/src/app/services/workspace.service.spec.ts +++ b/frontend/src/app/services/workspace.service.spec.ts @@ -328,6 +328,30 @@ describe('WorkspaceService', () => { ); }); + it('clears indexed apartment errors after a structural collection change', () => { + apartments.updateApartment.mockReturnValueOnce( + throwError(() => ({ + status: 400, + error: { + code: 'validation_failed', + errors: [ + { field: 'privateAreas[0].label', code: 'not_blank' }, + { field: 'privateAreas[1].family', code: 'not_null' }, + { field: 'name', code: 'not_blank' }, + ], + }, + })), + ); + const service = TestBed.inject(WorkspaceService); + service.saveApartment(apartment, apartment.id); + + service.clearApartmentSaveFieldError('privateAreas[', apartment.id); + + expect(service.apartmentSaveFieldErrors(apartment.id)).toEqual([ + { field: 'name', code: 'not_blank' }, + ]); + }); + it('removes a confirmed deletion locally before reconciliation finishes', () => { const deleteResponse = new Subject(); const refreshResponse = new Subject<(typeof apartment)[]>(); @@ -529,12 +553,14 @@ describe('WorkspaceService', () => { throwError(() => ({ error: { message: 'Format refusé' } })), ); service.uploadTenantAttachment('tenant-1', new File(['id'], 'id.pdf')); - expect(service.tenantAttachmentErrors()['tenant-1']).toBe('Format refusé'); + expect(service.tenantAttachmentErrors()['tenant-1']).toBe( + 'Impossible d’envoyer cette pièce jointe. Vérifiez le fichier et sa taille maximale.', + ); tenants.uploadAttachment.mockReturnValueOnce(throwError(() => new Error('offline'))); service.uploadTenantAttachment('tenant-1', new File(['id'], 'id.pdf')); expect(service.tenantAttachmentErrors()['tenant-1']).toBe( - 'Impossible d’envoyer cette pièce jointe.', + 'Impossible d’envoyer cette pièce jointe. Vérifiez le fichier et sa taille maximale.', ); });