refactor(evidence): extract permission validation and queries to evidence_service, use domain exceptions
This commit is contained in:
@@ -89,12 +89,12 @@ for mod_name in [
|
||||
# Imports
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
from fastapi import HTTPException
|
||||
from app.domain.errors import PermissionViolation
|
||||
from app.models.enums import TeamSide, TestState
|
||||
from app.routers.evidence import (
|
||||
router,
|
||||
_validate_upload_permission,
|
||||
_validate_delete_permission,
|
||||
from app.routers.evidence import router
|
||||
from app.services.evidence_service import (
|
||||
validate_delete_permission,
|
||||
validate_upload_permission,
|
||||
)
|
||||
|
||||
|
||||
@@ -132,7 +132,7 @@ def test_red_tech_upload_red_in_red_executing():
|
||||
test = _make_test(TestState.red_executing)
|
||||
user = _make_user("red_tech")
|
||||
# Should not raise
|
||||
_validate_upload_permission(test, TeamSide.red, user)
|
||||
validate_upload_permission(test, TeamSide.red, user.role)
|
||||
print(" [PASS] red_tech can upload team=red in red_executing")
|
||||
|
||||
|
||||
@@ -144,7 +144,7 @@ def test_red_tech_upload_red_in_red_executing():
|
||||
def test_red_tech_upload_red_in_draft():
|
||||
test = _make_test(TestState.draft)
|
||||
user = _make_user("red_tech")
|
||||
_validate_upload_permission(test, TeamSide.red, user)
|
||||
validate_upload_permission(test, TeamSide.red, user.role)
|
||||
print(" [PASS] red_tech can upload team=red in draft")
|
||||
|
||||
|
||||
@@ -157,10 +157,10 @@ def test_red_tech_cannot_upload_blue():
|
||||
test = _make_test(TestState.red_executing)
|
||||
user = _make_user("red_tech")
|
||||
try:
|
||||
_validate_upload_permission(test, TeamSide.blue, user)
|
||||
assert False, "Should have raised HTTPException"
|
||||
except HTTPException as exc:
|
||||
assert exc.status_code == 403
|
||||
validate_upload_permission(test, TeamSide.blue, user.role)
|
||||
assert False, "Should have raised PermissionViolation"
|
||||
except PermissionViolation:
|
||||
pass
|
||||
print(" [PASS] red_tech CANNOT upload team=blue (403)")
|
||||
|
||||
|
||||
@@ -172,7 +172,7 @@ def test_red_tech_cannot_upload_blue():
|
||||
def test_blue_tech_upload_blue_in_blue_evaluating():
|
||||
test = _make_test(TestState.blue_evaluating)
|
||||
user = _make_user("blue_tech")
|
||||
_validate_upload_permission(test, TeamSide.blue, user)
|
||||
validate_upload_permission(test, TeamSide.blue, user.role)
|
||||
print(" [PASS] blue_tech can upload team=blue in blue_evaluating")
|
||||
|
||||
|
||||
@@ -185,10 +185,10 @@ def test_blue_tech_cannot_upload_red():
|
||||
test = _make_test(TestState.blue_evaluating)
|
||||
user = _make_user("blue_tech")
|
||||
try:
|
||||
_validate_upload_permission(test, TeamSide.red, user)
|
||||
assert False, "Should have raised HTTPException"
|
||||
except HTTPException as exc:
|
||||
assert exc.status_code == 403
|
||||
validate_upload_permission(test, TeamSide.red, user.role)
|
||||
assert False, "Should have raised PermissionViolation"
|
||||
except PermissionViolation:
|
||||
pass
|
||||
print(" [PASS] blue_tech CANNOT upload team=red (403)")
|
||||
|
||||
|
||||
@@ -223,10 +223,10 @@ def test_delete_in_review_fails():
|
||||
user = _make_user("red_tech")
|
||||
evidence = _make_evidence(TeamSide.red, uploaded_by=user.id)
|
||||
try:
|
||||
_validate_delete_permission(test, evidence, user)
|
||||
assert False, "Should have raised HTTPException"
|
||||
except HTTPException as exc:
|
||||
assert exc.status_code == 403
|
||||
validate_delete_permission(test, evidence, user.role, user.id)
|
||||
assert False, "Should have raised PermissionViolation"
|
||||
except PermissionViolation:
|
||||
pass
|
||||
print(" [PASS] DELETE in in_review -> 403")
|
||||
|
||||
|
||||
@@ -240,7 +240,7 @@ def test_delete_red_evidence_in_red_executing():
|
||||
user = _make_user("red_tech")
|
||||
evidence = _make_evidence(TeamSide.red, uploaded_by=user.id)
|
||||
# Should not raise
|
||||
_validate_delete_permission(test, evidence, user)
|
||||
validate_delete_permission(test, evidence, user.role, user.id)
|
||||
print(" [PASS] DELETE red evidence in red_executing -> allowed")
|
||||
|
||||
|
||||
@@ -254,11 +254,11 @@ def test_admin_bypass():
|
||||
|
||||
# Red in blue_evaluating (normally blocked)
|
||||
test1 = _make_test(TestState.blue_evaluating)
|
||||
_validate_upload_permission(test1, TeamSide.red, admin)
|
||||
validate_upload_permission(test1, TeamSide.red, admin.role)
|
||||
|
||||
# Blue in draft (normally blocked)
|
||||
test2 = _make_test(TestState.draft)
|
||||
_validate_upload_permission(test2, TeamSide.blue, admin)
|
||||
validate_upload_permission(test2, TeamSide.blue, admin.role)
|
||||
|
||||
print(" [PASS] Admin can upload any team in any state")
|
||||
|
||||
|
||||
@@ -419,56 +419,57 @@ def test_dual_validation_red_approves_blue_rejects(mock_log):
|
||||
|
||||
def test_evidence_team_separation():
|
||||
"""Verify evidence router logic separates red and blue evidence correctly."""
|
||||
from app.routers.evidence import _validate_upload_permission, _RED_EDITABLE_STATES, _BLUE_EDITABLE_STATES
|
||||
from app.domain.errors import BusinessRuleViolation, PermissionViolation
|
||||
from app.models.enums import TeamSide
|
||||
from app.services.evidence_service import validate_upload_permission
|
||||
|
||||
# Red tech can upload red evidence in draft
|
||||
test = _make_test(TestState.draft)
|
||||
red_user = _make_user("red_tech")
|
||||
red_user.role = "red_tech"
|
||||
from app.models.enums import TeamSide
|
||||
_validate_upload_permission(test, TeamSide.red, red_user) # should not raise
|
||||
validate_upload_permission(test, TeamSide.red, red_user.role) # should not raise
|
||||
|
||||
# Red tech can upload red evidence in red_executing
|
||||
test.state = TestState.red_executing
|
||||
_validate_upload_permission(test, TeamSide.red, red_user) # should not raise
|
||||
validate_upload_permission(test, TeamSide.red, red_user.role) # should not raise
|
||||
|
||||
# Red tech CANNOT upload red evidence in blue_evaluating
|
||||
# Red tech CANNOT upload red evidence in blue_evaluating (state violation -> 400)
|
||||
test.state = TestState.blue_evaluating
|
||||
try:
|
||||
_validate_upload_permission(test, TeamSide.red, red_user)
|
||||
assert False, "Should have raised HTTPException"
|
||||
except HTTPException as exc:
|
||||
assert exc.status_code == 400
|
||||
validate_upload_permission(test, TeamSide.red, red_user.role)
|
||||
assert False, "Should have raised BusinessRuleViolation"
|
||||
except BusinessRuleViolation:
|
||||
pass
|
||||
|
||||
# Red tech CANNOT upload blue evidence
|
||||
# Red tech CANNOT upload blue evidence (role violation -> 403)
|
||||
test.state = TestState.blue_evaluating
|
||||
try:
|
||||
_validate_upload_permission(test, TeamSide.blue, red_user)
|
||||
assert False, "Should have raised HTTPException"
|
||||
except HTTPException as exc:
|
||||
assert exc.status_code == 403
|
||||
validate_upload_permission(test, TeamSide.blue, red_user.role)
|
||||
assert False, "Should have raised PermissionViolation"
|
||||
except PermissionViolation:
|
||||
pass
|
||||
|
||||
# Blue tech can upload blue evidence in blue_evaluating
|
||||
test.state = TestState.blue_evaluating
|
||||
blue_user = _make_user("blue_tech")
|
||||
blue_user.role = "blue_tech"
|
||||
_validate_upload_permission(test, TeamSide.blue, blue_user) # should not raise
|
||||
validate_upload_permission(test, TeamSide.blue, blue_user.role) # should not raise
|
||||
|
||||
# Blue tech CANNOT upload blue evidence in draft
|
||||
# Blue tech CANNOT upload blue evidence in draft (state violation -> 400)
|
||||
test.state = TestState.draft
|
||||
try:
|
||||
_validate_upload_permission(test, TeamSide.blue, blue_user)
|
||||
assert False, "Should have raised HTTPException"
|
||||
except HTTPException as exc:
|
||||
assert exc.status_code == 400
|
||||
validate_upload_permission(test, TeamSide.blue, blue_user.role)
|
||||
assert False, "Should have raised BusinessRuleViolation"
|
||||
except BusinessRuleViolation:
|
||||
pass
|
||||
|
||||
# Blue tech CANNOT upload red evidence
|
||||
# Blue tech CANNOT upload red evidence (role violation -> 403)
|
||||
test.state = TestState.draft
|
||||
try:
|
||||
_validate_upload_permission(test, TeamSide.red, blue_user)
|
||||
assert False, "Should have raised HTTPException"
|
||||
except HTTPException as exc:
|
||||
assert exc.status_code == 403
|
||||
validate_upload_permission(test, TeamSide.red, blue_user.role)
|
||||
assert False, "Should have raised PermissionViolation"
|
||||
except PermissionViolation:
|
||||
pass
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
|
||||
Reference in New Issue
Block a user