diff --git a/frontend/ai.client/src/app/admin/manage-announcements/announcement-form.page.spec.ts b/frontend/ai.client/src/app/admin/manage-announcements/announcement-form.page.spec.ts index 391684d3..f3747437 100644 --- a/frontend/ai.client/src/app/admin/manage-announcements/announcement-form.page.spec.ts +++ b/frontend/ai.client/src/app/admin/manage-announcements/announcement-form.page.spec.ts @@ -82,6 +82,52 @@ describe('AnnouncementFormPage', () => { }); } + describe('submit gate reactivity', () => { + /** + * The regression that browser verification caught and the original specs + * missed: they only ever read `canSubmit()` *after* filling the form, so + * its first evaluation saw a valid form, tracked every signal, and stayed + * reactive. In the real page the first read happens while the form is + * empty — and an early `return` on the non-signal `form.invalid` shortened + * the computed's dependency set to `isSubmitting` alone. Nothing could + * re-enable the button afterwards. + * + * Reading it empty FIRST is the whole point of these tests. + */ + it('enables once the form is filled, having first been read while empty', async () => { + const page = await createPage(); + + expect(page.canSubmit()).toBe(false); // first evaluation, empty form + + fill(page, {}); + + expect(page.canSubmit()).toBe(true); + }); + + it('stays reactive to a later invalidation', async () => { + const page = await createPage(); + expect(page.canSubmit()).toBe(false); + + fill(page, {}); + expect(page.canSubmit()).toBe(true); + + // Clearing a required field must disable it again. + page.form.patchValue({ title: '' }); + expect(page.canSubmit()).toBe(false); + }); + + it('re-enables after a blocking rule is satisfied, read empty first', async () => { + const page = await createPage(); + expect(page.canSubmit()).toBe(false); + + fill(page, { banner: true }); + expect(page.canSubmit()).toBe(false); // banner with no expiry + + page.form.patchValue({ expires_at: '2099-01-01T00:00' }); + expect(page.canSubmit()).toBe(true); + }); + }); + describe('surfaces', () => { it('always sends panel, even though the form has no panel checkbox', async () => { // The server forces it too (§D1), but sending it keeps the payload diff --git a/frontend/ai.client/src/app/admin/manage-announcements/announcement-form.page.ts b/frontend/ai.client/src/app/admin/manage-announcements/announcement-form.page.ts index 61f794e3..054812ac 100644 --- a/frontend/ai.client/src/app/admin/manage-announcements/announcement-form.page.ts +++ b/frontend/ai.client/src/app/admin/manage-announcements/announcement-form.page.ts @@ -485,6 +485,8 @@ export class AnnouncementFormPage implements OnInit { private readonly expiresAtSig = signal(''); private readonly ctaLabelSig = signal(''); private readonly ctaUrlSig = signal(''); + // Form validity is not a signal on FormGroup, so mirror it like the rest. + private readonly formValid = signal(false); protected readonly modalSelected = this.modalSig.asReadonly(); protected readonly allRolesSelected = this.allRolesSig.asReadonly(); @@ -519,16 +521,39 @@ export class AnnouncementFormPage implements OnInit { protected readonly roles = computed(() => this.rolesService.getRoles()); + /** + * Whether the form can be submitted. + * + * ⚠️ **Every dependency is read unconditionally, and form validity comes from + * a signal.** Both details are load-bearing, and getting either wrong + * deadlocks the button. + * + * A `computed` tracks the signals actually read during its *last* execution, + * so an early `return` shortens its dependency set. This began as a chain of + * guard clauses with `if (this.form.invalid) return false` near the top — + * and `FormGroup.invalid` is a plain getter, not a signal. On the first + * evaluation the form was empty, so it returned there having read only + * `isSubmitting()`; nothing else was tracked, no later edit could schedule a + * recompute, and `isSubmitting` only changes inside `onSubmit`, which the + * disabled button prevented. The submit button could never enable. + * + * So: mirror validity into `formValid` (fed by `statusChanges`), read every + * input before combining them, and never guard-clause out of this computed. + */ protected readonly canSubmit = computed(() => { - if (this.isSubmitting()) return false; - if (this.form.invalid) return false; - if (this.bodyOverLimit()) return false; - if (this.expiryMissing() || this.expiryBeforePublish()) return false; - if (this.ctaIncomplete()) return false; - if (!this.allRolesSig() && this.roles().length > 0 && this.selectedRoles().length === 0) { - return false; - } - return true; + const submitting = this.isSubmitting(); + const formValid = this.formValid(); + const overLimit = this.bodyOverLimit(); + const missingExpiry = this.expiryMissing(); + const badExpiry = this.expiryBeforePublish(); + const badCta = this.ctaIncomplete(); + const rolesChosen = + this.allRolesSig() || this.roles().length === 0 || this.selectedRoles().length > 0; + + return ( + !submitting && formValid && !overLimit && !missingExpiry && !badExpiry && + !badCta && rolesChosen + ); }); async ngOnInit(): Promise { @@ -547,6 +572,8 @@ export class AnnouncementFormPage implements OnInit { c.expires_at.valueChanges.subscribe(v => this.expiresAtSig.set(v)); c.cta_label.valueChanges.subscribe(v => this.ctaLabelSig.set(v)); c.cta_url.valueChanges.subscribe(v => this.ctaUrlSig.set(v)); + this.form.statusChanges.subscribe(status => this.formValid.set(status === 'VALID')); + this.formValid.set(this.form.valid); const id = this.route.snapshot.paramMap.get('id'); if (!id) return;