Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions api/howler/actions/demote.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def execute(
*hit_helper.demote_hit(escalation=escalation),
odm_helper.update("howler.assessment", None),
odm_helper.update("howler.rationale", None),
odm_helper.update("howler.assignment", None),
odm_helper.update("howler.triaged", None),
],
)
Comment thread
cccs-mdr marked this conversation as resolved.
else:
Expand All @@ -94,7 +94,7 @@ def execute(
ds.hit.update_by_query(
query,
[
*hit_helper.assess_hit(assessment, rationale, user=(user if user else "automation")),
*hit_helper.assess_hit(assessment, rationale),
odm_helper.update(
"howler.assignment",
user.get("uname", "automation") if user else "automation",
Expand Down
8 changes: 2 additions & 6 deletions api/howler/actions/promote.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
AssessmentEscalationMap,
Escalation,
)
from howler.odm.models.user import User
from howler.utils.str_utils import sanitize_lucene_query

OPERATION_ID = "promote"
Expand All @@ -27,7 +26,6 @@ def execute(
escalation: Escalation = Escalation.ALERT,
assessment: Optional[str] = None,
rationale: Optional[str] = None,
user: Optional[User] = None,
**kwargs,
):
"""Promote a hit.
Expand Down Expand Up @@ -75,7 +73,7 @@ def execute(
*hit_helper.promote_hit(escalation=escalation),
odm_helper.update("howler.assessment", None),
odm_helper.update("howler.rationale", None),
odm_helper.update("howler.assignment", None),
odm_helper.update("howler.triaged", None),
],
)
Comment thread
cccs-mdr marked this conversation as resolved.
else:
Expand All @@ -90,9 +88,7 @@ def execute(
)
return report

ds.hit.update_by_query(
query, hit_helper.assess_hit(assessment, rationale, user=(user if user else "automation"))
)
ds.hit.update_by_query(query, hit_helper.assess_hit(assessment, rationale))

report.append(
{
Expand Down
15 changes: 5 additions & 10 deletions api/howler/helper/hit.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,13 @@ def assess_hit(
assessment: Optional[str] = None,
rationale: Optional[str] = None,
hit: Optional[Union[dict[str, Any], Hit]] = None,
*,
user: User | str,
**kwargs,
) -> list[OdmUpdateOperation]:
"""Update the assessment and esclation of a hit

Args:
user (User | str): The user making the assessment
assessment (Optional[str], optional): The assessment to set the hit to. Defaults to None.
hit (Optional[Union[dict[str, Any], Hit]], optional): The hit to update. Defaults to None.
hit (Optional[dict[str, Any]], optional): The hit to update. Defaults to None.
Comment thread
Copilot marked this conversation as resolved.
Outdated

Raises:
InvalidDataException: An invalid assessment was provided
Expand All @@ -55,6 +52,9 @@ def assess_hit(
if assessment is None and rationale:
rationale = None

# reset the timestamp to None if removing assessment (re-assessing)
triaged_timestamp = "NOW" if assessment is not None else None

Comment on lines +58 to +62
logger.debug(
"Updating assessment of %s to %s",
hit["howler"]["id"] if hit else "unknown",
Expand All @@ -66,16 +66,11 @@ def assess_hit(
escalation,
)

if assessment is None:
assessor_id = None
else:
assessor_id = user.get("uname", user.get("username", None)) if isinstance(user, User) else user

return [
odm_helper.update("howler.assessment", assessment),
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),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't directly related, but also set howler.status to resolved.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

]


Expand Down
2 changes: 1 addition & 1 deletion api/howler/odm/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,10 @@ def generate_useful_hit(lookups: dict[str, dict[str, Any]], users: list[str], pr

hit.howler.assessment = None
hit.howler.rationale = None
hit.howler.triaged = None
hit.howler.status = "open"
hit.howler.assignment = "unassigned"
hit.howler.escalation = choice([Escalation.HIT, Escalation.ALERT])
hit.howler.assessor = None

if randint(1, 10) > 9:
hit.howler.expiry = datetime.now() + timedelta(days=randint(1, 60))
Expand Down
8 changes: 2 additions & 6 deletions api/howler/odm/models/howler_data.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# mypy: ignore-errors
from datetime import datetime
from typing import Optional

from howler import odm
Expand Down Expand Up @@ -210,12 +211,6 @@ class HowlerData(odm.Model):
description="Unique identifier of the assigned user.",
default=DEFAULT_ASSIGNMENT,
)
assessor: Optional[str] = odm.Optional(
odm.Keyword(
description="The most recent person to assess a hit",
default=None,
)
)
bundles: list[str] = odm.List(
odm.Keyword(
description="A list of bundle IDs this hit is a part of. Corresponds to the howler.id of the bundle."
Expand Down Expand Up @@ -297,6 +292,7 @@ class HowlerData(odm.Model):
)
)
)
triaged: Optional[datetime] = odm.Optional(odm.Date(description="Timestamp at which the hit was triaged."))
comment: list[Comment] = odm.List(
odm.Compound(Comment),
default=[],
Expand Down
1 change: 0 additions & 1 deletion api/howler/odm/random_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,6 @@ def create_hits(ds: HowlerDatastore, hit_count: int = 200):
hit.howler.id,
[
*assess_hit(
user=user,
assessment=choice(Assessment.list()),
rationale=get_random_string(),
hit=hit,
Expand Down
2 changes: 1 addition & 1 deletion api/howler/services/hit_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ def get_hit_workflow() -> Workflow:
"source": [HitStatus.OPEN, HitStatus.IN_PROGRESS],
"transition": HitStatusTransition.ASSESS,
"dest": HitStatus.RESOLVED,
"actions": [assess_hit],
"actions": [assess_hit, assign_hit],
}
),
Transition(
Expand Down
42 changes: 29 additions & 13 deletions api/test/integration/api/test_hit_transition.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import datetime
import json
from typing import Any, Optional
from typing import Any, Literal, Optional

import pytest

Expand Down Expand Up @@ -42,11 +43,8 @@ def datastore(datastore_connection: HowlerDatastore):
wipe_hits(datastore_connection)


def test_full_transition_flow(datastore: HowlerDatastore, login_session):
"""Test that /api/v1/hit/<id>/transitions/start endpoint performs the correct transition"""
session, host = login_session

assert datastore.hit.get(HIT_ID).howler.status == HitStatus.OPEN
@pytest.fixture(scope="module")
def transition_data(datastore: HowlerDatastore) -> list[dict[str, Any]]:

def check_assignment(user: str):
def check():
Expand All @@ -66,23 +64,30 @@ def check():

return check

def check_assessor(user: str):
def check_triaged(assessment_time: Literal["NOW"] | None):
def check():
assert datastore.hit.get(HIT_ID).howler.assessor == user
tolerance = datetime.timedelta(seconds=1)
Comment thread
Copilot marked this conversation as resolved.
Outdated

triaged_timestamp = datastore.hit.get(HIT_ID).howler.triaged
if assessment_time is None:
assert triaged_timestamp is None
else:
assert triaged_timestamp is not None
assert abs(triaged_timestamp - datetime.datetime.now(datetime.timezone.utc)) < tolerance

return check

transition_data: list[dict[str, Any]] = [
return [
{
"transition": HitStatusTransition.ASSESS,
"data": {"assessment": Assessment.AMBIGUOUS},
"dest": HitStatus.RESOLVED,
"check": [check_assessor("admin")],
"check": [check_assignment("admin"), check_triaged("NOW")],
},
{
"transition": HitStatusTransition.RE_EVALUATE,
"dest": HitStatus.IN_PROGRESS,
"check": [check_assessment(None), check_assignment("admin"), check_assessor(None)],
"check": [check_assessment(None), check_assignment("admin"), check_triaged(None)],
},
{
"transition": HitStatusTransition.RELEASE,
Expand Down Expand Up @@ -148,12 +153,16 @@ def check():
"transition": HitStatusTransition.ASSESS,
"data": {"assessment": Assessment.AMBIGUOUS},
"dest": HitStatus.RESOLVED,
"check": [check_assessment(Assessment.AMBIGUOUS), check_assessor("admin")],
"check": [
check_assessment(Assessment.AMBIGUOUS),
check_assignment("admin"),
check_triaged("NOW"),
],
},
{
"transition": HitStatusTransition.RE_EVALUATE,
"dest": HitStatus.IN_PROGRESS,
"check": [check_assessment(None), check_assignment("admin"), check_assessor(None)],
"check": [check_assessment(None), check_assignment("admin"), check_triaged(None)],
},
{
"transition": HitStatusTransition.RELEASE,
Expand All @@ -168,6 +177,13 @@ def check():
},
]


def test_full_transition_flow(transition_data, datastore, login_session):
"""Test that /api/v1/hit/<id>/transitions/start endpoint performs the correct transition"""
session, host = login_session

assert datastore.hit.get(HIT_ID).howler.status == HitStatus.OPEN

for data in transition_data:
checks = data.pop("check", None)
_, version = datastore.hit.get(HIT_ID, as_obj=False, version=True)
Comment thread
cccs-hxp marked this conversation as resolved.
Expand Down
4 changes: 2 additions & 2 deletions ui/src/components/elements/hit/HitBanner.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ import { Link } from 'react-router-dom';
import { ESCALATION_COLORS, PROVIDER_COLORS } from 'utils/constants';
import { stringToColor } from 'utils/utils';
import PluginTypography from '../PluginTypography';
import Assigned from './elements/Assigned';
import EscalationChip from './elements/EscalationChip';
import HitTimestamp from './elements/HitTimestamp';
import HitUsers from './elements/HitUsers';
import HitBannerTooltip from './HitBannerTooltip';
import { HitLayout } from './HitLayout';

Expand Down Expand Up @@ -345,7 +345,7 @@ const HitBanner: FC<HitBannerProps> = ({ hit, layout = HitLayout.NORMAL, showAss
]}
>
<HitTimestamp hit={hit} layout={layout} />
<HitUsers hit={hit} layout={layout} showAssigned={showAssigned} />
{showAssigned && <Assigned hit={hit} layout={layout} />}
{hit.howler.links?.[0]?.href && (
<Chip
icon={<OpenInNew />}
Expand Down
71 changes: 71 additions & 0 deletions ui/src/components/elements/hit/elements/Assigned.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { avatarClasses, AvatarGroup, Chip, Stack } from '@mui/material';
import { useAppUser } from 'commons/components/app/hooks';
import HowlerAvatar from 'components/elements/display/HowlerAvatar';
import type { Hit } from 'models/entities/generated/Hit';
import type { HowlerUser } from 'models/entities/HowlerUser';
import type { FC } from 'react';
import { useTranslation } from 'react-i18next';
import { HitLayout } from '../HitLayout';

const Assigned: FC<{ hit: Hit; layout: HitLayout; hideLabel?: boolean }> = ({ hit, layout, hideLabel = false }) => {
const { t } = useTranslation();
const { user } = useAppUser<HowlerUser>();

const userAvatar = (
<HowlerAvatar
userId={hit.howler.assignment}
sx={{ height: layout !== HitLayout.COMFY ? 24 : 32, width: layout !== HitLayout.COMFY ? 24 : 32 }}
/>
);

return (
<Stack direction="row" spacing={0.5}>
{hideLabel ? (
userAvatar
) : (
<Chip
variant="outlined"
sx={{
width: 'fit-content',
'& .MuiChip-icon': {
marginLeft: 0
}
}}
icon={userAvatar}
label={
!hideLabel &&
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
(hit?.howler.assignment !== 'unassigned'
? hit?.howler.assignment
: t('app.drawer.hit.assignment.unassigned.name'))
}
size={layout !== HitLayout.COMFY ? 'small' : 'medium'}
/>
)}
<AvatarGroup
max={3}
sx={{ [`.${avatarClasses.root}`]: { border: 0, marginLeft: 0.5 } }}
componentsProps={{
additionalAvatar: {
sx: {
height: layout !== HitLayout.COMFY ? 24 : 32,
width: layout !== HitLayout.COMFY ? 24 : 32,
fontSize: '12px'
}
}
}}
>
{[...new Set(hit?.howler.viewers)]
.filter(viewer => viewer !== user.username)
.map(viewer => (
<HowlerAvatar
key={viewer}
userId={viewer}
sx={{ height: layout !== HitLayout.COMFY ? 24 : 32, width: layout !== HitLayout.COMFY ? 24 : 32 }}
/>
))}
</AvatarGroup>
</Stack>
);
};

export default Assigned;
Loading
Loading