diff --git a/CHANGELOG.md b/CHANGELOG.md index 696861b21..80b96a9f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,13 @@ This project (not yet) adheres to [Semantic Versioning](https://semver.org/spec/ ### Added +- Management command ``send_newsletter_settings_notice`` plus an English/German + notice email to inform users whose newsletter opt-in may have been reset by + the profile-edit bug. Recipients are the refined candidate pool; every sent + user is appended with a timestamp to ``/var/log/django/newsletter_settings_notice.log``, + so re-runs and staged rollouts (``--limit``) never send twice. Supports + ``--dry-run``, ``--log-file`` and ``--to`` (single test copy to a chosen + account) - Organisation page: follow button to get notified about new projects - Notifications: organisation followers receive email and in-app notification when a project is published - Project detail and module pages: alert for guest users on registered-users-only projects to register or log in with a personal account @@ -40,6 +47,23 @@ This project (not yet) adheres to [Semantic Versioning](https://semver.org/spec/ ### Fixed +- Profile editing no longer silently clears the newsletter opt-in + (``User.get_newsletters``). The newsletter toggle on the notification + settings page now controls this actual opt-in. +- Periodic notifications: the project started/completed and event reminder + tasks now use configurable coverage windows (``NOTIFICATION_PROJECT_STARTED_HOURS``, + ``NOTIFICATION_PROJECT_COMPLETED_HOURS``, ``NOTIFICATION_EVENT_STARTING_HOURS``, + default 72h) and deduplicate, so each project/event is notified exactly once. + Previously the tasks ran every 3 days with a 24h window, so most project + start/completion emails and short-notice event reminders were never sent; + the schedule now runs daily, mirroring adhocracy4's ``create_system_actions`` + idempotency. +- Guest account conversion: delete the Guest row immediately when a guest + converts to a regular account (previously only on email confirmation). + A leftover Guest row made the platform treat converted users as guests and + excluded them from all notification emails. Added management command + ``delete_converted_guest_rows`` to clean up stale Guest rows of users who + already converted. - tests/organisations: fix ``test_initiator_can_update`` assertion to use ``switch_language`` context manager (django-parler behaviour change caused the default-language description to return the primary language value) @@ -118,7 +142,7 @@ This project (not yet) adheres to [Semantic Versioning](https://semver.org/spec/ - Improvements to pagination - Footer: New Footer Module -- Hierarchical platform breadcrumbs +- Hierarchical platform breadcrumbs - Design: Update Colour Pallette - Design: New Landing Page @@ -151,7 +175,7 @@ This project (not yet) adheres to [Semantic Versioning](https://semver.org/spec/ ## v2602.2 -### Added +### Added - apps/projects: export_utils for exporting project data diff --git a/adhocracy-plus/config/settings/base.py b/adhocracy-plus/config/settings/base.py index ff8dc503b..f2d234329 100644 --- a/adhocracy-plus/config/settings/base.py +++ b/adhocracy-plus/config/settings/base.py @@ -530,6 +530,23 @@ ("a4_candy_budgeting", "proposal"), ) +# Coverage windows (in hours) for the periodic system notifications. The beat +# schedule runs the tasks daily and notifications are only created once per +# project/event, so the effective timing is determined by the daily cadence: +# +# - PROJECT_STARTED_HOURS / PROJECT_COMPLETED_HOURS are LOOKBACK margins. +# Projects are notified at the first daily run after their start/end date, +# i.e. within ~24h of the event; the margin only catches backdated phases +# or missed runs. +# - EVENT_STARTING_HOURS is a LOOKAHEAD margin. Events are reminded at the +# first daily run after they enter the window, i.e. roughly 48-72h before +# the event (less for events announced shortly before). +# +# Mirrors adhocracy4's create_system_actions configuration. +NOTIFICATION_PROJECT_STARTED_HOURS = 72 +NOTIFICATION_PROJECT_COMPLETED_HOURS = 72 +NOTIFICATION_EVENT_STARTING_HOURS = 72 + A4_CATEGORIZABLE = ( ("a4_candy_ideas", "idea"), ("a4_candy_mapideas", "mapidea"), diff --git a/adhocracy-plus/config/settings/docker.py b/adhocracy-plus/config/settings/docker.py index f80a5aac3..14d48a193 100644 --- a/adhocracy-plus/config/settings/docker.py +++ b/adhocracy-plus/config/settings/docker.py @@ -27,15 +27,20 @@ }, "send-recently-started-project-notifications": { "task": "send_recently_started_project_notifications", - "schedule": timedelta(days=3), + # Coverage window is NOTIFICATION_PROJECT_STARTED_HOURS (default 72h); + # the tasks deduplicate, so frequent runs are safe and each project is + # notified exactly once. + "schedule": timedelta(days=1), }, "send-recently-completed-project-notifications": { "task": "send_recently_completed_project_notifications", - "schedule": timedelta(days=3), + # Coverage window is NOTIFICATION_PROJECT_COMPLETED_HOURS (default 72h). + "schedule": timedelta(days=1), }, "send_upcoming-event-notifications": { "task": "send_upcoming_event_notifications", - "schedule": timedelta(days=3), + # Coverage window is NOTIFICATION_EVENT_STARTING_HOURS (default 72h). + "schedule": timedelta(days=1), }, "refresh_project_summaries": { "task": "refresh_project_summaries", diff --git a/adhocracy-plus/config/settings/production.py b/adhocracy-plus/config/settings/production.py index 6103473d6..0df54dd20 100644 --- a/adhocracy-plus/config/settings/production.py +++ b/adhocracy-plus/config/settings/production.py @@ -42,14 +42,19 @@ }, "send-recently-started-project-notifications": { "task": "send_recently_started_project_notifications", - "schedule": timedelta(days=3), + # Coverage window is NOTIFICATION_PROJECT_STARTED_HOURS (default 72h); + # the tasks deduplicate, so frequent runs are safe and each project is + # notified exactly once. + "schedule": timedelta(days=1), }, "send-recently-completed-project-notifications": { "task": "send_recently_completed_project_notifications", - "schedule": timedelta(days=3), + # Coverage window is NOTIFICATION_PROJECT_COMPLETED_HOURS (default 72h). + "schedule": timedelta(days=1), }, "send_upcoming-event-notifications": { "task": "send_upcoming_event_notifications", - "schedule": timedelta(days=3), + # Coverage window is NOTIFICATION_EVENT_STARTING_HOURS (default 72h). + "schedule": timedelta(days=1), }, } diff --git a/apps/account/forms.py b/apps/account/forms.py index 0db0e5ac1..d6aa47f4c 100644 --- a/apps/account/forms.py +++ b/apps/account/forms.py @@ -17,7 +17,6 @@ class Meta: "homepage", "facebook_handle", "twitter_handle", - "get_newsletters", "language", ] diff --git a/apps/account/views.py b/apps/account/views.py index 2cca938e6..c04a47368 100644 --- a/apps/account/views.py +++ b/apps/account/views.py @@ -12,6 +12,8 @@ from guest_user.functions import is_guest_user from guest_user.mixins import GuestUserRequiredMixin from guest_user.mixins import RegularUserRequiredMixin +from guest_user.models import Guest +from guest_user.signals import converted from guest_user.views import ConvertFormView from apps.users.forms import GuestConvertForm @@ -70,6 +72,12 @@ def get_form_kwargs(self): def form_valid(self, form): user = form.save(self.request) + # Delete the guest marker immediately, as guest_user's own convert flow + # does. Waiting for the email_confirmed signal instead would leave the + # user flagged as guest (and thus excluded from all notification + # emails) until they confirm their email address. + Guest.objects.filter(user=user).delete() + converted.send(Guest.objects, user=user) response = complete_signup( self.request, user, diff --git a/apps/newsletters/emails.py b/apps/newsletters/emails.py index b998e19e1..ee12230d2 100644 --- a/apps/newsletters/emails.py +++ b/apps/newsletters/emails.py @@ -1,3 +1,5 @@ +from django.urls import reverse + from apps.users.emails import EmailAplus as Email from apps.users.models import User @@ -40,3 +42,23 @@ def get_context(self): class NewsletterEmailAll(NewsletterEmail): def get_receivers(self): return User.objects.filter(is_active=True).distinct() + + +class NewsletterSettingsNoticeEmail(Email): + """One-time service email informing users that their newsletter opt-in + may have been reset by a bug. Sent regardless of the current opt-in.""" + + template_name = "a4_candy_newsletters/emails/newsletter_settings_notice" + + def __init__(self, user): + self._user = user + + def get_receivers(self): + return [self._user] + + def get_context(self): + context = super().get_context() + context["settings_url"] = self.get_host() + reverse( + "account_notification_settings" + ) + return context diff --git a/apps/newsletters/management/commands/send_newsletter_settings_notice.py b/apps/newsletters/management/commands/send_newsletter_settings_notice.py new file mode 100644 index 000000000..63c2e87b4 --- /dev/null +++ b/apps/newsletters/management/commands/send_newsletter_settings_notice.py @@ -0,0 +1,117 @@ +import os +from datetime import datetime + +from django.contrib.auth import get_user_model +from django.core.management.base import BaseCommand +from django.utils import timezone + +from apps.newsletters.emails import NewsletterSettingsNoticeEmail + +User = get_user_model() + +# v2601.1 (2026-01-22) shipped the change that reset the newsletter opt-in +# whenever a user saved their profile. Users who were logged in after that +# date could plausibly have been affected, but only those currently opted out +# are candidates. +BUG_DATE = timezone.make_aware(datetime(2026, 1, 22)) + +DEFAULT_LOG_FILE = "/var/log/django/newsletter_settings_notice.log" + + +class Command(BaseCommand): + help = ( + "Send a one-time notice email to users whose newsletter opt-in may " + "have been reset by a bug. Recipients are the refined candidate pool: " + "currently opted out, active, and logged in since the bug shipped. " + "Guest accounts are excluded. Every recipient is appended to a log " + "file, so re-runs are idempotent and never send twice." + ) + + def add_arguments(self, parser): + parser.add_argument( + "--dry-run", + action="store_true", + help="Only print the number of recipients; do not send emails.", + ) + parser.add_argument( + "--limit", + type=int, + default=None, + help="Send to at most this many users (useful for staged rollouts). " + "Sent users are logged and skipped on later runs.", + ) + parser.add_argument( + "--to", + dest="to_email", + default=None, + help="Send a single test copy to this email address instead of " + "the candidate pool (nothing is logged).", + ) + parser.add_argument( + "--log-file", + dest="log_file", + default=DEFAULT_LOG_FILE, + help=f"File to append sent recipients to (default: {DEFAULT_LOG_FILE}).", + ) + + def handle(self, *args, **options): + if options["to_email"]: + self._send_test_copy(options["to_email"]) + return + + dry_run = options["dry_run"] + limit = options["limit"] + log_file = options["log_file"] + + already_sent = self._load_sent_emails(log_file) + + candidates = ( + User.objects.filter( + get_newsletters=False, + is_active=True, + last_login__gte=BUG_DATE, + ) + .exclude(email__startswith="guest+") + .exclude(email__in=already_sent) + .order_by("pk") + ) + count = candidates.count() + if limit: + candidates = candidates[:limit] + + verb = "would send" if dry_run else "sending" + self.stdout.write(f"{verb} notice email to {count} user(s)") + + if dry_run: + return + + if count: + os.makedirs(os.path.dirname(log_file) or ".", exist_ok=True) + + sent = 0 + with open(log_file, "a") as log: + for user in candidates.iterator(): + NewsletterSettingsNoticeEmail(user).dispatch(user) + log.write(f"{timezone.now():%Y-%m-%d %H:%M:%S} {user.email}\n") + sent += 1 + + self.stdout.write(self.style.SUCCESS(f"done ({sent} sent)")) + + def _load_sent_emails(self, path): + """Return the set of email addresses already logged as sent.""" + sent = set() + if os.path.exists(path): + with open(path) as f: + for line in f: + line = line.strip() + if line: + sent.add(line.split()[-1]) + return sent + + def _send_test_copy(self, email): + user = User.objects.filter(email__iexact=email).first() + if not user: + self.stderr.write(f"No user with email {email}") + return + NewsletterSettingsNoticeEmail(user).dispatch(user) + self.stdout.write(self.style.SUCCESS(f"test copy sent to {email}")) diff --git a/apps/newsletters/templates/a4_candy_newsletters/emails/newsletter_settings_notice.en.email b/apps/newsletters/templates/a4_candy_newsletters/emails/newsletter_settings_notice.en.email new file mode 100644 index 000000000..66cdaa478 --- /dev/null +++ b/apps/newsletters/templates/a4_candy_newsletters/emails/newsletter_settings_notice.en.email @@ -0,0 +1,22 @@ +{% extends 'email_base.'|add:part_type %} +{% load i18n %} + +{% block headline %}{% translate "Update on your newsletter settings" %}{% endblock %} + +{% block subject %}{% translate "Update on your newsletter settings" %}{% endblock %} + +{% block content %} +

+{% blocktranslate %}Organisations on adhocracy plus send newsletters to their followers. Due to an issue, your user setting that controls whether you receive newsletters from organisations may have been reset.{% endblocktranslate %} +

+

+{% blocktranslate %}You are currently opted out. If you wish to receive newsletters from the organisations you subscribe to, visit your user settings and enable this setting.{% endblocktranslate %} +

+

+{% translate "Your notification settings" %} +

+{% endblock %} + +{% block reason %} +{% blocktranslate with receiver_mail=receiver.email site_name=site.name %}This email was sent to {{ receiver_mail }} because you are registered on {{ site_name }}. You receive this email because the newsletter setting on your account may have been reset.{% endblocktranslate %} +{% endblock %} diff --git a/apps/notifications/admin.py b/apps/notifications/admin.py index 051d3195f..eb7a1d890 100644 --- a/apps/notifications/admin.py +++ b/apps/notifications/admin.py @@ -32,7 +32,6 @@ class NotificationAdmin(admin.ModelAdmin): class NotificationSettingsAdmin(admin.ModelAdmin): list_display = ("id", "user", "email_moderation", "notify_moderation") list_filter = ( - "email_newsletter", "email_initiator_publish_results", "email_project_updates", "notify_project_updates", diff --git a/apps/notifications/forms.py b/apps/notifications/forms.py index a3a89da20..1a8038dfd 100644 --- a/apps/notifications/forms.py +++ b/apps/notifications/forms.py @@ -1,15 +1,23 @@ from django import forms +from django.utils.translation import gettext_lazy as _ from .models import NotificationSettings class NotificationSettingsForm(forms.ModelForm): + # Newsletter opt-in lives on the User model (User.get_newsletters) and is + # read by the newsletter sender. It must NOT default to True: this field + # only reflects and changes the user's explicit choice. + get_newsletters = forms.BooleanField( + label=_("Email Newsletter"), + required=False, + ) + class Meta: model = NotificationSettings fields = [ # Project related "email_initiator_publish_results", - "email_newsletter", "email_project_updates", "notify_project_updates", "email_project_events", @@ -34,6 +42,13 @@ class Meta: def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) + self.fields["get_newsletters"].initial = self.instance.user.get_newsletters for field_name, field in self.fields.items(): if isinstance(field, forms.BooleanField): field.widget.attrs.update({"class": "form-check-input"}) + + def save(self, commit=True): + user = self.instance.user + user.get_newsletters = self.cleaned_data["get_newsletters"] + user.save() + return super().save(commit) diff --git a/apps/notifications/migrations/0005_notification_project.py b/apps/notifications/migrations/0005_notification_project.py new file mode 100644 index 000000000..0e52c7480 --- /dev/null +++ b/apps/notifications/migrations/0005_notification_project.py @@ -0,0 +1,27 @@ +# Generated by Django 5.2.15 on 2026-08-04 08:47 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("a4_candy_notifications", "0004_alter_notification_notification_type"), + ("a4projects", "0055_alter_project_allow_guest_users_default"), + ] + + operations = [ + migrations.AddField( + model_name="notification", + name="project", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="+", + to="a4projects.project", + verbose_name="Project", + ), + ), + ] diff --git a/apps/notifications/migrations/0006_remove_notificationsettings_email_newsletter.py b/apps/notifications/migrations/0006_remove_notificationsettings_email_newsletter.py new file mode 100644 index 000000000..855203205 --- /dev/null +++ b/apps/notifications/migrations/0006_remove_notificationsettings_email_newsletter.py @@ -0,0 +1,17 @@ +# Generated by Django 5.2.15 on 2026-08-04 16:10 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("a4_candy_notifications", "0005_notification_project"), + ] + + operations = [ + migrations.RemoveField( + model_name="notificationsettings", + name="email_newsletter", + ), + ] diff --git a/apps/notifications/models.py b/apps/notifications/models.py index f096e2197..2bb0b87c1 100644 --- a/apps/notifications/models.py +++ b/apps/notifications/models.py @@ -119,7 +119,6 @@ class NotificationCategory: NotificationCategory.INVITATIONS: ("email_invitations", "notify_invitations"), NotificationCategory.MODERATION: ("email_moderation", "notify_moderation"), NotificationCategory.WARNINGS: ("email_warnings", "notify_warnings"), - "newsletter": ("email_newsletter", None), # No in-app for newsletter "system": (None, None), # Always deliver system notifications } @@ -141,11 +140,6 @@ def get_for_user(cls, user): related_name="notification_settings", ) - # Project related - Email newsletter - email_newsletter = models.BooleanField( - default=True, verbose_name=_("Email newsletter") - ) - # Initiator-only e-mails email_initiator_publish_results = models.BooleanField( default=True, @@ -243,7 +237,6 @@ def get_email_fields(self): "email_project_updates", "email_project_events", "email_user_engagement", - "email_newsletter", ] def get_notification_fields(self): @@ -290,6 +283,14 @@ class Notification(models.Model): choices=NotificationType.choices, verbose_name=_("Notification Type"), ) + project = models.ForeignKey( + "a4projects.Project", + null=True, + blank=True, + on_delete=models.CASCADE, + related_name="+", + verbose_name=_("Project"), + ) read = models.BooleanField(default=False) read_at = models.DateTimeField(null=True, blank=True) created = models.DateTimeField(auto_now_add=True) diff --git a/apps/notifications/services.py b/apps/notifications/services.py index c7c85a5ce..375b15c49 100644 --- a/apps/notifications/services.py +++ b/apps/notifications/services.py @@ -6,6 +6,7 @@ from django.utils.safestring import mark_safe from guest_user.functions import is_guest_user +from adhocracy4.projects.models import Project from apps.users.emails import EmailAplus as Email from .models import NOTIFICATION_TYPE_MAPPING @@ -142,6 +143,7 @@ def create_notifications(obj, strategy) -> None: # Remove email_context before creating notifications notification_data.pop("email_context", None) notification_data.pop("translated_message_template", None) + notification_data["project"] = NotificationService._get_project(obj) # Create in-app notifications notifications = [ @@ -152,6 +154,26 @@ def create_notifications(obj, strategy) -> None: if notifications: Notification.objects.bulk_create(notifications) + @staticmethod + def _get_project(obj): + """Best-effort lookup of the project a notification belongs to.""" + if isinstance(obj, Project): + # Some notifications are created from a post_delete signal (e.g. + # project deletion), when the project row no longer exists. + if Project.objects.filter(pk=obj.pk).exists(): + return obj + return None + project = getattr(obj, "project", None) + if project is None: + content_object = getattr(obj, "content_object", None) + if content_object is not None: + project = getattr(content_object, "project", None) + if project is None: + comment = getattr(obj, "comment", None) + if comment is not None: + project = getattr(comment, "project", None) + return project + @staticmethod def _get_filtered_recipients(all_recipients, notification_type): """ diff --git a/apps/notifications/strategies/event_strategies.py b/apps/notifications/strategies/event_strategies.py index c8f81509f..7e16e0791 100644 --- a/apps/notifications/strategies/event_strategies.py +++ b/apps/notifications/strategies/event_strategies.py @@ -145,6 +145,8 @@ def create_notification_data(self, offline_event): "event_url": offline_event.get_absolute_url(), "event_date": format_event_date(offline_event.date), }, + # Used as dedup key by the periodic task: each event is notified once. + "target_url": offline_event.get_absolute_url(), "email_context": email_context, } diff --git a/apps/notifications/tasks.py b/apps/notifications/tasks.py index 598b1eacb..a6c1ecc9b 100644 --- a/apps/notifications/tasks.py +++ b/apps/notifications/tasks.py @@ -1,25 +1,61 @@ from datetime import timedelta from celery import shared_task +from django.conf import settings from django.db.models import Q from django.utils import timezone from adhocracy4.phases.models import Phase from apps.offlineevents.models import OfflineEvent +from .models import Notification +from .models import NotificationType from .services import NotificationService from .strategies import OfflineEventReminder from .strategies import ProjectEnded from .strategies import ProjectStarted +def _hours(setting_name, default): + """Read a notification window setting in hours (default 72).""" + return getattr(settings, setting_name, default) + + +def _project_already_notified(project, notification_type) -> bool: + """True if a notification of this type already exists for the project. + + Mirrors the idempotency of adhocracy4's ``create_system_actions`` + management command: the first run within the coverage window creates the + notification (and sends the email), later runs skip it so every project + is only notified once. + """ + return Notification.objects.filter( + project=project, notification_type=notification_type + ).exists() + + +def _event_already_notified(event) -> bool: + """True if an upcoming-event notification already exists for the event.""" + return Notification.objects.filter( + notification_type=NotificationType.EVENT_SOON, + target_url=event.get_absolute_url(), + ).exists() + + @shared_task(name="send_recently_started_project_notifications") def send_recently_started_project_notifications(): """ - Send notifications to project followers for project started + Notify followers that a project's first phase has started. + + Effective timing: the task runs daily and looks back + NOTIFICATION_PROJECT_STARTED_HOURS (default 72h), so a project is notified + at the first run after its start date - i.e. within ~24h of the start. The + lookback is a safety margin so backdated starts or missed runs are still + caught. Deduplication guarantees each project is notified exactly once. """ now = timezone.now() - last_check = now - timedelta(hours=24) + window = timedelta(hours=_hours("NOTIFICATION_PROJECT_STARTED_HOURS", 72)) + last_check = now - window started_phases = Phase.objects.filter( Q(start_date__gte=last_check, start_date__lte=now) @@ -38,6 +74,8 @@ def send_recently_started_project_notifications(): strategy = ProjectStarted() for project in started_projects: + if _project_already_notified(project, NotificationType.PROJECT_STARTED): + continue NotificationService.create_notifications(project, strategy) return len(started_projects) @@ -54,10 +92,17 @@ def is_last_phase_in_project(phase): @shared_task(name="send_recently_completed_project_notifications") def send_recently_completed_project_notifications(): """ - Send notifications to project followers for project completed + Notify followers that a project has been completed. + + Effective timing: the task runs daily and looks back + NOTIFICATION_PROJECT_COMPLETED_HOURS (default 72h), so a project is + notified at the first run after its last phase ends - i.e. within ~24h of + the end. The lookback is a safety margin so missed runs are still caught. + Deduplication guarantees each project is notified exactly once. """ now = timezone.now() - last_check = now - timedelta(hours=24) + window = timedelta(hours=_hours("NOTIFICATION_PROJECT_COMPLETED_HOURS", 72)) + last_check = now - window completed_phases = Phase.objects.filter( Q(end_date__gte=last_check, end_date__lte=now) @@ -68,6 +113,8 @@ def send_recently_completed_project_notifications(): ] strategy = ProjectEnded() for project in ended_projects: + if _project_already_notified(project, NotificationType.PROJECT_COMPLETED): + continue NotificationService.create_notifications(project, strategy) return @@ -76,11 +123,17 @@ def send_recently_completed_project_notifications(): @shared_task(name="send_upcoming_event_notifications") def send_upcoming_event_notifications(): """ - Send notifications to project followers for events starting within 24 hours + Remind followers of events starting soon. + + Effective timing: the task runs daily and looks ahead + NOTIFICATION_EVENT_STARTING_HOURS (default 72h), so an event is notified at + the first run after it enters the window - i.e. roughly 48-72h before the + event (sooner for events announced after they entered the window). + Deduplication guarantees each event is notified exactly once. """ now = timezone.now() - future = now + timedelta(hours=72) + future = now + timedelta(hours=_hours("NOTIFICATION_EVENT_STARTING_HOURS", 72)) upcoming_events = OfflineEvent.objects.filter( Q(date__gte=now, date__lte=future) @@ -92,6 +145,9 @@ def send_upcoming_event_notifications(): if not event.project: continue + if _event_already_notified(event): + continue + NotificationService.create_notifications(event, strategy) return diff --git a/apps/notifications/templates/a4_candy_notifications/settings.html b/apps/notifications/templates/a4_candy_notifications/settings.html index 4e3a9c831..7e76ffdef 100644 --- a/apps/notifications/templates/a4_candy_notifications/settings.html +++ b/apps/notifications/templates/a4_candy_notifications/settings.html @@ -72,8 +72,8 @@
{% trans "Email Newsletter" %}
{% trans "Receive email newsletters with updates and news from the projects you follow." %}

- {{ form.email_newsletter|add_class:"project-related-switch" }} -
diff --git a/apps/users/management/commands/delete_converted_guest_rows.py b/apps/users/management/commands/delete_converted_guest_rows.py new file mode 100644 index 000000000..84b1f9392 --- /dev/null +++ b/apps/users/management/commands/delete_converted_guest_rows.py @@ -0,0 +1,39 @@ +from django.core.management.base import BaseCommand +from guest_user.models import Guest + + +class Command(BaseCommand): + help = ( + "Delete Guest rows belonging to users who have converted to a regular " + "account (identified by a non-guest email address). A leftover Guest " + "row makes the platform treat the account as a guest, which excludes " + "it from notification emails. Only the Guest row is deleted, never " + "the user." + ) + + def add_arguments(self, parser): + parser.add_argument( + "--dry-run", + action="store_true", + help="Only show which Guest rows would be deleted.", + ) + + def handle(self, *args, **options): + dry_run = options["dry_run"] + + # Guest accounts are created with a guest+@liqd.net + # placeholder email; a different email means the user converted. + converted = Guest.objects.exclude(user__email__startswith="guest+") + + count = converted.count() + for guest in converted.select_related("user"): + self.stdout.write( + f"{'[dry-run] would delete' if dry_run else 'deleting'} " + f"Guest row of user {guest.user.pk} ({guest.user.email})" + ) + + if not dry_run: + converted.delete() + + verb = "would delete" if dry_run else "deleted" + self.stdout.write(f"{verb} {count} stale Guest row(s).") diff --git a/locale-source/locale/de/LC_MESSAGES/django.po b/locale-source/locale/de/LC_MESSAGES/django.po index f2aa02c7c..111e0734b 100644 --- a/locale-source/locale/de/LC_MESSAGES/django.po +++ b/locale-source/locale/de/LC_MESSAGES/django.po @@ -7757,3 +7757,42 @@ msgstr "Mit Konto eines sozialen Netzwerks einloggen" #: adhocracy-plus/templates/socialaccount/snippets/provider_list.html:25 msgid "Login with " msgstr "Einloggen mit " + +#: apps/newsletters/templates/a4_candy_newsletters/emails/newsletter_settings_notice.en.email:6 +msgid "Update on your newsletter settings" +msgstr "Neuigkeiten zu Ihren Newsletter-Einstellungen" + +#: apps/newsletters/templates/a4_candy_newsletters/emails/newsletter_settings_notice.en.email:10 +msgid "" +"Organisations on adhocracy plus send newsletters to their followers. Due to " +"an issue, your user setting that controls whether you receive newsletters " +"from organisations may have been reset." +msgstr "" +"Organisationen auf adhocracy plus senden Newsletter an ihre Follower. " +"Aufgrund eines Problems wurde Ihre Einstellung, ob Sie Newsletter von " +"Organisationen erhalten möchten, möglicherweise zurückgesetzt." + +#: apps/newsletters/templates/a4_candy_newsletters/emails/newsletter_settings_notice.en.email:13 +msgid "" +"You are currently opted out. If you wish to receive newsletters from the " +"organisations you subscribe to, visit your user settings and enable this " +"setting." +msgstr "" +"Sie sind derzeit abgemeldet. Wenn Sie Newsletter von Organisationen, denen " +"Sie folgen, erhalten möchten, besuchen Sie Ihre Benutzer-Einstellungen und " +"aktivieren Sie diese Einstellung." + +#: apps/newsletters/templates/a4_candy_newsletters/emails/newsletter_settings_notice.en.email:16 +msgid "Your notification settings" +msgstr "Ihre Benachrichtigungs-Einstellungen" + +#: apps/newsletters/templates/a4_candy_newsletters/emails/newsletter_settings_notice.en.email:21 +#, python-format +msgid "" +"This email was sent to %(receiver_mail)s because you are registered on " +"%(site_name)s. You receive this email because the newsletter setting on your " +"account may have been reset." +msgstr "" +"Diese E-Mail wurde an %(receiver_mail)s gesendet, da Sie auf %(site_name)s " +"registriert sind. Sie erhalten diese E-Mail, weil die " +"Newsletter-Einstellung Ihres Kontos möglicherweise zurückgesetzt wurde." diff --git a/tests/account/test_forms.py b/tests/account/test_forms.py index 58f14dccb..1f01165ba 100644 --- a/tests/account/test_forms.py +++ b/tests/account/test_forms.py @@ -4,6 +4,35 @@ from apps.account import forms +@pytest.mark.django_db +def test_profile_save_keeps_newsletter_optin(client, user): + """Saving the profile must not silently clear the newsletter opt-in. + + The profile page does not render the get_newsletters checkbox, so the + form must not include the field either (a missing checkbox is submitted + as unchecked and would reset the user's opt-in on every save). + """ + user.get_newsletters = True + user.save() + client.login(email=user.email, password="password") + + response = client.post( + reverse("account_profile"), + { + "username": user.username, + "bio": "Updated my bio", + "twitter_handle": "", + "facebook_handle": "", + "homepage": "", + "language": "en", + }, + ) + + assert response.status_code == 302 + user.refresh_from_db() + assert user.get_newsletters is True + + @pytest.mark.django_db def test_clean_username(user_factory): user = user_factory(username="username") diff --git a/tests/account/test_guest_convert.py b/tests/account/test_guest_convert.py new file mode 100644 index 000000000..9c7da032a --- /dev/null +++ b/tests/account/test_guest_convert.py @@ -0,0 +1,28 @@ +import pytest +from django.urls import reverse +from guest_user.functions import is_guest_user +from guest_user.models import Guest + +from tests.helpers import GuestUserCreator + + +@pytest.mark.django_db +def test_guest_convert_deletes_guest_row(client): + guest = GuestUserCreator().create_guest_user() + client.force_login(guest) + + response = client.post( + reverse("guest_convert"), + { + "email": "converted@example.com", + "username": "converteduser", + "password1": "testpassword123", + "password2": "testpassword123", + "terms_of_use": "on", + }, + ) + + assert response.status_code == 302 + guest.refresh_from_db() + assert not Guest.objects.filter(user=guest).exists() + assert not is_guest_user(guest) diff --git a/tests/conftest.py b/tests/conftest.py index b4faf158b..79dbc0680 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,3 +1,4 @@ +import uuid from io import BytesIO import factory @@ -48,27 +49,46 @@ def apiclient(): @pytest.fixture def image_png(): - return factory.django.ImageField(width=1500, height=1400, format="PNG") + return factory.django.ImageField( + width=1500, + height=1400, + format="PNG", + filename=f"example-{uuid.uuid4().hex}.png", + ) @pytest.fixture def image_webp(): - return factory.django.ImageField(width=1500, height=1400, format="WEBP") + return factory.django.ImageField( + width=1500, + height=1400, + format="WEBP", + filename=f"example-{uuid.uuid4().hex}.webp", + ) @pytest.fixture def image_bmp(): - return factory.django.ImageField(width=1500, height=1400, format="BMP") + return factory.django.ImageField( + width=1500, + height=1400, + format="BMP", + filename=f"example-{uuid.uuid4().hex}.bmp", + ) @pytest.fixture def small_image(): - return factory.django.ImageField(width=200, height=200) + return factory.django.ImageField( + width=200, height=200, filename=f"example-{uuid.uuid4().hex}.jpg" + ) @pytest.fixture def big_image(): - return factory.django.ImageField(width=1500, height=1400) + return factory.django.ImageField( + width=1500, height=1400, filename=f"example-{uuid.uuid4().hex}.jpg" + ) @pytest.fixture diff --git a/tests/newsletters/test_newsletter_settings_notice.py b/tests/newsletters/test_newsletter_settings_notice.py new file mode 100644 index 000000000..a07dfb7c9 --- /dev/null +++ b/tests/newsletters/test_newsletter_settings_notice.py @@ -0,0 +1,141 @@ +from datetime import timedelta + +import pytest +from django.core import mail +from django.core.management import call_command +from django.utils import timezone + +from apps.newsletters.emails import NewsletterSettingsNoticeEmail +from tests.helpers import GuestUserCreator + + +def log_lines(tmp_path): + log_file = tmp_path / "notice.log" + if not log_file.exists(): + return [] + return log_file.read_text().strip().splitlines() + + +@pytest.mark.django_db +def test_command_dry_run_sends_nothing(user_factory, tmp_path): + log_file = tmp_path / "notice.log" + user = user_factory(get_newsletters=False) + user.last_login = timezone.now() + user.save() + + call_command( + "send_newsletter_settings_notice", dry_run=True, log_file=str(log_file) + ) + + assert len(mail.outbox) == 0 + assert not log_file.exists() + + +@pytest.mark.django_db +def test_command_sends_to_refined_candidate_pool(user_factory, tmp_path): + log_file = tmp_path / "notice.log" + affected = user_factory(get_newsletters=False) + affected.last_login = timezone.now() + affected.save() + + call_command("send_newsletter_settings_notice", log_file=str(log_file)) + + assert len(mail.outbox) == 1 + assert mail.outbox[0].to == [affected.email] + assert "newsletter" in mail.outbox[0].subject.lower() + lines = log_lines(tmp_path) + assert len(lines) == 1 + assert lines[0].endswith(affected.email) + + +@pytest.mark.django_db +def test_command_excludes_non_candidates(user_factory, tmp_path): + subscribed = user_factory(get_newsletters=True) + subscribed.last_login = timezone.now() + subscribed.save() + + inactive = user_factory(get_newsletters=False, is_active=False) + inactive.last_login = timezone.now() + inactive.save() + + not_logged_in_since = user_factory(get_newsletters=False) + not_logged_in_since.last_login = timezone.now() - timedelta(days=400) + not_logged_in_since.save() + + guest = GuestUserCreator().create_guest_user() + guest.last_login = timezone.now() + guest.save() + + call_command( + "send_newsletter_settings_notice", log_file=str(tmp_path / "notice.log") + ) + + assert len(mail.outbox) == 0 + + +@pytest.mark.django_db +def test_command_limit_limits_recipients(user_factory, tmp_path): + log_file = tmp_path / "notice.log" + for _ in range(3): + user = user_factory(get_newsletters=False) + user.last_login = timezone.now() + user.save() + + call_command("send_newsletter_settings_notice", limit=1, log_file=str(log_file)) + + assert len(mail.outbox) == 1 + assert len(log_lines(tmp_path)) == 1 + + +@pytest.mark.django_db +def test_command_rerun_skips_already_notified(user_factory, tmp_path): + log_file = tmp_path / "notice.log" + affected = user_factory(get_newsletters=False) + affected.last_login = timezone.now() + affected.save() + + call_command("send_newsletter_settings_notice", log_file=str(log_file)) + call_command("send_newsletter_settings_notice", log_file=str(log_file)) + + assert len(mail.outbox) == 1 + assert len(log_lines(tmp_path)) == 1 + + +@pytest.mark.django_db +def test_command_to_sends_test_copy_without_logging(user_factory, tmp_path): + log_file = tmp_path / "notice.log" + user_factory(email="test@example.com") + + call_command( + "send_newsletter_settings_notice", to="test@example.com", log_file=str(log_file) + ) + + assert len(mail.outbox) == 1 + assert mail.outbox[0].to == ["test@example.com"] + assert not log_file.exists() + + +@pytest.mark.django_db +def test_email_renders_in_english_and_german(user_factory): + user = user_factory() + user.language = "de" + user.save() + + mail.outbox.clear() + NewsletterSettingsNoticeEmail(user).dispatch(user) + + assert len(mail.outbox) == 1 + subject = mail.outbox[0].subject + assert "Neuigkeiten zu Ihren Newsletter-Einstellungen" in subject + html = mail.outbox[0].alternatives[0][0] + assert "Ihre Benachrichtigungs-Einstellungen" in html + + user.language = "en" + user.save() + mail.outbox.clear() + NewsletterSettingsNoticeEmail(user).dispatch(user) + + assert len(mail.outbox) == 1 + assert "Update on your newsletter settings" in mail.outbox[0].subject + html = mail.outbox[0].alternatives[0][0] + assert "Your notification settings" in html diff --git a/tests/notifications/test_guest_notifications.py b/tests/notifications/test_guest_notifications.py index 97e3e2ec4..9da4f9aea 100644 --- a/tests/notifications/test_guest_notifications.py +++ b/tests/notifications/test_guest_notifications.py @@ -35,3 +35,41 @@ def test_guest_cannot_reach_notification_settings(client): assert response.status_code == 302 assert response.url != reverse("account_notification_settings") + + +@pytest.mark.django_db +def test_converted_guest_receives_notification_emails( + client, + module_factory, + idea_factory, + project_factory, + comment_factory, + user_factory, +): + """A converted guest must get notification emails like any regular user.""" + project = project_factory(allow_guest_users=True) + module = module_factory(project=project) + guest = GuestUserCreator().create_guest_user() + idea = idea_factory(module=module, creator=guest) + client.force_login(guest) + + response = client.post( + reverse("guest_convert"), + { + "email": "converted@example.com", + "username": "converteduser", + "password1": "testpassword123", + "password2": "testpassword123", + "terms_of_use": "on", + }, + ) + assert response.status_code == 302 + + # Refresh the in-memory user: idea.creator still points to the stale + # pre-conversion instance. In production every request loads fresh objects. + guest.refresh_from_db() + + mail.outbox.clear() + comment_factory(content_object=idea, creator=user_factory(), project=project) + + assert len(get_emails_for_address("converted@example.com")) == 1 diff --git a/tests/notifications/test_new_user_notifications.py b/tests/notifications/test_new_user_notifications.py new file mode 100644 index 000000000..f8778fcc1 --- /dev/null +++ b/tests/notifications/test_new_user_notifications.py @@ -0,0 +1,62 @@ +from datetime import timedelta + +import pytest +from django.core import mail +from django.utils import timezone + +from adhocracy4.follows.models import Follow +from apps.notifications.models import NotificationSettings +from apps.notifications.services import NotificationService +from apps.notifications.strategies import ProjectInvitationCreated +from apps.notifications.tasks import send_recently_started_project_notifications +from apps.projects.models import ParticipantInvite + + +@pytest.mark.django_db +def test_new_user_without_settings_gets_project_started_email( + project_factory, phase_factory, user_factory +): + project = project_factory() + phase_factory( + module__project=project, + start_date=timezone.now() - timedelta(hours=12), + end_date=timezone.now() + timedelta(days=7), + ) + user = user_factory() + # Simulate a user that was created before notification settings existed + NotificationSettings.objects.filter(user=user).delete() + assert not NotificationSettings.objects.filter(user=user).exists() + + Follow.objects.get_or_create( + project=project, creator=user, defaults={"enabled": True} + ) + + mail.outbox.clear() + send_recently_started_project_notifications() + + settings = NotificationSettings.objects.get(user=user) + assert settings.email_project_updates is True + assert len([m for m in mail.outbox if user.email in m.to]) == 1 + + +@pytest.mark.django_db +def test_new_user_without_settings_gets_project_invitation_email( + project_factory, user_factory +): + project = project_factory() + user = user_factory() + NotificationSettings.objects.filter(user=user).delete() + assert not NotificationSettings.objects.filter(user=user).exists() + + invite = ParticipantInvite.objects.create( + project=project, + creator=user_factory(), + email=user.email, + site="example.com", + ) + + mail.outbox.clear() + NotificationService.create_notifications(invite, ProjectInvitationCreated()) + + assert NotificationSettings.objects.filter(user=user).exists() + assert len([m for m in mail.outbox if user.email in m.to]) == 1 diff --git a/tests/notifications/test_notification_settings.py b/tests/notifications/test_notification_settings.py new file mode 100644 index 000000000..43a18ccd8 --- /dev/null +++ b/tests/notifications/test_notification_settings.py @@ -0,0 +1,70 @@ +import pytest + +from apps.notifications.forms import NotificationSettingsForm +from apps.notifications.models import NotificationSettings + + +def settings_form_data(get_newsletters=False): + """POST data for the notification settings form.""" + data = { + "email_initiator_publish_results": "on", + "email_project_updates": "on", + "notify_project_updates": "on", + "email_project_events": "on", + "notify_project_events": "on", + "email_user_engagement": "on", + "notify_user_engagement": "on", + "email_messages": "on", + "notify_messages": "on", + "email_invitations": "on", + "notify_invitations": "on", + "email_moderation": "on", + "notify_moderation": "on", + "email_warnings": "on", + "notify_warnings": "on", + "track_project_updates": "on", + "track_project_events": "on", + "track_user_engagement": "on", + } + if get_newsletters: + data["get_newsletters"] = "on" + return data + + +@pytest.mark.django_db +def test_newsletter_toggle_turns_optin_on(user): + settings = NotificationSettings.get_for_user(user) + form = NotificationSettingsForm( + instance=settings, data=settings_form_data(get_newsletters=True) + ) + assert form.is_valid(), form.errors + form.save() + + user.refresh_from_db() + assert user.get_newsletters is True + + +@pytest.mark.django_db +def test_newsletter_toggle_turns_optin_off(user): + user.get_newsletters = True + user.save() + settings = NotificationSettings.get_for_user(user) + + form = NotificationSettingsForm( + instance=settings, data=settings_form_data(get_newsletters=False) + ) + assert form.is_valid(), form.errors + form.save() + + user.refresh_from_db() + assert user.get_newsletters is False + + +@pytest.mark.django_db +def test_newsletter_toggle_initial_reflects_current_optin(user): + user.get_newsletters = True + user.save() + settings = NotificationSettings.get_for_user(user) + + form = NotificationSettingsForm(instance=settings) + assert form["get_newsletters"].initial is True diff --git a/tests/notifications/test_notification_tasks.py b/tests/notifications/test_notification_tasks.py index 800cd728f..6b284a261 100644 --- a/tests/notifications/test_notification_tasks.py +++ b/tests/notifications/test_notification_tasks.py @@ -183,3 +183,127 @@ def test_send_upcoming_event_notifications( assert len(follower_emails) == 1 assert "event in project" in follower_emails[0].subject.lower() assert project.name.lower() in follower_emails[0].subject.lower() + + +@pytest.mark.django_db +def test_no_upcoming_event_notification_beyond_window( + project_factory, offline_event_factory, user2 +): + """Events starting more than the configured window ahead must not be + notified yet.""" + project = project_factory() + offline_event_factory( + project=project, + date=timezone.now() + timedelta(hours=100), + ) + + Follow.objects.get_or_create( + project=project, + creator=user2, + defaults={"enabled": True}, + ) + + send_upcoming_event_notifications() + + assert Notification.objects.filter(recipient=user2).count() == 0 + assert len(get_emails_for_address(user2.email)) == 0 + + +@pytest.mark.django_db +def test_upcoming_event_notification_window_is_configurable( + project_factory, offline_event_factory, user2, settings +): + """The lookahead window honours the NOTIFICATION_EVENT_STARTING_HOURS + setting.""" + project = project_factory() + offline_event_factory( + project=project, + date=timezone.now() + timedelta(hours=30), + ) + Follow.objects.get_or_create( + project=project, + creator=user2, + defaults={"enabled": True}, + ) + + settings.NOTIFICATION_EVENT_STARTING_HOURS = 24 + + send_upcoming_event_notifications() + + assert Notification.objects.filter(recipient=user2).count() == 0 + + +@pytest.mark.django_db +def test_no_project_started_notification_beyond_window( + phase_factory, project_factory, user2 +): + """Projects started more than the configured window ago must not be + notified.""" + project = project_factory() + phase_factory( + module__project=project, + start_date=timezone.now() - timedelta(hours=100), + end_date=timezone.now() + timedelta(days=7), + ) + + Follow.objects.get_or_create( + project=project, + creator=user2, + defaults={"enabled": True}, + ) + + send_recently_started_project_notifications() + + assert Notification.objects.filter(recipient=user2).count() == 0 + assert len(get_emails_for_address(user2.email)) == 0 + + +@pytest.mark.django_db +def test_upcoming_event_notification_only_sent_once( + project_factory, offline_event_factory, user2 +): + """An event within the lookahead window must only be notified once, even + if the task runs repeatedly (dedup via existing EVENT_SOON notification).""" + project = project_factory() + offline_event_factory( + project=project, + date=timezone.now() + timedelta(hours=30), + ) + + Follow.objects.get_or_create( + project=project, + creator=user2, + defaults={"enabled": True}, + ) + + send_upcoming_event_notifications() + send_upcoming_event_notifications() + + assert Notification.objects.filter(recipient=user2).count() == 1 + assert len(get_emails_for_address(user2.email)) == 1 + + +@pytest.mark.django_db +def test_project_started_notification_only_sent_once( + phase_factory, project_factory, user2 +): + """A started project must only be notified once, even if the task runs + repeatedly (dedup via existing PROJECT_STARTED notification).""" + project = project_factory() + phase_factory( + module__project=project, + start_date=timezone.now() - timedelta(hours=12), + end_date=timezone.now() + timedelta(days=7), + ) + + Follow.objects.get_or_create( + project=project, + creator=user2, + defaults={"enabled": True}, + ) + + send_recently_started_project_notifications() + send_recently_started_project_notifications() + + assert Notification.objects.filter(recipient=user2).count() == 1 + assert len(get_emails_for_address(user2.email)) == 1 diff --git a/tests/users/test_delete_converted_guest_rows.py b/tests/users/test_delete_converted_guest_rows.py new file mode 100644 index 000000000..77af76dec --- /dev/null +++ b/tests/users/test_delete_converted_guest_rows.py @@ -0,0 +1,34 @@ +import pytest +from django.core.management import call_command +from guest_user.models import Guest + +from tests.helpers import GuestUserCreator + + +def make_converted_guest(): + """A guest who converted: real email and username, leftover Guest row.""" + guest = GuestUserCreator().create_guest_user() + guest.email = "converted@example.com" + guest.username = "converteduser" + guest.save() + return guest + + +@pytest.mark.django_db +def test_command_deletes_only_converted_guest_rows(): + real_guest = GuestUserCreator().create_guest_user() + converted_guest = make_converted_guest() + + call_command("delete_converted_guest_rows") + + assert Guest.objects.filter(user=real_guest).exists() + assert not Guest.objects.filter(user=converted_guest).exists() + + +@pytest.mark.django_db +def test_command_dry_run_deletes_nothing(): + converted_guest = make_converted_guest() + + call_command("delete_converted_guest_rows", dry_run=True) + + assert Guest.objects.filter(user=converted_guest).exists()