Compare commits

..

2 commits

Author SHA1 Message Date
6aa925992f Fetch all accounts users for the user report, not just ticket owners
fetch_users previously only surfaced users who appeared as ticket
owners. Add list_users(limit, offset) to hit the accounts user-list
endpoint directly with raw limit/offset params -- the generated OpenAPI
wrapper only supports page, which this endpoint silently ignores.
fetch_users now paginates that list and hydrates each entry with
get_user_detailed_info to keep the existing email/phone/address columns.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-19 17:29:21 +03:30
4946ec8c8f Wire real Accounts/Advertising data into the standalone report endpoint
fetch_users/fetch_tickets were placeholder stubs returning empty
lists, so the generated Excel was always empty. Wire them to the
existing utils.accounts_client / utils.advertising_client integrations
(same ones the bundled-zip report on master uses), and update
UserReport/TicketsReport to the real Account/Ticket field names
instead of the old mock dict shape. Drop the now-unused local OAuth
helper in favor of each client's own client-credentials flow.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-19 17:25:59 +03:30
7 changed files with 245 additions and 189 deletions

View file

@ -54,7 +54,7 @@ def merge_upstream_schemas(result, generator, request, public):
"""drf-spectacular POSTPROCESSING_HOOKS entry point. """drf-spectacular POSTPROCESSING_HOOKS entry point.
Folds every proxied service's own OpenAPI document into this service's Folds every proxied service's own OpenAPI document into this service's
schema under `/api/<service>/...` paths (mirroring apps.proxy.views), so the schema under `/<service>/...` paths (mirroring apps.proxy.views), so the
combined swagger covers every endpoint this proxy forwards to, DTOs combined swagger covers every endpoint this proxy forwards to, DTOs
included. Schemas are cached per service; a service that's unconfigured included. Schemas are cached per service; a service that's unconfigured
or unreachable is skipped rather than failing the whole document. or unreachable is skipped rather than failing the whole document.
@ -109,6 +109,6 @@ def merge_upstream_schemas(result, generator, request, public):
operation["tags"] = [label] operation["tags"] = [label]
if "operationId" in operation: if "operationId" in operation:
operation["operationId"] = f"{service}_{operation['operationId']}" operation["operationId"] = f"{service}_{operation['operationId']}"
result["paths"][f"/api/{service}{path}"] = proxied_operations result["paths"][f"/{service}{path}"] = proxied_operations
return result return result

View file

@ -1,41 +1,75 @@
from urllib.parse import urljoin import logging
import requests from utils.accounts_client import get_user_detailed_info, list_users
from django.conf import settings from utils.advertising_client import get_crm_application_tickets
from apps.reports.services.oauth import OAuth2ClientCredentials logger = logging.getLogger(__name__)
class ReportDataSources: class ReportDataSources:
""" def __init__(self):
Keep downstream calls for report generation in one place. self._tickets_cache = None
The concrete service endpoints are intentionally placeholders until the
exact internal service URLs and response shapes are known.
"""
def __init__(self, oauth_client=None):
self.oauth_client = oauth_client or OAuth2ClientCredentials()
def fetch_users(self, filters):
return []
def fetch_user_metrics(self, user_ids, filters):
return {}
def fetch_tickets(self, filters): def fetch_tickets(self, filters):
return [] if self._tickets_cache is not None:
return self._tickets_cache
tickets = []
offset = 0
limit = 1000
while True:
response = get_crm_application_tickets(limit=limit, offset=offset)
if not response or not response.results:
break
tickets.extend(response.results)
if response.next_ is None:
break
offset += limit
self._tickets_cache = tickets
return tickets
def fetch_users(self, filters):
users = []
limit = 100
offset = 0
while True:
page = list_users(limit=limit, offset=offset)
results = page.get("results") or []
if not results:
break
for entry in results:
user_id = entry.get("uuid")
if not user_id:
continue
try:
user = get_user_detailed_info(user_id)
except Exception:
logger.exception("Failed to fetch user %s", user_id)
continue
if user is not None:
users.append(user)
if not page.get("next"):
break
offset += limit
return users
def fetch_user_metrics(self, user_ids, filters):
tickets = self.fetch_tickets(filters)
counts = {}
for ticket in tickets:
user_id = str(ticket.user)
counts[user_id] = counts.get(user_id, 0) + 1
return {user_id: {"ticket_count": count} for user_id, count in counts.items()}
def fetch_ticket_metrics(self, ticket_ids, filters): def fetch_ticket_metrics(self, ticket_ids, filters):
return {} return {}
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()

