Address PR review comments

- Move imports to top of file instead of inside function
- Update _STATUS_LABELS in models to use Bootstrap 5 class names directly
- Remove mapping logic from template tags
- Remove unnecessary comments from JavaScript
- Use relevant_commitfests() in context processor for efficiency
- Revert close button to simple Bootstrap 5 btn-close styling
- Reuse existing cfs variable in home view instead of querying again

Author: JelteF

media/commitfest/css/commitfest.css

@@ -146,3 +146,9 @@ a:hover .badge {
 .navbar-dark .github-logo {
     filter: brightness(0) invert(1);
 }
+
+/* Simple close button without float */
+.btn-close-nofloat {
+    float: none;
+    margin-left: 0.5rem;
+}

media/commitfest/js/commitfest.js

@@ -280,11 +280,7 @@ function sortpatches(sortby) {
 function toggleButtonCollapse(buttonId, collapseId) {
     const button = document.getElementById(buttonId);
     const collapse = document.getElementById(collapseId);
-
-    // Toggle button active state
     button.classList.toggle("active");
-
-    // Use Bootstrap 5 Collapse API
     const bsCollapse = new bootstrap.Collapse(collapse, {
         toggle: true,
     });

pgcommitfest/commitfest/context_processors.py

@@ -3,7 +3,8 @@
 
 def global_context(request):
     """Add global context variables available in all templates."""
+    cfs = CommitFest.relevant_commitfests()
     return {
-        "current_cf": CommitFest.get_current(),
-        "open_cf": CommitFest.get_open_regular(),
+        "current_cf": cfs.get("in_progress") or cfs.get("open"),
+        "open_cf": cfs.get("open"),
     }

pgcommitfest/commitfest/models.py

@@ -535,7 +535,7 @@ class PatchOnCommitFest(models.Model):
         (STATUS_WITHDRAWN, "Withdrawn"),
     )
     _STATUS_LABELS = (
-        (STATUS_REVIEW, "default"),
+        (STATUS_REVIEW, "secondary"),
         (STATUS_AUTHOR, "primary"),
         (STATUS_COMMITTER, "info"),
         (STATUS_COMMITTED, "success"),

pgcommitfest/commitfest/templates/patch.html

@@ -135,7 +135,7 @@
      {%endif%}
      <dl>
       {%for t in patch.mailthread_set.all%}
-       <dt><a href="https://www.postgresql.org/message-id/flat/{{t.messageid}}">{{t.subject}}</a> <button type="button" class="btn btn-outline-danger btn-sm ms-2" title="Detach this thread" onclick="detachThread({{cf.id}},{{patch.id}},'{{t.messageid}}')">×</button></dt>
+       <dt><a href="https://www.postgresql.org/message-id/flat/{{t.messageid}}">{{t.subject}}</a> <button type="button" class="btn-close btn-close-nofloat" title="Detach this thread" onclick="detachThread({{cf.id}},{{patch.id}},'{{t.messageid}}')"></button></dt>
        <dd>
         First at <a href="https://www.postgresql.org/message-id/{{t.messageid}}">{{t.firstmessage}}</a> by {{t.firstauthor|hidemail}}<br/>
         Latest at <a href="https://www.postgresql.org/message-id/{{t.latestmsgid}}">{{t.latestmessage}}</a> by {{t.latestauthor|hidemail}}<br/>

pgcommitfest/commitfest/templatetags/commitfest.py

@@ -25,17 +25,7 @@ def commitfeststatusstring(value):
 @stringfilter
 def commitfeststatuslabel(value):
     i = int(value)
-    bootstrap3_class = [v for k, v in CommitFest._STATUS_LABELS if k == i][0]
-    # Map Bootstrap 3 label classes to Bootstrap 5 badge classes
-    mapping = {
-        "default": "secondary",
-        "primary": "primary",
-        "info": "info",
-        "success": "success",
-        "warning": "warning",
-        "danger": "danger",
-    }
-    return mapping.get(bootstrap3_class, "secondary")
+    return [v for k, v in CommitFest._STATUS_LABELS if k == i][0]
 
 
 @register.filter(name="patchstatusstring")
@@ -49,17 +39,7 @@ def patchstatusstring(value):
 @stringfilter
 def patchstatuslabel(value):
     i = int(value)
-    bootstrap3_class = [v for k, v in PatchOnCommitFest._STATUS_LABELS if k == i][0]
-    # Map Bootstrap 3 label classes to Bootstrap 5 badge classes
-    mapping = {
-        "default": "secondary",
-        "primary": "primary",
-        "info": "info",
-        "success": "success",
-        "warning": "warning",  # Keep warning but we'll override the color in CSS
-        "danger": "danger",
-    }
-    return mapping.get(bootstrap3_class, "secondary")
+    return [v for k, v in PatchOnCommitFest._STATUS_LABELS if k == i][0]
 
 
 @register.filter(name="tagname")

pgcommitfest/commitfest/views.py

@@ -11,13 +11,14 @@
     HttpResponseRedirect,
 )
 from django.shortcuts import get_object_or_404, render
+from django.utils import timezone
 from django.views.decorators.csrf import csrf_exempt
 
 import collections
 import hmac
 import json
 import urllib
-from datetime import datetime
+from datetime import datetime, timedelta
 from email.mime.text import MIMEText
 from email.utils import formatdate, make_msgid
 
@@ -60,10 +61,6 @@ def home(request):
     # Add dashboard content for logged-in users (same as me() view)
     if request.user.is_authenticated:
         # Check if user is experienced (has been active for a while)
-        from django.db import connection
-        from django.utils import timezone
-
-        from datetime import timedelta
 
         # Consider user experienced if they joined more than 30 days ago
         is_experienced_user = request.user.date_joined < timezone.now() - timedelta(
@@ -74,16 +71,9 @@ def home(request):
 
         curs = connection.cursor()
         curs.execute("SET TRANSACTION ISOLATION LEVEL REPEATABLE READ")
-        current_cfs = list(
-            CommitFest.objects.filter(status=CommitFest.STATUS_INPROGRESS)
-        )
-        if len(current_cfs) == 0:
-            current_cfs = list(CommitFest.objects.filter(status=CommitFest.STATUS_OPEN))
 
-        if len(current_cfs) > 0:
-            cf = current_cfs[0]
-        else:
-            cf = None
+        # Use existing cfs data instead of querying again
+        cf = cfs.get("in_progress") or cfs.get("open")
 
         form = CommitFestFilterForm(request.GET)
         patch_list = patchlist(request, cf, personalized=True)