Skip to content

Fix Active Dref Operations List n+1 query issue (active-dref)#2759

Closed
szabozoltan69 wants to merge 1 commit into
developfrom
fix/active-dref-n+1
Closed

Fix Active Dref Operations List n+1 query issue (active-dref)#2759
szabozoltan69 wants to merge 1 commit into
developfrom
fix/active-dref-n+1

Conversation

@szabozoltan69

@szabozoltan69 szabozoltan69 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Refers to Sentry issue /organizations/ifrc-go/issues/6882.

Maybe these changes makes the code too unreadable, and the benefit is not so much. Should we go on with merge?

@szabozoltan69 szabozoltan69 requested a review from susilnem June 4, 2026 08:09
@szabozoltan69 szabozoltan69 force-pushed the fix/active-dref-n+1 branch from c116213 to 941909b Compare June 4, 2026 08:37
Comment thread dref/views.py
Comment on lines +355 to +359
Prefetch(
"dreffinalreport",
queryset=DrefFinalReport.objects.all(),
to_attr="prefetched_final_report",
),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is 1:1 Field, So no need to Prefetch DrefFinalReport.

Comment thread dref/views.py
filterset_class = ActiveDrefFilterSet
queryset = (
Dref.objects.prefetch_related("planned_interventions", "needs_identified", "national_society_actions", "users")
Dref.objects.select_related("country")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
Dref.objects.select_related("country")
Dref.objects.select_related("country", "dreffinalreport",)

Comment thread dref/serializers.py
return sum(1 for op in prefetched if op.status != Dref.Status.APPROVED)
return DrefOperationalUpdate.objects.filter(dref_id=obj.id).exclude(status=Dref.Status.APPROVED).count()

def get_unpublished_final_report_count(self, obj) -> int:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need this field? Given that DrefFinalReport is a OneToOneField, the count can only ever be 0 or 1, so it doesn't seem to provide much additional information.

If it's required for backward compatibility, we could annotate it in the queryset instead of calculating it in the serializer.

Comment thread dref/serializers.py
return None
return MiniDrefFinalReportActiveSerializer(final_report).data

def get_has_ops_update(self, obj) -> bool:

@susilnem susilnem Jun 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We can shift most of these checks on the query level.

@szabozoltan69

Copy link
Copy Markdown
Contributor Author

We can drop this fix.
Before the change:
kép
After the change, less queries in longer time!
kép

@szabozoltan69

Copy link
Copy Markdown
Contributor Author

Not effective n+1 query fix.
Nevertheless, if you have suggestions, dear @susilnem or @batpad , maybe about adding "dreffinalreport", let me know and let's do another fix.

@szabozoltan69 szabozoltan69 deleted the fix/active-dref-n+1 branch June 11, 2026 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants