From ab5429a66410966cebbce2616e216abf02f50b32 Mon Sep 17 00:00:00 2001 From: Natalia <124304+nessita@users.noreply.github.com> Date: Wed, 8 Apr 2026 12:07:49 -0300 Subject: Allowed creation of site-wide banners via the admin interface. Fixes #1550. A banner requires a title, and optionally HTML body and CTA label/URL. A banner can be active or inactive, and only one can be active at a time. Banners can be previewed from the admin via the "View on site" options on a Banner's detail page. This initial implementation is intentionally minimal to provide a robust but flexible MVP, with the goal to gather feedback for future iterations. Future improvements could include: - Active since and until dates. - Flexible CTA URL handling, such as URL names or local URLs. Thanks to Sarahs for the reviews. --- aggregator/tests.py | 4 +- djangoproject/scss/_style.scss | 6 + djangoproject/templates/base.html | 4 +- djangoproject/templates/foundation/banner.html | 9 ++ .../templates/foundation/banner_preview.html | 9 ++ djangoproject/templates/fundraising/index.html | 2 + djangoproject/urls/www.py | 7 +- foundation/admin.py | 29 +++++ foundation/migrations/0008_banner.py | 92 ++++++++++++++ foundation/models.py | 82 +++++++++++- foundation/templatetags/banner.py | 10 ++ foundation/tests.py | 138 ++++++++++++++++++++- foundation/views.py | 8 ++ 13 files changed, 391 insertions(+), 9 deletions(-) create mode 100644 djangoproject/templates/foundation/banner.html create mode 100644 djangoproject/templates/foundation/banner_preview.html create mode 100644 foundation/migrations/0008_banner.py create mode 100644 foundation/templatetags/banner.py diff --git a/aggregator/tests.py b/aggregator/tests.py index afe67b0c..f5716c58 100644 --- a/aggregator/tests.py +++ b/aggregator/tests.py @@ -92,7 +92,7 @@ class AggregatorTests(TestCase): def test_community_index_number_of_queries(self): """Intended to prevent an n+1 issue on the community index view""" url = reverse("community-index") - with self.assertNumQueries(8): + with self.assertNumQueries(9): self.client.get(url) def test_empty_feed_type_not_rendered(self): @@ -119,7 +119,7 @@ class AggregatorTests(TestCase): url = reverse( "community-feed-list", kwargs={"feed_type_slug": self.feed_type.slug} ) - with self.assertNumQueries(8): + with self.assertNumQueries(9): self.client.get(url) def test_management_command_sends_no_email_with_no_pending_feeds(self): diff --git a/djangoproject/scss/_style.scss b/djangoproject/scss/_style.scss index 6a8565d9..be2c29de 100644 --- a/djangoproject/scss/_style.scss +++ b/djangoproject/scss/_style.scss @@ -711,6 +711,12 @@ header { } } + .banner-preview-notice { + background-color: $warning-bg; + text-align: center; + margin: 0; + padding: 0; + } a, h1 a { color: $black; diff --git a/djangoproject/templates/base.html b/djangoproject/templates/base.html index ec6d3399..d6860ff8 100644 --- a/djangoproject/templates/base.html +++ b/djangoproject/templates/base.html @@ -1,4 +1,4 @@ -{% load static %} +{% load static banner %} @@ -59,7 +59,7 @@
- {% block billboard %}{% endblock %} + {% block billboard %}{% active_banner %}{% endblock %}
diff --git a/djangoproject/templates/foundation/banner.html b/djangoproject/templates/foundation/banner.html new file mode 100644 index 00000000..a412845f --- /dev/null +++ b/djangoproject/templates/foundation/banner.html @@ -0,0 +1,9 @@ +{% if banner %} + +{% endif %} diff --git a/djangoproject/templates/foundation/banner_preview.html b/djangoproject/templates/foundation/banner_preview.html new file mode 100644 index 00000000..ff3a4894 --- /dev/null +++ b/djangoproject/templates/foundation/banner_preview.html @@ -0,0 +1,9 @@ +{% extends "homepage.html" %} + +{% block billboard %} + + {% include "foundation/banner.html" %} +{% endblock %} diff --git a/djangoproject/templates/fundraising/index.html b/djangoproject/templates/fundraising/index.html index fe3d19e4..6cc16d09 100644 --- a/djangoproject/templates/fundraising/index.html +++ b/djangoproject/templates/fundraising/index.html @@ -15,6 +15,8 @@

