Skip to content

Commit 7167b7a

Browse files
Clarify invoice privileged role messaging (#300)
* Add Workflows Engineer to invoice privileged roles * Fix denial message and test assertions to derive from _PRIVILEGED_ROLES - Build the access-denied message dynamically from _PRIVILEGED_ROLES so role names are always in sync without manual updates - Update the header comment to reference _PRIVILEGED_ROLES instead of hard-coding "Steering Committee" - Import _PRIVILEGED_ROLES in tests and assert against it so future role additions are covered automatically - Add Workflows Engineer fixture and access test * Clarify invoice privileged role messaging --------- Co-authored-by: Michael Wu <michaelmwu@gmail.com>
1 parent 60b08c3 commit 7167b7a

2 files changed

Lines changed: 33 additions & 6 deletions

File tree

apps/discord_bot/src/five08/discord_bot/cogs/invoices.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,13 @@
2727
STATUS_LABEL = {0: "Draft", 1: "Submitted", 2: "Cancelled"}
2828

2929
# Invoice access rules — a caller may validate an invoice if any of these hold:
30-
# 1. They have Steering Committee role or above (full access).
30+
# 1. They have Steering Committee-level access (full access).
3131
# 2. They created the invoice (invoice owner matches one of their ERP emails).
3232
# 3. They are on the invoice's ERP project roster.
33-
_PRIVILEGED_ROLES = ["Steering Committee"]
33+
#
34+
# Workflows Engineer is a Steering Committee-level role in role_decorators.
35+
_PRIVILEGED_ROLE_REQUIREMENTS = ["Steering Committee"]
36+
_PRIVILEGED_ROLE_LABELS = ["Steering Committee", "Workflows Engineer"]
3437

3538

3639
def _can_view_invoice(
@@ -72,7 +75,7 @@ def _resolve_access(
7275
with no ERP identity returns (False, [], []).
7376
"""
7477
roles = getattr(interaction.user, "roles", [])
75-
if check_user_roles_with_hierarchy(roles, _PRIVILEGED_ROLES):
78+
if check_user_roles_with_hierarchy(roles, _PRIVILEGED_ROLE_REQUIREMENTS):
7679
return True, [], []
7780

7881
emails = project_viewer_emails_for_discord(settings, str(interaction.user.id))
@@ -115,8 +118,9 @@ async def validate_invoice_command(
115118
self._resolve_access, interaction
116119
)
117120
if not include_all and not emails:
121+
roles_str = " or ".join(_PRIVILEGED_ROLE_LABELS)
118122
await interaction.followup.send(
119-
"Invoice validation is available to Steering Committee members "
123+
f"Invoice validation is restricted to {roles_str} "
120124
"or confirmed ERP project members.",
121125
ephemeral=True,
122126
)

tests/unit/test_invoices_cog.py

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
import pytest
66

7-
from five08.discord_bot.cogs.invoices import InvoicesCog
7+
from five08.discord_bot.cogs.invoices import InvoicesCog, _PRIVILEGED_ROLE_LABELS
88
from five08.clients.erpnext import ERPNextAPIError
99

1010

@@ -45,6 +45,12 @@ def mock_interaction() -> AsyncMock:
4545
return _make_interaction(role_names=["Steering Committee"], user_id=1001)
4646

4747

48+
@pytest.fixture
49+
def mock_workflows_engineer_interaction() -> AsyncMock:
50+
"""A privileged (Workflows Engineer) caller with full invoice access."""
51+
return _make_interaction(role_names=["Workflows Engineer"], user_id=1002)
52+
53+
4854
@pytest.fixture
4955
def mock_member_interaction() -> AsyncMock:
5056
"""A non-privileged caller subject to owner/project access rules."""
@@ -148,10 +154,27 @@ async def test_validate_invoice_denied_without_erp_identity(
148154
cog, mock_member_interaction, mock_doctype, "TEST-SINV-0001"
149155
)
150156
sent = mock_member_interaction.followup.send.call_args.args[0]
151-
assert "Steering Committee" in sent
157+
assert all(role in sent for role in _PRIVILEGED_ROLE_LABELS)
152158
cog.client.get_invoice.assert_not_called()
153159

154160

161+
@pytest.mark.asyncio
162+
async def test_validate_invoice_allowed_for_workflows_engineer(
163+
cog, mock_workflows_engineer_interaction, mock_doctype
164+
):
165+
"""Workflows Engineer gets include_all access without needing an ERP identity."""
166+
cog.client.get_invoice = Mock(return_value=VALID_INVOICE)
167+
with patch(
168+
"five08.discord_bot.cogs.invoices.project_viewer_emails_for_discord"
169+
) as mock_project_viewer_emails:
170+
await cog.validate_invoice_command.callback(
171+
cog, mock_workflows_engineer_interaction, mock_doctype, "TEST-SINV-0001"
172+
)
173+
embed = mock_workflows_engineer_interaction.followup.send.call_args.kwargs["embed"]
174+
assert "No issues found" in embed.title
175+
mock_project_viewer_emails.assert_not_called()
176+
177+
155178
@pytest.mark.asyncio
156179
async def test_validate_invoice_allowed_for_invoice_owner(
157180
cog, mock_member_interaction, mock_doctype

0 commit comments

Comments
 (0)