diff --git a/.env.example b/.env.example index 1645fa31..e6ead732 100644 --- a/.env.example +++ b/.env.example @@ -77,3 +77,11 @@ JIRA_API_TOKEN=your_jira_api_token JIRA_DEFAULT_PROJECT=BDMS # Optional — Slack notifications are skipped if this is blank SLACK_FEEDBACK_WEBHOOK_URL= + +# Edit notifications (Slack) — POST/PATCH/DELETE mutations and uploads +# Optional — notifications are skipped if this is blank +SLACK_EDITS_WEBHOOK_URL= +# Base URL for deep links in Slack messages (no trailing slash) +# Staging: https://ocotillo-staging.newmexicowaterdata.org +# Production: https://ocotillo.newmexicowaterdata.org +OCOTILLO_UI_BASE_URL= diff --git a/.github/app.template.yaml b/.github/app.template.yaml index d3eb23ab..3abdacb6 100644 --- a/.github/app.template.yaml +++ b/.github/app.template.yaml @@ -45,3 +45,6 @@ env_variables: JIRA_DEFAULT_PROJECT: "${JIRA_DEFAULT_PROJECT}" SLACK_FEEDBACK_WEBHOOK_URL: |- ${SLACK_FEEDBACK_WEBHOOK_URL} + SLACK_EDITS_WEBHOOK_URL: |- + ${SLACK_EDITS_WEBHOOK_URL} + OCOTILLO_UI_BASE_URL: "${OCOTILLO_UI_BASE_URL}" diff --git a/.github/workflows/CD_production.yml b/.github/workflows/CD_production.yml index 2f89abef..0985f7a7 100644 --- a/.github/workflows/CD_production.yml +++ b/.github/workflows/CD_production.yml @@ -82,6 +82,7 @@ jobs: jira_email:${{ vars.GCP_PROJECT_ID }}/jira-email jira_api_token:${{ vars.GCP_PROJECT_ID }}/jira-api-token slack_feedback_webhook_url:${{ vars.GCP_PROJECT_ID }}/slack-feedback-webhook-url + slack_edits_webhook_url:${{ vars.GCP_PROJECT_ID }}/slack-edits-webhook-url - name: Run Alembic migrations on production database env: @@ -142,6 +143,8 @@ jobs: JIRA_API_TOKEN: "${{ steps.feedback-secrets.outputs.jira_api_token }}" JIRA_DEFAULT_PROJECT: "${{ vars.JIRA_DEFAULT_PROJECT || 'BDMS' }}" SLACK_FEEDBACK_WEBHOOK_URL: "${{ steps.feedback-secrets.outputs.slack_feedback_webhook_url }}" + SLACK_EDITS_WEBHOOK_URL: "${{ steps.feedback-secrets.outputs.slack_edits_webhook_url }}" + OCOTILLO_UI_BASE_URL: "${{ vars.OCOTILLO_UI_BASE_URL || 'https://ocotillo.newmexicowaterdata.org' }}" run: | export MAX_INSTANCES="10" export SERVICE_NAME="ocotillo-api" diff --git a/.github/workflows/CD_staging.yml b/.github/workflows/CD_staging.yml index 94012dbd..357dba3b 100644 --- a/.github/workflows/CD_staging.yml +++ b/.github/workflows/CD_staging.yml @@ -47,6 +47,7 @@ jobs: jira_email:${{ vars.GCP_PROJECT_ID }}/jira-email jira_api_token:${{ vars.GCP_PROJECT_ID }}/jira-api-token slack_feedback_webhook_url:${{ vars.GCP_PROJECT_ID }}/slack-feedback-webhook-url + slack_edits_webhook_url:${{ vars.GCP_PROJECT_ID }}/slack-edits-webhook-url - name: Run Alembic migrations on staging database env: @@ -102,6 +103,8 @@ jobs: JIRA_API_TOKEN: "${{ steps.feedback-secrets.outputs.jira_api_token }}" JIRA_DEFAULT_PROJECT: "${{ vars.JIRA_DEFAULT_PROJECT || 'BDMS' }}" SLACK_FEEDBACK_WEBHOOK_URL: "${{ steps.feedback-secrets.outputs.slack_feedback_webhook_url }}" + SLACK_EDITS_WEBHOOK_URL: "${{ steps.feedback-secrets.outputs.slack_edits_webhook_url }}" + OCOTILLO_UI_BASE_URL: "${{ vars.OCOTILLO_UI_BASE_URL || 'https://ocotillo-staging.newmexicowaterdata.org' }}" run: | export MAX_INSTANCES="10" export SERVICE_NAME="ocotillo-api-staging" diff --git a/.github/workflows/CD_testing.yml b/.github/workflows/CD_testing.yml index be7fffb9..c10ff6d6 100644 --- a/.github/workflows/CD_testing.yml +++ b/.github/workflows/CD_testing.yml @@ -47,6 +47,7 @@ jobs: jira_email:${{ vars.GCP_PROJECT_ID }}/jira-email jira_api_token:${{ vars.GCP_PROJECT_ID }}/jira-api-token slack_feedback_webhook_url:${{ vars.GCP_PROJECT_ID }}/slack-feedback-webhook-url + slack_edits_webhook_url:${{ vars.GCP_PROJECT_ID }}/slack-edits-webhook-url - name: Run Alembic migrations on staging database env: @@ -102,6 +103,8 @@ jobs: JIRA_API_TOKEN: "${{ steps.feedback-secrets.outputs.jira_api_token }}" JIRA_DEFAULT_PROJECT: "${{ vars.JIRA_DEFAULT_PROJECT || 'BDMS' }}" SLACK_FEEDBACK_WEBHOOK_URL: "${{ steps.feedback-secrets.outputs.slack_feedback_webhook_url }}" + SLACK_EDITS_WEBHOOK_URL: "${{ steps.feedback-secrets.outputs.slack_edits_webhook_url }}" + OCOTILLO_UI_BASE_URL: "${{ vars.OCOTILLO_UI_BASE_URL || 'https://ocotillo-staging.newmexicowaterdata.org' }}" run: | export MAX_INSTANCES="10" export SERVICE_NAME="ocotillo-api-testing" diff --git a/api/asset.py b/api/asset.py index f56f3f3d..32fa8858 100644 --- a/api/asset.py +++ b/api/asset.py @@ -41,6 +41,11 @@ from schemas.asset import AssetResponse, CreateAsset, UpdateAsset from services.audit_helper import audit_add from services.crud_helper import model_patcher, model_deleter +from services.edit_notification_helper import ( + EditEvent, + format_file_size, + notify_edit_event, +) from services.env import get_bool_env from services.exceptions_helper import PydanticStyleException from services.query_helper import simple_get_by_id @@ -345,6 +350,29 @@ async def upload_and_record_asset( raise session.refresh(asset) + + thing_label = thing.name or f"Thing {thing_id}" + file_name = asset.name or file.filename or "attachment" + + notify_edit_event( + user, + EditEvent( + action="attachment_uploaded", + resource_type="well" if thing.thing_type == "water well" else "thing", + resource_id=thing_id, + resource_label=thing_label, + summary=( + f"Uploaded {file_name} ({asset.mime_type}, " + f"{format_file_size(asset.size or file_size)}) to {thing_label}" + ), + metadata={ + "file_name": file_name, + "mime_type": asset.mime_type, + "size": asset.size or file_size, + "asset_id": asset.id, + }, + ), + ) return asset diff --git a/api/group.py b/api/group.py index d095dfd9..bd507c79 100644 --- a/api/group.py +++ b/api/group.py @@ -131,7 +131,7 @@ def remove_thing_from_group_route( "/{group_id}", summary="Delete a group by ID", status_code=HTTP_204_NO_CONTENT ) def delete_group(user: admin_dependency, group_id: int, session: session_dependency): - return model_deleter(session, Group, group_id) + return model_deleter(session, Group, group_id, user=user) # ============= EOF ============================================= diff --git a/docs/edit-notifications-and-activity-log.md b/docs/edit-notifications-and-activity-log.md new file mode 100644 index 00000000..522c7ff3 --- /dev/null +++ b/docs/edit-notifications-and-activity-log.md @@ -0,0 +1,77 @@ +# Edit notifications and the activity log (Epic 6) + +BDMS-921 adds Slack notifications when Ocotillo data is edited. Epic 6 (activity log) will persist the same events for in-app history. This document describes how the two fit together. + +## Current state (BDMS-921) + +Mutations in the OcotilloAPI service layer call `notify_edit_event(user, event)` from `services/edit_notification_helper.py`. + +- `EditEvent` carries action, resource type/id/label, summary, optional field diffs, and metadata. +- When `SLACK_EDITS_WEBHOOK_URL` is set, the helper posts a Block Kit message to Slack in a background thread. +- When the webhook is unset (local dev), or `user` is not a dict (auth disabled in tests), notification is a no-op. +- Failures are logged and never fail the HTTP request. + +Wired today: + +| Action | Where | +|--------|--------| +| `attachment_uploaded` | `api/asset.py` `upload_and_record_asset` (new uploads only) | +| `project_added` / `project_removed` | `services/group_helper.py` | +| `record_created` / `record_updated` / `record_deleted` | `services/crud_helper.py` | + +## Future state (Epic 6.1) + +Epic 6 introduces an `ActivityLog` table and a service helper, roughly: + +```python +def log_activity( + session, + actor, + action, + resource_type, + resource_id, + *, + resource_label=None, + field_changes=None, + metadata=None, +): + # persist ActivityLog row (not built yet) + notify_edit_event( + actor, + EditEvent( + action=action, + resource_type=resource_type, + resource_id=resource_id, + resource_label=resource_label or f"ID {resource_id}", + summary=_activity_summary(...), + field_changes=field_changes, + metadata=metadata or {}, + ), + ) +``` + +### Migration path + +1. **Keep `EditEvent` as the shared event shape** so Slack payloads and the activity log UI read the same fields (`actor`, `action`, `resource_*`, `field_changes`, `metadata`). +2. **Move call sites from `notify_edit_event` to `log_activity`** as Epic 6.1 lands. `log_activity` writes to PostgreSQL first, then calls `notify_edit_event` as a side effect. +3. **Retire direct `notify_edit_event` calls** in route handlers and one-off helpers once those paths go through `log_activity`. +4. **Map action names** between Slack labels and Epic 6 enums where they differ (e.g. `project_added` → activity log `update` with metadata describing the project change). + +### Field diffs + +`model_patcher` already computes `{field: {before, after}}` for Slack. Epic 6 stores the same JSON on `ActivityLog.field_changes`. No second diff format is needed. + +### Exclusions (unchanged) + +- `POST /feedback` keeps its own Slack webhook. +- Transfer scripts, bulk imports, and non-user mutations should not call `log_activity` or `notify_edit_event`. + +## Environment variables + +| Variable | Purpose | +|----------|---------| +| `SLACK_EDITS_WEBHOOK_URL` | Incoming webhook for edit notifications (Secret Manager in deployed envs) | +| `OCOTILLO_UI_BASE_URL` | UI origin for deep links in Slack messages | +| `ENVIRONMENT` | `staging` or `production`; prefixed in Slack headers | + +See `.env.example` for local defaults. diff --git a/services/crud_helper.py b/services/crud_helper.py index 01eaeb25..49d1c914 100644 --- a/services/crud_helper.py +++ b/services/crud_helper.py @@ -13,14 +13,26 @@ # See the License for the specific language governing permissions and # limitations under the License. # =============================================================================== +from typing import Any + from fastapi import Response from pydantic import BaseModel -from sqlalchemy.orm import Session, DeclarativeBase +from sqlalchemy.orm import DeclarativeBase, Session from starlette.status import HTTP_204_NO_CONTENT from db.notes import NotesMixin +from services.edit_notification_helper import EditEvent, notify_edit_event from services.query_helper import simple_get_by_id +TABLE_RESOURCE_TYPES: dict[str, str] = { + "contact": "contact", + "group": "group", + "asset": "asset", + "location": "location", + "sensor": "sensor", + "sample": "sample", +} + def model_adder(session, table, model, user=None, **kwargs): """ @@ -53,6 +65,22 @@ def model_adder(session, table, model, user=None, **kwargs): session.commit() session.refresh(obj) + + if user: + resource_type = _resource_type_for_item(table, obj) + if resource_type: + label = _resource_label(obj) + notify_edit_event( + user, + EditEvent( + action="record_created", + resource_type=resource_type, + resource_id=obj.id, + resource_label=label, + summary=f"Created {resource_type} {label}", + ), + ) + return obj @@ -72,8 +100,10 @@ def model_patcher( exclude_unset ensures that fields that are not set in the payload do not update record fields to None """ + updates = payload.model_dump(exclude_unset=True) + before = _snapshot_field_values(item, updates.keys()) - for key, value in payload.model_dump(exclude_unset=True).items(): + for key, value in updates.items(): if isinstance(item, NotesMixin) and key == "notes": # delete all notes and re-add for note in item.notes: @@ -91,15 +121,106 @@ def model_patcher( session.commit() session.refresh(item) + + if user: + resource_type = _resource_type_for_item(model, item) + if resource_type: + label = _resource_label(item) + field_changes = _compute_field_changes(before, item, updates.keys()) + summary = f"Updated {resource_type} {label}" + notify_edit_event( + user, + EditEvent( + action="record_updated", + resource_type=resource_type, + resource_id=item.id, + resource_label=label, + summary=summary, + field_changes=field_changes or None, + ), + ) + return item -def model_deleter(session: Session, model: DeclarativeBase, item_id: int): +def model_deleter( + session: Session, + model: DeclarativeBase, + item_id: int, + user: dict | None = None, +): # simple_get_by_id raises HTTP_404_NOT_FOUND if the item is not found item = simple_get_by_id(session, model, item_id) + resource_type = _resource_type_for_item(model, item) + label = _resource_label(item) + item_id_value = item.id + session.delete(item) session.commit() + + if user and resource_type: + notify_edit_event( + user, + EditEvent( + action="record_deleted", + resource_type=resource_type, + resource_id=item_id_value, + resource_label=label, + summary=f"Deleted {resource_type} {label}", + ), + ) + return Response(status_code=HTTP_204_NO_CONTENT) +def _resource_type_for_item(model: type, item: Any) -> str | None: + table = getattr(model, "__tablename__", None) + if table == "thing": + thing_type = getattr(item, "thing_type", None) + if thing_type == "water well": + return "well" + if thing_type == "spring": + return "spring" + return "thing" + return TABLE_RESOURCE_TYPES.get(table or "") + + +def _resource_label(item: Any) -> str: + for attr in ("name", "label", "title", "site_name"): + value = getattr(item, attr, None) + if value: + return str(value) + item_id = getattr(item, "id", None) + return f"ID {item_id}" if item_id is not None else "record" + + +def _snapshot_field_values(item: Any, keys: Any) -> dict[str, Any]: + return {key: _serialize_field_value(getattr(item, key, None)) for key in keys} + + +def _compute_field_changes( + before: dict[str, Any], + item: Any, + keys: Any, +) -> dict[str, dict[str, Any]]: + changes: dict[str, dict[str, Any]] = {} + for key in keys: + after_value = _serialize_field_value(getattr(item, key, None)) + if before.get(key) != after_value: + changes[key] = {"before": before.get(key), "after": after_value} + return changes + + +def _serialize_field_value(value: Any) -> Any: + if value is None: + return None + if hasattr(value, "isoformat"): + return value.isoformat() + if hasattr(value, "wkt"): + return value.wkt + if isinstance(value, (list, dict)): + return value + return value + + # ============= EOF ============================================= diff --git a/services/edit_notification_helper.py b/services/edit_notification_helper.py new file mode 100644 index 00000000..13052972 --- /dev/null +++ b/services/edit_notification_helper.py @@ -0,0 +1,215 @@ +# =============================================================================== +# Copyright 2025 ross +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# =============================================================================== +from __future__ import annotations + +import logging +import os +import threading +from datetime import datetime, timezone +from typing import Any, Literal + +import httpx +from pydantic import BaseModel, Field + +logger = logging.getLogger(__name__) + +EditAction = Literal[ + "attachment_uploaded", + "project_added", + "project_removed", + "record_updated", + "record_created", + "record_deleted", +] + +NOTIFY_RESOURCE_TYPES = frozenset( + { + "well", + "spring", + "thing", + "contact", + "asset", + "group", + "location", + "sensor", + "sample", + } +) + +ACTION_HEADINGS: dict[EditAction, str] = { + "attachment_uploaded": "Attachment uploaded", + "project_added": "Project added", + "project_removed": "Project removed", + "record_updated": "Record updated", + "record_created": "Record created", + "record_deleted": "Record deleted", +} + +RESOURCE_UI_PATHS: dict[str, str] = { + "well": "ocotillo/well/show/{resource_id}", + "spring": "ocotillo/spring/show/{resource_id}", + "thing": "ocotillo/well/show/{resource_id}", + "contact": "ocotillo/contact/show/{resource_id}", + "group": "ocotillo/group/show/{resource_id}", + "asset": "ocotillo/asset/show/{resource_id}", + "location": "ocotillo/location/show/{resource_id}", + "sensor": "ocotillo/sensor/show/{resource_id}", + "sample": "ocotillo/sample/show/{resource_id}", +} + + +class EditEvent(BaseModel): + action: EditAction + resource_type: str + resource_id: int | str + resource_label: str + summary: str + field_changes: dict[str, dict[str, Any]] | None = None + metadata: dict[str, Any] = Field(default_factory=dict) + + +def format_file_size(size_bytes: int) -> str: + if size_bytes < 1024: + return f"{size_bytes} B" + if size_bytes < 1024**2: + return f"{size_bytes / 1024:.1f} KB" + return f"{size_bytes / (1024**2):.1f} MB" + + +def environment_label(environment: str | None = None) -> str: + raw = environment or os.environ.get("ENVIRONMENT", "unknown") + env = raw.strip().lower() + if env == "production": + return "PRODUCTION" + if env == "staging": + return "STAGING" + return env.upper() or "UNKNOWN" + + +def build_record_url(resource_type: str, resource_id: int | str) -> str | None: + base = (os.environ.get("OCOTILLO_UI_BASE_URL") or "").strip().rstrip("/") + if not base: + return None + + path_template = RESOURCE_UI_PATHS.get(resource_type) + if not path_template: + return None + + return f"{base}/{path_template.format(resource_id=resource_id)}" + + +def format_field_changes( + field_changes: dict[str, dict[str, Any]] | None, +) -> str: + if not field_changes: + return "" + + lines: list[str] = [] + for field, change in field_changes.items(): + before = _format_display_value(change.get("before")) + after = _format_display_value(change.get("after")) + lines.append(f"{field}: {before} → {after}") + return "\n".join(lines) + + +def build_slack_payload( + event: EditEvent, + user: dict[str, Any], + environment: str | None = None, +) -> dict[str, Any]: + env_label = environment_label(environment) + heading_action = ACTION_HEADINGS.get(event.action, event.action) + header = f"[{env_label}] {heading_action} — {event.resource_label}" + + actor_name = user.get("name") or user.get("preferred_username") or "Unknown" + actor_email = user.get("email") + who = actor_name if not actor_email else f"{actor_name} ({actor_email})" + when = datetime.now(timezone.utc).strftime("%Y-%m-%d %H:%M UTC") + + fields: list[dict[str, str]] = [ + {"type": "mrkdwn", "text": f"*Who:*\n{who}"}, + {"type": "mrkdwn", "text": f"*When:*\n{when}"}, + {"type": "mrkdwn", "text": f"*What:*\n{event.summary}"}, + ] + + diff_text = format_field_changes(event.field_changes) + if diff_text: + fields.append({"type": "mrkdwn", "text": f"*Changes:*\n{diff_text}"}) + + header_block = { + "type": "header", + "text": {"type": "plain_text", "text": header[:150]}, + } + blocks: list[dict[str, Any]] = [ + header_block, + {"type": "section", "fields": fields[:10]}, + ] + + record_url = build_record_url(event.resource_type, event.resource_id) + if record_url: + blocks.append( + { + "type": "section", + "text": { + "type": "mrkdwn", + "text": f"<{record_url}|View in Ocotillo →>", + }, + } + ) + + return {"text": header, "blocks": blocks} + + +def notify_edit_event(user: Any, event: EditEvent) -> None: + if not isinstance(user, dict): + return + + webhook = os.environ.get("SLACK_EDITS_WEBHOOK_URL") + if not webhook: + return + + if event.resource_type not in NOTIFY_RESOURCE_TYPES: + return + + payload = build_slack_payload(event, user) + _post_slack_async(webhook, payload) + + +def _post_slack_async(webhook_url: str, payload: dict[str, Any]) -> None: + def _send() -> None: + try: + httpx.post(webhook_url, json=payload, timeout=10.0) + except Exception: + logger.warning( + "Slack edit notification failed", + exc_info=True, + ) + + threading.Thread(target=_send, daemon=True).start() + + +def _format_display_value(value: Any) -> str: + if value is None: + return "N/A" + if isinstance(value, str) and not value.strip(): + return "N/A" + text = str(value) + if len(text) > 200: + return f"{text[:197]}..." + return text + + +# ============= EOF ============================================= diff --git a/services/group_helper.py b/services/group_helper.py index 58de1dce..2804a371 100644 --- a/services/group_helper.py +++ b/services/group_helper.py @@ -25,9 +25,18 @@ from db.thing import Thing from schemas.group import GroupResponse from services.audit_helper import audit_add, audit_update +from services.edit_notification_helper import EditEvent, notify_edit_event from services.query_helper import order_sort_filter +def _thing_resource_type(thing: Thing) -> str: + if thing.thing_type == "water well": + return "well" + if thing.thing_type == "spring": + return "spring" + return "thing" + + def get_well_counts_by_group_id( session: Session, group_ids: list[int] ) -> dict[int, int]: @@ -85,6 +94,20 @@ def add_thing_to_group( session.add(assoc) session.commit() session.refresh(assoc) + + thing_label = thing.name or f"Thing {thing_id}" + group_name = group.name or f"Group {group_id}" + notify_edit_event( + user, + EditEvent( + action="project_added", + resource_type=_thing_resource_type(thing), + resource_id=thing_id, + resource_label=thing_label, + summary=f'Added {thing_label} to project "{group_name}"', + metadata={"group_id": group_id, "group_name": group_name}, + ), + ) return assoc @@ -94,6 +117,9 @@ def remove_thing_from_group( thing_id: int, user: dict, ) -> None: + group = session.get(Group, group_id) + thing = session.get(Thing, thing_id) + assoc = session.execute( select(GroupThingAssociation).where( GroupThingAssociation.group_id == group_id, @@ -114,6 +140,21 @@ def remove_thing_from_group( session.delete(assoc) session.commit() + if thing is not None: + thing_label = thing.name or f"Thing {thing_id}" + group_name = (group.name if group else None) or f"Group {group_id}" + notify_edit_event( + user, + EditEvent( + action="project_removed", + resource_type=_thing_resource_type(thing), + resource_id=thing_id, + resource_label=thing_label, + summary=f'Removed {thing_label} from project "{group_name}"', + metadata={"group_id": group_id, "group_name": group_name}, + ), + ) + def paginated_groups_getter( session: Session, diff --git a/tests/test_asset.py b/tests/test_asset.py index d7ee0289..533cd560 100644 --- a/tests/test_asset.py +++ b/tests/test_asset.py @@ -452,6 +452,76 @@ def test_upload_and_record_asset_duplicate_returns_existing(water_well_thing): cleanup_post_test(Asset, first_id) +def test_upload_and_record_asset_notifies_slack(water_well_thing, monkeypatch): + calls: list[tuple[str, dict]] = [] + + def _capture(webhook_url: str, payload: dict) -> None: + calls.append((webhook_url, payload)) + + monkeypatch.setenv("SLACK_EDITS_WEBHOOK_URL", "https://hooks.slack.test/edit") + monkeypatch.setattr( + "services.edit_notification_helper._post_slack_async", + _capture, + ) + + path = "tests/data/riochama.png" + with open(path, "rb") as f: + response = client.post( + "/asset/upload-and-record", + data={"thing_id": water_well_thing.id, "label": "Slack test photo"}, + files={"file": ("slack-test.png", f, "image/png")}, + ) + + assert response.status_code == 201 + assert len(calls) == 1 + payload = calls[0][1] + assert water_well_thing.name in payload["text"] + what_field = next( + field + for field in payload["blocks"][1]["fields"] + if field["text"].startswith("*What:*") + ) + assert "slack-test.png" in what_field["text"] + + cleanup_post_test(Asset, response.json()["id"]) + + +def test_upload_and_record_asset_duplicate_does_not_notify_slack( + water_well_thing, monkeypatch +): + calls: list[tuple[str, dict]] = [] + + def _capture(webhook_url: str, payload: dict) -> None: + calls.append((webhook_url, payload)) + + monkeypatch.setenv("SLACK_EDITS_WEBHOOK_URL", "https://hooks.slack.test/edit") + monkeypatch.setattr( + "services.edit_notification_helper._post_slack_async", + _capture, + ) + + path = "tests/data/riochama.png" + with open(path, "rb") as f: + first = client.post( + "/asset/upload-and-record", + data={"thing_id": water_well_thing.id}, + files={"file": ("riochama.png", f, "image/png")}, + ) + assert first.status_code == 201 + assert len(calls) == 1 + + with open(path, "rb") as f: + second = client.post( + "/asset/upload-and-record", + data={"thing_id": water_well_thing.id}, + files={"file": ("riochama.png", f, "image/png")}, + ) + assert second.status_code == 201 + assert len(calls) == 1 + + cleanup_post_test(Asset, first.json()["id"]) + + def test_upload_and_record_asset_bad_thing_id(): """ Providing a thing_id that does not exist must return 409 Conflict. diff --git a/tests/test_edit_notification_helper.py b/tests/test_edit_notification_helper.py new file mode 100644 index 00000000..2a8d45b4 --- /dev/null +++ b/tests/test_edit_notification_helper.py @@ -0,0 +1,145 @@ +import pytest + +from services.edit_notification_helper import ( + EditEvent, + build_record_url, + build_slack_payload, + environment_label, + format_field_changes, + format_file_size, + notify_edit_event, +) + + +@pytest.fixture +def slack_capture(monkeypatch): + calls: list[tuple[str, dict]] = [] + + def _capture(webhook_url: str, payload: dict) -> None: + calls.append((webhook_url, payload)) + + monkeypatch.setenv("SLACK_EDITS_WEBHOOK_URL", "https://hooks.slack.test/edit") + monkeypatch.setenv("OCOTILLO_UI_BASE_URL", "https://ocotillo.example.org") + monkeypatch.setattr( + "services.edit_notification_helper._post_slack_async", + _capture, + ) + return calls + + +def test_environment_label(): + assert environment_label("staging") == "STAGING" + assert environment_label("production") == "PRODUCTION" + assert environment_label("dev") == "DEV" + + +def test_format_file_size(): + assert format_file_size(512) == "512 B" + assert format_file_size(2048) == "2.0 KB" + assert format_file_size(2 * 1024 * 1024) == "2.0 MB" + + +def test_build_record_url(monkeypatch): + monkeypatch.setenv("OCOTILLO_UI_BASE_URL", "https://ocotillo.example.org") + assert ( + build_record_url("well", 42) + == "https://ocotillo.example.org/ocotillo/well/show/42" + ) + assert build_record_url("unknown", 1) is None + + +def test_build_slack_payload_includes_environment_and_diffs(monkeypatch): + monkeypatch.setenv("OCOTILLO_UI_BASE_URL", "https://ocotillo.example.org") + event = EditEvent( + action="record_updated", + resource_type="contact", + resource_id=7, + resource_label="Jane Doe", + summary="Updated contact Jane Doe", + field_changes={ + "phone": {"before": "505-555-1234", "after": "505-555-5678"}, + }, + ) + user = {"name": "Jeremy Zilar", "email": "jeremy@example.org"} + + payload = build_slack_payload(event, user, environment="staging") + header = payload["blocks"][0]["text"]["text"] + + assert header.startswith("[STAGING] Record updated — Jane Doe") + assert payload["blocks"][1]["fields"][0]["text"].startswith("*Who:*") + assert "505-555-1234" in payload["blocks"][1]["fields"][3]["text"] + assert "View in Ocotillo" in payload["blocks"][2]["text"]["text"] + + +def test_format_field_changes_empty(): + assert format_field_changes(None) == "" + assert format_field_changes({}) == "" + + +def test_build_slack_payload_attachment_upload(): + event = EditEvent( + action="attachment_uploaded", + resource_type="well", + resource_id=28251, + resource_label="NM-28251", + summary=( + "Uploaded construction_log.pdf (application/pdf, 1.2 MB) " "to NM-28251" + ), + ) + payload = build_slack_payload( + event, + {"name": "Tyler Smith", "email": "tyler@example.org"}, + environment="production", + ) + + assert payload["blocks"][0]["text"]["text"].startswith( + "[PRODUCTION] Attachment uploaded — NM-28251" + ) + + +def test_notify_edit_event_skips_without_webhook(monkeypatch, slack_capture): + monkeypatch.delenv("SLACK_EDITS_WEBHOOK_URL", raising=False) + notify_edit_event( + {"name": "Test User"}, + EditEvent( + action="project_added", + resource_type="well", + resource_id=1, + resource_label="NM-1", + summary='Added NM-1 to project "Demo"', + ), + ) + assert slack_capture == [] + + +def test_notify_edit_event_skips_non_dict_user(slack_capture): + notify_edit_event( + True, + EditEvent( + action="project_added", + resource_type="well", + resource_id=1, + resource_label="NM-1", + summary='Added NM-1 to project "Demo"', + ), + ) + assert slack_capture == [] + + +def test_notify_edit_event_posts_payload(slack_capture): + notify_edit_event( + {"name": "Test User", "email": "test@example.org"}, + EditEvent( + action="project_removed", + resource_type="well", + resource_id=99, + resource_label="NM-99", + summary='Removed NM-99 from project "Demo"', + ), + ) + + assert len(slack_capture) == 1 + webhook, payload = slack_capture[0] + assert webhook == "https://hooks.slack.test/edit" + assert "project_removed" not in payload["text"] + assert "NM-99" in payload["text"] diff --git a/tests/test_group.py b/tests/test_group.py index 8ab05879..1c66f514 100644 --- a/tests/test_group.py +++ b/tests/test_group.py @@ -272,3 +272,59 @@ def test_remove_thing_from_group_route(group, water_well_thing): GroupThingAssociation(group_id=group.id, thing_id=water_well_thing.id) ) session.commit() + + +def test_add_thing_to_group_notifies_slack(spring_thing, monkeypatch): + calls: list[tuple[str, dict]] = [] + + def _capture(webhook_url: str, payload: dict) -> None: + calls.append((webhook_url, payload)) + + monkeypatch.setenv("SLACK_EDITS_WEBHOOK_URL", "https://hooks.slack.test/edit") + monkeypatch.setattr( + "services.edit_notification_helper._post_slack_async", + _capture, + ) + + payload = { + "release_status": "private", + "name": "Slack Association Group", + "description": "Temporary group for Slack test.", + } + create_response = client.post("/group", json=payload) + group_id = create_response.json()["id"] + + response = client.post(f"/group/{group_id}/things/{spring_thing.id}") + assert response.status_code == 201 + assoc_id = response.json()["id"] + + project_calls = [call for call in calls if "Project added" in call[1]["text"]] + assert len(project_calls) == 1 + assert spring_thing.name in project_calls[0][1]["text"] + + cleanup_post_test(GroupThingAssociation, assoc_id) + cleanup_post_test(Group, group_id) + + +def test_remove_thing_from_group_notifies_slack(group, water_well_thing, monkeypatch): + calls: list[tuple[str, dict]] = [] + + def _capture(webhook_url: str, payload: dict) -> None: + calls.append((webhook_url, payload)) + + monkeypatch.setenv("SLACK_EDITS_WEBHOOK_URL", "https://hooks.slack.test/edit") + monkeypatch.setattr( + "services.edit_notification_helper._post_slack_async", + _capture, + ) + + response = client.delete(f"/group/{group.id}/things/{water_well_thing.id}") + assert response.status_code == 204 + assert len(calls) == 1 + assert water_well_thing.name in calls[0][1]["text"] + + with session_ctx() as session: + session.add( + GroupThingAssociation(group_id=group.id, thing_id=water_well_thing.id) + ) + session.commit() diff --git a/tests/test_lazy_admin.py b/tests/test_lazy_admin.py index 5b70ed88..ac2f2244 100644 --- a/tests/test_lazy_admin.py +++ b/tests/test_lazy_admin.py @@ -1,14 +1,29 @@ import os +from collections.abc import Iterable from core.factory import create_api_app from fastapi.testclient import TestClient +def _iter_route_paths(routes: Iterable) -> Iterable[str]: + for route in routes: + path = getattr(route, "path", None) + if path: + yield path + nested = getattr(route, "routes", None) + if nested: + yield from _iter_route_paths(nested) + + +def _has_admin_route(routes: Iterable) -> bool: + return any(path.startswith("/admin") for path in _iter_route_paths(routes)) + + def test_admin_is_lazy_loaded_on_first_admin_request(): os.environ["SESSION_SECRET_KEY"] = "test-session-secret-key" app = create_api_app() - assert not any(route.path.startswith("/admin") for route in app.routes) + assert not _has_admin_route(app.routes) assert getattr(app.state, "admin_configured", False) is False with TestClient(app) as client: @@ -16,4 +31,4 @@ def test_admin_is_lazy_loaded_on_first_admin_request(): assert response.status_code in {200, 302, 307} assert app.state.admin_configured is True - assert any(route.path.startswith("/admin") for route in app.routes) + assert _has_admin_route(app.routes)