fix(m7): stamping executed_at no longer requires a prior state transition
User reported `HTTP 400 — executed_at can only be set when state is
executed/reviewed_by_blue` when typing the timestamp inline in the new
scenario table. The state-gate predates the simplified UX — it made
sense back when the workflow was "Mark executed button + override
toggle", but the user has since asked for a single freely-typeable
datetime input.
- update_mission_test_fields drops the state check. Stamping a non-null
executed_at while state ∈ {pending, skipped, blocked} now auto-promotes
the state to `executed` in the same write. The promotion is gated by
the same mission.write_red_fields perm that executed_at already
required — no privilege escalation.
- MissionTestPage.tsx drops the state-based UI gate on canEditExecutedAt;
red perm alone now unlocks the input regardless of state.
- Replaced the old "rejection while pending" test with two new tests:
pending→executed via inline stamp + blue 403, and skipped→executed via
inline stamp.
- 139 pytest green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
24
CHANGELOG.md
24
CHANGELOG.md
@@ -4,6 +4,30 @@ All notable changes to this project will be documented here. Format: [Keep a Cha
|
|||||||
|
|
||||||
## [Unreleased]
|
## [Unreleased]
|
||||||
|
|
||||||
|
### Fixed (post-amendement 2026-05-15) — stamping executed_at no longer needs a prior state transition
|
||||||
|
|
||||||
|
User feedback: when a red user typed `executed_at` inline on a pending test
|
||||||
|
in the new scenario table, the backend rejected with `HTTP 400 — executed_at
|
||||||
|
can only be set when state is executed/reviewed_by_blue`. The state-gate
|
||||||
|
was a holdover from the original "Mark executed button + override toggle"
|
||||||
|
workflow; it made no sense once the UX let the operator type the time
|
||||||
|
directly.
|
||||||
|
|
||||||
|
- `update_mission_test_fields` (`backend/app/services/mission_tests.py`) no
|
||||||
|
longer rejects writes based on the source state. Stamping a non-null
|
||||||
|
`executed_at` while state ∈ {`pending`, `skipped`, `blocked`} now
|
||||||
|
auto-promotes the state to `executed` in the same write. The promotion
|
||||||
|
rides on the same `mission.write_red_fields` perm that the executed_at
|
||||||
|
field already required — no privilege escalation.
|
||||||
|
- `MissionTestPage.tsx` drops the state-based gate on `canEditExecutedAt`:
|
||||||
|
the field is editable any time the viewer holds `mission.write_red_fields`.
|
||||||
|
- Tests: `test_executed_at_override_requires_red_perm_and_state` was the
|
||||||
|
old guard; it's split into two new cases — `test_red_setting_executed_at_on_pending_auto_transitions_to_executed`
|
||||||
|
(pending → executed via inline stamp, blue still 403'd) and
|
||||||
|
`test_red_setting_executed_at_from_skipped_state_auto_transitions`
|
||||||
|
(skipped → executed via the same path).
|
||||||
|
- Total: **139 pytest** green.
|
||||||
|
|
||||||
### Added — M7 amendment (2026-05-15) — blue review fields + full-width scenario table
|
### Added — M7 amendment (2026-05-15) — blue review fields + full-width scenario table
|
||||||
|
|
||||||
User feedback after the M7 ship: the blue team used to maintain 5 extra
|
User feedback after the M7 ship: the blue team used to maintain 5 extra
|
||||||
|
|||||||
@@ -590,13 +590,16 @@ def update_mission_test_fields(
|
|||||||
)
|
)
|
||||||
|
|
||||||
if "executed_at_overridden" in touched or "executed_at" in touched:
|
if "executed_at_overridden" in touched or "executed_at" in touched:
|
||||||
# Editing executed_at is a red-only privilege and only valid when
|
# Editing executed_at is a red-only privilege (gated above via
|
||||||
# the test is past the `executed` milestone. Spec M7: override is
|
# _RED_FIELDS). We no longer reject the write based on the
|
||||||
# behind a deliberate toggle so the auto-stamp default is sticky.
|
# current state — the spec amendement 2026-05-15 lets the red
|
||||||
if test.state not in {"executed", "reviewed_by_blue"}:
|
# team record an execution timestamp inline, which would be
|
||||||
raise InvalidTestPayload(
|
# circular if they had to transition the state machine first.
|
||||||
"executed_at can only be set when state is executed/reviewed_by_blue"
|
# Instead, stamping a non-null timestamp implicitly bumps the
|
||||||
)
|
# state forward from any non-executed source so the persisted
|
||||||
|
# record stays internally consistent. The same `mission.
|
||||||
|
# write_red_fields` perm covers both moves, so this isn't a
|
||||||
|
# privilege escalation.
|
||||||
new_overridden = (
|
new_overridden = (
|
||||||
bool(executed_at_overridden)
|
bool(executed_at_overridden)
|
||||||
if "executed_at_overridden" in touched
|
if "executed_at_overridden" in touched
|
||||||
@@ -609,6 +612,8 @@ def update_mission_test_fields(
|
|||||||
)
|
)
|
||||||
if "executed_at" in touched and new_at is not None and not isinstance(new_at, datetime):
|
if "executed_at" in touched and new_at is not None and not isinstance(new_at, datetime):
|
||||||
raise InvalidTestPayload("executed_at must be an ISO datetime")
|
raise InvalidTestPayload("executed_at must be an ISO datetime")
|
||||||
|
if new_at is not None and test.state in {"pending", "skipped", "blocked"}:
|
||||||
|
test.state = "executed"
|
||||||
test.executed_at = new_at
|
test.executed_at = new_at
|
||||||
test.executed_at_overridden = new_overridden
|
test.executed_at_overridden = new_overridden
|
||||||
|
|
||||||
|
|||||||
@@ -569,18 +569,33 @@ def test_mark_executed_stamps_executed_at(
|
|||||||
assert body["executed_at_overridden"] is False
|
assert body["executed_at_overridden"] is False
|
||||||
|
|
||||||
|
|
||||||
def test_executed_at_override_requires_red_perm_and_state(
|
def test_red_setting_executed_at_on_pending_auto_transitions_to_executed(
|
||||||
client, admin_token, catalogue, red_user, blue_user
|
client, admin_token, catalogue, red_user, blue_user
|
||||||
):
|
):
|
||||||
|
"""Post-amendement (2026-05-15): the red team should be able to stamp
|
||||||
|
executed_at inline without first POST-ing /transition. The service
|
||||||
|
auto-flips state pending→executed in the same write so the persisted
|
||||||
|
row stays internally consistent.
|
||||||
|
|
||||||
|
Blue still cannot touch executed_at (red-side field) — perm gating is
|
||||||
|
unchanged.
|
||||||
|
"""
|
||||||
mission = _make_mission(
|
mission = _make_mission(
|
||||||
client, admin_token, name="m7-override",
|
client, admin_token, name="m7-exec-at-pending",
|
||||||
scenario_id=catalogue["scenario"]["id"],
|
scenario_id=catalogue["scenario"]["id"],
|
||||||
red_id=red_user["id"], blue_id=blue_user["id"],
|
red_id=red_user["id"], blue_id=blue_user["id"],
|
||||||
)
|
)
|
||||||
tid = _first_test_id(mission)
|
tid = _first_test_id(mission)
|
||||||
|
|
||||||
# Override while still pending → invalid_request (no executed milestone yet).
|
# Sanity: starts as pending.
|
||||||
bad = client.put(
|
detail = client.get(
|
||||||
|
f"/api/v1/missions/{mission['id']}/tests/{tid}",
|
||||||
|
headers=_bearer(red_user["token"]),
|
||||||
|
)
|
||||||
|
assert detail.get_json()["state"] == "pending"
|
||||||
|
|
||||||
|
# Red stamps executed_at directly — must succeed and bump the state.
|
||||||
|
stamped = client.put(
|
||||||
f"/api/v1/missions/{mission['id']}/tests/{tid}",
|
f"/api/v1/missions/{mission['id']}/tests/{tid}",
|
||||||
headers=_bearer(red_user["token"]),
|
headers=_bearer(red_user["token"]),
|
||||||
json={
|
json={
|
||||||
@@ -588,39 +603,49 @@ def test_executed_at_override_requires_red_perm_and_state(
|
|||||||
"executed_at_overridden": True,
|
"executed_at_overridden": True,
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
assert bad.status_code == 400, bad.get_data(as_text=True)
|
assert stamped.status_code == 200, stamped.get_data(as_text=True)
|
||||||
|
body = stamped.get_json()
|
||||||
|
assert body["state"] == "executed"
|
||||||
|
assert body["executed_at"].startswith("2026-05-14T10:00:00")
|
||||||
|
assert body["executed_at_overridden"] is True
|
||||||
|
|
||||||
# Mark executed first so we're allowed to override.
|
# Blue cannot touch executed_at — red-side field.
|
||||||
client.post(
|
|
||||||
f"/api/v1/missions/{mission['id']}/tests/{tid}/transition",
|
|
||||||
headers=_bearer(red_user["token"]),
|
|
||||||
json={"target_state": "executed"},
|
|
||||||
)
|
|
||||||
|
|
||||||
# Blue cannot override (executed_at is a red field).
|
|
||||||
forbidden = client.put(
|
forbidden = client.put(
|
||||||
f"/api/v1/missions/{mission['id']}/tests/{tid}",
|
f"/api/v1/missions/{mission['id']}/tests/{tid}",
|
||||||
headers=_bearer(blue_user["token"]),
|
headers=_bearer(blue_user["token"]),
|
||||||
json={
|
json={"executed_at": "2026-05-14T11:00:00+00:00"},
|
||||||
"executed_at": "2026-05-14T10:00:00+00:00",
|
|
||||||
"executed_at_overridden": True,
|
|
||||||
},
|
|
||||||
)
|
)
|
||||||
assert forbidden.status_code == 403
|
assert forbidden.status_code == 403
|
||||||
|
|
||||||
# Red successfully overrides.
|
|
||||||
ok = client.put(
|
def test_red_setting_executed_at_from_skipped_state_auto_transitions(
|
||||||
|
client, admin_token, catalogue, red_user
|
||||||
|
):
|
||||||
|
"""Skipped → executed by stamping a timestamp: same implicit transition
|
||||||
|
so the operator who marked the test skipped by mistake can simply type
|
||||||
|
the actual execution time."""
|
||||||
|
mission = _make_mission(
|
||||||
|
client, admin_token, name="m7-exec-at-skipped",
|
||||||
|
scenario_id=catalogue["scenario"]["id"], red_id=red_user["id"],
|
||||||
|
)
|
||||||
|
tid = _first_test_id(mission)
|
||||||
|
# Move to skipped first.
|
||||||
|
skip = client.post(
|
||||||
|
f"/api/v1/missions/{mission['id']}/tests/{tid}/transition",
|
||||||
|
headers=_bearer(red_user["token"]),
|
||||||
|
json={"target_state": "skipped"},
|
||||||
|
)
|
||||||
|
assert skip.status_code == 200
|
||||||
|
assert skip.get_json()["state"] == "skipped"
|
||||||
|
|
||||||
|
# Stamp executed_at inline → should land in executed.
|
||||||
|
stamped = client.put(
|
||||||
f"/api/v1/missions/{mission['id']}/tests/{tid}",
|
f"/api/v1/missions/{mission['id']}/tests/{tid}",
|
||||||
headers=_bearer(red_user["token"]),
|
headers=_bearer(red_user["token"]),
|
||||||
json={
|
json={"executed_at": "2026-05-14T12:00:00+00:00"},
|
||||||
"executed_at": "2026-05-14T10:00:00+00:00",
|
|
||||||
"executed_at_overridden": True,
|
|
||||||
},
|
|
||||||
)
|
)
|
||||||
assert ok.status_code == 200, ok.get_data(as_text=True)
|
assert stamped.status_code == 200, stamped.get_data(as_text=True)
|
||||||
body = ok.get_json()
|
assert stamped.get_json()["state"] == "executed"
|
||||||
assert body["executed_at_overridden"] is True
|
|
||||||
assert body["executed_at"].startswith("2026-05-14T10:00:00")
|
|
||||||
|
|
||||||
|
|
||||||
def test_state_machine_rejects_invalid_transitions(
|
def test_state_machine_rejects_invalid_transitions(
|
||||||
|
|||||||
@@ -228,8 +228,11 @@ function RedZone({ test, missionId, canWriteRed }: RedZoneProps) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const apiErr = save.error instanceof ApiError ? save.error : null;
|
const apiErr = save.error instanceof ApiError ? save.error : null;
|
||||||
const canEditExecutedAt =
|
// Post-amendement 2026-05-15: the backend auto-promotes the state from
|
||||||
canWriteRed && (test.state === 'executed' || test.state === 'reviewed_by_blue');
|
// pending/skipped/blocked when a non-null executed_at is stamped, so the
|
||||||
|
// UI no longer needs to disable the input on those states. Red perm is
|
||||||
|
// still required.
|
||||||
|
const canEditExecutedAt = canWriteRed;
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<Card accent="red" className="flex flex-col gap-3" data-testid="red-zone">
|
<Card accent="red" className="flex flex-col gap-3" data-testid="red-zone">
|
||||||
|
|||||||
Reference in New Issue
Block a user