From 1376ead48a2992537a45c0a4e29add4f5119151f Mon Sep 17 00:00:00 2001 From: Ali Asadi Date: Wed, 19 Aug 2026 17:13:53 +0330 Subject: [PATCH] Revert reports app to standalone single-report generation Restore the original report_type-driven endpoint (users/tickets picked individually via serializer + orchestrator) that was replaced by the bundled zip export in b293509, on top of current master. Co-Authored-By: Claude Sonnet 5 --- apps/reports/serializers.py | 8 + apps/reports/services/data_sources.py | 63 +++--- apps/reports/services/excel.py | 10 - apps/reports/services/oauth.py | 69 +++++++ apps/reports/services/orchestrator.py | 11 ++ apps/reports/services/report_builder.py | 78 -------- apps/reports/services/report_types.py | 100 ++++++++++ apps/reports/tests.py | 243 +++++++++++++----------- apps/reports/views.py | 62 ++---- 9 files changed, 364 insertions(+), 280 deletions(-) create mode 100644 apps/reports/serializers.py create mode 100644 apps/reports/services/oauth.py create mode 100644 apps/reports/services/orchestrator.py delete mode 100644 apps/reports/services/report_builder.py create mode 100644 apps/reports/services/report_types.py diff --git a/apps/reports/serializers.py b/apps/reports/serializers.py new file mode 100644 index 0000000..69d4d6f --- /dev/null +++ b/apps/reports/serializers.py @@ -0,0 +1,8 @@ +from rest_framework import serializers + +from apps.reports.services.orchestrator import ReportOrchestrator + + +class ReportGenerateSerializer(serializers.Serializer): + report_type = serializers.ChoiceField(choices=ReportOrchestrator.available_reports()) + filters = serializers.DictField(required=False, default=dict) diff --git a/apps/reports/services/data_sources.py b/apps/reports/services/data_sources.py index 6d39ea4..293c942 100644 --- a/apps/reports/services/data_sources.py +++ b/apps/reports/services/data_sources.py @@ -1,46 +1,41 @@ -import logging +from urllib.parse import urljoin -from utils.accounts_client import get_user_detailed_info -from utils.advertising_client import get_crm_application_tickets +import requests +from django.conf import settings -logger = logging.getLogger(__name__) +from apps.reports.services.oauth import OAuth2ClientCredentials class ReportDataSources: - def fetch_application_tickets(self): - tickets = [] - offset = 0 - limit = 1000 + """ + Keep downstream calls for report generation in one place. - while True: - response = get_crm_application_tickets(limit=limit, offset=offset) - if not response or not response.results: - break + The concrete service endpoints are intentionally placeholders until the + exact internal service URLs and response shapes are known. + """ - tickets.extend(response.results) - if response.next_ is None: - break - offset += limit + def __init__(self, oauth_client=None): + self.oauth_client = oauth_client or OAuth2ClientCredentials() - return tickets + def fetch_users(self, filters): + return [] - def fetch_users_for_tickets(self, tickets): - users = [] - seen = set() + def fetch_user_metrics(self, user_ids, filters): + return {} - for ticket in tickets: - user_id = str(ticket.user) - if user_id in seen: - continue - seen.add(user_id) + def fetch_tickets(self, filters): + return [] - try: - user = get_user_detailed_info(user_id) - except Exception: - logger.exception("Failed to fetch user %s", user_id) - continue + def fetch_ticket_metrics(self, ticket_ids, filters): + return {} - if user is not None: - users.append(user) - - return users + def get_json(self, base_url, path, params=None): + url = urljoin(f"{base_url.rstrip('/')}/", path.lstrip("/")) + response = requests.get( + url, + params=params, + headers=self.oauth_client.authorization_header(), + timeout=settings.SERVICE_REQUEST_TIMEOUT, + ) + response.raise_for_status() + return response.json() diff --git a/apps/reports/services/excel.py b/apps/reports/services/excel.py index dd7ea79..569efa6 100644 --- a/apps/reports/services/excel.py +++ b/apps/reports/services/excel.py @@ -41,13 +41,3 @@ def build_report_workbook(title, columns, rows): output = BytesIO() workbook.save(output) return output.getvalue() - - -def build_report_zip(files: dict[str, bytes]) -> bytes: - from zipfile import ZipFile, ZIP_DEFLATED - - output = BytesIO() - with ZipFile(output, "w", ZIP_DEFLATED) as zip_file: - for filename, content in files.items(): - zip_file.writestr(filename, content) - return output.getvalue() diff --git a/apps/reports/services/oauth.py b/apps/reports/services/oauth.py new file mode 100644 index 0000000..9371157 --- /dev/null +++ b/apps/reports/services/oauth.py @@ -0,0 +1,69 @@ +from dataclasses import dataclass +from time import monotonic +from urllib.parse import urljoin + +import requests +from django.conf import settings + + +class OAuth2ConfigurationError(RuntimeError): + pass + + +@dataclass +class OAuth2Token: + access_token: str + expires_at: float + token_type: str = "Bearer" + + def is_valid(self) -> bool: + return bool(self.access_token) and monotonic() < self.expires_at + + +class OAuth2ClientCredentials: + def __init__(self): + self._token = None + + @property + def token_url(self): + provider_private_url = settings.OAUTH2_PROVIDER_PRIVATE_URL.rstrip("/") + if not provider_private_url: + raise OAuth2ConfigurationError("OAUTH2_PROVIDER_PRIVATE_URL is required.") + return urljoin(f"{provider_private_url}/", "oauth2/token") + + def get_access_token(self): + if self._token and self._token.is_valid(): + return self._token.access_token + + if not settings.OAUTH2_CLIENT_ID or not settings.OAUTH2_CLIENT_SECRET: + raise OAuth2ConfigurationError( + "OAUTH2_CLIENT_ID and OAUTH2_CLIENT_SECRET are required." + ) + + response = requests.post( + self.token_url, + data={ + "grant_type": "client_credentials", + "scope": settings.OAUTH2_SCOPES, + }, + auth=(settings.OAUTH2_CLIENT_ID, settings.OAUTH2_CLIENT_SECRET), + timeout=settings.SERVICE_REQUEST_TIMEOUT, + ) + response.raise_for_status() + payload = response.json() + + access_token = payload.get("access_token") + if not access_token: + raise OAuth2ConfigurationError("OAuth2 response did not include access_token.") + + expires_in = int(payload.get("expires_in", 3600)) + token_type = payload.get("token_type", "Bearer") + self._token = OAuth2Token( + access_token=access_token, + expires_at=monotonic() + max(expires_in - 60, 1), + token_type=token_type, + ) + return self._token.access_token + + def authorization_header(self): + return {"Authorization": f"Bearer {self.get_access_token()}"} diff --git a/apps/reports/services/orchestrator.py b/apps/reports/services/orchestrator.py new file mode 100644 index 0000000..5aadd4c --- /dev/null +++ b/apps/reports/services/orchestrator.py @@ -0,0 +1,11 @@ +from apps.reports.services.report_types import REPORT_TYPES + + +class ReportOrchestrator: + @classmethod + def available_reports(cls): + return [(report_type, report_type) for report_type in REPORT_TYPES.keys()] + + def generate(self, payload): + report = REPORT_TYPES[payload["report_type"]]() + return report.generate(payload.get("filters") or {}) diff --git a/apps/reports/services/report_builder.py b/apps/reports/services/report_builder.py deleted file mode 100644 index 5bcc113..0000000 --- a/apps/reports/services/report_builder.py +++ /dev/null @@ -1,78 +0,0 @@ -TICKET_STATE_LABELS = { - 1: "INIT", - 10: "CLOSED", -} - -USERS_COLUMNS = [ - {"key": "user_uuid", "label": "accounts_user_uuid"}, - {"key": "username", "label": "accounts_username"}, - {"key": "first_name", "label": "accounts_first_name"}, - {"key": "last_name", "label": "accounts_last_name"}, - {"key": "email", "label": "accounts_email"}, - {"key": "phone_number", "label": "accounts_phone_number"}, - {"key": "address", "label": "accounts_address"}, -] - -TICKETS_COLUMNS = [ - {"key": "ticket_uuid", "label": "advertising_ticket_uuid"}, - {"key": "user_uuid", "label": "advertising_user_uuid"}, - {"key": "title", "label": "advertising_title"}, - {"key": "description", "label": "advertising_description"}, - {"key": "state", "label": "advertising_state"}, - {"key": "created_at", "label": "advertising_created_at"}, - {"key": "updated_at", "label": "advertising_updated_at"}, -] - - -def _format_value(value): - if value is None: - return "" - if type(value).__name__ == "Unset": - return "" - if hasattr(value, "value") and not isinstance(value, (str, bytes)): - value = value.value - if value is None: - return "" - return value - - -def _format_ticket_state(state): - if not state: - return "" - return TICKET_STATE_LABELS.get(state.value, str(state.value)) - - -def build_user_rows(users): - rows = [] - for user in users: - if user is None: - continue - rows.append( - { - "user_uuid": str(user.uuid), - "username": _format_value(user.username), - "first_name": _format_value(user.first_name), - "last_name": _format_value(user.last_name), - "email": _format_value(user.email), - "phone_number": _format_value(user.phone_number), - "address": _format_value(user.address), - } - ) - return rows - - -def build_ticket_rows(tickets): - rows = [] - for ticket in tickets: - rows.append( - { - "ticket_uuid": str(ticket.uuid), - "user_uuid": str(ticket.user), - "title": _format_value(ticket.title), - "description": _format_value(ticket.description), - "state": _format_ticket_state(ticket.state), - "created_at": str(ticket.created_at) if ticket.created_at else "", - "updated_at": str(ticket.updated_at) if ticket.updated_at else "", - } - ) - return rows diff --git a/apps/reports/services/report_types.py b/apps/reports/services/report_types.py new file mode 100644 index 0000000..451294d --- /dev/null +++ b/apps/reports/services/report_types.py @@ -0,0 +1,100 @@ +from apps.reports.services.data_sources import ReportDataSources + + +class BaseReport: + report_type = None + title = None + filename = None + columns = [] + + def __init__(self, data_sources=None): + self.data_sources = data_sources or ReportDataSources() + + def generate(self, filters): + return { + "title": self.title, + "filename": self.filename, + "columns": self.columns, + "rows": self.build_rows(filters), + } + + def build_rows(self, filters): + raise NotImplementedError + + +class UserReport(BaseReport): + report_type = "user" + title = "User Report" + filename = "user-report.xlsx" + columns = [ + {"key": "user_id", "label": "User ID"}, + {"key": "full_name", "label": "Full Name"}, + {"key": "email", "label": "Email"}, + {"key": "status", "label": "Status"}, + {"key": "ticket_count", "label": "Ticket Count"}, + ] + + def build_rows(self, filters): + users = self.data_sources.fetch_users(filters) + user_ids = [user.get("id") for user in users if user.get("id")] + metrics_by_user_id = self.data_sources.fetch_user_metrics(user_ids, filters) + + rows = [] + for user in users: + user_id = user.get("id") + metrics = metrics_by_user_id.get(user_id, {}) + rows.append( + { + "user_id": user_id, + "full_name": user.get("full_name") or user.get("name") or "", + "email": user.get("email", ""), + "status": user.get("status", ""), + "ticket_count": metrics.get("ticket_count", 0), + } + ) + return rows + + +class TicketsReport(BaseReport): + report_type = "tickets" + title = "Tickets Report" + filename = "tickets-report.xlsx" + columns = [ + {"key": "ticket_id", "label": "Ticket ID"}, + {"key": "title", "label": "Title"}, + {"key": "status", "label": "Status"}, + {"key": "priority", "label": "Priority"}, + {"key": "assignee", "label": "Assignee"}, + {"key": "created_at", "label": "Created At"}, + ] + + def build_rows(self, filters): + tickets = self.data_sources.fetch_tickets(filters) + ticket_ids = [ticket.get("id") for ticket in tickets if ticket.get("id")] + metrics_by_ticket_id = self.data_sources.fetch_ticket_metrics(ticket_ids, filters) + + rows = [] + for ticket in tickets: + ticket_id = ticket.get("id") + metrics = metrics_by_ticket_id.get(ticket_id, {}) + rows.append( + { + "ticket_id": ticket_id, + "title": ticket.get("title", ""), + "status": ticket.get("status", ""), + "priority": ticket.get("priority", ""), + "assignee": ( + ticket.get("assignee_name") + or ticket.get("assignee", {}).get("name", "") + ), + "created_at": ticket.get("created_at", ""), + **metrics, + } + ) + return rows + + +REPORT_TYPES = { + UserReport.report_type: UserReport, + TicketsReport.report_type: TicketsReport, +} diff --git a/apps/reports/tests.py b/apps/reports/tests.py index 1411dbd..8c8b5dd 100644 --- a/apps/reports/tests.py +++ b/apps/reports/tests.py @@ -1,138 +1,151 @@ -from datetime import datetime from io import BytesIO -from unittest.mock import MagicMock, patch -from uuid import uuid4 -from zipfile import ZipFile +from unittest.mock import patch +from django.test import override_settings from openpyxl import load_workbook from rest_framework import status from rest_framework.test import APITestCase -from utils.clients.gooyal_advertising_client.models.ticket_state_enum import TicketStateEnum +from apps.reports.services.oauth import OAuth2ClientCredentials class GenerateReportTests(APITestCase): - def _build_ticket(self, user_uuid=None): - ticket = MagicMock() - ticket.uuid = uuid4() - ticket.user = user_uuid or uuid4() - ticket.title = "Login issue" - ticket.description = "Cannot login" - ticket.state = TicketStateEnum(10) - ticket.created_at = datetime(2026, 5, 20, 10, 0) - ticket.updated_at = datetime(2026, 5, 21, 10, 0) - return ticket - - def _build_user(self, user_uuid): - user = MagicMock() - user.uuid = user_uuid - user.username = "alice" - user.first_name = "Alice" - user.last_name = "Doe" - user.email = "alice@example.com" - user.phone_number = "09120000000" - user.address = "Tehran" - return user - - @patch("apps.reports.services.data_sources.ReportDataSources.fetch_users_for_tickets") - @patch("apps.reports.services.data_sources.ReportDataSources.fetch_application_tickets") - def test_generate_report_returns_zip_with_two_excel_files( + @patch("apps.reports.services.data_sources.ReportDataSources.fetch_user_metrics") + @patch("apps.reports.services.data_sources.ReportDataSources.fetch_users") + def test_generate_user_report_is_public_and_returns_excel( self, - mock_fetch_tickets, mock_fetch_users, + mock_fetch_user_metrics, ): - ticket = self._build_ticket() - user = self._build_user(ticket.user) - mock_fetch_tickets.return_value = [ticket] - mock_fetch_users.return_value = [user] + mock_fetch_users.return_value = [ + { + "id": 1, + "full_name": "Alice Doe", + "email": "alice@example.com", + "status": "active", + } + ] + mock_fetch_user_metrics.return_value = { + 1: {"ticket_count": 3}, + } - response = self.client.post("/api/reports/generate/") - - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response["Content-Type"], "application/zip") - self.assertEqual( - response["Content-Disposition"], - 'attachment; filename="crm-reports.zip"', + response = self.client.post( + "/api/reports/generate/", + { + "report_type": "user", + "filters": {"status": "active"}, + }, + format="json", ) - with ZipFile(BytesIO(response.content)) as archive: - self.assertEqual( - set(archive.namelist()), - {"users-report.xlsx", "tickets-report.xlsx"}, - ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual( + response["Content-Type"], + "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", + ) + self.assertEqual( + response["Content-Disposition"], + 'attachment; filename="user-report.xlsx"', + ) - users_workbook = load_workbook(BytesIO(archive.read("users-report.xlsx"))) - users_sheet = users_workbook.active - self.assertEqual(users_sheet["A1"].value, "Users Report") - self.assertEqual( - [cell.value for cell in users_sheet[2]], - [ - "accounts_uuid", - "accounts_username", - "accounts_first_name", - "accounts_last_name", - "accounts_email", - "accounts_phone_number", - "accounts_address", - ], - ) - self.assertEqual( - [cell.value for cell in users_sheet[3]], - [ - str(user.uuid), - "alice", - "Alice", - "Doe", - "alice@example.com", - "09120000000", - "Tehran", - ], - ) + workbook = load_workbook(BytesIO(response.content)) + worksheet = workbook.active + self.assertEqual(worksheet["A1"].value, "User Report") + self.assertEqual( + [cell.value for cell in worksheet[2]], + ["User ID", "Full Name", "Email", "Status", "Ticket Count"], + ) + self.assertEqual( + [cell.value for cell in worksheet[3]], + [1, "Alice Doe", "alice@example.com", "active", 3], + ) + mock_fetch_users.assert_called_once_with({"status": "active"}) - tickets_workbook = load_workbook( - BytesIO(archive.read("tickets-report.xlsx")) - ) - tickets_sheet = tickets_workbook.active - self.assertEqual(tickets_sheet["A1"].value, "Tickets Report") - self.assertEqual( - [cell.value for cell in tickets_sheet[2]], - [ - "advertising_uuid", - "advertising_user", - "advertising_title", - "advertising_description", - "advertising_state", - "advertising_created_at", - "advertising_updated_at", - ], - ) - self.assertEqual( - [cell.value for cell in tickets_sheet[3]], - [ - str(ticket.uuid), - str(ticket.user), - "Login issue", - "Cannot login", - "CLOSED", - "2026-05-20 10:00:00", - "2026-05-21 10:00:00", - ], - ) - - @patch("apps.reports.services.data_sources.ReportDataSources.fetch_users_for_tickets") - @patch("apps.reports.services.data_sources.ReportDataSources.fetch_application_tickets") - def test_generate_report_skips_missing_users( + @patch("apps.reports.services.data_sources.ReportDataSources.fetch_ticket_metrics") + @patch("apps.reports.services.data_sources.ReportDataSources.fetch_tickets") + def test_generate_tickets_report_uses_static_columns( self, mock_fetch_tickets, - mock_fetch_users, + mock_fetch_ticket_metrics, ): - ticket = self._build_ticket() - mock_fetch_tickets.return_value = [ticket] - mock_fetch_users.return_value = [None] + mock_fetch_tickets.return_value = [ + { + "id": 11, + "title": "Login issue", + "status": "open", + "priority": "high", + "assignee": {"name": "Support Agent"}, + "created_at": "2026-05-20T10:00:00Z", + } + ] + mock_fetch_ticket_metrics.return_value = {} - response = self.client.post("/api/reports/generate/") + response = self.client.post( + "/api/reports/generate/", + {"report_type": "tickets"}, + format="json", + ) self.assertEqual(response.status_code, status.HTTP_200_OK) - with ZipFile(BytesIO(response.content)) as archive: - users_workbook = load_workbook(BytesIO(archive.read("users-report.xlsx"))) - self.assertIsNone(users_workbook.active["A3"].value) + self.assertEqual( + response["Content-Disposition"], + 'attachment; filename="tickets-report.xlsx"', + ) + + workbook = load_workbook(BytesIO(response.content)) + worksheet = workbook.active + self.assertEqual(worksheet["A1"].value, "Tickets Report") + self.assertEqual( + [cell.value for cell in worksheet[2]], + ["Ticket ID", "Title", "Status", "Priority", "Assignee", "Created At"], + ) + self.assertEqual( + [cell.value for cell in worksheet[3]], + [ + 11, + "Login issue", + "open", + "high", + "Support Agent", + "2026-05-20T10:00:00Z", + ], + ) + + def test_unknown_report_type_returns_validation_error(self): + response = self.client.post( + "/api/reports/generate/", + {"report_type": "unknown"}, + format="json", + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + +class OAuth2ClientCredentialsTests(APITestCase): + @override_settings( + OAUTH2_PROVIDER_PRIVATE_URL="https://auth.internal", + OAUTH2_CLIENT_ID="client-id", + OAUTH2_CLIENT_SECRET="client-secret", + OAUTH2_SCOPES="reports:read customers:read", + SERVICE_REQUEST_TIMEOUT=10, + ) + @patch("apps.reports.services.oauth.requests.post") + def test_get_access_token_uses_client_credentials(self, mock_post): + mock_post.return_value.json.return_value = { + "access_token": "token-value", + "expires_in": 3600, + } + mock_post.return_value.raise_for_status.return_value = None + + token = OAuth2ClientCredentials().get_access_token() + + self.assertEqual(token, "token-value") + mock_post.assert_called_once_with( + "https://auth.internal/oauth2/token", + data={ + "grant_type": "client_credentials", + "scope": "reports:read customers:read", + }, + auth=("client-id", "client-secret"), + timeout=10, + ) diff --git a/apps/reports/views.py b/apps/reports/views.py index 36257fb..9fc640f 100644 --- a/apps/reports/views.py +++ b/apps/reports/views.py @@ -1,60 +1,36 @@ from django.http import HttpResponse -from drf_spectacular.types import OpenApiTypes -from drf_spectacular.utils import OpenApiResponse, extend_schema from rest_framework import status from rest_framework.views import APIView -from apps.reports.services.data_sources import ReportDataSources -from apps.reports.services.excel import build_report_workbook, build_report_zip -from apps.reports.services.report_builder import ( - TICKETS_COLUMNS, - USERS_COLUMNS, - build_ticket_rows, - build_user_rows, -) +from apps.reports.serializers import ReportGenerateSerializer +from apps.reports.services.excel import build_report_workbook +from apps.reports.services.orchestrator import ReportOrchestrator class GenerateReportView(APIView): authentication_classes = [] permission_classes = [] - @extend_schema( - tags=["Reports"], - summary="Generate CRM reports", - description=( - "Fetches application tickets and related user profiles, then returns a " - "ZIP archive containing `users-report.xlsx` and `tickets-report.xlsx`." - ), - request=None, - responses={ - (200, "application/zip"): OpenApiResponse( - response=OpenApiTypes.BINARY, - description="ZIP archive with users and tickets Excel reports.", - ), - }, - ) def post(self, request): - data_sources = ReportDataSources() - tickets = data_sources.fetch_application_tickets() - users = data_sources.fetch_users_for_tickets(tickets) + serializer = ReportGenerateSerializer(data=request.data) + serializer.is_valid(raise_exception=True) - files = { - "users-report.xlsx": build_report_workbook( - title="Users Report", - columns=USERS_COLUMNS, - rows=build_user_rows(users), - ), - "tickets-report.xlsx": build_report_workbook( - title="Tickets Report", - columns=TICKETS_COLUMNS, - rows=build_ticket_rows(tickets), - ), - } + orchestrator = ReportOrchestrator() + report_data = orchestrator.generate(serializer.validated_data) + workbook = build_report_workbook( + title=report_data["title"], + columns=report_data["columns"], + rows=report_data["rows"], + ) response = HttpResponse( - build_report_zip(files), - content_type="application/zip", + workbook, + content_type=( + "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" + ), status=status.HTTP_200_OK, ) - response["Content-Disposition"] = 'attachment; filename="crm-reports.zip"' + response["Content-Disposition"] = ( + f'attachment; filename="{report_data["filename"]}"' + ) return response -- 2.45.3