Skip to content

Commit 8b94cc3

Browse files
msujawsclaude
andcommitted
fix: prevent moving unassigned bugs out of backlog
Add validation to prevent moving bugs assigned to nobody@mozilla.org out of the backlog column. Bugzilla requires a real assignee for ASSIGNED, IN_PROGRESS, and RESOLVED statuses. - Add validateMove helper to check if move is valid - Add onInvalidMove callback to Board for error handling - Show toast error when invalid move is attempted - Allow move if assignee is staged to change to a real person Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 5ec3386 commit 8b94cc3

File tree

3 files changed

+161
-3
lines changed

3 files changed

+161
-3
lines changed

src/App.tsx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,14 @@ function App() {
118118
[bugs, stageAssigneeChange],
119119
)
120120

121+
// Handle invalid move attempt (e.g., unassigned bug out of backlog)
122+
const handleInvalidMove = useCallback(
123+
(_bugId: number, reason: string) => {
124+
addToast('error', reason)
125+
},
126+
[addToast],
127+
)
128+
121129
// Handle clear staged changes
122130
const handleClearChanges = useCallback(() => {
123131
clearAllChanges()
@@ -275,6 +283,7 @@ function App() {
275283
stagedChanges={changes}
276284
onBugMove={handleBugMove}
277285
onAssigneeChange={handleAssigneeChange}
286+
onInvalidMove={handleInvalidMove}
278287
isLoading={isLoadingBugs}
279288
onApplyChanges={handleApplyChanges}
280289
onClearChanges={handleClearChanges}

src/components/Board/Board.test.tsx

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -772,5 +772,108 @@ describe('Board', () => {
772772
expect(screen.getByText(/drop here/i)).toBeInTheDocument()
773773
})
774774
})
775+
776+
describe('unassigned bug move validation', () => {
777+
const nobodyBug: BugzillaBug = {
778+
id: 99,
779+
summary: 'Unassigned bug',
780+
status: 'NEW',
781+
assigned_to: 'nobody@mozilla.org',
782+
priority: 'P2',
783+
severity: 'normal',
784+
component: 'Core',
785+
whiteboard: '[kanban]',
786+
last_change_time: '2024-01-15T10:00:00Z',
787+
creation_time: '2024-01-01T00:00:00Z',
788+
}
789+
790+
it('should call onInvalidMove when trying to move unassigned bug out of backlog', () => {
791+
const onBugMove = vi.fn()
792+
const onInvalidMove = vi.fn()
793+
render(
794+
<Board
795+
{...defaultProps}
796+
bugs={[nobodyBug]}
797+
onBugMove={onBugMove}
798+
onInvalidMove={onInvalidMove}
799+
/>,
800+
)
801+
802+
// Select the bug
803+
fireEvent.keyDown(document, { key: 'ArrowDown' })
804+
// Grab it
805+
fireEvent.keyDown(document, { key: 'Shift' })
806+
// Move right to todo
807+
fireEvent.keyDown(document, { key: 'ArrowRight' })
808+
// Release to drop
809+
fireEvent.keyUp(document, { key: 'Shift' })
810+
811+
expect(onBugMove).not.toHaveBeenCalled()
812+
expect(onInvalidMove).toHaveBeenCalledWith(99, expect.stringContaining('assign'))
813+
})
814+
815+
it('should allow moving unassigned bug if assignee is staged to change', () => {
816+
const onBugMove = vi.fn()
817+
const onInvalidMove = vi.fn()
818+
const stagedChanges = new Map<number, StagedChange>([
819+
[99, { assignee: { from: 'nobody@mozilla.org', to: 'dev@mozilla.com' } }],
820+
])
821+
render(
822+
<Board
823+
{...defaultProps}
824+
bugs={[nobodyBug]}
825+
stagedChanges={stagedChanges}
826+
onBugMove={onBugMove}
827+
onInvalidMove={onInvalidMove}
828+
/>,
829+
)
830+
831+
// Select the bug
832+
fireEvent.keyDown(document, { key: 'ArrowDown' })
833+
// Grab it
834+
fireEvent.keyDown(document, { key: 'Shift' })
835+
// Move right to todo
836+
fireEvent.keyDown(document, { key: 'ArrowRight' })
837+
// Release to drop
838+
fireEvent.keyUp(document, { key: 'Shift' })
839+
840+
expect(onBugMove).toHaveBeenCalledWith(99, 'backlog', 'todo')
841+
expect(onInvalidMove).not.toHaveBeenCalled()
842+
})
843+
844+
it('should still block move if staged assignee is also nobody', () => {
845+
const onBugMove = vi.fn()
846+
const onInvalidMove = vi.fn()
847+
const stagedChanges = new Map<number, StagedChange>([
848+
[99, { assignee: { from: 'dev@mozilla.com', to: 'nobody@mozilla.org' } }],
849+
])
850+
// Bug originally had a real assignee, but staged to nobody
851+
const bugWithStagedNobody: BugzillaBug = {
852+
...nobodyBug,
853+
assigned_to: 'dev@mozilla.com',
854+
}
855+
render(
856+
<Board
857+
{...defaultProps}
858+
bugs={[bugWithStagedNobody]}
859+
stagedChanges={stagedChanges}
860+
onBugMove={onBugMove}
861+
onInvalidMove={onInvalidMove}
862+
/>,
863+
)
864+
865+
// Select the bug
866+
fireEvent.keyDown(document, { key: 'ArrowDown' })
867+
// Grab it
868+
fireEvent.keyDown(document, { key: 'Shift' })
869+
// Move right to todo
870+
fireEvent.keyDown(document, { key: 'ArrowRight' })
871+
// Release to drop
872+
fireEvent.keyUp(document, { key: 'Shift' })
873+
874+
expect(onBugMove).not.toHaveBeenCalled()
875+
expect(onInvalidMove).toHaveBeenCalled()
876+
})
877+
})
775878
})
776879
})

src/components/Board/Board.tsx

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,14 @@ import type { BugzillaBug } from '@/lib/bugzilla/types'
1616
import type { StagedChange } from '@/store/slices/staged-slice'
1717
import { useBoardAssignees } from '@/hooks/use-board-assignees'
1818

19+
const NOBODY_EMAIL = 'nobody@mozilla.org'
20+
1921
interface BoardProps {
2022
bugs: BugzillaBug[]
2123
stagedChanges: Map<number, StagedChange>
2224
onBugMove: (bugId: number, fromColumn: KanbanColumn, toColumn: KanbanColumn) => void
2325
onAssigneeChange?: (bugId: number, newAssignee: string) => void
26+
onInvalidMove?: (bugId: number, reason: string) => void
2427
isLoading?: boolean
2528
onApplyChanges?: () => void
2629
onClearChanges?: () => void
@@ -39,6 +42,7 @@ export function Board({
3942
stagedChanges,
4043
onBugMove,
4144
onAssigneeChange,
45+
onInvalidMove,
4246
isLoading = false,
4347
onApplyChanges,
4448
onClearChanges,
@@ -118,6 +122,28 @@ export function Board({
118122
// Get all assignees from bugs on the board
119123
const allAssignees = useBoardAssignees(bugs)
120124

125+
// Check if a bug can be moved out of backlog
126+
// Returns error message if invalid, undefined if valid
127+
const validateMove = useCallback(
128+
(bugId: number, fromColumn: KanbanColumn, toColumn: KanbanColumn): string | undefined => {
129+
// Only validate moves OUT of backlog
130+
if (fromColumn === 'backlog' && toColumn !== 'backlog') {
131+
const bug = bugs.find((b) => b.id === bugId)
132+
if (!bug) return undefined
133+
134+
// Check the effective assignee (staged or original)
135+
const stagedChange = stagedChanges.get(bugId)
136+
const effectiveAssignee = stagedChange?.assignee?.to ?? bug.assigned_to
137+
138+
if (effectiveAssignee === NOBODY_EMAIL) {
139+
return 'Cannot move unassigned bug out of backlog. Please assign someone first.'
140+
}
141+
}
142+
return undefined
143+
},
144+
[bugs, stagedChanges],
145+
)
146+
121147
// Find first non-empty column
122148
const findFirstNonEmptyColumn = useCallback((): number => {
123149
for (const [i, column] of columns.entries()) {
@@ -326,15 +352,29 @@ export function Board({
326352
const fromColumn = columns[grabStartColumn]
327353
const toColumn = columns[targetColumnIndex]
328354
if (fromColumn && toColumn) {
329-
onBugMove(grabbedBugId, fromColumn, toColumn)
355+
// Validate the move
356+
const error = validateMove(grabbedBugId, fromColumn, toColumn)
357+
if (error) {
358+
onInvalidMove?.(grabbedBugId, error)
359+
} else {
360+
onBugMove(grabbedBugId, fromColumn, toColumn)
361+
}
330362
}
331363
}
332364
setGrabStartColumn(null)
333365
setGrabbedBugId(null)
334366
setTargetColumnIndex(null)
335367
}
336368
},
337-
[isGrabbing, grabStartColumn, grabbedBugId, targetColumnIndex, onBugMove],
369+
[
370+
isGrabbing,
371+
grabStartColumn,
372+
grabbedBugId,
373+
targetColumnIndex,
374+
onBugMove,
375+
validateMove,
376+
onInvalidMove,
377+
],
338378
)
339379

340380
// Set up keyboard event listeners
@@ -378,7 +418,13 @@ export function Board({
378418

379419
// Only trigger move if dropping on a different column
380420
if (currentColumn !== targetColumn) {
381-
onBugMove(bugId, currentColumn, targetColumn)
421+
// Validate the move
422+
const error = validateMove(bugId, currentColumn, targetColumn)
423+
if (error) {
424+
onInvalidMove?.(bugId, error)
425+
} else {
426+
onBugMove(bugId, currentColumn, targetColumn)
427+
}
382428
}
383429
}
384430

0 commit comments

Comments
 (0)