diff --git a/addons/base/views.py b/addons/base/views.py
index 3f96d763781..be3342eb4d7 100644
--- a/addons/base/views.py
+++ b/addons/base/views.py
@@ -213,6 +213,7 @@ def _record_file_download(target, file_node, query_params, auth, version=None):
storage_provider=getattr(file_node, 'provider', '') or '',
user_guid=getattr(getattr(auth, 'user', None), '_id', None),
ip=request.remote_addr,
+ user_agent=request.headers.get('User-Agent', ''),
source_area=query_params.get('source', ''),
tz=query_params.get('tz', ''),
)
@@ -246,6 +247,7 @@ def _record_zip_download(payload):
status_code=action_meta.get('status_code'),
user_guid=(payload.get('auth') or {}).get('id'),
ip=action_meta.get('ip'),
+ user_agent=(payload.get('request_meta') or {}).get('user_agent', ''),
source_area=action_meta.get('source', ''),
tz=action_meta.get('tz', ''),
)
diff --git a/admin/templates/download_events/download_events.html b/admin/templates/download_events/download_events.html
index 0149733d2a8..10a5c46ad75 100644
--- a/admin/templates/download_events/download_events.html
+++ b/admin/templates/download_events/download_events.html
@@ -78,6 +78,10 @@
Total GB
diff --git a/osf/admin.py b/osf/admin.py
index ad63d6bd03d..ebf9790300f 100644
--- a/osf/admin.py
+++ b/osf/admin.py
@@ -6,7 +6,7 @@
from django.template.response import TemplateResponse
from django_extensions.admin import ForeignKeyAutocompleteAdmin
from django.contrib.auth.models import Group
-from django.db.models import Q, Count, Sum, F, Min, Max
+from django.db.models import Q, Count, Sum, F, Min, Max, Case, When, Value, IntegerField
from django.db.models.functions import Trunc
from django.http import HttpResponseRedirect, HttpResponse, JsonResponse
from django.utils import timezone
@@ -31,7 +31,7 @@
Notification,
DownloadEvent
)
-from osf.models import AbstractNode
+from osf.models import AbstractNode, Preprint, Guid
from osf.models.notification_type import get_default_frequency_choices
from osf.models.notable_domain import DomainReference
@@ -450,7 +450,7 @@ class DownloadEventsView(admin.ModelAdmin):
change_list_template = 'download_events/download_events.html'
list_display = (
'resource_guid',
- 'user',
+ 'user_display',
'download_type',
'outcome',
'zip_completed',
@@ -462,6 +462,7 @@ class DownloadEventsView(admin.ModelAdmin):
'storage_region',
'ip',
'source_area',
+ 'user_agent_display',
'created'
)
list_filter = (
@@ -487,11 +488,46 @@ class DownloadEventsView(admin.ModelAdmin):
'storage_provider',
'user_region',
'storage_region',
- 'source_area'
+ 'source_area',
+ 'user_agent'
)
- search_help_text = 'Search by username, full name, user or node guid, ip, path, storage provider, user or storage region, source area.'
+ search_help_text = 'Search by username, full name, user or node guid, ip, path, storage provider, user or storage region, source area, user agent.'
+
+ def get_queryset(self, request):
+ """Annotate an outcome rank so the computed Outcome column is sortable.
+
+ The rank mirrors :meth:`outcome` exactly. It's just an ordering key — it doesn't
+ change what rows are returned, so the table and the dashboard aggregates are
+ unaffected.
+ """
+ return super().get_queryset(request).annotate(
+ _outcome_rank=Case(
+ When(zip_completed=True, then=Value(0)), # Completed
+ When(
+ zip_completed=False,
+ status_code__gte=DOWNLOAD_FAILURE_MIN_STATUS,
+ then=Value(2), # Failed
+ ),
+ When(zip_completed=False, then=Value(1)), # Cancelled
+ default=Value(3), # single files have no outcome ('—')
+ output_field=IntegerField(),
+ )
+ )
+
+ @admin.display(description='User', ordering='user__username')
+ def user_display(self, obj):
+ """Sort the User column by the username (email) rather than the raw FK id."""
+ return obj.user or '—'
+
+ @admin.display(description='User agent', ordering='user_agent')
+ def user_agent_display(self, obj):
+ """Truncated in the table so it doesn't dominate the row; the full value is still
+ searchable and shows on the record's detail view."""
+ if not obj.user_agent:
+ return '—'
+ return obj.user_agent if len(obj.user_agent) <= 80 else obj.user_agent[:79] + '…'
- @admin.display(description='Outcome')
+ @admin.display(description='Outcome', ordering='_outcome_rank')
def outcome(self, obj):
"""Human-readable end state. Single files have no outcome — they're recorded at the
redirect before any bytes move, so they never report completion."""
@@ -760,7 +796,9 @@ def _build_region_breakdown(self, queryset, field_name):
breakdown[region_name]['file_count'] += row['file_count']
breakdown[region_name]['zip_count'] += row['zip_count']
- ordered = sorted(breakdown.items(), key=lambda item: item[1]['gb'], reverse=True)[:10]
+ # gb descending, then name ascending so the order is deterministic when GB ties
+ # (and never depends on the incoming queryset's row order)
+ ordered = sorted(breakdown.items(), key=lambda item: (-item[1]['gb'], item[0]))[:10]
max_gb = max((data['gb'] for _, data in ordered), default=0)
max_downloads = max((data['downloads'] for _, data in ordered), default=0)
return [
@@ -788,6 +826,14 @@ def _build_top_resource_breakdown(self, queryset):
titles = dict(
AbstractNode.objects.filter(guids___id__in=guids).values_list('guids___id', 'title')
)
+ # preprints aren't nodes, and a preprint guid can be versioned (e.g. abcde_v1), which
+ # the node query above never matches. Resolve whatever's left through the guid — at
+ # most ten lookups, since this is a top-ten table.
+ for guid in guids:
+ if guid not in titles:
+ referent, _ = Guid.load_referent(guid)
+ if isinstance(referent, Preprint) and referent.title:
+ titles[guid] = referent.title
return [
{
# a deleted project keeps its title, but fall back to the bare guid so
diff --git a/osf/migrations/0050_downloadevent_user_agent.py b/osf/migrations/0050_downloadevent_user_agent.py
new file mode 100644
index 00000000000..7ec8653f902
--- /dev/null
+++ b/osf/migrations/0050_downloadevent_user_agent.py
@@ -0,0 +1,18 @@
+# Generated by Django 4.2.26 on 2026-08-12 12:09
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('osf', '0049_project_enter'),
+ ]
+
+ operations = [
+ migrations.AddField(
+ model_name='downloadevent',
+ name='user_agent',
+ field=models.TextField(blank=True, default=''),
+ ),
+ ]
diff --git a/osf/models/download_event.py b/osf/models/download_event.py
index 6484dd47b76..f06a34c1b4c 100644
--- a/osf/models/download_event.py
+++ b/osf/models/download_event.py
@@ -44,6 +44,9 @@ class DownloadEvent(models.Model):
user_region = models.CharField(max_length=64, blank=True, default='')
ip = models.GenericIPAddressField(null=True, blank=True)
source_area = models.CharField(max_length=128, blank=True, default='')
+ # the client's User-Agent, to tell frontend downloads apart from API clients and
+ # crawlers. Blank when we couldn't read one (and for rows recorded before this field).
+ user_agent = models.TextField(blank=True, default='')
# nullable: anonymous downloads of public files
user = models.ForeignKey(
diff --git a/osf/utils/download_telemetry.py b/osf/utils/download_telemetry.py
index 66dc5a3e9a0..f7fa0079070 100644
--- a/osf/utils/download_telemetry.py
+++ b/osf/utils/download_telemetry.py
@@ -63,6 +63,7 @@ def write_download_event(
status_code=None,
user_guid=None,
ip=None,
+ user_agent='',
source_area='',
tz='',
):
@@ -99,6 +100,8 @@ def write_download_event(
storage_region=_truncate(storage_region, 64),
user_region=_truncate(derive_user_region(tz, user, storage_region), 64),
ip=ip or None,
+ # capped: the User-Agent comes off the request, so it's client-controlled
+ user_agent=_truncate(user_agent, 512),
source_area=_truncate(source_area, 128),
user=user,
)
diff --git a/tests/test_download_events_dashboard.py b/tests/test_download_events_dashboard.py
index 2d54c1f6edb..94224a490d7 100644
--- a/tests/test_download_events_dashboard.py
+++ b/tests/test_download_events_dashboard.py
@@ -3,11 +3,12 @@
from django.apps import apps as global_apps
from django.contrib.admin.sites import AdminSite
from django.contrib.auth.models import Group, Permission
+from django.test import RequestFactory
from django.utils import timezone
from osf.admin import DASHBOARD_GROUP_NAME, DownloadEventsView
from osf.models import DownloadEvent
-from osf_tests.factories import AuthUserFactory, ProjectFactory
+from osf_tests.factories import AuthUserFactory, ProjectFactory, PreprintFactory
from tests.base import OsfTestCase
@@ -265,6 +266,16 @@ def test_top_projects_falls_back_to_the_bare_guid(self):
assert data['top_projects'][0]['name'] == 'notaguid'
+ def test_top_projects_resolves_preprint_title(self):
+ """Preprints aren't nodes (and their guid can be versioned), but the name should
+ still resolve — ENG-11849."""
+ preprint = PreprintFactory(title='A Preprint About Downloads')
+ make_event(resource_guid=preprint._id, size_bytes=4 * 1024 ** 3)
+
+ data = self.admin.get_dashboard_data(DownloadEvent.objects.all())
+
+ assert data['top_projects'][0]['name'] == f'A Preprint About Downloads ({preprint._id})'
+
def test_time_series_buckets_by_type(self):
make_event(size_bytes=1024 ** 3)
make_event(size_bytes=3 * 1024 ** 3, download_type=DownloadEvent.FOLDER_ZIP)
@@ -332,6 +343,65 @@ def test_unique_users_ignores_anonymous(self):
assert data['summary']['unique_users'] == 1
+class TestSortableColumns(OsfTestCase):
+ """Outcome and User are made sortable (ENG-11863, ENG-11864)."""
+
+ def setUp(self):
+ super().setUp()
+ self.admin = DownloadEventsView(DownloadEvent, AdminSite())
+ self.request = RequestFactory().get('/admin/osf/downloadevent/')
+
+ def test_outcome_column_declares_a_sort_field(self):
+ assert self.admin.outcome.admin_order_field == '_outcome_rank'
+
+ def test_outcome_rank_orders_completed_cancelled_failed_then_single(self):
+ completed = make_event(download_type=DownloadEvent.FOLDER_ZIP, zip_completed=True)
+ cancelled = make_event(download_type=DownloadEvent.FOLDER_ZIP, zip_completed=False, status_code=200)
+ failed = make_event(download_type=DownloadEvent.PROJECT, zip_completed=False, status_code=404)
+ single = make_event(download_type=DownloadEvent.FILE)
+
+ ordered = list(
+ self.admin.get_queryset(self.request).order_by('_outcome_rank').values_list('id', flat=True)
+ )
+
+ assert ordered == [completed.id, cancelled.id, failed.id, single.id]
+
+ def test_user_column_sorts_by_username_not_pk(self):
+ assert self.admin.user_display.admin_order_field == 'user__username'
+
+ def test_user_display_falls_back_for_anonymous(self):
+ anon = make_event(user=None)
+ assert self.admin.user_display(anon) == '—'
+
+ def test_user_agent_column_declares_a_sort_field(self):
+ assert self.admin.user_agent_display.admin_order_field == 'user_agent'
+
+ def test_user_agent_display_shows_short_agents_in_full(self):
+ event = make_event(user_agent='curl/8.0')
+ assert self.admin.user_agent_display(event) == 'curl/8.0'
+
+ def test_user_agent_display_truncates_long_agents(self):
+ event = make_event(user_agent='Mozilla/5.0 ' + 'x' * 200)
+ shown = self.admin.user_agent_display(event)
+ assert len(shown) == 80 and shown.endswith('…')
+
+ def test_user_agent_display_falls_back_when_blank(self):
+ assert self.admin.user_agent_display(make_event(user_agent='')) == '—'
+
+ def test_outcome_annotation_does_not_change_dashboard_numbers(self):
+ """Production feeds get_queryset() (annotated with _outcome_rank for sorting) into
+ get_dashboard_data. The annotation must not alter any aggregate — guards against a
+ stray GROUP BY. Full equality, since the region sort is now deterministic."""
+ make_event(download_type=DownloadEvent.FILE, storage_region='Germany', size_bytes=2 * 1024 ** 3)
+ make_event(download_type=DownloadEvent.FOLDER_ZIP, storage_region='Germany', zip_completed=True, size_bytes=3 * 1024 ** 3)
+ make_event(download_type=DownloadEvent.PROJECT, storage_region='United States', zip_completed=False, status_code=404, size_bytes=5 * 1024 ** 3)
+
+ annotated = self.admin.get_dashboard_data(self.admin.get_queryset(self.request))
+ plain = self.admin.get_dashboard_data(DownloadEvent.objects.all())
+
+ assert annotated == plain
+
+
class TestStaffAccessMigration(OsfTestCase):
"""Django's admin rejects anyone without `is_staff` before our gate runs, so
the allow-listed users need it to reach the page at all."""
diff --git a/tests/test_download_telemetry.py b/tests/test_download_telemetry.py
index ac71b6afe41..74a4e2b1719 100644
--- a/tests/test_download_telemetry.py
+++ b/tests/test_download_telemetry.py
@@ -108,6 +108,7 @@ def build_payload(self, materialized='/', action='download_zip', **action_meta):
'provider': 'osfstorage',
},
'action_meta': meta,
+ 'request_meta': {'user_agent': 'TestZipAgent/1.0'},
}
message, signature = signing.default_signer.sign_payload(options)
return {'payload': message, 'signature': signature}
@@ -215,6 +216,29 @@ def test_storage_provider_comes_from_the_callback(self):
assert DownloadEvent.objects.get().storage_provider == 'osfstorage'
+ def test_user_agent_comes_from_the_callback(self):
+ self.app.put(self.url, json=self.build_payload())
+
+ assert DownloadEvent.objects.get().user_agent == 'TestZipAgent/1.0'
+
+ def test_missing_request_meta_leaves_user_agent_blank(self):
+ """A callback from a WaterButler build that predates request_meta records an empty
+ user agent rather than failing."""
+ options = {
+ 'auth': {'id': self.user._id},
+ 'action': 'download_zip',
+ 'provider': 'osfstorage',
+ 'time': time.time() + 1000,
+ 'metadata': {'nid': self.node._id, 'materialized': '/', 'path': '/',
+ 'kind': 'folder', 'provider': 'osfstorage'},
+ 'action_meta': {},
+ }
+ message, signature = signing.default_signer.sign_payload(options)
+ res = self.app.put(self.url, json={'payload': message, 'signature': signature})
+
+ assert res.status_code == 200
+ assert DownloadEvent.objects.get().user_agent == ''
+
def test_callback_still_succeeds_when_recording_fails(self, ):
with pytest.MonkeyPatch.context() as patch:
patch.setattr(
@@ -262,6 +286,23 @@ def test_storage_provider_comes_from_the_file(self):
assert DownloadEvent.objects.get().storage_provider == 'osfstorage'
+ def test_user_agent_comes_from_the_request(self):
+ self.app.get(
+ f'/download/{self.guid}/', auth=self.user.auth,
+ headers={'User-Agent': 'PytestClient/9.9'},
+ )
+
+ assert DownloadEvent.objects.get().user_agent == 'PytestClient/9.9'
+
+ def test_long_user_agent_is_capped(self):
+ """The User-Agent is client-controlled, so the write caps it to the column width."""
+ self.app.get(
+ f'/download/{self.guid}/', auth=self.user.auth,
+ headers={'User-Agent': 'x' * 900},
+ )
+
+ assert len(DownloadEvent.objects.get().user_agent) == 512
+
def test_zip_completed_is_unset_for_single_files(self):
"""Only zips stream through WaterButler, so nothing reports completion here."""
self.app.get(f'/download/{self.guid}/', auth=self.user.auth)