Code review and fixes
This commit is contained in:
65
server.js
65
server.js
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user