From a8a3d7a7478e8f4dfe75fcf3713ae3208b7fec2f Mon Sep 17 00:00:00 2001 From: Nessar Date: Thu, 16 Jul 2026 20:15:35 +0200 Subject: [PATCH] fix: complete loading review coverage (#18) --- frontend/src/app/app.spec.ts | 22 +++++++++++ .../apartment-form/apartment-form.spec.ts | 21 ++++++++++ .../apartments-page/apartments-page.spec.ts | 10 +++++ .../src/app/services/file-download.service.ts | 17 ++------ frontend/src/app/services/immutable-state.ts | 11 ++++++ .../tenant-attachment-transfer.service.ts | 19 ++------- .../src/app/services/workspace-resource.ts | 39 ++++++++----------- .../app/services/workspace.service.spec.ts | 20 ++++++++++ 8 files changed, 107 insertions(+), 52 deletions(-) create mode 100644 frontend/src/app/services/immutable-state.ts diff --git a/frontend/src/app/app.spec.ts b/frontend/src/app/app.spec.ts index 44ef07e..7421227 100644 --- a/frontend/src/app/app.spec.ts +++ b/frontend/src/app/app.spec.ts @@ -17,6 +17,8 @@ describe('App', () => { beforeEach(() => { status.set('checking'); session.isLoggedIn.set(false); + session.username.set(''); + session.email.set(''); vi.clearAllMocks(); TestBed.configureTestingModule({ providers: [provideRouter([]), { provide: SessionService, useValue: session }], @@ -41,4 +43,24 @@ describe('App', () => { expect(session.refresh).toHaveBeenCalled(); expect(fixture.nativeElement.querySelector('app-sign-in-page')).toBeNull(); }); + + it('renders the sign-in page for an anonymous session', () => { + status.set('anonymous'); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + expect(fixture.nativeElement.querySelector('app-sign-in-page')).not.toBeNull(); + expect(fixture.nativeElement.querySelector('app-workspace-navigation')).toBeNull(); + }); + + it('renders the workspace shell for an authenticated session', () => { + status.set('authenticated'); + session.isLoggedIn.set(true); + session.username.set('Camille'); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + expect(fixture.nativeElement.querySelector('app-workspace-navigation')).not.toBeNull(); + expect(fixture.nativeElement.querySelector('app-sign-in-page')).toBeNull(); + }); }); diff --git a/frontend/src/app/components/apartment-form/apartment-form.spec.ts b/frontend/src/app/components/apartment-form/apartment-form.spec.ts index 74e0c75..4217b79 100644 --- a/frontend/src/app/components/apartment-form/apartment-form.spec.ts +++ b/frontend/src/app/components/apartment-form/apartment-form.spec.ts @@ -64,4 +64,25 @@ describe('ApartmentForm', () => { 'Impossible d’enregistrer cet appartement.', ); }); + + it('preserves entered values after a save fails', async () => { + const fixture = TestBed.createComponent(ApartmentForm); + fixture.detectChanges(); + await fixture.whenStable(); + const form = fixture.componentInstance; + form.name.set('T2 rénové'); + form.address.set('45 rue des Entrepreneurs'); + form.purchasePrice.set(265000); + fixture.componentRef.setInput('saveError', 'Impossible d’enregistrer cet appartement.'); + fixture.detectChanges(); + await fixture.whenStable(); + + expect(fixture.nativeElement.querySelector('input[name=name]').value).toBe('T2 rénové'); + expect(fixture.nativeElement.querySelector('input[name=address]').value).toBe( + '45 rue des Entrepreneurs', + ); + expect(fixture.nativeElement.textContent).toContain( + 'Impossible d’enregistrer cet appartement.', + ); + }); }); diff --git a/frontend/src/app/components/apartments-page/apartments-page.spec.ts b/frontend/src/app/components/apartments-page/apartments-page.spec.ts index 07ee526..77d23a8 100644 --- a/frontend/src/app/components/apartments-page/apartments-page.spec.ts +++ b/frontend/src/app/components/apartments-page/apartments-page.spec.ts @@ -53,6 +53,16 @@ describe('ApartmentsPage', () => { expect(fixture.nativeElement.textContent).not.toContain('Aucun appartement enregistré'); }); + it('shows the empty state only after a successful empty load', () => { + workspace.apartmentLoadState.set({ loaded: true, loading: false, error: '' }); + const fixture = TestBed.createComponent(ApartmentsPage); + fixture.detectChanges(); + + expect(fixture.nativeElement.textContent).toContain('Aucun appartement enregistré'); + expect(fixture.nativeElement.querySelector('.skeleton-card')).toBeNull(); + expect(fixture.nativeElement.querySelector('.load-error')).toBeNull(); + }); + it('keeps established apartments visible during a background refresh', () => { workspace.apartments.set([ { diff --git a/frontend/src/app/services/file-download.service.ts b/frontend/src/app/services/file-download.service.ts index e0254eb..7681400 100644 --- a/frontend/src/app/services/file-download.service.ts +++ b/frontend/src/app/services/file-download.service.ts @@ -1,5 +1,6 @@ import { Injectable, signal } from '@angular/core'; import { finalize, Observable } from 'rxjs'; +import { withoutRecordKey, withoutSetValue } from './immutable-state'; @Injectable({ providedIn: 'root' }) export class FileDownloadService { @@ -17,9 +18,9 @@ export class FileDownloadService { download(key: string, request: Observable, fileName: string, errorMessage: string): void { if (this.pendingKeys().has(key)) return; this.pendingKeys.update((keys) => new Set(keys).add(key)); - this.errors.update((errors) => this.withoutKey(errors, key)); + this.errors.update((errors) => withoutRecordKey(errors, key)); request - .pipe(finalize(() => this.pendingKeys.update((keys) => this.without(keys, key)))) + .pipe(finalize(() => this.pendingKeys.update((keys) => withoutSetValue(keys, key)))) .subscribe({ next: (blob) => this.save(blob, fileName), error: () => this.errors.update((errors) => ({ ...errors, [key]: errorMessage })), @@ -34,16 +35,4 @@ export class FileDownloadService { link.click(); URL.revokeObjectURL(url); } - - private without(values: ReadonlySet, value: string): ReadonlySet { - const next = new Set(values); - next.delete(value); - return next; - } - - private withoutKey(record: Record, key: string): Record { - const next = { ...record }; - delete next[key]; - return next; - } } diff --git a/frontend/src/app/services/immutable-state.ts b/frontend/src/app/services/immutable-state.ts new file mode 100644 index 0000000..e455290 --- /dev/null +++ b/frontend/src/app/services/immutable-state.ts @@ -0,0 +1,11 @@ +export function withoutSetValue(values: ReadonlySet, value: T): ReadonlySet { + const next = new Set(values); + next.delete(value); + return next; +} + +export function withoutRecordKey(record: Record, key: string): Record { + const next = { ...record }; + delete next[key]; + return next; +} diff --git a/frontend/src/app/services/tenant-attachment-transfer.service.ts b/frontend/src/app/services/tenant-attachment-transfer.service.ts index f0e3e94..7f62c07 100644 --- a/frontend/src/app/services/tenant-attachment-transfer.service.ts +++ b/frontend/src/app/services/tenant-attachment-transfer.service.ts @@ -3,6 +3,7 @@ import { computed, Injectable, inject, signal } from '@angular/core'; import { finalize } from 'rxjs'; import { FileDownloadService } from './file-download.service'; import { TenantAttachment, TenantService } from './tenant.service'; +import { withoutRecordKey, withoutSetValue } from './immutable-state'; @Injectable({ providedIn: 'root' }) export class TenantAttachmentTransferService { @@ -30,15 +31,15 @@ export class TenantAttachmentTransferService { upload(tenantId: string, file: File, onUploaded: (attachment: TenantAttachment) => void): void { if (this.uploadingTenantIds().has(tenantId)) return; - this.errors.update((errors) => this.withoutKey(errors, tenantId)); + this.errors.update((errors) => withoutRecordKey(errors, tenantId)); this.uploadingTenantIds.update((ids) => new Set(ids).add(tenantId)); this.uploadProgress.update((progress) => ({ ...progress, [tenantId]: null })); this.tenantService .uploadAttachment(tenantId, file) .pipe( finalize(() => { - this.uploadingTenantIds.update((ids) => this.without(ids, tenantId)); - this.uploadProgress.update((progress) => this.withoutKey(progress, tenantId)); + this.uploadingTenantIds.update((ids) => withoutSetValue(ids, tenantId)); + this.uploadProgress.update((progress) => withoutRecordKey(progress, tenantId)); }), ) .subscribe({ @@ -78,18 +79,6 @@ export class TenantAttachmentTransferService { return `attachment:${attachmentId}`; } - private without(values: ReadonlySet, value: string): ReadonlySet { - const next = new Set(values); - next.delete(value); - return next; - } - - private withoutKey(record: Record, key: string): Record { - const next = { ...record }; - delete next[key]; - return next; - } - private errorMessage(error: unknown, fallback: string): string { return typeof error === 'object' && error !== null && diff --git a/frontend/src/app/services/workspace-resource.ts b/frontend/src/app/services/workspace-resource.ts index a55cb27..a50e27d 100644 --- a/frontend/src/app/services/workspace-resource.ts +++ b/frontend/src/app/services/workspace-resource.ts @@ -1,5 +1,6 @@ import { signal } from '@angular/core'; import { finalize, Observable } from 'rxjs'; +import { withoutRecordKey, withoutSetValue } from './immutable-state'; export interface LoadState { loaded: boolean; @@ -37,21 +38,16 @@ export class WorkspaceResource { this.loadState.set({ loaded: true, loading: false, error: '' }); if (revision !== this.revision) this.load(); }, - error: () => - this.loadState.update((state) => ({ - ...state, - loading: false, - error: this.messages.load, - })), + error: () => this.handleLoadError(revision), }); } save(key: string, request: Observable, onSuccess?: () => void): void { if (this.savingKeys().has(key)) return; this.savingKeys.update((keys) => new Set(keys).add(key)); - this.saveErrors.update((errors) => this.withoutKey(errors, key)); + this.saveErrors.update((errors) => withoutRecordKey(errors, key)); request - .pipe(finalize(() => this.savingKeys.update((keys) => this.without(keys, key)))) + .pipe(finalize(() => this.savingKeys.update((keys) => withoutSetValue(keys, key)))) .subscribe({ next: (saved) => { this.upsert(saved); @@ -65,9 +61,9 @@ export class WorkspaceResource { delete(id: string, request: Observable): void { if (this.deletingIds().has(id)) return; this.deletingIds.update((ids) => new Set(ids).add(id)); - this.deleteErrors.update((errors) => this.withoutKey(errors, id)); + this.deleteErrors.update((errors) => withoutRecordKey(errors, id)); request - .pipe(finalize(() => this.deletingIds.update((ids) => this.without(ids, id)))) + .pipe(finalize(() => this.deletingIds.update((ids) => withoutSetValue(ids, id)))) .subscribe({ next: () => { this.confirmChange((items) => items.filter((item) => item.id !== id)); @@ -91,7 +87,16 @@ export class WorkspaceResource { } clearSaveError(key: string): void { - this.saveErrors.update((errors) => this.withoutKey(errors, key)); + this.saveErrors.update((errors) => withoutRecordKey(errors, key)); + } + + private handleLoadError(revision: number): void { + this.loadState.update((state) => ({ + ...state, + loading: false, + error: this.messages.load, + })); + if (revision !== this.revision) this.load(); } private confirmChange(updateItems: (items: T[]) => T[]): void { @@ -99,16 +104,4 @@ export class WorkspaceResource { this.items.update(updateItems); this.loadState.update((state) => ({ ...state, loaded: true, error: '' })); } - - private without(values: ReadonlySet, value: string): ReadonlySet { - const next = new Set(values); - next.delete(value); - return next; - } - - private withoutKey(record: Record, key: string): Record { - const next = { ...record }; - delete next[key]; - return next; - } } diff --git a/frontend/src/app/services/workspace.service.spec.ts b/frontend/src/app/services/workspace.service.spec.ts index f753c66..c4dd5cc 100644 --- a/frontend/src/app/services/workspace.service.spec.ts +++ b/frontend/src/app/services/workspace.service.spec.ts @@ -160,6 +160,26 @@ describe('WorkspaceService', () => { expect(apartments.getApartments).toHaveBeenCalledTimes(2); }); + it('reconciles after an older refresh fails behind a confirmed apartment save', () => { + const updatedApartment = { ...apartment, name: 'T2 rénové' }; + const staleRefresh = new Subject<(typeof apartment)[]>(); + const reconciledRefresh = new Subject<(typeof apartment)[]>(); + apartments.getApartments + .mockReturnValueOnce(staleRefresh) + .mockReturnValueOnce(reconciledRefresh); + apartments.updateApartment.mockReturnValueOnce(of(updatedApartment)); + const service = TestBed.inject(WorkspaceService); + service.apartments.set([apartment]); + + service.loadApartments(); + service.saveApartment(updatedApartment, apartment.id); + staleRefresh.error(new Error('offline')); + + expect(service.apartments()).toEqual([updatedApartment]); + expect(apartments.getApartments).toHaveBeenCalledTimes(2); + expect(service.apartmentLoadState()).toEqual({ loaded: true, loading: true, error: '' }); + }); + it('scopes concurrent apartment saves and failures to their records', () => { const firstSave = new Subject(); const secondApartment = { ...apartment, id: 'apartment-2', name: 'T3' };