diff --git a/.github/workflows/pr-build-merge.yaml b/.github/workflows/pr-build-merge.yaml index a04186a..bb0bfbb 100644 --- a/.github/workflows/pr-build-merge.yaml +++ b/.github/workflows/pr-build-merge.yaml @@ -52,10 +52,10 @@ jobs: run: cp env.example .env - name: Run formatting checks - run: uv --project backend run ruff --config backend/pyproject.toml format --check --diff backend + run: uv --project backend run ruff format --check --diff backend - name: Run linting - run: uv --project backend run ruff --config backend/pyproject.toml check backend + run: uv --project backend run ruff check backend - name: Run security checks run: uv --project backend run bandit -c backend/pyproject.toml -r backend -f json -o bandit.json diff --git a/backend/app/billing/notification_helper.py b/backend/app/billing/notification_helper.py index a7c6136..24cdbad 100644 --- a/backend/app/billing/notification_helper.py +++ b/backend/app/billing/notification_helper.py @@ -1,6 +1,8 @@ import calendar +import json import logging import textwrap +from collections.abc import Awaitable, Callable from datetime import date from adaptive_cards import card_types as ct # type: ignore[import-untyped] @@ -12,20 +14,32 @@ from app.notifications import ( ColumnHeader, NotificationDetails, + build_card_payload, send_exception, send_info, send_warning, ) logger = logging.getLogger(__name__) +# MS Teams Incoming Webhook limit is 28 KB; we leave headroom for +# title/emoji/(Part i/N) prefixes and JSON whitespace not counted by our +# size estimator. See: +# https://learn.microsoft.com/microsoftteams/platform/webhooks-and-connectors/how-to/add-incoming-webhook +MSTEAMS_PAYLOAD_BUDGET = 22 * 1024 +MAX_MESSAGE_CHARS = 2000 -NOTIFICATION_FUNCTIONS = { +NOTIFICATION_TITLE_COLORS: dict[NotificationLevel, ct.Colors] = { + NotificationLevel.SUCCESS: ct.Colors.ACCENT, + NotificationLevel.IN_PROGRESS: ct.Colors.WARNING, + NotificationLevel.ERROR: ct.Colors.ATTENTION, +} + +NOTIFICATION_FUNCTIONS: dict[NotificationLevel, Callable[..., Awaitable[None]]] = { NotificationLevel.SUCCESS: send_info, NotificationLevel.IN_PROGRESS: send_warning, NotificationLevel.ERROR: send_exception, } - NOTIFICATION_TEXTS: dict[NotificationLevel, tuple[str, str]] = { NotificationLevel.SUCCESS: ( "{month_name} {year} Billing Finalized.", @@ -42,6 +56,12 @@ ), } +RESULT_ICON: dict[str, str] = { + "JOURNAL_GENERATED": "✅", + "JOURNAL_SKIPPED": "⏭️", + "ERROR": "❌", +} + def _build_notification_title_text( level: NotificationLevel, month_name: str, year: int @@ -53,55 +73,139 @@ def _build_notification_title_text( ) -def _build_notification_details(details: list) -> NotificationDetails: +def _build_header() -> tuple[ColumnHeader, ...]: + return ( + ColumnHeader( + "Authorization", width="120px", horizontal_alignment=ct.HorizontalAlignment.CENTER + ), + ColumnHeader("Journal", width="120px", horizontal_alignment=ct.HorizontalAlignment.CENTER), + ColumnHeader("Status", width="50px", horizontal_alignment=ct.HorizontalAlignment.CENTER), + ColumnHeader("Message", width="stretch"), + ) + + +def _build_rows(details: list[ProcessResultInfo]) -> list[tuple[str, ...]]: + return [ + ( + f"{item.authorization_id}", + f"{item.journal_id or ''}", + f"{RESULT_ICON.get(item.result.value.upper(), '')}", + "\n\n".join(textwrap.wrap((item.message or "")[:MAX_MESSAGE_CHARS], width=80)), + ) + for item in details + ] + + +def _chunk_rows_by_size( + rows: list[tuple[str, ...]], + measure: Callable[[list[tuple[str, ...]]], int], + budget: int = MSTEAMS_PAYLOAD_BUDGET, +) -> list[list[tuple[str, ...]]]: """ - This function builds a NotificationDetails object depending on the - given notification level. + Partition `rows` so each chunk's measured size stays within `budget`. + `measure` returns the serialized size (bytes) of a card containing the + given subset of rows, including all per-card overhead. Raises + ``ValueError`` if a single row alone exceeds the budget — callers must + cap the row's variable-size fields (the message column) at + ``MAX_MESSAGE_CHARS`` in `_build_rows` so this invariant holds. """ - prefix_icon = { - "JOURNAL_GENERATED": "✅", - "JOURNAL_SKIPPED": "⏭️", - "ERROR": "❌", - } - - return NotificationDetails( - header=( - ColumnHeader( - "Authorization", width="120px", horizontal_alignment=ct.HorizontalAlignment.CENTER - ), - ColumnHeader( - "Journal", width="120px", horizontal_alignment=ct.HorizontalAlignment.CENTER - ), - ColumnHeader( - "Status", width="50px", horizontal_alignment=ct.HorizontalAlignment.CENTER - ), - ColumnHeader("Message", width="stretch"), - ), - rows=[ - ( - f"{item.authorization_id}", - f"{item.journal_id or ''}", - f"{prefix_icon.get(item.result.value.upper(), '')}", - "\n\n".join(textwrap.wrap(item.message or "", width=80)), + + # json.dumps default separator between list items is ", " (2 bytes). + separator_bytes = 2 + + # Measure once: empty-card overhead. + empty_size = measure([]) + # N measurements for marginal row sizes (each row in isolation). + row_sizes = [measure([row]) - empty_size for row in rows] + + chunks: list[list[tuple[str, ...]]] = [] + buffer: list[tuple[str, ...]] = [] + buffer_size = empty_size + + for row, row_size in zip(rows, row_sizes, strict=True): + added = row_size + (separator_bytes if buffer else 0) + if buffer_size + added <= budget: + buffer.append(row) + buffer_size += added + continue + if buffer: + chunks.append(buffer) + if empty_size + row_size > budget: + raise ValueError( + f"Row for authorization={row[0]} contributes {row_size} bytes; " + f"exceeds per-chunk budget {budget - empty_size}." ) - for item in details - ], + buffer = [row] + buffer_size = empty_size + row_size + if buffer: + chunks.append(buffer) + return chunks + + +def _measure_card_payload_size( + title: str, + text: str, + header: tuple[ColumnHeader, ...], + rows: list[tuple[str, ...]], + title_color: ct.Colors = ct.Colors.DEFAULT, + open_url: str | None = None, +) -> int: + details = NotificationDetails(header=header, rows=rows) + card_payload = build_card_payload( + title=title, + text=text, + title_color=title_color, + details=details, + open_url=open_url, ) + return len(json.dumps(card_payload).encode("utf-8")) async def _send_notification( - level: NotificationLevel, month_name: str, year: int, results_counter_details: list + level: NotificationLevel, + month_name: str, + year: int, + results_counter_details: list[ProcessResultInfo], ) -> None: """ - This function sends a notification at the given level. + Sends one or more notifications at the given level. Rows are split + into multiple messages whenever the serialized payload would exceed + `MSTEAMS_PAYLOAD_BUDGET`. Each part carries a "(Part i/N)" suffix + in the title when N > 1. """ + func = NOTIFICATION_FUNCTIONS[level] - title, text = _build_notification_title_text(level, month_name, year) - await func( - title=title, - text=text, - details=_build_notification_details(details=results_counter_details), - ) + title, text = _build_notification_title_text(level=level, month_name=month_name, year=year) + header = _build_header() + rows = _build_rows(details=results_counter_details) + if not rows: + await func( + title=title, + text=text, + details=NotificationDetails(header=header, rows=[]), + ) + return + + title_color = NOTIFICATION_TITLE_COLORS[level] + + def measure(chunk_rows: list[tuple[str, ...]]) -> int: + return _measure_card_payload_size( + title=title, + text=text, + header=header, + rows=chunk_rows, + title_color=title_color, + ) + + chunks = _chunk_rows_by_size(rows=rows, measure=measure) + total = len(chunks) + for idx, chunk_rows in enumerate(chunks, start=1): + chunk_title = f"{title} (Part {idx}/{total})" if total > 1 else title + await func( + title=chunk_title, + text=text, + details=NotificationDetails(header=header, rows=chunk_rows), + ) def check_results( @@ -116,7 +220,9 @@ def check_results( return ProcessResult.JOURNAL_GENERATED in results_type, ProcessResult.ERROR in results_type -async def send_notifications(results: list, year: int, month: int, cutoff_day: int = 5): +async def send_notifications( + results: list[ProcessResultInfo], year: int, month: int, cutoff_day: int = 5 +) -> None: """ This function process the given results list and sends notifications according to the number of journals successfully generated diff --git a/backend/app/billing/process_billing.py b/backend/app/billing/process_billing.py index 74fcb23..a8e4f1b 100644 --- a/backend/app/billing/process_billing.py +++ b/backend/app/billing/process_billing.py @@ -78,8 +78,8 @@ async def process_billing( if authorization_id: authorization = await mpt_client.get_authorization(authorization_id) processor = AuthorizationProcessor(year, month, authorization, dry_run) - await processor.process() - return + response = [await processor.process()] + else: tasks = [] semaphore = asyncio.Semaphore(int(settings.ffc_billing_process_max_concurrency)) @@ -91,9 +91,7 @@ async def process_billing( logger.info(f"Processing {len(tasks)} authorizations for {product_id}") response = list(await asyncio.gather(*tasks)) - await send_notifications( - results=response, year=year, month=month, cutoff_day=cutoff_day - ) + await send_notifications(results=response, year=year, month=month, cutoff_day=cutoff_day) finally: await mpt_client.httpx_client.aclose() @@ -236,11 +234,11 @@ async def evaluate_journal_status(self, journal_external_id) -> dict[str, Any] | ) self.logger.error(error_msg) raise JournalStatusError(error_msg, journal_id) - except ValueError: + except ValueError as error: journal_status = journal["status"] error_msg = f"Found the journal {journal_id} with status {journal_status}" self.logger.error(error_msg) - raise JournalStatusError(error_msg, journal_id) + raise JournalStatusError(error_msg, journal_id) from error async def process(self) -> ProcessResultInfo: """ @@ -314,7 +312,6 @@ async def process(self) -> ProcessResultInfo: journal, journal_external_id, ) - await self.maybe_call(self._safe_unlink, filepath) result_info = ProcessResultInfo( authorization_id=self.authorization_id, result=ProcessResult.JOURNAL_GENERATED, @@ -337,6 +334,8 @@ async def process(self) -> ProcessResultInfo: result_info.message = str(error) result_info.journal_id = error.journal_id return result_info + finally: + await self.maybe_call(self._safe_unlink, filepath) except HTTPStatusError as error: status = error.response.status_code @@ -985,4 +984,4 @@ def split_entitlement_days_into_ranges(entitlement_days: set[int]): return ranges -VALID_JOURNAL_STATUSES = frozenset(JournalStatus) +VALID_JOURNAL_STATUSES = frozenset({JournalStatus.DRAFT, JournalStatus.VALIDATED}) diff --git a/backend/app/notifications.py b/backend/app/notifications.py index e0c6e35..6171120 100644 --- a/backend/app/notifications.py +++ b/backend/app/notifications.py @@ -1,5 +1,6 @@ import logging from dataclasses import dataclass +from typing import Any import httpx from adaptive_cards import card_types as ct @@ -13,6 +14,11 @@ logger = logging.getLogger(__name__) +TITLE_PREFIX_INFO = "\U0001f44d" # 👍 +TITLE_PREFIX_WARNING = "\u26a0\ufe0f" # ⚠ +TITLE_PREFIX_ERROR = "\U0001f4a3" # 💣 +TITLE_PREFIX_EXCEPTION = "\U0001f525" # 🔥 + @dataclass class ColumnHeader: @@ -23,8 +29,13 @@ class ColumnHeader: class NotificationDetails: def __init__(self, header: tuple[str | ColumnHeader, ...], rows: list[tuple[str, ...]]): - if not all(len(t) == len(header) for t in rows): - raise ValueError("All rows must have the same number of columns as the header.") + header_len = len(header) + for i, row in enumerate(rows): + if len(row) != header_len: + raise ValueError( + f"Row {i} has {len(row)} columns; expected {header_len}. " + f"All rows must have the same number of columns as the header." + ) self.header = header self.rows = rows @@ -63,7 +74,7 @@ def to_container(self) -> Container: items.append(ColumnSet(columns=header_columns)) # Data rows - for _idx, row in enumerate(self.rows): + for row in self.rows: row_columns = [] for col_idx, value in enumerate(row): col = self.header[col_idx] @@ -96,19 +107,15 @@ def to_container(self) -> Container: return Container(items=items) -async def send_notification( +def build_card_payload( + *, title: str, text: str, - title_color: ct.Colors = ct.Colors.DEFAULT, + title_color: ct.Colors, details: NotificationDetails | None = None, open_url: str | None = None, -) -> None: - settings = get_settings() - if not settings.msteams_notifications_webhook_url: # pragma: no cover - logger.warning("MSTeams notifications are disabled.") - return - - card_items = [ +) -> dict[str, Any]: + card_items: list[Any] = [ TextBlock( text=title, size=ct.FontSize.LARGE, @@ -125,7 +132,7 @@ async def send_notification( if details: card_items.append(details.to_container()) - card_actions = [] + card_actions: list[ActionOpenUrl] = [] if open_url: card_actions.append(ActionOpenUrl(title="Open", url=open_url)) @@ -143,6 +150,24 @@ async def send_notification( }, ], } + return message + + +async def send_notification( + title: str, + text: str, + title_color: ct.Colors = ct.Colors.DEFAULT, + details: NotificationDetails | None = None, + open_url: str | None = None, +) -> None: + settings = get_settings() + if not settings.msteams_notifications_webhook_url: # pragma: no cover + logger.warning("MSTeams notifications are disabled.") + return + + message = build_card_payload( + title=title, text=text, title_color=title_color, details=details, open_url=open_url + ) async with httpx.AsyncClient() as client: response = await client.post( @@ -150,7 +175,7 @@ async def send_notification( json=message, headers={"Content-Type": "application/json"}, ) - if response.status_code != 202: + if response.status_code not in (200, 202): logger.error( f"Failed to send notification to MSTeams: {response.status_code} - {response.text}" ) @@ -163,7 +188,7 @@ async def send_info( open_url: str | None = None, ) -> None: await send_notification( - f"\U0001f44d {title}", + f"{TITLE_PREFIX_INFO} {title}", text, title_color=ct.Colors.ACCENT, details=details, @@ -178,7 +203,7 @@ async def send_warning( open_url: str | None = None, ) -> None: await send_notification( - f"\u2622 {title}", + f"{TITLE_PREFIX_WARNING} {title}", text, title_color=ct.Colors.WARNING, details=details, @@ -193,7 +218,7 @@ async def send_error( open_url: str | None = None, ) -> None: await send_notification( - f"\U0001f4a3 {title}", + f"{TITLE_PREFIX_ERROR} {title}", text, title_color=ct.Colors.ATTENTION, details=details, @@ -208,7 +233,7 @@ async def send_exception( open_url: str | None = None, ) -> None: await send_notification( - f"\U0001f525 {title}", + f"{TITLE_PREFIX_EXCEPTION} {title}", text, title_color=ct.Colors.ATTENTION, details=details, diff --git a/backend/tests/api/test_accounts_api.py b/backend/tests/api/test_accounts_api.py index c660da2..ae16784 100644 --- a/backend/tests/api/test_accounts_api.py +++ b/backend/tests/api/test_accounts_api.py @@ -2,6 +2,12 @@ from uuid import uuid4 import pytest +from fastapi import HTTPException, status +from httpx import AsyncClient +from pytest_mock import MockerFixture +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + from app.db.handlers import NotFoundError from app.db.models import Account, AccountUser, System, User from app.dependencies.db import AccountRepository @@ -12,12 +18,6 @@ validate_required_conditions_before_update, ) from app.schemas.accounts import AccountCreate -from fastapi import HTTPException, status -from httpx import AsyncClient -from pytest_mock import MockerFixture -from sqlalchemy import select -from sqlalchemy.ext.asyncio import AsyncSession - from tests.types import ModelFactory diff --git a/backend/tests/api/test_additional_admin_api.py b/backend/tests/api/test_additional_admin_api.py index b31405f..965607d 100644 --- a/backend/tests/api/test_additional_admin_api.py +++ b/backend/tests/api/test_additional_admin_api.py @@ -1,12 +1,12 @@ import logging from _pytest.logging import LogCaptureFixture -from app import Settings -from app.db.models import Organization -from app.enums import OrganizationStatus from httpx import AsyncClient from pytest_httpx import HTTPXMock +from app import Settings +from app.db.models import Organization +from app.enums import OrganizationStatus from tests.types import ModelFactory FAKE_USER_ID = "1bf6f063-d90b-4d45-8e7f-62fefa9f5471" diff --git a/backend/tests/api/test_datasources_api.py b/backend/tests/api/test_datasources_api.py index abeef56..a9bbf70 100644 --- a/backend/tests/api/test_datasources_api.py +++ b/backend/tests/api/test_datasources_api.py @@ -2,12 +2,12 @@ from typing import Any import pytest -from app.conf import Settings -from app.db.models import Organization -from app.enums import DatasourceType from httpx import AsyncClient from pytest_httpx import HTTPXMock +from app.conf import Settings +from app.db.models import Organization +from app.enums import DatasourceType from tests.types import ModelFactory # =========================== diff --git a/backend/tests/api/test_employees_api.py b/backend/tests/api/test_employees_api.py index 8fbf22a..80660a6 100644 --- a/backend/tests/api/test_employees_api.py +++ b/backend/tests/api/test_employees_api.py @@ -1,11 +1,11 @@ import uuid -from app.conf import Settings -from app.db.models import Organization from httpx import AsyncClient from pytest_httpx import HTTPXMock from pytest_mock import MockerFixture +from app.conf import Settings +from app.db.models import Organization from tests.types import ModelFactory diff --git a/backend/tests/api/test_entitlements_api.py b/backend/tests/api/test_entitlements_api.py index 2f04a2f..086ce87 100644 --- a/backend/tests/api/test_entitlements_api.py +++ b/backend/tests/api/test_entitlements_api.py @@ -1,14 +1,14 @@ from datetime import UTC, datetime import pytest -from app.conf import Settings -from app.db.models import Account, Entitlement, System -from app.enums import AccountStatus, DatasourceType, EntitlementStatus, OrganizationStatus from httpx import AsyncClient from pytest_httpx import HTTPXMock from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession +from app.conf import Settings +from app.db.models import Account, Entitlement, System +from app.enums import AccountStatus, DatasourceType, EntitlementStatus, OrganizationStatus from tests.types import ModelFactory # ==================== diff --git a/backend/tests/api/test_expenses_api.py b/backend/tests/api/test_expenses_api.py index 052e479..83cee95 100644 --- a/backend/tests/api/test_expenses_api.py +++ b/backend/tests/api/test_expenses_api.py @@ -2,12 +2,12 @@ from decimal import Decimal import pytest -from app.db.models import DatasourceExpense, Organization -from app.enums import DatasourceType, OrganizationStatus from faker import Faker from httpx import AsyncClient from sqlalchemy.ext.asyncio import AsyncSession +from app.db.models import DatasourceExpense, Organization +from app.enums import DatasourceType, OrganizationStatus from tests.types import ModelFactory diff --git a/backend/tests/api/test_organizations_api.py b/backend/tests/api/test_organizations_api.py index d6c5e37..05b772a 100644 --- a/backend/tests/api/test_organizations_api.py +++ b/backend/tests/api/test_organizations_api.py @@ -1,13 +1,13 @@ import pytest -from app.conf import Settings -from app.db.models import Organization, System -from app.enums import OrganizationStatus from httpx import AsyncClient from pytest_httpx import HTTPXMock from pytest_mock import MockerFixture from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession +from app.conf import Settings +from app.db.models import Organization, System +from app.enums import OrganizationStatus from tests.types import ModelFactory # ================= diff --git a/backend/tests/commands/conftest.py b/backend/tests/commands/conftest.py index 47b4542..e8302b1 100644 --- a/backend/tests/commands/conftest.py +++ b/backend/tests/commands/conftest.py @@ -1,7 +1,8 @@ import pytest -from app.conf import Settings from pytest_mock import MockerFixture +from app.conf import Settings + @pytest.fixture(autouse=True) def mock_cli_settings(mocker: MockerFixture, test_settings: Settings) -> None: diff --git a/backend/tests/commands/test_calculate_accounts_stats.py b/backend/tests/commands/test_calculate_accounts_stats.py index de1a8f8..59aa174 100644 --- a/backend/tests/commands/test_calculate_accounts_stats.py +++ b/backend/tests/commands/test_calculate_accounts_stats.py @@ -1,8 +1,9 @@ import pytest +from sqlalchemy.ext.asyncio import AsyncSession + from app.commands.calculate_accounts_stats import calculate_accounts_stats from app.conf import Settings from app.enums import AccountStatus, EntitlementStatus -from sqlalchemy.ext.asyncio import AsyncSession @pytest.mark.parametrize( diff --git a/backend/tests/commands/test_cleanup_obsolete_datasource_expenses.py b/backend/tests/commands/test_cleanup_obsolete_datasource_expenses.py index 7564010..e109b17 100644 --- a/backend/tests/commands/test_cleanup_obsolete_datasource_expenses.py +++ b/backend/tests/commands/test_cleanup_obsolete_datasource_expenses.py @@ -3,15 +3,15 @@ import pytest import time_machine -from app.cli import app -from app.commands import cleanup_obsolete_datasource_expenses -from app.conf import Settings -from app.db.models import DatasourceExpense, Organization from pytest_mock import MockerFixture from sqlalchemy import func, select from sqlalchemy.ext.asyncio import AsyncSession from typer.testing import CliRunner +from app.cli import app +from app.commands import cleanup_obsolete_datasource_expenses +from app.conf import Settings +from app.db.models import DatasourceExpense, Organization from tests.types import ModelFactory diff --git a/backend/tests/commands/test_create_admin_account.py b/backend/tests/commands/test_create_admin_account.py index 6a98616..b044aad 100644 --- a/backend/tests/commands/test_create_admin_account.py +++ b/backend/tests/commands/test_create_admin_account.py @@ -1,13 +1,14 @@ import pytest +from pytest_mock import MockerFixture +from sqlalchemy.ext.asyncio import AsyncSession +from typer.testing import CliRunner + from app.cli import app from app.commands.create_admin_account import create_admin_account from app.conf import Settings from app.db.handlers import AccountHandler from app.db.models import Account from app.enums import AccountStatus, AccountType -from pytest_mock import MockerFixture -from sqlalchemy.ext.asyncio import AsyncSession -from typer.testing import CliRunner async def test_create_admin_account( diff --git a/backend/tests/commands/test_fetch_datasource_expenses.py b/backend/tests/commands/test_fetch_datasource_expenses.py index 65cd035..81de9c4 100644 --- a/backend/tests/commands/test_fetch_datasource_expenses.py +++ b/backend/tests/commands/test_fetch_datasource_expenses.py @@ -5,18 +5,18 @@ import pytest import time_machine -from app.cli import app -from app.commands import fetch_datasource_expenses -from app.conf import Settings -from app.db.handlers import DatasourceExpenseHandler -from app.db.models import DatasourceExpense, Organization -from app.enums import DatasourceType, OrganizationStatus from fastapi import status from pytest_httpx import HTTPXMock from pytest_mock import MockerFixture from sqlalchemy.ext.asyncio import AsyncSession from typer.testing import CliRunner +from app.cli import app +from app.commands import fetch_datasource_expenses +from app.conf import Settings +from app.db.handlers import DatasourceExpenseHandler +from app.db.models import DatasourceExpense, Organization +from app.enums import DatasourceType, OrganizationStatus from tests.fixtures.mock_api_clients import MockOptscaleClient from tests.types import ModelFactory diff --git a/backend/tests/commands/test_openapi.py b/backend/tests/commands/test_openapi.py index 7b4150c..55179c7 100644 --- a/backend/tests/commands/test_openapi.py +++ b/backend/tests/commands/test_openapi.py @@ -3,10 +3,11 @@ from pathlib import Path import yaml -from app.cli import app from pytest_mock import MockerFixture from typer.testing import CliRunner +from app.cli import app + def test_openapi(mocker: MockerFixture): spec = {"test": "openapi"} diff --git a/backend/tests/commands/test_redeem_entitlements.py b/backend/tests/commands/test_redeem_entitlements.py index 7c88c42..518c5d4 100644 --- a/backend/tests/commands/test_redeem_entitlements.py +++ b/backend/tests/commands/test_redeem_entitlements.py @@ -2,17 +2,18 @@ import pytest import time_machine +from httpx import HTTPStatusError, ReadTimeout +from pytest_httpx import HTTPXMock +from pytest_mock import MockerFixture +from sqlalchemy.ext.asyncio import AsyncSession +from typer.testing import CliRunner + from app.cli import app from app.commands.redeem_entitlements import fetch_datasources_for_organization, redeem_entitlements from app.conf import Settings from app.db.models import Entitlement, Organization from app.enums import DatasourceType, EntitlementStatus from app.notifications import ColumnHeader -from httpx import HTTPStatusError, ReadTimeout -from pytest_httpx import HTTPXMock -from pytest_mock import MockerFixture -from sqlalchemy.ext.asyncio import AsyncSession -from typer.testing import CliRunner @time_machine.travel("2025-03-07T10:00:00Z", tick=False) diff --git a/backend/tests/commands/test_serve.py b/backend/tests/commands/test_serve.py index c4e4caf..ba62737 100644 --- a/backend/tests/commands/test_serve.py +++ b/backend/tests/commands/test_serve.py @@ -1,7 +1,8 @@ -from app.cli import app from pytest_mock import MockerFixture from typer.testing import CliRunner +from app.cli import app + def test_serve(mocker: MockerFixture): mocked_settings = mocker.MagicMock() diff --git a/backend/tests/commands/test_shell.py b/backend/tests/commands/test_shell.py index 31d2775..29f5abc 100644 --- a/backend/tests/commands/test_shell.py +++ b/backend/tests/commands/test_shell.py @@ -1,7 +1,8 @@ -from app.cli import app from pytest_mock import MockerFixture from typer.testing import CliRunner +from app.cli import app + def test_shell(mocker: MockerFixture): mocked_ishell = mocker.patch("app.commands.shell.InteractiveShellEmbed") diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 728bb0b..b46e7ea 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -11,6 +11,16 @@ import jwt import pytest import responses +from asgi_lifespan import LifespanManager +from faker import Faker +from fastapi import FastAPI +from httpx import ASGITransport, AsyncClient +from pytest_asyncio import is_async_test +from sqlalchemy.ext.asyncio import ( + AsyncEngine, + AsyncSession, +) + from app.api_clients.base import BaseAPIClient from app.billing.dataclasses import ProcessResultInfo from app.billing.enum import ProcessResult @@ -39,16 +49,6 @@ SystemStatus, UserStatus, ) -from asgi_lifespan import LifespanManager -from faker import Faker -from fastapi import FastAPI -from httpx import ASGITransport, AsyncClient -from pytest_asyncio import is_async_test -from sqlalchemy.ext.asyncio import ( - AsyncEngine, - AsyncSession, -) - from tests.db.models import ModelForTests, ParentModelForTests # noqa: F401 from tests.types import ModelFactory @@ -2533,3 +2533,41 @@ def process_result_with_error(): def temp_charges_file(): with tempfile.NamedTemporaryFile("w", suffix=".jsonl", delete=False) as f: yield f.name + + +MSTEAMS_FAKE_WEBHOOK_URL = "https://localhost/webhook" + + +@pytest.fixture() +def _no_real_http_post(mocker): + """Replace ``httpx.AsyncClient.post`` with an ``AsyncMock`` returning a + fake 202 response. Opt-in (not autouse): tests that do not need it must + not pull it in, otherwise FastAPI integration tests using ASGI transport + would have their POSTs intercepted too. + """ + mock_response = mocker.MagicMock() + mock_response.status_code = 202 + mock_response.text = "" + return mocker.patch( + "httpx.AsyncClient.post", + new=mocker.AsyncMock(return_value=mock_response), + ) + + +@pytest.fixture() +def configured_webhook(mocker): + mocked_settings = mocker.MagicMock() + mocked_settings.msteams_notifications_webhook_url = MSTEAMS_FAKE_WEBHOOK_URL + mocker.patch("app.notifications.get_settings", return_value=mocked_settings) + return mocked_settings + + +@pytest.fixture() +def send_notification_spy(mocker): + """Observes calls to ``app.notifications.send_notification`` without + replacing it. Pair with ``_no_real_http_post`` to keep the underlying + POST offline. + """ + import app.notifications as notifications_module + + return mocker.spy(notifications_module, "send_notification") diff --git a/backend/tests/db/models.py b/backend/tests/db/models.py index c9bda51..897e96c 100644 --- a/backend/tests/db/models.py +++ b/backend/tests/db/models.py @@ -1,10 +1,11 @@ import enum -from app.db.human_readable_pk import HumanReadablePKMixin -from app.db.models import AuditableMixin, Base from sqlalchemy import Enum, ForeignKey, String from sqlalchemy.orm import Mapped, mapped_column, relationship +from app.db.human_readable_pk import HumanReadablePKMixin +from app.db.models import AuditableMixin, Base + class ModelForTests(Base, HumanReadablePKMixin): __tablename__ = "test_models" diff --git a/backend/tests/db/test_handlers.py b/backend/tests/db/test_handlers.py index ecb4f25..6549248 100644 --- a/backend/tests/db/test_handlers.py +++ b/backend/tests/db/test_handlers.py @@ -1,4 +1,8 @@ import pytest +from pytest_mock import MockerFixture +from sqlalchemy.ext.asyncio import AsyncSession +from sqlalchemy.orm import joinedload + from app.auth.context import AuthenticationContext from app.db.handlers import ( AccountUserHandler, @@ -9,10 +13,6 @@ ) from app.db.models import Account, AccountUser, Base, User from app.enums import AccountStatus, AccountUserStatus, UserStatus -from pytest_mock import MockerFixture -from sqlalchemy.ext.asyncio import AsyncSession -from sqlalchemy.orm import joinedload - from tests.db.models import ( DeletableAuditModelForTests, DeletableModelForTests, diff --git a/backend/tests/db/test_models.py b/backend/tests/db/test_models.py index bbafae2..2892aa6 100644 --- a/backend/tests/db/test_models.py +++ b/backend/tests/db/test_models.py @@ -1,14 +1,15 @@ from datetime import UTC, datetime import pytest -from app.db.human_readable_pk import HumanReadablePKMixin -from app.db.models import Account, Actor, Entitlement, Organization, System -from app.enums import ActorType, EntitlementStatus from pytest_mock import MockerFixture from sqlalchemy import select from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncSession +from app.db.human_readable_pk import HumanReadablePKMixin +from app.db.models import Account, Actor, Entitlement, Organization, System +from app.enums import ActorType, EntitlementStatus + async def test_actor_inheritance(db_session: AsyncSession): # Create a system which inherits from Actor diff --git a/backend/tests/fixtures/mock_api_clients.py b/backend/tests/fixtures/mock_api_clients.py index fc72cd4..25aa1e8 100644 --- a/backend/tests/fixtures/mock_api_clients.py +++ b/backend/tests/fixtures/mock_api_clients.py @@ -4,13 +4,14 @@ import httpx import pytest +from fastapi import status +from pydantic.v1.utils import deep_update +from pytest_httpx import HTTPXMock + from app.api_clients.base import BaseAPIClient from app.api_clients.optscale import OptscaleClient from app.conf import Settings from app.db.models import Organization -from fastapi import status -from pydantic.v1.utils import deep_update -from pytest_httpx import HTTPXMock C = TypeVar("C", bound=BaseAPIClient) diff --git a/backend/tests/test_exchange_rates_client.py b/backend/tests/test_exchange_rates_client.py index 94f9210..2b80f23 100644 --- a/backend/tests/test_exchange_rates_client.py +++ b/backend/tests/test_exchange_rates_client.py @@ -2,6 +2,7 @@ import httpx import pytest + from app.api_clients.exchage_rates import ExchangeRatesClient from app.billing.exceptions import ExchangeRatesClientError diff --git a/backend/tests/test_logging.py b/backend/tests/test_logging.py index 960ea80..63ef360 100644 --- a/backend/tests/test_logging.py +++ b/backend/tests/test_logging.py @@ -1,5 +1,6 @@ import pytest import pytest_mock + from app.logging import get_logging_config, setup_logging diff --git a/backend/tests/test_notification_helper.py b/backend/tests/test_notification_helper.py new file mode 100644 index 0000000..6f01c14 --- /dev/null +++ b/backend/tests/test_notification_helper.py @@ -0,0 +1,168 @@ +import pytest +from freezegun import freeze_time + +from app.billing.dataclasses import ProcessResultInfo +from app.billing.enum import ProcessResult +from app.billing.notification_helper import ( + MAX_MESSAGE_CHARS, + _chunk_rows_by_size, + send_notifications, +) + +# Module-level: every test in this file gets the httpx mock without having +# to declare the fixture explicitly. Equivalent to a file-scoped autouse, +# but the fixture itself stays opt-in at the conftest level so it doesn't +# leak into FastAPI integration tests that use httpx for real ASGI calls. +pytestmark = pytest.mark.usefixtures("_no_real_http_post") + + +def _make_results( + count: int, result: ProcessResult, message_size: int = 400 +) -> list[ProcessResultInfo]: + """Build a list of ProcessResultInfo entries large enough to force chunking.""" + long_message = "x" * message_size + return [ + ProcessResultInfo( + authorization_id=f"AUTH-{i:04d}-0000", + journal_id=f"BJO-{i:04d}-0000", + result=result, + message=f"{long_message} idx={i}", + ) + for i in range(count) + ] + + +def _extract_titles(spy) -> list[str]: + titles: list[str] = [] + for call in spy.await_args_list or spy.call_args_list: + title = call.args[0] if call.args else call.kwargs.get("title") + titles.append(title) + return titles + + +async def test_send_notifications_success_splits_into_multiple_messages( + configured_webhook, send_notification_spy +): + """A large success result set must produce more than one real HTTP POST + to the configured Teams webhook, with each message titled `(Part i/N)`.""" + results = _make_results(count=80, result=ProcessResult.JOURNAL_GENERATED) + + await send_notifications(results=results, year=2025, month=9) + + total = send_notification_spy.call_count + assert total > 1, f"expected multiple messages, got {total}" + + titles = _extract_titles(send_notification_spy) + for idx, title in enumerate(titles, start=1): + assert f"(Part {idx}/{total})" in title + + +@freeze_time("2025-09-08") +async def test_send_notifications_error_splits_into_multiple_messages( + configured_webhook, send_notification_spy +): + """A large error result set (after the cutoff day) must split into + multiple ERROR notifications, all really sent to Teams.""" + results = _make_results(count=80, result=ProcessResult.ERROR) + + await send_notifications(results=results, year=2025, month=9, cutoff_day=5) + + total = send_notification_spy.call_count + assert total > 1 + + titles = _extract_titles(send_notification_spy) + for idx, title in enumerate(titles, start=1): + assert f"(Part {idx}/{total})" in title + assert "completed with Errors" in title + + +@freeze_time("2025-09-01") +async def test_send_notifications_in_progress_splits_into_multiple_messages( + configured_webhook, send_notification_spy +): + """Before the cutoff day, errors are reported as IN_PROGRESS and must + still be chunked across multiple real messages.""" + results = _make_results(count=80, result=ProcessResult.ERROR) + + await send_notifications(results=results, year=2025, month=9, cutoff_day=5) + + total = send_notification_spy.call_count + assert total > 1 + + titles = _extract_titles(send_notification_spy) + for idx, title in enumerate(titles, start=1): + assert f"(Part {idx}/{total})" in title + assert "in progress" in title + + +async def test_send_notifications_small_payload_sends_single_message( + configured_webhook, send_notification_spy +): + """A small result set must produce exactly one real HTTP POST and the + title must NOT contain a `(Part i/N)` suffix.""" + results = _make_results(count=2, result=ProcessResult.JOURNAL_GENERATED, message_size=20) + + await send_notifications(results=results, year=2025, month=9) + + assert send_notification_spy.call_count == 1 + titles = _extract_titles(send_notification_spy) + assert "(Part" not in titles[0] + + +async def test_send_notifications_each_part_carries_subset_of_rows( + configured_webhook, send_notification_spy +): + """Every authorization_id in the input must appear in exactly one of + the sent messages, with no duplicates and no losses across the chunks.""" + results = _make_results(count=80, result=ProcessResult.JOURNAL_GENERATED) + expected_auth_ids = {r.authorization_id for r in results} + + await send_notifications(results=results, year=2025, month=9) + + assert send_notification_spy.call_count > 1 + + seen: list[str] = [] + for call in send_notification_spy.call_args_list: + details = call.kwargs.get("details") + assert details is not None + for row in details.rows: + seen.append(row[0]) + + assert sorted(seen) == sorted(expected_auth_ids), ( + "each authorization_id should appear in exactly one part" + ) + + +async def test_send_notifications_truncates_long_message(configured_webhook, send_notification_spy): + """Messages longer than MAX_MESSAGE_CHARS must be truncated before they + reach the rendered row, so any content past the cap never lands in Teams.""" + sentinel = "PAST_THE_CAP" + results = [ + ProcessResultInfo( + authorization_id="AUTH-0001-0000", + journal_id="BJO-0001-0000", + result=ProcessResult.JOURNAL_GENERATED, + message="x" * MAX_MESSAGE_CHARS + sentinel, + ) + ] + + await send_notifications(results=results, year=2025, month=9) + + assert send_notification_spy.call_count == 1 + details = send_notification_spy.call_args_list[0].kwargs["details"] + rendered_message = details.rows[0][3] + assert sentinel not in rendered_message + + +def test_chunk_rows_by_size_raises_when_single_row_exceeds_budget(): + """The chunker's invariant is that no single row exceeds the budget on + its own. _build_rows enforces this via MAX_MESSAGE_CHARS; the chunker + is the defense-in-depth check that fails loudly if the invariant breaks.""" + big_row = ("AUTH-0001-0000", "BJO-0001-0000", "X", "x" * 30_000) + + # Fake measure: empty card = 100 bytes, each row contributes its char total. + def measure(chunk: list[tuple[str, ...]]) -> int: + return 100 + sum(len(value) for row in chunk for value in row) + + with pytest.raises(ValueError, match="exceeds per-chunk budget"): + _chunk_rows_by_size(rows=[big_row], measure=measure, budget=1024) diff --git a/backend/tests/test_notifications.py b/backend/tests/test_notifications.py index 6d55369..724dbfa 100644 --- a/backend/tests/test_notifications.py +++ b/backend/tests/test_notifications.py @@ -1,5 +1,8 @@ import pytest from adaptive_cards import card_types as ct +from pytest_httpx import HTTPXMock +from pytest_mock import MockerFixture + from app.notifications import ( NotificationDetails, send_error, @@ -8,15 +11,13 @@ send_notification, send_warning, ) -from pytest_httpx import HTTPXMock -from pytest_mock import MockerFixture @pytest.mark.parametrize( ("function", "color", "icon"), [ (send_info, ct.Colors.ACCENT, "\U0001f44d"), - (send_warning, ct.Colors.WARNING, "\u2622"), + (send_warning, ct.Colors.WARNING, "\u26a0\ufe0f"), (send_error, ct.Colors.ATTENTION, "\U0001f4a3"), (send_exception, ct.Colors.ATTENTION, "\U0001f525"), ], diff --git a/backend/tests/test_openapi.py b/backend/tests/test_openapi.py index bac8e37..cae8464 100644 --- a/backend/tests/test_openapi.py +++ b/backend/tests/test_openapi.py @@ -1,6 +1,7 @@ +from fastapi import FastAPI + from app.conf import Settings from app.openapi import generate_openapi_spec -from fastapi import FastAPI def test_gen_openapi(fastapi_app: FastAPI, test_settings: Settings): diff --git a/backend/tests/test_process_billing.py b/backend/tests/test_process_billing.py index 307e32d..b265df2 100644 --- a/backend/tests/test_process_billing.py +++ b/backend/tests/test_process_billing.py @@ -9,11 +9,12 @@ import httpx import pytest import typer +from freezegun import freeze_time + from app.billing.dataclasses import CurrencyConversionInfo, ProcessResultInfo, Refund from app.billing.enum import ProcessResult from app.billing.exceptions import ExchangeRatesClientError, JournalStatusError, JournalSubmitError from app.billing.notification_helper import check_results -from freezegun import freeze_time # - test evaluate_journal_status() diff --git a/backend/tests/test_schemas.py b/backend/tests/test_schemas.py index a479b88..a18329e 100644 --- a/backend/tests/test_schemas.py +++ b/backend/tests/test_schemas.py @@ -1,6 +1,8 @@ from datetime import UTC, datetime import pytest +from pydantic import ValidationError + from app.db.models import Account, Actor, Entitlement, Organization, System from app.enums import AccountType, ActorType, EntitlementStatus, OrganizationStatus, SystemStatus from app.schemas.core import ActorRead, convert_model_to_schema, convert_schema_to_model @@ -12,7 +14,6 @@ OrganizationUpdate, ) from app.schemas.systems import SystemCreate, SystemRead -from pydantic import ValidationError def test_actor_read_convert_model_to_schema(): diff --git a/backend/tests/test_telemetry.py b/backend/tests/test_telemetry.py index 49122ee..f296c80 100644 --- a/backend/tests/test_telemetry.py +++ b/backend/tests/test_telemetry.py @@ -1,9 +1,10 @@ +from pytest_mock import MockerFixture + from app.telemetry import ( setup_fastapi_instrumentor, setup_sqlalchemy_instrumentor, setup_telemetry, ) -from pytest_mock import MockerFixture def test_setup_telemetry(mocker: MockerFixture): diff --git a/backend/tests/test_utils.py b/backend/tests/test_utils.py index ea239ce..d1c4b14 100644 --- a/backend/tests/test_utils.py +++ b/backend/tests/test_utils.py @@ -1,6 +1,7 @@ +from pytest_mock import MockerFixture + from app.conf import Settings from app.utils import send_email -from pytest_mock import MockerFixture def test_send_email_success(test_settings: Settings, mocker: MockerFixture):