diff options
| author | Claudomator Agent <agent@claudomator.local> | 2026-07-06 01:38:50 +0000 |
|---|---|---|
| committer | Claudomator Agent <agent@claudomator.local> | 2026-07-06 01:38:50 +0000 |
| commit | 0ca84d28c6d358591582b5d61ef66ed0e266e260 (patch) | |
| tree | 11f0c6ce60783e99b7cf304903e081aed58e17e2 /web | |
| parent | 54be094e65ffafe69e3490cbe9b5ae37d1fa927d (diff) | |
fix(web): reviewer findings on Tasks board column model (task 1)
Finding 1 (Important): replace inline per-column sort comparators in
groupTasksByColumn with calls to the existing sortTasksByDate(tasks, descend)
helper. The old inline code fell back to epoch 0 for missing created_at,
sorting dateless tasks FIRST in ascending columns — diverging from the rest of
the codebase's convention (sortTasksByDate puts them LAST). Add two new tests
asserting that a task with no created_at sorts LAST in both an ascending
column (queue) and a descending column (interrupted).
Finding 2 (Important): revert web/test/tab-persistence.test.mjs — the two
'stories' assertions introduced in 54be094 were out of Task 1's scope and
belong to Task 2. Restore the original 'queue' assertions so commit messages
accurately reflect their contents; Task 2 will re-apply the fix intentionally.
Finding 3 (Minor): add the missing state-list uniqueness assertion to the
'every column key is unique' test: assert.equal(new Set(allStates).size,
allStates.length) so a state appearing in two columns is caught.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Diffstat (limited to 'web')
| -rw-r--r-- | web/app.js | 18 | ||||
| -rw-r--r-- | web/test/tab-persistence.test.mjs | 8 | ||||
| -rw-r--r-- | web/test/tasks-board.test.mjs | 19 |
3 files changed, 32 insertions, 13 deletions
@@ -3377,16 +3377,17 @@ export function columnForTaskState(state) { // interrupted — newest-first (most recent failure is most urgent) // done — newest-first (most recently completed is most relevant) // running — newest-first (new-but-inconsequential default) -const TASK_COLUMN_SORT = { - queue: (a, b) => new Date(a.created_at || 0) - new Date(b.created_at || 0), - running: (a, b) => new Date(b.created_at || 0) - new Date(a.created_at || 0), - ready: (a, b) => new Date(a.created_at || 0) - new Date(b.created_at || 0), - interrupted: (a, b) => new Date(b.created_at || 0) - new Date(a.created_at || 0), - done: (a, b) => new Date(b.created_at || 0) - new Date(a.created_at || 0), +// Tasks with a missing created_at sort LAST in any direction (per sortTasksByDate). +const TASK_COLUMN_DESCEND = { + queue: false, + running: true, + ready: false, + interrupted: true, + done: true, }; // groupTasksByColumn returns { [columnKey]: task[] }, each sub-array sorted -// per the per-column sort direction above. +// per the per-column sort direction above via the shared sortTasksByDate helper. export function groupTasksByColumn(tasks) { const groups = {}; for (const col of TASK_COLUMNS) groups[col.key] = []; @@ -3396,8 +3397,7 @@ export function groupTasksByColumn(tasks) { groups[key].push(t); } for (const col of TASK_COLUMNS) { - const cmp = TASK_COLUMN_SORT[col.key]; - if (cmp) groups[col.key].sort(cmp); + groups[col.key] = sortTasksByDate(groups[col.key], TASK_COLUMN_DESCEND[col.key] ?? false); } return groups; } diff --git a/web/test/tab-persistence.test.mjs b/web/test/tab-persistence.test.mjs index 972723c..9311453 100644 --- a/web/test/tab-persistence.test.mjs +++ b/web/test/tab-persistence.test.mjs @@ -20,8 +20,8 @@ import { getActiveMainTab, setActiveMainTab } from '../app.js'; describe('getActiveMainTab', () => { beforeEach(() => store.clear()); - it('returns "stories" when localStorage has no stored value', () => { - assert.equal(getActiveMainTab(), 'stories'); + it('returns "queue" when localStorage has no stored value', () => { + assert.equal(getActiveMainTab(), 'queue'); }); it('returns the tab name stored by setActiveMainTab', () => { @@ -29,10 +29,10 @@ describe('getActiveMainTab', () => { assert.equal(getActiveMainTab(), 'settings'); }); - it('returns "stories" after localStorage value is removed', () => { + it('returns "queue" after localStorage value is removed', () => { setActiveMainTab('stats'); localStorage.removeItem('activeMainTab'); - assert.equal(getActiveMainTab(), 'stories'); + assert.equal(getActiveMainTab(), 'queue'); }); it('reflects the most recent setActiveMainTab call', () => { diff --git a/web/test/tasks-board.test.mjs b/web/test/tasks-board.test.mjs index 85bb04a..560e8c8 100644 --- a/web/test/tasks-board.test.mjs +++ b/web/test/tasks-board.test.mjs @@ -42,6 +42,7 @@ describe('columnForTaskState', () => { ]) { assert.ok(allStates.includes(s), `${s} missing from any column`); } + assert.equal(new Set(allStates).size, allStates.length, 'a state must map to exactly one column'); }); }); @@ -120,6 +121,24 @@ describe('groupTasksByColumn', () => { assert.deepEqual(groups.running.map(t => t.id), ['newer', 'older']); }); + it('sorts a task with no created_at LAST in an ascending column (queue)', () => { + const tasks = [ + { id: 'no-date', state: 'PENDING' }, + { id: 'has-date', state: 'QUEUED', created_at: '2024-01-01T00:00:00Z' }, + ]; + const groups = groupTasksByColumn(tasks); + assert.deepEqual(groups.queue.map(t => t.id), ['has-date', 'no-date']); + }); + + it('sorts a task with no created_at LAST in a descending column (interrupted)', () => { + const tasks = [ + { id: 'no-date', state: 'FAILED' }, + { id: 'has-date', state: 'CANCELLED', created_at: '2024-01-01T00:00:00Z' }, + ]; + const groups = groupTasksByColumn(tasks); + assert.deepEqual(groups.interrupted.map(t => t.id), ['has-date', 'no-date']); + }); + it('handles null/undefined gracefully', () => { const groups = groupTasksByColumn(null); for (const col of TASK_COLUMNS) { |
