Skip to content

Commit be4fc6b

Browse files
feat: add bulk processing for learner credit requests
1 parent 683136c commit be4fc6b

File tree

14 files changed

+1849
-605
lines changed

14 files changed

+1849
-605
lines changed

enterprise_access/apps/api/serializers/subsidy_requests.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,7 @@ class LearnerCreditRequestApproveRequestSerializer(serializers.Serializer):
294294
help_text="The UUID of the SubsidyAccessPolicy to use for this approval."
295295
)
296296

297+
297298
class LearnerCreditRequestApproveAllSerializer(serializers.Serializer):
298299
"""
299300
Request serializer to validate the approve-all action.

enterprise_access/apps/api/v1/tests/test_browse_and_request_views.py

Lines changed: 133 additions & 102 deletions
Large diffs are not rendered by default.

enterprise_access/apps/api/v1/views/browse_and_request.py

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -965,8 +965,7 @@ def approve(self, request, *args, **kwargs):
965965
966966
- On success, returns a `200 OK` with a list of the approved request objects.
967967
- If any of the specified requests fail to be approved, returns a
968-
`422 Unprocessable Entity` with a detail message. The successful
969-
approvals will still be committed.
968+
`422 Unprocessable Entity`. The successful approvals will still be committed.
970969
"""
971970
serializer = serializers.LearnerCreditRequestApproveRequestSerializer(data=request.data)
972971
serializer.is_valid(raise_exception=True)
@@ -990,14 +989,26 @@ def approve(self, request, *args, **kwargs):
990989
policy_uuid=policy_uuid,
991990
reviewer=request.user
992991
)
993-
if response['failed_approval']:
992+
if response.get("error_message"):
993+
error_msg = (
994+
f"[LC REQUEST APPROVAL] Failed to approve learner credit requests. "
995+
f"Reason: {response['error_message']}."
996+
)
997+
logger.exception(error_msg)
998+
999+
return Response({"detail": error_msg}, status=status.HTTP_422_UNPROCESSABLE_ENTITY)
1000+
if response.get("failed_approval"):
9941001
return Response(
1002+
{"detail": "[LC REQUEST APPROVAL] Failed to approve some learner credit requests."},
9951003
status=status.HTTP_422_UNPROCESSABLE_ENTITY
9961004
)
997-
response_data = self.get_serializer(response['approved'], many=True).data
998-
return Response(response_data, status=status.HTTP_200_OK)
999-
except Exception: # pylint: disable=broad-except
1000-
return Response(status=status.HTTP_422_UNPROCESSABLE_ENTITY)
1005+
return Response(status=status.HTTP_200_OK)
1006+
except Exception as e: # pylint: disable=broad-except
1007+
logger.exception(e)
1008+
return Response(
1009+
{"detail": "Unexpected error during bulk approval."},
1010+
status=status.HTTP_422_UNPROCESSABLE_ENTITY
1011+
)
10011012

10021013
@permission_required(
10031014
constants.REQUESTS_ADMIN_ACCESS_PERMISSION,
@@ -1035,13 +1046,23 @@ def approve_all(self, request, *args, **kwargs):
10351046
policy_uuid=policy_uuid,
10361047
reviewer=request.user
10371048
)
1038-
if response['failed_approval']:
1049+
if response.get("error_message"):
1050+
error_msg = (
1051+
f"[LC REQUEST APPROVAL] Failed to approve learner credit requests. "
1052+
f"Reason: {response['error_message']}."
1053+
)
1054+
return Response({"detail": error_msg}, status=status.HTTP_422_UNPROCESSABLE_ENTITY)
1055+
if response.get('failed_approval'):
10391056
return Response(
1057+
{"detail": "[LC REQUEST APPROVAL] Failed to approve some learner credit requests."},
10401058
status=status.HTTP_422_UNPROCESSABLE_ENTITY
10411059
)
10421060
return Response(status=status.HTTP_202_ACCEPTED)
10431061
except Exception: # pylint: disable=broad-except
1044-
return Response(status=status.HTTP_422_UNPROCESSABLE_ENTITY)
1062+
return Response(
1063+
{"detail": "Unexpected error during bulk approval."},
1064+
status=status.HTTP_422_UNPROCESSABLE_ENTITY
1065+
)
10451066

