Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 0 additions & 13 deletions apps/authentication/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@
from common.apps.refresh_tokens.services import create_jwt_tokens
from django.conf import settings
from django.core.cache import cache
from django.template.loader import render_to_string
from rest_framework import status
from rest_framework.exceptions import ValidationError
from rest_framework.response import Response

from apps.authentication.models import RootUser
Expand Down Expand Up @@ -92,14 +90,3 @@ def handle_access_token(access_token, provider: Literal["GOOGLE"]):
"default_organization": default_organization_slug,
},
)


def render_email_format(template, data):
try:
html_message = render_to_string(
template,
data,
)
return html_message
except Exception as e:
raise ValidationError({"error": f"Error: {e}"})
22 changes: 13 additions & 9 deletions apps/authentication/views.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from datetime import datetime, timezone

from common.apps.refresh_tokens.serializers import TokenPairSerializer
from common.utils.email_context import get_email_context, render_email_format
from common.utils.send_email import send_email
from common.utils.token_jwt import generate_token
from django.conf import settings
Expand All @@ -22,10 +23,9 @@
SendEmailSerializer,
UserSerializer,
)
from apps.authentication.services import (
create_organization_access_token,
render_email_format,
)
from apps.authentication.services import create_organization_access_token
from apps.custom_email.constants import EmailTypes
from apps.custom_email.service import get_custom_email_item


class LoginAPIView(TokenObtainPairView):
Expand Down Expand Up @@ -69,11 +69,15 @@ def post(self, request):

subject = "🔒 Forgot your password? Reset now"
token = generate_token({"email": email})
data = {
"redirect_url": f"{settings.HOST_FRONTEND_ADMIN}/auth/reset-password?token={token}",
"host": settings.HOST,
}
message = render_email_format("email_forget_password.html", data)
custom_email_item = get_custom_email_item(EmailTypes.RESET_PASSWORD)
email_context = get_email_context(
{
"host": settings.HOST,
"redirect_url": f"{settings.HOST_FRONTEND_ADMIN}/auth/reset-password?token={token}",
},
custom_email=custom_email_item,
)
message = render_email_format("email_forget_password.html", email_context)
send_email(settings.DEFAULT_FROM_EMAIL, [email], subject, message)
return Response(
{
Expand Down
27 changes: 27 additions & 0 deletions apps/custom_email/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,22 @@

from apps.custom_email.constants import EmailTypes
from apps.custom_email.models import OrganizationEmail
from apps.custom_email.service import get_theme_logo_url


class OrganizationEmailSerializer(serializers.ModelSerializer):
brand_logo_dark = serializers.SerializerMethodField()
brand_logo_light = serializers.SerializerMethodField()
brand_name = serializers.SerializerMethodField()

class Meta:
model = OrganizationEmail
fields = [
"id",
"brand_logo_dark",
"brand_logo_light",
"brand_name",
"email_type",
"email_type",
"sender_name",
Comment on lines +21 to 24

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

The field "email_type" is duplicated in the fields list of OrganizationEmailSerializer. One of them should be removed to avoid redundancy and keep the code clean.

Suggested change
"brand_name",
"email_type",
"email_type",
"sender_name",
"brand_name",
"email_type",
"sender_name",

"sender_email",
Expand All @@ -25,6 +34,9 @@ class Meta:
"id": {"read_only": True},
"email_type": {"required": True},
"header_image": {"write_only": True},
"brand_logo_dark": {"read_only": True},
"brand_logo_light": {"read_only": True},
"brand_name": {"read_only": True},
}

def to_representation(self, instance):
Expand All @@ -47,3 +59,18 @@ def to_representation(self, instance):
f"{host}/static/images/{image_path}" if image_path else ""
)
return data

def get_brand_logo_dark(self, instance):
return get_theme_logo_url(instance, "dark")

def get_brand_logo_light(self, instance):
return get_theme_logo_url(instance, "light")

def get_brand_name(self, instance):
organization = getattr(instance, "organization", None)
setting = (
getattr(organization, "organization_settings", None)
if organization
else None
)
return getattr(setting, "brand_name", "") or ""
43 changes: 43 additions & 0 deletions apps/custom_email/service.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,31 @@
from common.apps.upload_file.service import get_presigned_url
from django.conf import settings

from apps.custom_email.constants import EmailTypes
from apps.custom_email.models import OrganizationEmail
from apps.organization_setting.constants import ThemeType


def get_theme_logo_url(instance, theme_key):
host = settings.HOST.rstrip("/")
default_logo = "logo_white.png" if theme_key == ThemeType.DARK else "logo_black.png"
organization = getattr(instance, "organization", None)
if not organization:
return f"{host}/static/images/branding/{default_logo}"

setting = getattr(organization, "organization_settings", None)
if not setting:
return f"{host}/static/images/branding/{default_logo}"

theme = setting.themes.filter(theme_key=theme_key).first()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Using .filter() on the themes related manager triggers a database query every time it is called. When serializing a list of custom emails, this leads to an N+1 query problem (specifically, multiple queries per email item to fetch dark and light themes).

By changing this to use Python's next() with setting.themes.all(), we can leverage Django's prefetch_related cache. Even if prefetching is not used, fetching all themes (typically only 2: light and dark) in a single query is more efficient than executing multiple filtered queries.

Suggested change
theme = setting.themes.filter(theme_key=theme_key).first()
theme = next((t for t in setting.themes.all() if t.theme_key == theme_key), None)


if not theme or not theme.logo:
return f"{host}/static/images/branding/{default_logo}"

return get_presigned_url(
settings.AWS_S3.get("AWS_STORAGE_BUCKET_NAME"),
f"uploads/{theme.logo}",
)


def get_default_organization_emails():
Expand Down Expand Up @@ -57,3 +83,20 @@ def create_default_organization_email(organization):
for email_data in get_default_organization_emails()
]
)


