mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-17 16:37:43 +00:00
* fix(planner): don't erase day-scheduled subtasks on parent plan (#9019) Planning a parent task ran removeTasksFromPlannerDays(task.subTaskIds), which stripped its subtasks from EVERY planner day. As a result, any day-scheduling the user had given a subtask was silently wiped the moment its parent was planned anywhere, so those subtasks disappeared from the planner and schedule (timed subtasks survived via a separate, plannerDays- independent path). handlePlanTaskForDay now collapses the parent's subtasks on the target day only, matching the target-day-only behaviour handleTransferTask already had. Subtasks scheduled for other days keep their scheduling and stay visible. Add removeTasksFromSinglePlannerDay helper (reuses removeTasksFromList) for this; the explicit "SinglePlannerDay" name keeps it from being confused with the all-days removeTasksFromPlannerDays (the mix-up that caused the bug). Same-day subtasks still fold into the parent by design: showing them as siblings would double-count their time (the parent estimate rolls up its subtasks) in the day total and on the schedule timeline. No schema barrier: the op payload and planner.days/dueDay shapes are unchanged, so this is a pure reducer behaviour fix, not a new payload semantic (op-log rules 2.5). In a mixed-version fleet a not-yet-upgraded client still applies the old all-days removal for these ops, but the task's dueDay is preserved either way and state re-converges once clients upgrade. * test(planner): e2e for day-scheduled subtask visibility (#9019) Drives the real UI: add a task + subtask, schedule the subtask for a day (day-only, via the 'S' shortcut + quick-access Tomorrow), plan the parent for a different day, then assert the subtask is still visible on its own day in the planner. Fails against the pre-fix all-days removal, passes with the target-day-only fix (both verified locally).
This commit is contained in:
parent
a4dee9d5f7
commit
0923fe66b2
4 changed files with 169 additions and 4 deletions
|
|
@ -0,0 +1,84 @@
|
|||
import type { Locator, Page } from '@playwright/test';
|
||||
import { expect, test } from '../../fixtures/test.fixture';
|
||||
|
||||
const QUICK_ACCESS_BUTTON = '.quick-access button';
|
||||
|
||||
/**
|
||||
* Schedule a task for a quick-access day (day-only, no time) via the keyboard
|
||||
* shortcut ('S' opens the schedule dialog for the focused task) and the
|
||||
* dialog's quick-access buttons (Today=0, Tomorrow=1, Next Week=2, Next
|
||||
* Month=3). onQuickAccessClick auto-submits, so clicking a day both sets the
|
||||
* due day and closes the dialog.
|
||||
*
|
||||
* The keyboard path is used deliberately: it is independent of whether the task
|
||||
* is already scheduled (a scheduled task's detail item shows a "Reschedule"
|
||||
* control instead of "Schedule Task") and of task height (hovering a parent
|
||||
* with sub-tasks lands on a sub-task row).
|
||||
*/
|
||||
const scheduleTaskForQuickDay = async (
|
||||
page: Page,
|
||||
task: Locator,
|
||||
quickAccessIndex: number,
|
||||
): Promise<void> => {
|
||||
await task.scrollIntoViewIfNeeded();
|
||||
await task.focus();
|
||||
await expect(task).toBeFocused();
|
||||
|
||||
await page.keyboard.press('s');
|
||||
|
||||
const scheduleDialog = page.locator('dialog-schedule-task');
|
||||
await expect(scheduleDialog).toBeVisible({ timeout: 10000 });
|
||||
// No time set -> this creates a date-only dueDay, not a timed dueWithTime.
|
||||
await expect(scheduleDialog.locator('input[type="time"]')).toHaveValue('');
|
||||
|
||||
await scheduleDialog.locator(QUICK_ACCESS_BUTTON).nth(quickAccessIndex).click();
|
||||
await scheduleDialog.waitFor({ state: 'hidden', timeout: 10000 });
|
||||
};
|
||||
|
||||
const getTomorrowDateString = (): string => {
|
||||
const tomorrow = new Date();
|
||||
tomorrow.setDate(tomorrow.getDate() + 1);
|
||||
|
||||
const year = tomorrow.getFullYear();
|
||||
const month = String(tomorrow.getMonth() + 1).padStart(2, '0');
|
||||
const day = String(tomorrow.getDate()).padStart(2, '0');
|
||||
return `${year}-${month}-${day}`;
|
||||
};
|
||||
|
||||
test.describe('Planner: scheduled subtask visibility (#9019)', () => {
|
||||
test('keeps a date-only subtask visible on its own day after planning its parent', async ({
|
||||
page,
|
||||
plannerPage,
|
||||
taskPage,
|
||||
workViewPage,
|
||||
}) => {
|
||||
await workViewPage.waitForTaskList();
|
||||
await workViewPage.addTask('Parent 9019');
|
||||
|
||||
const parentTask = taskPage.getTaskByText('Parent 9019').first();
|
||||
await expect(parentTask).toBeVisible();
|
||||
await workViewPage.addSubTask(parentTask, 'Sub 9019');
|
||||
|
||||
const subTask = taskPage
|
||||
.getSubTasks(parentTask)
|
||||
.filter({ hasText: 'Sub 9019' })
|
||||
.first();
|
||||
await expect(subTask).toBeVisible();
|
||||
|
||||
// Schedule the SUBTASK for tomorrow (day-only).
|
||||
await scheduleTaskForQuickDay(page, subTask, 1);
|
||||
|
||||
// Plan the PARENT for a different day (next week). Before #9019 this removed
|
||||
// the subtask from EVERY planner day, so it vanished from the planner.
|
||||
await scheduleTaskForQuickDay(page, parentTask, 2);
|
||||
|
||||
await plannerPage.navigateToPlanner();
|
||||
await plannerPage.waitForPlannerView();
|
||||
|
||||
const subTaskDay = page.locator(`planner-day[data-day="${getTomorrowDateString()}"]`);
|
||||
await expect(subTaskDay).toBeVisible({ timeout: 15000 });
|
||||
await expect(
|
||||
subTaskDay.locator('planner-task').filter({ hasText: 'Sub 9019' }),
|
||||
).toBeVisible({ timeout: 15000 });
|
||||
});
|
||||
});
|
||||
|
|
@ -83,6 +83,26 @@ describe('plannerSharedMetaReducer', () => {
|
|||
expect(resultState.planner.days['2099-12-31']).toContain('task1');
|
||||
});
|
||||
|
||||
it('should not touch a sub-task scheduled for a different day when transferring the parent (#9019)', () => {
|
||||
const todayStr = getDbDateStr();
|
||||
const testState = createStateWithExistingTasks([], [], [], []);
|
||||
testState.planner = {
|
||||
...testState.planner,
|
||||
days: {
|
||||
'2024-01-16': ['sub1'], // sub scheduled for its own day
|
||||
'2024-01-20': ['other'],
|
||||
},
|
||||
};
|
||||
const task = createMockTask({ id: 'parent', subTaskIds: ['sub1', 'sub2'] });
|
||||
// parent is transferred onto a different day than the sub-task
|
||||
const action = createTransferTaskAction(task, todayStr, 0, '2024-01-20', todayStr);
|
||||
|
||||
metaReducer(testState, action);
|
||||
const resultState = mockReducer.calls.mostRecent().args[0];
|
||||
expect(resultState.planner.days['2024-01-16']).toEqual(['sub1']);
|
||||
expect(resultState.planner.days['2024-01-20']).toEqual(['parent', 'other']);
|
||||
});
|
||||
|
||||
it('should update task.dueDay when transferring from today to different day', () => {
|
||||
const todayStr = getDbDateStr();
|
||||
const newDay = '2026-01-15';
|
||||
|
|
@ -366,6 +386,27 @@ describe('plannerSharedMetaReducer', () => {
|
|||
]);
|
||||
});
|
||||
|
||||
it('should only collapse sub-tasks on the target day, not erase them from other days (#9019)', () => {
|
||||
const testState = createStateWithExistingTasks([], [], [], []);
|
||||
// sub1 is scheduled for a *different* day than the parent is planned for
|
||||
testState.planner = {
|
||||
...testState.planner,
|
||||
days: {
|
||||
'2024-01-16': ['sub1'],
|
||||
'2024-01-20': ['other'],
|
||||
},
|
||||
};
|
||||
const task = createMockTask({ id: 'parent', subTaskIds: ['sub1', 'sub2'] });
|
||||
const action = createPlanTaskForDayAction(task, '2024-01-20', false);
|
||||
|
||||
metaReducer(testState, action);
|
||||
const resultState = mockReducer.calls.mostRecent().args[0];
|
||||
// sub1 stays on its own day (2024-01-16); the parent lands on 2024-01-20.
|
||||
// Before the fix, planning the parent wiped sub1 from ALL days.
|
||||
expect(resultState.planner.days['2024-01-16']).toEqual(['sub1']);
|
||||
expect(resultState.planner.days['2024-01-20']).toEqual(['other', 'parent']);
|
||||
});
|
||||
|
||||
// Virtual tag pattern tests: TODAY_TAG membership is determined by task.dueDay,
|
||||
// NOT by task.tagIds. TODAY_TAG should NEVER be in task.tagIds.
|
||||
// See: docs/ai/today-tag-architecture.md
|
||||
|
|
|
|||
|
|
@ -13,7 +13,7 @@ import {
|
|||
getTag,
|
||||
hasInvalidTodayTag,
|
||||
removeTaskFromPlannerDays,
|
||||
removeTasksFromPlannerDays,
|
||||
removeTasksFromSinglePlannerDay,
|
||||
updateTags,
|
||||
} from './task-shared-helpers';
|
||||
import {
|
||||
|
|
@ -84,7 +84,8 @@ const handleTransferTask = (
|
|||
task.id,
|
||||
...targetDays.slice(targetIndex),
|
||||
])
|
||||
// when moving a parent to the day, remove all sub-tasks
|
||||
// when moving a parent to the day, collapse its sub-tasks under it on
|
||||
// THIS day only (other days keep their own scheduling — see #9019)
|
||||
.filter((id: string) => !task.subTaskIds.includes(id));
|
||||
}
|
||||
|
||||
|
|
@ -247,9 +248,12 @@ const handlePlanTaskForDay = (
|
|||
// Add to target day if not today (today's ordering is managed by TODAY_TAG.taskIds)
|
||||
if (day !== todayStr) {
|
||||
state = addTaskToPlannerDay(state, task.id, day, isAddToTop ? 0 : Infinity);
|
||||
// When moving a parent, remove sub-tasks from the target day
|
||||
// Collapse the parent's sub-tasks under it for THIS day only. Previously we
|
||||
// removed them from ALL planner days, which silently erased any day the user
|
||||
// had scheduled a sub-task for once its parent was planned anywhere (#9019).
|
||||
// Matches the target-day-only behaviour of handleTransferTask.
|
||||
if (task.subTaskIds.length > 0) {
|
||||
state = removeTasksFromPlannerDays(state, task.subTaskIds);
|
||||
state = removeTasksFromSinglePlannerDay(state, task.subTaskIds, day);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -423,6 +423,42 @@ export const removeTasksFromPlannerDays = (
|
|||
};
|
||||
};
|
||||
|
||||
/**
|
||||
* Removes multiple tasks from a SINGLE planner day, leaving all other days
|
||||
* intact. Distinct from `removeTasksFromPlannerDays` (all days) — the scope
|
||||
* difference is data-loss-sensitive, hence the explicit name.
|
||||
* @param state Root state
|
||||
* @param taskIds Task IDs to remove
|
||||
* @param day Day (date string) to remove them from
|
||||
* @returns Updated state, or original state if no changes
|
||||
*/
|
||||
export const removeTasksFromSinglePlannerDay = (
|
||||
state: RootState,
|
||||
taskIds: string[],
|
||||
day: string,
|
||||
): RootState => {
|
||||
const dayTasks = state.planner?.days?.[day];
|
||||
if (!dayTasks || taskIds.length === 0) {
|
||||
return state;
|
||||
}
|
||||
|
||||
const filtered = removeTasksFromList(dayTasks, taskIds);
|
||||
if (filtered.length === dayTasks.length) {
|
||||
return state;
|
||||
}
|
||||
|
||||
return {
|
||||
...state,
|
||||
[plannerFeatureKey]: {
|
||||
...state.planner,
|
||||
days: {
|
||||
...state.planner.days,
|
||||
[day]: filtered,
|
||||
},
|
||||
},
|
||||
};
|
||||
};
|
||||
|
||||
/**
|
||||
* Adds a task to a specific planner day.
|
||||
* Removes the task from all other days first (task can only be in one day).
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue