Skip to content

Add ability to send a notification email on review of community libra… #6050

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
marcellamaki wants to merge 1 commit into
base: hotfixes
Choose a base branch
from
+87 −0
Open

Add ability to send a notification email on review of community libra… #6050

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions contentcuration/contentcuration/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@
from django.db.models.query_utils import DeferredAttribute
from django.db.models.sql import Query
from django.dispatch import receiver
from django.template.loader import render_to_string
from django.urls import reverse
from django.utils import timezone
from django.utils.translation import gettext as _
from django_cte import CTEManager
Expand Down Expand Up @@ -87,6 +89,7 @@
from contentcuration.db.models.manager import CustomManager
from contentcuration.utils.cache import delete_public_channel_cache_keys
from contentcuration.utils.parser import load_json_string
from contentcuration.utils.urls import canonical_url
from contentcuration.viewsets.sync.constants import ALL_CHANGES
from contentcuration.viewsets.sync.constants import ALL_TABLES
from contentcuration.viewsets.sync.constants import PUBLISHABLE_CHANGE_TABLES
Expand Down Expand Up @@ -3049,6 +3052,42 @@ def notify_update_to_channel_editors(self, exclude_user_id=None):

User.notify_users(editors, date=self.date_updated)

def send_resolution_email(self):
"""
Send an email to the submission author letting them know their
Community Library submission has been resolved (approved or
rejected).
"""
is_approved = self.status == community_library_submission.STATUS_APPROVED

if is_approved:
subject_text = _("Your Community Library submission has been approved")

@rtibblesbot rtibblesbot Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocking: This email introduces six net-new Django gettext strings — the two subjects here (3064/3066) plus five template strings ("Available in Community Library", "Your previously submitted version needs changes…", "Notes from the reviewer", "Thanks for using Kolibri Studio!", "The Learning Equality Team"). None exist in locale/en/LC_MESSAGES/django.po; the matching frontend contentcuration-messages.json text is a separate ($tr) catalog. Two issues: (1) linked issue #5993 scoped this to reusing existing strings, but in the Django catalog these are new; (2) this PR targets hotfixes, not unstable, so new strings miss the normal extraction/translation cycle and will ship English-only until forward-merged. If English-only on the hotfix is an accepted P1 tradeoff, that's fine — please confirm it's intentional and that extraction also happens on unstable.

else:
subject_text = _("Your Community Library submission requires changes")

subject = render_to_string(
"registration/custom_email_subject.txt",
{"subject": subject_text},
)
subject = "".join(subject.splitlines())

message = render_to_string(
"community_library/submission_resolved_email.html",
{
"name": f"{self.author.first_name} {self.author.last_name}".strip(),

@rtibblesbot rtibblesbot Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: f"{self.author.first_name} {self.author.last_name}".strip() duplicates User.get_full_name(). Use self.author.get_full_name().

"channel": self.channel,
"channel_url": canonical_url(
reverse("channel", kwargs={"channel_id": self.channel.pk})
),
"approved": is_approved,
"feedback_notes": self.feedback_notes,
},
)

self.author.email_user(
subject, message, settings.DEFAULT_FROM_EMAIL, html_message=message
)

@classmethod
def filter_view_queryset(cls, queryset, user):
if user.is_anonymous:
Expand Down
31 changes: 31 additions & 0 deletions contentcuration/contentcuration/templates/community_library/submission_resolved_email.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<!DOCTYPE html>
{% load i18n %}
<html>
<head>
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
</head>
<body>
{% autoescape off %}
<p>{% blocktrans with name=name %}Hello {{ name }},{% endblocktrans %}</p>

<p><a href="{{ channel_url }}" target="_blank">{% blocktrans with channel_name=channel.name %}{{ channel_name }}{% endblocktrans %}</a> ({{ channel_url }})</p>

@rtibblesbot rtibblesbot Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: {% blocktrans with channel_name=channel.name %}{{ channel_name }}{% endblocktrans %} emits a msgid of just the placeholder — nothing for a translator to act on. Plain {{ channel.name }} is equivalent. Same at line 21 for feedback_notes. Matches the existing channel_published_email.html, so take it or leave it.


{% if approved %}
<p>{% translate "Available in Community Library" %}</p>
{% else %}
<p>{% translate "Your previously submitted version needs changes. Make sure you have addressed all comments before resubmitting." %}</p>
{% endif %}

{% if feedback_notes %}
<p>{% translate "Notes from the reviewer" %}: {% blocktrans with notes=feedback_notes %}{{ notes }}{% endblocktrans %}</p>
{% endif %}

<p>
{% translate "Thanks for using Kolibri Studio!" %}
<br>
{% translate "The Learning Equality Team" %}
</p>
{% endautoescape %}
</body>
</html>
16 changes: 16 additions & 0 deletions contentcuration/contentcuration/tests/viewsets/test_community_library_submission.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from unittest import mock

import pytz
from django.core import mail
from django.urls import reverse

from contentcuration.constants import (
Expand Down Expand Up @@ -731,6 +732,13 @@ def test_resolve_submission__accept_correct(self, apply_task_mock):
channel_id=self.submission.channel.id,
)

self.assertEqual(len(mail.outbox), 1)

@rtibblesbot rtibblesbot Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: Both paths assert against the real locmem mail backend — recipient, subject, body copy, channel name, and feedback notes — rather than mocking the mail layer. Solid end-to-end coverage of the new path.

sent_email = mail.outbox[0]
self.assertEqual(sent_email.to, [self.submission.author.email])
self.assertIn("approved", sent_email.subject.lower())
self.assertIn("available in community library", sent_email.body.lower())
self.assertIn(self.submission.channel.name, sent_email.body)

@mock.patch(
"contentcuration.viewsets.community_library_submission.apply_channel_changes_task"
)
Expand Down Expand Up @@ -770,6 +778,14 @@ def test_resolve_submission__reject_correct(self, apply_task_mock):
)
apply_task_mock.fetch_or_enqueue.assert_not_called()

self.assertEqual(len(mail.outbox), 1)
sent_email = mail.outbox[0]
self.assertEqual(sent_email.to, [self.submission.author.email])
self.assertIn("requires changes", sent_email.subject.lower())
self.assertIn("needs changes", sent_email.body.lower())
self.assertIn(self.submission.channel.name, sent_email.body)
self.assertIn(self.feedback_notes, sent_email.body)

def test_resolve_submission__reject_missing_resolution_reason(self):
self.client.force_authenticate(user=self.admin_user)
metadata = self.resolve_reject_metadata.copy()
Expand Down
1 change: 1 addition & 0 deletions contentcuration/contentcuration/viewsets/community_library_submission.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ def resolve(self, request, pk=None):
)

submission.notify_update_to_channel_editors()
submission.send_resolution_email()

@rtibblesbot rtibblesbot Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: send_resolution_email() runs before the approval side-effects (_mark_previous_pending_submissions_as_superseded / _add_to_community_library / mark_channel_version_as_distributable). email_user only swallows the two Postmark recipient exceptions; any other send/render error propagates. With no transaction.atomic here and ATOMIC_REQUESTS unset, serializer.save() has already committed APPROVED — a send failure would 500 and leave the submission approved but never added to the community library. Move the send after the state-mutation block so a mail failure is the least-damaging failure point.


if submission.status == community_library_submission_constants.STATUS_APPROVED:
self._mark_previous_pending_submissions_as_superseded(submission)
Expand Down
Loading