Rename things for clarity

api/src/zimitfrontend/routes/hook.py

@@ -4,7 +4,7 @@
 
 from zimitfrontend.constants import logger
 from zimitfrontend.routes.schemas import HookStatus, ZimfarmTask
-from zimitfrontend.routes.utils import SUCCESS, convert_hook_to_mail
+from zimitfrontend.routes.utils import SUCCESS, process_zimfarm_hook_call
 from zimitfrontend.utils import send_email_via_mailgun
 
 router = APIRouter(
@@ -29,12 +29,19 @@ def webhook(
     task: ZimfarmTask | None = None,
 ) -> HookStatus:
 
-    result = convert_hook_to_mail(token, target, lang, task)
-    if result.status == SUCCESS and result.target and result.subject and result.body:
+    result = process_zimfarm_hook_call(token, target, lang, task)
+    if (
+        result.hook_response_status == SUCCESS
+        and result.mail_target
+        and result.mail_subject
+        and result.mail_body
+    ):
         try:
-            resp = send_email_via_mailgun(result.target, result.subject, result.body)
+            resp = send_email_via_mailgun(
+                result.mail_target, result.mail_subject, result.mail_body
+            )
             if resp:
                 logger.info(f"Mailgun notif sent: {resp}")
         except Exception as exc:
             logger.error(f"Failed to send mailgun notif: {exc}", exc_info=exc)
-    return result.status
+    return result.hook_response_status

api/src/zimitfrontend/routes/schemas.py

@@ -81,8 +81,8 @@ class HookStatus(BaseModel):
     status: str
 
 
-class MailToSend(BaseModel):
-    status: HookStatus
-    target: str | None = None
-    subject: str | None = None
-    body: str | None = None
+class HookProcessingResult(BaseModel):
+    hook_response_status: HookStatus
+    mail_target: str | None = None
+    mail_subject: str | None = None
+    mail_body: str | None = None

api/src/zimitfrontend/routes/utils.py

@@ -3,8 +3,8 @@
 from zimitfrontend.constants import ApiConfiguration, logger
 from zimitfrontend.i18n import change_locale
 from zimitfrontend.routes.schemas import (
+    HookProcessingResult,
     HookStatus,
-    MailToSend,
     TaskInfo,
     TaskInfoFlag,
     ZimfarmTask,
@@ -81,9 +81,9 @@ def get_task_info(task: Any) -> TaskInfo:
     )
 
 
-def convert_hook_to_mail(
+def process_zimfarm_hook_call(
     token: str | None, target: str | None, lang: str, task: ZimfarmTask | None
-) -> MailToSend:
+) -> HookProcessingResult:
     """Transforms message received from Zimfarm via hook to mail info
 
     Returned object is ready to be sent (or contains information that failure occured)
@@ -94,22 +94,23 @@ def convert_hook_to_mail(
     # otherwises exposes us to spam abuse
     if token != ApiConfiguration.hook_token:
         logger.error(f"Incorrect token value received in POST /hook: {token}")
-        return MailToSend(status=FAILED)
+        return HookProcessingResult(hook_response_status=FAILED)
 
     # without a `target` arg, we have nowhere to send the notification to
     if not target:
         logger.error(f"Incorrect target received in POST /hook: {target}")
-        return MailToSend(status=FAILED)
+        return HookProcessingResult(hook_response_status=FAILED)
 
     if not task:
         logger.error("No task received in POST /hook body")
-        return MailToSend(status=FAILED)
+        return HookProcessingResult(hook_response_status=FAILED)
 
     # discard hooks registered for events we don't plan on sending email for
+    # hook_response_status is still SUCCESS because hook call is valid
     if task.status not in ("requested", "succeeded", "failed", "canceled"):
-        return MailToSend(status=SUCCESS)
+        return HookProcessingResult(hook_response_status=SUCCESS)
 
-    # force fail status, see https://github.com/openzim/zimit-frontend/issues/90
+    # force task fail status, see https://github.com/openzim/zimit-frontend/issues/90
     if task.files is None or len(task.files) == 0:
         task.status = "failed"
 
@@ -130,4 +131,9 @@ def convert_hook_to_mail(
         jinja_env.get_template("email_subject.txt").render(**context).replace("\n", "")
     )
     body = jinja_env.get_template("email_body.html").render(**context)
-    return MailToSend(status=SUCCESS, target=target, subject=subject, body=body)
+    return HookProcessingResult(
+        hook_response_status=SUCCESS,
+        mail_target=target,
+        mail_subject=subject,
+        mail_body=body,
+    )

api/tests/unit/routes/test_utils.py

@@ -3,12 +3,17 @@
 import pytest
 
 from zimitfrontend.constants import ApiConfiguration
-from zimitfrontend.routes.schemas import MailToSend, TaskInfo, TaskInfoFlag, ZimfarmTask
+from zimitfrontend.routes.schemas import (
+    HookProcessingResult,
+    TaskInfo,
+    TaskInfoFlag,
+    ZimfarmTask,
+)
 from zimitfrontend.routes.utils import (
     FAILED,
     SUCCESS,
-    convert_hook_to_mail,
     get_task_info,
+    process_zimfarm_hook_call,
 )
 
 
@@ -135,11 +140,11 @@ def test_convert_zimfarm_task_to_info(task: Any, expected: TaskInfo):
             "en",
             DEFAULT_HOOK_TASK,
             "requested",
-            MailToSend(
-                status=SUCCESS,
-                target="bob@acme.com",
-                subject="Youzim.it task 6341c requested",
-                body=r"""<html  lang="en">
+            HookProcessingResult(
+                hook_response_status=SUCCESS,
+                mail_target="bob@acme.com",
+                mail_subject="Youzim.it task 6341c requested",
+                mail_body=r"""<html  lang="en">
 <body>
 
 <h2>ZIM requested!</h2>
@@ -165,11 +170,11 @@ def test_convert_zimfarm_task_to_info(task: Any, expected: TaskInfo):
             "en",
             DEFAULT_HOOK_TASK,
             "succeeded",
-            MailToSend(
-                status=SUCCESS,
-                target="bob@acme.com",
-                subject="Youzim.it task 6341c succeeded",
-                body=r"""<html  lang="en">
+            HookProcessingResult(
+                hook_response_status=SUCCESS,
+                mail_target="bob@acme.com",
+                mail_subject="Youzim.it task 6341c succeeded",
+                mail_body=r"""<html  lang="en">
 <body>
 
 
@@ -203,11 +208,11 @@ def test_convert_zimfarm_task_to_info(task: Any, expected: TaskInfo):
             "en",
             DEFAULT_HOOK_TASK,
             "failed",
-            MailToSend(
-                status=SUCCESS,
-                target="bob@acme.com",
-                subject="Youzim.it task 6341c failed",
-                body=r"""<html  lang="en">
+            HookProcessingResult(
+                hook_response_status=SUCCESS,
+                mail_target="bob@acme.com",
+                mail_subject="Youzim.it task 6341c failed",
+                mail_body=r"""<html  lang="en">
 <body>
 
 
@@ -235,11 +240,11 @@ def test_convert_zimfarm_task_to_info(task: Any, expected: TaskInfo):
             "en",
             DEFAULT_HOOK_TASK,
             "bad_status",
-            MailToSend(
-                status=SUCCESS,
-                target=None,
-                subject=None,
-                body=None,
+            HookProcessingResult(
+                hook_response_status=SUCCESS,
+                mail_target=None,
+                mail_subject=None,
+                mail_body=None,
             ),
             id="bad_status",
         ),
@@ -249,11 +254,11 @@ def test_convert_zimfarm_task_to_info(task: Any, expected: TaskInfo):
             "en",
             DEFAULT_HOOK_TASK,
             "succeeded",
-            MailToSend(
-                status=FAILED,
-                target=None,
-                subject=None,
-                body=None,
+            HookProcessingResult(
+                hook_response_status=FAILED,
+                mail_target=None,
+                mail_subject=None,
+                mail_body=None,
             ),
             id="bad_target1",
         ),
@@ -263,11 +268,11 @@ def test_convert_zimfarm_task_to_info(task: Any, expected: TaskInfo):
             "en",
             DEFAULT_HOOK_TASK,
             "succeeded",
-            MailToSend(
-                status=FAILED,
-                target=None,
-                subject=None,
-                body=None,
+            HookProcessingResult(
+                hook_response_status=FAILED,
+                mail_target=None,
+                mail_subject=None,
+                mail_body=None,
             ),
             id="bad_target2",
         ),
@@ -277,11 +282,11 @@ def test_convert_zimfarm_task_to_info(task: Any, expected: TaskInfo):
             "en",
             DEFAULT_HOOK_TASK,
             "succeeded",
-            MailToSend(
-                status=FAILED,
-                target=None,
-                subject=None,
-                body=None,
+            HookProcessingResult(
+                hook_response_status=FAILED,
+                mail_target=None,
+                mail_subject=None,
+                mail_body=None,
             ),
             id="bad_token",
         ),
@@ -291,11 +296,11 @@ def test_convert_zimfarm_task_to_info(task: Any, expected: TaskInfo):
             "en",
             None,
             "succeeded",
-            MailToSend(
-                status=FAILED,
-                target=None,
-                subject=None,
-                body=None,
+            HookProcessingResult(
+                hook_response_status=FAILED,
+                mail_target=None,
+                mail_subject=None,
+                mail_body=None,
             ),
             id="bad_task",
         ),
@@ -307,17 +312,17 @@ def test_convert_hook_to_mail(
     lang: str,
     task: ZimfarmTask | None,
     task_status: str,
-    expected: MailToSend,
+    expected: HookProcessingResult,
 ):
     if task:
         task.status = task_status
-    result = convert_hook_to_mail(
+    result = process_zimfarm_hook_call(
         token=token,
         target=target,
         lang=lang,
         task=task,
     )
-    assert result.status == expected.status
-    assert result.target == expected.target
-    assert result.subject == expected.subject
-    assert result.body == expected.body
+    assert result.hook_response_status == expected.hook_response_status
+    assert result.mail_target == expected.mail_target
+    assert result.mail_subject == expected.mail_subject
+    assert result.mail_body == expected.mail_body