Skip to content

Improve queries with in bulk #3477

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cp-at-mit wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Improve queries with in bulk #3477

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
bb0683d
Use in_bulk for batched DB lookups
cp-at-mit Apr 8, 2026
e629003
Use in_bulk for CourseRun and Order lookups
cp-at-mit Apr 8, 2026
304aefb
Merge branch 'main' into improve-queries-with-in-bulk
cp-at-mit Apr 10, 2026
1a38dd6
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 10, 2026
54eb754
pre-commit
cp-at-mit Apr 10, 2026
fb61b7c
Merge branch 'improve-queries-with-in-bulk' of https://github.com/mit…
cp-at-mit Apr 10, 2026
547894c
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 10, 2026
6f9ec41
Revert "pre-commit"
cp-at-mit Apr 10, 2026
707d193
Merge branch 'improve-queries-with-in-bulk' of https://github.com/mit…
cp-at-mit Apr 11, 2026
b53edde
hold
cp-at-mit Apr 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion courses/management/commands/create_local_enrollments.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,20 @@ def handle(self, *args, **options): # noqa: ARG002

edx_client = get_edx_api_service_client()

runs_by_courseware_id = CourseRun.objects.in_bulk(
courseware_ids, field_name="courseware_id"
)

created_count = {}
for courseware_id in courseware_ids:
run = CourseRun.objects.filter(courseware_id=courseware_id).first()
run = runs_by_courseware_id.get(courseware_id)
if run is None:
self.stderr.write(
self.style.ERROR(
f"Could not find course run with courseware_id={courseware_id}"
)
)
continue

edx_enrollments = edx_client.enrollments.get_enrollments(
course_id=courseware_id
Expand Down
15 changes: 10 additions & 5 deletions courses/migrations/0013_backfill_paid_courserun.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,16 @@ def backfill_paidcourserun(apps, schema_editor):
for order in Order.objects.filter(state__in=["fulfilled", "review"]):
# couldn't use order.purchased_runs here from app defined model
content_type = ContentType.objects.get_for_model(CourseRun)
for run_obj in order.lines.filter(purchased_content_type=content_type):
course_run = CourseRun.objects.get(pk=run_obj.purchased_object_id)
PaidCourseRun.objects.get_or_create(
order=order, course_run=course_run, user=order.purchaser
)
run_objects = order.lines.filter(purchased_content_type=content_type)
course_run_ids = [run_obj.purchased_object_id for run_obj in run_objects]
course_runs_by_id = CourseRun.objects.in_bulk(course_run_ids)

for run_obj in run_objects:
course_run = course_runs_by_id.get(run_obj.purchased_object_id)
if course_run:
PaidCourseRun.objects.get_or_create(
order=order, course_run=course_run, user=order.purchaser
)


class Migration(migrations.Migration):
Expand Down
54 changes: 39 additions & 15 deletions ecommerce/serializers/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -567,17 +567,24 @@ class OrderHistorySerializer(serializers.ModelSerializer):
@extend_schema_field(serializers.ListField)
def get_titles(self, instance):
titles = []

for line in instance.lines.all():
product = models.Product.all_objects.get(
pk=line.product_version.field_dict["id"]
)
if product.content_type.model == "courserun":
titles.append(product.purchasable_object.course.title)
elif product.content_type.model == "programrun":
titles.append(product.description)
else:
titles.append(f"No Title - {product.id}")
# Access prefetched lines data
lines = (
getattr(instance, "_prefetched_objects_cache", {}).get("lines")
or instance.lines.all()
)
product_ids = [line.product_version.field_dict["id"] for line in lines]
products_by_id = models.Product.all_objects.in_bulk(product_ids)

for line in lines:
product_id = line.product_version.field_dict["id"]
product = products_by_id.get(product_id)
if product:
if product.content_type.model == "courserun":
titles.append(product.purchasable_object.course.title)
elif product.content_type.model == "programrun":
titles.append(product.description)
else:
titles.append(f"No Title - {product.id}")

return titles

Expand Down Expand Up @@ -701,9 +708,18 @@ def to_representation(self, instance):
if not isinstance(instance, Order):
raise AttributeError # noqa: TRY004

transaction = instance.transactions.order_by("-created_on").first()
# Access prefetched transactions
transactions = getattr(instance, "_prefetched_objects_cache", {}).get(
"transactions"
)
if transactions:
transaction = (
max(transactions, key=lambda t: t.created_on) if transactions else None
)
else:
transaction = instance.transactions.order_by("-created_on").first()

return transaction # noqa: RET504
return transaction


class TransactionPurchaseSerializer(TransactionDataSerializer):
Copy link
Copy Markdown

@sentry sentry Bot Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The code calls .data on the result of super().to_representation(instance), which can be None if an order has no transactions, causing an AttributeError.
Severity: HIGH

Suggested Fix

Before accessing the .data attribute on the transaction object, add a check to ensure it is not None. This handles cases where an order may not have any associated transactions.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: ecommerce/serializers/__init__.py#L725

Potential issue: In `TransactionPurchaseSerializer` and
`TransactionPurchaserSerializer`, the `to_representation` method calls
`super().to_representation(instance).data`. However, the parent method,
`TransactionDataSerializer.to_representation`, can return `None` if an order does not
have any associated transactions (e.g., for free courses, failed payments, or pending
orders). Calling `.data` on a `None` value will raise an `AttributeError`, causing a
crash. This will prevent users from viewing their order receipt for any order that lacks
a transaction.

Expand Down Expand Up @@ -818,7 +834,13 @@ class Meta:

class TransactionLineSerializer(serializers.BaseSerializer):
def to_representation(self, instance):
coupon_redemption = instance.order.discounts.first()
# Access prefetched discounts
discounts = getattr(instance.order, "_prefetched_objects_cache", {}).get(
"discounts"
)
coupon_redemption = (
discounts[0] if discounts else instance.order.discounts.first()
)
discount = 0.0

if coupon_redemption:
Expand Down Expand Up @@ -888,7 +910,9 @@ def get_order(self, instance):

def get_coupon(self, instance):
"""Get discount code from the discount redemption if available"""
coupon_redemption = instance.discounts.first()
# Access prefetched discounts
discounts = getattr(instance, "_prefetched_objects_cache", {}).get("discounts")
coupon_redemption = discounts[0] if discounts else instance.discounts.first()
if not coupon_redemption:
return None
return DiscountRedemptionSerializer(coupon_redemption).data
Expand Down
49 changes: 35 additions & 14 deletions ecommerce/serializers/v0/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,17 +653,27 @@ class OrderHistorySerializer(serializers.ModelSerializer):
@extend_schema_field(serializers.ListField)
def get_titles(self, instance):
titles = []

for line in instance.lines.all():
product = models.Product.all_objects.get(
pk=line.product_version.field_dict["id"]
)
if product.content_type.model == "courserun" and product.purchasable_object:
titles.append(product.purchasable_object.course.title)
elif product.content_type.model == "programrun":
titles.append(product.description)
else:
titles.append(f"No Title - {product.id}")
# Access prefetched lines data
lines = (
getattr(instance, "_prefetched_objects_cache", {}).get("lines")
or instance.lines.all()
)
product_ids = [line.product_version.field_dict["id"] for line in lines]
products_by_id = models.Product.all_objects.in_bulk(product_ids)

for line in lines:
product_id = line.product_version.field_dict["id"]
product = products_by_id.get(product_id)
if product:
if (
product.content_type.model == "courserun"
and product.purchasable_object
):
titles.append(product.purchasable_object.course.title)
elif product.content_type.model == "programrun":
titles.append(product.description)
else:
titles.append(f"No Title - {product.id}")

return titles

Expand Down Expand Up @@ -804,9 +814,18 @@ def to_representation(self, instance):
if not isinstance(instance, Order):
raise AttributeError # noqa: TRY004

transaction = instance.transactions.order_by("-created_on").first()
# Access prefetched transactions
transactions = getattr(instance, "_prefetched_objects_cache", {}).get(
"transactions"
)
if transactions:
transaction = (
max(transactions, key=lambda t: t.created_on) if transactions else None
)
else:
transaction = instance.transactions.order_by("-created_on").first()

return transaction # noqa: RET504
return transaction


class TransactionPurchaseSerializer(TransactionDataSerializer):
Expand Down Expand Up @@ -948,7 +967,9 @@ def get_order(self, instance):

def get_coupon(self, instance):
"""Get discount code from the discount redemption if available"""
coupon_redemption = instance.discounts.first()
# Access prefetched discounts
discounts = getattr(instance, "_prefetched_objects_cache", {}).get("discounts")
coupon_redemption = discounts[0] if discounts else instance.discounts.first()
if not coupon_redemption:
return None
return DiscountRedemptionSerializer(coupon_redemption).data
Expand Down
11 changes: 10 additions & 1 deletion ecommerce/views/legacy/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1074,4 +1074,13 @@ class OrderReceiptView(RetrieveAPIView):
permission_classes = [IsAuthenticated]

def get_queryset(self):
return Order.objects.filter(purchaser=self.request.user).all()
return (
Order.objects.filter(purchaser=self.request.user)
.prefetch_related(
"lines__product__purchasable_object__course",
"lines__product__content_type",
"transactions",
"discounts__discount",
)
.all()
)
11 changes: 10 additions & 1 deletion ecommerce/views/v0/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -901,4 +901,13 @@ class OrderReceiptView(RetrieveAPIView):

def get_queryset(self):
"""Return only the user's orders"""
return Order.objects.filter(purchaser=self.request.user).all()
return (
Order.objects.filter(purchaser=self.request.user)
.prefetch_related(
"lines__product__purchasable_object__course",
"lines__product__content_type",
"transactions",
"discounts__discount",
)
.all()
)
74 changes: 40 additions & 34 deletions hubspot_sync/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -770,16 +770,17 @@ def make_contact_update_message_list_from_user_ids(
List[dict]: List of dictionaries containing User properties.
"""
chunk_dictionary = dict(chunk)
users = User.objects.filter(id__in=chunk_dictionary.keys())
users_by_id = User.objects.in_bulk(chunk_dictionary.keys())
request_input = []
for user_id, hubspot_id in chunk_dictionary.items():
user = users.filter(id=user_id).first()
request_input.append(
{
"id": hubspot_id,
"properties": make_contact_sync_message_from_user(user).properties,
}
)
user = users_by_id.get(user_id)
if user:
request_input.append(
{
"id": hubspot_id,
"properties": make_contact_sync_message_from_user(user).properties,
}
)
return request_input


Expand Down Expand Up @@ -856,16 +857,17 @@ def make_deal_update_message_list_from_order_ids(
List[dict]: List of dictionaries containing Order properties.
"""
chunk_dictionary = dict(chunk)
orders = Order.objects.filter(id__in=chunk_dictionary.keys())
orders_by_id = Order.objects.in_bulk(chunk_dictionary.keys())
request_input = []
for order_id, hubspot_id in chunk_dictionary.items():
order = orders.filter(id=order_id).first()
request_input.append(
{
"id": hubspot_id,
"properties": make_deal_sync_message_from_order(order).properties,
}
)
order = orders_by_id.get(order_id)
if order:
request_input.append(
{
"id": hubspot_id,
"properties": make_deal_sync_message_from_order(order).properties,
}
)
return request_input


Expand Down Expand Up @@ -917,16 +919,19 @@ def make_line_item_update_message_list_from_line_ids(
List[dict]: List of dictionaries containing Line properties.
"""
chunk_dictionary = dict(chunk)
lines = Line.objects.filter(id__in=chunk_dictionary.keys())
lines_by_id = Line.objects.in_bulk(chunk_dictionary.keys())
request_input = []
for line_id, hubspot_id in chunk_dictionary.items():
line = lines.filter(id=line_id).first()
request_input.append(
{
"id": hubspot_id,
"properties": make_line_item_sync_message_from_line(line).properties,
}
)
line = lines_by_id.get(line_id)
if line:
request_input.append(
{
"id": hubspot_id,
"properties": make_line_item_sync_message_from_line(
line
).properties,
}
)
return request_input


Expand Down Expand Up @@ -978,18 +983,19 @@ def make_product_update_message_list_from_product_ids(
List[dict]: List of dictionaries containing Product properties.
"""
chunk_dictionary = dict(chunk)
products = Product.objects.filter(id__in=chunk_dictionary.keys())
products_by_id = Product.objects.in_bulk(chunk_dictionary.keys())
request_input = []
for product_id, hubspot_id in chunk_dictionary.items():
product = products.filter(id=product_id).first()
request_input.append(
{
"id": hubspot_id,
"properties": make_product_sync_message_from_product(
product
).properties,
}
)
product = products_by_id.get(product_id)
if product:
request_input.append(
{
"id": hubspot_id,
"properties": make_product_sync_message_from_product(
product
).properties,
}
)
return request_input


Expand Down
5 changes: 4 additions & 1 deletion hubspot_sync/tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,8 +500,11 @@ def batch_upsert_associations_chunked(order_ids: List[int]): # noqa: UP006
line_associations_batch = []
hubspot_client = HubspotApi()
deal_count = len(order_ids)
deals_by_id = Order.objects.in_bulk(order_ids)
for idx, order_id in enumerate(order_ids):
deal = Order.objects.get(id=order_id)
deal = deals_by_id.get(order_id)
if not deal:
continue
Comment on lines +505 to +507
Copy link
Copy Markdown

@sentry sentry Bot Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: If the last order in a batch is missing from the database, accumulated association data is silently dropped and never sent to HubSpot due to a skipped final flush.
Severity: HIGH

Suggested Fix

Decouple the final flush logic from the item processing loop. After the for loop concludes, add a final, unconditional check to flush any remaining items in contact_associations_batch and line_associations_batch. This ensures all accumulated data is sent, regardless of whether the last item was processed.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: hubspot_sync/tasks.py#L505-L507

Potential issue: In the `batch_upsert_associations_chunked` function, data is processed
in batches and flushed to HubSpot when the batch is full or on the last item. The code
checks for the last item using `idx == deal_count - 1`. However, if the last `order_id`
in the input list does not correspond to an existing `Order` in the database, a
`continue` statement is executed, skipping the rest of the loop body. This prevents the
final flush condition from ever being evaluated, causing any accumulated data in
`contact_associations_batch` and `line_associations_batch` to be silently dropped
instead of being sent to HubSpot.

Did we get this right? 👍 / 👎 to inform future reviews.

contact_id = get_hubspot_id_for_object(deal.purchaser)
deal_id = get_hubspot_id_for_object(deal)
for line in deal.lines.iterator():
Expand Down
12 changes: 10 additions & 2 deletions users/management/tests/retire_users_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,12 @@ def test_multiple_success(mocker):

COMMAND.handle("retire_users", users=test_usernames)

users_by_username = User.objects.in_bulk(
test_usernames, field_name="openedx_users__edx_username"
)
for user_name in test_usernames:
user = User.objects.get(openedx_users__edx_username=user_name)
user = users_by_username.get(user_name)
assert user is not None
assert user.is_active is False
assert "retired_email" in user.email
mock_bulk_retire_edx_users.assert_called()
Expand Down Expand Up @@ -129,8 +133,12 @@ def test_multiple_success_blocking_user(mocker):

COMMAND.handle("retire_users", users=test_usernames, block_users=True)

users_by_username = User.objects.in_bulk(
test_usernames, field_name="openedx_users__edx_username"
)
for user_name in test_usernames:
user = User.objects.get(openedx_users__edx_username=user_name)
user = users_by_username.get(user_name)
assert user is not None
assert user.is_active is False
assert "retired_email" in user.email

Expand Down
Loading