def get_custom_email_item(email_type):
from apps.custom_email.serializers import OrganizationEmailSerializer

custom_email = next(
(
item
for item in get_default_organization_emails()
if item.get("email_type") == email_type
),
None,
)
if not custom_email:
return {}

return OrganizationEmailSerializer(OrganizationEmail(**custom_email)).data
1 change: 1 addition & 0 deletions apps/custom_email/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ class ListCustomEmailView(OrganizationListAPIView):
queryset = OrganizationEmail.objects.all()
organization_field = "organization"
Comment on lines 12 to 13

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

To completely eliminate the N+1 query problem when listing custom emails, we should optimize the queryset in ListCustomEmailView by pre-fetching the related organization, organization_settings, and themes using select_related and prefetch_related.

Suggested change
queryset = OrganizationEmail.objects.all()
organization_field = "organization"
queryset = OrganizationEmail.objects.select_related(
"organization__organization_settings"
).prefetch_related(
"organization__organization_settings__themes"
)
organization_field = "organization"

pagination_class = BasePagination
filterset_fields = ["email_type"]
filter_backends = [DjangoFilterBackend, OrderingFilter]
ordering = ["-created_at"]
3 changes: 0 additions & 3 deletions apps/organization_setting/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,6 @@ def _update_custom_emails(self, instance, custom_emails_data):
)
self._save_instance(custom_email, custom_email_data)

def to_representation(self, instance):
return OrganizationConfigSerializer(instance, context=self.context).data

def update(self, instance, validated_data):
with transaction.atomic():
custom_pages_data = validated_data.pop("custom_pages", [])
Expand Down
2 changes: 1 addition & 1 deletion apps/organization_setting/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"logo": "",
"theme_colors": {
"primary": "#171A28",
"outline": "#FFFFFF",
"outline": "#171A28",
"background": "#FFFFFF",
"text": "#1F2937",
"input": "#F0F1F3",
Expand Down
23 changes: 22 additions & 1 deletion apps/organization_setting/views.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
from django.shortcuts import get_object_or_404
from rest_framework import generics
from rest_framework.response import Response

from apps.organization.models import Organization
from apps.organization_setting.models import OrganizationSetting
from apps.organization_setting.serializers import UpdateOrganizationSettingSerializer
from apps.organization_setting.serializers import (
OrganizationConfigSerializer,
UpdateOrganizationSettingSerializer,
)


class UpdateOrganizationSettingView(generics.UpdateAPIView):
Expand All @@ -23,3 +27,20 @@ def get_object(self):
is_active=True,
)
return get_object_or_404(self.get_queryset(), organization=organization)

def update(self, request, *args, **kwargs):
partial = kwargs.pop("partial", False)
instance = self.get_object()
serializer = self.get_serializer(
instance,
data=request.data,
partial=partial,
)
serializer.is_valid(raise_exception=True)
updated_instance = serializer.save()
fresh_instance = self.get_queryset().get(pk=updated_instance.pk)
response_serializer = OrganizationConfigSerializer(
fresh_instance,
context=self.get_serializer_context(),
)
return Response(response_serializer.data)
Loading