From 724529ca41ad0619d6b43dd65a5d130dcc9c369d Mon Sep 17 00:00:00 2001
From: Alexander J Sheehan <asheehan@2u.com>
Date: Thu, 28 Mar 2024 18:35:21 +0000
Subject: [PATCH] feat: allowing for sorting and filtering of the enterprise
 group learner endpoints

---
 CHANGELOG.rst                                 |   4 +
 enterprise/__init__.py                        |   2 +-
 enterprise/api/v1/serializers.py              |  29 ++--
 enterprise/api/v1/views/enterprise_group.py   |  37 +++-
 enterprise/constants.py                       |   9 +
 .../management/commands/manufacture_data.py   |  10 +-
 .../migrations/0206_auto_20240408_1344.py     |  33 ++++
 enterprise/models.py                          | 160 +++++++++++++++---
 enterprise/utils.py                           |   7 +
 tests/test_enterprise/api/test_views.py       | 152 ++++++++++++++++-
 tests/test_enterprise/test_signals.py         |   3 +
 11 files changed, 391 insertions(+), 55 deletions(-)
 create mode 100644 enterprise/migrations/0206_auto_20240408_1344.py

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index ab73f25b51..edef324f71 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -15,6 +15,10 @@ Change Log
 
 Unreleased
 ----------
+[4.15.1]
+--------
+* feat: allowing for sorting and filtering of the enterprise group learner endpoints
+
 [4.15.0]
 ---------
 * feat: Add new languages in enterprise customer admin
diff --git a/enterprise/__init__.py b/enterprise/__init__.py
index dff3a96b8f..bbd36b5529 100644
--- a/enterprise/__init__.py
+++ b/enterprise/__init__.py
@@ -2,4 +2,4 @@
 Your project description goes here.
 """
 
-__version__ = "4.15.0"
+__version__ = "4.15.1"
diff --git a/enterprise/api/v1/serializers.py b/enterprise/api/v1/serializers.py
index c95cbbce1a..a212b2278c 100644
--- a/enterprise/api/v1/serializers.py
+++ b/enterprise/api/v1/serializers.py
@@ -603,7 +603,7 @@ class EnterpriseGroupMembershipSerializer(serializers.ModelSerializer):
 
     member_details = serializers.SerializerMethodField()
     recent_action = serializers.SerializerMethodField()
-    member_status = serializers.SerializerMethodField()
+    status = serializers.CharField(required=False)
 
     class Meta:
         model = models.EnterpriseGroupMembership
@@ -613,7 +613,7 @@ class Meta:
             'enterprise_group_membership_uuid',
             'member_details',
             'recent_action',
-            'member_status',
+            'status',
         )
 
     def get_member_details(self, obj):
@@ -634,16 +634,6 @@ def get_recent_action(self, obj):
             return f"Accepted: {obj.activated_at.strftime('%B %d, %Y')}"
         return f"Invited: {obj.created.strftime('%B %d, %Y')}"
 
-    def get_member_status(self, obj):
-        """
-        Return the status related to the membership.
-        """
-        if obj.is_removed:
-            return "removed"
-        if obj.enterprise_customer_user:
-            return "accepted"
-        return "pending"
-
 
 class EnterpriseCustomerUserReadOnlySerializer(serializers.ModelSerializer):
     """
@@ -1700,3 +1690,18 @@ class Meta:
     client_secret = serializers.CharField(read_only=True, default=generate_client_secret())
     redirect_uris = serializers.CharField(required=False)
     updated = serializers.DateTimeField(required=False, read_only=True)
+
+
+class EnterpriseGroupLearnersRequestQuerySerializer(serializers.Serializer):
+    """
+    Serializer for the Enterprise Group Learners endpoint query filter
+    """
+    user_query = serializers.CharField(required=False, max_length=320)
+    sort_by = serializers.ChoiceField(
+        choices=[
+            ('member_details', 'member_details'),
+            ('status', 'status'),
+            ('recent_action', 'recent_action')
+        ],
+        required=False,
+    )
diff --git a/enterprise/api/v1/views/enterprise_group.py b/enterprise/api/v1/views/enterprise_group.py
index 951aace252..e867c19ff5 100644
--- a/enterprise/api/v1/views/enterprise_group.py
+++ b/enterprise/api/v1/views/enterprise_group.py
@@ -12,7 +12,7 @@
 from django.db.models import Q
 from django.http import Http404
 
-from enterprise import models, rules, utils
+from enterprise import constants, models, rules, utils
 from enterprise.api.utils import get_enterprise_customer_from_enterprise_group_id
 from enterprise.api.v1 import serializers
 from enterprise.api.v1.views.base_views import EnterpriseReadWriteModelViewSet
@@ -101,6 +101,13 @@ def get_learners(self, *args, **kwargs):
         Request Arguments:
         - ``group_uuid`` (URL location, required): The uuid of the group from which learners should be listed.
 
+        Optional query params:
+        - ``q`` (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.
+
         Returns: Paginated list of learners that are associated with the enterprise group uuid::
 
             {
@@ -117,11 +124,23 @@ def get_learners(self, *args, **kwargs):
             }
 
         """
+        query_params = self.request.query_params.copy()
+        is_reversed = bool(query_params.get('is_reversed', False))
+
+        param_serializers = serializers.EnterpriseGroupLearnersRequestQuerySerializer(
+            data=query_params
+        )
+
+        if not param_serializers.is_valid():
+            return Response(param_serializers.errors, status=400)
+
+        user_query = param_serializers.validated_data.get('user_query')
+        sort_by = param_serializers.validated_data.get('sort_by')
 
         group_uuid = kwargs.get('group_uuid')
         try:
             group_object = self.get_queryset().get(uuid=group_uuid)
-            members = group_object.get_all_learners()
+            members = group_object.get_all_learners(user_query, sort_by, desc_order=is_reversed)
             page = self.paginate_queryset(members)
             serializer = serializers.EnterpriseGroupMembershipSerializer(page, many=True)
             response = self.get_paginated_response(serializer.data)
@@ -197,6 +216,7 @@ def assign_learners(self, request, group_uuid):
                 ent_customer_users = [
                     models.EnterpriseGroupMembership(
                         activated_at=localized_utcnow(),
+                        status=constants.GROUP_MEMBERSHIP_ACCEPTED_STATUS,
                         enterprise_customer_user=ecu,
                         group=group
                     )
@@ -275,8 +295,15 @@ def remove_learners(self, request, group_uuid):
                 )
                 records_deleted += len(records_to_delete)
                 records_to_delete.delete()
-            data = {
-                'records_deleted': records_deleted,
-            }
+
+                # Woohoo! Records removed! Now to update the soft deleted records
+                deleted_records = models.EnterpriseGroupMembership.all_objects.filter(
+                    group_q & (ecu_in_q | pecu_in_q),
+                )
+                deleted_records.update(
+                    status=constants.GROUP_MEMBERSHIP_REMOVED_STATUS,
+                    removed_at=localized_utcnow()
+                )
+            data = {'records_deleted': records_deleted}
             return Response(data, status=200)
         return Response(data="Error: missing request data: `learner_emails`.", status=400)
diff --git a/enterprise/constants.py b/enterprise/constants.py
index 5efb678957..4d20fff79a 100644
--- a/enterprise/constants.py
+++ b/enterprise/constants.py
@@ -241,3 +241,12 @@ class FulfillmentTypes:
 
 # The maximum length of a text field in the database.
 MAX_ALLOWED_TEXT_LENGTH = 16_000_000
+
+GROUP_MEMBERSHIP_PENDING_STATUS = 'pending'
+GROUP_MEMBERSHIP_REMOVED_STATUS = 'removed'
+GROUP_MEMBERSHIP_ACCEPTED_STATUS = 'accepted'
+GROUP_MEMBERSHIP_STATUS_CHOICES = (
+    (GROUP_MEMBERSHIP_REMOVED_STATUS, 'Removed'),
+    (GROUP_MEMBERSHIP_ACCEPTED_STATUS, 'Accepted'),
+    (GROUP_MEMBERSHIP_PENDING_STATUS, 'Pending'),
+)
diff --git a/enterprise/management/commands/manufacture_data.py b/enterprise/management/commands/manufacture_data.py
index f1ce5e3ae8..7f922a4565 100644
--- a/enterprise/management/commands/manufacture_data.py
+++ b/enterprise/management/commands/manufacture_data.py
@@ -3,7 +3,6 @@
 """
 
 import logging
-import re
 import sys
 
 import factory
@@ -13,6 +12,8 @@
 from django.core.management.base import BaseCommand, CommandError, SystemCheckError, handle_default_options
 from django.db import connections
 
+from enterprise.utils import convert_to_snake
+
 # We have to import the enterprise test factories to ensure it's loaded and found by __subclasses__
 # To ensure factories outside of the enterprise package are loaded and found by the script,
 # add any additionally desired factories as an import to this file. Make sure to catch the ImportError
@@ -53,13 +54,6 @@ def all_subclasses(cls):
         [s for c in cls.__subclasses__() for s in all_subclasses(c)])
 
 
-def convert_to_snake(string):
-    """
-    Helper method to convert strings to snake case.
-    """
-    return re.sub(r'(?<!^)(?=[A-Z])', '_', string).lower()
-
-
 class Node():
     """
     Non-binary tree node class for building out a dependency tree of objects to create with customizations.
diff --git a/enterprise/migrations/0206_auto_20240408_1344.py b/enterprise/migrations/0206_auto_20240408_1344.py
new file mode 100644
index 0000000000..cca0a8c98a
--- /dev/null
+++ b/enterprise/migrations/0206_auto_20240408_1344.py
@@ -0,0 +1,33 @@
+# Generated by Django 3.2.23 on 2024-04-08 13:44
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('enterprise', '0205_auto_20240408_1117'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='enterprisegroupmembership',
+            name='removed_at',
+            field=models.DateTimeField(blank=True, default=None, help_text='The moment at which the membership record was revoked by an Enterprise admin.', null=True),
+        ),
+        migrations.AddField(
+            model_name='enterprisegroupmembership',
+            name='status',
+            field=models.CharField(blank=True, choices=[('removed', 'Removed'), ('accepted', 'Accepted'), ('pending', 'Pending')], default='pending', help_text='Current status of the membership record', max_length=20, null=True, verbose_name='Membership Status'),
+        ),
+        migrations.AddField(
+            model_name='historicalenterprisegroupmembership',
+            name='removed_at',
+            field=models.DateTimeField(blank=True, default=None, help_text='The moment at which the membership record was revoked by an Enterprise admin.', null=True),
+        ),
+        migrations.AddField(
+            model_name='historicalenterprisegroupmembership',
+            name='status',
+            field=models.CharField(blank=True, choices=[('removed', 'Removed'), ('accepted', 'Accepted'), ('pending', 'Pending')], default='pending', help_text='Current status of the membership record', max_length=20, null=True, verbose_name='Membership Status'),
+        ),
+    ]
diff --git a/enterprise/models.py b/enterprise/models.py
index 4acdd367bf..75157378ca 100644
--- a/enterprise/models.py
+++ b/enterprise/models.py
@@ -27,6 +27,7 @@
 from django.core.exceptions import NON_FIELD_ERRORS, ObjectDoesNotExist, ValidationError
 from django.core.validators import MaxValueValidator, MinValueValidator
 from django.db import IntegrityError, models, transaction
+from django.db.models import Q
 from django.template import Context, Template
 from django.urls import reverse
 from django.utils import timezone
@@ -49,6 +50,9 @@
     ALL_ACCESS_CONTEXT,
     AVAILABLE_LANGUAGES,
     ENTERPRISE_OPERATOR_ROLE,
+    GROUP_MEMBERSHIP_ACCEPTED_STATUS,
+    GROUP_MEMBERSHIP_PENDING_STATUS,
+    GROUP_MEMBERSHIP_STATUS_CHOICES,
     MAX_INVITE_KEYS,
     DefaultColors,
     FulfillmentTypes,
@@ -1502,7 +1506,8 @@ def fulfill_pending_group_memberships(self, enterprise_customer_user):
         self.memberships.update(
             activated_at=localized_utcnow(),
             pending_enterprise_customer_user=None,
-            enterprise_customer_user=enterprise_customer_user
+            enterprise_customer_user=enterprise_customer_user,
+            status=GROUP_MEMBERSHIP_ACCEPTED_STATUS
         )
 
     def fulfill_pending_course_enrollments(self, enterprise_customer_user):
@@ -4266,35 +4271,105 @@ class Meta:
         unique_together = (("name", "enterprise_customer"),)
         ordering = ['-modified']
 
-    def get_all_learners(self):
+    def _get_filtered_ecu_ids(self, user_query):
         """
