From feb651148e1261893ae162189c15e13e59d36704 Mon Sep 17 00:00:00 2001 From: Nessar Date: Sat, 17 Oct 2026 11:22:38 +0200 Subject: [PATCH] fix(planning): react only to calculations started by the mounted calendar Re-mounting the calendar no longer records a duration sample or announces the previous result, explanation or rejected explanation. Starting a calculation during a cancellation announces loading and keeps focus where it is. Only rejected explanation deliveries are announced, as before. Replacement tests now replace a real plan. Refs #275, #274 Co-Authored-By: Claude Opus 5.5 --- .../calendar-year-planning.spec.ts | 165 +++++++++++++++++- .../components/calendar-year/calendar-year.ts | 20 ++- .../components/user-input/user-input.spec.ts | 27 +++ .../services/planning-calculation.service.ts | 16 +- 4 files changed, 218 insertions(+), 10 deletions(-) diff --git a/src/app/components/calendar-year/calendar-year-planning.spec.ts b/src/app/components/calendar-year/calendar-year-planning.spec.ts index b8400c0..3602085 100644 --- a/src/app/components/calendar-year/calendar-year-planning.spec.ts +++ b/src/app/components/calendar-year/calendar-year-planning.spec.ts @@ -1992,6 +1992,146 @@ describe('CalendarYear planning outcome boundary', () => { }, ); + it('does not calibrate or announce a previous result when the planner is remounted', async () => { + const { calendar } = calendarFor(plan, explanation); + calendar.createPlanningCalculation + .mockReturnValueOnce(calculationFor(plan, explanation)) + .mockReturnValueOnce({ + ...calculationFor(plan, explanation), + outcome: new Promise(() => undefined), + }); + await TestBed.configureTestingModule({ + imports: [CalendarYear], + providers: [ + provideZonelessChangeDetection(), + { provide: CalendarService, useValue: calendar }, + ], + }).compileComponents(); + const record = vi.spyOn(TestBed.inject(PlanningDurationService), 'recordCompletion'); + const settle = async (fixture: ComponentFixture) => { + fixture.detectChanges(); + await fixture.whenStable(); + await new Promise((resolve) => setTimeout(resolve, 0)); + fixture.detectChanges(); + }; + const first = TestBed.createComponent(CalendarYear); + await settle(first); + expect(record).toHaveBeenCalledTimes(1); + first.destroy(); + + const second = TestBed.createComponent(CalendarYear); + await settle(second); + expect(record).toHaveBeenCalledTimes(1); + expect(second.componentInstance['liveMessage']()).toBe( + second.componentInstance['tr']().loading, + ); + }); + + it('does not announce a previous rejected explanation when the planner is remounted', async () => { + const { calendar } = calendarFor(plan, explanation); + const rejected = { + ...calculationFor(plan, explanation), + explanation: Promise.reject(new Error('explanation exploded')), + }; + rejected.explanation.catch(() => undefined); + calendar.createPlanningCalculation.mockReturnValueOnce(rejected).mockReturnValueOnce({ + ...calculationFor(plan, explanation), + outcome: new Promise(() => undefined), + }); + await TestBed.configureTestingModule({ + imports: [CalendarYear], + providers: [ + provideZonelessChangeDetection(), + { provide: CalendarService, useValue: calendar }, + ], + }).compileComponents(); + const settle = async (fixture: ComponentFixture) => { + fixture.detectChanges(); + await fixture.whenStable(); + await new Promise((resolve) => setTimeout(resolve, 0)); + fixture.detectChanges(); + }; + const first = TestBed.createComponent(CalendarYear); + await settle(first); + expect(first.componentInstance['liveMessage']()).toContain('explanation exploded'); + first.destroy(); + + const second = TestBed.createComponent(CalendarYear); + await settle(second); + expect(second.componentInstance['liveMessage']()).toBe( + second.componentInstance['tr']().loading, + ); + }); + + it('announces loading and keeps focus when a calculation starts during a cancellation', async () => { + const first = deferred(); + const { calendar, calculation } = calendarFor(plan, undefined); + calendar.createPlanningCalculation + .mockReturnValueOnce({ ...calculation, outcome: first.promise }) + .mockReturnValueOnce({ ...calculation, outcome: new Promise(() => undefined) }); + await TestBed.configureTestingModule({ + imports: [CalendarYear], + providers: [ + provideZonelessChangeDetection(), + { provide: CalendarService, useValue: calendar }, + ], + }).compileComponents(); + const fixture = TestBed.createComponent(CalendarYear); + fixture.detectChanges(); + const root = fixture.nativeElement as HTMLElement; + const live = () => fixture.componentInstance['liveMessage'](); + const tr = fixture.componentInstance['tr'](); + activateWithKeyboard(root.querySelector('.cancel-calculation')!); + fixture.detectChanges(); + expect(live()).toBe(tr.planning.cancelling); + const outside = document.createElement('button'); + document.body.append(outside); + outside.focus(); + + TestBed.inject(PlanningCalculationService).start(); + fixture.detectChanges(); + await fixture.whenStable(); + first.resolve(plan); + await fixture.whenStable(); + fixture.detectChanges(); + + expect(document.activeElement).toBe(outside); + expect(live()).toBe(tr.loading); + outside.remove(); + }); + + it.each([ + ['a delivered explanation error', false], + ['a rejected explanation', true], + ])('announces only %s when it rejected', async (_, rejected) => { + const reason = { code: 'explanation-error', message: 'Explanation boom' }; + const { calendar } = calendarFor(plan, undefined); + calendar.createPlanningCalculation.mockReturnValue({ + ...calculationFor(plan, undefined), + explanation: rejected + ? Promise.reject(new Error(reason.message)) + : Promise.resolve(undefined), + explanationError: Promise.resolve(rejected ? undefined : reason), + }); + await TestBed.configureTestingModule({ + imports: [CalendarYear], + providers: [ + provideZonelessChangeDetection(), + { provide: CalendarService, useValue: calendar }, + ], + }).compileComponents(); + const fixture = TestBed.createComponent(CalendarYear); + fixture.detectChanges(); + await fixture.whenStable(); + await new Promise((resolve) => setTimeout(resolve, 0)); + fixture.detectChanges(); + + const service = TestBed.inject(PlanningCalculationService); + expect(service.explanationError()).toMatchObject(reason); + expect(service.explanationRejected()).toBe(rejected); + expect(fixture.componentInstance['liveMessage']().includes(reason.message)).toBe(rejected); + }); + it('displays an accepted historical year below 100 without shifting it to the twentieth century', async () => { await TestBed.configureTestingModule({ imports: [CalendarYear], @@ -2190,6 +2330,9 @@ describe('CalendarYear planning outcome boundary', () => { ['PlanningIncomplete without incumbent', incomplete], ] as const)('clears stale calendar results for %s', async (_, outcome: PlanningOutcome) => { const { calendar } = calendarFor(outcome); + calendar.createPlanningCalculation + .mockReturnValueOnce(calculationFor(plan, explanation)) + .mockReturnValueOnce(calculationFor(outcome)); await TestBed.configureTestingModule({ imports: [CalendarYear], providers: [ @@ -2198,10 +2341,26 @@ describe('CalendarYear planning outcome boundary', () => { ], }).compileComponents(); const fixture: ComponentFixture = TestBed.createComponent(CalendarYear); - fixture.detectChanges(); - await fixture.whenStable(); + const service = TestBed.inject(PlanningCalculationService); + const settle = async () => { + fixture.detectChanges(); + await fixture.whenStable(); + await new Promise((resolve) => setTimeout(resolve, 0)); + fixture.detectChanges(); + }; + await settle(); + const root = fixture.nativeElement as HTMLElement; + expect(service.displayedPlan()).toBeDefined(); + expect( + Array.from(root.querySelectorAll('.plan-summary dd')).map((e) => e.textContent?.trim()), + ).toEqual(['2', '4', '6']); - expect(TestBed.inject(PlanningCalculationService).displayedPlan()).toBeUndefined(); + service.start(); + await settle(); + + expect(service.displayedPlan()).toBeUndefined(); + expect(root.querySelector('.plan-summary')).toBeNull(); + expect(root.querySelectorAll('button.selected')).toHaveLength(0); }); it('keeps and summarizes a returned result', async () => { diff --git a/src/app/components/calendar-year/calendar-year.ts b/src/app/components/calendar-year/calendar-year.ts index 590fdce..e5b3a9b 100644 --- a/src/app/components/calendar-year/calendar-year.ts +++ b/src/app/components/calendar-year/calendar-year.ts @@ -147,6 +147,9 @@ export class CalendarYear { private counterfactualVersion = 0; private wasCancelling = false; + private readonly initialAccepted = this.calculations.accepted(); + private readonly initialExplanation = this.calculations.explanation(); + private readonly initialExplanationError = this.calculations.explanationError(); private displayDateLocale = ''; private displayDateFormatter?: Intl.DateTimeFormat; @@ -314,18 +317,20 @@ export class CalendarYear { }); }); effect(() => { - if (this.isLoading()) untracked(() => this.liveMessage.set(this.tr().loading)); + if (this.calculations.pendingId() !== null) + untracked(() => this.liveMessage.set(this.tr().loading)); }); effect(() => { + const pending = this.calculations.pendingId() !== null; const cancelling = this.isCancelling(); untracked(() => { - if (this.wasCancelling && !cancelling) this.resultsTitle()?.nativeElement.focus(); + if (this.wasCancelling && !pending) this.resultsTitle()?.nativeElement.focus(); this.wasCancelling = cancelling; }); }); effect(() => { const accepted = this.calculations.accepted(); - if (!accepted) return; + if (!accepted || accepted === this.initialAccepted) return; untracked(() => { this.closeStatistics(); this.selectedPlacementIndex.set(0); @@ -351,7 +356,8 @@ export class CalendarYear { }); effect(() => { const explanation = this.calculations.explanation(); - if (explanation?.kind !== 'PlanExplanation') return; + if (explanation?.kind !== 'PlanExplanation' || explanation === this.initialExplanation) + return; untracked(() => { this.selectPlacement(0, false); const outcome = this.planningOutcome(); @@ -362,7 +368,11 @@ export class CalendarYear { }); effect(() => { const reason = this.explanationError(); - if (reason?.code === 'explanation-error') + if ( + reason && + reason !== this.initialExplanationError && + this.calculations.explanationRejected() + ) untracked(() => this.liveMessage.set( `${this.tr().planning.explanationError} ${reason.message}`, diff --git a/src/app/components/user-input/user-input.spec.ts b/src/app/components/user-input/user-input.spec.ts index 97f8e6f..810a198 100644 --- a/src/app/components/user-input/user-input.spec.ts +++ b/src/app/components/user-input/user-input.spec.ts @@ -155,6 +155,33 @@ describe('UserInput', () => { expect(card.textContent).toBe(explanation); }); + it('clears the remaining-days summary when a valid plan is replaced by a conflict', async () => { + await calculate({ + horizon: { start: '2027-05-01', end: '2027-05-10' }, + balances: [ + { + id: 'other', + type: 'other', + quantity: 1, + availableFrom: '2027-05-10', + expiryDate: '2028-05-10', + }, + ], + rules: [], + }); + const root = fixture.nativeElement as HTMLElement; + expect(calculations.displayedPlan()).toBeDefined(); + expect(root.querySelector('.warning-card')).not.toBeNull(); + + await show(calculations.accepted()!.calculation!, { + kind: 'PlanningConflict', + reasons: [], + conflictingRuleIds: ['rule'], + }); + expect(calculations.displayedPlan()).toBeUndefined(); + expect(root.querySelector('.warning-card')).toBeNull(); + }); + it('explains usable leave kept for later and incomplete balances without attempt controls', async () => { TestBed.inject(TranslationService).setLanguage('fr'); await calculate({ diff --git a/src/app/services/planning-calculation.service.ts b/src/app/services/planning-calculation.service.ts index e7871e5..80e8d90 100644 --- a/src/app/services/planning-calculation.service.ts +++ b/src/app/services/planning-calculation.service.ts @@ -17,6 +17,7 @@ import { UserInputService } from './user-input-service'; type Horizon = { start: string; end: string }; interface PendingCalculation { + id: number; horizon: Horizon; progress: number; cancelling: boolean; @@ -78,6 +79,7 @@ export class PlanningCalculationService { private readonly acceptedState = signal(null); private readonly explanationState = signal(null); private readonly explanationErrorState = signal(null); + private readonly explanationRejectedState = signal(false); readonly request = computed(() => { const horizon = this.settings.planningHorizon(); @@ -101,6 +103,8 @@ export class PlanningCalculationService { readonly accepted = this.acceptedState.asReadonly(); readonly explanation = this.explanationState.asReadonly(); readonly explanationError = this.explanationErrorState.asReadonly(); + readonly explanationRejected = this.explanationRejectedState.asReadonly(); + readonly pendingId = computed(() => this.pendingState()?.id ?? null); readonly isCalculating = computed(() => this.pendingState() !== null); readonly isCancelling = computed(() => this.pendingState()?.cancelling ?? false); readonly progress = computed(() => this.pendingState()?.progress ?? 0); @@ -124,7 +128,12 @@ export class PlanningCalculationService { const requestedAt = performance.now(); const version = ++this.version; this.userInput.declarationEntrySignal.set(false); - this.pendingState.set({ horizon: request.horizon, progress: 0, cancelling: false }); + this.pendingState.set({ + id: version, + horizon: request.horizon, + progress: 0, + cancelling: false, + }); let calculation: PlanningCalculationContext; try { calculation = this.calendar.createPlanningCalculation({ @@ -193,6 +202,7 @@ export class PlanningCalculationService { const cancelling = this.pendingState()?.cancelling ?? false; this.explanationState.set(null); this.explanationErrorState.set(null); + this.explanationRejectedState.set(false); this.pendingState.set(null); this.acceptedState.set({ calculation, @@ -209,7 +219,9 @@ export class PlanningCalculationService { private loadArtifacts(calculation: PlanningCalculationContext): void { const current = () => this.acceptedState()?.calculation === calculation; const fail = (error: unknown) => { - if (current()) this.explanationErrorState.set(explanationFailure(error)); + if (!current()) return; + this.explanationRejectedState.set(true); + this.explanationErrorState.set(explanationFailure(error)); }; calculation.explanation .then((explanation) => current() && this.explanationState.set(explanation ?? null))