View file

@ -1,69 +0,0 @@
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()}"}

View file

@ -1,5 +1,28 @@
from apps.reports.services.data_sources import ReportDataSources from apps.reports.services.data_sources import ReportDataSources
TICKET_STATE_LABELS = {
1: "INIT",
10: "CLOSED",
}
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))
class BaseReport: class BaseReport:
report_type = None report_type = None
@ -27,28 +50,32 @@ class UserReport(BaseReport):
title = "User Report" title = "User Report"
filename = "user-report.xlsx" filename = "user-report.xlsx"
columns = [ columns = [
{"key": "user_id", "label": "User ID"}, {"key": "user_uuid", "label": "User UUID"},
{"key": "full_name", "label": "Full Name"}, {"key": "username", "label": "Username"},
{"key": "first_name", "label": "First Name"},
{"key": "last_name", "label": "Last Name"},
{"key": "email", "label": "Email"}, {"key": "email", "label": "Email"},
{"key": "status", "label": "Status"}, {"key": "phone_number", "label": "Phone Number"},
{"key": "ticket_count", "label": "Ticket Count"}, {"key": "ticket_count", "label": "Ticket Count"},
] ]
def build_rows(self, filters): def build_rows(self, filters):
users = self.data_sources.fetch_users(filters) users = self.data_sources.fetch_users(filters)
user_ids = [user.get("id") for user in users if user.get("id")] user_ids = [str(user.uuid) for user in users]
metrics_by_user_id = self.data_sources.fetch_user_metrics(user_ids, filters) metrics_by_user_id = self.data_sources.fetch_user_metrics(user_ids, filters)
rows = [] rows = []
for user in users: for user in users:
user_id = user.get("id") user_id = str(user.uuid)
metrics = metrics_by_user_id.get(user_id, {}) metrics = metrics_by_user_id.get(user_id, {})
rows.append( rows.append(
{ {
"user_id": user_id, "user_uuid": user_id,
"full_name": user.get("full_name") or user.get("name") or "", "username": _format_value(user.username),
"email": user.get("email", ""), "first_name": _format_value(user.first_name),
"status": user.get("status", ""), "last_name": _format_value(user.last_name),
"email": _format_value(user.email),
"phone_number": _format_value(user.phone_number),
"ticket_count": metrics.get("ticket_count", 0), "ticket_count": metrics.get("ticket_count", 0),
} }
) )
@ -60,34 +87,33 @@ class TicketsReport(BaseReport):
title = "Tickets Report" title = "Tickets Report"
filename = "tickets-report.xlsx" filename = "tickets-report.xlsx"
columns = [ columns = [
{"key": "ticket_id", "label": "Ticket ID"}, {"key": "ticket_uuid", "label": "Ticket UUID"},
{"key": "user_uuid", "label": "User UUID"},
{"key": "title", "label": "Title"}, {"key": "title", "label": "Title"},
{"key": "status", "label": "Status"}, {"key": "description", "label": "Description"},
{"key": "priority", "label": "Priority"}, {"key": "state", "label": "State"},
{"key": "assignee", "label": "Assignee"},
{"key": "created_at", "label": "Created At"}, {"key": "created_at", "label": "Created At"},
{"key": "updated_at", "label": "Updated At"},
] ]
def build_rows(self, filters): def build_rows(self, filters):
tickets = self.data_sources.fetch_tickets(filters) tickets = self.data_sources.fetch_tickets(filters)
ticket_ids = [ticket.get("id") for ticket in tickets if ticket.get("id")] ticket_ids = [str(ticket.uuid) for ticket in tickets]
metrics_by_ticket_id = self.data_sources.fetch_ticket_metrics(ticket_ids, filters) metrics_by_ticket_id = self.data_sources.fetch_ticket_metrics(ticket_ids, filters)
rows = [] rows = []
for ticket in tickets: for ticket in tickets:
ticket_id = ticket.get("id") ticket_id = str(ticket.uuid)
metrics = metrics_by_ticket_id.get(ticket_id, {}) metrics = metrics_by_ticket_id.get(ticket_id, {})
rows.append( rows.append(
{ {
"ticket_id": ticket_id, "ticket_uuid": ticket_id,
"title": ticket.get("title", ""), "user_uuid": str(ticket.user),
"status": ticket.get("status", ""), "title": _format_value(ticket.title),
"priority": ticket.get("priority", ""), "description": _format_value(ticket.description),
"assignee": ( "state": _format_ticket_state(ticket.state),
ticket.get("assignee_name") "created_at": str(ticket.created_at) if ticket.created_at else "",
or ticket.get("assignee", {}).get("name", "") "updated_at": str(ticket.updated_at) if ticket.updated_at else "",
),
"created_at": ticket.get("created_at", ""),
**metrics, **metrics,
} }
) )