10461067
@permission_required(
10471068
constants.REQUESTS_ADMIN_ACCESS_PERMISSION,

enterprise_access/apps/content_assignments/api.py

Lines changed: 160 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
send_exec_ed_enrollment_warmer,
2323
send_reminder_email_for_pending_assignment
2424
)
25+
from enterprise_access.apps.content_metadata.api import get_and_cache_catalog_content_metadata, summary_data_for_content
2526
from enterprise_access.apps.core.models import User
2627
from enterprise_access.apps.subsidy_access_policy.content_metadata_api import get_and_cache_content_metadata
2728
from enterprise_access.apps.subsidy_request.constants import SubsidyRequestStates
@@ -452,84 +453,88 @@ def _do_async_tasks_after_assignment_writes(updated_assignments, created_assignm
452453
send_email_for_new_assignment.delay(assignment.uuid)
453454

454455

455-
def allocate_assignment_for_request(
456+
def allocate_assignment_for_requests(
456457
assignment_configuration,
457-
learner_email,
458-
content_key,
459-
content_price_cents,
460-
lms_user_id,
458+
learner_credit_requests,
461459
):
462460
"""
463-
Creates or reallocates an assignment record for the given ``content_key`` in the given ``assignment_configuration``,
464-
and the provided ``learner_email``.
461+
Creates or reallocates LearnerContentAssignment records in bulk for a batch
462+
of LearnerCreditRequests.
465463
466-
Params:
467-
- ``assignment_configuration``: The AssignmentConfiguration record under which assignments should be allocated.
468-
- ``learner_email``: The email address of the learner to whom the assignment should be allocated.
469-
- ``content_key``: Either a course or course run key, representing the content to be allocated.
470-
- ``content_price_cents``: The cost of redeeming the content, in USD cents, at the time of allocation. Should
471-
always be an integer >= 0.
472-
- ``lms_user_id``: lms user id of the user.
464+
Args:
465+
assignment_configuration (AssignmentConfiguration): The config to use.
466+
learner_credit_requests (list[LearnerCreditRequest]): The requests to process.
473467
474-
Returns: A LearnerContentAssignment record that was created or None.
468+
Returns:
469+
dict: A map of {request.uuid: assignment_object}.
475470
"""
476471
# Set a batch ID to track assignments updated and/or created together.
477472
allocation_batch_id = uuid4()
478-
479-
message = (
480-
'Allocating assignments: assignment_configuration=%s, batch_id=%s, '
481-
'learner_email=%s, content_key=%s, content_price_cents=%s'
482-
)
483-
logger.info(
484-
message, assignment_configuration.uuid, allocation_batch_id,
485-
learner_email, content_key, content_price_cents
486-
)
487-
488-
if content_price_cents < 0:
489-
raise AllocationException('Allocation price must be >= 0')
490-
491-
# We store the allocated quantity as a (future) debit
492-
# against a store of value, so we negate the provided non-negative
493-
# content_price_cents, and then persist that in the assignment records.
494-
content_quantity = content_price_cents * -1
495-
lms_user_ids_by_email = {learner_email.lower(): lms_user_id}
496-
existing_assignments = _get_existing_assignments_for_allocation(
473+
assignments_to_update = []
474+
requests_for_new_assignments = []
475+
476+
# Fetch all unique course metadata in a single API call.
477+
all_content_keys = list(set(req.course_id for req in learner_credit_requests))
478+
metadata_by_key = {
479+
meta['key']: summary_data_for_content(meta['key'], meta)
480+
for meta in get_and_cache_catalog_content_metadata(
481+
assignment_configuration.subsidy_access_policy.catalog_uuid,
482+
all_content_keys
483+
)
484+
}
485+
# Find all existing, re-allocatable assignments for the entire batch of requests.
486+
existing_assignments_map = _get_existing_assignments_for_requests(
497487
assignment_configuration,
498-
[learner_email],
499-
content_key,
500-
lms_user_ids_by_email,
488+
learner_credit_requests,
501489
)
502490

503-
# Re-allocate existing assignment
504-
if len(existing_assignments) > 0:
505-
assignment = next(iter(existing_assignments), None)
506-
if assignment and assignment.state in LearnerContentAssignmentStateChoices.REALLOCATE_STATES:
507-
preferred_course_run_key = _get_preferred_course_run_key(assignment_configuration, content_key)
508-
parent_content_key = _get_parent_content_key(assignment_configuration, content_key)
509-
is_assigned_course_run = bool(parent_content_key)
491+
# Separate requests into "update" vs "create" paths.
492+
for request in learner_credit_requests:
493+
lookup_key = (request.user.email.lower(), request.course_id)
494+
existing_assignment = existing_assignments_map.get(lookup_key)
495+
496+
if existing_assignment:
497+
# This request corresponds to an existing assignment that can be re-used.
498+
# We prepare it for reallocation by updating its state and price.
499+
metadata = metadata_by_key.get(request.course_id, {})
510500
_reallocate_assignment(
511-
assignment,
512-
content_quantity,
513-
allocation_batch_id,
514-
preferred_course_run_key,
515-
parent_content_key,
516-
is_assigned_course_run,
501+
assignment=existing_assignment,
502+
content_quantity=request.course_price * -1,
503+
allocation_batch_id=allocation_batch_id,
504+
preferred_course_run_key=metadata.get('course_run_key'),
505+
parent_content_key=metadata.get('parent_content_key'),
506+
is_assigned_course_run=bool(metadata.get('parent_content_key')),
517507
)
518-
assignment.save()
519-
return assignment
508+
assignments_to_update.append(existing_assignment)
509+
else:
510+
# This request requires a brand new assignment.
511+
requests_for_new_assignments.append(request)
520512

521-
assignment = _create_new_assignments(
522-
assignment_configuration,
523-
[learner_email],
524-
content_key,
525-
content_quantity,
526-
lms_user_ids_by_email,
527-
allocation_batch_id,
528-
)
529-
# If the assignment was created, it will be a list with one item.
530-
if assignment:
531-
return assignment[0]
532-
return None
513+
with transaction.atomic():
514+
# Bulk update and get a list of refreshed objects
515+
updated_assignments = _update_and_refresh_assignments(
516+
assignments_to_update,
517+
ASSIGNMENT_REALLOCATION_FIELDS
518+
)
519+
520+
created_assignments = _create_new_assignments_for_requests(
521+
assignment_configuration,
522+
requests_for_new_assignments,
523+
allocation_batch_id,
524+
metadata_by_key
525+
)
526+
527+
# Map all affected assignments back to their original requests
528+
all_affected_assignments = list(updated_assignments) + created_assignments
529+
assignments_by_learner_and_course = {
530+
(asg.lms_user_id, asg.content_key): asg for asg in all_affected_assignments
531+
}
532+
533+
request_to_assignment_map = {
534+
req.uuid: assignments_by_learner_and_course.get((req.user.lms_user_id, req.course_id))
535+
for req in learner_credit_requests
536+
}
537+
return request_to_assignment_map
533538

534539

535540
def _deduplicate_learner_emails_to_allocate(learner_emails):
@@ -636,6 +641,52 @@ def _get_existing_assignments_for_allocation(
636641
return existing_assignments
637642

638643

644+
def _get_existing_assignments_for_requests(assignment_configuration, learner_credit_requests):
645+
"""
646+
Finds all existing, re-allocatable assignments for a heterogeneous batch
647+
of learner credit requests in a single, efficient query.
648+
649+
This correctly checks for matches on both (email, content_key) and
650+
(lms_user_id, content_key).
651+
652+
Args:
653+
assignment_configuration (AssignmentConfiguration): The configuration to search within.
654+
learner_credit_requests (list[LearnerCreditRequest]): The list of requests.
655+
656+
Returns:
657+
dict: A mapping of (learner_email, content_key) to the existing assignment object.
658+
"""
659+
if not learner_credit_requests:
660+
return {}
661+
662+
# Build a complex Q object to find all matches in one query.
663+
# For each request, we look for an assignment that matches either the email/course
664+
# combination OR the lms_user_id/course combination.
665+
query = Q()
666+
for request in learner_credit_requests:
667+
# Always check for a match on the email and course key.
668+
sub_query = Q(learner_email__iexact=request.user.email, content_key=request.course_id)
669+
670+
# If the request has a valid lms_user_id, also check for a match on that.
671+
if request.user.lms_user_id:
672+
sub_query |= Q(lms_user_id=request.user.lms_user_id, content_key=request.course_id)
673+
674+
query |= sub_query
675+
676+
# Execute a single query to get all potentially matching assignments.
677+
existing_assignments = LearnerContentAssignment.objects.filter(
678+
query,
679+
assignment_configuration=assignment_configuration,
680+
state__in=LearnerContentAssignmentStateChoices.REALLOCATE_STATES
681+
)
682+
683+
# Returns a dictionary keyed by (email, content_key) for fast lookups.
684+
return {
685+
(assignment.learner_email.lower(), assignment.content_key): assignment
686+
for assignment in existing_assignments
687+
}
688+
689+
639690
def _reallocate_assignment(
640691
assignment,
641692
content_quantity,
@@ -791,6 +842,51 @@ def _create_new_assignments(
791842
)
792843

793844

845+
def _create_new_assignments_for_requests(
846+
assignment_configuration,
847+
learner_credit_requests,
848+
allocation_batch_id,
849+
metadata_by_key
850+
):
851+
"""
852+
Helper to bulk save new LearnerContentAssignment instances from a list of
853+
heterogeneous LearnerCreditRequest objects.
854+
"""
855+
if not learner_credit_requests:
856+
return []
857+
858+
# 2. Prepare all assignment objects in memory.
859+
assignments_to_create = []
860+
for request in learner_credit_requests:
861+
metadata = metadata_by_key.get(request.course_id, {})
862+
assignment = LearnerContentAssignment(
863+
assignment_configuration=assignment_configuration,
864+
learner_email=request.user.email,
865+
lms_user_id=request.user.lms_user_id,
866+
content_key=request.course_id,
867+
content_quantity=request.course_price * -1,
868+
content_title=metadata.get('content_title'),
869+
parent_content_key=metadata.get('parent_content_key'),
870+
preferred_course_run_key=metadata.get('course_run_key'),
871+
is_assigned_course_run=bool(metadata.get('parent_content_key')),
872+
state=LearnerContentAssignmentStateChoices.ALLOCATED,
873+
allocation_batch_id=allocation_batch_id,
874+
allocated_at=localized_utcnow(),
875+
)
876+
assignments_to_create.append(assignment)
877+
878+
# 3. Validate and bulk create all at once.
879+
for assignment in assignments_to_create:
880+
assignment.clean()
881+
created_assignments = LearnerContentAssignment.objects.bulk_create(assignments_to_create)
882+
883+
return list(
884+
LearnerContentAssignment.objects.prefetch_related('actions').filter(
885+
uuid__in=[record.uuid for record in created_assignments],
886+
)
887+
)
888+
889+
794890
def cancel_assignments(assignments: Iterable[LearnerContentAssignment], send_cancel_email_to_learner=True) -> dict:
795891
"""
796892
Bulk cancel assignments.

0 commit comments

Comments
 (0)