diff --git a/app/data/serializers.py b/app/data/serializers.py index c3cd6613..677ecb07 100644 --- a/app/data/serializers.py +++ b/app/data/serializers.py @@ -2,7 +2,12 @@ import datetime import pytz -from rest_framework.serializers import (ModelSerializer, SerializerMethodField, ValidationError) +from rest_framework.serializers import ( + CharField, + ModelSerializer, + SerializerMethodField, + ValidationError, +) from ashlar import serializers from ashlar import serializer_fields @@ -48,6 +53,14 @@ def filter_details_only(self, key, value): raise serializer_fields.DropJsonKeyException +class DetailsReadOnlyRecordNonPublicSerializer(DetailsReadOnlyRecordSerializer): + """ + Serialize records with only read-only fields included plus non-public fields + (only available to admins and analysts) + """ + created_by = CharField() + + class DetailsReadOnlyRecordSchemaSerializer(serializers.RecordSchemaSerializer): """Serialize Schema with only read-only fields included""" schema = serializer_fields.MethodTransformJsonField('make_read_only_schema') diff --git a/app/data/tests/test_views.py b/app/data/tests/test_views.py index 2e8f2dd5..5b33270d 100644 --- a/app/data/tests/test_views.py +++ b/app/data/tests/test_views.py @@ -35,9 +35,15 @@ def set_up_admin_client(self): self.admin_client.force_authenticate(user=self.admin) def set_up_public_client(self): - self.public = User.objects.create_user('public', 'public@ashlar', 'public') - self.public.groups.add(Group.objects.get(name='public')) - self.public.save() + try: + self.public = User.objects.get(username='public') + except User.DoesNotExist: + self.public = User.objects.create_user('public', 'public@ashlar', 'public') + self.public.save() + + if not self.public.groups.exists(): + self.public.groups.add(Group.objects.get(name='public')) + self.public_client = APIClient() self.public_client.force_authenticate(user=self.public) @@ -105,13 +111,31 @@ def set_up_records(self): location_text='Equator', schema=self.schema) + def set_up_audit_log(self): + self.audit_log_entry1 = RecordAuditLogEntry.objects.create( + user=self.admin, + username=self.admin.username, + action=RecordAuditLogEntry.ActionTypes.CREATE, + record=self.record2, + record_uuid=self.record2.uuid, + ) + # CREATE audit log entry where user has been deleted + self.audit_log_entry2 = RecordAuditLogEntry.objects.create( + user=None, + username='banana', + action=RecordAuditLogEntry.ActionTypes.CREATE, + record=self.record3, + record_uuid=self.record3.uuid, + ) class DriverRecordViewTestCase(APITestCase, ViewTestSetUpMixin): def setUp(self): super(DriverRecordViewTestCase, self).setUp() self.set_up_admin_client() + self.set_up_public_client() self.set_up_records() + self.set_up_audit_log() self.factory = APIRequestFactory() def test_toddow(self): @@ -156,6 +180,21 @@ def test_arbitrary_filters(self): response_data2 = json.loads(self.admin_client.get(url2).content) self.assertEqual(len(response_data2), 2) + def test_created_by_admin_client_email(self): + url = '/api/records/{uuid}/?details_only=True'.format(uuid=self.record2.uuid) + response_data = json.loads(self.admin_client.get(url).content) + self.assertEqual(response_data['created_by'], self.audit_log_entry1.user.email) + + def test_created_by_admin_client_username(self): + url = '/api/records/{uuid}/?details_only=True'.format(uuid=self.record3.uuid) + response_data = json.loads(self.admin_client.get(url).content) + self.assertEqual(response_data['created_by'], self.audit_log_entry2.username) + + def test_created_by_public_client(self): + url = '/api/records/?details_only=True' + public_response_data = json.loads(self.public_client.get(url).content) + self.assertTrue(all('created_by' not in result for result in public_response_data['results'])) + def test_tilekey_param(self): """Ensure that the tilekey param stores a SQL query in Redis and returns an access token""" # Since the call to store in redis won't have access to a real Redis instance under test, @@ -178,16 +217,18 @@ def test_tilekey_param(self): def test_get_serializer_class(self): """Test that get_serializer_class returns read-only serializer correctly""" - read_only = User.objects.create_user('public', 'public@public.com', 'public') view = DriverRecordViewSet() mock_req = mock.Mock(spec=Request) - mock_req.user = read_only + mock_req.user = self.public view.request = mock_req serializer_class = view.get_serializer_class() self.assertEqual(serializer_class, DetailsReadOnlyRecordSerializer) def test_audit_log_creation(self): """Test that audit logs are generated on create operations""" + # Start from clean slate + RecordAuditLogEntry.objects.all().delete() + url = '/api/records/' post_data = { 'data': { diff --git a/app/data/views.py b/app/data/views.py index 8f25e765..c9945fb4 100644 --- a/app/data/views.py +++ b/app/data/views.py @@ -13,16 +13,21 @@ from django.conf import settings from django.db import transaction -from django.db.models import (Case, - When, - IntegerField, - DateTimeField, - CharField, - UUIDField, - Value, - Count, - Sum, - Q) +from django.db.models import ( + Case, + CharField, + Count, + DateTimeField, + IntegerField, + OuterRef, + Q, + Subquery, + Sum, + UUIDField, + Value, + When, +) +from django.db.models.functions import Coalesce from django_redis import get_redis_connection from rest_framework import viewsets @@ -60,7 +65,8 @@ from models import RecordAuditLogEntry, RecordDuplicate, RecordCostConfig from serializers import (DriverRecordSerializer, DetailsReadOnlyRecordSerializer, DetailsReadOnlyRecordSchemaSerializer, RecordAuditLogEntrySerializer, - RecordDuplicateSerializer, RecordCostConfigSerializer) + RecordDuplicateSerializer, RecordCostConfigSerializer, + DetailsReadOnlyRecordNonPublicSerializer) import transformers from driver import mixins @@ -103,13 +109,31 @@ def get_serializer_class(self): if details_only_param == 'True' or details_only_param == 'true': requested_details_only = True - if is_admin_or_writer(self.request.user) and not requested_details_only: - return DriverRecordSerializer + if is_admin_or_writer(self.request.user): + if requested_details_only: + return DetailsReadOnlyRecordNonPublicSerializer + else: + return DriverRecordSerializer return DetailsReadOnlyRecordSerializer def get_queryset(self): - """Override default model ordering""" qs = super(DriverRecordViewSet, self).get_queryset() + if self.get_serializer_class() is DetailsReadOnlyRecordNonPublicSerializer: + # Add in `created_by` field for user who created the record + created_by_query = ( + RecordAuditLogEntry.objects.filter( + record=OuterRef('pk'), + action=RecordAuditLogEntry.ActionTypes.CREATE + ) + .annotate( + # Fall back to username if the user has been deleted + email_or_username=Coalesce('user__email', 'username') + ) + .values('email_or_username') + [:1] + ) + qs = qs.annotate(created_by=Subquery(created_by_query, output_field=CharField())) + # Override default model ordering return qs.order_by('-occurred_from') def get_filtered_queryset(self, request): diff --git a/web/app/i18n/en-us.json b/web/app/i18n/en-us.json index e775437e..901a9dc8 100644 --- a/web/app/i18n/en-us.json +++ b/web/app/i18n/en-us.json @@ -185,6 +185,7 @@ "MODIFIED": "Modified", "MODIFIED_BY": "Modified by", "DATE_AND_TIME": "Date & Time", + "CREATED_BY": "Created by", "DATE_FILTER": "Filter events by date of occurrence", "CREATED_FILTER": "Filter events by date of record creation", "DETAILS": "Details", diff --git a/web/app/scripts/views/record/list-partial.html b/web/app/scripts/views/record/list-partial.html index 9ddb568d..e243d532 100755 --- a/web/app/scripts/views/record/list-partial.html +++ b/web/app/scripts/views/record/list-partial.html @@ -10,6 +10,8 @@ {{ ::headerKey }} +