{% endblock %} +{% block billboard %}{% endblock %} + {% block messages %} {% if messages %} diff --git a/djangoproject/urls/www.py b/djangoproject/urls/www.py index de4f1818..33fb8edc 100644 --- a/djangoproject/urls/www.py +++ b/djangoproject/urls/www.py @@ -15,7 +15,7 @@ from blog.feeds import WeblogEntryFeed from blog.sitemaps import WeblogSitemap from djangoproject.sitemaps import TemplateViewSitemap from foundation.feeds import FoundationMinutesFeed -from foundation.views import CoreDevelopers +from foundation.views import BannerPreview, CoreDevelopers admin.autodiscover() @@ -98,6 +98,11 @@ urlpatterns = [ ), path("checklists/", include("checklists.urls")), path("contact/", include("contact.urls")), + path( + "foundation/banners//preview/", + BannerPreview.as_view(), + name="foundation_banner_preview", + ), path("foundation/django_core/", CoreDevelopers.as_view()), path("foundation/minutes/", include("foundation.urls.meetings")), path("foundation/", include("members.urls")), diff --git a/foundation/admin.py b/foundation/admin.py index 1c905040..13a604b4 100644 --- a/foundation/admin.py +++ b/foundation/admin.py @@ -52,6 +52,35 @@ class ActionItemInline(admin.StackedInline): model = models.ActionItem +@admin.register(models.Banner) +class BannerAdmin(admin.ModelAdmin): + list_display = ["title", "is_active", "created_at", "updated_at"] + list_editable = ["is_active"] + readonly_fields = ["created_by", "created_at", "updated_by", "updated_at"] + fieldsets = [ + (None, {"fields": ["title", "body", "is_active"]}), + ( + "Call to action", + { + "description": ( + "Both fields should be defined for the CTA button to be displayed." + ), + "fields": ["cta_label", "cta_url"], + }, + ), + ( + "Metadata", + {"fields": ["created_by", "created_at", "updated_by", "updated_at"]}, + ), + ] + + def save_model(self, request, obj, form, change): + if not change: + obj.created_by = request.user + obj.updated_by = request.user + super().save_model(request, obj, form, change) + + @admin.register(models.Meeting) class MeetingAdmin(admin.ModelAdmin): fieldsets = ( diff --git a/foundation/migrations/0008_banner.py b/foundation/migrations/0008_banner.py new file mode 100644 index 00000000..16d29dbd --- /dev/null +++ b/foundation/migrations/0008_banner.py @@ -0,0 +1,92 @@ +# Generated by Django 6.0.3 on 2026-04-13 09:20 + +import django.db.models.deletion +import django.utils.timezone +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("foundation", "0007_boardmember_account_protect"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name="Banner", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("title", models.CharField(max_length=512)), + ( + "body", + models.TextField( + blank=True, + help_text="Optional banner body text. HTML is supported and will be rendered as given.", + ), + ), + ( + "cta_label", + models.CharField( + blank=True, + help_text="Label for the call-to-action button. Both label and URL must be provided for the button to appear.", + max_length=100, + verbose_name="Call-to-action button label", + ), + ), + ( + "cta_url", + models.URLField( + blank=True, + help_text="URL for the call-to-action button. Both label and URL must be provided for the button to appear.", + verbose_name="Call-to-action button URL", + ), + ), + ( + "is_active", + models.BooleanField( + default=False, + help_text="Only one banner can be active at a time. Activating this banner will deactivate all others.", + ), + ), + ( + "created_at", + models.DateTimeField( + default=django.utils.timezone.now, editable=False + ), + ), + ("updated_at", models.DateTimeField(auto_now=True)), + ( + "created_by", + models.ForeignKey( + blank=True, + editable=False, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="+", + to=settings.AUTH_USER_MODEL, + ), + ), + ( + "updated_by", + models.ForeignKey( + blank=True, + editable=False, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="+", + to=settings.AUTH_USER_MODEL, + ), + ), + ], + ), + ] diff --git a/foundation/models.py b/foundation/models.py index 7ae1fa4a..86826e76 100644 --- a/foundation/models.py +++ b/foundation/models.py @@ -1,10 +1,12 @@ from decimal import Decimal from django.conf import settings +from django.core.exceptions import ValidationError from django.db import models -from django.urls import reverse +from django.utils import timezone from django.utils.dateformat import format as date_format from django.utils.translation import gettext_lazy as _ +from django_hosts.resolvers import reverse from djmoney.models.fields import MoneyField from djmoney.settings import CURRENCIES from docutils.core import publish_parts @@ -186,6 +188,84 @@ class ApprovedCorporateMember(models.Model): return self.name +class Banner(models.Model): + """A site-wide campaign banner displayed in the billboard area. + + Only one banner can be active at a time; activating a banner automatically + deactivates all others. + + Users with the view_banner permission can preview inactive banners on the + live site. + """ + + title = models.CharField(max_length=512) + body = models.TextField( + blank=True, + help_text=( + "Optional banner body text. HTML is supported and will be rendered as given." + ), + ) + cta_label = models.CharField( + "Call-to-action button label", + max_length=100, + blank=True, + help_text=( + "Label for the call-to-action button. Both label and URL must be provided " + "for the button to appear." + ), + ) + cta_url = models.URLField( + "Call-to-action button URL", + blank=True, + help_text=( + "URL for the call-to-action button. Both label and URL must be provided " + "for the button to appear." + ), + ) + is_active = models.BooleanField( + default=False, + help_text=( + "Only one banner can be active at a time. Activating this banner will " + "deactivate all others." + ), + ) + created_at = models.DateTimeField(default=timezone.now, editable=False) + created_by = models.ForeignKey( + settings.AUTH_USER_MODEL, + null=True, + blank=True, + editable=False, + on_delete=models.SET_NULL, + related_name="+", + ) + updated_at = models.DateTimeField(auto_now=True) + updated_by = models.ForeignKey( + settings.AUTH_USER_MODEL, + null=True, + blank=True, + editable=False, + on_delete=models.SET_NULL, + related_name="+", + ) + + def __str__(self): + return self.title + + def get_absolute_url(self): + return reverse("foundation_banner_preview", kwargs={"pk": self.pk}) + + def clean(self): + if bool(self.cta_label) != bool(self.cta_url): + raise ValidationError( + "Both a call-to-action label and URL must be provided, or neither." + ) + + def save(self, *args, **kwargs): + if self.is_active: + Banner.objects.exclude(pk=self.pk).update(is_active=False) + super().save(*args, **kwargs) + + class Business(models.Model): """ Business of the DSF Board. diff --git a/foundation/templatetags/banner.py b/foundation/templatetags/banner.py new file mode 100644 index 00000000..bc3fac45 --- /dev/null +++ b/foundation/templatetags/banner.py @@ -0,0 +1,10 @@ +from django import template + +from foundation.models import Banner + +register = template.Library() + + +@register.inclusion_tag("foundation/banner.html") +def active_banner(): + return {"banner": Banner.objects.filter(is_active=True).first()} diff --git a/foundation/tests.py b/foundation/tests.py index 3b1a88ac..802fd789 100644 --- a/foundation/tests.py +++ b/foundation/tests.py @@ -1,13 +1,145 @@ from datetime import date -from django.contrib.auth.models import User +from django.contrib.auth.models import Permission, User +from django.core.exceptions import ValidationError from django.test import TestCase -from django.urls import reverse +from django_hosts.resolvers import reverse from djmoney.money import Money from djangoproject.tests import ReleaseMixin -from .models import ApprovedGrant, BoardMember, Business, Meeting, Office, Term +from .models import ApprovedGrant, Banner, BoardMember, Business, Meeting, Office, Term + + +class BannerTestCase(ReleaseMixin, TestCase): + def test_activating_banner_deactivates_others(self): + b1 = Banner.objects.create(title="First", is_active=True) + b2 = Banner.objects.create(title="Second", is_active=True) + b1.refresh_from_db() + self.assertIs(b1.is_active, False) + self.assertIs(b2.is_active, True) + + def test_deactivating_banner_leaves_others_unchanged(self): + b1 = Banner.objects.create(title="First", is_active=True) + b2 = Banner.objects.create(title="Second") + self.assertIs(b2.is_active, False) + + b2.title = "New Title" + b2.save() + b1.refresh_from_db() + self.assertIs(b1.is_active, True) + b2.refresh_from_db() + self.assertIs(b2.is_active, False) + + def test_active_banner_tag_no_active_banner(self): + Banner.objects.create(title="Inactive", is_active=False) + response = self.client.get("/") + self.assertNotContains(response, '