rework from review feeback

src/firefighter/slack/messages/slack_messages.py

@@ -817,7 +817,7 @@ def get_text(self) -> str:
 
 
 class SlackMessageChannelReminderPostMortem(SlackMessageSurface):
-    id = "ff_reminder_post_mortem"
+    id = "ff_incident_postmortem_late_channel_reminder"
 
     def __init__(self, incident: Incident):
         self.incident = incident
@@ -835,20 +835,17 @@ def get_blocks(self) -> list[Block]:
             ),
             ContextBlock(
                 elements=[
-                    TextObject(
-                        type="mrkdwn",
+                    MarkdownTextObject(
                         text=":bulb: Please take a moment to complete the post-mortem process. "
-                        """
-                            `It's essential for continuous improvement!`
-                        """,
+                        "It's essential for continuous improvement",
                     )
                 ]
             ),
         ]
 
 
 class SlackMessageIncidentUpdateReminderCommander(SlackMessageSurface):
-    id = "ff_incident_reminder_commander"
+    id = "ff_incident_postmortem_commander_late_reminder"
 
     def __init__(self, incident: Incident, time_delta_fmt: str):
         self.incident = incident
@@ -857,9 +854,9 @@ def __init__(self, incident: Incident, time_delta_fmt: str):
 
     def get_blocks(self) -> list[Block]:
         if self.incident.priority.value >= 4:
-            text = f"Hey there ! You may want to close your this {self.incident.slack_channel_name} incident 📝"
+            text = f"Hey there ! You may want to close your this {self.incident.title} incident 📝"
         else:
-            text = f"It is a {self.incident.slack_channel_name} incident and a lot of people, customers and employees, are probably impacted. This incident has no update for {self.time_delta_fmt}. Please make sure it is done even if you are not the one doing it."
+            text = f"Hey there ! Just a reminder for this incident {self.incident.title}. He has no update since {self.time_delta_fmt}. Please make sure the postmortem is done even if you are not the one doing it."
 
         return [
             HeaderBlock(
@@ -869,8 +866,9 @@ def get_blocks(self) -> list[Block]:
                 )
             ),
             SectionBlock(text=MarkdownTextObject(text=text)),
-            SectionBlockUpdateIntent(self.incident),
         ]
 
     def get_text(self) -> str:
-        return "👋 Hi there! Just a reminder to ensure the postmortem for this incident"
+        return (
+            "👋 Hey there! Just a reminder to ensure the postmortem for this incident"
+        )

src/firefighter/slack/models/user.py

@@ -451,6 +451,22 @@ def send_private_message(
         **kwargs: Any,
     ) -> None:
         """Send a private message to the user."""
+        client.chat_postMessage(
+            channel=self.slack_id,
+            text=message.get_text(),
+            metadata=message.get_metadata(),
+            blocks=message.get_blocks(),
+            **kwargs,
+        )
+
+    @slack_client
+    def send_and_save_private_message(
+        self,
+        message: SlackMessageSurface,
+        client: WebClient = DefaultWebClient,
+        **kwargs: Any,
+    ) -> None:
+        """Send a private message to the user and save it to DB."""
         from firefighter.slack.models.conversation import (  # noqa: PLC0415
             Conversation,
             ConversationStatus,

src/firefighter/slack/tasks/reminder_postmortem.py

@@ -13,9 +13,12 @@
 from firefighter.slack.slack_app import DefaultWebClient, slack_client
 
 if TYPE_CHECKING:
+    from collections.abc import Iterable
+
     from slack_sdk.web.client import WebClient
 
     from firefighter.incidents.models.incident import Incident
+    from firefighter.incidents.models.incident_membership import IncidentRole
 
 
 logger = logging.getLogger(__name__)
@@ -77,20 +80,22 @@ def publish_fixed_next_actions(
     )
 
 
-def send_reminder(incident: Incident, *, to_channel: bool) -> None:
-    prefetched_roles = getattr(incident, "roles_prefetched", [])
+def send_reminder(
+    incident: Incident, commander_role: Iterable[IncidentRole], *, to_channel: bool
+) -> None:
+    """Sends a postmortem reminder related to an incident either through a channel or privately to the commander."""
     if to_channel:
         # Send a message in the incident channel
         incident.conversation.send_message_and_save(
             SlackMessageChannelReminderPostMortem(incident)
         )
     else:
         # Send a private message to commander
-        for role in prefetched_roles:
-            user = role.user
+        for commander in commander_role:
+            user = commander.user
             slack_user = user.slack_user
             if slack_user:
                 private_message = SlackMessageIncidentUpdateReminderCommander(
                     incident=incident, time_delta_fmt=str(incident.created_at)
                 )
-                slack_user.send_private_message(private_message)
+                slack_user.send_and_save_private_message(private_message)

src/firefighter/slack/tasks/send_reminders.py

@@ -128,8 +128,14 @@ def send_reminders() -> None:
     group(list_fn).apply_async()
 
 
-@shared_task(name="slack.send_reminders_to_commander")
-def send_reminders_to_commander() -> None:
+@shared_task(name="slack.send_postmortem_late_reminder")
+def send_postmortem_late_reminder() -> None:
+    """Sends reminder messages for incidents requiring postmortem updates.
+
+      - For each incident, it checks:
+    a. If a reminder to the commander was sent more than two days ago and no channel message exists, sends a new reminder to the incident's channel.
+    b. If no reminder has ever been sent to the commander and the last incident update was over two days ago, sends a direct reminder to the commander.
+    """
     # Skip if it's out of office hours
     if not is_during_office_hours(timezone.localtime()):
         logger.debug("out of office hours")
@@ -149,58 +155,62 @@ def send_reminders_to_commander() -> None:
                 queryset=IncidentRole.objects.all()
                 .order_by("id")
                 .select_related("role_type", "user", "user__slack_user")
-                .filter(role_type__slug="commander"),
-                to_attr="roles_prefetched",
+                .filter(role_type__slug="commander_roles"),
+                to_attr="commander",
             ),
         )
     )
 
     for incident in opened_incidents:
