2 Commits

Author SHA1 Message Date
da36edbba6 vscode 2026-03-11 22:58:53 -05:00
jason
d6585c01c6 Code review and fixes 2026-03-11 16:33:38 -05:00
10 changed files with 102 additions and 18 deletions

4
.vscode/settings.json vendored Normal file
View File

@@ -0,0 +1,4 @@
{
"giteaActions.baseUrl": "https://git.alwisp.com",
"giteaActions.discovery.mode": "allAccessible"
}

View File

@@ -6,6 +6,8 @@ import ToastProvider from './components/ToastProvider';
import './styles/mobile.css';
const REPO_URL = 'https://git.alwisp.com/jason/cpas';
// TODO [CLEANUP #18]: DevTicker is a dev vanity widget that ships to prod.
// Either gate with `import.meta.env.DEV` or remove from the footer.
const PROJECT_START = new Date('2026-03-06T11:33:32-06:00');
function elapsed(from) {
@@ -101,7 +103,9 @@ const tabs = [
{ id: 'violation', label: '+ New Violation' },
];
// Responsive utility hook
// TODO [MAJOR #8]: Move to src/hooks/useMediaQuery.js — this hook is duplicated
// verbatim in Dashboard.jsx. Also remove `matches` from the useEffect dep array
// (it changes inside the effect, which can cause a loop on strict-mode mount).
function useMediaQuery(query) {
const [matches, setMatches] = useState(false);
useEffect(() => {
@@ -237,6 +241,8 @@ export default function App() {
return (
<ToastProvider>
{/* TODO [MAJOR #9]: Inline <style> tags re-inject on every render and duplicate
the same block from Dashboard.jsx. Move all shared mobile CSS to mobile.css */}
<style>{mobileStyles}</style>
<div style={s.app}>
<nav style={s.nav} className="app-nav">
@@ -248,7 +254,8 @@ export default function App() {
<div className="nav-tabs">
{tabs.map(t => (
<button key={t.id} style={s.tab(tab === t.id)} className="nav-tab" onClick={() => setTab(t.id)}>
{isMobile ? t.label.replace('📊 ', '📊 ').replace('+ New ', '+ ') : t.label}
{/* TODO [MINOR #17]: first .replace('📊 ', '📊 ') replaces string with itself — no-op. Remove it. */}
{isMobile ? t.label.replace('📊 ', '📊 ').replace('+ New ', '+ ') : t.label}
</button>
))}
</div>

View File

@@ -112,6 +112,9 @@ export default function AuditLog({ onClose }) {
const [filterAction, setFilterAction] = useState('');
const LIMIT = 50;
// TODO [MAJOR #5]: `offset` in useCallback deps causes the callback to be
// re-created on each load-more, which triggers the filterType/filterAction
// useEffect unexpectedly. Track offset in a useRef instead.
const load = useCallback((reset = false) => {
setLoading(true);
const o = reset ? 0 : offset;
@@ -121,7 +124,8 @@ export default function AuditLog({ onClose }) {
axios.get('/api/audit', { params })
.then(r => {
const data = r.data;
// Client-side action filter (cheap enough at this scale)
// TODO [MINOR]: client-side action filter means server still fetches LIMIT
// rows before filtering — add server-side `action` param to /api/audit.
const filtered = filterAction ? data.filter(e => e.action === filterAction) : data;
setEntries(prev => reset ? filtered : [...prev, ...filtered]);
setHasMore(data.length === LIMIT);

View File

@@ -29,7 +29,8 @@ function isAtRisk(points) {
return boundary !== null && (boundary - points) <= AT_RISK_THRESHOLD;
}
// Media query hook
// TODO [MAJOR #8]: Same hook is defined in App.jsx — extract to src/hooks/useMediaQuery.js
// Also: `matches` in the dep array can cause a loop on strict-mode initial mount.
function useMediaQuery(query) {
const [matches, setMatches] = useState(false);
useEffect(() => {
@@ -186,6 +187,7 @@ export default function Dashboard() {
return (
<>
{/* TODO [MAJOR #9]: Same mobileStyles block exists in App.jsx. Move to mobile.css */}
<style>{mobileStyles}</style>
<div style={s.wrap} className="dashboard-wrap">
<div style={s.header} className="dashboard-header">

View File

@@ -103,13 +103,12 @@ export default function EmployeeModal({ employeeId, onClose }) {
const load = useCallback(() => {
setLoading(true);
Promise.all([
axios.get('/api/employees'),
axios.get(`/api/employees/${employeeId}`),
axios.get(`/api/employees/${employeeId}/score`),
axios.get(`/api/violations/employee/${employeeId}?limit=100`),
])
.then(([empRes, scoreRes, violRes]) => {
const emp = empRes.data.find((e) => e.id === employeeId);
setEmployee(emp || null);
setEmployee(empRes.data || null);
setScore(scoreRes.data);
setViolations(violRes.data);
})

View File

@@ -1,5 +1,6 @@
import React, { useState } from 'react';
import axios from 'axios';
import { useToast } from './ToastProvider';
const s = {
wrapper: { marginTop: '20px' },
@@ -53,14 +54,23 @@ export default function EmployeeNotes({ employeeId, initialNotes, onSaved }) {
const [draft, setDraft] = useState(initialNotes || '');
const [saved, setSaved] = useState(initialNotes || '');
const [saving, setSaving] = useState(false);
const [saveErr, setSaveErr] = useState('');
const toast = useToast();
const handleSave = async () => {
setSaving(true);
setSaveErr('');
try {
await axios.patch(`/api/employees/${employeeId}/notes`, { notes: draft });
setSaved(draft);
setEditing(false);
if (onSaved) onSaved(draft);
} catch (err) {
const msg = err.response?.data?.error || err.message || 'Failed to save notes';
setSaveErr(msg);
toast.error('Notes save failed: ' + msg);
// Keep editing open so the user doesn't lose their changes
} finally {
setSaving(false);
}
@@ -130,6 +140,11 @@ export default function EmployeeNotes({ employeeId, initialNotes, onSaved }) {
placeholder="Free-text notes — one per line or comma-separated. Does not affect CPAS scoring."
autoFocus
/>
{saveErr && (
<div style={{ fontSize: '12px', color: '#ff7070', marginBottom: '6px' }}>
{saveErr}
</div>
)}
<div style={s.actions}>
<button style={s.saveBtn} onClick={handleSave} disabled={saving}>
{saving ? 'Saving…' : 'Save Notes'}

View File

@@ -1,8 +1,8 @@
import React, { useEffect, useState } from 'react';
import axios from 'axios';
// Tier thresholds used to compute what tier an employee would drop to
// after a given violation rolls off.
// TODO [MINOR #10]: This TIER_THRESHOLDS array duplicates tiers defined in CpasBadge.jsx
// and Dashboard.jsx. Export TIERS from CpasBadge.jsx and import here instead.
const TIER_THRESHOLDS = [
{ min: 30, label: 'Separation', color: '#ff1744' },
{ min: 25, label: 'Final Decision', color: '#ff6d00' },

View File

@@ -78,14 +78,12 @@ export default function NegateModal({ violation, onConfirm, onCancel }) {
});
};
// FIX: overlay click only closes on backdrop, NOT modal children
const handleOverlayClick = (e) => {
if (e.target === e.currentTarget && onCancel) onCancel();
};
return (
<div style={s.overlay} onClick={handleOverlayClick}>
{/* FIX: stopPropagation prevents modal clicks from bubbling to overlay */}
<div style={s.modal} onClick={(e) => e.stopPropagation()}>
<div style={s.header}>

View File

@@ -35,6 +35,7 @@ const s = {
const EMPTY_FORM = {
employeeId: '', employeeName: '', department: '', supervisor: '', witnessName: '',
violationType: '', incidentDate: '', incidentTime: '',
// TODO [MAJOR #6]: `amount` and `minutesLate` are rendered but never sent to the API
amount: '', minutesLate: '', location: '', additionalDetails: '', points: 1,
acknowledgedBy: '', acknowledgedDate: '',
};
@@ -43,7 +44,7 @@ export default function ViolationForm() {
const [employees, setEmployees] = useState([]);
const [form, setForm] = useState(EMPTY_FORM);
const [violation, setViolation] = useState(null);
const [status, setStatus] = useState(null);
const [status, setStatus] = useState(null); // TODO [MAJOR #7]: remove — toast covers this
const [lastViolId, setLastViolId] = useState(null);
const [pdfLoading, setPdfLoading] = useState(false);
@@ -108,6 +109,7 @@ export default function ViolationForm() {
setEmployees(empList.data);
toast.success(`Violation #${newId} recorded — click Download PDF to save the document.`);
// TODO [MAJOR #7]: remove setStatus — toast above already covers this message
setStatus({ ok: true, msg: `✓ Violation #${newId} recorded — click Download PDF to save the document.` });
setForm(EMPTY_FORM);
setViolation(null);

View File

@@ -11,6 +11,10 @@ app.use(cors());
app.use(express.json());
app.use(express.static(path.join(__dirname, 'client', 'dist')));
// TODO [CRITICAL #1]: No authentication on any route. Add an auth middleware
// (e.g. express-session + password, or JWT) before all /api/* routes.
// Anyone on the network can currently create, delete, or negate violations.
// ── Demo static route ─────────────────────────────────────────────────────────
// Serves the standalone stakeholder demo page at /demo/index.html
// Must be registered before the SPA catch-all below.
@@ -49,6 +53,13 @@ app.get('/api/employees', (req, res) => {
res.json(rows);
});
// GET /api/employees/:id — single employee record
app.get('/api/employees/:id', (req, res) => {
const emp = db.prepare('SELECT id, name, department, supervisor, notes FROM employees WHERE id = ?').get(req.params.id);
if (!emp) return res.status(404).json({ error: 'Employee not found' });
res.json(emp);
});
app.post('/api/employees', (req, res) => {
const { name, department, supervisor } = req.body;
if (!name) return res.status(400).json({ error: 'name is required' });
@@ -58,6 +69,9 @@ app.post('/api/employees', (req, res) => {
db.prepare('UPDATE employees SET department = COALESCE(?, department), supervisor = COALESCE(?, supervisor) WHERE id = ?')
.run(department || null, supervisor || null, existing.id);
}
// TODO [MINOR #16]: Spreading `existing` then overwriting with possibly-undefined
// `department`/`supervisor` returns `undefined` for unset fields.
// Re-query after update or only spread defined values.
return res.json({ ...existing, department, supervisor });
}
const result = db.prepare('INSERT INTO employees (name, department, supervisor) VALUES (?, ?, ?)')
@@ -290,6 +304,17 @@ app.post('/api/violations', (req, res) => {
// PATCH /api/violations/:id/amend — edit mutable fields; logs a diff per changed field
const AMENDABLE_FIELDS = ['incident_time', 'location', 'details', 'submitted_by', 'witness_name', 'acknowledged_by', 'acknowledged_date'];
// Pre-build one prepared UPDATE statement per amendable field combination is not
// practical (2^n combos), so instead we validate columns against the static
// whitelist and build the clause only from known-safe names at startup.
// The whitelist itself is the guard; no user-supplied column name ever enters SQL.
const AMEND_UPDATE_STMTS = Object.fromEntries(
AMENDABLE_FIELDS.map(f => [
f,
db.prepare(`UPDATE violations SET ${f} = ? WHERE id = ?`)
])
);
app.patch('/api/violations/:id/amend', (req, res) => {
const id = parseInt(req.params.id);
const { changed_by, ...updates } = req.body;
@@ -307,18 +332,14 @@ app.patch('/api/violations/:id/amend', (req, res) => {
}
const amendTransaction = db.transaction(() => {
// Build UPDATE
const setClauses = Object.keys(allowed).map(k => `${k} = ?`).join(', ');
const values = [...Object.values(allowed), id];
db.prepare(`UPDATE violations SET ${setClauses} WHERE id = ?`).run(...values);
// Insert an amendment record per changed field
const insertAmendment = db.prepare(`
INSERT INTO violation_amendments (violation_id, changed_by, field_name, old_value, new_value)
VALUES (?, ?, ?, ?, ?)
`);
for (const [field, newVal] of Object.entries(allowed)) {
const oldVal = violation[field];
// Use the pre-built statement for this field — no runtime interpolation
AMEND_UPDATE_STMTS[field].run(newVal, id);
if (String(oldVal) !== String(newVal)) {
insertAmendment.run(id, changed_by || null, field, oldVal ?? null, newVal ?? null);
}
@@ -391,6 +412,38 @@ app.delete('/api/violations/:id', (req, res) => {
res.json({ success: true });
});
// ── Violation counts per employee ────────────────────────────────────────────
// GET /api/employees/:id/violation-counts
// Returns { violation_type: count } for the rolling 90-day window (non-negated).
app.get('/api/employees/:id/violation-counts', (req, res) => {
const rows = db.prepare(`
SELECT violation_type, COUNT(*) AS count
FROM violations
WHERE employee_id = ?
AND negated = 0
AND incident_date >= DATE('now', '-90 days')
GROUP BY violation_type
`).all(req.params.id);
const result = {};
for (const r of rows) result[r.violation_type] = r.count;
res.json(result);
});
// GET /api/employees/:id/violation-counts/alltime
// Returns { violation_type: { count, max_points_used } } across all time (non-negated).
app.get('/api/employees/:id/violation-counts/alltime', (req, res) => {
const rows = db.prepare(`
SELECT violation_type, COUNT(*) AS count, MAX(points) AS max_points_used
FROM violations
WHERE employee_id = ?
AND negated = 0
GROUP BY violation_type
`).all(req.params.id);
const result = {};
for (const r of rows) result[r.violation_type] = { count: r.count, max_points_used: r.max_points_used };
res.json(result);
});
// ── Audit log ────────────────────────────────────────────────────────────────
app.get('/api/audit', (req, res) => {
const limit = Math.min(parseInt(req.query.limit) || 100, 500);