From 4139aa269ca5701a9a4074cf389f75f23a5f2faa Mon Sep 17 00:00:00 2001 From: Alex Manning Date: Tue, 12 Aug 2025 16:31:43 +0100 Subject: [PATCH 1/5] Allow searching metadata in the grant interface. --- jasmin_services/admin/grant.py | 30 +- jasmin_services/admin/request.py | 28 +- .../tests/test_admin_metadata_search.py | 289 ++++++++++++++++++ tox.ini | 2 +- 4 files changed, 342 insertions(+), 7 deletions(-) create mode 100644 jasmin_services/tests/test_admin_metadata_search.py diff --git a/jasmin_services/admin/grant.py b/jasmin_services/admin/grant.py index f1ac709..78e03bc 100644 --- a/jasmin_services/admin/grant.py +++ b/jasmin_services/admin/grant.py @@ -3,8 +3,6 @@ import operator from urllib.parse import urlparse -import django.db.models.fields.related -import django.db.models.fields.reverse_related from django.contrib import admin from django.contrib.contenttypes.models import ContentType from django.shortcuts import redirect, render @@ -85,9 +83,7 @@ def send_expiry_notifications(self, request, queryset): send_expiry_notifications.short_description = "Send expiry notifications" def revoke_grants(self, request, queryset): - """ - Admin action that revokes the selected grants. - """ + """Admin action that revokes the selected grants.""" selected = queryset.values_list("pk", flat=True) selected_ids = "_".join(str(pk) for pk in selected) @@ -222,6 +218,30 @@ def get_metadata_form_initial_data(self, request, obj): return {d.key: d.value for d in metadata.all()} return super().get_metadata_form_initial_data(request, obj) + def get_search_results(self, request, queryset, search_term): + """Override search to include metadata values.""" + # Get the standard search results first + queryset, use_distinct = super().get_search_results(request, queryset, search_term) + + if search_term: + grant_content_type = ContentType.objects.get_for_model(Grant) + metadata_objects = Metadatum.objects.filter(content_type=grant_content_type) + + matching_ids = [] + for metadata in metadata_objects: + # Convert pickled value to string and search + value_str = str(metadata.value) if metadata.value is not None else "" + if search_term.lower() in value_str.lower(): + matching_ids.append(metadata.object_id) + + if matching_ids: + # Combine with existing queryset + metadata_queryset = self.model.objects.filter(pk__in=matching_ids) + queryset = queryset | metadata_queryset + use_distinct = True + + return queryset, use_distinct + def get_urls(self): return [ re_path( diff --git a/jasmin_services/admin/request.py b/jasmin_services/admin/request.py index 94f9004..e198e40 100644 --- a/jasmin_services/admin/request.py +++ b/jasmin_services/admin/request.py @@ -1,13 +1,15 @@ from django.contrib import admin from django.contrib.admin.utils import quote +from django.contrib.contenttypes.models import ContentType from django.urls import reverse from django.utils.safestring import mark_safe from jasmin_metadata.admin import HasMetadataModelAdmin +from jasmin_metadata.models import Metadatum from ..actions import remind_pending from ..forms import AdminRequestForm -from ..models import RequestState, Role +from ..models import Request, RequestState, Role # Load the admin for behaviours which are turned on. from . import behaviour # unimport:skip @@ -119,3 +121,27 @@ def get_changeform_initial_data(self, request): initial = super().get_changeform_initial_data(request) initial["requested_by"] = request.user.username return initial + + def get_search_results(self, request, queryset, search_term): + """Override search to include metadata values.""" + # Get the standard search results first + queryset, use_distinct = super().get_search_results(request, queryset, search_term) + + if search_term: + request_content_type = ContentType.objects.get_for_model(Request) + metadata_objects = Metadatum.objects.filter(content_type=request_content_type) + + matching_ids = [] + for metadata in metadata_objects: + # Convert pickled value to string and search + value_str = str(metadata.value) if metadata.value is not None else "" + if search_term.lower() in value_str.lower(): + matching_ids.append(metadata.object_id) + + if matching_ids: + # Combine with existing queryset + metadata_queryset = self.model.objects.filter(pk__in=matching_ids) + queryset = queryset | metadata_queryset + use_distinct = True + + return queryset, use_distinct diff --git a/jasmin_services/tests/test_admin_metadata_search.py b/jasmin_services/tests/test_admin_metadata_search.py new file mode 100644 index 0000000..3fb6069 --- /dev/null +++ b/jasmin_services/tests/test_admin_metadata_search.py @@ -0,0 +1,289 @@ +"""Tests for admin metadata search functionality.""" + +import django.contrib.auth +import django.test +from django.contrib.contenttypes.models import ContentType + +import jasmin_metadata.models +import jasmin_services.models +from jasmin_services.admin.grant import GrantAdmin +from jasmin_services.admin.request import RequestAdmin + + +class AdminMetadataSearchTestCase(django.test.TestCase): + """Test metadata search functionality in admin classes.""" + + def setUp(self): + """Set up test data with metadata.""" + # Create user + User = django.contrib.auth.get_user_model() + self.user1 = User.objects.create_user( + username="testuser1", email="testuser1@example.com", first_name="Test", last_name="User" + ) + self.user2 = User.objects.create_user( + username="testuser2", + email="testuser2@example.com", + first_name="Another", + last_name="Person", + ) + + # Create category and service + self.category = jasmin_services.models.Category.objects.create( + name="test_cat", long_name="Test Category", position=1 + ) + self.service = jasmin_services.models.Service.objects.create( + category=self.category, + name="test_service", + summary="Test Service", + description="A service for testing", + ) + + # Create role + self.role = jasmin_services.models.Role.objects.create( + service=self.service, name="test_role", description="Test role" + ) + + # Create access objects + self.access1 = jasmin_services.models.Access.objects.create(role=self.role, user=self.user1) + self.access2 = jasmin_services.models.Access.objects.create(role=self.role, user=self.user2) + + # Create grants + self.grant1 = jasmin_services.models.Grant.objects.create( + access=self.access1, granted_by="admin" + ) + self.grant2 = jasmin_services.models.Grant.objects.create( + access=self.access2, granted_by="admin" + ) + + # Create requests + self.request1 = jasmin_services.models.Request.objects.create( + access=self.access1, requested_by="testuser1" + ) + self.request2 = jasmin_services.models.Request.objects.create( + access=self.access2, requested_by="testuser2" + ) + + # Create metadata for grants + grant_ct = ContentType.objects.get_for_model(jasmin_services.models.Grant) + jasmin_metadata.models.Metadatum.objects.create( + content_type=grant_ct, + object_id=self.grant1.pk, + key="project_name", + value="Climate Research Project", + ) + jasmin_metadata.models.Metadatum.objects.create( + content_type=grant_ct, + object_id=self.grant1.pk, + key="institution", + value="University of Oxford", + ) + jasmin_metadata.models.Metadatum.objects.create( + content_type=grant_ct, + object_id=self.grant2.pk, + key="project_name", + value="Ocean Data Analysis", + ) + jasmin_metadata.models.Metadatum.objects.create( + content_type=grant_ct, + object_id=self.grant2.pk, + key="storage_quota", + value=500, + ) + + # Create metadata for requests + request_ct = ContentType.objects.get_for_model(jasmin_services.models.Request) + jasmin_metadata.models.Metadatum.objects.create( + content_type=request_ct, + object_id=self.request1.pk, + key="project_description", + value="Analyzing climate data for polar regions", + ) + jasmin_metadata.models.Metadatum.objects.create( + content_type=request_ct, + object_id=self.request1.pk, + key="contact_email", + value="researcher@oxford.ac.uk", + ) + jasmin_metadata.models.Metadatum.objects.create( + content_type=request_ct, + object_id=self.request2.pk, + key="project_description", + value="Marine biology research project", + ) + jasmin_metadata.models.Metadatum.objects.create( + content_type=request_ct, object_id=self.request2.pk, key="urgency_level", value="High" + ) + + # Initialize admin instances + self.grant_admin = GrantAdmin(jasmin_services.models.Grant, None) + self.request_admin = RequestAdmin(jasmin_services.models.Request, None) + + def test_grant_admin_metadata_search_text(self): + """Test searching grants by text metadata values.""" + queryset = jasmin_services.models.Grant.objects.all() + + # Search for "Climate" should find grant1 + results, use_distinct = self.grant_admin.get_search_results(None, queryset, "Climate") + self.assertTrue(use_distinct) + self.assertIn(self.grant1, results) + self.assertNotIn(self.grant2, results) + + # Search for "Oxford" should find grant1 + results, use_distinct = self.grant_admin.get_search_results(None, queryset, "Oxford") + self.assertTrue(use_distinct) + self.assertIn(self.grant1, results) + self.assertNotIn(self.grant2, results) + + # Search for "Ocean" should find grant2 + results, use_distinct = self.grant_admin.get_search_results(None, queryset, "Ocean") + self.assertTrue(use_distinct) + self.assertNotIn(self.grant1, results) + self.assertIn(self.grant2, results) + + def test_grant_admin_metadata_search_numeric(self): + """Test searching grants by numeric metadata values.""" + queryset = jasmin_services.models.Grant.objects.all() + + # Search for "500" should find grant2 + results, use_distinct = self.grant_admin.get_search_results(None, queryset, "500") + self.assertTrue(use_distinct) + self.assertNotIn(self.grant1, results) + self.assertIn(self.grant2, results) + + def test_grant_admin_metadata_search_case_insensitive(self): + """Test that metadata search is case insensitive.""" + queryset = jasmin_services.models.Grant.objects.all() + + # Search for "climate" (lowercase) should find grant1 + results, use_distinct = self.grant_admin.get_search_results(None, queryset, "climate") + self.assertTrue(use_distinct) + self.assertIn(self.grant1, results) + + # Search for "OXFORD" (uppercase) should find grant1 + results, use_distinct = self.grant_admin.get_search_results(None, queryset, "OXFORD") + self.assertTrue(use_distinct) + self.assertIn(self.grant1, results) + + def test_request_admin_metadata_search_text(self): + """Test searching requests by text metadata values.""" + queryset = jasmin_services.models.Request.objects.all() + + # Search for "polar" should find request1 + results, use_distinct = self.request_admin.get_search_results(None, queryset, "polar") + self.assertTrue(use_distinct) + self.assertIn(self.request1, results) + self.assertNotIn(self.request2, results) + + # Search for "marine" should find request2 + results, use_distinct = self.request_admin.get_search_results(None, queryset, "marine") + self.assertTrue(use_distinct) + self.assertNotIn(self.request1, results) + self.assertIn(self.request2, results) + + # Search for "oxford.ac.uk" should find request1 + results, use_distinct = self.request_admin.get_search_results( + None, queryset, "oxford.ac.uk" + ) + self.assertTrue(use_distinct) + self.assertIn(self.request1, results) + self.assertNotIn(self.request2, results) + + def test_request_admin_metadata_search_case_insensitive(self): + """Test that request metadata search is case insensitive.""" + queryset = jasmin_services.models.Request.objects.all() + + # Search for "HIGH" (uppercase) should find request2 + results, use_distinct = self.request_admin.get_search_results(None, queryset, "HIGH") + self.assertTrue(use_distinct) + self.assertNotIn(self.request1, results) + self.assertIn(self.request2, results) + + def test_metadata_search_no_results(self): + """Test searching for terms that don't exist in metadata.""" + queryset = jasmin_services.models.Grant.objects.all() + + # Search for non-existent term + results, use_distinct = self.grant_admin.get_search_results( + None, queryset, "nonexistentterm" + ) + self.assertFalse(use_distinct) # No metadata matches, no combining needed + self.assertEqual(list(results), []) + + def test_empty_search_term(self): + """Test that empty search terms don't crash.""" + queryset = jasmin_services.models.Grant.objects.all() + + # Empty search term should return original queryset + results, use_distinct = self.grant_admin.get_search_results(None, queryset, "") + self.assertFalse(use_distinct) + self.assertEqual(list(results), list(queryset)) + + # None search term should return original queryset + results, use_distinct = self.grant_admin.get_search_results(None, queryset, None) + self.assertFalse(use_distinct) + self.assertEqual(list(results), list(queryset)) + + def test_metadata_search_with_standard_search(self): + """Test that metadata search combines with standard search fields.""" + queryset = jasmin_services.models.Grant.objects.all() + + # Search for username should work (standard search) + results, use_distinct = self.grant_admin.get_search_results(None, queryset, "testuser1") + # Should find grant1 through standard user search + self.assertIn(self.grant1, results) + + def test_special_characters_in_metadata(self): + """Test searching metadata with special characters.""" + # Add metadata with special characters + grant_ct = ContentType.objects.get_for_model(jasmin_services.models.Grant) + jasmin_metadata.models.Metadatum.objects.create( + content_type=grant_ct, + object_id=self.grant1.pk, + key="special_field", + value="test@domain.com & other/special-chars_123", + ) + + queryset = jasmin_services.models.Grant.objects.all() + + # Search for part with special characters + results, use_distinct = self.grant_admin.get_search_results( + None, queryset, "test@domain.com" + ) + self.assertTrue(use_distinct) + self.assertIn(self.grant1, results) + + # Search for part with underscore + results, use_distinct = self.grant_admin.get_search_results( + None, queryset, "special-chars_123" + ) + self.assertTrue(use_distinct) + self.assertIn(self.grant1, results) + + def test_none_metadata_values(self): + """Test that None metadata values don't cause crashes.""" + # Create metadata with None value + grant_ct = ContentType.objects.get_for_model(jasmin_services.models.Grant) + jasmin_metadata.models.Metadatum.objects.create( + content_type=grant_ct, object_id=self.grant1.pk, key="null_field", value=None + ) + + queryset = jasmin_services.models.Grant.objects.all() + + # Search should not crash with None values + results, use_distinct = self.grant_admin.get_search_results(None, queryset, "anything") + # Should complete without error + self.assertIsNotNone(results) + + def test_partial_matches(self): + """Test that partial string matches work correctly.""" + queryset = jasmin_services.models.Grant.objects.all() + + # Search for partial match "Clim" should find "Climate Research Project" + results, use_distinct = self.grant_admin.get_search_results(None, queryset, "Clim") + self.assertTrue(use_distinct) + self.assertIn(self.grant1, results) + + # Search for partial match "Data" should find "Ocean Data Analysis" + results, use_distinct = self.grant_admin.get_search_results(None, queryset, "Data") + self.assertTrue(use_distinct) + self.assertIn(self.grant2, results) diff --git a/tox.ini b/tox.ini index 5117a53..c5b3e25 100644 --- a/tox.ini +++ b/tox.ini @@ -19,7 +19,7 @@ commands_pre = poetry install --sync --only=main,test --all-extras commands = django-admin check - django-admin test --parallel {posargs} + django-admin test {posargs} [testenv:coverage] commands = From 51a2813c4f179b926e0c6396269a0cd27db1d6d5 Mon Sep 17 00:00:00 2001 From: Alex Manning Date: Tue, 12 Aug 2025 17:08:44 +0100 Subject: [PATCH 2/5] Add metadata form to role. --- jasmin_services/tests/test_admin_metadata_search.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/jasmin_services/tests/test_admin_metadata_search.py b/jasmin_services/tests/test_admin_metadata_search.py index 3fb6069..7a937e2 100644 --- a/jasmin_services/tests/test_admin_metadata_search.py +++ b/jasmin_services/tests/test_admin_metadata_search.py @@ -38,9 +38,15 @@ def setUp(self): description="A service for testing", ) + # Create metadata form for role + self.metadata_form = jasmin_metadata.models.Form.objects.create(name="Test Role Form") + # Create role self.role = jasmin_services.models.Role.objects.create( - service=self.service, name="test_role", description="Test role" + service=self.service, + name="test_role", + description="Test role", + metadata_form=self.metadata_form, ) # Create access objects From 3f972240672c732899cf210a9c6c4306522a42d2 Mon Sep 17 00:00:00 2001 From: Alex Manning Date: Tue, 12 Aug 2025 17:13:54 +0100 Subject: [PATCH 3/5] Fix settings issue. --- jasmin_services/tests/test_admin_metadata_search.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/jasmin_services/tests/test_admin_metadata_search.py b/jasmin_services/tests/test_admin_metadata_search.py index 7a937e2..cf3b63e 100644 --- a/jasmin_services/tests/test_admin_metadata_search.py +++ b/jasmin_services/tests/test_admin_metadata_search.py @@ -3,6 +3,7 @@ import django.contrib.auth import django.test from django.contrib.contenttypes.models import ContentType +from django.test import override_settings import jasmin_metadata.models import jasmin_services.models @@ -10,6 +11,7 @@ from jasmin_services.admin.request import RequestAdmin +@override_settings(MULTIPLE_REQUESTS_ALLOWED=False) class AdminMetadataSearchTestCase(django.test.TestCase): """Test metadata search functionality in admin classes.""" From 34e59ce601ec35516db5a71fabcf3ae881543cef Mon Sep 17 00:00:00 2001 From: Alex Manning Date: Wed, 13 Aug 2025 11:18:13 +0100 Subject: [PATCH 4/5] Fix notifications problem in tests. --- .../tests/test_admin_metadata_search.py | 48 +++++++++++++------ tox.ini | 6 +-- 2 files changed, 36 insertions(+), 18 deletions(-) diff --git a/jasmin_services/tests/test_admin_metadata_search.py b/jasmin_services/tests/test_admin_metadata_search.py index cf3b63e..8c5898a 100644 --- a/jasmin_services/tests/test_admin_metadata_search.py +++ b/jasmin_services/tests/test_admin_metadata_search.py @@ -3,10 +3,12 @@ import django.contrib.auth import django.test from django.contrib.contenttypes.models import ContentType +from django.db.models import signals from django.test import override_settings import jasmin_metadata.models import jasmin_services.models +from jasmin_services import notifications from jasmin_services.admin.grant import GrantAdmin from jasmin_services.admin.request import RequestAdmin @@ -17,10 +19,21 @@ class AdminMetadataSearchTestCase(django.test.TestCase): def setUp(self): """Set up test data with metadata.""" - # Create user + # These signals do not exist on the standard django user model. + signals.post_save.disconnect( + notifications.confirm_request, sender=jasmin_services.models.Request + ) + signals.post_save.disconnect( + notifications.notify_approvers_created, sender=jasmin_services.models.Request + ) + + # Create users. User = django.contrib.auth.get_user_model() self.user1 = User.objects.create_user( - username="testuser1", email="testuser1@example.com", first_name="Test", last_name="User" + username="testuser1", + email="testuser1@example.com", + first_name="Test", + last_name="User", ) self.user2 = User.objects.create_user( username="testuser2", @@ -29,9 +42,11 @@ def setUp(self): last_name="Person", ) - # Create category and service + # Create a service. self.category = jasmin_services.models.Category.objects.create( - name="test_cat", long_name="Test Category", position=1 + name="test_cat", + long_name="Test Category", + position=1, ) self.service = jasmin_services.models.Service.objects.create( category=self.category, @@ -40,10 +55,8 @@ def setUp(self): description="A service for testing", ) - # Create metadata form for role + # And associated role. self.metadata_form = jasmin_metadata.models.Form.objects.create(name="Test Role Form") - - # Create role self.role = jasmin_services.models.Role.objects.create( service=self.service, name="test_role", @@ -51,19 +64,15 @@ def setUp(self): metadata_form=self.metadata_form, ) - # Create access objects + # Create grants self.access1 = jasmin_services.models.Access.objects.create(role=self.role, user=self.user1) self.access2 = jasmin_services.models.Access.objects.create(role=self.role, user=self.user2) - - # Create grants self.grant1 = jasmin_services.models.Grant.objects.create( access=self.access1, granted_by="admin" ) self.grant2 = jasmin_services.models.Grant.objects.create( access=self.access2, granted_by="admin" ) - - # Create requests self.request1 = jasmin_services.models.Request.objects.create( access=self.access1, requested_by="testuser1" ) @@ -71,7 +80,7 @@ def setUp(self): access=self.access2, requested_by="testuser2" ) - # Create metadata for grants + # Create some fake metadata grant_ct = ContentType.objects.get_for_model(jasmin_services.models.Grant) jasmin_metadata.models.Metadatum.objects.create( content_type=grant_ct, @@ -98,7 +107,6 @@ def setUp(self): value=500, ) - # Create metadata for requests request_ct = ContentType.objects.get_for_model(jasmin_services.models.Request) jasmin_metadata.models.Metadatum.objects.create( content_type=request_ct, @@ -122,10 +130,20 @@ def setUp(self): content_type=request_ct, object_id=self.request2.pk, key="urgency_level", value="High" ) - # Initialize admin instances self.grant_admin = GrantAdmin(jasmin_services.models.Grant, None) self.request_admin = RequestAdmin(jasmin_services.models.Request, None) + def tearDown(self): + """Clean up after tests.""" + # Reconnect signals. + signals.post_save.connect( + notifications.confirm_request, sender=jasmin_services.models.Request + ) + signals.post_save.connect( + notifications.notify_approvers_created, sender=jasmin_services.models.Request + ) + super().tearDown() + def test_grant_admin_metadata_search_text(self): """Test searching grants by text metadata values.""" queryset = jasmin_services.models.Grant.objects.all() diff --git a/tox.ini b/tox.ini index c5b3e25..0b5643c 100644 --- a/tox.ini +++ b/tox.ini @@ -16,7 +16,7 @@ setenv = allowlist_externals = poetry commands_pre = - poetry install --sync --only=main,test --all-extras + poetry sync --only=main,test --all-extras commands = django-admin check django-admin test {posargs} @@ -29,11 +29,11 @@ commands = [testenv:black] commands_pre= - poetry install --sync --only=dev --all-extras + poetry sync --only=dev --all-extras commands = black --check --fast . [testenv:bandit] commands_pre= - poetry install --sync --only=dev --all-extras + poetry sync --only=dev --all-extras commands = bandit -c pyproject.toml -r --severity-level high jasmin_services From 226203620eaa04b179eecaea7c97d8ffd50467d1 Mon Sep 17 00:00:00 2001 From: Alex Manning Date: Wed, 13 Aug 2025 11:21:52 +0100 Subject: [PATCH 5/5] Update poetry version. --- .github/workflows/tox.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tox.yaml b/.github/workflows/tox.yaml index 6cd4be3..7ff76c0 100644 --- a/.github/workflows/tox.yaml +++ b/.github/workflows/tox.yaml @@ -30,7 +30,7 @@ jobs: - name: Install Poetry uses: snok/install-poetry@v1 with: - version: 1.4.2 + version: 2.0.1 - name: Install test dependencies. run: pip install tox>=4.0.16 tox-gh - name: Setup test suite