diff --git a/enterprise_access/apps/api/serializers/content_assignments/assignment.py b/enterprise_access/apps/api/serializers/content_assignments/assignment.py index 83e6850c..f998443c 100644 --- a/enterprise_access/apps/api/serializers/content_assignments/assignment.py +++ b/enterprise_access/apps/api/serializers/content_assignments/assignment.py @@ -24,6 +24,8 @@ class LearnerContentAssignmentActionSerializer(serializers.ModelSerializer): class Meta: model = LearnerContentAssignmentAction fields = [ + 'created', + 'modified', 'uuid', 'action_type', 'completed_at', @@ -100,10 +102,35 @@ class LearnerContentAssignmentAdminResponseSerializer(LearnerContentAssignmentRe ), choices=AssignmentLearnerStates.CHOICES, ) + error_reason = serializers.SerializerMethodField() class Meta(LearnerContentAssignmentResponseSerializer.Meta): fields = LearnerContentAssignmentResponseSerializer.Meta.fields + [ 'recent_action', 'learner_state', + 'error_reason', ] read_only_fields = fields + + @extend_schema_field(serializers.CharField) + def get_error_reason(self, assignment): + """ + Resolves the error reason for the assignment, if any, for display purposes based on + any associated actions. + """ + # If the assignment is not in an errored state, there should be no error reason. + if assignment.state != LearnerContentAssignmentStateChoices.ERRORED: + return None + + # Assignment is an errored state, so determine the appropriate error reason so clients don't need to. + related_actions_with_error = assignment.actions.filter(error_reason__isnull=False).order_by('-created') + if not related_actions_with_error: + logger.warning( + 'LearnerContentAssignment with UUID %s is in an errored state, but has no related ' + 'actions in an error state.', + assignment.uuid, + ) + return None + + # Get the most recently errored action. + return related_actions_with_error.first().error_reason diff --git a/enterprise_access/apps/api/v1/tests/test_allocation_view.py b/enterprise_access/apps/api/v1/tests/test_allocation_view.py index 8aa27aba..20a05034 100644 --- a/enterprise_access/apps/api/v1/tests/test_allocation_view.py +++ b/enterprise_access/apps/api/v1/tests/test_allocation_view.py @@ -6,7 +6,6 @@ import ddt from django.core.cache import cache as django_cache -from django.core.exceptions import ValidationError from rest_framework import status from rest_framework.reverse import reverse from rest_framework.serializers import ValidationError diff --git a/enterprise_access/apps/api/v1/tests/test_assignment_views.py b/enterprise_access/apps/api/v1/tests/test_assignment_views.py index 3619f009..eb606168 100644 --- a/enterprise_access/apps/api/v1/tests/test_assignment_views.py +++ b/enterprise_access/apps/api/v1/tests/test_assignment_views.py @@ -342,13 +342,16 @@ def test_retrieve(self, role_context_dict): 'transaction_uuid': self.assignment_allocated_pre_link.transaction_uuid, 'actions': [ { + 'created': action.created.strftime('%Y-%m-%dT%H:%M:%S.%fZ'), + 'modified': action.modified.strftime('%Y-%m-%dT%H:%M:%S.%fZ'), 'uuid': str(action.uuid), 'action_type': action.action_type, 'completed_at': action.completed_at.strftime('%Y-%m-%dT%H:%M:%S.%fZ'), - 'error_reason': None, + 'error_reason': action.error_reason, } - for action in self.assignment_allocated_pre_link.actions.order_by('completed_at') + for action in self.assignment_allocated_pre_link.actions.order_by('created') ], + 'error_reason': None, 'recent_action': { 'action_type': AssignmentRecentActionTypes.ASSIGNED, 'timestamp': self.assignment_allocated_pre_link.created.strftime('%Y-%m-%dT%H:%M:%S.%fZ'), @@ -588,12 +591,14 @@ def test_retrieve(self, role_context_dict): 'transaction_uuid': str(self.requester_assignment_accepted.transaction_uuid), 'actions': [ { + 'created': action.created.strftime('%Y-%m-%dT%H:%M:%S.%fZ'), + 'modified': action.modified.strftime('%Y-%m-%dT%H:%M:%S.%fZ'), 'uuid': str(action.uuid), 'action_type': action.action_type, - 'completed_at': str(action.completed_at.strftime('%Y-%m-%dT%H:%M:%S.%fZ')), - 'error_reason': None, + 'completed_at': action.completed_at.strftime('%Y-%m-%dT%H:%M:%S.%fZ'), + 'error_reason': action.error_reason, } - for action in self.requester_assignment_accepted.actions.order_by('completed_at') + for action in self.requester_assignment_accepted.actions.order_by('created') ], } diff --git a/enterprise_access/apps/content_assignments/migrations/0010_alter_learnercontentassignmentaction_assignment.py b/enterprise_access/apps/content_assignments/migrations/0010_alter_learnercontentassignmentaction_assignment.py new file mode 100644 index 00000000..de4deb85 --- /dev/null +++ b/enterprise_access/apps/content_assignments/migrations/0010_alter_learnercontentassignmentaction_assignment.py @@ -0,0 +1,34 @@ +# Generated by Django 3.2.21 on 2023-11-01 20:35 + +from django.db import migrations, models +import django.db.models.deletion + + +def delete_actions_without_assignment(apps, schema_editor): + """ + Deletes Actions without Assignments. This is a workaround for a bug where at least one Assignment was deleted + before the associated Actions were, resulting in Django Admin for Actions throwing an error. + + By running a data migration to delete the Actions without an associated Assignment, this error should be avoided. + + This issue will be prevented in the future by also setting `on_delete=CASCADE` such that when an Assignment is + deleted, its associated Actions will be deleted as well. + """ + LearnerContentAssignmentAction = apps.get_model('content_assignments', 'LearnerContentAssignmentAction') + LearnerContentAssignmentAction.objects.filter(assignment__isnull=True).delete() + + +class Migration(migrations.Migration): + + dependencies = [ + ('content_assignments', '0009_historicallearnercontentassignment_content_title_and_more'), + ] + + operations = [ + migrations.RunPython(delete_actions_without_assignment, migrations.RunPython.noop), + migrations.AlterField( + model_name='learnercontentassignmentaction', + name='assignment', + field=models.ForeignKey(help_text='The LearnerContentAssignment on which this action was performed.', on_delete=django.db.models.deletion.CASCADE, related_name='actions', to='content_assignments.learnercontentassignment'), + ), + ] diff --git a/enterprise_access/apps/content_assignments/models.py b/enterprise_access/apps/content_assignments/models.py index 7e9cd3fa..a5b63b99 100644 --- a/enterprise_access/apps/content_assignments/models.py +++ b/enterprise_access/apps/content_assignments/models.py @@ -323,8 +323,8 @@ def annotate_dynamic_fields_onto_queryset(cls, queryset): QuerySet: LearnerContentAssignment queryset, same objects but with extra fields annotated. """ # Annotate a derived field ``recent_action_time`` using pure ORM so that we can order_by() it later. - # ``recent_action_time`` is defined as the time of the most recent reminder, and falls back to assignment - # creation time if there are no reminders. + # ``recent_action_time`` is defined as the time of the most recent successful reminder, and falls + # back to assignment creation time if there are no successful reminders. new_queryset = queryset.annotate( recent_action_time=Coalesce( # Time of most recent reminder. @@ -342,6 +342,8 @@ def annotate_dynamic_fields_onto_queryset(cls, queryset): LearnerContentAssignmentAction.objects.filter( assignment=OuterRef('uuid'), action_type=AssignmentActions.REMINDED, + error_reason__isnull=True, + completed_at__isnull=False, ) ) ).annotate( @@ -361,6 +363,8 @@ def annotate_dynamic_fields_onto_queryset(cls, queryset): LearnerContentAssignmentAction.objects.filter( assignment=OuterRef('uuid'), action_type=AssignmentActions.NOTIFIED, + error_reason__isnull=True, + completed_at__isnull=False, ) ) ).annotate( @@ -418,8 +422,7 @@ class LearnerContentAssignmentAction(TimeStampedModel): assignment = models.ForeignKey( LearnerContentAssignment, related_name="actions", - on_delete=models.SET_NULL, - null=True, + on_delete=models.CASCADE, help_text="The LearnerContentAssignment on which this action was performed.", ) action_type = models.CharField( @@ -452,6 +455,9 @@ class LearnerContentAssignmentAction(TimeStampedModel): history = HistoricalRecords() + class Meta: + ordering = ['created'] + def __str__(self): return ( f'uuid={self.uuid}, action_type={self.action_type}, error_reason={self.error_reason}' diff --git a/enterprise_access/apps/subsidy_access_policy/migrations/0018_alter_subsidyaccesspolicy_assignment_configuration.py b/enterprise_access/apps/subsidy_access_policy/migrations/0018_alter_subsidyaccesspolicy_assignment_configuration.py new file mode 100644 index 00000000..81e5be8b --- /dev/null +++ b/enterprise_access/apps/subsidy_access_policy/migrations/0018_alter_subsidyaccesspolicy_assignment_configuration.py @@ -0,0 +1,20 @@ +# Generated by Django 3.2.21 on 2023-11-01 20:35 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('content_assignments', '0010_alter_learnercontentassignmentaction_assignment'), + ('subsidy_access_policy', '0017_active_assignment_config_unique'), + ] + + operations = [ + migrations.AlterField( + model_name='subsidyaccesspolicy', + name='assignment_configuration', + field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='subsidy_access_policy', to='content_assignments.assignmentconfiguration'), + ), + ] diff --git a/enterprise_access/apps/subsidy_access_policy/models.py b/enterprise_access/apps/subsidy_access_policy/models.py index a4fe7a7a..d8b5893d 100644 --- a/enterprise_access/apps/subsidy_access_policy/models.py +++ b/enterprise_access/apps/subsidy_access_policy/models.py @@ -274,7 +274,7 @@ def __new__(cls, *args, **kwargs): except Exception: # pylint: disable=broad-except pass - return super().__new__(proxy_class) # pylint: disable=lost-exception + return super().__new__(proxy_class) def subsidy_record(self): """ diff --git a/enterprise_access/settings/devstack.py b/enterprise_access/settings/devstack.py index f26b66ca..e1d9b6bb 100644 --- a/enterprise_access/settings/devstack.py +++ b/enterprise_access/settings/devstack.py @@ -74,12 +74,18 @@ # CORS CONFIG CORS_ORIGIN_WHITELIST = [ - 'http://localhost:1991', # frontend-admin-portal + 'http://localhost:1991', # frontend-app-admin-portal 'http://localhost:8734', # frontend-app-learner-portal-enterprise 'http://localhost:18450', # frontend-app-support-tools ] # END CORS +# CSRF CONFIG +CSRF_TRUSTED_ORIGINS = [ + 'http://localhost:1991', # frontend-app-admin-portal +] +# END CSRF CONFIG + ECOMMERCE_URL = 'http://edx.devstack.ecommerce:18130' LICENSE_MANAGER_URL = 'http://license-manager.app:18170' LMS_URL = 'http://edx.devstack.lms:18000'