Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
8d43f85
feat(functions): add shared rate-limit helper reading rate_limits/{key}
claude Apr 18, 2026
0d9e1a8
feat(functions): add responder-eligibility query for dispatchResponder
claude Apr 18, 2026
a5320de
fix(functions): add missing updatedAt field to rate-limit document
claude Apr 18, 2026
dc98e75
test(functions): add failing cancelDispatch (pending-only) tests
claude Apr 18, 2026
812f46f
test(functions): add failing verifyReport happy-path tests
claude Apr 18, 2026
c8c1334
test(functions): add seedReportAtStatus and seedDispatch factories fo…
claude Apr 18, 2026
3d855f7
test(functions): add failing dispatchResponder happy-path tests
claude Apr 18, 2026
dc4fb0e
test(functions): add failing rejectReport tests
claude Apr 18, 2026
f967af1
feat(functions): implement verifyReport callable with two-branch tran…
claude Apr 18, 2026
a2f7f82
feat(functions): implement rejectReport callable
claude Apr 18, 2026
50ff0e7
feat(functions): implement dispatchResponder callable with severity-b…
claude Apr 18, 2026
fc18bb4
fix(functions): resolve non-null assertion in verify-report.ts
claude Apr 18, 2026
aed5074
fix(functions): resolve unsafe any casts in cancel-dispatch.ts
claude Apr 18, 2026
33b5fa4
fix(test): rename seedDispatch to match spec, update all callers
claude Apr 18, 2026
cb52197
test(functions): cover verifyReport error paths (wrong-muni, FAILED_P…
claude Apr 18, 2026
1e84a7a
test(rules): lock admin muni-scoped onSnapshot contract with 4 positi…
claude Apr 18, 2026
d83005b
feat(admin-desktop): wire muni-scoped queue, detail panel, verify+rej…
claude Apr 18, 2026
cf37f27
feat(admin-desktop): add hooks, services, and ReportDetailPanel for t…
claude Apr 18, 2026
45e0774
test(functions): add dispatchResponder error-path tests for cross-mun…
claude Apr 18, 2026
9633186
fix(typecheck): resolve verbatimModuleSyntax and staffClaims API errors
claude Apr 18, 2026
1f56e76
feat(responder-app): read-only own-dispatches list for 3b visibility …
claude Apr 18, 2026
bb3fa38
chore(scripts): add idempotent bootstrap for phase-3b test responder
claude Apr 18, 2026
246e999
feat(monitoring): add dispatch.created log metric and dashboard panel
claude Apr 18, 2026
ef40a60
fix(responder-app): add redirect state, role verification on login
claude Apr 18, 2026
d09bfdf
docs(phase-3b): add Phase 3b progress section, fix acceptance exit code
claude Apr 18, 2026
8e809e4
docs(runbooks): capture 3b admin verify + dispatch walkthrough
claude Apr 18, 2026
8247779
fix(validators): restructure dispatchDocSchema to use nested assignedTo
claude Apr 18, 2026
18ede24
fix(validators): add isActive field to responderDocSchema
claude Apr 18, 2026
34b4009
fix(responder-app): add error handler to useOwnDispatches onSnapshot
claude Apr 18, 2026
d4a3a95
fix(admin-desktop): guard App Check init on valid recaptcha key
claude Apr 18, 2026
1da17af
test(callables): cover isActive:false error path in dispatchResponder
claude Apr 18, 2026
a8a6b80
test(rate-limit): cover window eviction of old timestamps
claude Apr 18, 2026
dc466f7
fix(rate-limit): prune timestamps to [limit] before writing
claude Apr 18, 2026
65dbb60
fix(dispatch-responder): re-check shift status inside transaction to …
claude Apr 18, 2026
252ded8
fix(responder-eligibility): include municipalityId in EligibleResponder
claude Apr 18, 2026
1bdc80f
fix(admin-desktop): prefix useReportDetail errors with collection name
claude Apr 18, 2026
b0bed63
test(reject-report): cover FAILED_PRECONDITION when report already ve…
claude Apr 18, 2026
57d8acf
test(cancel-dispatch): cover INVALID_STATUS_TRANSITION on second cancel
claude Apr 18, 2026
f96bbce
test(verify-report): cover INVALID_STATUS_TRANSITION on terminal-stat…
claude Apr 18, 2026
068f04d
fix(typecheck): correct staffClaims signature and type imports in cal…
claude Apr 18, 2026
2be0eff
fix(admin-desktop): adopt full DispatchModal from feature branch + ty…
claude Apr 18, 2026
30d7ab7
fix(reject-report): add explicit awaiting_verify guard instead of rel…
claude Apr 18, 2026
53ec5a4
fix(callables): add FAILED_PRECONDITION code, fix rejectReport error …
claude Apr 18, 2026
4879853
Potential fix for pull request finding 'CodeQL / Unused variable, imp…
Exc1D Apr 19, 2026
27f454f
fix(responder-app): reset useOwnDispatches state on logout
claude Apr 19, 2026
c11cffe
fix(responder-app): add missing firebase dependency
claude Apr 19, 2026
4d3406d
fix(functions): add critical severity to DEADLINE_BY_SEVERITY with sa…
claude Apr 19, 2026
0b99693
fix(functions): use FAILED_PRECONDITION for non-cancellable dispatch …
claude Apr 19, 2026
971607e
test(functions): add NOT_FOUND and non-current-dispatch cancelDispatc…
claude Apr 19, 2026
31bfe92
fix: add pending→cancelled dispatch transition, format routes.tsx, re…
claude Apr 19, 2026
5d991c4
fix(admin-desktop): reset useEligibleResponders state when municipali…
claude Apr 19, 2026
d6d2d48
test(shared-validators): update DISPATCH_TRANSITIONS length assertion…
claude Apr 19, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion apps/admin-desktop/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
"dependencies": {
"@bantayog/shared-types": "workspace:*",
"@bantayog/shared-ui": "workspace:*",
"firebase": "^12.12.0",
"react": "^19.2.5",
"react-dom": "^19.2.5"
"react-dom": "^19.2.5",
"react-router-dom": "^7.14.1"
},
"devDependencies": {
"@types/react": "^19.2.14",
Expand Down
13 changes: 7 additions & 6 deletions apps/admin-desktop/src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import styles from './App.module.css'
import { RouterProvider } from 'react-router-dom'
import { AuthProvider } from './app/auth-provider'
import { router } from './routes'

export function App() {
export default function App() {
return (
<main className={styles.container}>
<h1 className={styles.heading}>Bantayog Alert — Admin</h1>
<p className={styles.subheading}>Phase 0 scaffolding. Admin dashboard arrives in Phase 3.</p>
</main>
<AuthProvider>
<RouterProvider router={router} />
</AuthProvider>
)
}
70 changes: 70 additions & 0 deletions apps/admin-desktop/src/app/auth-provider.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { createContext, useContext, useEffect, useState, type ReactNode } from 'react'
import { onAuthStateChanged, signOut as fbSignOut, type User } from 'firebase/auth'
import { auth } from './firebase'

export interface AdminClaims {
role?: string
municipalityId?: string
active?: boolean
}

interface AuthState {
user: User | null
claims: AdminClaims | null
loading: boolean
signOut: () => Promise<void>
refreshClaims: () => Promise<void>
}

const Ctx = createContext<AuthState | null>(null)

export function AuthProvider({ children }: { children: ReactNode }) {
const [user, setUser] = useState<User | null>(null)
const [claims, setClaims] = useState<AdminClaims | null>(null)
const [loading, setLoading] = useState(true)

const refreshClaims = async () => {
if (!auth.currentUser) {
setClaims(null)
return
}
const tok = await auth.currentUser.getIdTokenResult(true)
setClaims({
role: tok.claims.role as string | undefined,
municipalityId: tok.claims.municipalityId as string | undefined,
active: tok.claims.active === true,
} as AdminClaims)
}

useEffect(() => {
const unsubscribe = onAuthStateChanged(auth, (u) => {
setUser(u)
if (u) {
void u.getIdTokenResult().then((tok) => {
setClaims({
role: tok.claims.role as string | undefined,
municipalityId: tok.claims.municipalityId as string | undefined,
active: tok.claims.active === true,
} as AdminClaims)
setLoading(false)
})
Comment on lines +39 to +50
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Handle token-fetch failures and stale async resolution in auth state listener.

If getIdTokenResult() rejects, loading may never flip to false. Also, async completion can race after auth state changes and set stale claims.

Proposed fix
   useEffect(() => {
     const unsubscribe = onAuthStateChanged(auth, (u) => {
       setUser(u)
       if (u) {
-        void u.getIdTokenResult().then((tok) => {
-          setClaims({
-            role: tok.claims.role as string | undefined,
-            municipalityId: tok.claims.municipalityId as string | undefined,
-            active: tok.claims.active === true,
-          } as AdminClaims)
-          setLoading(false)
-        })
+        void u
+          .getIdTokenResult()
+          .then((tok) => {
+            if (auth.currentUser?.uid !== u.uid) return
+            setClaims({
+              role: tok.claims.role as string | undefined,
+              municipalityId: tok.claims.municipalityId as string | undefined,
+              active: tok.claims.active === true,
+            } as AdminClaims)
+          })
+          .catch(() => {
+            setClaims(null)
+          })
+          .finally(() => {
+            if (auth.currentUser?.uid === u.uid) setLoading(false)
+          })
       } else {
         setClaims(null)
         setLoading(false)
       }
     })
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
useEffect(() => {
const unsubscribe = onAuthStateChanged(auth, (u) => {
setUser(u)
if (u) {
void u.getIdTokenResult().then((tok) => {
setClaims({
role: tok.claims.role as string | undefined,
municipalityId: tok.claims.municipalityId as string | undefined,
active: tok.claims.active === true,
} as AdminClaims)
setLoading(false)
})
useEffect(() => {
const unsubscribe = onAuthStateChanged(auth, (u) => {
setUser(u)
if (u) {
void u
.getIdTokenResult()
.then((tok) => {
if (auth.currentUser?.uid !== u.uid) return
setClaims({
role: tok.claims.role as string | undefined,
municipalityId: tok.claims.municipalityId as string | undefined,
active: tok.claims.active === true,
} as AdminClaims)
})
.catch(() => {
setClaims(null)
})
.finally(() => {
if (auth.currentUser?.uid === u.uid) setLoading(false)
})
} else {
setClaims(null)
setLoading(false)
}
})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/admin-desktop/src/app/auth-provider.tsx` around lines 39 - 50, The
onAuthStateChanged listener in useEffect must handle getIdTokenResult rejections
and avoid applying stale async results: when auth state changes, ensure you
setClaims(undefined) and setLoading(false) immediately for null users; for
non-null users, capture an identifier (e.g., currentUid = u.uid) or a cancelled
flag scoped to this effect before calling u.getIdTokenResult(), and inside the
then/catch/finally only call setClaims/setLoading if the identifier/flag still
matches (user hasn't changed); also add a .catch to handle rejection and a
.finally to guarantee setLoading(false) runs even on error; keep unsubscribe
cleanup as-is to cancel the listener.

} else {
setClaims(null)
setLoading(false)
}
})
return unsubscribe
}, [])

return (
<Ctx.Provider value={{ user, claims, loading, signOut: () => fbSignOut(auth), refreshClaims }}>
{children}
</Ctx.Provider>
)
}

export function useAuth() {
const v = useContext(Ctx)
if (!v) throw new Error('useAuth must be used inside AuthProvider')
return v
}
46 changes: 46 additions & 0 deletions apps/admin-desktop/src/app/firebase.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { initializeApp } from 'firebase/app'
import { getFirestore, connectFirestoreEmulator } from 'firebase/firestore'
import { getAuth, connectAuthEmulator } from 'firebase/auth'
import { getFunctions, connectFunctionsEmulator } from 'firebase/functions'
import { initializeAppCheck, ReCaptchaV3Provider } from 'firebase/app-check'

const useEmulator = import.meta.env.VITE_USE_EMULATOR === 'true'

const firebaseConfig = {
apiKey: import.meta.env.VITE_FIREBASE_API_KEY,
authDomain: import.meta.env.VITE_FIREBASE_AUTH_DOMAIN,
projectId: import.meta.env.VITE_FIREBASE_PROJECT_ID,
storageBucket: import.meta.env.VITE_FIREBASE_STORAGE_BUCKET,
messagingSenderId: import.meta.env.VITE_FIREBASE_MSG_SENDER_ID,
appId: import.meta.env.VITE_FIREBASE_APP_ID,
}

export const firebaseApp = initializeApp(firebaseConfig)

const recaptchaKey = import.meta.env.VITE_RECAPTCHA_SITE_KEY

if (!useEmulator) {
if (recaptchaKey) {
initializeAppCheck(firebaseApp, {
provider: new ReCaptchaV3Provider(recaptchaKey as string),
isTokenAutoRefreshEnabled: true,
})
} else {
console.warn(
'[firebase] VITE_RECAPTCHA_SITE_KEY not set — App Check disabled. DO NOT USE IN PRODUCTION.',
)
Comment on lines +28 to +31
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fail fast in production when App Check key is missing.

In non-emulator mode, missing VITE_RECAPTCHA_SITE_KEY only logs a warning and continues without App Check. That weakens abuse protection if misconfigured deployment reaches prod.

🔧 Proposed guard
   } else {
-    console.warn(
-      '[firebase] VITE_RECAPTCHA_SITE_KEY not set — App Check disabled. DO NOT USE IN PRODUCTION.',
-    )
+    if (import.meta.env.PROD) {
+      throw new Error(
+        '[firebase] Missing VITE_RECAPTCHA_SITE_KEY in production; refusing to start without App Check.',
+      )
+    }
+    console.warn('[firebase] VITE_RECAPTCHA_SITE_KEY not set — App Check disabled in non-prod.')
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/admin-desktop/src/app/firebase.ts` around lines 28 - 31, The current
console.warn on missing VITE_RECAPTCHA_SITE_KEY allows startups in non-emulator
environments; change the branch that handles missing VITE_RECAPTCHA_SITE_KEY
(the block that currently calls console.warn('[firebase] VITE_RECAPTCHA_SITE_KEY
not set — App Check disabled. DO NOT USE IN PRODUCTION.')) to fail fast in
production by throwing a descriptive Error (or calling process.exit(1)) when not
running under the emulator, e.g., check the existing emulator flag/condition and
if emulator mode is false and VITE_RECAPTCHA_SITE_KEY is absent, throw new Error
with a clear message mentioning VITE_RECAPTCHA_SITE_KEY and App Check so
deployments cannot start without the key.

}
} else if (typeof window !== 'undefined') {
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access -- Firebase App Check debug token is a browser global
;(self as any).FIREBASE_APPCHECK_DEBUG_TOKEN = import.meta.env.VITE_APPCHECK_DEBUG_TOKEN ?? true
}

export const db = getFirestore(firebaseApp)
export const auth = getAuth(firebaseApp)
export const functions = getFunctions(firebaseApp, 'asia-southeast1')

if (useEmulator) {
connectFirestoreEmulator(db, 'localhost', 8080)
connectAuthEmulator(auth, 'http://localhost:9099', { disableWarnings: true })
connectFunctionsEmulator(functions, 'localhost', 5001)
}
32 changes: 32 additions & 0 deletions apps/admin-desktop/src/app/protected-route.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { type ReactNode } from 'react'
import { Navigate, useLocation } from 'react-router-dom'
import { useAuth } from './auth-provider'

export function ProtectedRoute({ children }: { children: ReactNode }) {
const { user, claims, loading } = useAuth()
const location = useLocation()

if (loading) return <div>Loading…</div>
if (!user) return <Navigate to="/login" state={{ from: location }} replace />

if (claims?.role !== 'municipal_admin' && claims?.role !== 'provincial_superadmin') {
return (
<div role="alert">
You don&apos;t have admin access on this account. Contact your municipality&apos;s
superadmin.
</div>
)
}
if (claims.active !== true) {
return <div role="alert">Your account is not active. Please contact your superadmin.</div>
}
if (claims.role === 'municipal_admin' && !claims.municipalityId) {
return (
<div role="alert">
Your admin account is missing a municipality assignment. Contact superadmin.
</div>
)
}

return <>{children}</>
}
59 changes: 59 additions & 0 deletions apps/admin-desktop/src/hooks/useEligibleResponders.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { useEffect, useState } from 'react'
import { collection, onSnapshot, query, where } from 'firebase/firestore'
import { db } from '../app/firebase'
import { getDatabase, ref, onValue } from 'firebase/database'
import { firebaseApp } from '../app/firebase'

export interface EligibleResponder {
uid: string
displayName: string
agencyId: string
}

export function useEligibleResponders(municipalityId: string | undefined) {
const [responders, setResponders] = useState<Record<string, EligibleResponder>>({})
const [shift, setShift] = useState<Record<string, { isOnShift: boolean }>>({})

useEffect(() => {
if (!municipalityId) {
setResponders({})
return
}
const q = query(
collection(db, 'responders'),
where('municipalityId', '==', municipalityId),
where('isActive', '==', true),
)
return onSnapshot(q, (snap) => {
const out: Record<string, EligibleResponder> = {}
snap.docs.forEach((d) => {
const data = d.data()
out[d.id] = {
uid: d.id,
displayName: String(data.displayName ?? d.id),
agencyId: String(data.agencyId ?? 'unknown'),
}
Comment on lines +31 to +35
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Align fallback normalization with backend eligibility contract.

Line [33]-Line [34] default to uid / 'unknown', but functions/src/services/responder-eligibility.ts defaults to empty strings. This can cause client/server divergence in sorting and rendered labels for missing profile fields.

Proposed fix
         out[d.id] = {
           uid: d.id,
-          displayName: String(data.displayName ?? d.id),
-          agencyId: String(data.agencyId ?? 'unknown'),
+          displayName: String(data.displayName ?? ''),
+          agencyId: String(data.agencyId ?? ''),
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
out[d.id] = {
uid: d.id,
displayName: String(data.displayName ?? d.id),
agencyId: String(data.agencyId ?? 'unknown'),
}
out[d.id] = {
uid: d.id,
displayName: String(data.displayName ?? ''),
agencyId: String(data.agencyId ?? ''),
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/admin-desktop/src/hooks/useEligibleResponders.ts` around lines 31 - 35,
The client fallback defaults in useEligibleResponders.ts currently set
displayName to String(data.displayName ?? d.id) and agencyId to
String(data.agencyId ?? 'unknown'); change these to match the backend
eligibility contract by defaulting both to empty strings (i.e., use
data.displayName ?? '' and data.agencyId ?? '') so sorting and rendered labels
align with functions/src/services/responder-eligibility.ts; update the object
assigned to out[d.id] (uid, displayName, agencyId) accordingly.

})
setResponders(out)
})
}, [municipalityId])

useEffect(() => {
if (!municipalityId) {
setShift({})
return
}
const rtdb = getDatabase(firebaseApp)
const node = ref(rtdb, `/responder_index/${municipalityId}`)
const unsub = onValue(node, (s) => {
const snapVal = s.val()
setShift(snapVal !== null ? (snapVal as Record<string, { isOnShift: boolean }>) : {})
})
Comment on lines +48 to +51
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Validate RTDB snapshot shape before committing to state.

Line [50] trusts untyped RTDB payload via direct cast. This skips boundary validation and can store malformed entries.

Proposed fix
   const unsub = onValue(node, (s) => {
     const snapVal = s.val()
-    setShift(snapVal !== null ? (snapVal as Record<string, { isOnShift: boolean }>) : {})
+    if (!snapVal || typeof snapVal !== 'object') {
+      setShift({})
+      return
+    }
+    const normalized: Record<string, { isOnShift: boolean }> = {}
+    for (const [uid, value] of Object.entries(snapVal as Record<string, unknown>)) {
+      if (value && typeof value === 'object' && (value as { isOnShift?: unknown }).isOnShift === true) {
+        normalized[uid] = { isOnShift: true }
+      }
+    }
+    setShift(normalized)
   })

As per coding guidelines: "Use defensive programming: validate external input at boundaries and never swallow errors with empty catch blocks."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const unsub = onValue(node, (s) => {
const snapVal = s.val()
setShift(snapVal !== null ? (snapVal as Record<string, { isOnShift: boolean }>) : {})
})
const unsub = onValue(node, (s) => {
const snapVal = s.val()
if (!snapVal || typeof snapVal !== 'object') {
setShift({})
return
}
const normalized: Record<string, { isOnShift: boolean }> = {}
for (const [uid, value] of Object.entries(snapVal as Record<string, unknown>)) {
if (value && typeof value === 'object' && (value as { isOnShift?: unknown }).isOnShift === true) {
normalized[uid] = { isOnShift: true }
}
}
setShift(normalized)
})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/admin-desktop/src/hooks/useEligibleResponders.ts` around lines 48 - 51,
The RTDB snapshot is being cast unchecked and stored via setShift in the
useEligibleResponders hook; instead validate snapVal's shape before committing:
check that snapVal is an object (not null/array), iterate its keys and ensure
each value is an object with a boolean isOnShift (filter or coerce invalid
entries), and only then call setShift with the cleaned/typed record; also wrap
parsing/validation in try/catch and log any errors rather than swallowing them
so malformed payloads don’t corrupt state (refer to the onValue callback,
snapVal, and setShift).

return unsub
}, [municipalityId])

const eligible = Object.values(responders)
.filter((r) => shift[r.uid]?.isOnShift === true)
.sort((a, b) => a.displayName.localeCompare(b.displayName))
return eligible
}
58 changes: 58 additions & 0 deletions apps/admin-desktop/src/hooks/useMuniReports.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { useEffect, useState } from 'react'
import { collection, onSnapshot, query, where, orderBy, limit, Timestamp } from 'firebase/firestore'
import { db } from '../app/firebase'

export interface MuniReportRow {
reportId: string
status: string
severityDerived: string
createdAt: Timestamp
municipalityLabel: string
}

export function useMuniReports(municipalityId: string | undefined) {
const [rows, setRows] = useState<MuniReportRow[]>([])
const [loading, setLoading] = useState(true)
const [error, setError] = useState<string | null>(null)

useEffect(() => {
if (!municipalityId) {
setRows([])
setLoading(false)
return
}
Comment on lines +19 to +23
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Reset error on boundary change and successful snapshots.

The hook sets error on failures but never clears it on success (or when municipalityId becomes undefined). This can leave the page stuck showing stale errors.

Proposed fix
   useEffect(() => {
     if (!municipalityId) {
       setRows([])
+      setError(null)
       setLoading(false)
       return
     }
+    setError(null)
     setLoading(true)
@@
       (snap) => {
         setRows(
@@
         )
+        setError(null)
         setLoading(false)
       },

Also applies to: 34-50

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/admin-desktop/src/hooks/useMuniReports.ts` around lines 19 - 23, The
hook useMuniReports currently sets error on failures but never clears it; update
the logic so setError(null) is called both when municipalityId is falsy (near
the branch that calls setRows([]) and setLoading(false)) and after successful
snapshot/fetch handling (where you call setRows and setLoading on success).
Locate uses of municipalityId, setRows, setLoading and the error state setter
(setError) and add calls to clear the error on boundary change and immediately
after successful data processing to avoid stale errors remaining visible.

setLoading(true)
const q = query(
collection(db, 'reports'),
where('municipalityId', '==', municipalityId),
where('status', 'in', ['new', 'awaiting_verify', 'verified', 'assigned']),
orderBy('createdAt', 'desc'),
limit(100),
)
const unsub = onSnapshot(
q,
(snap) => {
setRows(
snap.docs.map((d) => {
const data = d.data()
return {
reportId: d.id,
status: String(data.status),
severityDerived: String(data.severityDerived ?? 'medium'),
createdAt: data.createdAt as Timestamp,
municipalityLabel: String(data.municipalityLabel ?? ''),
}
}),
)
setLoading(false)
},
(err) => {
setError(err.message)
setLoading(false)
},
)
return unsub
}, [municipalityId])

return { rows, loading, error }
}
63 changes: 63 additions & 0 deletions apps/admin-desktop/src/hooks/useReportDetail.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { useEffect, useState } from 'react'
import { doc, onSnapshot } from 'firebase/firestore'
import { db } from '../app/firebase'
import type { Timestamp } from 'firebase/firestore'

export interface ReportDetail {
reportId: string
status: string
municipalityLabel: string
severityDerived: string
createdAt: Timestamp
verifiedBy?: string
verifiedAt?: Timestamp
currentDispatchId?: string
}
export interface ReportOps {
verifyQueuePriority: number
assignedMunicipalityAdmins: string[]
}

export function useReportDetail(reportId: string | undefined) {
const [report, setReport] = useState<ReportDetail | null>(null)
const [ops, setOps] = useState<ReportOps | null>(null)
const [error, setError] = useState<string | null>(null)

useEffect(() => {
if (!reportId) {
setReport(null)
setOps(null)
return
Comment on lines +27 to +30
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Reset error on report switch and successful snapshots.

error is only ever appended/set on failures. It should be cleared when reportId changes to empty and when listeners recover successfully.

Proposed fix
   useEffect(() => {
     if (!reportId) {
       setReport(null)
       setOps(null)
+      setError(null)
       return
     }
+    setError(null)
     const u1 = onSnapshot(
       doc(db, 'reports', reportId),
       (s) => {
@@
         )
+        setError(null)
       },
@@
       (s) => {
         setOps(s.exists() ? (s.data() as ReportOps) : null)
+        setError(null)
       },

Also applies to: 41-53

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/admin-desktop/src/hooks/useReportDetail.ts` around lines 27 - 30, When
clearing state because reportId is falsy and when a successful listener/snapshot
updates state, also clear the error state; update the block in useReportDetail
where you do setReport(null) and setOps(null) to also call setError(null), and
likewise add setError(null) inside the listener/snapshot success handler (the
function that sets report and ops around the code referenced at 41-53) so
recovered snapshots remove prior errors.

}
const u1 = onSnapshot(
doc(db, 'reports', reportId),
(s) => {
setReport(
s.exists()
? ({ reportId: s.id, ...(s.data() as Partial<ReportDetail>) } as ReportDetail)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Prevent persisted payload from overriding canonical reportId.

Current spread order allows Firestore data to overwrite reportId if that field exists in the document.

Proposed fix
-            ? ({ reportId: s.id, ...(s.data() as Partial<ReportDetail>) } as ReportDetail)
+            ? ({ ...(s.data() as Partial<ReportDetail>), reportId: s.id } as ReportDetail)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/admin-desktop/src/hooks/useReportDetail.ts` at line 37, The current
object construction in useReportDetail uses ({ reportId: s.id, ...(s.data() as
Partial<ReportDetail>) } as ReportDetail) which lets the Firestore document
fields overwrite reportId; change the spread order so the canonical s.id wins by
spreading s.data() first and then setting reportId (e.g., build the object from
...(s.data() as Partial<ReportDetail>) and then reportId: s.id), or explicitly
remove reportId from the spread before composing the object—update the
expression in useReportDetail that references s.data() and s.id accordingly so
reportId cannot be overridden.

: null,
)
},
(err) => {
setError(`reports: ${err.message}`)
},
)
const u2 = onSnapshot(
doc(db, 'report_ops', reportId),
(s) => {
setOps(s.exists() ? (s.data() as ReportOps) : null)
},
(err) => {
setError((prev) =>
prev ? `${prev}; report_ops: ${err.message}` : `report_ops: ${err.message}`,
)
},
)
return () => {
u1()
u2()
}
}, [reportId])

return { report, ops, error }
}
9 changes: 2 additions & 7 deletions apps/admin-desktop/src/main.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
import { StrictMode } from 'react'
import { createRoot } from 'react-dom/client'
import { App } from './App.js'
import App from './App.js'

const rootEl = document.getElementById('root')
if (!rootEl) throw new Error('#root element not found')

createRoot(rootEl).render(
<StrictMode>
<App />
</StrictMode>,
)
createRoot(rootEl).render(<App />)
Loading
Loading