Merge pull request #2640 from carpentries/feature/2638-improvements-t…

…o-scheduled-emails-list-and-details [Emails] Improvements to scheduled emails list and details
carpentries · Apr 22, 2024 · db8b8be · db8b8be
2 parents bccb715 + b0ed1b4
commit db8b8be
Show file tree

Hide file tree

Showing 9 changed files with 74 additions and 55 deletions.
diff --git a/Makefile b/Makefile
@@ -50,7 +50,7 @@ node_modules : package.json
 ## serve        : run a server
 serve :
 	gunicorn \
-		--workers=4 \
+		--workers=1 \
 		--bind=127.0.0.1:8000 \
 		--access-logfile - \
 		--capture-output \

diff --git a/Pipfile b/Pipfile
@@ -35,7 +35,7 @@ rq-scheduler = "~=0.13.0"
 social-auth-core = "~=4.1.0"
 social-auth-app-django = "~=5.0.0"
 gunicorn = "~=22.0.0"
-whitenoise = "~=6.1"
+whitenoise = "~=6.6"
 django-better-admin-arrayfield = "==1.4.2"
 django-test-migrations = "~=1.3.0"
 typing-extensions = "*"

diff --git a/Pipfile.lock b/Pipfile.lock
diff --git a/amy/emails/tests/test_utils.py b/amy/emails/tests/test_utils.py
@@ -186,17 +186,20 @@ def test_messages_action_scheduled(self, mock_messages_info) -> None:
         signal_name = "test_signal"
         scheduled_at = timezone.now()
         scheduled_email = ScheduledEmail(scheduled_at=scheduled_at)
+        name = (
+            scheduled_email.template.name if scheduled_email.template else signal_name
+        )
 
         # Act
         messages_action_scheduled(request, signal_name, scheduled_email)
 
         # Assert
         mock_messages_info.assert_called_once_with(
             request,
-            f"New email action ({signal_name}) was scheduled to run "
+            f"New email action was scheduled to run "
             f'<relative-time datetime="{scheduled_at}"></relative-time>: '
             f'<a href="{scheduled_email.get_absolute_url()}"><code>'
-            f"{scheduled_email.pk}</code></a>.",
+            f"{name}</code></a>.",
             extra_tags=settings.ONLY_FOR_ADMINS_TAG,
         )
 
@@ -209,6 +212,9 @@ def test_messages_action_updated(self, mock_messages_info) -> None:
         signal_name = "test_signal"
         scheduled_at = timezone.now()
         scheduled_email = ScheduledEmail(scheduled_at=scheduled_at)
+        name = (
+            scheduled_email.template.name if scheduled_email.template else signal_name
+        )
 
         # Act
         messages_action_updated(request, signal_name, scheduled_email)
@@ -217,7 +223,7 @@ def test_messages_action_updated(self, mock_messages_info) -> None:
         mock_messages_info.assert_called_once_with(
             request,
             f'Existing <a href="{scheduled_email.get_absolute_url()}">email action '
-            f"({signal_name})</a> was updated.",
+            f"({name})</a> was updated.",
             extra_tags=settings.ONLY_FOR_ADMINS_TAG,
         )
 
@@ -230,6 +236,9 @@ def test_messages_action_cancelled(self, mock_messages_warning) -> None:
         signal_name = "test_signal"
         scheduled_at = timezone.now()
         scheduled_email = ScheduledEmail(scheduled_at=scheduled_at)
+        name = (
+            scheduled_email.template.name if scheduled_email.template else signal_name
+        )
 
         # Act
         messages_action_cancelled(request, signal_name, scheduled_email)
@@ -238,7 +247,7 @@ def test_messages_action_cancelled(self, mock_messages_warning) -> None:
         mock_messages_warning.assert_called_once_with(
             request,
             f'Existing <a href="{scheduled_email.get_absolute_url()}">email action '
-            f"({signal_name})</a> was cancelled.",
+            f"({name})</a> was cancelled.",
             extra_tags=settings.ONLY_FOR_ADMINS_TAG,
         )
 

