From a46aa157f3da4598c2a58613fd15c58eed59bd1c Mon Sep 17 00:00:00 2001 From: kitos Date: Fri, 10 Jul 2026 13:08:23 +0200 Subject: [PATCH] fix(assign): exclude admin from assignable operators; sync Jira on assign even for first-time users MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Admin can no longer be picked as a red_tech/blue_tech assignee — it administers the site, it doesn't operate tests. Applied to the eligible-assignee list on both the picker UI and the assign endpoint's validation, and dropped from the GET /users/operators pool. push_assignee_update() now falls back to a fresh Jira account-id lookup when the assignee hasn't logged in yet (so jira_account_id is still empty), instead of silently no-op'ing — assignment now syncs to Jira immediately regardless of whether the assignee has ever logged into Aegis before. --- backend/app/routers/tests.py | 4 ++-- backend/app/routers/users.py | 5 ++-- backend/app/services/jira_service.py | 10 +++++++- backend/tests/test_assign_operators.py | 13 ++++++++++ backend/tests/test_jira_service.py | 24 +++++++++++++++++++ backend/tests/test_list_operators.py | 8 +++++++ .../test-detail/AssigneeControl.tsx | 4 ++-- 7 files changed, 61 insertions(+), 7 deletions(-) diff --git a/backend/app/routers/tests.py b/backend/app/routers/tests.py index d25cacc..bada35b 100644 --- a/backend/app/routers/tests.py +++ b/backend/app/routers/tests.py @@ -1286,14 +1286,14 @@ def assign_test_operators( if payload.red_tech_assignee is not None: u = db.query(User).filter(User.id == payload.red_tech_assignee).first() - if not u or u.role not in ("red_tech", "red_lead", "admin"): + if not u or u.role not in ("red_tech", "red_lead"): raise HTTPException(status_code=400, detail="Invalid red tech assignee") test.red_tech_assignee = payload.red_tech_assignee newly_assigned = u if payload.blue_tech_assignee is not None: u = db.query(User).filter(User.id == payload.blue_tech_assignee).first() - if not u or u.role not in ("blue_tech", "blue_lead", "admin"): + if not u or u.role not in ("blue_tech", "blue_lead"): raise HTTPException(status_code=400, detail="Invalid blue tech assignee") test.blue_tech_assignee = payload.blue_tech_assignee newly_assigned = u diff --git a/backend/app/routers/users.py b/backend/app/routers/users.py index a4bd4f6..8cf08b0 100644 --- a/backend/app/routers/users.py +++ b/backend/app/routers/users.py @@ -161,13 +161,14 @@ def list_operators_route( """Return active red/blue operators and leads, for lead/manager assignment pickers. Not reachable by admin — admin administers the site, leads/managers - coordinate operators. Returns only id/username/role — no emails or + coordinate operators, and admin cannot itself be assigned as an + operator either. Returns only id/username/role — no emails or tokens — since this is reachable by non-admin leads. """ return ( db.query(User) .filter( - User.role.in_(["red_tech", "red_lead", "blue_tech", "blue_lead", "admin"]), + User.role.in_(["red_tech", "red_lead", "blue_tech", "blue_lead"]), User.is_active.is_(True), ) .order_by(User.username) diff --git a/backend/app/services/jira_service.py b/backend/app/services/jira_service.py index 2f5d371..44c28e5 100644 --- a/backend/app/services/jira_service.py +++ b/backend/app/services/jira_service.py @@ -893,7 +893,15 @@ def push_assignee_update(db: Session, test: Test, assignee: User) -> None: jira_account_id = getattr(assignee, "jira_account_id", None) if not jira_account_id: - return + # Normally populated on login (see lookup_user_jira_account_id) — + # but a lead may assign someone who has never logged in yet. Try + # a fresh lookup now instead of silently giving up, so assignment + # still syncs to Jira immediately. + lookup_user_jira_account_id(db, assignee) + db.flush() + jira_account_id = getattr(assignee, "jira_account_id", None) + if not jira_account_id: + return try: jira = get_admin_jira_client(db) diff --git a/backend/tests/test_assign_operators.py b/backend/tests/test_assign_operators.py index 6a50407..50c6f59 100644 --- a/backend/tests/test_assign_operators.py +++ b/backend/tests/test_assign_operators.py @@ -55,6 +55,19 @@ def test_assign_rejects_wrong_role_for_side(client, db, red_lead_headers, red_le assert resp.status_code == 400 +def test_admin_cannot_be_assigned_as_operator(client, db, red_lead_headers, red_lead_user, admin_user): + """Admin administers the site and can't be assigned as an operator either.""" + technique = _seed_technique(db) + test = _seed_test(db, technique, red_lead_user.id) + + resp = client.post( + f"/api/v1/tests/{test.id}/assign", + json={"red_tech_assignee": str(admin_user.id)}, + headers=red_lead_headers, + ) + assert resp.status_code == 400 + + def test_red_tech_cannot_assign(client, db, red_tech_headers, red_tech_user): technique = _seed_technique(db) test = _seed_test(db, technique, red_tech_user.id) diff --git a/backend/tests/test_jira_service.py b/backend/tests/test_jira_service.py index 2865c1d..c9626e9 100644 --- a/backend/tests/test_jira_service.py +++ b/backend/tests/test_jira_service.py @@ -488,6 +488,7 @@ def test_push_assignee_update_assigns_jira_issue(mock_get_client, mock_configure @patch("app.services.jira_service.get_admin_jira_client") def test_push_assignee_update_noop_without_jira_account_id(mock_get_client, mock_configured, db): mock_jira = MagicMock() + mock_jira.user_find_by_user_string.return_value = [] # fallback lookup finds nobody either mock_get_client.return_value = mock_jira test = _make_test() link = _make_link(test.id) @@ -500,6 +501,29 @@ def test_push_assignee_update_noop_without_jira_account_id(mock_get_client, mock mock_jira.assign_issue.assert_not_called() +@patch("app.services.jira_service.has_admin_jira_configured", return_value=True) +@patch("app.services.jira_service.get_admin_jira_client") +def test_push_assignee_update_looks_up_missing_account_id(mock_get_client, mock_configured, db): + """Regression: a lead can assign someone who has never logged in, so + jira_account_id isn't populated yet. Must try a fresh lookup instead of + silently giving up, so the assignment still syncs immediately.""" + mock_jira = MagicMock() + mock_jira.user_find_by_user_string.return_value = [ + {"emailAddress": "gerardo.ruiz@kaseya.com", "accountId": "freshly-found-id"} + ] + mock_get_client.return_value = mock_jira + test = _make_test() + link = _make_link(test.id) + db.add(link) + db.commit() + assignee = _make_user(jira_account_id=None) + + jira_service.push_assignee_update(db, test, assignee) + + mock_jira.assign_issue.assert_called_once_with(link.jira_issue_key, account_id="freshly-found-id") + assert assignee.jira_account_id == "freshly-found-id" + + @patch("app.services.jira_service.has_admin_jira_configured", return_value=True) @patch("app.services.jira_service.get_admin_jira_client") def test_push_assignee_update_noop_without_link(mock_get_client, mock_configured, db): diff --git a/backend/tests/test_list_operators.py b/backend/tests/test_list_operators.py index 7a8cbb9..2499ca9 100644 --- a/backend/tests/test_list_operators.py +++ b/backend/tests/test_list_operators.py @@ -44,3 +44,11 @@ def test_manager_can_list_operators(client, manager_headers, red_tech_user): assert resp.status_code == 200, resp.text usernames = {u["username"] for u in resp.json()} assert "redtech" in usernames + + +def test_operators_list_excludes_admin(client, manager_headers, admin_user): + """Admin can't be assigned as an operator, so it shouldn't appear in the picker.""" + resp = client.get("/api/v1/users/operators", headers=manager_headers) + assert resp.status_code == 200 + usernames = {u["username"] for u in resp.json()} + assert admin_user.username not in usernames diff --git a/frontend/src/components/test-detail/AssigneeControl.tsx b/frontend/src/components/test-detail/AssigneeControl.tsx index 7b18ee6..bd0ea5e 100644 --- a/frontend/src/components/test-detail/AssigneeControl.tsx +++ b/frontend/src/components/test-detail/AssigneeControl.tsx @@ -19,8 +19,8 @@ const SIDE_STYLE = { }; const SIDE_ROLES: Record<"red" | "blue", string[]> = { - red: ["red_tech", "red_lead", "admin"], - blue: ["blue_tech", "blue_lead", "admin"], + red: ["red_tech", "red_lead"], + blue: ["blue_tech", "blue_lead"], }; /** Lead/manager picker for the red_tech_assignee / blue_tech_assignee fields. */