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>
This commit is contained in:
parent
4946ec8c8f
commit
6aa925992f
3 changed files with 88 additions and 15 deletions
|
|
@ -1,6 +1,6 @@
|
||||||
import logging
|
import logging
|
||||||
|
|
||||||
from utils.accounts_client import get_user_detailed_info
|
from utils.accounts_client import get_user_detailed_info, list_users
|
||||||
from utils.advertising_client import get_crm_application_tickets
|
from utils.advertising_client import get_crm_application_tickets
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
@ -32,15 +32,20 @@ class ReportDataSources:
|
||||||
return tickets
|
return tickets
|
||||||
|
|
||||||
def fetch_users(self, filters):
|
def fetch_users(self, filters):
|
||||||
tickets = self.fetch_tickets(filters)
|
|
||||||
users = []
|
users = []
|
||||||
seen = set()
|
limit = 100
|
||||||
|
offset = 0
|
||||||
|
|
||||||
for ticket in tickets:
|
while True:
|
||||||
user_id = str(ticket.user)
|
page = list_users(limit=limit, offset=offset)
|
||||||
if user_id in seen:
|
results = page.get("results") or []
|
||||||
|
if not results:
|
||||||
|
break
|
||||||
|
|
||||||
|
for entry in results:
|
||||||
|
user_id = entry.get("uuid")
|
||||||
|
if not user_id:
|
||||||
continue
|
continue
|
||||||
seen.add(user_id)
|
|
||||||
|
|
||||||
try:
|
try:
|
||||||
user = get_user_detailed_info(user_id)
|
user = get_user_detailed_info(user_id)
|
||||||
|
|
@ -51,6 +56,10 @@ class ReportDataSources:
|
||||||
if user is not None:
|
if user is not None:
|
||||||
users.append(user)
|
users.append(user)
|
||||||
|
|
||||||
|
if not page.get("next"):
|
||||||
|
break
|
||||||
|
offset += limit
|
||||||
|
|
||||||
return users
|
return users
|
||||||
|
|
||||||
def fetch_user_metrics(self, user_ids, filters):
|
def fetch_user_metrics(self, user_ids, filters):
|
||||||
|
|
|
||||||
|
|
@ -3,13 +3,54 @@ from io import BytesIO
|
||||||
from unittest.mock import MagicMock, patch
|
from unittest.mock import MagicMock, patch
|
||||||
from uuid import uuid4
|
from uuid import uuid4
|
||||||
|
|
||||||
|
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.data_sources import ReportDataSources
|
||||||
from utils.clients.gooyal_advertising_client.models.ticket_state_enum import TicketStateEnum
|
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):
|
def _build_ticket(self, user_uuid=None):
|
||||||
ticket = MagicMock()
|
ticket = MagicMock()
|
||||||
|
|
|
||||||
|
|
@ -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()
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue