Skip to content

Commit

Permalink
fix: case-insensitive email matching for existing assignments
Browse files Browse the repository at this point in the history
ENT-8129 | Also do filters for existing assignments against
a batch of emails to allocate in a case-insensitive manner.
  • Loading branch information
iloveagent57 committed Dec 12, 2023
1 parent bc30d18 commit 513235c
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 12 deletions.
35 changes: 25 additions & 10 deletions enterprise_access/apps/content_assignments/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,19 @@ class AllocationException(Exception):
user_message = 'An error occurred during allocation'


def _inexact_email_filter(emails, field_name='email'):
"""
Helper that produces a Django Queryset filter
to query for records by an ``email`` feel
in a case-insensitive way.
"""
email_filter = Q()
for email in emails:
kwargs = {f'{field_name}__iexact': email}
email_filter |= Q(**kwargs)
return email_filter


def create_assignment_configuration(enterprise_customer_uuid, **kwargs):
"""
Create a new ``AssignmentConfiguration`` for the given customer identifier.
Expand All @@ -67,6 +80,7 @@ def get_assignment_configuration(uuid):

def get_assignments_for_configuration(
assignment_configuration,
*args,
**additional_filters,
):
"""
Expand All @@ -77,6 +91,7 @@ def get_assignments_for_configuration(
queryset = LearnerContentAssignment.objects.select_related(
'assignment_configuration',
).filter(
*args,
assignment_configuration=assignment_configuration,
**additional_filters,
)
Expand Down Expand Up @@ -113,7 +128,7 @@ def get_assignments_for_admin(
"""
return get_assignments_for_configuration(
assignment_configuration,
learner_email__in=learner_emails,
_inexact_email_filter(learner_emails, field_name='learner_email'),
content_key=content_key,
)

Expand Down Expand Up @@ -270,7 +285,7 @@ def allocate_assignments(assignment_configuration, learner_emails, content_key,

# Split up the existing assignment records by state
for assignment in existing_assignments:
learner_emails_with_existing_assignments.add(assignment.learner_email)
learner_emails_with_existing_assignments.add(assignment.learner_email.lower())
if assignment.state in LearnerContentAssignmentStateChoices.REALLOCATE_STATES:
assignment.content_quantity = content_quantity
assignment.state = LearnerContentAssignmentStateChoices.ALLOCATED
Expand All @@ -297,9 +312,10 @@ def allocate_assignments(assignment_configuration, learner_emails, content_key,
)

# Narrow down creation list of learner emails
learner_emails_for_assignment_creation = (
set(learner_emails_to_allocate) - learner_emails_with_existing_assignments
)
learner_emails_for_assignment_creation = {
email for email in learner_emails_to_allocate
if email.lower() not in learner_emails_with_existing_assignments
}

# Initialize and save LearnerContentAssignment instances for each of them
created_assignments = _create_new_assignments(
Expand Down Expand Up @@ -396,11 +412,10 @@ def _try_populate_assignments_lms_user_id(assignments):
# this is the part that could exceed max statement length if batch size is too large.
# There's no case-insensitive IN query in Django, so we have to build up a big
# OR type of query.
email_filter = Q()
for assignment_email in emails:
email_filter |= Q(email__iexact=assignment_email)

email_lms_user_id = User.objects.filter(email_filter, lms_user_id__isnull=False).annotate(
email_lms_user_id = User.objects.filter(
_inexact_email_filter(emails, field_name='email'),
lms_user_id__isnull=False,
).annotate(
email_lower=Lower('email'),
).values_list('email_lower', 'lms_user_id')

Expand Down
6 changes: 4 additions & 2 deletions enterprise_access/apps/content_assignments/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,10 @@ def test_allocate_assignments_happy_path(
content_price_cents = 100
# add a duplicate email to the input list to ensure only one
# allocation occurs.
# We throw a couple ALL UPPER CASE emails into the requested emails to allocate
# to verify that we filter for pre-existing assignments in a case-insensitive manner.
learners_to_assign = [
f'{name}@foo.com' for name in ('alice', 'bob', 'carol', 'david', 'eugene', 'eugene', 'bob', 'eugene')
f'{name}@foo.com' for name in ('ALICE', 'bob', 'CAROL', 'david', 'eugene', 'eugene', 'bob', 'eugene')
]
mock_get_and_cache_content_metadata.return_value = {
'content_title': content_title,
Expand All @@ -251,7 +253,7 @@ def test_allocate_assignments_happy_path(
)
accepted_assignment = LearnerContentAssignmentFactory.create(
assignment_configuration=self.assignment_configuration,
learner_email='bob@foo.com',
learner_email='BOB@foo.com',
content_key=content_key,
content_title=content_title,
content_quantity=-content_price_cents,
Expand Down

0 comments on commit 513235c

Please sign in to comment.