Skip to content

chore: improve query performance #31

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

Merged
ngovinh2k2 merged 1 commit into from
Jul 3, 2026
Merged

chore: improve query performance #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 5 additions & 1 deletion apps/custom_email/service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@ def get_theme_logo_url(instance, theme_key):
if not setting:
return f"{host}/static/images/branding/{default_logo}"

theme = setting.themes.filter(theme_key=theme_key).first()
themes = getattr(setting, "_prefetched_objects_cache", {}).get("themes")
if themes is not None:
theme = next((item for item in themes if item.theme_key == theme_key), None)
else:
theme = setting.themes.filter(theme_key=theme_key).first()
Comment on lines +20 to +24

@gemini-code-assist gemini-code-assist Bot Jul 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Accessing Django's private _prefetched_objects_cache attribute is fragile and can break in future Django updates. Instead, you can use the public setting.themes.all() API. If the themes relation has been prefetched, Django will automatically use the prefetched cache without hitting the database. If it hasn't been prefetched, it will perform a query to fetch the themes. Since an organization setting typically has very few themes (e.g., light and dark), fetching all of them is highly efficient and much more maintainable.

Suggested change
themes = getattr(setting, "_prefetched_objects_cache", {}).get("themes")
if themes is not None:
theme = next((item for item in themes if item.theme_key == theme_key), None)
else:
theme = setting.themes.filter(theme_key=theme_key).first()
theme = next((item for item in setting.themes.all() if item.theme_key == theme_key), None)


if not theme or not theme.logo:
return f"{host}/static/images/branding/{default_logo}"
Expand Down
12 changes: 11 additions & 1 deletion apps/custom_email/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,25 @@
from common.pagination.base_pagination import BasePagination
from django.db.models import Prefetch
from django_filters.rest_framework import DjangoFilterBackend
from rest_framework.filters import OrderingFilter

from apps.custom_email.models import OrganizationEmail
from apps.custom_email.serializers import OrganizationEmailSerializer
from apps.organization_setting.models import OrganizationTheme
Comment on lines +2 to +8

@gemini-code-assist gemini-code-assist Bot Jul 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Since Prefetch and OrganizationTheme are no longer needed after simplifying the queryset, their imports can be safely removed to keep the file clean.

Suggested change
from django.db.models import Prefetch
from django_filters.rest_framework import DjangoFilterBackend
from rest_framework.filters import OrderingFilter
from apps.custom_email.models import OrganizationEmail
from apps.custom_email.serializers import OrganizationEmailSerializer
from apps.organization_setting.models import OrganizationTheme
from django_filters.rest_framework import DjangoFilterBackend
from rest_framework.filters import OrderingFilter
from apps.custom_email.models import OrganizationEmail
from apps.custom_email.serializers import OrganizationEmailSerializer

from utils.views import OrganizationListAPIView


class ListCustomEmailView(OrganizationListAPIView):
serializer_class = OrganizationEmailSerializer
queryset = OrganizationEmail.objects.all()
queryset = OrganizationEmail.objects.select_related(
"organization",
"organization__organization_settings",
).prefetch_related(
Prefetch(
"organization__organization_settings__themes",
queryset=OrganizationTheme.objects.all(),
)
)
Comment on lines +14 to +22

@gemini-code-assist gemini-code-assist Bot Jul 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Using Prefetch with a default queryset (OrganizationTheme.objects.all()) is redundant here. You can simplify the prefetch_related call by passing the lookup path as a string. This is cleaner, more readable, and avoids unnecessary imports.

    queryset = OrganizationEmail.objects.select_related(
        "organization",
        "organization__organization_settings",
    ).prefetch_related(
        "organization__organization_settings__themes"
    )

organization_field = "organization"
pagination_class = BasePagination
filterset_fields = ["email_type"]
Expand Down
24 changes: 14 additions & 10 deletions apps/organization_roles/services.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,22 +108,26 @@


def create_default_policies(organization):
organization_policies = []
for policy in default_policies:
organization_policy = OrganizationPolicy(**policy, organization=organization)
organization_policy.save()
organization_policies.append(organization_policy.pk)
organization_policies = [
OrganizationPolicy(**policy, organization=organization)
for policy in default_policies
]
OrganizationPolicy.objects.bulk_create(organization_policies)
return organization_policies
Comment on lines 110 to 116

@gemini-code-assist gemini-code-assist Bot Jul 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Django's bulk_create returns a list of the created objects with their primary keys populated (on supported databases like PostgreSQL). Returning the original organization_policies list directly is risky because the objects in the original list are not guaranteed to have their primary keys set in-place across all environments or database backends (e.g., during testing with SQLite). Returning the result of bulk_create ensures that the primary keys are always populated in the returned list.

Suggested change
def create_default_policies(organization):
organization_policies = []
for policy in default_policies:
organization_policy = OrganizationPolicy(**policy, organization=organization)
organization_policy.save()
organization_policies.append(organization_policy.pk)
organization_policies = [
OrganizationPolicy(**policy, organization=organization)
for policy in default_policies
]
OrganizationPolicy.objects.bulk_create(organization_policies)
return organization_policies
def create_default_policies(organization):
organization_policies = [
OrganizationPolicy(**policy, organization=organization)
for policy in default_policies
]
return OrganizationPolicy.objects.bulk_create(organization_policies)



def create_default_organization_role_by_policy_tag(name, tag, organization):
policies = OrganizationPolicy.objects.filter(
tags__icontains=tag, organization=organization
).all()
def create_default_organization_role_by_policy_tag(
name, tag, organization, policies=None
):
if policies is None:
policies = OrganizationPolicy.objects.filter(
tags__contains=[tag], organization=organization
).all()
else:
policies = [policy for policy in policies if tag in policy.tags]
organization_role = OrganizationRole(name=name, organization=organization)
organization_role.save()
organization_role.policies.set([policy.pk for policy in policies])
organization_role.save()
return organization_role


Expand Down
5 changes: 3 additions & 2 deletions bootstrap_service/management/commands/init_organization.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ def _create_organization_with_roles(self, org_id, org_name, org_slug, result):
)
self.stdout.write(self.style.SUCCESS(f"Created organization: {org_name}"))

create_default_policies(organization)
organization_policies = create_default_policies(organization)

create_default_pages(organization)
create_default_organization_email(organization)
create_default_organization_setting(organization)
Expand All @@ -135,7 +136,7 @@ def _create_organization_with_roles(self, org_id, org_name, org_slug, result):
owner_role = None
for role_type, policy_tag in role_mappings:
role = create_default_organization_role_by_policy_tag(
role_type, policy_tag, organization
role_type, policy_tag, organization, organization_policies
)
if role_type == OrganizationRoleType.OWNER_ROLE:
owner_role = role
Expand Down
1 change: 1 addition & 0 deletions bootstrap_service/settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ def silky_intercept_func(request):
SILKY_MAX_RESPONSE_BODY_SIZE = 0
SILKY_META = True
SILKY_INTERCEPT_PERCENT = 10
SILKY_MAX_RECORDED_REQUESTS = 500
SILKY_MAX_RECORDED_REQUESTS_CHECK_PERCENT = 10

# Internationalization
Expand Down
Loading