feat: [assignments] backend support for "State" and "Recent action" columns #292

pwnage101 · 2023-10-12T00:04:20Z

Assignment serializer contains three additional output keys:

actions (list of dict): A list of all serialized actions for the assignment.
recent_action (dict): intended to power the "Recent action" column in the frontend display.
learner_state (str): intended to power the "Status" column in the frontend display.

Assignment list views are now sortable on two keys:

learner_state_sort_order: This sorts based on the internal sort order of dynamic field learner_state.
recent_action_time: This is a dynamic field defined as coalesce(most recent reminder, assignment creation)

Sample URLs

Listing assignments sorted by learner_state:

/api/v1/assignment-configurations/{assignment_configuration_uuid}/admin/assignments/?ordering=learner_state_sort_order
/api/v1/assignment-configurations/{assignment_configuration_uuid}/admin/assignments/?ordering=-learner_state_sort_order

Listing assignments sorted by recent_action_time:

/api/v1/assignment-configurations/{assignment_configuration_uuid}/admin/assignments/?ordering=recent_action_time
/api/v1/assignment-configurations/{assignment_configuration_uuid}/admin/assignments/?ordering=-recent_action_time

Sample Response Snippets

Serialized assignment:

{
    [...]
    'actions': [
        {
            'uuid': <action uuid>,
            'action_type': <one of AssignmentActions: ["learner_linked", "notified", "reminded"]>,
            'completed_at': <action completed_at timestamp>,
            'error_reason': <action error message>,
        },
        [...]
    ],
    'recent_action': {
        'action_type': <one of AssignmentRecentActionTypes: ["assigned", "reminded"]>,
        'timestamp': <timestamp of recent action>,
    },
    'learner_state': <one of AssignmentLearnerStates: ["notifying", "waiting", "failed"]>,
}

ENT-7780

Redoc preview:

pwnage101 · 2023-10-17T01:35:25Z

enterprise_access/apps/api/v1/views/content_assignments/assignments_admin.py

@@ -3,18 +3,23 @@
 """
 import logging

+from django.db.models import Case, Exists, F, Max, OuterRef, Prefetch, Q, Value, When


I think this one line exemplifies my concerns over the last week about frontend-backend design compatibility and tech debt. There's a time and place for ORM magic, this should not have been one of them.

pwnage101 · 2023-10-17T01:40:14Z

Right now this doesn't give the frontend an direct field to distinguish between "Notifying" and "Waiting" assignments, but it is sorted correctly. I'd like to address that in a separate PR tomorrow.

…olumns Assignment serializer contains three additional output keys: * `actions` (list of dict): A list of all serialized actions for the assignment. * `recent_action` (dict): intended to power the "Recent action" column in the frontend display. * `learner_state` (str): intended to power the "Status" column in the frontend display. Assignment list views are now sortable on two keys: * `learner_state_sort_order`: This sorts based on the internal sort order of dynamic field `learner_state`. * `recent_action_time`: This is a dynamic field defined as `coalesce(most recent reminder, assignment creation)` Sample URLs ----------- Listing assignments sorted by learner_state: ``` /api/v1/assignment-configurations/{assignment_configuration_uuid}/admin/assignments/?ordering=learner_state_sort_order /api/v1/assignment-configurations/{assignment_configuration_uuid}/admin/assignments/?ordering=-learner_state_sort_order ``` Listing assignments sorted by recent_action_time: ``` /api/v1/assignment-configurations/{assignment_configuration_uuid}/admin/assignments/?ordering=recent_action_time /api/v1/assignment-configurations/{assignment_configuration_uuid}/admin/assignments/?ordering=-recent_action_time ``` Sample Response Snippets ------------------------ Serialized assignment: ``` { [...] 'actions': [ { 'uuid': <action uuid>, 'action_type': <one of AssignmentActions: ["learner_linked", "notified", "reminded"]>, 'completed_at': <action completed_at timestamp>, 'error_reason': <action error message>, }, [...] ], 'recent_action': { 'action_type': <one of AssignmentRecentActionTypes: ["assigned", "reminded"]>, 'timestamp': <timestamp of recent action>, }, 'learner_state': <one of AssignmentLearnerStates: ["notifying", "waiting", "failed"]>, } ``` ENT-7780

iloveagent57

(Experimenting with big red button)
This looks great! Two questions to maybe help with DB query count.

iloveagent57 · 2023-10-18T14:09:06Z

enterprise_access/apps/api/serializers/content_assignments/assignment.py

+    )
+    recent_action = LearnerContentAssignmentRecentActionSerializer(
+        help_text='Structured data about the most recent action. Meant to power a frontend table column.',
+        source='get_recent_action_data',


Couldn't these use the annotated fields you added to the queryset? That would save a couple of queries per row of response data.

...or it's possible that an upstream select_related() would avoid the additional queries, and that using these functions as the source is fine.

I think adding the select_related is a good idea. It's not perfect, but it is potentially an easier path forward than fixing (2):

Something I maybe didn't fully communicate/document is the reason for duplicating the logic on both the python-layer and ORM-layer. I found that we use the LearnerContentAssignmentResponseSerializer on more than just objects that were fetched via LearnerContentAssignmentAdminViewSet.get_queryset(). It turns out we also use it indirectly on serializing policies from the policy viewset, as well as the django admin. Instead of spending more time figuring out how to annotate the assignment querysets in all of those cases (the policy viewset case baffles me the most), I decided to make the serializer less sensitive to whether or not the annotations got added.

iloveagent57 · 2023-10-18T14:11:26Z

enterprise_access/apps/api/v1/views/content_assignments/assignments_admin.py

@@ -62,14 +70,25 @@ def get_queryset(self):
        """
        A base queryset to list or retrieve ``LearnerContentAssignment`` records.
        """
+        queryset = LearnerContentAssignment.objects.all()


Should we throw a select_related('actions') on here?

I'd love to, but it's throwing this error:

E django.core.exceptions.FieldError: Invalid field name(s) given in select_related: 'actions'. Choices are: assignment_configuration

According to the docs, select_related only works in the reverse direction with OneToOneField: https://docs.djangoproject.com/en/4.2/ref/models/querysets/#select-related

iloveagent57 · 2023-10-18T14:29:51Z

enterprise_access/apps/content_assignments/models.py

+        Returns:
+            str: One of AssignmentLearnerStates, or None if the assignment doesn't map to one.
+        """
+        has_notification = bool(LearnerContentAssignmentAction.objects.filter(


nit: switch to self.actions.filter(action_type=AssignmentActions.NOTIFIED) to take advantage of any upstream select_relateds()

This gives the benefit of allowing us to remove duplicate code (in-memory calculations of recent_action + learner_state). ENT-7780

pwnage101 force-pushed the pwnage101/ENT-7780 branch 4 times, most recently from c09d31b to d46dee8 Compare October 13, 2023 03:43

pwnage101 marked this pull request as ready for review October 13, 2023 03:44

pwnage101 changed the title ~~feat: serialize 3 new fields for assignments~~ feat: [assignments] backend support for "State" and "Recent action" columns Oct 13, 2023

pwnage101 force-pushed the pwnage101/ENT-7780 branch 5 times, most recently from 532df68 to 7e9cb7e Compare October 17, 2023 01:28

pwnage101 commented Oct 17, 2023

View reviewed changes

pwnage101 force-pushed the pwnage101/ENT-7780 branch from 7e9cb7e to acede51 Compare October 17, 2023 01:37

pwnage101 force-pushed the pwnage101/ENT-7780 branch from acede51 to d19497e Compare October 17, 2023 23:42

pwnage101 force-pushed the pwnage101/ENT-7780 branch from d19497e to e5dda66 Compare October 17, 2023 23:44

iloveagent57 requested changes Oct 18, 2023

View reviewed changes

iloveagent57 reviewed Oct 18, 2023

View reviewed changes

iloveagent57 approved these changes Oct 18, 2023

View reviewed changes

pwnage101 force-pushed the pwnage101/ENT-7780 branch from d91ff33 to 2c2251a Compare October 18, 2023 16:59

refactor: split assignment serializer into admin vs. non-admin

8fcf408

This gives the benefit of allowing us to remove duplicate code (in-memory calculations of recent_action + learner_state). ENT-7780

pwnage101 force-pushed the pwnage101/ENT-7780 branch from 2c2251a to 8fcf408 Compare October 18, 2023 17:02

iloveagent57 approved these changes Oct 18, 2023

View reviewed changes

pwnage101 merged commit e7f7035 into main Oct 18, 2023

pwnage101 deleted the pwnage101/ENT-7780 branch October 18, 2023 17:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: [assignments] backend support for "State" and "Recent action" columns #292

feat: [assignments] backend support for "State" and "Recent action" columns #292

pwnage101 commented Oct 12, 2023 •

edited

Loading

pwnage101 Oct 17, 2023

pwnage101 commented Oct 17, 2023

iloveagent57 left a comment

iloveagent57 Oct 18, 2023

iloveagent57 Oct 18, 2023

pwnage101 Oct 18, 2023 •

edited

Loading

iloveagent57 Oct 18, 2023

pwnage101 Oct 18, 2023

iloveagent57 Oct 18, 2023

feat: [assignments] backend support for "State" and "Recent action" columns #292

feat: [assignments] backend support for "State" and "Recent action" columns #292

Conversation

pwnage101 commented Oct 12, 2023 • edited Loading

Sample URLs

Sample Response Snippets

pwnage101 Oct 17, 2023

Choose a reason for hiding this comment

pwnage101 commented Oct 17, 2023

iloveagent57 left a comment

Choose a reason for hiding this comment

iloveagent57 Oct 18, 2023

Choose a reason for hiding this comment

iloveagent57 Oct 18, 2023

Choose a reason for hiding this comment

pwnage101 Oct 18, 2023 • edited Loading

Choose a reason for hiding this comment

iloveagent57 Oct 18, 2023

Choose a reason for hiding this comment

pwnage101 Oct 18, 2023

Choose a reason for hiding this comment

iloveagent57 Oct 18, 2023

Choose a reason for hiding this comment

pwnage101 commented Oct 12, 2023 •

edited

Loading

pwnage101 Oct 18, 2023 •

edited

Loading