mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-18 08:56:41 +00:00
fix(task-view-customizer): scope Group By to current work context (#7279)
Group By: Tag / Group By: Project pulled tasks from every project when
used inside a specific project view, because de1b5d485c (fix for #7050)
deliberately switched the source to all undone tasks for these two
options. That closed #7050 but broke the more common case reported in
#7279 (and its duplicate #7290).
- TaskViewCustomizerService: revert customizeUndoneTasks() to the work
context's undoneTasks$, drop _allUndoneTasksWithSubTasks$ and the
cross-store selector/switchMap.
- Hide Group By: Project in a PROJECT context (it would collapse into
one group) via a new availableGroupOptions computed signal backed by
isActiveWorkContextProject$.
- Sanitize any stored group=project state inside the service's
context-load subscription, not from a lazy panel effect — the panel
is only instantiated when the mobile bottom-sheet customizer opens.
- Drop the now-redundant cross-context selection guard in
WorkViewComponent and retire its #7269 test block: after this fix
customizedUndoneTasks.list is always a subset of undoneTasks, so the
scenario those tests constructed can no longer occur.
- Stub isActiveWorkContextProject$ in all WorkContextService test
doubles so the service's toSignal() call has something to subscribe
to.
Closes #7290.
This commit is contained in:
Johannes Millan
parent
ecba6c4240
commit
e9ee2c4e45
5 changed files with 97 additions and 125 deletions
|
|
@ -99,7 +99,7 @@
|
|||
|
||||
<!-- Group submenu -->
|
||||
<mat-menu #groupMenu="matMenu">
|
||||
@for (opt of OPTIONS.group.list; track opt.type) {
|
||||
@for (opt of customizerService.availableGroupOptions(); track opt.type) {
|
||||
<button
|
||||
mat-menu-item
|
||||
(click)="customizerService.setGroup(opt)"
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import { selectAllTags } from '../tag/store/tag.reducer';
|
|||
import { getTomorrow } from '../../util/get-tomorrow';
|
||||
import { getDbDateStr } from '../../util/get-db-date-str';
|
||||
import { BehaviorSubject, Observable, of } from 'rxjs';
|
||||
import { map } from 'rxjs/operators';
|
||||
import { WorkContextType } from '../work-context/work-context.model';
|
||||
import { WorkContextService } from '../work-context/work-context.service';
|
||||
import { selectAllTasksWithSubTasks } from '../tasks/store/task.selectors';
|
||||
|
|
@ -41,6 +42,7 @@ describe('TaskViewCustomizerService', () => {
|
|||
activeId: string;
|
||||
activeType: WorkContextType;
|
||||
}>;
|
||||
isActiveWorkContextProject$: Observable<boolean>;
|
||||
mainListTasks$: Observable<TaskWithSubTasks[]>;
|
||||
undoneTasks$: Observable<TaskWithSubTasks[]>;
|
||||
};
|
||||
|
|
@ -133,6 +135,7 @@ describe('TaskViewCustomizerService', () => {
|
|||
activeId: 'TODAY',
|
||||
activeType: WorkContextType.TAG,
|
||||
}),
|
||||
isActiveWorkContextProject$: of(false),
|
||||
mainListTasks$: of<TaskWithSubTasks[]>([]),
|
||||
undoneTasks$: of<TaskWithSubTasks[]>([]),
|
||||
};
|
||||
|
|
@ -1035,6 +1038,9 @@ describe('TaskViewCustomizerService', () => {
|
|||
activeWorkContextId: null,
|
||||
activeWorkContextType: null,
|
||||
activeWorkContextTypeAndId$: ctx$,
|
||||
isActiveWorkContextProject$: ctx$.pipe(
|
||||
map(({ activeType }) => activeType === WorkContextType.PROJECT),
|
||||
),
|
||||
mainListTasks$: of<TaskWithSubTasks[]>([]),
|
||||
undoneTasks$: of<TaskWithSubTasks[]>([]),
|
||||
},
|
||||
|
|
@ -1148,12 +1154,12 @@ describe('TaskViewCustomizerService', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('customizeUndoneTasks with group by project (issue #7050)', () => {
|
||||
const inboxTask: TaskWithSubTasks = {
|
||||
id: 'inbox-task',
|
||||
title: 'Inbox Task',
|
||||
tagIds: [],
|
||||
projectId: 'INBOX_PROJECT',
|
||||
describe('customizeUndoneTasks respects current work context (issue #7279)', () => {
|
||||
const projectATask: TaskWithSubTasks = {
|
||||
id: 'project-a-task',
|
||||
title: 'Project A Task',
|
||||
tagIds: ['Tag A'],
|
||||
projectId: 'Project A',
|
||||
timeEstimate: 0,
|
||||
timeSpentOnDay: {},
|
||||
created: 1,
|
||||
|
|
@ -1164,11 +1170,11 @@ describe('TaskViewCustomizerService', () => {
|
|||
attachments: [],
|
||||
} as TaskWithSubTasks;
|
||||
|
||||
const projectATask: TaskWithSubTasks = {
|
||||
id: 'project-a-task',
|
||||
title: 'Project A Task',
|
||||
tagIds: [],
|
||||
projectId: 'Project A',
|
||||
const projectBTask: TaskWithSubTasks = {
|
||||
id: 'project-b-task',
|
||||
title: 'Project B Task',
|
||||
tagIds: ['Tag B'],
|
||||
projectId: 'Project B',
|
||||
timeEstimate: 0,
|
||||
timeSpentOnDay: {},
|
||||
created: 2,
|
||||
|
|
@ -1179,23 +1185,7 @@ describe('TaskViewCustomizerService', () => {
|
|||
attachments: [],
|
||||
} as TaskWithSubTasks;
|
||||
|
||||
const projectBTask: TaskWithSubTasks = {
|
||||
id: 'project-b-task',
|
||||
title: 'Project B Task',
|
||||
tagIds: [],
|
||||
projectId: 'Project B',
|
||||
timeEstimate: 0,
|
||||
timeSpentOnDay: {},
|
||||
created: 3,
|
||||
subTasks: [],
|
||||
subTaskIds: [],
|
||||
timeSpent: 0,
|
||||
isDone: false,
|
||||
attachments: [],
|
||||
} as TaskWithSubTasks;
|
||||
|
||||
const allProjects: Project[] = [
|
||||
{ id: 'INBOX_PROJECT', title: 'Inbox', backlogTaskIds: [] } as unknown as Project,
|
||||
{ id: 'Project A', title: 'Project A', backlogTaskIds: [] } as unknown as Project,
|
||||
{ id: 'Project B', title: 'Project B', backlogTaskIds: [] } as unknown as Project,
|
||||
];
|
||||
|
|
@ -1211,14 +1201,15 @@ describe('TaskViewCustomizerService', () => {
|
|||
});
|
||||
|
||||
mockWorkContextService = {
|
||||
activeWorkContextId: 'INBOX_PROJECT',
|
||||
activeWorkContextId: 'Project A',
|
||||
activeWorkContextType: WorkContextType.PROJECT,
|
||||
activeWorkContextTypeAndId$: of({
|
||||
activeId: 'INBOX_PROJECT',
|
||||
activeId: 'Project A',
|
||||
activeType: WorkContextType.PROJECT,
|
||||
}),
|
||||
mainListTasks$: of<TaskWithSubTasks[]>([inboxTask]),
|
||||
undoneTasks$: of<TaskWithSubTasks[]>([inboxTask]),
|
||||
isActiveWorkContextProject$: of(true),
|
||||
mainListTasks$: of<TaskWithSubTasks[]>([projectATask]),
|
||||
undoneTasks$: of<TaskWithSubTasks[]>([projectATask]),
|
||||
};
|
||||
|
||||
TestBed.configureTestingModule({
|
||||
|
|
@ -1242,7 +1233,7 @@ describe('TaskViewCustomizerService', () => {
|
|||
{ selector: selectAllTags, value: mockTags },
|
||||
{
|
||||
selector: selectAllTasksWithSubTasks,
|
||||
value: [inboxTask, projectATask, projectBTask],
|
||||
value: [projectATask, projectBTask],
|
||||
},
|
||||
],
|
||||
}),
|
||||
|
|
@ -1253,30 +1244,46 @@ describe('TaskViewCustomizerService', () => {
|
|||
(testService as any)._allTags = mockTags;
|
||||
});
|
||||
|
||||
it('should show tasks from ALL projects when group by project is selected, not just current context', (done) => {
|
||||
// Simulate being on Inbox — undoneTasks$ only has the inbox task
|
||||
const contextUndoneTasks$ = of<TaskWithSubTasks[]>([inboxTask]);
|
||||
it('should only group tasks from the current project when group by tag is selected', (done) => {
|
||||
// User is in Project A — undoneTasks$ only has Project A's task
|
||||
const contextUndoneTasks$ = of<TaskWithSubTasks[]>([projectATask]);
|
||||
|
||||
// Set group to project
|
||||
testService.setGroup({ type: GROUP_OPTION_TYPE.project, label: 'Project' });
|
||||
testService.setGroup({ type: GROUP_OPTION_TYPE.tag, label: 'Tag' });
|
||||
|
||||
const result$ = TestBed.runInInjectionContext(() =>
|
||||
testService.customizeUndoneTasks(contextUndoneTasks$),
|
||||
);
|
||||
|
||||
requestAnimationFrame(() => {
|
||||
result$.subscribe((result) => {
|
||||
expect(result.grouped).toBeDefined();
|
||||
// Project B's task must NOT leak into the view
|
||||
expect(result.list.map((t) => t.id)).toEqual(['project-a-task']);
|
||||
expect(Object.keys(result.grouped!)).toEqual(['Tag A']);
|
||||
expect(result.grouped!['Tag A']?.length).toBe(1);
|
||||
expect(result.grouped!['Tag A']?.[0].id).toBe('project-a-task');
|
||||
done();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
it('should only group tasks from the current project when group by project is selected', (done) => {
|
||||
// Edge case: even though the panel hides this option in a project context,
|
||||
// the service must still scope to the current context if it's set.
|
||||
const contextUndoneTasks$ = of<TaskWithSubTasks[]>([projectATask]);
|
||||
|
||||
testService.setGroup({ type: GROUP_OPTION_TYPE.project, label: 'Project' });
|
||||
|
||||
// customizeUndoneTasks uses toObservable which requires injection context
|
||||
const result$ = TestBed.runInInjectionContext(() =>
|
||||
testService.customizeUndoneTasks(contextUndoneTasks$),
|
||||
);
|
||||
|
||||
// Use requestAnimationFrame since customizeUndoneTasks uses animationFrameScheduler
|
||||
requestAnimationFrame(() => {
|
||||
result$.subscribe((result) => {
|
||||
expect(result.grouped).toBeDefined();
|
||||
const groupKeys = Object.keys(result.grouped!);
|
||||
// Should contain tasks from ALL projects, not just Inbox
|
||||
expect(groupKeys).toContain('Inbox');
|
||||
expect(groupKeys).toContain('Project A');
|
||||
expect(groupKeys).toContain('Project B');
|
||||
expect(result.grouped!['Inbox']?.length).toBe(1);
|
||||
expect(result.grouped!['Project A']?.length).toBe(1);
|
||||
expect(result.grouped!['Project B']?.length).toBe(1);
|
||||
expect(groupKeys).toEqual(['Project A']);
|
||||
expect(groupKeys).not.toContain('Project B');
|
||||
done();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,14 +1,13 @@
|
|||
import { Injectable, signal, inject, effect } from '@angular/core';
|
||||
import { Observable, animationFrameScheduler, combineLatest } from 'rxjs';
|
||||
import { map, observeOn, switchMap, take } from 'rxjs/operators';
|
||||
import { map, observeOn, take } from 'rxjs/operators';
|
||||
import { TaskWithSubTasks } from '../tasks/task.model';
|
||||
import { selectAllProjects } from '../project/store/project.selectors';
|
||||
import { selectAllTasksWithSubTasks } from '../tasks/store/task.selectors';
|
||||
import { selectAllTags } from './../tag/store/tag.reducer';
|
||||
import { Store } from '@ngrx/store';
|
||||
import { Project } from '../project/project.model';
|
||||
import { Tag } from '../tag/tag.model';
|
||||
import { takeUntilDestroyed, toObservable } from '@angular/core/rxjs-interop';
|
||||
import { takeUntilDestroyed, toObservable, toSignal } from '@angular/core/rxjs-interop';
|
||||
import { computed } from '@angular/core';
|
||||
import { getDbDateStr } from '../../util/get-db-date-str';
|
||||
import { getWeekRange } from '../../util/get-week-range';
|
||||
|
|
@ -28,6 +27,7 @@ import {
|
|||
FILTER_SCHEDULE,
|
||||
SORT_ORDER,
|
||||
FILTER_COMMON,
|
||||
OPTIONS,
|
||||
} from './types';
|
||||
import { DateAdapter } from '@angular/material/core';
|
||||
import { lsGetJSON, lsSetJSON } from '../../util/ls-util';
|
||||
|
|
@ -36,6 +36,10 @@ import { LanguageService } from 'src/app/core/language/language.service';
|
|||
import { TranslateService } from '@ngx-translate/core';
|
||||
import { T } from '../../t.const';
|
||||
|
||||
const GROUP_OPTIONS_NO_PROJECT = OPTIONS.group.list.filter(
|
||||
(opt) => opt.type !== GROUP_OPTION_TYPE.project,
|
||||
);
|
||||
|
||||
@Injectable({ providedIn: 'root' })
|
||||
export class TaskViewCustomizerService {
|
||||
private store = inject(Store);
|
||||
|
|
@ -60,6 +64,16 @@ export class TaskViewCustomizerService {
|
|||
].some((x) => x !== null);
|
||||
});
|
||||
|
||||
private _isInProject = toSignal(this._workContextService.isActiveWorkContextProject$, {
|
||||
initialValue: false,
|
||||
});
|
||||
|
||||
// "Group By: Project" would collapse into a single group inside a single-project
|
||||
// view, so offer it only for tag contexts. See issue #7279.
|
||||
availableGroupOptions = computed(() =>
|
||||
this._isInProject() ? GROUP_OPTIONS_NO_PROJECT : OPTIONS.group.list,
|
||||
);
|
||||
|
||||
private _stateByContext: Record<string, CustomizerContextState> = lsGetJSON<
|
||||
Record<string, CustomizerContextState>
|
||||
>(LS.TASK_VIEW_CUSTOMIZER_BY_CONTEXT, {});
|
||||
|
|
@ -77,7 +91,7 @@ export class TaskViewCustomizerService {
|
|||
this._currentContextKey = `${activeType}:${activeId}`;
|
||||
const stored = this._stateByContext[this._currentContextKey];
|
||||
this.selectedSort.set(stored?.sort ?? DEFAULT_OPTIONS.sort);
|
||||
this.selectedGroup.set(stored?.group ?? DEFAULT_OPTIONS.group);
|
||||
this.selectedGroup.set(this._sanitizeGroupForContext(stored?.group, activeType));
|
||||
this.selectedFilter.set(stored?.filter ?? DEFAULT_OPTIONS.filter);
|
||||
});
|
||||
|
||||
|
|
@ -123,24 +137,28 @@ export class TaskViewCustomizerService {
|
|||
}
|
||||
}
|
||||
|
||||
private _sanitizeGroupForContext(
|
||||
stored: GroupOption | undefined,
|
||||
activeType: WorkContextType,
|
||||
): GroupOption {
|
||||
if (!stored) return DEFAULT_OPTIONS.group;
|
||||
if (
|
||||
activeType === WorkContextType.PROJECT &&
|
||||
stored.type === GROUP_OPTION_TYPE.project
|
||||
) {
|
||||
return DEFAULT_OPTIONS.group;
|
||||
}
|
||||
return stored;
|
||||
}
|
||||
|
||||
customizeUndoneTasks(undoneTasks$: Observable<TaskWithSubTasks[]>): Observable<{
|
||||
list: TaskWithSubTasks[];
|
||||
grouped?: Record<string, TaskWithSubTasks[]>;
|
||||
}> {
|
||||
const group$ = toObservable(this.selectedGroup);
|
||||
const tasks$ = group$.pipe(
|
||||
switchMap((group) => {
|
||||
const isCrossContext =
|
||||
group.type === GROUP_OPTION_TYPE.project ||
|
||||
group.type === GROUP_OPTION_TYPE.tag;
|
||||
return isCrossContext ? this._allUndoneTasksWithSubTasks$() : undoneTasks$;
|
||||
}),
|
||||
);
|
||||
|
||||
return combineLatest([
|
||||
tasks$,
|
||||
undoneTasks$,
|
||||
toObservable(this.selectedSort),
|
||||
group$,
|
||||
toObservable(this.selectedGroup),
|
||||
toObservable(this.selectedFilter),
|
||||
]).pipe(
|
||||
observeOn(animationFrameScheduler),
|
||||
|
|
@ -171,27 +189,6 @@ export class TaskViewCustomizerService {
|
|||
);
|
||||
}
|
||||
|
||||
private _allUndoneTasksWithSubTasks$(): Observable<TaskWithSubTasks[]> {
|
||||
return this.store.select(selectAllTasksWithSubTasks).pipe(
|
||||
map((tasks) => {
|
||||
const hiddenProjectIds = new Set(
|
||||
this._allProjects
|
||||
.filter((p) => p.isHiddenFromMenu || p.isArchived)
|
||||
.map((p) => p.id),
|
||||
);
|
||||
const backlogTaskIds = new Set(
|
||||
this._allProjects.flatMap((p) => p.backlogTaskIds || []),
|
||||
);
|
||||
return tasks.filter(
|
||||
(task) =>
|
||||
!task.isDone &&
|
||||
(!task.projectId || !hiddenProjectIds.has(task.projectId)) &&
|
||||
!backlogTaskIds.has(task.id),
|
||||
);
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
private applyFilter(
|
||||
tasks: TaskWithSubTasks[],
|
||||
type: FILTER_OPTION_TYPE | FILTER_COMMON | null,
|
||||
|
|
|
|||
|
|
@ -27,26 +27,20 @@ import {
|
|||
import { TODAY_TAG } from '../tag/tag.const';
|
||||
|
||||
/**
|
||||
* Tests for issue #7269: Detail panel closes immediately when opened for a task
|
||||
* that only appears in the task-view customizer's list.
|
||||
*
|
||||
* The WorkViewComponent has a constructor `effect()` that deselects the currently
|
||||
* selected task whenever it is no longer present in any of the visible task lists.
|
||||
* Previously it only consulted the primary context lists (undone / done / later /
|
||||
* overdue / backlog). When the customizer pulls tasks in from other work
|
||||
* contexts, the selected task would not be found in any of those lists and would
|
||||
* be deselected immediately, closing the detail panel.
|
||||
*
|
||||
* The fix additionally checks `customizedUndoneTasks().list` before resetting
|
||||
* the selection. These tests exercise the real component; the template is
|
||||
* overridden to a no-op so we don't have to stand up every child component.
|
||||
* Tests for the constructor effect() in WorkViewComponent that deselects the
|
||||
* currently selected task when it is no longer present in any visible task list
|
||||
* (undone / done / later / overdue / backlog). The customizer's list is
|
||||
* intentionally NOT consulted: after #7279 it is always a subset of the context's
|
||||
* undoneTasks, so checking undoneTasks is sufficient. These tests exercise the
|
||||
* real component; the template is overridden to a no-op so we don't have to
|
||||
* stand up every child component.
|
||||
*/
|
||||
|
||||
const buildTask = (id: string, subTasks: TaskWithSubTasks[] = []): TaskWithSubTasks =>
|
||||
({ id, subTasks }) as unknown as TaskWithSubTasks;
|
||||
|
||||
describe('WorkViewComponent', () => {
|
||||
describe('selected task retention effect (#7269)', () => {
|
||||
describe('selected task retention effect', () => {
|
||||
let selectedTaskId: ReturnType<typeof signal<string | null>>;
|
||||
let setSelectedId: jasmine.Spy;
|
||||
let customized$: BehaviorSubject<{ list: TaskWithSubTasks[] }>;
|
||||
|
|
@ -149,29 +143,8 @@ describe('WorkViewComponent', () => {
|
|||
store.overrideSelector(selectTaskRepeatCfgsByTagId, []);
|
||||
});
|
||||
|
||||
it('keeps the selection when the task is only in customizedUndoneTasks.list (bug #7269)', async () => {
|
||||
it('deselects when the task is absent from every list', async () => {
|
||||
await createComponent();
|
||||
customized$.next({ list: [buildTask('cross-ctx-1')] });
|
||||
selectedTaskId.set('cross-ctx-1');
|
||||
TestBed.flushEffects();
|
||||
|
||||
expect(setSelectedId).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('keeps the selection when the task is a subtask inside customizedUndoneTasks.list', async () => {
|
||||
await createComponent();
|
||||
customized$.next({
|
||||
list: [buildTask('parent', [buildTask('nested-sub')])],
|
||||
});
|
||||
selectedTaskId.set('nested-sub');
|
||||
TestBed.flushEffects();
|
||||
|
||||
expect(setSelectedId).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('deselects when the task is absent from every list (including customizedUndoneTasks)', async () => {
|
||||
await createComponent();
|
||||
customized$.next({ list: [buildTask('other')] });
|
||||
selectedTaskId.set('ghost');
|
||||
TestBed.flushEffects();
|
||||
|
||||
|
|
|
|||
|
|
@ -225,11 +225,6 @@ export class WorkViewComponent implements OnInit, OnDestroy {
|
|||
// Check if task is in backlog
|
||||
if (this._hasTaskInList(this.backlogTasks(), currentSelectedId)) return;
|
||||
|
||||
// Group/filter may pull tasks from other work contexts into the view;
|
||||
// keep those selected so the detail panel can open for them.
|
||||
if (this._hasTaskInList(this.customizedUndoneTasks()?.list, currentSelectedId))
|
||||
return;
|
||||
|
||||
// if task really is gone
|
||||
this.taskService.setSelectedId(null);
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue