From d49f762a5baf7074fe715d91eecdc7f78541706d Mon Sep 17 00:00:00 2001 From: Alexander J Sheehan Date: Fri, 3 May 2024 17:36:24 +0000 Subject: [PATCH] fix: hard deleting expired group memberships --- CHANGELOG.rst | 4 ++++ enterprise/__init__.py | 2 +- enterprise/api/v1/serializers.py | 7 +++++++ enterprise/api/v1/views/enterprise_group.py | 14 ++++++++------ .../remove_expired_pending_group_memberships.py | 9 ++++++--- enterprise/models.py | 4 ++-- ...est_remove_expired_pending_group_memberships.py | 2 +- 7 files changed, 29 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index cc909f0623..acf6167721 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -17,6 +17,10 @@ Unreleased ---------- * nothing unreleased +[4.17.5] +-------- +* fix: hard deleting expired group memberships + [4.17.4] -------- * fix: update language cookie if langauge cookie is not same as user's language preference diff --git a/enterprise/__init__.py b/enterprise/__init__.py index ace7fdb00a..9c598a0fc6 100644 --- a/enterprise/__init__.py +++ b/enterprise/__init__.py @@ -2,4 +2,4 @@ Your project description goes here. """ -__version__ = "4.17.4" +__version__ = "4.17.5" diff --git a/enterprise/api/v1/serializers.py b/enterprise/api/v1/serializers.py index 552be6a3c9..ad83608b71 100644 --- a/enterprise/api/v1/serializers.py +++ b/enterprise/api/v1/serializers.py @@ -1732,3 +1732,10 @@ class EnterpriseGroupLearnersRequestQuerySerializer(serializers.Serializer): required=False, ) pending_users_only = serializers.BooleanField(required=False, default=False) + show_removed = serializers.BooleanField(required=False, default=False) + is_reversed = serializers.BooleanField(required=False, default=False) + page = serializers.IntegerField(required=False) + lms_users = serializers.ListField( + child=serializers.IntegerField(required=True), + required=False, + ) diff --git a/enterprise/api/v1/views/enterprise_group.py b/enterprise/api/v1/views/enterprise_group.py index b28a55ac0e..70e27e70a3 100644 --- a/enterprise/api/v1/views/enterprise_group.py +++ b/enterprise/api/v1/views/enterprise_group.py @@ -107,12 +107,14 @@ def get_learners(self, request, *args, **kwargs): Optional query params: - ``pending_users_only`` (string, optional): Specify that results should only contain pending learners - - ``q`` (string, optional): Filter the returned members by user email and name with a provided sub-string + - ``user_query`` (string, optional): Filter the returned members by user email and name with a provided + sub-string - ``sort_by`` (string, optional): Specify how the returned members should be ordered. Supported sorting values are `memberDetails`, `memberStatus`, and `recentAction`. Ordering can be reversed by supplying a `-` at the beginning of the sorting value ie `-memberStatus`. - ``page`` (int, optional): Which page of paginated data to return. - ``show_removed`` (bool, optional): Include removed learners in the response. + - ``is_reversed`` (bool, optional): Include to reverse the order of returned records. Returns: Paginated list of learners that are associated with the enterprise group uuid:: @@ -136,10 +138,7 @@ def get_learners(self, request, *args, **kwargs): } """ - query_params = self.request.query_params.copy() - is_reversed = bool(query_params.get('is_reversed', False)) - show_removed = bool(query_params.get('show_removed', False)) - + query_params = self.request.query_params param_serializers = serializers.EnterpriseGroupLearnersRequestQuerySerializer( data=query_params ) @@ -148,8 +147,11 @@ def get_learners(self, request, *args, **kwargs): return Response(param_serializers.errors, status=400) user_query = param_serializers.validated_data.get('user_query') + show_removed = param_serializers.validated_data.get('show_removed', False) + is_reversed = param_serializers.validated_data.get('is_reversed', False) + sort_by = param_serializers.validated_data.get('sort_by') - pending_users_only = param_serializers.validated_data.get('pending_users_only') + pending_users_only = param_serializers.validated_data.get('pending_users_only', False) group_uuid = kwargs.get('group_uuid') try: diff --git a/enterprise/management/commands/remove_expired_pending_group_memberships.py b/enterprise/management/commands/remove_expired_pending_group_memberships.py index ffbaee5098..1fdf9e79ef 100644 --- a/enterprise/management/commands/remove_expired_pending_group_memberships.py +++ b/enterprise/management/commands/remove_expired_pending_group_memberships.py @@ -29,7 +29,7 @@ def add_arguments(self, parser): parser.add_argument("-e", "--enterprise_customer") def handle(self, *args, **options): - queryset = EnterpriseGroupMembership.objects.all() + queryset = EnterpriseGroupMembership.all_objects.all() if enterprise_arg := options.get("enterprise_customer"): try: enterprise_customer = EnterpriseCustomer.objects.get(uuid=enterprise_arg) @@ -43,5 +43,8 @@ def handle(self, *args, **options): created__lte=localized_utcnow() - timedelta(days=90) ) for membership in expired_memberships: - membership.pending_enterprise_customer_user.delete() - expired_memberships.delete() + pecu_to_delete = membership.pending_enterprise_customer_user + pecu_to_delete.delete() + membership.refresh_from_db() + # https://github.com/jazzband/django-model-utils/blob/master/model_utils/models.py#L133-L158 + membership.delete(soft=False) diff --git a/enterprise/models.py b/enterprise/models.py index fffff4a64b..8272f89e13 100644 --- a/enterprise/models.py +++ b/enterprise/models.py @@ -4402,14 +4402,14 @@ class EnterpriseGroupMembership(TimeStampedModel, SoftDeletableModel): blank=True, null=True, related_name='memberships', - on_delete=models.deletion.CASCADE, + on_delete=models.deletion.SET_NULL, ) pending_enterprise_customer_user = models.ForeignKey( PendingEnterpriseCustomerUser, blank=True, null=True, related_name='memberships', - on_delete=models.deletion.CASCADE, + on_delete=models.deletion.SET_NULL, ) activated_at = models.DateTimeField( default=None, diff --git a/tests/test_enterprise/management/test_remove_expired_pending_group_memberships.py b/tests/test_enterprise/management/test_remove_expired_pending_group_memberships.py index e414875108..ac79035b03 100644 --- a/tests/test_enterprise/management/test_remove_expired_pending_group_memberships.py +++ b/tests/test_enterprise/management/test_remove_expired_pending_group_memberships.py @@ -93,7 +93,7 @@ def test_removing_old_records(self): call_command(self.command) - # Assert that memberships and pending customers are removed + # Assert that pending memberships and pending customers are removed assert not models.EnterpriseGroupMembership.all_objects.filter(pk=membership_to_remove.pk) assert not models.PendingEnterpriseCustomerUser.objects.filter( pk=membership_to_remove.pending_enterprise_customer_user.pk