+        latest_update = None
         try:
             latest_update = IncidentUpdate.objects.filter(incident=incident).latest(
                 "updated_at"
             )
+        except IncidentUpdate.DoesNotExist:
+            logger.exception(f"No update found for incident: {incident.id}")
+            continue
 
-            existing_messages_commander = (
-                (
-                    Message.objects.filter(
-                        ff_type=SlackMessageIncidentUpdateReminderCommander.id,
-                        incident=incident,
-                        ts__gte=latest_update.updated_at - timedelta(days=2),
-                    )
+        existing_messages_commander = (
+            (
+                Message.objects.filter(
+                    ff_type=SlackMessageIncidentUpdateReminderCommander.id,
+                    incident=incident,
+                    ts__gte=latest_update.updated_at - timedelta(days=2),
                 )
-                .select_related("conversation")
-                .order_by("-ts")
-                .first()
             )
+            .select_related("conversation")
+            .order_by("-ts")
+            .first()
+        )
 
-            existing_messages_channel = (
-                (
-                    Message.objects.filter(
-                        ff_type=SlackMessageChannelReminderPostMortem.id,
-                        incident=incident,
-                    )
+        existing_messages_channel = (
+            (
+                Message.objects.filter(
+                    ff_type=SlackMessageChannelReminderPostMortem.id,
+                    incident=incident,
                 )
-                .select_related("conversation")
-                .exists()
             )
+            .select_related("conversation")
+            .exists()
+        )
 
-            time_since_update = timezone.now() - latest_update.updated_at
+        time_since_update = timezone.now() - latest_update.updated_at
+        commander_roles = incident.roles_set.all()
 
-            if existing_messages_commander:
-                time_since_last_commander_message = (
-                    timezone.now() - existing_messages_commander.ts
-                )
-                if (
-                    time_since_last_commander_message >= timedelta(days=2)
-                    and not existing_messages_channel
-                ):
-                    send_reminder(incident=incident, to_channel=True)
-
-            if not existing_messages_commander and time_since_update >= timedelta(
-                minutes=5
+        if existing_messages_commander:
+            time_since_last_commander_message = (
+                timezone.now() - existing_messages_commander.ts
+            )
+            if (
+                time_since_last_commander_message >= timedelta(days=3)
+                and not existing_messages_channel
             ):
-                send_reminder(incident=incident, to_channel=False)
+                send_reminder(
+                    incident=incident, commander_role=commander_roles, to_channel=True
+                )
 
-        except IncidentUpdate.DoesNotExist:
-            logger.exception(f"No update found for incident: {incident.id}")
+        if not existing_messages_commander and time_since_update >= timedelta(days=3):
+            send_reminder(
+                incident=incident, commander_role=commander_roles, to_channel=False
+            )