Fix pre-registration review issues

Author: snu-BuMinSeong

wacruit/src/apps/pre_registration/repositories.py

@@ -1,5 +1,6 @@
 from fastapi import Depends
 from sqlalchemy import delete
+from sqlalchemy import func
 from sqlalchemy import select
 from sqlalchemy import true
 from sqlalchemy.orm import Session
@@ -59,6 +60,22 @@ def create_pre_registration_user(
             self.session.add(pre_registration_user)
         return pre_registration_user
 
+    def count_pre_registration_users(
+        self,
+        pre_registration_id: int | None,
+        active_only: bool,
+    ) -> int:
+        query = (
+            select(func.count()).select_from(PreRegistrationUser).join(PreRegistration)
+        )
+        if pre_registration_id is not None:
+            query = query.where(
+                PreRegistrationUser.pre_registration_id == pre_registration_id
+            )
+        if active_only:
+            query = query.where(PreRegistration.is_active == true())
+        return int(self.session.execute(query).scalar_one())
+
     def get_pre_registration_users(
         self,
         pre_registration_id: int | None,

wacruit/src/apps/pre_registration/schemas.py

@@ -61,3 +61,4 @@ class SendPreRegistrationEmailResponse(BaseModel):
     total_count: int
     queued_count: int
     recipient_limit: int
+    is_truncated: bool

wacruit/src/apps/pre_registration/services.py

@@ -1,3 +1,4 @@
+import logging
 from typing import Annotated
 
 from fastapi import BackgroundTasks
@@ -23,6 +24,8 @@
 from wacruit.src.apps.pre_registration.schemas import SendPreRegistrationEmailResponse
 from wacruit.src.apps.pre_registration.schemas import UpdatePreRegistrationRequest
 
+logger = logging.getLogger(__name__)
+
 PRE_REGISTRATION_EMAIL_BATCH_LIMIT = 200
 
 
@@ -155,6 +158,10 @@ def send_email_to_pre_registration_users(
         request: SendPreRegistrationEmailRequest,
         background_tasks: BackgroundTasks,
     ) -> SendPreRegistrationEmailResponse:
+        total_count = self.pre_registration_repository.count_pre_registration_users(
+            pre_registration_id=request.pre_registration_id,
+            active_only=request.active_only,
+        )
         pre_registration_users = (
             self.pre_registration_repository.get_pre_registration_users(
                 pre_registration_id=request.pre_registration_id,
@@ -176,9 +183,10 @@ def send_email_to_pre_registration_users(
 
         return SendPreRegistrationEmailResponse(
             status="queued",
-            total_count=len(recipient_emails),
+            total_count=total_count,
             queued_count=len(recipient_emails),
             recipient_limit=PRE_REGISTRATION_EMAIL_BATCH_LIMIT,
+            is_truncated=total_count > len(recipient_emails),
         )
 
     def _send_email_to_recipients(
@@ -196,5 +204,15 @@ def _send_email_to_recipients(
                     content=content,
                     html_content=html_content,
                 )
-            except (MailConfigException, MailSendFailedException):
+            except MailConfigException:
+                logger.exception(
+                    "Pre-registration email configuration failed for recipient %s",
+                    recipient_email,
+                )
+                break
+            except MailSendFailedException:
+                logger.exception(
+                    "Failed to send pre-registration email to recipient %s",
+                    recipient_email,
+                )
                 continue

wacruit/src/database/connection.py

@@ -64,5 +64,9 @@ def __exit__(self, exc_type, exc_val, exc_tb) -> bool:
             self._session.rollback()
             return False
 
-        self._session.flush()
+        try:
+            self._session.flush()
+        except Exception:
+            self._session.rollback()
+            raise
         return True

wacruit/src/tests/pre_registration/conftest.py

@@ -3,6 +3,7 @@
 import pytest
 from sqlalchemy.orm import Session
 
+from wacruit.src.apps.mail.exceptions import MailConfigException
 from wacruit.src.apps.mail.exceptions import MailSendFailedException
 from wacruit.src.apps.mail.services import EmailService
 from wacruit.src.apps.pre_registration.models import PreRegistration
@@ -17,6 +18,7 @@ class FakeEmailService(EmailService):
     def __init__(self) -> None:
         self.sent_emails: list[tuple[str, str, str, str | None]] = []
         self.failed_emails: set[str] = set()
+        self.config_failed_emails: set[str] = set()
 
     def send_email(
         self,
@@ -25,6 +27,8 @@ def send_email(
         content: str,
         html_content: str | None = None,
     ) -> None:
+        if to_email in self.config_failed_emails:
+            raise MailConfigException()
         if to_email in self.failed_emails:
             raise MailSendFailedException()
         self.sent_emails.append((to_email, subject, content, html_content))

wacruit/src/tests/pre_registration/test_pre_resgistration_service.py

@@ -1,3 +1,4 @@
+import logging
 from typing import List
 from typing import cast
 
@@ -215,6 +216,15 @@ def test_create_duplicate_pre_registration_user(
     with pytest.raises(PreRegistUserAlreadyExistException):
         pre_registration_service.create_pre_registration_user(request)
 
+    users = pre_registration_service.get_pre_registration_users(
+        pre_registration_id=created_active_pre_registration.id,
+        active_only=False,
+        limit=50,
+        offset=0,
+    )
+
+    assert users == []
+
 
 def test_get_active_pre_registration_users(
     db_session: Session,
@@ -319,6 +329,7 @@ def test_send_email_to_active_pre_registration_users_queues_background_task(
     assert response.total_count == 1
     assert response.queued_count == 1
     assert response.recipient_limit == PRE_REGISTRATION_EMAIL_BATCH_LIMIT
+    assert response.is_truncated is False
     assert fake_email_service.sent_emails == []
     assert len(background_tasks.tasks) == 1
 
@@ -361,12 +372,59 @@ def test_send_email_to_pre_registration_users_applies_recipient_cap(
     )
 
     assert response.status == "queued"
-    assert response.total_count == PRE_REGISTRATION_EMAIL_BATCH_LIMIT
+    assert response.total_count == user_count
     assert response.queued_count == PRE_REGISTRATION_EMAIL_BATCH_LIMIT
     assert response.recipient_limit == PRE_REGISTRATION_EMAIL_BATCH_LIMIT
+    assert response.is_truncated is True
     assert fake_email_service.sent_emails == []
 
     [background_task] = background_tasks.tasks
     background_task.func(*background_task.args, **background_task.kwargs)
 
     assert len(fake_email_service.sent_emails) == PRE_REGISTRATION_EMAIL_BATCH_LIMIT
+
+
+def test_send_email_to_recipients_logs_send_failure_and_continues(
+    pre_registration_service: PreRegistrationService,
+    fake_email_service: FakeEmailService,
+    caplog: pytest.LogCaptureFixture,
+):
+    fake_email_service.failed_emails.add("failed@example.com")
+
+    with caplog.at_level(
+        logging.ERROR, logger="wacruit.src.apps.pre_registration.services"
+    ):
+        pre_registration_service._send_email_to_recipients(
+            recipient_emails=["failed@example.com", "success@example.com"],
+            subject="subject",
+            content="content",
+            html_content=None,
+        )
+
+    assert fake_email_service.sent_emails == [
+        ("success@example.com", "subject", "content", None)
+    ]
+    assert "Failed to send pre-registration email" in caplog.text
+    assert "failed@example.com" in caplog.text
+
+
+def test_send_email_to_recipients_logs_config_failure_and_stops(
+    pre_registration_service: PreRegistrationService,
+    fake_email_service: FakeEmailService,
+    caplog: pytest.LogCaptureFixture,
+):
+    fake_email_service.config_failed_emails.add("config@example.com")
+
+    with caplog.at_level(
+        logging.ERROR, logger="wacruit.src.apps.pre_registration.services"
+    ):
+        pre_registration_service._send_email_to_recipients(
+            recipient_emails=["config@example.com", "skipped@example.com"],
+            subject="subject",
+            content="content",
+            html_content=None,
+        )
+
+    assert fake_email_service.sent_emails == []
+    assert "Pre-registration email configuration failed" in caplog.text
+    assert "config@example.com" in caplog.text