-
Notifications
You must be signed in to change notification settings - Fork 0
feat(phase5): Cluster A — Admin UI Hardening #65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 27 commits
908cba5
0d55081
7cd3a71
7906394
5e00328
661eb08
1ec884d
055134e
eb8b5d6
71cc33c
9e10af1
58fbb93
86a7b4a
c0e85aa
e21409a
9afbaff
c9195ce
cc82ec1
83cc91f
7be2d19
e010d41
15e3e4a
44764f1
32c6e72
c8e6592
a55fc35
eb0e69f
f502012
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| { | ||
| "@bantayog/shared-types": 0, | ||
| "@bantayog/citizen-pwa": 0, | ||
| "@bantayog/shared-sms-parser": 0, | ||
| "@bantayog/responder-app": 0, | ||
| "@bantayog/shared-ui": 0, | ||
| "@bantayog/functions": 0, | ||
| "@bantayog/admin-desktop": 0, | ||
| "@bantayog/shared-validators": 0, | ||
| "@bantayog/shared-data": 0, | ||
| "@bantayog/e2e-tests": 0 | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| import { describe, it, expect, vi, beforeEach } from 'vitest' | ||
| import { render, screen } from '@testing-library/react' | ||
| import userEvent from '@testing-library/user-event' | ||
|
|
||
| const mockInitiateHandoff = vi.hoisted(() => vi.fn()) | ||
|
|
||
| vi.mock('../app/firebase', () => ({ db: {} })) | ||
|
|
||
| vi.mock('@bantayog/shared-ui', () => ({ | ||
| useAuth: () => ({ | ||
| claims: { municipalityId: 'daet', role: 'municipal_admin' }, | ||
| signOut: vi.fn(), | ||
| }), | ||
| })) | ||
|
|
||
| vi.mock('../services/callables', () => ({ | ||
| callables: { | ||
| verifyReport: vi.fn(), | ||
| rejectReport: vi.fn(), | ||
| initiateShiftHandoff: mockInitiateHandoff, | ||
| acceptShiftHandoff: vi.fn(), | ||
| }, | ||
| })) | ||
|
|
||
| vi.mock('../hooks/useMuniReports', () => ({ | ||
| useMuniReports: () => ({ | ||
| reports: [], | ||
| hasMore: false, | ||
| loadMore: vi.fn(), | ||
| loading: false, | ||
| error: null, | ||
| }), | ||
| })) | ||
|
|
||
| vi.mock('../hooks/usePendingHandoffs', () => ({ | ||
| usePendingHandoffs: () => [], | ||
| })) | ||
|
|
||
| vi.mock('../pages/ReportDetailPanel', () => ({ ReportDetailPanel: () => <div>detail</div> })) | ||
| vi.mock('../pages/DispatchModal', () => ({ DispatchModal: () => <div>dispatch</div> })) | ||
| vi.mock('../pages/CloseReportModal', () => ({ CloseReportModal: () => <div>close</div> })) | ||
|
|
||
| import { TriageQueuePage } from '../pages/TriageQueuePage' | ||
|
|
||
| describe('ShiftHandoffModal', () => { | ||
| beforeEach(() => { | ||
| mockInitiateHandoff.mockResolvedValue({ success: true, handoffId: 'h-new-1' }) | ||
| }) | ||
|
|
||
| it('renders Start Handoff button in header', () => { | ||
| render(<TriageQueuePage />) | ||
| expect(screen.getByRole('button', { name: /start handoff/i })).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it('opens ShiftHandoffModal on Start Handoff click', async () => { | ||
| const user = userEvent.setup() | ||
| render(<TriageQueuePage />) | ||
| await user.click(screen.getByRole('button', { name: /start handoff/i })) | ||
| expect(screen.getByRole('dialog', { name: /shift handoff/i })).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it('calls initiateShiftHandoff on Initiate click', async () => { | ||
| const user = userEvent.setup() | ||
| render(<TriageQueuePage />) | ||
| await user.click(screen.getByRole('button', { name: /start handoff/i })) | ||
| const notesField = screen.getByLabelText(/notes/i) | ||
| await user.type(notesField, 'End of day shift') | ||
| await user.click(screen.getByRole('button', { name: /initiate/i })) | ||
| expect(mockInitiateHandoff).toHaveBeenCalledWith( | ||
| expect.objectContaining({ notes: 'End of day shift' }), | ||
| ) | ||
| }) | ||
| }) | ||
|
|
||
| describe('Incoming handoff banner', () => { | ||
| it('shows no banner when no pending handoffs', () => { | ||
| render(<TriageQueuePage />) | ||
| expect(screen.queryByRole('button', { name: /accept handoff/i })).not.toBeInTheDocument() | ||
| }) | ||
| }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,188 @@ | ||
| import { describe, it, expect, vi, beforeEach } from 'vitest' | ||
| import { render, screen, fireEvent } from '@testing-library/react' | ||
| import userEvent from '@testing-library/user-event' | ||
|
|
||
| const mockUseMuniReports = vi.fn() | ||
|
|
||
| vi.mock('../hooks/useMuniReports', () => ({ | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||
| useMuniReports: (...args: unknown[]) => mockUseMuniReports(...args), | ||
| })) | ||
|
|
||
| vi.mock('../app/firebase', () => ({ | ||
| db: {}, | ||
| })) | ||
|
|
||
| vi.mock('@bantayog/shared-ui', () => ({ | ||
| useAuth: () => ({ | ||
| claims: { municipalityId: 'daet', role: 'municipal_admin' }, | ||
| signOut: vi.fn(), | ||
| }), | ||
| })) | ||
|
|
||
| vi.mock('../services/callables', () => ({ | ||
| callables: { | ||
| verifyReport: vi.fn(), | ||
| rejectReport: vi.fn(), | ||
| }, | ||
| })) | ||
|
|
||
| vi.mock('../hooks/usePendingHandoffs', () => ({ | ||
| usePendingHandoffs: () => [], | ||
| })) | ||
|
|
||
| vi.mock('../pages/ReportDetailPanel', () => ({ | ||
| ReportDetailPanel: () => <div>detail</div>, | ||
| })) | ||
| vi.mock('../pages/DispatchModal', () => ({ | ||
| DispatchModal: () => <div>dispatch</div>, | ||
| })) | ||
| vi.mock('../pages/CloseReportModal', () => ({ | ||
| CloseReportModal: () => <div>close</div>, | ||
| })) | ||
|
|
||
| import { TriageQueuePage } from '../pages/TriageQueuePage' | ||
|
|
||
| describe('TriageQueuePage', () => { | ||
| beforeEach(() => { | ||
| mockUseMuniReports.mockReturnValue({ | ||
| reports: [], | ||
| hasMore: false, | ||
| loadMore: vi.fn(), | ||
| loading: false, | ||
| error: null, | ||
| }) | ||
| }) | ||
|
|
||
| it('renders Load More button when hasMore is true', () => { | ||
| mockUseMuniReports.mockReturnValue({ | ||
| reports: [ | ||
| { reportId: 'r1', status: 'new', severity: 'high', createdAt: null, municipalityLabel: '' }, | ||
| ], | ||
| hasMore: true, | ||
| loadMore: vi.fn(), | ||
| loading: false, | ||
| error: null, | ||
| }) | ||
| render(<TriageQueuePage />) | ||
| expect(screen.getByRole('button', { name: /load more/i })).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it('does not render Load More button when hasMore is false', () => { | ||
| render(<TriageQueuePage />) | ||
| expect(screen.queryByRole('button', { name: /load more/i })).not.toBeInTheDocument() | ||
| }) | ||
|
|
||
| it('calls loadMore when Load More is clicked', () => { | ||
| const loadMore = vi.fn() | ||
| mockUseMuniReports.mockReturnValue({ | ||
| reports: [ | ||
| { reportId: 'r1', status: 'new', severity: 'high', createdAt: null, municipalityLabel: '' }, | ||
| ], | ||
| hasMore: true, | ||
| loadMore, | ||
| loading: false, | ||
| error: null, | ||
| }) | ||
| render(<TriageQueuePage />) | ||
| fireEvent.click(screen.getByRole('button', { name: /load more/i })) | ||
| expect(loadMore).toHaveBeenCalledTimes(1) | ||
| }) | ||
|
|
||
| it('shows Showing X count', () => { | ||
| mockUseMuniReports.mockReturnValue({ | ||
| reports: [ | ||
| { reportId: 'r1', status: 'new', severity: 'high', createdAt: null, municipalityLabel: '' }, | ||
| { | ||
| reportId: 'r2', | ||
| status: 'new', | ||
| severity: 'medium', | ||
| createdAt: null, | ||
| municipalityLabel: '', | ||
| }, | ||
| ], | ||
| hasMore: true, | ||
| loadMore: vi.fn(), | ||
| loading: false, | ||
| error: null, | ||
| }) | ||
| render(<TriageQueuePage />) | ||
| expect(screen.getByText(/showing 2/i)).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it('renders severity from severity field, not severityDerived', () => { | ||
| mockUseMuniReports.mockReturnValue({ | ||
| reports: [ | ||
| { reportId: 'r1', status: 'new', severity: 'high', createdAt: null, municipalityLabel: '' }, | ||
| ], | ||
| hasMore: false, | ||
| loadMore: vi.fn(), | ||
| loading: false, | ||
| error: null, | ||
| }) | ||
| render(<TriageQueuePage />) | ||
| expect(screen.getByText(/high/i)).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it('pressing j selects the next report in the list', async () => { | ||
| const user = userEvent.setup() | ||
| mockUseMuniReports.mockReturnValue({ | ||
| reports: [ | ||
| { reportId: 'r1', status: 'new', severity: 'high', createdAt: null, municipalityLabel: '' }, | ||
| { | ||
| reportId: 'r2', | ||
| status: 'new', | ||
| severity: 'medium', | ||
| createdAt: null, | ||
| municipalityLabel: '', | ||
| }, | ||
| ], | ||
| hasMore: false, | ||
| loadMore: vi.fn(), | ||
| loading: false, | ||
| error: null, | ||
| }) | ||
| render(<TriageQueuePage />) | ||
| await user.keyboard('j') | ||
| expect(screen.getByText('detail')).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it('pressing k moves selection backward', async () => { | ||
| const user = userEvent.setup() | ||
| mockUseMuniReports.mockReturnValue({ | ||
| reports: [ | ||
| { reportId: 'r1', status: 'new', severity: 'high', createdAt: null, municipalityLabel: '' }, | ||
| { | ||
| reportId: 'r2', | ||
| status: 'new', | ||
| severity: 'medium', | ||
| createdAt: null, | ||
| municipalityLabel: '', | ||
| }, | ||
| ], | ||
| hasMore: false, | ||
| loadMore: vi.fn(), | ||
| loading: false, | ||
| error: null, | ||
| }) | ||
| render(<TriageQueuePage />) | ||
| await user.keyboard('jj') | ||
| await user.keyboard('k') | ||
| expect(screen.getByText('detail')).toBeInTheDocument() | ||
| }) | ||
|
Comment on lines
+150
to
+172
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Test assertion does not verify backward movement. The test presses Consider asserting on a distinguishing attribute of the selected report (e.g., via 🤖 Prompt for AI Agents |
||
|
|
||
| it('keyboard shortcuts do not fire when a modal is open', async () => { | ||
| const user = userEvent.setup() | ||
| mockUseMuniReports.mockReturnValue({ | ||
| reports: [], | ||
| hasMore: false, | ||
| loadMore: vi.fn(), | ||
| loading: false, | ||
| error: null, | ||
| }) | ||
| render(<TriageQueuePage />) | ||
| await user.keyboard('j') | ||
| await user.keyboard('k') | ||
| expect(screen.queryByText('detail')).not.toBeInTheDocument() | ||
| }) | ||
|
Comment on lines
+174
to
+187
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Test does not verify modal-blocking behavior. This test uses an empty Consider testing with a non-empty reports array and setting 💡 Suggested fix it('keyboard shortcuts do not fire when a modal is open', async () => {
const user = userEvent.setup()
+ // Need a way to trigger modal state - consider exposing a prop or
+ // clicking a button that sets dispatchForReportId/closeForReportId
mockUseMuniReports.mockReturnValue({
- reports: [],
+ reports: [
+ { reportId: 'r1', status: 'new', severity: 'high', createdAt: null, municipalityLabel: '' },
+ ],
hasMore: false,
loadMore: vi.fn(),
loading: false,
error: null,
})
render(<TriageQueuePage />)
+ // First select a report, then open a modal, then verify j/k is blocked
+ await user.keyboard('j')
+ expect(screen.getByText('detail')).toBeInTheDocument()
+ // Trigger modal open (e.g., via dispatch button click)
+ // Then verify subsequent j/k does not change selection
- await user.keyboard('j')
- await user.keyboard('k')
- expect(screen.queryByText('detail')).not.toBeInTheDocument()
})🤖 Prompt for AI Agents |
||
| }) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Mock scaffolding is over the smell budget for a unit test.
This setup is quite mock-heavy and will be brittle as
TriageQueuePageevolves. Consider moving this flow to an emulator-backed integration test and keeping only a minimal unit smoke test here.As per coding guidelines: “Mocks are a smell budget. If a unit test needs more than ~20 lines of mock setup, consider integration test with emulators instead.”
🤖 Prompt for AI Agents