Skip to content

Commit

Permalink
refactor: split assignment serializer into admin vs. non-admin
Browse files Browse the repository at this point in the history
This gives the benefit of allowing us to remove duplicate code
(in-memory calculations of recent_action + learner_state).

ENT-7780
  • Loading branch information
pwnage101 committed Oct 18, 2023
1 parent e5dda66 commit 2c2251a
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 104 deletions.
5 changes: 4 additions & 1 deletion enterprise_access/apps/api/serializers/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
"""
API serializers module.
"""
from .content_assignments.assignment import LearnerContentAssignmentResponseSerializer
from .content_assignments.assignment import (
LearnerContentAssignmentAdminResponseSerializer,
LearnerContentAssignmentResponseSerializer
)
from .content_assignments.assignment_configuration import (
AssignmentConfigurationCreateRequestSerializer,
AssignmentConfigurationDeleteRequestSerializer,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,11 @@ class LearnerContentAssignmentRecentActionSerializer(serializers.Serializer):
action_type = serializers.ChoiceField(
help_text='Type of the recent action.',
choices=AssignmentRecentActionTypes.CHOICES,
source='recent_action',
)
timestamp = serializers.DateTimeField(
help_text='Date and time when the action was taken.',
source='recent_action_time',
)


Expand All @@ -60,18 +62,6 @@ class LearnerContentAssignmentResponseSerializer(serializers.ModelSerializer):
help_text='All actions associated with this assignment.',
many=True,
)
recent_action = LearnerContentAssignmentRecentActionSerializer(
help_text='Structured data about the most recent action. Meant to power a frontend table column.',
source='get_recent_action_data',
)
learner_state = serializers.ChoiceField(
help_text=(
'learner_state is an admin-facing dynamic state, not to be confused with `state`. Meant to power a '
'frontend table column.'
),
choices=AssignmentLearnerStates.CHOICES,
source='get_learner_state',
)

class Meta:
model = LearnerContentAssignment
Expand All @@ -86,6 +76,32 @@ class Meta:
'transaction_uuid',
'last_notification_at',
'actions',
]
read_only_fields = fields


class LearnerContentAssignmentAdminResponseSerializer(LearnerContentAssignmentResponseSerializer):
"""
A read-only Serializer for responding to requests for ``LearnerContentAssignment`` records FOR ADMINS.
Important: This serializer depends on extra dynamic fields annotated by calling
``LearnerContentAssignment.annotate_dynamic_fields_onto_queryset()`` on the assignment queryset.
"""

recent_action = LearnerContentAssignmentRecentActionSerializer(
help_text='Structured data about the most recent action. Meant to power a frontend table column.',
source='*',
)
learner_state = serializers.ChoiceField(
help_text=(
'learner_state is an admin-facing dynamic state, not to be confused with `state`. Meant to power a '
'frontend table column.'
),
choices=AssignmentLearnerStates.CHOICES,
)

class Meta(LearnerContentAssignmentResponseSerializer.Meta):
fields = LearnerContentAssignmentResponseSerializer.Meta.fields + [
'recent_action',
'learner_state',
]
Expand Down
20 changes: 0 additions & 20 deletions enterprise_access/apps/api/v1/tests/test_allocation_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,6 @@ def test_allocate_happy_path(self, mock_allocate, mock_can_allocate):
'transaction_uuid': None,
'uuid': str(self.alice_assignment.uuid),
'actions': [],
'recent_action': {
'action_type': AssignmentRecentActionTypes.ASSIGNED,
'timestamp': self.alice_assignment.created.strftime('%Y-%m-%dT%H:%M:%S.%fZ'),
},
'learner_state': None,
},
],
'created': [
Expand All @@ -166,11 +161,6 @@ def test_allocate_happy_path(self, mock_allocate, mock_can_allocate):
'transaction_uuid': None,
'uuid': str(self.bob_assignment.uuid),
'actions': [],
'recent_action': {
'action_type': AssignmentRecentActionTypes.ASSIGNED,
'timestamp': self.bob_assignment.created.strftime('%Y-%m-%dT%H:%M:%S.%fZ'),
},
'learner_state': AssignmentLearnerStates.NOTIFYING,
},
],
'no_change': [
Expand All @@ -185,11 +175,6 @@ def test_allocate_happy_path(self, mock_allocate, mock_can_allocate):
'transaction_uuid': None,
'uuid': str(self.carol_assignment.uuid),
'actions': [],
'recent_action': {
'action_type': AssignmentRecentActionTypes.ASSIGNED,
'timestamp': self.carol_assignment.created.strftime('%Y-%m-%dT%H:%M:%S.%fZ'),
},
'learner_state': AssignmentLearnerStates.NOTIFYING,
},
],
}
Expand Down Expand Up @@ -420,11 +405,6 @@ def test_allocate_happy_path_e2e(
'transaction_uuid': None,
'uuid': str(new_allocation.uuid),
'actions': [],
'recent_action': {
'action_type': AssignmentRecentActionTypes.ASSIGNED,
'timestamp': new_allocation.created.strftime('%Y-%m-%dT%H:%M:%S.%fZ'),
},
'learner_state': AssignmentLearnerStates.NOTIFYING,
},
],
'no_change': [],
Expand Down
5 changes: 0 additions & 5 deletions enterprise_access/apps/api/v1/tests/test_assignment_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -593,11 +593,6 @@ def test_retrieve(self, role_context_dict):
}
for action in self.requester_assignment_accepted.actions.order_by('completed_at')
],
'recent_action': {
'action_type': AssignmentRecentActionTypes.ASSIGNED,
'timestamp': self.requester_assignment_accepted.created.strftime('%Y-%m-%dT%H:%M:%S.%fZ'),
},
'learner_state': None,
}

