Fix duplicate chart entries on compliance page when multiple verticals share a report_date
Aggregate /trends, /top-recurring, /category-trend by report_date instead of per-upload row. Add sibling-upload disclosure to /summary. Filter persistUpload snapshot query by the upload's vertical to prevent cross-vertical contamination. Fixes GitLab #12 (reported by nkapur — STEAM active findings chart showed 3 entries for 5/11 after uploading three vertical data sets for that date). Includes 30 property-based tests covering bug condition and preservation.
This commit is contained in:
Jordan Ramos
@@ -78,8 +78,14 @@ async function computeDiff(incomingItems) {
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Write a parsed upload to the DB (within a transaction)
|
||||
//
|
||||
// `vertical` defaults to null for legacy AEO uploads (the /commit route).
|
||||
// When threaded through from a multi-vertical caller it filters the
|
||||
// compliance_snapshots aggregation so the snapshot reflects only the
|
||||
// snapshotted vertical's items — this prevents cross-vertical
|
||||
// contamination on dates where multiple verticals share a report_date.
|
||||
// ---------------------------------------------------------------------------
|
||||
async function persistUpload({ items, summary, reportDate, filename, userId }) {
|
||||
async function persistUpload({ items, summary, reportDate, filename, userId, vertical = null }) {
|
||||
const { rows: activeRows } = await pool.query(
|
||||
`SELECT id, hostname, metric_id, seen_count, first_seen_upload_id FROM compliance_items WHERE status = 'active'`
|
||||
);
|
||||
@@ -153,27 +159,38 @@ async function persistUpload({ items, summary, reportDate, filename, userId }) {
|
||||
try {
|
||||
const currentMonth = new Date().toISOString().slice(0, 7); // YYYY-MM
|
||||
|
||||
// Compute per-vertical compliance percentages from current state
|
||||
// Compute compliance percentages for the snapshotted vertical only.
|
||||
// `IS NOT DISTINCT FROM` matches the legacy `vertical IS NULL` case
|
||||
// when the upload is an AEO-only upload (vertical = null), so the
|
||||
// single-vertical-month preservation path keeps its previous
|
||||
// semantics.
|
||||
const { rows: verticalStats } = await pool.query(
|
||||
`SELECT team AS vertical,
|
||||
`SELECT vertical, team,
|
||||
COUNT(DISTINCT hostname)::int AS total_devices,
|
||||
COUNT(DISTINCT CASE WHEN status = 'resolved' THEN hostname END)::int AS compliant,
|
||||
COUNT(DISTINCT CASE WHEN status = 'active' THEN hostname END)::int AS non_compliant
|
||||
FROM compliance_items
|
||||
WHERE team IS NOT NULL
|
||||
GROUP BY team`
|
||||
WHERE team IS NOT NULL AND vertical IS NOT DISTINCT FROM $1
|
||||
GROUP BY vertical, team`,
|
||||
[vertical]
|
||||
);
|
||||
|
||||
for (const vs of verticalStats) {
|
||||
const total = vs.total_devices;
|
||||
const compPct = total > 0 ? Math.round((vs.compliant / total) * 100 * 100) / 100 : 0;
|
||||
// For non-null verticals (multi-vertical uploads), the snapshot
|
||||
// row is keyed by the actual vertical so /vcl/stats consumers
|
||||
// see the correct breakdown. For legacy AEO uploads (vertical
|
||||
// is null), preserve the historical team-as-vertical key so
|
||||
// existing single-vertical-month consumers are unchanged.
|
||||
const snapshotVertical = vs.vertical || vs.team;
|
||||
|
||||
await pool.query(
|
||||
`INSERT INTO compliance_snapshots (snapshot_month, vertical, total_devices, compliant, non_compliant, compliance_pct)
|
||||
VALUES ($1, $2, $3, $4, $5, $6)
|
||||
ON CONFLICT (snapshot_month, vertical)
|
||||
DO UPDATE SET total_devices = $3, compliant = $4, non_compliant = $5, compliance_pct = $6`,
|
||||
[currentMonth, vs.vertical, total, vs.compliant, vs.non_compliant, compPct]
|
||||
[currentMonth, snapshotVertical, total, vs.compliant, vs.non_compliant, compPct]
|
||||
);
|
||||
}
|
||||
} catch (snapshotErr) {
|
||||
@@ -488,9 +505,15 @@ function createComplianceRouter(upload) {
|
||||
* Returns the summary data from the most recent compliance upload, optionally filtered by team.
|
||||
*
|
||||
* @query team — optional, one of STEAM | ACCESS-ENG | ACCESS-OPS | INTELDEV
|
||||
* @response 200 { entries: Array, overall_scores: object, upload: { id, report_date, uploaded_at } | null }
|
||||
* @response 200 { entries: Array, overall_scores: object, upload: { id, report_date, uploaded_at } | null, multi_vertical_uploads?: Array<{ id, vertical, uploaded_at }> }
|
||||
* @response 400 { error } — invalid team
|
||||
* @response 500 { error } — database error
|
||||
*
|
||||
* When two or more uploads share the latest `report_date` (multi-vertical
|
||||
* upload day), the `multi_vertical_uploads` field discloses the sibling
|
||||
* uploads (id/vertical/uploaded_at) so callers know the response is
|
||||
* partial. The field is omitted on single-upload-per-date dates to
|
||||
* preserve the legacy response shape.
|
||||
*/
|
||||
router.get('/summary', async (req, res) => {
|
||||
const team = req.query.team;
|
||||
@@ -515,7 +538,28 @@ function createComplianceRouter(upload) {
|
||||
let entries = summary.entries || [];
|
||||
if (team) entries = entries.filter(e => e.team === team);
|
||||
|
||||
res.json({ entries, overall_scores: summary.overall_scores || {}, upload: { id: latestUpload.id, report_date: latestUpload.report_date, uploaded_at: latestUpload.uploaded_at } });
|
||||
// Disclose sibling uploads sharing the same report_date so callers
|
||||
// know the response is a single vertical's view of a multi-vertical
|
||||
// day. Field is omitted when no siblings exist (preserves legacy
|
||||
// single-upload-per-date response shape).
|
||||
const { rows: siblingRows } = await pool.query(
|
||||
`SELECT id, vertical, uploaded_at FROM compliance_uploads WHERE report_date = $1 AND id != $2 ORDER BY id ASC`,
|
||||
[latestUpload.report_date, latestUpload.id]
|
||||
);
|
||||
|
||||
const response = {
|
||||
entries,
|
||||
overall_scores: summary.overall_scores || {},
|
||||
upload: { id: latestUpload.id, report_date: latestUpload.report_date, uploaded_at: latestUpload.uploaded_at },
|
||||
};
|
||||
if (siblingRows.length > 0) {
|
||||
response.multi_vertical_uploads = siblingRows.map(s => ({
|
||||
id: s.id,
|
||||
vertical: s.vertical,
|
||||
uploaded_at: s.uploaded_at,
|
||||
}));
|
||||
}
|
||||
res.json(response);
|
||||
} catch (err) {
|
||||
console.error('[Compliance] GET /summary error:', err.message);
|
||||
res.status(500).json({ error: 'Database error' });
|
||||
@@ -768,19 +812,31 @@ function createComplianceRouter(upload) {
|
||||
router.get('/trends', async (req, res) => {
|
||||
try {
|
||||
const { rows: uploads } = await pool.query(
|
||||
`SELECT id, report_date, COALESCE(new_count, 0) AS new_count, COALESCE(recurring_count, 0) AS recurring_count, COALESCE(resolved_count, 0) AS resolved_count, COALESCE(new_count, 0) + COALESCE(recurring_count, 0) AS total_active FROM compliance_uploads ORDER BY report_date ASC`
|
||||
`SELECT report_date,
|
||||
SUM(COALESCE(new_count, 0))::int AS new_count,
|
||||
SUM(COALESCE(recurring_count, 0))::int AS recurring_count,
|
||||
SUM(COALESCE(resolved_count, 0))::int AS resolved_count,
|
||||
SUM(COALESCE(new_count, 0) + COALESCE(recurring_count, 0))::int AS total_active
|
||||
FROM compliance_uploads
|
||||
WHERE report_date IS NOT NULL
|
||||
GROUP BY report_date
|
||||
ORDER BY report_date ASC`
|
||||
);
|
||||
if (uploads.length === 0) return res.json({ trends: [] });
|
||||
|
||||
const { rows: teamRows } = await pool.query(
|
||||
`SELECT ci.upload_id, ci.team, COUNT(ci.id)::int AS count FROM compliance_items ci WHERE ci.team IS NOT NULL GROUP BY ci.upload_id, ci.team`
|
||||
`SELECT cu.report_date, ci.team, COUNT(ci.id)::int AS count
|
||||
FROM compliance_items ci
|
||||
JOIN compliance_uploads cu ON ci.upload_id = cu.id
|
||||
WHERE ci.team IS NOT NULL AND cu.report_date IS NOT NULL
|
||||
GROUP BY cu.report_date, ci.team`
|
||||
);
|
||||
const teamMap = {};
|
||||
teamRows.forEach(r => { if (!teamMap[r.upload_id]) teamMap[r.upload_id] = {}; teamMap[r.upload_id][r.team] = r.count; });
|
||||
teamRows.forEach(r => { if (!teamMap[r.report_date]) teamMap[r.report_date] = {}; teamMap[r.report_date][r.team] = r.count; });
|
||||
|
||||
const trends = uploads.map(u => ({
|
||||
report_date: u.report_date, new_count: u.new_count, recurring_count: u.recurring_count, resolved_count: u.resolved_count, total_active: u.total_active,
|
||||
STEAM: teamMap[u.id]?.STEAM || 0, 'ACCESS-ENG': teamMap[u.id]?.['ACCESS-ENG'] || 0, 'ACCESS-OPS': teamMap[u.id]?.['ACCESS-OPS'] || 0, INTELDEV: teamMap[u.id]?.INTELDEV || 0,
|
||||
STEAM: teamMap[u.report_date]?.STEAM || 0, 'ACCESS-ENG': teamMap[u.report_date]?.['ACCESS-ENG'] || 0, 'ACCESS-OPS': teamMap[u.report_date]?.['ACCESS-OPS'] || 0, INTELDEV: teamMap[u.report_date]?.INTELDEV || 0,
|
||||
}));
|
||||
res.json({ trends });
|
||||
} catch (err) {
|
||||
@@ -818,7 +874,14 @@ function createComplianceRouter(upload) {
|
||||
router.get('/top-recurring', async (req, res) => {
|
||||
try {
|
||||
const { rows } = await pool.query(
|
||||
`SELECT id, report_date, COALESCE(new_count, 0) AS new_count, COALESCE(recurring_count, 0) AS recurring_count, COALESCE(resolved_count, 0) AS resolved_count FROM compliance_uploads ORDER BY report_date ASC`
|
||||
`SELECT report_date,
|
||||
SUM(COALESCE(new_count, 0))::int AS new_count,
|
||||
SUM(COALESCE(recurring_count, 0))::int AS recurring_count,
|
||||
SUM(COALESCE(resolved_count, 0))::int AS resolved_count
|
||||
FROM compliance_uploads
|
||||
WHERE report_date IS NOT NULL
|
||||
GROUP BY report_date
|
||||
ORDER BY report_date ASC`
|
||||
);
|
||||
const waterfall = computeWaterfall(rows);
|
||||
res.json({ waterfall });
|
||||
@@ -838,9 +901,13 @@ function createComplianceRouter(upload) {
|
||||
router.get('/category-trend', async (req, res) => {
|
||||
try {
|
||||
const { rows } = await pool.query(
|
||||
`SELECT cu.report_date, COALESCE(ci.category, 'Unknown') AS category, COUNT(ci.id)::int AS count
|
||||
`SELECT cu.report_date,
|
||||
COALESCE(ci.category, 'Unknown') AS category,
|
||||
COUNT(ci.id)::int AS count
|
||||
FROM compliance_uploads cu JOIN compliance_items ci ON ci.upload_id = cu.id
|
||||
GROUP BY cu.id, cu.report_date, category ORDER BY cu.report_date ASC`
|
||||
WHERE cu.report_date IS NOT NULL
|
||||
GROUP BY cu.report_date, COALESCE(ci.category, 'Unknown')
|
||||
ORDER BY cu.report_date ASC, category ASC`
|
||||
);
|
||||
res.json({ categoryTrend: rows });
|
||||
} catch (err) {
|
||||
@@ -852,11 +919,12 @@ function createComplianceRouter(upload) {
|
||||
/**
|
||||
* PATCH /items/:hostname/metadata
|
||||
* Updates resolution_date and/or remediation_plan for all active compliance items matching a hostname.
|
||||
* Records field-level change history in compliance_item_history for each modified field.
|
||||
*
|
||||
* @param hostname — the device hostname
|
||||
* @body { resolution_date?: string|null, remediation_plan?: string|null }
|
||||
* @body { resolution_date?: string|null, remediation_plan?: string|null, change_reason?: string|null }
|
||||
* @response 200 { updated: number }
|
||||
* @response 400 { error } — invalid hostname, invalid date format, plan exceeds 2000 chars, or no fields provided
|
||||
* @response 400 { error } — invalid hostname, invalid date format, plan exceeds 2000 chars, change_reason exceeds 500 chars, or no fields provided
|
||||
* @response 404 { error } — device not found
|
||||
* @response 500 { error } — update failure
|
||||
*/
|
||||
@@ -979,6 +1047,8 @@ function createComplianceRouter(upload) {
|
||||
}
|
||||
});
|
||||
|
||||
const VCL_TARGET_PCT = parseInt(process.env.VCL_TARGET_PCT, 10) || 95;
|
||||
|
||||
/**
|
||||
* GET /vcl/stats
|
||||
* Returns VCL executive summary statistics including device counts, compliance percentage,
|
||||
@@ -987,8 +1057,6 @@ function createComplianceRouter(upload) {
|
||||
* @response 200 { stats: { total_devices, in_scope, compliant, non_compliant, remediations_required, compliance_pct, target_pct }, donut: { blocked: { count, pct }, in_progress: { count, pct } }, heavy_hitters: Array<{ vertical, team, non_compliant, compliance_date, notes }>, vertical_breakdown: Array<{ vertical, compliance_pct, team, non_compliant, actual_burndown, forecast_burndown, blockers, risk_acceptances, notes }> }
|
||||
* @response 500 { error } — database error
|
||||
*/
|
||||
const VCL_TARGET_PCT = parseInt(process.env.VCL_TARGET_PCT, 10) || 95;
|
||||
|
||||
router.get('/vcl/stats', async (req, res) => {
|
||||
try {
|
||||
// Compute device-level stats using DISTINCT hostname
|
||||
@@ -1520,4 +1588,4 @@ function createComplianceRouter(upload) {
|
||||
return router;
|
||||
}
|
||||
|
||||
module.exports = { createComplianceRouter, bucketAgingItems, computeWaterfall };
|
||||
module.exports = { createComplianceRouter, bucketAgingItems, computeWaterfall, persistUpload };
|
||||
|
||||
Reference in New Issue
Block a user