Skip to content

Commit

Permalink
Merge pull request #708 from WorldBank-Transport/feature/add-created-…
Browse files Browse the repository at this point in the history
…by-to-record-list

Add created_by to record list
  • Loading branch information
pcaisse authored Sep 12, 2018
2 parents ec442b9 + 2cb3206 commit e9ef599
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 20 deletions.
15 changes: 14 additions & 1 deletion app/data/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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')
Expand Down
51 changes: 46 additions & 5 deletions app/data/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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,
Expand All @@ -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', '[email protected]', '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': {
Expand Down
52 changes: 38 additions & 14 deletions app/data/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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):
Expand Down
1 change: 1 addition & 0 deletions web/app/i18n/en-us.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 6 additions & 0 deletions web/app/scripts/views/record/list-partial.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
{{ ::headerKey }}
</th>

<th ng-if="ctl.userCanWrite">{{ 'RECORD.CREATED_BY' | translate }}</th>

<!-- View/Edit links -->
<th></th>
</tr>
Expand All @@ -31,6 +33,10 @@
</driver-details-field>
</td>

<td ng-if="ctl.userCanWrite" class="created_by">
{{ ::record.created_by }}
</td>

<td class="links">
<a ng-click="ctl.showDetailsModal(record)">
<span class="glyphicon glyphicon-log-in"></span> {{ 'COMMON.VIEW' | translate }}
Expand Down

0 comments on commit e9ef599

Please sign in to comment.