def test_retrieve_other_assignment_not_found(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ def get_queryset(self):
"""
A base queryset to list or retrieve ``LearnerContentAssignment`` records. In this viewset, only the assignments
assigned to the requester are returned.
Unlike in LearnerContentAssignmentAdminViewSet, here we are not going to annotate the extra dynamic fields using
`annotate_dynamic_fields_onto_queryset()`, so we will NOT serialize `learner_state` and `recent_action` for each
assignment.
"""
return LearnerContentAssignment.objects.filter(
learner_email=self.requesting_user_email,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class LearnerContentAssignmentAdminViewSet(
Viewset supporting all Admin CRUD operations on ``LearnerContentAssignment`` records.
"""
permission_classes = (permissions.IsAuthenticated,)
serializer_class = serializers.LearnerContentAssignmentResponseSerializer
serializer_class = serializers.LearnerContentAssignmentAdminResponseSerializer
authentication_classes = (JwtAuthentication, authentication.SessionAuthentication)
filter_backends = (filters.NoFilterOnDetailBackend, OrderingFilter)
filterset_class = filters.LearnerContentAssignmentAdminFilter
Expand Down Expand Up @@ -94,7 +94,7 @@ def get_queryset(self):
tags=[CONTENT_ASSIGNMENT_ADMIN_CRUD_API_TAG],
summary='Retrieve a content assignment by UUID.',
responses={
status.HTTP_200_OK: serializers.LearnerContentAssignmentResponseSerializer,
status.HTTP_200_OK: serializers.LearnerContentAssignmentAdminResponseSerializer,
status.HTTP_404_NOT_FOUND: None, # TODO: test that this actually returns 404 instead of 403 on RBAC error.
},
)
Expand All @@ -120,7 +120,7 @@ def list(self, request, *args, **kwargs):
tags=[CONTENT_ASSIGNMENT_ADMIN_CRUD_API_TAG],
summary='Cancel an assignment by UUID.',
responses={
status.HTTP_200_OK: serializers.LearnerContentAssignmentResponseSerializer,
status.HTTP_200_OK: serializers.LearnerContentAssignmentAdminResponseSerializer,
status.HTTP_404_NOT_FOUND: None,
status.HTTP_422_UNPROCESSABLE_ENTITY: None,
},
Expand Down Expand Up @@ -150,7 +150,7 @@ def cancel(self, request, *args, uuid=None, **kwargs):
# Serialize the assignment object obtained via get_queryset() instead of the one from the assignments_api.
# Only the former has the additional dynamic fields annotated, and those are required for serialization.
assignment_to_cancel.refresh_from_db()
response_serializer = serializers.LearnerContentAssignmentResponseSerializer(assignment_to_cancel)
response_serializer = serializers.LearnerContentAssignmentAdminResponseSerializer(assignment_to_cancel)
return Response(response_serializer.data, status=status.HTTP_200_OK)
else:
return Response(status=status.HTTP_422_UNPROCESSABLE_ENTITY)
63 changes: 1 addition & 62 deletions enterprise_access/apps/content_assignments/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -294,66 +294,6 @@ def add_successful_reminded_action(self):
completed_at=timezone.now(),
)

def get_recent_action_data(self):
"""
Return structured data about the most recent action, meant to feed the serializer.
Recent action can ONLY be one of:
* AssignmentRecentActionTypes.REMINDED: Most recent ``reminded`` action.
* AssignmentRecentActionTypes.ASSIGNED: Assignment record creation event, if no reminded actions exist.
Notes:
* These are not 1:1 with EITHER the AssignmentAction types, OR internal lifecycle states of assignments.
Instead it is a hybrid of both.
* This logic duplicates what is performed in annotate_dynamic_fields_onto_queryset() to annotate the
`recent_action` and `recent_action_time` fields on an assignment queryset. We rely on the serializer to
function even when the assignment object is not derived from a viewset's get_queryset().
Returns:
dict: {
'action_type': <one of AssignmentRecentActionTypes>,
'timestamp': <time of recent action>,
}
"""
reminded_actions = self.actions.filter(action_type=AssignmentActions.REMINDED)
if len(reminded_actions) > 0:
recent_action_type = AssignmentRecentActionTypes.REMINDED
recent_action_time = reminded_actions.order_by('completed_at').last().completed_at
else:
recent_action_type = AssignmentRecentActionTypes.ASSIGNED
recent_action_time = self.created
return {
'action_type': recent_action_type,
'timestamp': recent_action_time,
}

def get_learner_state(self):
"""
Returns the learner state of this assignment (not to be confused with state).
Notes:
* learner_state is not 1:1 with EITHER the AssignmentAction types, OR internal lifecycle states of assignments.
Instead it is a hybrid of both.
* This logic duplicates what is performed in annotate_dynamic_fields_onto_queryset() to annotate the
`learner_state` field on an assignment queryset. We rely on the serializer to function even when the
assignment object is not derived from a viewset's get_queryset().
Returns:
str: One of AssignmentLearnerStates, or None if the assignment doesn't map to one.
"""
has_notification = bool(LearnerContentAssignmentAction.objects.filter(
assignment=self,
action_type=AssignmentActions.NOTIFIED,
))
if self.state == LearnerContentAssignmentStateChoices.ALLOCATED and not has_notification:
return AssignmentLearnerStates.NOTIFYING
elif self.state == LearnerContentAssignmentStateChoices.ALLOCATED and has_notification:
return AssignmentLearnerStates.WAITING
elif self.state == LearnerContentAssignmentStateChoices.ERRORED and has_notification:
return AssignmentLearnerStates.FAILED
else:
return None

@classmethod
def annotate_dynamic_fields_onto_queryset(cls, queryset):
"""
Expand All @@ -366,8 +306,7 @@ def annotate_dynamic_fields_onto_queryset(cls, queryset):
* recent_action_time (DateTimeField)
Notes:
* This class method duplicates the logic found in instance methods get_learner_state() and
get_recent_action_data(), but using pure ORM queryset logic instead of in-memory calculations.
* In order to use LearnerContentAssignmentAdminResponseSerializer, you must call this method on the queryset.
Args:
queryset (QuerySet): LearnerContentAssignment queryset, vanilla.
Expand Down

0 comments on commit 2c2251a

Please sign in to comment.