-        Returns all users associated with the group, whether the group specifies the entire org else all associated
-        membership records.
+        Filter a group's enterprise customer user members by their User email.
         """
-        if self.applies_to_all_contexts:
-            members = []
+        # Unfortunately, user (and by extension email) is a property not a field on the ecu model. In order to
+        # filter ecu's by email,  we have to select from the Users table. To make this efficient, we join the user
+        # table on ecu table where the ecu is associated with the enterprise customer on the group. This limits how
+        # many User records we're calling `where email like %` on.
+        # Parameterizing the query makes this raw sql safe from sql injects re the Django docs-
+        # https://docs.djangoproject.com/en/5.0/topics/security/
+        var_q = f"%{user_query}%"
+        sql_string = """
+            select entcu.id from enterprise_enterprisecustomeruser entcu
+            join auth_user au on entcu.user_id = au.id
+            where entcu.enterprise_customer_id = %s and au.email like %s;
+        """
+        # Raw sql is picky about uuid format
+        customer_id = str(self.enterprise_customer.pk).replace("-", "")
+        ecus = EnterpriseCustomerUser.objects.raw(sql_string, (customer_id, var_q))
+        return [ecu.id for ecu in ecus]
+
+    def _get_implicit_group_members(self, user_query=None):
+        """
+        Fetches all implicit members of a group, indicated by a (pending) enterprise customer user records.
+        """
+        members = []
+        # Regardless of user_query, we will need all pecus related to the group's customer
+        pending_customer_users = PendingEnterpriseCustomerUser.objects.filter(
+            enterprise_customer=self.enterprise_customer,
+        )
+
+        if user_query:
+            # Get all ecus relevant to the user query
             customer_users = EnterpriseCustomerUser.objects.filter(
-                enterprise_customer=self.enterprise_customer,
-                active=True,
+                id__in=self._get_filtered_ecu_ids(user_query)
             )
-            pending_customer_users = PendingEnterpriseCustomerUser.objects.filter(
+            # pecu has user_email as a field, so we can filter directly
+            pending_customer_users = pending_customer_users.filter(user_email__icontains=user_query)
+        else:
+            # No filtering query so get all ecus related to the group's customer
+            customer_users = EnterpriseCustomerUser.objects.filter(
                 enterprise_customer=self.enterprise_customer,
+                active=True,
             )
-            for ent_user in customer_users:
-                members.append(EnterpriseGroupMembership(
-                    uuid=None,
-                    enterprise_customer_user=ent_user,
-                    group=self,
-                ))
-            for pending_user in pending_customer_users:
-                members.append(EnterpriseGroupMembership(
-                    uuid=None,
-                    pending_enterprise_customer_user=pending_user,
-                    group=self,
-                ))
-            return members
+        # Build an in memory array of all the implicit memberships
+        for ent_user in customer_users:
+            members.append(EnterpriseGroupMembership(
+                uuid=None,
+                enterprise_customer_user=ent_user,
+                group=self,
+            ))
+        for pending_user in pending_customer_users:
+            members.append(EnterpriseGroupMembership(
+                uuid=None,
+                pending_enterprise_customer_user=pending_user,
+                group=self,
+            ))
+        return members
+
+    def _get_explicit_group_members(self, user_query=None, fetch_removed=False):
+        """
+        Fetch explicitly defined members of a group, indicated by an existing membership record
+        """
+        members = self.members.select_related(
+            'enterprise_customer_user', 'pending_enterprise_customer_user'
+        )
+        if not fetch_removed:
+            members = members.filter(is_removed=False)
+        if user_query:
+            # filter the ecu's by joining the ecu table with the User table and selecting `where email like user_query`
+            ecu_filter = Q(enterprise_customer_user__id__in=self._get_filtered_ecu_ids(user_query))
+            # pecu has user_email as a field, so we can filter directly through the ORM with the user_query
+            pecu_filter = Q(pending_enterprise_customer_user__user_email__icontains=user_query)
+            members = members.filter(ecu_filter | pecu_filter)
+        return members
+
+    def get_all_learners(self, user_query=None, sort_by=None, desc_order=False, fetch_removed=False):
+        """
+        Returns all users associated with the group, whether the group specifies the entire org else all associated
+        membership records.
+
+        Params:
+            q (optional): filter the returned members list by user email and name with a provided sub-string
+            sort_by (optional): specify how the list of 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`.
+        """
+        if self.applies_to_all_contexts:
+            members = self._get_implicit_group_members(user_query)
         else:
-            return self.members.filter(is_removed=False)
+            members = self._get_explicit_group_members(user_query, fetch_removed)
+        if sort_by:
+            lambda_keys = {
+                'member_details': lambda t: t.member_email,
+                'status': lambda t: t.status,
+                'recent_action': lambda t: t.recent_action,
+            }
+            members = sorted(members, key=lambda_keys.get(sort_by), reverse=desc_order)
+        return members
 
 
 class EnterpriseGroupMembership(TimeStampedModel, SoftDeletableModel):
@@ -4333,6 +4408,23 @@ class EnterpriseGroupMembership(TimeStampedModel, SoftDeletableModel):
             "The moment at which the membership record is written with an Enterprise Customer User record."
         ),
     )
+    status = models.CharField(
+        verbose_name="Membership Status",
+        max_length=20,
+        blank=True,
+        null=True,
+        choices=GROUP_MEMBERSHIP_STATUS_CHOICES,
+        default=GROUP_MEMBERSHIP_PENDING_STATUS,
+        help_text=_("Current status of the membership record"),
+    )
+    removed_at = models.DateTimeField(
+        default=None,
+        blank=True,
+        null=True,
+        help_text=_(
+            "The moment at which the membership record was revoked by an Enterprise admin."
+        ),
+    )
     history = HistoricalRecords()
 
     class Meta:
@@ -4343,7 +4435,7 @@ class Meta:
         unique_together = (("group", "enterprise_customer_user"), ("group", "pending_enterprise_customer_user"))
         ordering = ['-modified']
 
-    @property
+    @cached_property
     def membership_user(self):
         """
         Return the user record associated with the membership, defaulting to ``enterprise_customer_user``
@@ -4351,6 +4443,26 @@ def membership_user(self):
         """
         return self.enterprise_customer_user or self.pending_enterprise_customer_user
 
+    @cached_property
+    def member_email(self):
+        """
+        Return the email associated with the member
+        """
+        if self.enterprise_customer_user:
+            return self.enterprise_customer_user.user_email
+        return self.pending_enterprise_customer_user.user_email
+
+    @cached_property
+    def recent_action(self):
+        """
+        Return the timestamp of the most recent action relating to the membership
+        """
+        if self.is_removed:
+            return self.removed_at
+        if self.enterprise_customer_user and self.activated_at:
+            return self.activated_at
+        return self.created
+
     def clean(self, *args, **kwargs):
         """
         Ensure that records added via Django Admin have matching customer records between learner and group.
diff --git a/enterprise/utils.py b/enterprise/utils.py
index f0f47d38b5..04334ad323 100644
--- a/enterprise/utils.py
+++ b/enterprise/utils.py
@@ -2394,3 +2394,10 @@ def truncate_string(string, max_length=MAX_ALLOWED_TEXT_LENGTH):
         was_truncated = True
         return (truncated_string, was_truncated)
     return (string, was_truncated)
+
+
+def convert_to_snake(string):
+    """
+    Helper method to convert strings to snake case.
+    """
+    return re.sub(r'(?<!^)(?=[A-Z])', '_', string).lower()
diff --git a/tests/test_enterprise/api/test_views.py b/tests/test_enterprise/api/test_views.py
index c008f715ba..b868d6d4c5 100644
--- a/tests/test_enterprise/api/test_views.py
+++ b/tests/test_enterprise/api/test_views.py
@@ -7402,9 +7402,132 @@ def test_list_learner_statuses(self):
         )
         response = self.client.get(url)
         assert response.json().get('count') == 2
-        statuses = [result.get('member_status') for result in response.json().get('results')]
+        statuses = [result.get('status') for result in response.json().get('results')]
         assert statuses.sort() == ['accepted', 'pending'].sort()
 
+    def test_list_learners_bad_sort_by(self):
+        """
+        Test that the list learners endpoint properly validates sort by query params
+        """
+        url = settings.TEST_SERVER + reverse(
+            'enterprise-group-learners',
+            kwargs={'group_uuid': self.group_1.uuid},
+        ) + "?sort_by=ayylmao"
+
+        response = self.client.get(url)
+        assert response.status_code == 400
+        assert response.data.get('sort_by')
+
+        long_query_string = "foo".join('bar' for _ in range(320))
+        url = settings.TEST_SERVER + reverse(
+            'enterprise-group-learners',
+            kwargs={'group_uuid': self.group_1.uuid},
+        ) + f"?user_query={long_query_string}"
+        response = self.client.get(url)
+        assert response.status_code == 400
+        assert response.data.get('user_query')
+
+    def test_list_learners_filtered(self):
+        """
+        Test that the list learners endpoint can be filtered by user details
+        """
+        group = EnterpriseGroupFactory(
+            enterprise_customer=self.enterprise_customer,
+            applies_to_all_contexts=True,
+        )
+        pending_user = PendingEnterpriseCustomerUserFactory(
+            user_email="foobar@example.com",
+            enterprise_customer=self.enterprise_customer,
+        )
+        pending_user_query_string = f'?user_query={pending_user.user_email}'
+        url = settings.TEST_SERVER + reverse(
+            'enterprise-group-learners',
+            kwargs={'group_uuid': group.uuid},
+        ) + pending_user_query_string
+        response = self.client.get(url)
+
+        assert response.json().get('count') == 1
+        assert response.json().get('results')[0].get('pending_learner_id') == pending_user.id
+
+        group.applies_to_all_contexts = False
+        group.save()
+        pending_membership = EnterpriseGroupMembershipFactory(
+            group=group,
+            pending_enterprise_customer_user=pending_user,
+            enterprise_customer_user=None,
+        )
+        existing_membership = EnterpriseGroupMembershipFactory(
+            group=group,
+            pending_enterprise_customer_user=None,
+            enterprise_customer_user__enterprise_customer=self.enterprise_customer,
+        )
+        existing_user = existing_membership.enterprise_customer_user.user
+        # Changing email to something that we know will be unique for collision purposes
+        existing_user.email = "ayylmao@example.com"
+        existing_user.save()
+        existing_user_query_string = '?user_query=ayylmao'
+        url = settings.TEST_SERVER + reverse(
+            'enterprise-group-learners',
+            kwargs={'group_uuid': group.uuid},
+        ) + existing_user_query_string
+        response = self.client.get(url)
+
+        assert response.json().get('count') == 1
+        assert response.json().get('results')[0].get(
+            'learner_id'
+        ) == existing_membership.enterprise_customer_user.id
+
+        url = settings.TEST_SERVER + reverse(
+            'enterprise-group-learners',
+            kwargs={'group_uuid': group.uuid},
+        ) + pending_user_query_string
+
+        response = self.client.get(url)
+
+        assert response.json().get('count') == 1
+        assert response.json().get('results')[0].get(
+            'pending_learner_id'
+        ) == pending_membership.pending_enterprise_customer_user.id
+
+    def test_list_learners_sort_by(self):
+        """
+        Test that the list learners endpoint can be sorted by 'recentAction', 'status', 'memberDetails'
+        values respectively
+        """
+        # Test sorting by the three sortable values
+        for sort_by_value in ['recent_action', 'status', 'member_details']:
+            url = settings.TEST_SERVER + reverse(
+                'enterprise-group-learners',
+                kwargs={'group_uuid': self.group_1.uuid},
+            ) + f"?sort_by={sort_by_value}"
+
+            response = self.client.get(url)
+            results = response.json().get('results')
+
+            returned_sorted_values = [result.get(sort_by_value) for result in results]
+            # Member details are returned as a dictionary, and is sorted by the user email value as it's guaranteed
+            # to exist in the object
+            if sort_by_value == 'member_details':
+                assert returned_sorted_values == sorted(returned_sorted_values, key=lambda t: t.get('user_email'))
+            else:
+                assert returned_sorted_values == sorted(returned_sorted_values)
+
+        # Test sorting in reverse order
+        url = settings.TEST_SERVER + reverse(
+            'enterprise-group-learners',
+            kwargs={'group_uuid': self.group_1.uuid},
+        ) + "?sort_by=member_details&is_reversed=True"
+        response = self.client.get(url)
+        results = response.json().get('results')
+        returned_sorted_values = [
+            value.get('member_details') for value in results
+        ]
+        assert returned_sorted_values == sorted(
+            returned_sorted_values,
+            key=lambda t: t.get('user_email'),
+            reverse=True,
+        )
+
     def test_successful_list_learners(self):
         """
         Test a successful GET request to the list endpoint.
@@ -7423,11 +7546,11 @@ def test_successful_list_learners(self):
                     'pending_learner_id': None,
                     'enterprise_group_membership_uuid': str(self.enterprise_group_memberships[i].uuid),
                     'member_details': {
+                        'user_email': member_user.user_email,
                         'user_name': member_user.name,
-                        'user_email': member_user.user_email
                     },
                     'recent_action': f'Accepted: {datetime.now().strftime("%B %d, %Y")}',
-                    'member_status': 'accepted',
+                    'status': 'pending',
                 },
             )
         expected_response = {
@@ -7455,11 +7578,11 @@ def test_successful_list_learners(self):
                     'pending_learner_id': None,
                     'enterprise_group_membership_uuid': str(self.enterprise_group_memberships[0].uuid),
                     'member_details': {
+                        'user_email': user.user_email,
                         'user_name': user.name,
-                        'user_email': user.user_email
                     },
                     'recent_action': f'Accepted: {datetime.now().strftime("%B %d, %Y")}',
-                    'member_status': 'accepted',
+                    'status': 'pending',
                 }
             ],
         }
@@ -7516,6 +7639,23 @@ def test_successful_list_with_filters(self):
         assert len(enterprise_filtered_response.json().get('results')) == 1
         assert learner_filtered_response.json().get('results')[0].get('uuid') == str(new_group.uuid)
 
+    def test_list_members_little_bobby_tables(self):
+        """
+        Test that we properly sanitize member user query filters
+        https://xkcd.com/327/
+        """
+        # url: 'http://testserver/enterprise/api/v1/enterprise_group/<group uuid>/learners/'
+        url = settings.TEST_SERVER + reverse(
+            'enterprise-group-learners',
+            kwargs={'group_uuid': self.group_1.uuid},
+        )
+        # The problematic child
+        filter_query_param = "?user_query=Robert`); DROP TABLE enterprise_enterprisecustomeruser;--"
+        sql_injection_protected_response = self.client.get(url + filter_query_param)
+        assert sql_injection_protected_response.status_code == 200
+        assert not sql_injection_protected_response.json().get('results')
+        assert EnterpriseCustomerUser.objects.all()
+
     def test_successful_post_group(self):
         """
         Test creating a new group record
@@ -7701,6 +7841,8 @@ def test_successful_remove_learners_from_group(self):
         assert response.status_code == 200
         assert response.data == {'records_deleted': 10}
         for membership in memberships_to_delete:
+            assert EnterpriseGroupMembership.all_objects.get(pk=membership.pk).status == 'removed'
+            assert EnterpriseGroupMembership.all_objects.get(pk=membership.pk).removed_at
             with self.assertRaises(EnterpriseGroupMembership.DoesNotExist):
                 EnterpriseGroupMembership.objects.get(pk=membership.pk)
 
diff --git a/tests/test_enterprise/test_signals.py b/tests/test_enterprise/test_signals.py
index ef5e6c47ff..e26ae4aa55 100644
--- a/tests/test_enterprise/test_signals.py
+++ b/tests/test_enterprise/test_signals.py
@@ -261,6 +261,8 @@ def test_handle_user_post_save_fulfills_pending_group_memberships(self):
             enterprise_customer_user=None
         )
         assert not new_membership.activated_at
+        assert new_membership.status == 'pending'
+
         parameters = {"instance": user, "created": False}
         handle_user_post_save(mock.Mock(), **parameters)
         # Should delete pending link
@@ -272,6 +274,7 @@ def test_handle_user_post_save_fulfills_pending_group_memberships(self):
         assert membership.pending_enterprise_customer_user is None
         assert membership.enterprise_customer_user == new_enterprise_user
         assert membership.activated_at
+        assert membership.status == 'accepted'
 
 
 @mark.django_db