From 88d3f3c2a5aa52c54d17e18620604dcc0759da9e Mon Sep 17 00:00:00 2001 From: Nessar Date: Fri, 24 Jul 2026 19:14:48 +0200 Subject: [PATCH] Align tenant and attachment validation (#48) --- .../adapter/in/web/tenant/TenantApi.java | 14 +- .../tenant/TenantAttachmentService.java | 3 + .../tenant/TenantDirectoryService.java | 9 +- .../rental/domain/tenant/Tenant.java | 19 ++- .../adapter/in/web/tenant/TenantWebTest.java | 61 +++++++ .../tenant/TenantAttachmentServicesTest.java | 16 ++ .../tenant/TenantDirectoryServiceTest.java | 40 +++++ .../rental/domain/tenant/TenantTest.java | 35 ++++ .../components/tenant-card/tenant-card.html | 2 +- .../tenant-card/tenant-card.spec.ts | 7 +- .../app/components/tenant-card/tenant-card.ts | 4 +- .../components/tenant-form/tenant-form.css | 6 + .../components/tenant-form/tenant-form.html | 112 ++++++------ .../tenant-form/tenant-form.spec.ts | 49 +++++- .../app/components/tenant-form/tenant-form.ts | 161 +++++++++++++++--- .../components/tenants-page/tenants-page.html | 2 + .../tenants-page/tenants-page.spec.ts | 2 + ...tenant-attachment-transfer.service.spec.ts | 87 ++++++++++ .../tenant-attachment-transfer.service.ts | 15 +- .../src/app/services/workspace.service.ts | 8 + 20 files changed, 550 insertions(+), 102 deletions(-) create mode 100644 backend/src/test/java/fr/nessar/lmnporganizer/rental/adapter/in/web/tenant/TenantWebTest.java create mode 100644 backend/src/test/java/fr/nessar/lmnporganizer/rental/application/tenant/TenantDirectoryServiceTest.java create mode 100644 frontend/src/app/services/tenant-attachment-transfer.service.spec.ts diff --git a/backend/src/main/java/fr/nessar/lmnporganizer/rental/adapter/in/web/tenant/TenantApi.java b/backend/src/main/java/fr/nessar/lmnporganizer/rental/adapter/in/web/tenant/TenantApi.java index d6572dd..dbcba64 100644 --- a/backend/src/main/java/fr/nessar/lmnporganizer/rental/adapter/in/web/tenant/TenantApi.java +++ b/backend/src/main/java/fr/nessar/lmnporganizer/rental/adapter/in/web/tenant/TenantApi.java @@ -1,7 +1,9 @@ package fr.nessar.lmnporganizer.rental.adapter.in.web.tenant; +import fr.nessar.lmnporganizer.rental.domain.shared.PhoneNumber; import fr.nessar.lmnporganizer.rental.domain.tenant.Tenant; import fr.nessar.lmnporganizer.rental.domain.tenant.TenantAttachment; +import jakarta.validation.constraints.AssertTrue; import jakarta.validation.constraints.Email; import jakarta.validation.constraints.NotBlank; import java.util.List; @@ -14,7 +16,17 @@ record Request( @NotBlank String lastName, @NotBlank @Email String email, String phone, - String address) {} + String address) { + @AssertTrue + boolean isPhoneValid() { + try { + new PhoneNumber(phone); + return true; + } catch (IllegalArgumentException exception) { + return false; + } + } +} record AttachmentResponse(String id, String fileName, String contentType, long size) { static AttachmentResponse from(TenantAttachment attachment) { diff --git a/backend/src/main/java/fr/nessar/lmnporganizer/rental/application/tenant/TenantAttachmentService.java b/backend/src/main/java/fr/nessar/lmnporganizer/rental/application/tenant/TenantAttachmentService.java index 3e4ab14..922fbab 100644 --- a/backend/src/main/java/fr/nessar/lmnporganizer/rental/application/tenant/TenantAttachmentService.java +++ b/backend/src/main/java/fr/nessar/lmnporganizer/rental/application/tenant/TenantAttachmentService.java @@ -34,6 +34,9 @@ public TenantAttachmentService( public TenantAttachment upload( LandlordId landlordId, TenantId tenantId, TenantAttachmentStorage.Upload upload) { Tenant tenant = tenant(landlordId, tenantId); + if (upload.size() == 0) { + throw new IllegalArgumentException("Tenant attachment file is required."); + } if (upload.size() > maximumSize) { throw new AttachmentTooLarge(maximumSize); } diff --git a/backend/src/main/java/fr/nessar/lmnporganizer/rental/application/tenant/TenantDirectoryService.java b/backend/src/main/java/fr/nessar/lmnporganizer/rental/application/tenant/TenantDirectoryService.java index 72c64f5..4c6c210 100644 --- a/backend/src/main/java/fr/nessar/lmnporganizer/rental/application/tenant/TenantDirectoryService.java +++ b/backend/src/main/java/fr/nessar/lmnporganizer/rental/application/tenant/TenantDirectoryService.java @@ -3,7 +3,6 @@ package fr.nessar.lmnporganizer.rental.application.tenant; import fr.nessar.lmnporganizer.rental.application.port.out.IdentifierGenerator; import fr.nessar.lmnporganizer.rental.application.shared.ResourceNotFound; import fr.nessar.lmnporganizer.rental.domain.landlord.LandlordId; -import fr.nessar.lmnporganizer.rental.domain.shared.PhoneNumber; import fr.nessar.lmnporganizer.rental.domain.tenant.Tenant; import fr.nessar.lmnporganizer.rental.domain.tenant.TenantId; import fr.nessar.lmnporganizer.rental.domain.tenant.TenantRepository; @@ -34,7 +33,7 @@ public Tenant create(LandlordId landlordId, ContactDetails details) { details.firstName(), details.lastName(), details.email(), - phone(details.phone()), + details.phone(), details.address(), List.of())); } @@ -45,15 +44,11 @@ public Tenant revise(LandlordId landlordId, TenantId tenantId, ContactDetails de details.firstName(), details.lastName(), details.email(), - phone(details.phone()), + details.phone(), details.address()); return tenants.save(tenant); } -private static String phone(String value) { - return new PhoneNumber(value).value(); -} - public record ContactDetails( String firstName, String lastName, String email, String phone, String address) {} } diff --git a/backend/src/main/java/fr/nessar/lmnporganizer/rental/domain/tenant/Tenant.java b/backend/src/main/java/fr/nessar/lmnporganizer/rental/domain/tenant/Tenant.java index ede3f4c..933d265 100644 --- a/backend/src/main/java/fr/nessar/lmnporganizer/rental/domain/tenant/Tenant.java +++ b/backend/src/main/java/fr/nessar/lmnporganizer/rental/domain/tenant/Tenant.java @@ -2,6 +2,7 @@ package fr.nessar.lmnporganizer.rental.domain.tenant; import fr.nessar.lmnporganizer.rental.domain.landlord.LandlordId; import fr.nessar.lmnporganizer.rental.domain.lease.LeaseId; +import fr.nessar.lmnporganizer.rental.domain.shared.PhoneNumber; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -103,8 +104,8 @@ public void updateContactDetails( String firstName, String lastName, String email, String phone, String address) { String updatedFirstName = required(firstName, "Tenant first name"); String updatedLastName = required(lastName, "Tenant last name"); - String updatedEmail = required(email, "Tenant email"); - String updatedPhone = phone == null ? "" : phone.trim(); + String updatedEmail = email(email); + String updatedPhone = new PhoneNumber(phone).value(); String updatedAddress = address == null ? "" : address.trim(); this.firstName = updatedFirstName; this.lastName = updatedLastName; @@ -161,6 +162,20 @@ private static String required(String value, String field) { return value.trim(); } +private static String email(String value) { + String email = required(value, "Tenant email"); + int at = email.indexOf('@'); + if (at <= 0 || at != email.lastIndexOf('@') || at == email.length() - 1) { + throw new IllegalArgumentException("Tenant email must be valid."); + } + for (int index = 0; index < email.length(); index++) { + if (Character.isWhitespace(email.charAt(index))) { + throw new IllegalArgumentException("Tenant email must be valid."); + } + } + return email; +} + public TenantId id() { return id; } diff --git a/backend/src/test/java/fr/nessar/lmnporganizer/rental/adapter/in/web/tenant/TenantWebTest.java b/backend/src/test/java/fr/nessar/lmnporganizer/rental/adapter/in/web/tenant/TenantWebTest.java new file mode 100644 index 0000000..074a1ea --- /dev/null +++ b/backend/src/test/java/fr/nessar/lmnporganizer/rental/adapter/in/web/tenant/TenantWebTest.java @@ -0,0 +1,61 @@ +package fr.nessar.lmnporganizer.rental.adapter.in.web.tenant; + +import static org.mockito.Mockito.verifyNoInteractions; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.oidcLogin; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import fr.nessar.lmnporganizer.LmnpOrganizerApplication; +import fr.nessar.lmnporganizer.identity.adapter.in.security.SecurityConfiguration; +import fr.nessar.lmnporganizer.rental.adapter.in.web.common.AuthenticatedLandlord; +import fr.nessar.lmnporganizer.rental.adapter.in.web.common.RentalApiExceptionHandler; +import fr.nessar.lmnporganizer.rental.application.landlord.LandlordAccountService; +import fr.nessar.lmnporganizer.rental.application.tenant.DeleteTenantService; +import fr.nessar.lmnporganizer.rental.application.tenant.TenantAttachmentService; +import fr.nessar.lmnporganizer.rental.application.tenant.TenantDirectoryService; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.webmvc.test.autoconfigure.WebMvcTest; +import org.springframework.context.annotation.Import; +import org.springframework.http.MediaType; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.test.web.servlet.MockMvc; + +@WebMvcTest(controllers = TenantController.class) +@ContextConfiguration(classes = LmnpOrganizerApplication.class) +@Import({AuthenticatedLandlord.class, RentalApiExceptionHandler.class, SecurityConfiguration.class}) +class TenantWebTest { +@Autowired MockMvc mockMvc; + +@MockitoBean LandlordAccountService accounts; +@MockitoBean TenantDirectoryService tenants; +@MockitoBean DeleteTenantService deletion; +@MockitoBean TenantAttachmentService attachments; + +@Test +void reportsStableTenantContactValidation() throws Exception { + mockMvc + .perform( + post("/api/tenants") + .with(oidcLogin()) + .with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content( + """ + {"firstName":" ","lastName":" ","email":"not-an-email","phone":"1234","address":" "} + """)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value("validation_failed")) + .andExpect( + jsonPath("$.errors[?(@.field == 'firstName' && @.code == 'not_blank')]").exists()) + .andExpect(jsonPath("$.errors[?(@.field == 'lastName' && @.code == 'not_blank')]").exists()) + .andExpect(jsonPath("$.errors[?(@.field == 'email' && @.code == 'email')]").exists()) + .andExpect( + jsonPath("$.errors[?(@.field == 'phoneValid' && @.code == 'assert_true')]").exists()); + + verifyNoInteractions(tenants); +} +} diff --git a/backend/src/test/java/fr/nessar/lmnporganizer/rental/application/tenant/TenantAttachmentServicesTest.java b/backend/src/test/java/fr/nessar/lmnporganizer/rental/application/tenant/TenantAttachmentServicesTest.java index bdb8ce3..810a133 100644 --- a/backend/src/test/java/fr/nessar/lmnporganizer/rental/application/tenant/TenantAttachmentServicesTest.java +++ b/backend/src/test/java/fr/nessar/lmnporganizer/rental/application/tenant/TenantAttachmentServicesTest.java @@ -39,6 +39,22 @@ private static final TenantAttachment SECOND_ATTACHMENT = new TenantAttachment( "attachment-2", "lease.pdf", "application/pdf", 20, "tenant-1/attachment-2"); +@Test +void rejectsAnEmptyUploadBeforeStorage() { + TenantRepository tenants = mock(TenantRepository.class); + TenantAttachmentStorage storage = mock(TenantAttachmentStorage.class); + Tenant tenant = tenant(List.of()); + when(tenants.find(LANDLORD_ID, TENANT_ID)).thenReturn(Optional.of(tenant)); + TenantAttachmentService service = + new TenantAttachmentService(tenants, storage, () -> "unused", 10); + + assertThrows( + IllegalArgumentException.class, + () -> service.upload(LANDLORD_ID, TENANT_ID, upload("empty.pdf", 0))); + + verify(storage, never()).store(any(), any(), any()); +} + @Test void rejectsOversizedUploadsAfterTheLandlordScopedLookupAndBeforeStorage() { TenantRepository tenants = mock(TenantRepository.class); diff --git a/backend/src/test/java/fr/nessar/lmnporganizer/rental/application/tenant/TenantDirectoryServiceTest.java b/backend/src/test/java/fr/nessar/lmnporganizer/rental/application/tenant/TenantDirectoryServiceTest.java new file mode 100644 index 0000000..cbbbb5e --- /dev/null +++ b/backend/src/test/java/fr/nessar/lmnporganizer/rental/application/tenant/TenantDirectoryServiceTest.java @@ -0,0 +1,40 @@ +package fr.nessar.lmnporganizer.rental.application.tenant; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import fr.nessar.lmnporganizer.rental.application.port.out.IdentifierGenerator; +import fr.nessar.lmnporganizer.rental.domain.landlord.LandlordId; +import fr.nessar.lmnporganizer.rental.domain.tenant.Tenant; +import fr.nessar.lmnporganizer.rental.domain.tenant.TenantId; +import fr.nessar.lmnporganizer.rental.domain.tenant.TenantRepository; +import java.util.Optional; +import org.junit.jupiter.api.Test; + +class TenantDirectoryServiceTest { +private static final LandlordId LANDLORD_ID = new LandlordId("landlord-1"); +private static final TenantId TENANT_ID = new TenantId("tenant-1"); + +@Test +void normalizesAnOptionalPhoneForCreationAndRevision() { + TenantRepository tenants = mock(TenantRepository.class); + IdentifierGenerator identifiers = () -> TENANT_ID.value(); + when(tenants.save(org.mockito.ArgumentMatchers.any())) + .thenAnswer(invocation -> invocation.getArgument(0)); + TenantDirectoryService service = new TenantDirectoryService(tenants, identifiers); + TenantDirectoryService.ContactDetails details = + new TenantDirectoryService.ContactDetails( + "Camille", "Martin", "camille@example.com", "06 12 34 56 78", ""); + + Tenant created = service.create(LANDLORD_ID, details); + when(tenants.find(LANDLORD_ID, TENANT_ID)).thenReturn(Optional.of(created)); + Tenant revised = service.revise(LANDLORD_ID, TENANT_ID, details); + + assertEquals("+33612345678", revised.phone()); + assertEquals("", revised.address()); + verify(tenants, times(2)).save(created); +} +} diff --git a/backend/src/test/java/fr/nessar/lmnporganizer/rental/domain/tenant/TenantTest.java b/backend/src/test/java/fr/nessar/lmnporganizer/rental/domain/tenant/TenantTest.java index 79caaf7..376fab8 100644 --- a/backend/src/test/java/fr/nessar/lmnporganizer/rental/domain/tenant/TenantTest.java +++ b/backend/src/test/java/fr/nessar/lmnporganizer/rental/domain/tenant/TenantTest.java @@ -10,6 +10,41 @@ import java.util.List; import org.junit.jupiter.api.Test; class TenantTest { +@Test +void requiresFormattedContactIdentity() { + assertThrows( + IllegalArgumentException.class, + () -> + new Tenant( + new TenantId("tenant-1"), + new LandlordId("landlord-1"), + "Camille", + "Martin", + "camille-at-example.com", + "", + "", + List.of())); +} + +@Test +void normalizesOptionalPhonesAndRejectsInvalidOnes() { + Tenant tenant = + new Tenant( + new TenantId("tenant-1"), + new LandlordId("landlord-1"), + "Camille", + "Martin", + "camille@example.com", + "06 12 34 56 78", + "", + List.of()); + + assertEquals("+33612345678", tenant.phone()); + assertThrows( + IllegalArgumentException.class, + () -> tenant.updateContactDetails("Camille", "Martin", "camille@example.com", "1234", "")); +} + @Test void ownsImmutableAttachmentMetadata() { List initialAttachments = new ArrayList<>(); diff --git a/frontend/src/app/components/tenant-card/tenant-card.html b/frontend/src/app/components/tenant-card/tenant-card.html index 3656c25..c8f5549 100644 --- a/frontend/src/app/components/tenant-card/tenant-card.html +++ b/frontend/src/app/components/tenant-card/tenant-card.html @@ -111,7 +111,7 @@ tenant().lastName " (change)="selectFile($event)" - />+ Ajouter une pièce jointe · 250 Mo maximum+ Ajouter une pièce jointe @if (uploading()) {
diff --git a/frontend/src/app/components/tenant-card/tenant-card.spec.ts b/frontend/src/app/components/tenant-card/tenant-card.spec.ts index d8de798..bd22490 100644 --- a/frontend/src/app/components/tenant-card/tenant-card.spec.ts +++ b/frontend/src/app/components/tenant-card/tenant-card.spec.ts @@ -2,7 +2,7 @@ import { TestBed } from '@angular/core/testing'; import { TenantCard } from './tenant-card'; describe('TenantCard', () => { - it('rejects files larger than the upload limit before emitting', () => { + it('forwards a selected file to the server-owned size guard', () => { const fixture = TestBed.createComponent(TenantCard); fixture.componentRef.setInput('tenant', { id: 'tenant-1', @@ -13,13 +13,14 @@ describe('TenantCard', () => { address: '', }); const file = new File(['identity'], 'archive.zip'); - Object.defineProperty(file, 'size', { value: 250 * 1024 * 1024 + 1 }); const input = document.createElement('input'); Object.defineProperty(input, 'files', { value: { item: () => file } }); + const requested = vi.fn(); + fixture.componentInstance.attachmentUploadRequested.subscribe(requested); fixture.componentInstance.selectFile({ target: input } as unknown as Event); - expect(fixture.componentInstance.uploadError()).toContain('250 Mo'); + expect(requested).toHaveBeenCalledWith({ tenantId: 'tenant-1', file }); }); it('shows measurable progress and disables only its upload control', () => { diff --git a/frontend/src/app/components/tenant-card/tenant-card.ts b/frontend/src/app/components/tenant-card/tenant-card.ts index 37db78d..7674869 100644 --- a/frontend/src/app/components/tenant-card/tenant-card.ts +++ b/frontend/src/app/components/tenant-card/tenant-card.ts @@ -41,9 +41,7 @@ export class TenantCard { const input = event.target as HTMLInputElement; const file = this.uploading() || this.deleting() ? null : input.files?.item(0); const tenantId = this.tenant().id; - if (file && file.size > 250 * 1024 * 1024) { - this.uploadError.set('Une pièce jointe ne peut pas dépasser 250 Mo.'); - } else if (tenantId && file) { + if (tenantId && file) { this.uploadError.set(''); this.attachmentUploadRequested.emit({ tenantId, file }); } diff --git a/frontend/src/app/components/tenant-form/tenant-form.css b/frontend/src/app/components/tenant-form/tenant-form.css index 16fed30..ec531c6 100644 --- a/frontend/src/app/components/tenant-form/tenant-form.css +++ b/frontend/src/app/components/tenant-form/tenant-form.css @@ -1,6 +1,12 @@ :host { display: block; } +.field-error { + display: block; + margin-top: -12px; + color: var(--danger); + font-size: 0.75rem; +} @media (min-width: 1180px) { .form-page { max-width: 900px; diff --git a/frontend/src/app/components/tenant-form/tenant-form.html b/frontend/src/app/components/tenant-form/tenant-form.html index 3d14d70..b7ffd84 100644 --- a/frontend/src/app/components/tenant-form/tenant-form.html +++ b/frontend/src/app/components/tenant-form/tenant-form.html @@ -21,94 +21,98 @@ (ngSubmit)="save()" [attr.aria-busy]="saving()" > - @if (saveError()) { + @if (showSaveError()) { } + @if (validationError()) { + + }
- Prénom + Prénom + - @if (tenantForm.firstName().touched() && tenantForm.firstName().invalid()) { - Le prénom est obligatoire. + @if (firstNameError()) { + {{ + firstNameError() + }} } - Nom + Nom + - @if (tenantForm.lastName().touched() && tenantForm.lastName().invalid()) { - Le nom est obligatoire. + @if (lastNameError()) { + {{ + lastNameError() + }} } - E-mail + E-mail + - @if (tenantForm.email().touched() && tenantForm.email().invalid()) { - - @if (tenantForm.email().getError('required')) { - L’e-mail est obligatoire. - } @else { - Saisissez une adresse e-mail valide. - } - + @if (emailError()) { + {{ + emailError() + }} } - Téléphone + Téléphone (facultatif) + - Adresse complète + /> + @if (phoneError()) { + {{ + phoneError() + }} + } + + + Adresse complète (facultative) + +
- +
diff --git a/frontend/src/app/components/tenant-form/tenant-form.spec.ts b/frontend/src/app/components/tenant-form/tenant-form.spec.ts index b621565..1d59f69 100644 --- a/frontend/src/app/components/tenant-form/tenant-form.spec.ts +++ b/frontend/src/app/components/tenant-form/tenant-form.spec.ts @@ -44,7 +44,7 @@ describe('TenantForm', () => { expect(submitted).not.toHaveBeenCalled(); }); - it('shows actionable validation guidance linked to invalid fields', () => { + it('focuses the first invalid field and explains optional-phone formatting', async () => { const fixture = TestBed.createComponent(TenantForm); fixture.detectChanges(); fixture.componentInstance.tenantModel.set({ @@ -56,6 +56,7 @@ describe('TenantForm', () => { }); fixture.componentInstance.save(); fixture.detectChanges(); + await fixture.whenStable(); const firstNameInput = fixture.nativeElement.querySelector('#firstName'); const emailInput = fixture.nativeElement.querySelector('#email'); @@ -64,7 +65,49 @@ describe('TenantForm', () => { expect(firstNameError?.textContent ?? '').toContain('Le prénom est obligatoire'); expect(emailError?.textContent ?? '').toContain('Saisissez une adresse e-mail valide'); - expect(firstNameInput.getAttribute('aria-describedby')).toContain('firstName-error'); - expect(emailInput.getAttribute('aria-describedby')).toContain('email-error'); + expect(document.activeElement).toBe(firstNameInput); + expect(emailInput.id).toBe('email'); + }); + + it('shows and clears only known server field errors without losing the form values', async () => { + const fixture = TestBed.createComponent(TenantForm); + const changed = vi.fn(); + fixture.componentInstance.fieldChanged.subscribe(changed); + fixture.componentRef.setInput('tenant', { + id: 'tenant-1', + firstName: 'Camille', + lastName: 'Martin', + email: 'camille@example.com', + phone: '1234', + address: '1 rue de Paris', + }); + fixture.componentRef.setInput('saveError', 'Veuillez corriger les champs signalés.'); + fixture.componentRef.setInput('saveFieldErrors', [ + { field: 'phoneValid', code: 'assert_true' }, + ]); + fixture.detectChanges(); + await fixture.whenStable(); + fixture.detectChanges(); + + expect(fixture.componentInstance.saveFieldErrors()).toEqual([ + { field: 'phoneValid', code: 'assert_true' }, + ]); + expect(fixture.componentInstance.phoneError()).toBe( + 'Saisissez un numéro de téléphone valide.', + ); + expect(fixture.nativeElement.textContent).toContain( + 'Saisissez un numéro de téléphone valide.', + ); + expect(fixture.nativeElement.textContent).not.toContain( + 'Veuillez corriger les champs signalés.', + ); + expect(document.activeElement?.id).toBe('phone'); + expect(fixture.componentInstance.tenantModel().address).toBe('1 rue de Paris'); + + (fixture.nativeElement.querySelector('#phone') as HTMLInputElement).dispatchEvent( + new Event('input'), + ); + expect(changed).toHaveBeenCalledWith('phone'); + expect(changed).toHaveBeenCalledWith('phoneValid'); }); }); diff --git a/frontend/src/app/components/tenant-form/tenant-form.ts b/frontend/src/app/components/tenant-form/tenant-form.ts index 367d9d2..4078e02 100644 --- a/frontend/src/app/components/tenant-form/tenant-form.ts +++ b/frontend/src/app/components/tenant-form/tenant-form.ts @@ -1,10 +1,12 @@ -import { Component, effect, input, output, signal } from '@angular/core'; +import { Component, effect, ElementRef, inject, input, output, signal } from '@angular/core'; import { FormsModule } from '@angular/forms'; -import { FormRoot, FormField, form, required, email } from '@angular/forms/signals'; +import { email, FormField, form, FormRoot, required, validate } from '@angular/forms/signals'; import { MatButtonModule } from '@angular/material/button'; import { MatFormFieldModule } from '@angular/material/form-field'; import { MatInputModule } from '@angular/material/input'; import { MatProgressSpinnerModule } from '@angular/material/progress-spinner'; +import { FieldValidationError } from '../../services/http-error-message'; +import { isValidPhoneNumber } from '../../services/phone-number'; import { Tenant } from '../../services/tenant.service'; export interface TenantSaveRequest { @@ -12,6 +14,8 @@ export interface TenantSaveRequest { tenantId?: string; } +type TenantField = 'firstName' | 'lastName' | 'email' | 'phone' | 'phoneValid' | 'address'; + @Component({ selector: 'app-tenant-form', imports: [ @@ -27,11 +31,15 @@ export interface TenantSaveRequest { styleUrl: './tenant-form.css', }) export class TenantForm { + private readonly element = inject>(ElementRef); readonly tenant = input(null); readonly saving = input(false); readonly saveError = input(''); + readonly saveFieldErrors = input([]); readonly cancelled = output(); readonly submitted = output(); + readonly fieldChanged = output(); + private previousServerErrorSignature = ''; readonly tenantModel = signal({ firstName: '', @@ -40,46 +48,155 @@ export class TenantForm { phone: '', address: '', }); - readonly tenantForm = form(this.tenantModel, (f) => { - required(f.firstName); - required(f.lastName); - required(f.email); - email(f.email); + required(f.firstName, { message: 'Le prénom est obligatoire.' }); + required(f.lastName, { message: 'Le nom est obligatoire.' }); + required(f.email, { message: 'L’e-mail est obligatoire.' }); + email(f.email, { message: 'Saisissez une adresse e-mail valide.' }); + validate(f.phone, ({ value }) => + isValidPhoneNumber(value()) + ? null + : { kind: 'phone', message: 'Saisissez un numéro de téléphone valide.' }, + ); }); + readonly validationError = signal(''); constructor() { - effect(() => this.fillForm(this.tenant())); + effect(() => { + this.fillForm(this.tenant()); + this.validationError.set(''); + }); + effect(() => { + const signature = this.saveFieldErrors() + .map((error) => `${error.field}:${error.code}`) + .sort() + .join('|'); + if (signature && signature !== this.previousServerErrorSignature) { + queueMicrotask(() => this.focusFirstInvalid()); + } + this.previousServerErrorSignature = signature; + }); } save(): void { if (this.saving()) return; - this.tenantModel.update((m) => ({ - firstName: m.firstName.trim(), - lastName: m.lastName.trim(), - email: m.email.trim(), - phone: m.phone.trim(), - address: m.address.trim(), + this.tenantModel.update((model) => ({ + ...model, + firstName: model.firstName.trim(), + lastName: model.lastName.trim(), + email: model.email.trim(), + phone: model.phone.trim(), + address: model.address.trim(), })); - this.tenantForm().markAsTouched(); - if (this.tenantForm().invalid()) { + if (this.tenantForm().invalid() || this.hasClientError()) { + this.validationError.set(this.firstError() ?? 'Saisissez des informations valides.'); + queueMicrotask(() => this.focusFirstInvalid()); return; } - const modelVal = this.tenantModel(); + this.validationError.set(''); + const tenant = this.tenantModel(); this.submitted.emit({ tenant: { - firstName: modelVal.firstName, - lastName: modelVal.lastName, - email: modelVal.email, - phone: modelVal.phone, - address: modelVal.address, + firstName: tenant.firstName, + lastName: tenant.lastName, + email: tenant.email, + phone: tenant.phone, + address: tenant.address, }, tenantId: this.tenant()?.id, }); } + changed(field: TenantField): void { + this.validationError.set(''); + this.fieldChanged.emit(field); + if (field === 'phone') this.fieldChanged.emit('phoneValid'); + } + + showSaveError(): boolean { + return ( + Boolean(this.saveError()) && + (this.saveFieldErrors().length === 0 || + this.saveFieldErrors().some((error) => !this.known(error))) + ); + } + + firstNameError(): string { + return this.requiredError('firstName', 'Le prénom est obligatoire.'); + } + + lastNameError(): string { + return this.requiredError('lastName', 'Le nom est obligatoire.'); + } + + emailError(): string { + if (this.tenantForm.email().touched() && !this.tenantModel().email.trim()) { + return 'L’e-mail est obligatoire.'; + } + if (this.tenantForm.email().touched() && this.tenantForm.email().invalid()) { + return 'Saisissez une adresse e-mail valide.'; + } + return this.hasServerError('email', 'not_blank') + ? 'L’e-mail est obligatoire.' + : this.hasServerError('email', 'email') + ? 'Saisissez une adresse e-mail valide.' + : ''; + } + + phoneError(): string { + if (this.tenantForm.phone().touched() && !isValidPhoneNumber(this.tenantModel().phone)) { + return 'Saisissez un numéro de téléphone valide.'; + } + return this.hasServerError('phoneValid', 'assert_true') + ? 'Saisissez un numéro de téléphone valide.' + : ''; + } + + private requiredError(field: 'firstName' | 'lastName', message: string): string { + if (this.tenantForm[field]().touched() && !this.tenantModel()[field].trim()) return message; + return this.hasServerError(field, 'not_blank') ? message : ''; + } + + private hasClientError(): boolean { + return Boolean( + this.firstNameError() || this.lastNameError() || this.emailError() || this.phoneError(), + ); + } + + private firstError(): string | undefined { + return ( + this.firstNameError() || this.lastNameError() || this.emailError() || this.phoneError() + ); + } + + private hasServerError(field: string, code: string): boolean { + return this.saveFieldErrors().some((error) => error.field === field && error.code === code); + } + + private known(error: FieldValidationError): boolean { + return ( + ((error.field === 'firstName' || error.field === 'lastName') && + error.code === 'not_blank') || + (error.field === 'email' && (error.code === 'not_blank' || error.code === 'email')) || + (error.field === 'phoneValid' && error.code === 'assert_true') + ); + } + + private focusFirstInvalid(): void { + const id = this.firstNameError() + ? 'firstName' + : this.lastNameError() + ? 'lastName' + : this.emailError() + ? 'email' + : this.phoneError() + ? 'phone' + : ''; + this.element.nativeElement.querySelector(`#${id}`)?.focus(); + } + private fillForm(tenant: Tenant | null): void { this.tenantModel.set({ id: tenant?.id, diff --git a/frontend/src/app/components/tenants-page/tenants-page.html b/frontend/src/app/components/tenants-page/tenants-page.html index 2f8d19c..9786aa8 100644 --- a/frontend/src/app/components/tenants-page/tenants-page.html +++ b/frontend/src/app/components/tenants-page/tenants-page.html @@ -118,7 +118,9 @@ [tenant]="editingTenant()" [saving]="workspace.tenantSaving(editingTenant()?.id)" [saveError]="workspace.tenantSaveError(editingTenant()?.id)" + [saveFieldErrors]="workspace.tenantSaveFieldErrors(editingTenant()?.id)" (cancelled)="closeForm()" (submitted)="saveTenant($event)" + (fieldChanged)="workspace.clearTenantSaveFieldError($event, editingTenant()?.id)" /> } diff --git a/frontend/src/app/components/tenants-page/tenants-page.spec.ts b/frontend/src/app/components/tenants-page/tenants-page.spec.ts index ff72e59..34b1569 100644 --- a/frontend/src/app/components/tenants-page/tenants-page.spec.ts +++ b/frontend/src/app/components/tenants-page/tenants-page.spec.ts @@ -11,6 +11,8 @@ describe('TenantsPage', () => { useValue: { loadTenants: () => undefined, clearTenantSaveError: () => undefined, + clearTenantSaveFieldError: () => undefined, + tenantSaveFieldErrors: () => [], }, }, ], diff --git a/frontend/src/app/services/tenant-attachment-transfer.service.spec.ts b/frontend/src/app/services/tenant-attachment-transfer.service.spec.ts new file mode 100644 index 0000000..569c5fc --- /dev/null +++ b/frontend/src/app/services/tenant-attachment-transfer.service.spec.ts @@ -0,0 +1,87 @@ +import { HttpEvent, HttpEventType, HttpResponse } from '@angular/common/http'; +import { TestBed } from '@angular/core/testing'; +import { signal } from '@angular/core'; +import { Observable, of, Subject, throwError } from 'rxjs'; +import { FileDownloadService } from './file-download.service'; +import { TenantAttachmentTransferService } from './tenant-attachment-transfer.service'; +import { TenantAttachment, TenantService } from './tenant.service'; + +describe('TenantAttachmentTransferService', () => { + let uploads: Subject>; + const tenants = { + uploadAttachment: + vi.fn<(tenantId: string, file: File) => Observable>>(), + deleteAttachment: vi.fn<(tenantId: string, attachmentId: string) => Observable>(), + downloadAttachment: vi.fn(), + }; + + beforeEach(() => { + vi.clearAllMocks(); + uploads = new Subject>(); + tenants.uploadAttachment.mockImplementation(() => uploads); + tenants.deleteAttachment.mockImplementation(() => of(undefined)); + TestBed.configureTestingModule({ + providers: [ + TenantAttachmentTransferService, + { provide: TenantService, useValue: tenants }, + { + provide: FileDownloadService, + useValue: { + pendingKeys: signal(new Set()), + errors: signal({}), + isPending: () => false, + download: () => undefined, + }, + }, + ], + }); + }); + + it('keeps upload feedback local to the selected tenant', () => { + const service = TestBed.inject(TenantAttachmentTransferService); + const uploaded = vi.fn(); + + service.upload('tenant-1', new File(['id'], 'id.pdf'), uploaded); + expect(service.uploadingTenantIds().has('tenant-1')).toBe(true); + uploads.next({ type: HttpEventType.UploadProgress, loaded: 1, total: 2 }); + expect(service.uploadProgress()['tenant-1']).toBe(50); + uploads.next( + new HttpResponse({ + body: { + id: 'attachment-1', + fileName: 'id.pdf', + contentType: 'application/pdf', + size: 2, + }, + }), + ); + expect(uploaded).toHaveBeenCalledOnce(); + uploads.complete(); + expect(service.uploadingTenantIds().has('tenant-1')).toBe(false); + }); + + it('keeps a failed attachment deletion actionable on that attachment', () => { + tenants.deleteAttachment.mockReturnValue( + throwError(() => ({ + status: 409, + error: { message: 'La pièce jointe est indisponible.' }, + })), + ); + const service = TestBed.inject(TenantAttachmentTransferService); + + service.delete('tenant-1', 'attachment-1', vi.fn()); + + expect(service.deletingIds().has('attachment-1')).toBe(false); + expect(service.deleteErrors()['attachment-1']).toBe('La pièce jointe est indisponible.'); + }); + + it('keeps a failed upload actionable on the selected tenant', () => { + tenants.uploadAttachment.mockReturnValue(throwError(() => ({ status: 413 }))); + const service = TestBed.inject(TenantAttachmentTransferService); + + service.upload('tenant-1', new File(['id'], 'id.pdf'), vi.fn()); + + expect(service.uploadingTenantIds().has('tenant-1')).toBe(false); + expect(service.errors()['tenant-1']).toContain('taille maximale'); + }); +}); diff --git a/frontend/src/app/services/tenant-attachment-transfer.service.ts b/frontend/src/app/services/tenant-attachment-transfer.service.ts index 56695f6..14554d9 100644 --- a/frontend/src/app/services/tenant-attachment-transfer.service.ts +++ b/frontend/src/app/services/tenant-attachment-transfer.service.ts @@ -4,7 +4,7 @@ import { finalize } from 'rxjs'; import { FileDownloadService } from './file-download.service'; import { TenantAttachment, TenantService } from './tenant.service'; import { withoutRecordKey, withoutSetValue } from './immutable-state'; -import { httpErrorMessage } from './http-error-message'; +import { saveFailure } from './http-error-message'; @Injectable({ providedIn: 'root' }) export class TenantAttachmentTransferService { @@ -60,10 +60,10 @@ export class TenantAttachmentTransferService { } }, error: (error: unknown) => { - const message = httpErrorMessage( + const message = saveFailure( error, - 'Impossible d’envoyer cette pièce jointe.', - ); + 'Impossible d’envoyer cette pièce jointe. Vérifiez le fichier et sa taille maximale.', + ).message; this.errors.update((errors) => ({ ...errors, [tenantId]: message })); }, }); @@ -99,10 +99,13 @@ export class TenantAttachmentTransferService { ) .subscribe({ next: () => onDeleted(), - error: () => + error: (error: unknown) => this.deleteErrors.update((errors) => ({ ...errors, - [attachmentId]: 'Impossible de supprimer cette pièce jointe.', + [attachmentId]: saveFailure( + error, + 'Impossible de supprimer cette pièce jointe. Réessayez.', + ).message, })), }); } diff --git a/frontend/src/app/services/workspace.service.ts b/frontend/src/app/services/workspace.service.ts index 9471382..1e35878 100644 --- a/frontend/src/app/services/workspace.service.ts +++ b/frontend/src/app/services/workspace.service.ts @@ -161,10 +161,18 @@ export class WorkspaceService { return this.tenantResource.saveErrors()[this.resourceKey(id)] ?? ''; } + tenantSaveFieldErrors(id?: string): FieldValidationError[] { + return this.tenantResource.saveFieldErrors()[this.resourceKey(id)] ?? []; + } + clearTenantSaveError(id?: string): void { this.tenantResource.clearSaveError(this.resourceKey(id)); } + clearTenantSaveFieldError(field: string, id?: string): void { + this.tenantResource.clearSaveFieldError(this.resourceKey(id), field); + } + deleteTenant(id: string): void { this.requestConfirmation( {