feat(api,ui): add howler.triaged to track time assessment is made#490
Conversation
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
Adds a new howler.triaged timestamp to record when a hit is assessed (enabling sort/filter by triage time), while reverting the previously introduced howler.assessor field and updating UI/API workflows accordingly.
Changes:
- API: Introduce
howler.triagedonHowlerDataand set/reset it as assessment is applied/removed; update transitions/workflow and related automations. - UI: Add
howler.triagedto the built-in hit sort fields; refactor hit “user chips” UI to remove assessor usage. - Tests: Update transition integration test expectations to validate
triagedbehavior.
Reviewed changes
Copilot reviewed 13 out of 16 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/models/entities/generated/Howler.d.ts | Removes assessor and adds triaged to the generated Howler type. |
| ui/src/components/routes/hits/search/shared/HitSort.tsx | Allows sorting by howler.triaged. |
| ui/src/components/elements/hit/HitBanner.tsx | Replaces HitUsers with new Assigned component usage. |
| ui/src/components/elements/hit/grid/HitRow.tsx | Replaces HitUsers with Assigned in dense/grid rows. |
| ui/src/components/elements/hit/elements/HitUsers.tsx | Removes combined assignee/assessor/viewers UI (deleted). |
| ui/src/components/elements/hit/elements/Assigned.tsx | New UI component for assignment + viewers rendering. |
| api/test/integration/api/test_hit_transition.py | Updates transition flow checks to validate triaged timestamps. |
| api/howler/services/hit_service.py | Ensures assess transitions also apply assignment updates. |
| api/howler/odm/random_data.py | Updates random hit generation to align with updated assessment helper usage. |
| api/howler/odm/models/howler_data.py | Removes assessor, adds triaged: Optional[datetime]. |
| api/howler/odm/helper.py | Initializes triaged to None in helper-generated hits. |
| api/howler/helper/hit.py | Updates assess_hit to write howler.triaged and removes assessor logic. |
| api/howler/actions/promote.py | Clears triaged when clearing assessment/rationale during promote. |
| api/howler/actions/demote.py | Clears triaged when clearing assessment/rationale during demote. |
| ui/src/locales/en/translation.json | Updated (content excluded from review). |
| ui/src/locales/fr/translation.json | Updated (content excluded from review). |
Files excluded by content exclusion policy (2)
- ui/src/locales/en/translation.json
- ui/src/locales/fr/translation.json
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
Error Output |
cccs-mdr
left a comment
There was a problem hiding this comment.
One other minor change to resolve a related issue
| odm_helper.update("howler.escalation", escalation), | ||
| odm_helper.update("howler.rationale", rationale, silent=True), | ||
| odm_helper.update("howler.assessor", assessor_id, silent=True), | ||
| odm_helper.update("howler.triaged", triaged_timestamp), |
There was a problem hiding this comment.
This isn't directly related, but also set howler.status to resolved.
There was a problem hiding this comment.
Context: howler.status is set properly when hit is assessed through a transition, but it isn't set when calling assess_hit directly (from promote / demote actions).
To match the the only transition out of resolved, if an assessment is removed by assess_hit we can set howler.status to in progress.
With the current workflow implementation, if an action sets the status, it takes priority over the transition defined destination status. I think it should be the other way around or even fail if values don't match. Personally, if there's a destination defined in the transition states, I expect this to be the behaviour, with any status changes in actions only being a fallback.
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
Copilot reviewed 12 out of 15 changed files in this pull request and generated 1 comment.
Files excluded by content exclusion policy (2)
- ui/src/locales/en/translation.json
- ui/src/locales/fr/translation.json
Comments suppressed due to low confidence (1)
api/test/integration/api/test_hit_transition.py:191
transition_datais a module-scoped fixture, but the test loop mutates each dict viapop("check", ...). This makes the fixture stateful and can create order-dependent failures if the fixture is reused (or the test is re-run within the same process). Also,destis only used for assertions and doesn't need to be sent in the API payload. Prefer building a per-iterationpayloadwithout mutating the fixture.
for data in transition_data:
checks = data.pop("check", None)
_, version = datastore.hit.get(HIT_ID, as_obj=False, version=True)
get_api_data(
session=session,
| status = Status.IN_PROGRESS | ||
| else: | ||
| triaged_timestamp = "NOW" | ||
| status = Status.RESOLVED | ||
|
|
The assessor field was proposed to help with finding all hits assessed by a specific user, but it overlaps with howler.assignment. It is possible to get all hits assessed by a user using the assignment field which is auto-set on assessment, but it is not possible to filter or sort these hits based on time of assessment. These changes add a howler.triaged field which tracks the time that a hit is assessed and adds this field to the available builtin sort fields in the UI.
Changes: