[TTAHUB-5331] Create calculated_category and make services use it#3675
Conversation
|
✅ Diff size: 169 lines — within the 500-line guideline. |
|
✅ Review count: 2 human approvals — thewatermethod, AdamAdHocTeam. |
There was a problem hiding this comment.
Pull request overview
This PR centralizes “finding category” selection logic by adding a new Citations.calculated_category field (preferring MonitoringFindings.source when present, otherwise falling back to MonitoringStandards.guidance) and updates monitoring widgets/services to consistently use that derived value.
Changes:
- Add
calculated_categoryto the Citations fact table population (and to the Citations table/model), including updatingFindingCategoriesto key offcalculated_category. - Update monitoring widgets/queries and sort logic to use
citation.calculated_categoryinstead ofcitation.guidance_category. - Update tests and monitoring fact table documentation to reflect the new canonical category source.
Impact assessment
- Benefits: Medium — removes duplicated “which field is category?” logic and aligns output with updated IT-AMS guidance.
- Risks: Medium — touches schema/migration ordering and the monitoring fact-table update pipeline; deployment/migration sequencing should be validated carefully.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/widgets/monitoring/reportCountByFindingCategory.ts | Aggregates counts by calculated_category instead of guidance_category. |
| src/widgets/monitoring/reportCountByFindingCategory.test.js | Updates mocks/expectations and test descriptions for calculated_category. |
| src/widgets/monitoring/monitoringTta.ts | Switches category selection + sorting to citation.calculated_category. |
| src/widgets/monitoring/monitoringTta.test.js | Updates fixtures/assertions to use calculated_category. |
| src/tools/updateMonitoringFactTables.ts | Computes/populates calculated_category; updates FindingCategories maintenance and Citations upsert accordingly; includes a defensive column-existence guard. |
| src/tools/updateMonitoringFactTables.test.js | Adds assertions verifying precedence/fallback behavior for calculated_category. |
| src/models/citation.js | Adds Sequelize attribute for calculated_category. |
| src/migrations/20260602001049-add_calculated_category_to_citations.js | Adds the calculated_category column with IF NOT EXISTS guard. |
| docs/monitoring-fact-tables.md | Documents calculated_category and updates FindingCategories description to match. |
|
I'm seeing |
Absolutely not intended |
b52c425 to
448e0ef
Compare
Fixed the missing files, and found another I hadn't noticed before |
Found something that needs a second look in the UI
thewatermethod
left a comment
There was a problem hiding this comment.
Dismissed my initial review but then realized I forgot to run the updateMonitoringTables script again post DB reset
📊 Review Metrics
Review Timeline
|
Description of change
We got updated guidance from IT-AMS on where to source the "Category" value for findings. The new guidance is to use
MonitoringFindings.sourceif available, andMonitoringStandards.guidanceif not. Instead of having each service figure out what to pull, we addcalculated_categoryto implement the guidance and then all services use that instead.How to test
The most obvious change is in "Finding category hot spots" where there will be few if any "No finding category assigned" entries. "Health" will go up ~100 or so in total.
Issue(s)
Checklists
Every PR
Before merge to main
Production Deploy
ready_for_reviewtransition triggers the Slack/Jira automation)elainaparrishis the authorized approver under normal circumstances)After merge/deploy