diff --git a/amy/emails/utils.py b/amy/emails/utils.py
@@ -85,16 +85,16 @@ def messages_missing_template_link(
 def messages_action_scheduled(
     request: HttpRequest, signal_name: str, scheduled_email: ScheduledEmail
 ) -> None:
+    name = scheduled_email.template.name if scheduled_email.template else signal_name
     messages.info(
         request,
         format_html(
-            "New email action ({}) was scheduled to run "
+            "New email action was scheduled to run "
             '<relative-time datetime="{}"></relative-time>: '
             '<a href="{}"><code>{}</code></a>.',
-            signal_name,
             scheduled_email.scheduled_at,
             scheduled_email.get_absolute_url(),
-            scheduled_email.pk,
+            name,
         ),
         extra_tags=settings.ONLY_FOR_ADMINS_TAG,
     )
@@ -103,12 +103,13 @@ def messages_action_scheduled(
 def messages_action_updated(
     request: HttpRequest, signal_name: str, scheduled_email: ScheduledEmail
 ) -> None:
+    name = scheduled_email.template.name if scheduled_email.template else signal_name
     messages.info(
         request,
         format_html(
             'Existing <a href="{}">email action ({})</a> was updated.',
             scheduled_email.get_absolute_url(),
-            signal_name,
+            name,
         ),
         extra_tags=settings.ONLY_FOR_ADMINS_TAG,
     )
@@ -117,12 +118,13 @@ def messages_action_updated(
 def messages_action_cancelled(
     request: HttpRequest, signal_name: str, scheduled_email: ScheduledEmail
 ) -> None:
+    name = scheduled_email.template.name if scheduled_email.template else signal_name
     messages.warning(
         request,
         format_html(
             'Existing <a href="{}">email action ({})</a> was cancelled.',
             scheduled_email.get_absolute_url(),
-            signal_name,
+            name,
         ),
         extra_tags=settings.ONLY_FOR_ADMINS_TAG,
     )

diff --git a/amy/recruitment/models.py b/amy/recruitment/models.py
@@ -161,3 +161,8 @@ def __str__(self) -> str:
             f"Instructor Signup applicant {self.person.full_name} "
             f"for {self.recruitment.event}"
         )
+
+    def get_absolute_url(self) -> str:
+        return reverse(
+            "instructorrecruitment_details", kwargs={"pk": self.recruitment.pk}
+        )
diff --git a/amy/templates/emails/scheduled_email_detail.html b/amy/templates/emails/scheduled_email_detail.html
@@ -57,8 +57,20 @@ <h1>
 
 <table class="table table-striped">
   <tr>
-    <th width="20%">ID:</th>
-    <td colspan="2">{{ scheduled_email.id }}</td>
+    <th>Email template:</th>
+    <td colspan="2"><a href="{{ scheduled_email.template.get_absolute_url }}">{{ scheduled_email.template }}</a></td>
+  </tr>
+  <tr>
+    <th>Related object:</th>
+    <td colspan="2">
+      {% if scheduled_email.generic_relation and scheduled_email.generic_relation.get_absolute_url %}
+      <a href="{{ scheduled_email.generic_relation.get_absolute_url }}">{{ scheduled_email.generic_relation }}</a>
+      {% elif scheduled_email.generic_relation %}
+      {{ scheduled_email.generic_relation }}
+      {% else %}
+      &mdash;
+      {% endif %}
+    </td>
   </tr>
   <tr>
     <th>State:</th>
@@ -141,20 +153,8 @@ <h1>
     </td>
   </tr>
   <tr>
-    <th>Email template:</th>
-    <td colspan="2"><a href="{{ scheduled_email.template.get_absolute_url }}">{{ scheduled_email.template }}</a></td>
-  </tr>
-  <tr>
-    <th>Related object:</th>
-    <td colspan="2">
-      {% if scheduled_email.generic_relation and scheduled_email.generic_relation.get_absolute_url %}
-      <a href="{{ scheduled_email.generic_relation.get_absolute_url }}">{{ scheduled_email.generic_relation }}</a>
-      {% elif scheduled_email.generic_relation %}
-      {{ scheduled_email.generic_relation }}
-      {% else %}
-      &mdash;
-      {% endif %}
-    </td>
+    <th width="20%">ID:</th>
+    <td colspan="2">{{ scheduled_email.id }}</td>
   </tr>
 </table>
 

diff --git a/amy/templates/emails/scheduled_email_list.html b/amy/templates/emails/scheduled_email_list.html
@@ -7,19 +7,30 @@
 {% if scheduled_emails %}
     <table class="table table-striped">
       <tr>
-        <th>ID</th>
+        <th>Name</th>
         <th>State</th>
         <th>Scheduled at</th>
+        <th>Related object</th>
         <th>To</th>
         <th>From</th>
         <th>Subject</th>
-        <th>Template</th>
       </tr>
     {% for email in scheduled_emails %}
       <tr>
-        <td><a href="{{ email.get_absolute_url }}">{{ email.id }}</a></td>
+        <td>
+          <a href="{{ email.get_absolute_url }}">{{ email.template.name }}</a>
+        </td>
         <td class="text-{% if email.state == 'failed' %}danger{% elif email.state == 'succeeded'%}success{% elif email.state == 'cancelled' %}secondary{% else %}info{% endif %}">{{ email.state }}</td>
         <td>{{ email.scheduled_at }}</td>
+        <td>
+          {% if email.generic_relation and email.generic_relation.get_absolute_url %}
+          <a href="{{ email.generic_relation.get_absolute_url }}">{{ email.generic_relation }}</a>
+          {% elif email.generic_relation %}
+          {{ email.generic_relation }}
+          {% else %}
+          &mdash;
+          {% endif %}
+        </td>
         <td>
             {% if email.to_header %}
             <ul>
@@ -33,7 +44,6 @@
         </td>
         <td>{{ email.from_header }}</td>
         <td>{{ email.subject }}</td>
-        <td><a href="{{ email.template.get_absolute_url }}">{{ email.template.name }}</a></td>
       </tr>
     {% endfor %}
     </table>

diff --git a/amy/workshops/models.py b/amy/workshops/models.py
@@ -1869,6 +1869,9 @@ def __str__(self):
             self.person, self.badge, self.awarded, self.event
         )
 
+    def get_absolute_url(self) -> str:
+        return reverse("person_details", args=[self.person.pk])
+
 
 # ------------------------------------------------------------