Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR resolves frontend merge-conflict fallout and updates the responder workflow UI/types to support a simplified incident lifecycle (NEW → CONFIRMED/FALSE_ALARM → CLOSED), responder “I’m responding” tracking, and optional CCTV preview links, alongside Vite/Firebase project wiring.
Changes:
- Adjust TypeScript/Vite configuration (
esModuleInterop,vite-env.d.ts) and add Firebase initialization module. - Update
Incidentdomain type (removeTRIAGED, addpreviewUrl,metadataLabel,responders). - Expand responder/incident pages with “latest incident” banner, verify/reject/close actions, responder counts, and publish gating.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| Frontend/tsconfig.json | Enables esModuleInterop for smoother module import compatibility. |
| Frontend/src/vite-env.d.ts | Adds Vite client type references for TS. |
| Frontend/src/types/incident.ts | Updates incident status union + adds responder/preview metadata fields. |
| Frontend/src/pages/ResponderPage.tsx | Adds latest-relevant incident banner, safer numeric formatting, and responder counts. |
| Frontend/src/pages/IncidentPage.tsx | Adds verify/reject/respond/close flows, responder count display, preview URL handling, and publish UX improvements. |
| Frontend/src/firebase.ts | Centralizes Firebase app/auth/firestore initialization via Vite env vars. |
| Frontend/package.json | Restores/defines proper frontend package manifest (Vite/React/Firebase deps). |
Comments suppressed due to low confidence (1)
Frontend/src/pages/ResponderPage.tsx:55
incsis typed asIncident[], but you’re storing objects that also includeid, which then forces repeated(… as any).idcasts later in the component. Consider introducing anIncidentWithId = Incident & { id: string }type and using that for state anditems, so the banner/link code can uselatestRelevant.idsafely withoutanycasts.
export function ResponderPage() {
const [incs, setIncs] = useState<Incident[]>([]);
const [minRisk, setMinRisk] = useState(0);
// controls dismissing the banner so it doesn’t annoy during demo
const [dismissedBannerId, setDismissedBannerId] = useState<string | null>(null);
useEffect(() => {
// Pull all incidents ordered by updatedAt.
// (Backend can decide who gets SMS; UI can show all, but banner prioritizes NEW/CONFIRMED.)
const q1 = query(collection(db, "incidents"), orderBy("updatedAt", "desc"));
const unsub = onSnapshot(q1, (snap) => {
const items: Incident[] = snap.docs.map((d) => ({ id: d.id, ...(d.data() as any) }));
setIncs(items);
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {filtered.length === 0 && <div className="small">No incidents.</div>} | ||
|
|
||
| {filtered.map((i) => ( | ||
| {filtered.map((i: any) => ( |
There was a problem hiding this comment.
filtered is already Incident[], so filtered.map((i: any) => …) needlessly drops type safety. Use a properly typed incident (ideally the same IncidentWithId type used in state) so field access stays checked by TypeScript.
| {filtered.map((i: any) => ( | |
| {filtered.map((i) => ( |
| setErr(""); | ||
| await updateDoc(doc(db, "incidents", id), { | ||
| ...patch, | ||
| notes, |
There was a problem hiding this comment.
patchIncident always writes the current notes state, even for actions like toggling responding/closing/publishing. This can unintentionally overwrite newer notes from another client (or overwrite notes when the responder didn’t intend to change them). Consider only updating notes when explicitly saving notes, or pass notes as part of patch only when it changed.
| notes, |
| try { | ||
| // best-effort “first responder wins”: re-check current status | ||
| const snap = await getDoc(doc(db, "incidents", id)); | ||
| if (!snap.exists()) throw new Error("Incident not found."); | ||
| const current = snap.data() as any; | ||
|
|
||
| if (current.status !== "NEW") { | ||
| throw new Error(`Cannot verify. Incident is already ${current.status}.`); | ||
| } | ||
|
|
||
| await patchIncident({ | ||
| status: "CONFIRMED", | ||
| confirmedBy: user?.uid ?? "unknown", | ||
| confirmedAt: serverTimestamp(), | ||
| }); |
There was a problem hiding this comment.
The “first responder wins” status check isn’t atomic: getDoc + updateDoc can still race (two responders can read NEW then both write). Use a Firestore transaction (or another atomic server-side mechanism) to verify the current status and apply the update in a single operation.
| const snap = await getDoc(doc(db, "incidents", id)); | ||
| if (!snap.exists()) throw new Error("Incident not found."); | ||
| const current = snap.data() as any; | ||
|
|
||
| if (current.status !== "NEW") { | ||
| throw new Error(`Cannot reject. Incident is already ${current.status}.`); | ||
| } | ||
|
|
||
| await patchIncident({ | ||
| status: "FALSE_ALARM", | ||
| rejectedBy: user?.uid ?? "unknown", | ||
| rejectedAt: serverTimestamp(), | ||
| }); |
There was a problem hiding this comment.
Same race condition here as in verifyIncident: getDoc followed by updateDoc doesn’t guarantee only one responder can reject a NEW incident. Use a Firestore transaction/atomic update with a status precondition to enforce the state transition.
| <div className="small" style={{ overflowWrap: "anywhere", opacity: 0.85 }}> | ||
| {previewUrl} | ||
| </div> | ||
| <button className="button secondary" onClick={() => window.open(previewUrl, "_blank")}> |
There was a problem hiding this comment.
window.open(previewUrl, "_blank") should include noopener,noreferrer to prevent the opened page from gaining access to window.opener (reverse-tabnabbing). Consider window.open(previewUrl, "_blank", "noopener,noreferrer") or an <a target="_blank" rel="noopener noreferrer"> link.
| <button className="button secondary" onClick={() => window.open(previewUrl, "_blank")}> | |
| <button | |
| className="button secondary" | |
| onClick={() => window.open(previewUrl, "_blank", "noopener,noreferrer")} | |
| > |
| const firebaseConfig = { | ||
| apiKey: import.meta.env.VITE_FIREBASE_API_KEY as string, | ||
| authDomain: import.meta.env.VITE_FIREBASE_AUTH_DOMAIN as string, | ||
| projectId: import.meta.env.VITE_FIREBASE_PROJECT_ID as string, | ||
| storageBucket: import.meta.env.VITE_FIREBASE_STORAGE_BUCKET as string, | ||
| messagingSenderId: import.meta.env.VITE_FIREBASE_MESSAGING_SENDER_ID as string, | ||
| appId: import.meta.env.VITE_FIREBASE_APP_ID as string, |
There was a problem hiding this comment.
The import.meta.env.… as string assertions will silently coerce undefined at compile-time, and Firebase initialization will then fail at runtime with a hard-to-debug error if an env var is missing. Consider validating required env vars up front (and throwing a clear error) before calling initializeApp.
| const firebaseConfig = { | |
| apiKey: import.meta.env.VITE_FIREBASE_API_KEY as string, | |
| authDomain: import.meta.env.VITE_FIREBASE_AUTH_DOMAIN as string, | |
| projectId: import.meta.env.VITE_FIREBASE_PROJECT_ID as string, | |
| storageBucket: import.meta.env.VITE_FIREBASE_STORAGE_BUCKET as string, | |
| messagingSenderId: import.meta.env.VITE_FIREBASE_MESSAGING_SENDER_ID as string, | |
| appId: import.meta.env.VITE_FIREBASE_APP_ID as string, | |
| function requireEnvVar(name: string, value: unknown): string { | |
| if (typeof value !== "string" || value.trim() === "") { | |
| throw new Error(`Missing or empty required environment variable: ${name}`); | |
| } | |
| return value; | |
| } | |
| const firebaseConfig = { | |
| apiKey: requireEnvVar("VITE_FIREBASE_API_KEY", import.meta.env.VITE_FIREBASE_API_KEY), | |
| authDomain: requireEnvVar("VITE_FIREBASE_AUTH_DOMAIN", import.meta.env.VITE_FIREBASE_AUTH_DOMAIN), | |
| projectId: requireEnvVar("VITE_FIREBASE_PROJECT_ID", import.meta.env.VITE_FIREBASE_PROJECT_ID), | |
| storageBucket: requireEnvVar("VITE_FIREBASE_STORAGE_BUCKET", import.meta.env.VITE_FIREBASE_STORAGE_BUCKET), | |
| messagingSenderId: requireEnvVar( | |
| "VITE_FIREBASE_MESSAGING_SENDER_ID", | |
| import.meta.env.VITE_FIREBASE_MESSAGING_SENDER_ID | |
| ), | |
| appId: requireEnvVar("VITE_FIREBASE_APP_ID", import.meta.env.VITE_FIREBASE_APP_ID), |
| const unsub = onSnapshot(ref, (snap) => { | ||
| if (!snap.exists()) { | ||
| setInc(null); | ||
| return; | ||
| } | ||
| const data = { id: snap.id, ...(snap.data() as any) } as Incident; | ||
| const data = { id: snap.id, ...(snap.data() as any) } as any; | ||
| setInc(data); | ||
| setNotes(data.notes ?? ""); | ||
| }); |
There was a problem hiding this comment.
onSnapshot always does setNotes(data.notes ?? ""). With the new actions (toggleResponding, verifyIncident, etc.) updating the incident frequently, any snapshot update can overwrite a responder’s in-progress edits in the notes textarea. Consider tracking a “notesDirty/isEditing” flag and only syncing notes from Firestore when the user hasn’t modified it locally (or provide an explicit Save Notes action).
No description provided.