From f141fa58a18f9a48a898439bc5c22d46819c49d8 Mon Sep 17 00:00:00 2001 From: jramos Date: Thu, 16 Apr 2026 14:28:44 -0600 Subject: [PATCH] Add multi-metric note selection to compliance detail panel --- .../.config.kiro | 1 + .../compliance-multi-metric-notes/design.md | 290 ++++++++++++++++++ .../requirements.md | 85 +++++ .../compliance-multi-metric-notes/tasks.md | 105 +++++++ .../add_compliance_notes_group_id.js | 29 ++ backend/routes/compliance.js | 77 +++-- .../components/pages/ComplianceDetailPanel.js | 154 +++++++--- 7 files changed, 684 insertions(+), 57 deletions(-) create mode 100644 .kiro/specs/compliance-multi-metric-notes/.config.kiro create mode 100644 .kiro/specs/compliance-multi-metric-notes/design.md create mode 100644 .kiro/specs/compliance-multi-metric-notes/requirements.md create mode 100644 .kiro/specs/compliance-multi-metric-notes/tasks.md create mode 100644 backend/migrations/add_compliance_notes_group_id.js diff --git a/.kiro/specs/compliance-multi-metric-notes/.config.kiro b/.kiro/specs/compliance-multi-metric-notes/.config.kiro new file mode 100644 index 0000000..580fd9e --- /dev/null +++ b/.kiro/specs/compliance-multi-metric-notes/.config.kiro @@ -0,0 +1 @@ +{"specId": "8ec01dea-8d5c-40c1-8778-ec2992adb37f", "workflowType": "requirements-first", "specType": "feature"} \ No newline at end of file diff --git a/.kiro/specs/compliance-multi-metric-notes/design.md b/.kiro/specs/compliance-multi-metric-notes/design.md new file mode 100644 index 0000000..7f82a1f --- /dev/null +++ b/.kiro/specs/compliance-multi-metric-notes/design.md @@ -0,0 +1,290 @@ +# Design Document: Multi-Metric Notes for Compliance Detail Panel + +## Overview + +This feature extends the compliance notes system so that a single note can be associated with multiple metrics in one action. Today, the `ComplianceDetailPanel` uses a single-select `` dropdown with a chip-based toggle list. Each active metric is rendered as a clickable `MetricChip`. Selected chips get a highlighted border/background. A "Select All" / "Deselect All" toggle appears when there are 2+ active metrics. + +2. **Submission logic** — `handleAddNote` sends `metric_ids` (array of selected metric IDs) instead of `metric_id` (single string). The submit button is disabled when no metrics are selected or note text is empty. + +3. **Note display grouping** — notes are grouped by `group_id` before rendering. Notes sharing a `group_id` are displayed as a single card with multiple `MetricChip` badges. Notes without a `group_id` (pre-migration legacy) render as individual entries, same as today. + +**Component structure:** + +``` +ComplianceDetailPanel +├── Header (hostname, IP, device type, team) +├── Section: Failing Metrics +│ └── MetricRow (per active metric) +├── Section: Resolved Metrics +│ └── MetricRow (per resolved metric) +├── Section: History +│ └── MetricChip + seen count (per active metric) +└── Section: Notes + ├── NoteCard (per group_id group, shows multiple MetricChips if multi-metric) + └── Add Note Form + ├── MetricChipSelector (multi-select chip toggles) + │ ├── MetricChip (per active metric, clickable) + │ └── Select All / Deselect All toggle + ├── Textarea (note text) + └── Send button (disabled when no metrics selected or text empty) +``` + +**MetricChipSelector behavior:** + +| State | Behavior | +|---|---| +| 1 active metric | Chip is pre-selected and non-removable. No Select All toggle. | +| 2+ active metrics, panel just opened | First metric pre-selected. Select All toggle visible. | +| User clicks unselected chip | Chip added to selection | +| User clicks selected chip (2+ selected) | Chip removed from selection | +| User clicks selected chip (only 1 selected, 2+ metrics exist) | No-op — at least one must remain selected | +| Select All clicked | All active metrics selected, toggle label changes to "Deselect All" | +| Deselect All clicked | All metrics deselected except the first (to maintain minimum selection) | + +**Design rationale — minimum selection of 1:** The submit button is disabled when no metrics are selected (Requirement 3.4). Rather than allowing the user to reach an empty state and see a disabled button, "Deselect All" keeps the first metric selected. This matches the current UX where a metric is always selected. + +## Data Models + +### compliance_notes table (modified) + +| Column | Type | Description | +|---|---|---| +| `id` | INTEGER PK | Auto-increment row ID | +| `hostname` | TEXT NOT NULL | Device hostname | +| `metric_id` | TEXT NOT NULL | Compliance metric ID | +| `note` | TEXT NOT NULL | Note text (max 1000 chars) | +| `group_id` | TEXT | Batch identifier — rows from the same submission share this value | +| `created_by` | INTEGER FK | User ID of the note author | +| `created_at` | DATETIME | Timestamp of creation | + +The `group_id` is a UUID v4 string generated server-side via `crypto.randomUUID()`. Single-metric submissions also receive a `group_id` so the frontend grouping logic is uniform. + +**Index:** `idx_compliance_notes_group ON compliance_notes(group_id)` — supports the frontend's grouping query. + +### API Response Shape + +`POST /api/compliance/notes` response (201): + +```json +{ + "notes": [ + { + "id": 42, + "hostname": "SERVER01", + "metric_id": "2.1.1", + "note": "Vendor ticket VT-1234 opened", + "group_id": "a1b2c3d4-...", + "created_at": "2025-01-15 14:30:00", + "created_by": "jsmith" + }, + { + "id": 43, + "hostname": "SERVER01", + "metric_id": "2.3.2", + "note": "Vendor ticket VT-1234 opened", + "group_id": "a1b2c3d4-...", + "created_at": "2025-01-15 14:30:00", + "created_by": "jsmith" + } + ] +} +``` + +`GET /api/compliance/items/:hostname` response — the existing `notes` array now includes `group_id`: + +```json +{ + "notes": [ + { "id": 43, "metric_id": "2.3.2", "note": "...", "group_id": "a1b2c3d4-...", "created_at": "...", "created_by": "jsmith" }, + { "id": 42, "metric_id": "2.1.1", "note": "...", "group_id": "a1b2c3d4-...", "created_at": "...", "created_by": "jsmith" }, + { "id": 10, "metric_id": "2.1.1", "note": "...", "group_id": "legacy-10", "created_at": "...", "created_by": "admin" } + ] +} +``` + +The frontend groups consecutive notes by `group_id` to render multi-metric notes as a single card. + +## Correctness Properties + +*A property is a characteristic or behavior that should hold true across all valid executions of a system — essentially, a formal statement about what the system should do. Properties serve as the bridge between human-readable specifications and machine-verifiable correctness guarantees.* + +### Property 1: Select All / Deselect All round-trip + +*For any* set of active metrics with size > 1, clicking "Select All" should result in all metrics being selected, and then clicking "Deselect All" should result in only the first metric remaining selected (minimum selection invariant). + +**Validates: Requirements 2.1, 2.2** + +### Property 2: Toggle label reflects selection state + +*For any* set of active metrics, if the user manually selects every metric one by one, the toggle label should read "Deselect All" — the label is a pure function of whether all metrics are selected, regardless of how that state was reached. + +**Validates: Requirements 2.3** + +### Property 3: Multi-metric submission creates correct rows with shared group_id + +*For any* valid hostname, non-empty note text, and non-empty array of valid metric IDs, submitting a note should create exactly N rows in `compliance_notes` (where N = length of the metric IDs array), all sharing the same `note` text, `created_by` user, `created_at` timestamp, and `group_id` value. + +**Validates: Requirements 3.1, 3.2, 5.3, 5.7, 6.1** + +### Property 4: Whitespace-only notes are rejected + +*For any* string composed entirely of whitespace characters (spaces, tabs, newlines, or combinations thereof), the Notes API should reject the submission with a 400 error and create zero rows in the database. + +**Validates: Requirements 3.3** + +### Property 5: Atomic validation — invalid metric IDs reject the entire batch + +*For any* array of metric IDs where at least one entry is invalid (empty string, exceeds 50 characters, or non-string), the Notes API should reject the entire request with a 400 error and insert zero rows, even if all other entries are valid. + +**Validates: Requirements 5.2, 5.6** + +### Property 6: Note grouping display + +*For any* set of notes where multiple notes share the same `group_id`, the Detail Panel should render them as a single note entry displaying all associated Metric Chips, rather than as separate entries. + +**Validates: Requirements 4.1, 4.2, 6.4** + +### Property 7: Reverse chronological note ordering + +*For any* set of notes with varying `created_at` timestamps and group sizes, the Detail Panel should display note groups in reverse chronological order (newest `created_at` first), regardless of how many metrics each group covers. + +**Validates: Requirements 4.3** + +## Error Handling + +### Backend + +| Scenario | Response | Behavior | +|---|---|---| +| Empty or whitespace-only note text | 400 `{ error: "Note cannot be empty" }` | No rows inserted | +| `metric_ids` is empty array | 400 `{ error: "At least one metric ID is required" }` | No rows inserted | +| Any metric ID in array is empty or >50 chars | 400 `{ error: "Invalid metric_id at index N" }` | No rows inserted (atomic rejection) | +| `metric_ids` is not an array (when provided) | 400 `{ error: "metric_ids must be an array" }` | Falls back to checking `metric_id` | +| Neither `metric_id` nor `metric_ids` provided | 400 `{ error: "metric_id or metric_ids is required" }` | No rows inserted | +| Database error during transaction | 500 `{ error: "Failed to save note" }` | Transaction rolled back, no partial inserts | +| Invalid hostname format | 400 `{ error: "Invalid hostname format" }` | No rows inserted (unchanged) | + +Transaction safety: all inserts for a multi-metric note happen inside `BEGIN TRANSACTION` / `COMMIT`. If any insert fails, the transaction is rolled back and no rows are persisted. + +### Frontend + +| Scenario | Behavior | +|---|---| +| API returns 400 validation error | Display error message below the note input (existing `noteError` state) | +| API returns 500 server error | Display error message below the note input | +| Network failure | Display "Failed to save note" error | +| No metrics selected | Submit button is disabled, no API call made | +| Successful submission | Clear note text, refresh notes list, retain metric selection | + +## Testing Strategy + +### Unit Tests (example-based) + +- **Backend:** + - Legacy `metric_id` field still creates a single note row (backward compatibility) + - Both `metric_id` and `metric_ids` provided — `metric_ids` takes precedence + - Single active metric pre-selects and is non-removable + - Response shape includes all created rows with `group_id` and `username` + +- **Frontend:** + - MetricChipSelector renders correct number of chips for given active metrics + - Clicking a chip toggles its selection state + - Submit button disabled when note text is empty or no metrics selected + - Notes without `group_id` (legacy) render as individual entries + - Single active metric auto-selects and hides Select All toggle + +### Property-Based Tests + +Property-based tests use `fast-check` (JavaScript PBT library) with a minimum of 100 iterations per property. + +Each property test is tagged with a comment referencing the design property: +- **Feature: compliance-multi-metric-notes, Property 3: Multi-metric submission creates correct rows with shared group_id** +- **Feature: compliance-multi-metric-notes, Property 4: Whitespace-only notes are rejected** +- **Feature: compliance-multi-metric-notes, Property 5: Atomic validation — invalid metric IDs reject the entire batch** + +Backend properties (3, 4, 5) are tested against the route handler using a test SQLite database. Frontend properties (1, 2, 6, 7) are tested against the component rendering/grouping logic using React Testing Library with generated inputs. + +### Integration Tests + +- End-to-end flow: open detail panel → select multiple metrics → submit note → verify grouped display +- Migration script: verify `group_id` column exists and legacy rows are backfilled +- Backward compatibility: existing `GET /items/:hostname` response includes `group_id` field on notes diff --git a/.kiro/specs/compliance-multi-metric-notes/requirements.md b/.kiro/specs/compliance-multi-metric-notes/requirements.md new file mode 100644 index 0000000..459250e --- /dev/null +++ b/.kiro/specs/compliance-multi-metric-notes/requirements.md @@ -0,0 +1,85 @@ +# Requirements Document + +## Introduction + +The compliance detail panel currently allows users to add notes to a single metric at a time via a dropdown selector. When a remediation action, vendor ticket, or status update applies to multiple metrics on the same device, users must repeat the same note for each metric individually. This feature adds multi-metric selection to the note creation flow so that a single note can be associated with multiple metrics in one action, while preserving the existing per-metric note history and display. + +## Glossary + +- **Detail_Panel**: The slide-out panel (`ComplianceDetailPanel.js`) that opens when a user clicks a device row on the Compliance page. It displays failing metrics, resolved metrics, upload history, and notes for a single hostname. +- **Note**: A timestamped, user-attributed text entry stored in the `compliance_notes` table, keyed on `(hostname, metric_id)`. Notes persist across uploads and form a historical record. +- **Metric_Selector**: The UI control in the Detail_Panel's notes section that allows the user to choose which metric(s) a note applies to. Currently a single-select dropdown; this feature replaces it with a multi-select control. +- **Metric_Chip**: A small colored badge displaying a metric ID, used throughout the compliance UI to visually identify metrics by category color. +- **Notes_API**: The `POST /api/compliance/notes` endpoint that persists a note to the database. +- **Active_Metric**: A compliance item with `status = 'active'` for the selected hostname — these are the metrics currently failing. + +## Requirements + +### Requirement 1: Multi-Metric Selection UI + +**User Story:** As a compliance analyst, I want to select multiple metrics when adding a note, so that I can document a single remediation action that covers several metrics without repeating myself. + +#### Acceptance Criteria + +1. WHEN the Detail_Panel is open for a hostname with more than one Active_Metric, THE Metric_Selector SHALL display all Active_Metrics as individually selectable options. +2. WHEN the user interacts with the Metric_Selector, THE Metric_Selector SHALL allow the user to select one or more Active_Metrics simultaneously. +3. WHEN the Detail_Panel is open for a hostname with exactly one Active_Metric, THE Metric_Selector SHALL pre-select that metric and remain visible as a single non-removable selection. +4. WHEN the Detail_Panel first opens for a hostname with multiple Active_Metrics, THE Metric_Selector SHALL pre-select the first Active_Metric by default. +5. THE Metric_Selector SHALL display each option using the Metric_Chip component with the metric's category color, so that metrics are visually distinguishable. + +### Requirement 2: Select All / Deselect All + +**User Story:** As a compliance analyst, I want a quick way to select or deselect all metrics, so that I can efficiently apply a note to every failing metric on a device. + +#### Acceptance Criteria + +1. WHEN the hostname has more than one Active_Metric, THE Metric_Selector SHALL display a "Select All" toggle that selects all Active_Metrics when activated. +2. WHEN all Active_Metrics are already selected, THE "Select All" toggle SHALL change to "Deselect All" and deselect all Active_Metrics when activated. +3. WHEN the user manually selects all Active_Metrics one by one, THE toggle label SHALL update to "Deselect All" to reflect the current state. + +### Requirement 3: Multi-Metric Note Submission + +**User Story:** As a compliance analyst, I want the system to save my note against all selected metrics in one action, so that the historical record accurately reflects which metrics the note covers. + +#### Acceptance Criteria + +1. WHEN the user submits a note with multiple metrics selected, THE Notes_API SHALL create one `compliance_notes` row per selected metric, all sharing the same note text, `created_by`, and `created_at` timestamp. +2. WHEN the user submits a note with a single metric selected, THE Notes_API SHALL create exactly one `compliance_notes` row, preserving backward compatibility with the existing behavior. +3. IF the note text is empty or contains only whitespace, THEN THE Notes_API SHALL reject the submission and return a validation error. +4. IF no metrics are selected, THEN THE Detail_Panel SHALL disable the submit button and prevent submission. +5. WHEN a multi-metric note is successfully saved, THE Detail_Panel SHALL clear the note text field, refresh the notes list, and retain the current metric selection. + +### Requirement 4: Multi-Metric Note Display + +**User Story:** As a compliance analyst, I want to see which metrics a note was applied to, so that I can understand the scope of past remediation actions. + +#### Acceptance Criteria + +1. WHEN a note was submitted for multiple metrics simultaneously, THE Detail_Panel SHALL display all associated Metric_Chips together on that note entry, visually grouped. +2. WHEN a note was submitted for a single metric, THE Detail_Panel SHALL continue to display a single Metric_Chip on that note entry, matching the current behavior. +3. THE Detail_Panel SHALL display notes in reverse chronological order, with the newest note first, regardless of how many metrics each note covers. + +### Requirement 5: Backend Multi-Metric Notes Endpoint + +**User Story:** As a developer, I want the notes API to accept an array of metric IDs, so that the frontend can submit a note for multiple metrics in a single request. + +#### Acceptance Criteria + +1. THE Notes_API SHALL accept a `metric_ids` field (array of strings) in the request body as an alternative to the existing `metric_id` field (single string). +2. WHEN `metric_ids` is provided, THE Notes_API SHALL validate that each entry is a non-empty string of 50 characters or fewer. +3. WHEN `metric_ids` is provided, THE Notes_API SHALL insert one `compliance_notes` row per metric ID, all within the same database transaction, sharing the same `created_at` timestamp. +4. WHEN the legacy `metric_id` field is provided instead of `metric_ids`, THE Notes_API SHALL continue to function as before, inserting a single row. +5. IF both `metric_id` and `metric_ids` are provided, THEN THE Notes_API SHALL use `metric_ids` and ignore `metric_id`. +6. IF any metric ID in the `metric_ids` array fails validation, THEN THE Notes_API SHALL reject the entire request and return a 400 error without inserting any rows. +7. THE Notes_API SHALL return all created note rows in the response, so the frontend can update the display without a separate fetch. + +### Requirement 6: Note Grouping Identifier + +**User Story:** As a developer, I want notes that were created together to share a group identifier, so that the frontend can visually group multi-metric notes in the display. + +#### Acceptance Criteria + +1. WHEN multiple notes are created from a single submission, THE Notes_API SHALL assign the same `group_id` value to all rows in that batch. +2. WHEN a single note is created, THE Notes_API SHALL assign a unique `group_id` to that row. +3. THE `group_id` SHALL be stored as a text column in the `compliance_notes` table. +4. THE Detail_Panel SHALL use the `group_id` to visually group notes that were submitted together, displaying them as a single note entry with multiple Metric_Chips rather than as separate entries. diff --git a/.kiro/specs/compliance-multi-metric-notes/tasks.md b/.kiro/specs/compliance-multi-metric-notes/tasks.md new file mode 100644 index 0000000..d237979 --- /dev/null +++ b/.kiro/specs/compliance-multi-metric-notes/tasks.md @@ -0,0 +1,105 @@ +# Implementation Plan: Multi-Metric Notes for Compliance Detail Panel + +## Overview + +Extend the compliance notes system so a single note can be associated with multiple metrics in one action. Changes span three layers: a new migration script adding `group_id` to `compliance_notes`, updates to the `POST /notes` endpoint in `backend/routes/compliance.js` to accept `metric_ids` (array) and insert rows transactionally, and frontend changes in `ComplianceDetailPanel.js` to replace the single-select dropdown with a multi-select chip selector and group notes by `group_id` in the display. + +## Tasks + +- [x] 1. Create database migration for `group_id` column + - [x] 1.1 Create `backend/migrations/add_compliance_notes_group_id.js` + - Add `group_id TEXT` column to `compliance_notes` table via `ALTER TABLE` + - Create index `idx_compliance_notes_group` on `compliance_notes(group_id)` + - Backfill existing rows: `UPDATE compliance_notes SET group_id = 'legacy-' || id WHERE group_id IS NULL` + - Follow the existing migration pattern (sqlite3, serialize, console logging) + - _Requirements: 6.1, 6.2, 6.3_ + +- [x] 2. Update `POST /notes` endpoint to support multi-metric submissions + - [x] 2.1 Modify the `POST /notes` handler in `backend/routes/compliance.js` to accept `metric_ids` array + - Accept `metric_ids` (array of strings) as an alternative to `metric_id` (single string) + - When both are provided, `metric_ids` takes precedence + - When neither is provided, return 400 with `"metric_id or metric_ids is required"` + - When `metric_ids` is provided but is not an array, return 400 with `"metric_ids must be an array"` + - Normalize single `metric_id` into a one-element array internally so the rest of the logic is uniform + - _Requirements: 5.1, 5.4, 5.5_ + + - [x] 2.2 Add validation for `metric_ids` array entries + - Validate that `metric_ids` has at least one entry; return 400 with `"At least one metric ID is required"` if empty + - Validate each entry is a non-empty string of 50 characters or fewer; return 400 with `"Invalid metric_id at index N"` on failure + - Reject the entire request if any entry fails validation (atomic rejection, no partial inserts) + - _Requirements: 5.2, 5.6_ + + - [x] 2.3 Implement transactional multi-row insert with `group_id` + - Generate a `group_id` using `crypto.randomUUID()` for each submission (single or multi) + - Wrap all inserts in `BEGIN TRANSACTION` / `COMMIT` with `ROLLBACK` on error + - Insert one `compliance_notes` row per metric ID, all sharing the same `note`, `group_id`, `created_by`, and `created_at` + - _Requirements: 3.1, 3.2, 5.3, 6.1, 6.2_ + + - [x] 2.4 Update the response to return all created note rows + - After commit, query all created rows (joined with `users` for `username`) and return as `{ notes: [...] }` + - Each row includes `id`, `hostname`, `metric_id`, `note`, `group_id`, `created_at`, `created_by` + - Return HTTP 201 status + - _Requirements: 5.7_ + +- [x] 3. Update `GET /items/:hostname` to include `group_id` in notes response + - Add `cn.group_id` to the SELECT in the notes query within the `GET /items/:hostname` handler + - The existing query already fetches notes for the hostname; just add the column + - No other changes to this endpoint + - _Requirements: 6.3, 6.4_ + +- [x] 4. Checkpoint — Verify backend changes + - Ensure all backend changes are syntactically correct, ask the user if questions arise. + +- [x] 5. Replace single-select dropdown with multi-select MetricChipSelector in `ComplianceDetailPanel.js` + - [x] 5.1 Replace `noteMetric` (string) state with `selectedMetrics` (array) state + - Initialize `selectedMetrics` with the first active metric's ID when detail loads (matching current default behavior) + - When there is exactly one active metric, pre-select it as a non-removable selection + - _Requirements: 1.3, 1.4_ + + - [x] 5.2 Build the multi-select chip-based metric selector UI + - Replace the existing ` setNoteMetric(e.target.value)} - style={{ - width: '100%', marginBottom: '0.5rem', - background: 'rgba(15,23,42,0.8)', border: '1px solid rgba(20,184,166,0.25)', - borderRadius: '0.25rem', color: '#CBD5E1', - padding: '0.4rem 0.5rem', fontSize: '0.75rem', fontFamily: 'monospace', - }}> - {activeMetrics.map(m => ( - - ))} - - )} + {activeMetrics.length > 1 && (() => { + const allSelected = activeMetrics.length > 0 && activeMetrics.every(m => selectedMetrics.includes(m.metric_id)); + return ( +
+
+ + Metrics + + +
+
+ {activeMetrics.map(m => { + const isSelected = selectedMetrics.includes(m.metric_id); + const color = categoryColor(m.category); + return ( + + ); + })} +
+
+ ); + })()}