View file

@ -1,15 +1,78 @@
from datetime import datetime
from io import BytesIO from io import BytesIO
from unittest.mock import patch from unittest.mock import MagicMock, patch
from uuid import uuid4
from django.test import override_settings from django.test import TestCase
from openpyxl import load_workbook from openpyxl import load_workbook
from rest_framework import status from rest_framework import status
from rest_framework.test import APITestCase from rest_framework.test import APITestCase
from apps.reports.services.oauth import OAuth2ClientCredentials from apps.reports.services.data_sources import ReportDataSources
from utils.clients.gooyal_advertising_client.models.ticket_state_enum import TicketStateEnum
class ReportDataSourcesFetchUsersTests(TestCase):
@patch("apps.reports.services.data_sources.get_user_detailed_info")
@patch("apps.reports.services.data_sources.list_users")
def test_fetch_users_paginates_full_user_list(self, mock_list_users, mock_get_user):
first_uuid, second_uuid = str(uuid4()), str(uuid4())
mock_list_users.side_effect = [
{"results": [{"uuid": first_uuid}], "next": "http://accounts/users/api/users/?limit=1&offset=1"},
{"results": [{"uuid": second_uuid}], "next": None},
]
mock_get_user.side_effect = lambda uuid: MagicMock(uuid=uuid)
users = ReportDataSources().fetch_users({})
self.assertEqual([user.uuid for user in users], [first_uuid, second_uuid])
mock_list_users.assert_any_call(limit=100, offset=0)
mock_list_users.assert_any_call(limit=100, offset=100)
mock_get_user.assert_any_call(first_uuid)
mock_get_user.assert_any_call(second_uuid)
@patch("apps.reports.services.data_sources.get_user_detailed_info")
@patch("apps.reports.services.data_sources.list_users")
def test_fetch_users_skips_user_whose_details_call_fails(self, mock_list_users, mock_get_user):
ok_uuid, broken_uuid = str(uuid4()), str(uuid4())
mock_list_users.return_value = {
"results": [{"uuid": broken_uuid}, {"uuid": ok_uuid}],
"next": None,
}
def get_user(uuid):
if uuid == broken_uuid:
raise Exception("boom")
return MagicMock(uuid=uuid)
mock_get_user.side_effect = get_user
users = ReportDataSources().fetch_users({})
self.assertEqual([user.uuid for user in users], [ok_uuid])
class GenerateReportTests(APITestCase): 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"
return user
@patch("apps.reports.services.data_sources.ReportDataSources.fetch_user_metrics") @patch("apps.reports.services.data_sources.ReportDataSources.fetch_user_metrics")
@patch("apps.reports.services.data_sources.ReportDataSources.fetch_users") @patch("apps.reports.services.data_sources.ReportDataSources.fetch_users")
def test_generate_user_report_is_public_and_returns_excel( def test_generate_user_report_is_public_and_returns_excel(
@ -17,17 +80,9 @@ class GenerateReportTests(APITestCase):
mock_fetch_users, mock_fetch_users,
mock_fetch_user_metrics, mock_fetch_user_metrics,
): ):
mock_fetch_users.return_value = [ user = self._build_user(uuid4())
{ mock_fetch_users.return_value = [user]
"id": 1, mock_fetch_user_metrics.return_value = {str(user.uuid): {"ticket_count": 3}}
"full_name": "Alice Doe",
"email": "alice@example.com",
"status": "active",
}
]
mock_fetch_user_metrics.return_value = {
1: {"ticket_count": 3},
}
response = self.client.post( response = self.client.post(
"/api/reports/generate/", "/api/reports/generate/",
@ -53,11 +108,27 @@ class GenerateReportTests(APITestCase):
self.assertEqual(worksheet["A1"].value, "User Report") self.assertEqual(worksheet["A1"].value, "User Report")
self.assertEqual( self.assertEqual(
[cell.value for cell in worksheet[2]], [cell.value for cell in worksheet[2]],
["User ID", "Full Name", "Email", "Status", "Ticket Count"], [
"User UUID",
"Username",
"First Name",
"Last Name",
"Email",
"Phone Number",
"Ticket Count",
],
) )
self.assertEqual( self.assertEqual(
[cell.value for cell in worksheet[3]], [cell.value for cell in worksheet[3]],
[1, "Alice Doe", "alice@example.com", "active", 3], [
str(user.uuid),
"alice",
"Alice",
"Doe",
"alice@example.com",
"09120000000",
3,
],
) )
mock_fetch_users.assert_called_once_with({"status": "active"}) mock_fetch_users.assert_called_once_with({"status": "active"})
@ -68,16 +139,8 @@ class GenerateReportTests(APITestCase):
mock_fetch_tickets, mock_fetch_tickets,
mock_fetch_ticket_metrics, mock_fetch_ticket_metrics,
): ):
mock_fetch_tickets.return_value = [ ticket = self._build_ticket()
{ mock_fetch_tickets.return_value = [ticket]
"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 = {} mock_fetch_ticket_metrics.return_value = {}
response = self.client.post( response = self.client.post(
@ -97,17 +160,26 @@ class GenerateReportTests(APITestCase):
self.assertEqual(worksheet["A1"].value, "Tickets Report") self.assertEqual(worksheet["A1"].value, "Tickets Report")
self.assertEqual( self.assertEqual(
[cell.value for cell in worksheet[2]], [cell.value for cell in worksheet[2]],
["Ticket ID", "Title", "Status", "Priority", "Assignee", "Created At"], [
"Ticket UUID",
"User UUID",
"Title",
"Description",
"State",
"Created At",
"Updated At",
],
) )
self.assertEqual( self.assertEqual(
[cell.value for cell in worksheet[3]], [cell.value for cell in worksheet[3]],
[ [
11, str(ticket.uuid),
str(ticket.user),
"Login issue", "Login issue",
"open", "Cannot login",
"high", "CLOSED",
"Support Agent", "2026-05-20 10:00:00",
"2026-05-20T10:00:00Z", "2026-05-21 10:00:00",
], ],
) )
@ -119,33 +191,3 @@ class GenerateReportTests(APITestCase):
) )
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) 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,
)

