diff --git a/e2e/tests/recurring/repeat-custom-weekday-checkboxes-frozen-bug-8025.spec.ts b/e2e/tests/recurring/repeat-custom-weekday-checkboxes-frozen-bug-8025.spec.ts new file mode 100644 index 0000000000..696eba8c0e --- /dev/null +++ b/e2e/tests/recurring/repeat-custom-weekday-checkboxes-frozen-bug-8025.spec.ts @@ -0,0 +1,192 @@ +import { expect, test } from '../../fixtures/test.fixture'; +import { type Page } from '@playwright/test'; +import { openRecurDialog, saveRecurDialog } from '../../utils/recurring-task-helpers'; + +/** + * Bug: https://github.com/super-productivity/super-productivity/issues/8025 + * + * In the CUSTOM recurring config, switching the "Recur cycle" away from Weekly + * (to Month/Year) and back to Weekly leaves the weekday checkboxes rendered but + * non-interactive: they "look enabled but are actually disabled". A click no + * longer updates the model, so the user's weekday selection is silently lost. + * + * Expected: after the Week -> Month -> Week round-trip the weekday checkboxes + * remain fully interactive: un-checking Monday persists, exactly as + * it would without the round-trip. + * + * This test reproduces the defect by toggling Monday off AFTER the round-trip + * and asserting the change survives a save (read back from the NgRx store). The + * weekday group is configured with `resetOnHide: false`, so preservation is the + * intended design; the failure is the lost interactivity, not a deliberate + * reset. + */ + +const DIALOG = 'mat-dialog-container'; + +/** Switch the quick-setting select (first select in the dialog). */ +const setQuickSetting = async (page: Page, optionLabel: RegExp): Promise => { + const dialog = page.locator(DIALOG); + await dialog.locator('mat-select').first().click(); + const option = page.locator('mat-option').filter({ hasText: optionLabel }); + await expect(option).toBeVisible({ timeout: 5000 }); + await option.click(); + await expect(page.locator('mat-option')).toHaveCount(0, { timeout: 5000 }); +}; + +/** Switch the CUSTOM "Recur cycle" select (Day / Week / Month / Year). */ +const setRepeatCycle = async (page: Page, optionLabel: RegExp): Promise => { + const dialog = page.locator(DIALOG); + const cycleSelect = dialog.locator('.repeat-cycle mat-select').first(); + await expect(cycleSelect).toBeVisible({ timeout: 5000 }); + await cycleSelect.click(); + const option = page.locator('mat-option').filter({ hasText: optionLabel }); + await expect(option).toBeVisible({ timeout: 5000 }); + await option.click(); + await expect(page.locator('mat-option')).toHaveCount(0, { timeout: 5000 }); +}; + +type WeekdayKey = + | 'monday' + | 'tuesday' + | 'wednesday' + | 'thursday' + | 'friday' + | 'saturday' + | 'sunday'; + +type PersistedRepeatCfgSnapshot = { + quickSetting: string | null; + repeatCycle: string | null; + weekday: boolean | null; +}; + +/** Read a recurrence snapshot for the repeat config created for the given title. */ +const getPersistedRepeatCfgSnapshot = async ( + page: Page, + title: string, + weekday: WeekdayKey, +): Promise => + page.evaluate( + ({ taskTitle, weekdayKey }: { taskTitle: string; weekdayKey: WeekdayKey }) => { + type RepeatCfgLike = { + quickSetting?: string | null; + repeatCycle?: string | null; + title?: string | null; + } & Partial>; + type StoreState = { + taskRepeatCfg?: { entities?: Record }; + }; + type StoreLike = { + subscribe: (next: (s: StoreState) => void) => { unsubscribe: () => void }; + }; + const store = (window as unknown as { __e2eTestHelpers?: { store?: StoreLike } }) + .__e2eTestHelpers?.store; + if (!store) { + throw new Error('__e2eTestHelpers.store missing'); + } + let latest: StoreState | undefined; + store + .subscribe((s) => { + latest = s; + }) + .unsubscribe(); + const cfg = Object.values(latest?.taskRepeatCfg?.entities ?? {}).find((c) => + c?.title?.includes(taskTitle), + ); + return { + quickSetting: cfg?.quickSetting ?? null, + repeatCycle: cfg?.repeatCycle ?? null, + weekday: cfg ? (cfg[weekdayKey] ?? null) : null, + }; + }, + { taskTitle: title, weekdayKey: weekday }, + ); + +test.describe('Recurring Task - Custom weekday checkboxes frozen after cycle round-trip (#8025)', () => { + test('should keep weekday checkboxes interactive after switching cycle to Month and back to Week', async ({ + page, + workViewPage, + taskPage, + testPrefix, + }) => { + const taskTitle = `${testPrefix}-WeekdayFrozen8025`; + + await workViewPage.waitForTaskList(); + await workViewPage.addTask(taskTitle); + const task = taskPage.getTaskByText(taskTitle).first(); + await expect(task).toBeVisible({ timeout: 10000 }); + await taskPage.openTaskDetail(task); + const dialog = await openRecurDialog(page); + + // Enter CUSTOM mode; repeatCycle defaults to WEEKLY with Mon-Fri checked. + await setQuickSetting(page, /Custom recurring config/i); + + const mondayCheckbox = dialog + .locator('.weekdays mat-checkbox') + .filter({ hasText: /Monday/i }); + const mondayInput = mondayCheckbox.locator('input[type="checkbox"]'); + await expect(mondayInput).toBeChecked(); // sanity: default Weekly has Monday on + + // The round-trip that triggers the bug: Week -> Month -> Week. + await setRepeatCycle(page, /^Month$/); + await setRepeatCycle(page, /^Week$/); + + // After the round-trip the checkbox is re-rendered. Try to un-check Monday. + await expect(mondayCheckbox).toBeVisible({ timeout: 5000 }); + await mondayCheckbox.click(); + + // Save and read the persisted config back from the store. With the bug the + // click never reached the model, so `monday` is still `true`. + await saveRecurDialog(page); + + await expect + .poll(() => getPersistedRepeatCfgSnapshot(page, taskTitle, 'monday'), { + timeout: 10000, + }) + .toEqual({ + quickSetting: 'CUSTOM', + repeatCycle: 'WEEKLY', + weekday: false, + }); + }); + + test('should keep weekday checkboxes interactive after switching away from CUSTOM and back', async ({ + page, + workViewPage, + taskPage, + testPrefix, + }) => { + const taskTitle = `${testPrefix}-CustomRoundTrip8025`; + + await workViewPage.waitForTaskList(); + await workViewPage.addTask(taskTitle); + const task = taskPage.getTaskByText(taskTitle).first(); + await expect(task).toBeVisible({ timeout: 10000 }); + await taskPage.openTaskDetail(task); + const dialog = await openRecurDialog(page); + + await setQuickSetting(page, /Custom recurring config/i); + await setQuickSetting(page, /Every day/i); + await setQuickSetting(page, /Custom recurring config/i); + await setRepeatCycle(page, /^Week$/); + + const tuesdayCheckbox = dialog + .locator('.weekdays mat-checkbox') + .filter({ hasText: /Tuesday/i }); + const tuesdayInput = tuesdayCheckbox.locator('input[type="checkbox"]'); + await expect(tuesdayInput).toBeChecked(); + + await tuesdayCheckbox.click(); + await saveRecurDialog(page); + + await expect + .poll(() => getPersistedRepeatCfgSnapshot(page, taskTitle, 'tuesday'), { + timeout: 10000, + }) + .toEqual({ + quickSetting: 'CUSTOM', + repeatCycle: 'WEEKLY', + weekday: false, + }); + }); +}); diff --git a/src/app/features/task-repeat-cfg/dialog-edit-task-repeat-cfg/dialog-edit-task-repeat-cfg.component.html b/src/app/features/task-repeat-cfg/dialog-edit-task-repeat-cfg/dialog-edit-task-repeat-cfg.component.html index e33a7ca3ab..49ea85ebc1 100644 --- a/src/app/features/task-repeat-cfg/dialog-edit-task-repeat-cfg/dialog-edit-task-repeat-cfg.component.html +++ b/src/app/features/task-repeat-cfg/dialog-edit-task-repeat-cfg/dialog-edit-task-repeat-cfg.component.html @@ -32,6 +32,16 @@ [model]="repeatCfg()" > + @if (isWeekdaySelectionInvalid()) { + + } + { // which only happens after repeatCfgInitial is set })); }); + + describe('isWeekdaySelectionInvalid (issue #8025)', () => { + const baseCfg = { + ...DEFAULT_TASK_REPEAT_CFG, + monday: false, + tuesday: false, + wednesday: false, + thursday: false, + friday: false, + saturday: false, + sunday: false, + }; + + it('should be true for a CUSTOM weekly config with no weekday selected', async () => { + const fixture = await setupTestBed({ task: mockTask }); + const component = fixture.componentInstance; + + component.repeatCfg.set({ + ...baseCfg, + quickSetting: 'CUSTOM', + repeatCycle: 'WEEKLY', + }); + + expect(component.isWeekdaySelectionInvalid()).toBe(true); + }); + + it('should be false once at least one weekday is selected', async () => { + const fixture = await setupTestBed({ task: mockTask }); + const component = fixture.componentInstance; + + component.repeatCfg.set({ + ...baseCfg, + quickSetting: 'CUSTOM', + repeatCycle: 'WEEKLY', + wednesday: true, + }); + + expect(component.isWeekdaySelectionInvalid()).toBe(false); + }); + + it('should be false for non-weekly cycles even with no weekday selected', async () => { + const fixture = await setupTestBed({ task: mockTask }); + const component = fixture.componentInstance; + + component.repeatCfg.set({ + ...baseCfg, + quickSetting: 'CUSTOM', + repeatCycle: 'MONTHLY', + }); + + expect(component.isWeekdaySelectionInvalid()).toBe(false); + }); + + it('should be false for non-CUSTOM quick settings (e.g. DAILY)', async () => { + const fixture = await setupTestBed({ task: mockTask }); + const component = fixture.componentInstance; + + component.repeatCfg.set({ + ...baseCfg, + quickSetting: 'DAILY', + repeatCycle: 'WEEKLY', + }); + + expect(component.isWeekdaySelectionInvalid()).toBe(false); + }); + + it('should block direct save when a CUSTOM weekly config has no weekday selected', async () => { + const fixture = await setupTestBed({ task: mockTask }); + const component = fixture.componentInstance; + + component.repeatCfg.set({ + ...baseCfg, + quickSetting: 'CUSTOM', + repeatCycle: 'WEEKLY', + }); + + component.save(); + + expect(mockTaskRepeatCfgService.addTaskRepeatCfgToTask).not.toHaveBeenCalled(); + expect(mockTaskRepeatCfgService.updateTaskRepeatCfg).not.toHaveBeenCalled(); + expect(mockDialogRef.close).not.toHaveBeenCalled(); + }); + }); }); diff --git a/src/app/features/task-repeat-cfg/dialog-edit-task-repeat-cfg/dialog-edit-task-repeat-cfg.component.ts b/src/app/features/task-repeat-cfg/dialog-edit-task-repeat-cfg/dialog-edit-task-repeat-cfg.component.ts index f6440a4a52..b5063e3998 100644 --- a/src/app/features/task-repeat-cfg/dialog-edit-task-repeat-cfg/dialog-edit-task-repeat-cfg.component.ts +++ b/src/app/features/task-repeat-cfg/dialog-edit-task-repeat-cfg/dialog-edit-task-repeat-cfg.component.ts @@ -64,6 +64,18 @@ const RELEVANT_KEYS_FOR_UPDATE_ALL_TASKS: (keyof TaskRepeatCfgCopy)[] = [ 'tagIds', ]; +// A CUSTOM weekly recurrence with no weekday checked never produces an +// occurrence, so it must be blocked at save time (#8025). +const WEEKDAY_KEYS: (keyof TaskRepeatCfgCopy)[] = [ + 'monday', + 'tuesday', + 'wednesday', + 'thursday', + 'friday', + 'saturday', + 'sunday', +]; + // TASK_REPEAT_CFG_FORM_CFG @Component({ selector: 'dialog-edit-task-repeat-cfg', @@ -113,6 +125,17 @@ export class DialogEditTaskRepeatCfgComponent { return false; }); + // A CUSTOM weekly config with zero weekdays selected would never recur; + // surface it as a blocking validation error (#8025). Derived from the + // `repeatCfg` signal so it re-evaluates on every checkbox toggle. + isWeekdaySelectionInvalid = computed(() => { + const cfg = this.repeatCfg(); + if (cfg.quickSetting !== 'CUSTOM' || cfg.repeatCycle !== 'WEEKLY') { + return false; + } + return !WEEKDAY_KEYS.some((day) => cfg[day]); + }); + repeatCfgId = computed(() => { const cfg = this.repeatCfg(); if ('id' in cfg && cfg.id) { @@ -292,6 +315,11 @@ export class DialogEditTaskRepeatCfgComponent { return; } + // Enter-key submit bypasses the disabled Save button, so re-check here (#8025). + if (this.isWeekdaySelectionInvalid()) { + return; + } + const currentRepeatCfg = this.repeatCfg(); // workaround for formly not always updating hidden fields correctly (in time??) diff --git a/src/app/features/task-repeat-cfg/dialog-edit-task-repeat-cfg/task-repeat-cfg-form.const.spec.ts b/src/app/features/task-repeat-cfg/dialog-edit-task-repeat-cfg/task-repeat-cfg-form.const.spec.ts index 902972a41b..adc53f2583 100644 --- a/src/app/features/task-repeat-cfg/dialog-edit-task-repeat-cfg/task-repeat-cfg-form.const.spec.ts +++ b/src/app/features/task-repeat-cfg/dialog-edit-task-repeat-cfg/task-repeat-cfg-form.const.spec.ts @@ -73,4 +73,59 @@ describe('TaskRepeatCfgFormConfig', () => { expect(remindAtField?.templateOptions?.required).toBe(true); }); }); + + describe('weekdays group visibility (issue #8025)', () => { + const repeatContainer = TASK_REPEAT_CFG_ESSENTIAL_FORM_CFG.find( + (field) => field.fieldGroupClassName === 'repeat-config-container', + ); + const weekdaysGroup = repeatContainer?.fieldGroup?.find( + (field) => field.fieldGroupClassName === 'weekdays', + ); + const WEEKDAY_KEYS = [ + 'monday', + 'tuesday', + 'wednesday', + 'thursday', + 'friday', + 'saturday', + 'sunday', + ]; + + it('should locate the weekdays group', () => { + expect(weekdaysGroup).toBeDefined(); + expect(weekdaysGroup?.fieldGroup?.length).toBe(7); + }); + + // Regression guard for #8025: a `hideExpression` makes formly destroy and + // recreate the checkbox views on every cycle switch, which breaks their + // wiring to the FormControls (clicks stop updating the model after a + // Week -> Month -> Week round-trip). Visibility must be driven by CSS instead. + it('should NOT use hideExpression (it would re-freeze the checkboxes)', () => { + expect(weekdaysGroup?.hideExpression).toBeUndefined(); + }); + + it('should toggle visibility via a dynamic className bound to repeatCycle', () => { + const className = weekdaysGroup?.expressionProperties?.['className'] as (model: { + repeatCycle: string; + }) => string; + expect(className).toEqual(jasmine.any(Function)); + expect(className({ repeatCycle: 'WEEKLY' })).toBe(''); + expect(className({ repeatCycle: 'MONTHLY' })).toBe('repeat-cfg-hidden'); + expect(className({ repeatCycle: 'YEARLY' })).toBe('repeat-cfg-hidden'); + expect(className({ repeatCycle: 'DAILY' })).toBe('repeat-cfg-hidden'); + }); + + // The group stays mounted, but the CUSTOM container above it still hides via + // hideExpression. Each checkbox needs resetOnHide:false so the selection + // survives a quickSetting != CUSTOM round-trip instead of being wiped. + it('should keep every weekday checkbox value across hide (resetOnHide:false)', () => { + WEEKDAY_KEYS.forEach((key) => { + const field = weekdaysGroup?.fieldGroup?.find((f) => f.key === key); + expect(field).withContext(`weekday field "${key}" exists`).toBeDefined(); + expect(field?.resetOnHide) + .withContext(`weekday field "${key}" resetOnHide`) + .toBe(false); + }); + }); + }); }); diff --git a/src/app/features/task-repeat-cfg/dialog-edit-task-repeat-cfg/task-repeat-cfg-form.const.ts b/src/app/features/task-repeat-cfg/dialog-edit-task-repeat-cfg/task-repeat-cfg-form.const.ts index d7d9e49ed9..3fccc6d28d 100644 --- a/src/app/features/task-repeat-cfg/dialog-edit-task-repeat-cfg/task-repeat-cfg-form.const.ts +++ b/src/app/features/task-repeat-cfg/dialog-edit-task-repeat-cfg/task-repeat-cfg-form.const.ts @@ -143,13 +143,25 @@ export const TASK_REPEAT_CFG_ESSENTIAL_FORM_CFG: FormlyFieldConfig[] = [ ], }, { + // Hide via a dynamic CSS class instead of `hideExpression`. With formly's + // default `lazyRender`, hiding a field group destroys its child views and + // recreates them on re-show, and the recreated mat-checkboxes lose their + // wiring to the (re-registered) FormControls. After a cycle round-trip + // (Week -> Month -> Week) the checkboxes then look enabled but are inert: + // clicks no longer update the model (#8025). Keeping the group mounted and + // toggling only its visibility preserves the control/view binding. + // `resetOnHide: false` on each checkbox keeps the selection when the + // CUSTOM container itself is hidden (quickSetting != CUSTOM). fieldGroupClassName: 'weekdays', - resetOnHide: false, - hideExpression: (model: any) => model.repeatCycle !== 'WEEKLY', + expressionProperties: { + className: (model: TaskRepeatCfg) => + model.repeatCycle === 'WEEKLY' ? '' : 'repeat-cfg-hidden', + }, fieldGroup: [ { key: 'monday', type: 'checkbox', + resetOnHide: false, templateOptions: { label: T.F.TASK_REPEAT.F.MONDAY, }, @@ -157,6 +169,7 @@ export const TASK_REPEAT_CFG_ESSENTIAL_FORM_CFG: FormlyFieldConfig[] = [ { key: 'tuesday', type: 'checkbox', + resetOnHide: false, templateOptions: { label: T.F.TASK_REPEAT.F.TUESDAY, }, @@ -164,6 +177,7 @@ export const TASK_REPEAT_CFG_ESSENTIAL_FORM_CFG: FormlyFieldConfig[] = [ { key: 'wednesday', type: 'checkbox', + resetOnHide: false, templateOptions: { label: T.F.TASK_REPEAT.F.WEDNESDAY, }, @@ -171,6 +185,7 @@ export const TASK_REPEAT_CFG_ESSENTIAL_FORM_CFG: FormlyFieldConfig[] = [ { key: 'thursday', type: 'checkbox', + resetOnHide: false, templateOptions: { label: T.F.TASK_REPEAT.F.THURSDAY, }, @@ -178,6 +193,7 @@ export const TASK_REPEAT_CFG_ESSENTIAL_FORM_CFG: FormlyFieldConfig[] = [ { key: 'friday', type: 'checkbox', + resetOnHide: false, templateOptions: { label: T.F.TASK_REPEAT.F.FRIDAY, }, @@ -185,6 +201,7 @@ export const TASK_REPEAT_CFG_ESSENTIAL_FORM_CFG: FormlyFieldConfig[] = [ { key: 'saturday', type: 'checkbox', + resetOnHide: false, templateOptions: { label: T.F.TASK_REPEAT.F.SATURDAY, }, @@ -192,6 +209,7 @@ export const TASK_REPEAT_CFG_ESSENTIAL_FORM_CFG: FormlyFieldConfig[] = [ { key: 'sunday', type: 'checkbox', + resetOnHide: false, templateOptions: { label: T.F.TASK_REPEAT.F.SUNDAY, }, diff --git a/src/app/t.const.ts b/src/app/t.const.ts index dd1d591829..3b9b45e8ec 100644 --- a/src/app/t.const.ts +++ b/src/app/t.const.ts @@ -1977,6 +1977,7 @@ const T = { WEDNESDAY: 'F.TASK_REPEAT.F.WEDNESDAY', WEEK_OF_MONTH: 'F.TASK_REPEAT.F.WEEK_OF_MONTH', WEEKDAY: 'F.TASK_REPEAT.F.WEEKDAY', + WEEKDAY_REQUIRED: 'F.TASK_REPEAT.F.WEEKDAY_REQUIRED', }, SNACK_REPEAT_DIALOG_FAIL: 'F.TASK_REPEAT.SNACK_REPEAT_DIALOG_FAIL', }, diff --git a/src/assets/i18n/en.json b/src/assets/i18n/en.json index 5e6f57a278..4c731fe2c5 100644 --- a/src/assets/i18n/en.json +++ b/src/assets/i18n/en.json @@ -1923,7 +1923,8 @@ "WAIT_FOR_COMPLETION_DESCRIPTION": "Prevents multiple pending recurring tasks from piling up. The next instance will only be created once the current task is completed", "WEDNESDAY": "Wednesday", "WEEK_OF_MONTH": "Week of month", - "WEEKDAY": "Weekday" + "WEEKDAY": "Weekday", + "WEEKDAY_REQUIRED": "Select at least one weekday" }, "SNACK_REPEAT_DIALOG_FAIL": "Could not open repeat config dialog. You can configure it from the task menu." },