View file

@ -16,5 +16,5 @@ urlpatterns = [
path("api/reports/", include("apps.reports.urls")), path("api/reports/", include("apps.reports.urls")),
# Catch-all reverse proxy: thisprojectbaseurl/<service>/<rest of url>. # Catch-all reverse proxy: thisprojectbaseurl/<service>/<rest of url>.
# Must stay last so it never shadows the routes above. # Must stay last so it never shadows the routes above.
path("api/", include("apps.proxy.urls")), path("", include("apps.proxy.urls")),
] ]

View file

@ -74,3 +74,26 @@ def get_user_detailed_info(uuid):
return response return response
def list_users(limit=100, offset=0):
"""Fetch one page of the accounts user list.
The generated OpenAPI client (users_api_users_list) only supports
`page`, and that param is silently ignored by the upstream endpoint --
every page returns the same first results. Its real pagination is
limit/offset, so this bypasses the generated wrapper with a raw request.
"""
access_token = login_as_client_credentials()['access_token']
base_url = settings.ACCOUNTS_BASE_PUBLIC_URL
url = f"{base_url.rstrip('/')}/users/api/users/"
response = request_with_ssl_fallback(
'GET',
url,
params={"limit": limit, "offset": offset},
headers={"Authorization": f"Bearer {access_token}"},
verify=resolve_verify_ssl(base_url),
)
response.raise_for_status()
return response.json()