Skip to content
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.

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

[feature] Batch email notifications #276

Merged
merged 25 commits into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
2c780c8
[chore] Basic implementation
Dhanus3133 Jun 4, 2024
949ea24
[chore] Updated changes
Dhanus3133 Jun 6, 2024
305d6c8
[chore] Add tests
Dhanus3133 Jun 7, 2024
0e54696
Merge branch 'master' into feat/batch-email
Dhanus3133 Jun 7, 2024
58bdc9d
[chore] Update Readme
Dhanus3133 Jun 8, 2024
4abc717
[chore] New changes
Dhanus3133 Jun 15, 2024
caf3031
Merge branch 'master' into feat/batch-email
Dhanus3133 Jun 15, 2024
233a1e7
[chore] Improvements
Dhanus3133 Jun 17, 2024
69365bb
[chore] Fix cache delete
Dhanus3133 Jun 17, 2024
9d38db1
[chore] New changes
Dhanus3133 Jun 18, 2024
01182c4
[chore] Updates
Dhanus3133 Jun 18, 2024
483df00
[chore] Add extra tests
Dhanus3133 Jun 18, 2024
68a1604
[fix] Tests
Dhanus3133 Jun 21, 2024
6757f7b
[chore] Bump changes
Dhanus3133 Jul 6, 2024
a826d5e
[feat] Automatically open notification widget
Dhanus3133 Jul 6, 2024
7a55502
[chore] Open notifications widget with #notifications
Dhanus3133 Jul 10, 2024
67de9e5
[tests] Fixed test_without_batch_email_notification
nemesifier Jul 15, 2024
12288de
[fix] Updated
Dhanus3133 Jul 17, 2024
3760173
[chore] Update tests
Dhanus3133 Jul 17, 2024
2087b3e
Merge branch 'gsoc24' into feat/batch-email
Dhanus3133 Jul 17, 2024
56ac4cf
[chore] Update email title notifications count
Dhanus3133 Jul 17, 2024
12d19e2
[QA] Checks
Dhanus3133 Jul 17, 2024
49087b2
[chore] Update batch_email txt
Dhanus3133 Jul 18, 2024
a34920a
[fix] Use email_message instead of message
Dhanus3133 Jul 24, 2024
65dbdc1
[chore] Add email content tests
Dhanus3133 Jul 26, 2024
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
13 changes: 12 additions & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1014,7 +1014,7 @@ The default configuration is as follows:
}

``OPENWISP_NOTIFICATIONS_EMAIL_BATCH_INTERVAL``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

+---------+-----------------------------------+
| type | ``int`` |
Expand All @@ -1026,6 +1026,17 @@ This setting defines the interval at which the email notifications are sent in b

If you want to send email notifications immediately, then set it to ``0``.

``OPENWISP_NOTIFICATIONS_EMAIL_BATCH_DISPLAY_LIMIT``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

+---------+-----------------------------------+
| type | ``int`` |
+---------+-----------------------------------+
| default | ``15`` |
+---------+-----------------------------------+

This setting defines the number of email notifications to be displayed in a batched email.

Exceptions
----------

Expand Down
11 changes: 7 additions & 4 deletions openwisp_notifications/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
logger = logging.getLogger(__name__)

EXTRA_DATA = app_settings.get_config()['USE_JSONFIELD']
EMAIL_BATCH_INTERVAL = app_settings.OPENWISP_NOTIFICATIONS_EMAIL_BATCH_INTERVAL
EMAIL_BATCH_INTERVAL = app_settings.EMAIL_BATCH_INTERVAL

User = get_user_model()

Expand Down Expand Up @@ -201,6 +201,7 @@ def send_email_notification(sender, instance, created, **kwargs):
'last_email_sent_time': None,
'batch_scheduled': False,
'pks': [],
'start_time': None,
'email_id': instance.recipient.email,
},
)
Expand All @@ -209,12 +210,13 @@ def send_email_notification(sender, instance, created, **kwargs):
# Case 1: Batch email sending logic
if not cache_data['batch_scheduled']:
# Schedule batch email notification task if not already scheduled
tasks.batch_email_notification.apply_async(
tasks.send_batched_email_notifications.apply_async(
(instance.recipient.id,), countdown=EMAIL_BATCH_INTERVAL
)
# Mark batch as scheduled to prevent duplicate scheduling
cache_data['batch_scheduled'] = True
cache_data['pks'] = [instance.id]
cache_data['start_time'] = timezone.now()
cache.set(cache_key, cache_data)
else:
# Add current instance ID to the list of IDs for batch
Expand All @@ -224,8 +226,9 @@ def send_email_notification(sender, instance, created, **kwargs):

# Case 2: Single email sending logic
# Update the last email sent time and cache the data
cache_data['last_email_sent_time'] = timezone.now()
cache.set(cache_key, cache_data, timeout=EMAIL_BATCH_INTERVAL)
if EMAIL_BATCH_INTERVAL > 0:
cache_data['last_email_sent_time'] = timezone.now()
cache.set(cache_key, cache_data, timeout=EMAIL_BATCH_INTERVAL)

send_notification_email(instance)

Expand Down
6 changes: 5 additions & 1 deletion openwisp_notifications/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,14 @@
'openwisp-notifications/audio/notification_bell.mp3',
)

OPENWISP_NOTIFICATIONS_EMAIL_BATCH_INTERVAL = getattr(
EMAIL_BATCH_INTERVAL = getattr(
settings, 'OPENWISP_NOTIFICATIONS_EMAIL_BATCH_INTERVAL', 30 * 60 # 30 minutes
)

EMAIL_BATCH_DISPLAY_LIMIT = getattr(
settings, 'OPENWISP_NOTIFICATIONS_EMAIL_BATCH_DISPLAY_LIMIT', 15
)

# Remove the leading "/static/" here as it will
# conflict with the "static()" call in context_processors.py.
# This is done for backward compatibility.
Expand Down
29 changes: 21 additions & 8 deletions openwisp_notifications/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from django.db.utils import OperationalError
from django.template.loader import render_to_string
from django.utils import timezone
from django.utils.translation import gettext as _

from openwisp_notifications import settings as app_settings
from openwisp_notifications import types
Expand All @@ -17,7 +18,7 @@
from openwisp_utils.admin_theme.email import send_email
from openwisp_utils.tasks import OpenwispCeleryTask

EMAIL_BATCH_INTERVAL = app_settings.OPENWISP_NOTIFICATIONS_EMAIL_BATCH_INTERVAL
EMAIL_BATCH_INTERVAL = app_settings.EMAIL_BATCH_INTERVAL

User = get_user_model()

Expand Down Expand Up @@ -213,7 +214,7 @@ def delete_ignore_object_notification(instance_id):


@shared_task(base=OpenwispCeleryTask)
def batch_email_notification(instance_id):
def send_batched_email_notifications(instance_id):
"""
Sends a summary of notifications to the specified email address.
"""
Expand All @@ -226,7 +227,10 @@ def batch_email_notification(instance_id):
if not cache_data['pks']:
return

unsent_notifications = Notification.objects.filter(id__in=cache_data['pks'])
display_limit = app_settings.EMAIL_BATCH_DISPLAY_LIMIT
unsent_notifications = Notification.objects.filter(
id__in=cache_data['pks']
).order_by('-timestamp')
notifications_count = unsent_notifications.count()
current_site = Site.objects.get_current()
email_id = cache_data.get('email_id')
Expand All @@ -236,8 +240,8 @@ def batch_email_notification(instance_id):
notification = unsent_notifications.first()
send_notification_email(notification)
else:
# Show notification description upto 5 notifications
show_notification_description = notifications_count <= 5
# Show the amount of notifications according to configured display limit
show_notification_description = notifications_count <= display_limit
Dhanus3133 marked this conversation as resolved.
Show resolved Hide resolved
for notification in unsent_notifications:
Dhanus3133 marked this conversation as resolved.
Show resolved Hide resolved
url = notification.data.get('url', '') if notification.data else None
if url:
Expand All @@ -247,24 +251,33 @@ def batch_email_notification(instance_id):
else:
notification.url = None

starting_time = (
cache_data.get('start_time')
.strftime('%B %-d, %Y, %-I:%M %p')
.lower()
.replace('am', 'a.m.')
.replace('pm', 'p.m.')
) + ' UTC'

context = {
'notifications': unsent_notifications,
'notifications_count': notifications_count,
'show_notification_description': show_notification_description,
'site_name': current_site.name,
'start_time': starting_time,
}
html_content = render_to_string('emails/batch_email.html', context)
plain_text_content = render_to_string('emails/batch_email.txt', context)

extra_context = {}
if notifications_count > 5:
if notifications_count > display_limit:
extra_context = {
'call_to_action_url': f"https://{current_site.domain}/admin",
'call_to_action_text': 'View all Notifications',
'call_to_action_text': _('View all Notifications'),
}

send_email(
subject=f'Summary of {notifications_count} Notifications from {current_site.name}',
subject=f'[{current_site.name}] {notifications_count} new notifications since {starting_time}',
body_text=plain_text_content,
body_html=html_content,
recipients=[email_id],
Expand Down
15 changes: 8 additions & 7 deletions openwisp_notifications/templates/emails/batch_email.txt
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
Summary of {{ notifications_count }} Notifications from {{ site_name }}
{% load i18n %}

[{{ site_name }}] {{ notifications_count }} {% translate "new notifications since" %} {{ start_time }}

{% for notification in notifications %}
- {{ notification.message }}
URL: {{ notification.url }}
Timestamp: {{ notification.timestamp|date:"F j, Y, g:i a" }}
{% translate "URL" %}: {{ notification.url }}
Dhanus3133 marked this conversation as resolved.
Show resolved Hide resolved
{% translate "Timestamp" %}: {{ notification.timestamp|date:"F j, Y, g:i a" }}
Dhanus3133 marked this conversation as resolved.
Show resolved Hide resolved
{% if show_notification_description %}
Description: {{ notification.rendered_description }}
{% translate "Description" %}: {{ notification.rendered_description }}
Dhanus3133 marked this conversation as resolved.
Show resolved Hide resolved
{% endif %}
{% endfor %}

{% if notifications_count > 5 %}
View all Notifications: {{ call_to_action_url }}
{% if not show_notification_description %}
{% translate "View all Notifications" %}: {{ call_to_action_url }}
Dhanus3133 marked this conversation as resolved.
Show resolved Hide resolved
{% endif %}

29 changes: 21 additions & 8 deletions openwisp_notifications/tests/test_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from django.core.exceptions import ImproperlyConfigured
from django.db.models.signals import post_migrate, post_save
from django.template import TemplateDoesNotExist
from django.test import TransactionTestCase
from django.test import TransactionTestCase, override_settings
from django.urls import reverse
from django.utils import timezone
from django.utils.timesince import timesince
Expand Down Expand Up @@ -944,7 +944,7 @@ def test_notification_for_unverified_email(self):
# we don't send emails to unverified email addresses
self.assertEqual(len(mail.outbox), 0)

@patch('openwisp_notifications.tasks.batch_email_notification.apply_async')
@patch('openwisp_notifications.tasks.send_batched_email_notifications.apply_async')
def test_batch_email_notification_with_descriptions(self, mock_send_email):
for _ in range(5):
notify.send(recipient=self.admin, **self.notification_options)
Expand All @@ -953,37 +953,50 @@ def test_batch_email_notification_with_descriptions(self, mock_send_email):
self.assertEqual(len(mail.outbox), 1)

# Call the task
tasks.batch_email_notification(self.admin.id)
tasks.send_batched_email_notifications(self.admin.id)

# Check if the rest of the notifications are sent in a batch
self.assertEqual(len(mail.outbox), 2)
self.assertIn('Summary of 4 Notifications', mail.outbox[1].subject)
self.assertIn('4 new notifications since', mail.outbox[1].subject)
self.assertNotIn('View all Notifications', mail.outbox[1].body)
self.assertIn('Test Notification', mail.outbox[1].body)

@patch('openwisp_notifications.tasks.batch_email_notification.apply_async')
@patch('openwisp_notifications.tasks.send_batched_email_notifications.apply_async')
def test_batch_email_notification_with_call_to_action(self, mock_send_email):
self.notification_options.update(
{
'message': 'Notification title',
'type': 'default',
}
)
for _ in range(11):
for _ in range(21):
notify.send(recipient=self.admin, **self.notification_options)

# Check if only one mail is sent initially
self.assertEqual(len(mail.outbox), 1)

# Call the task
tasks.batch_email_notification(self.admin.id)
tasks.send_batched_email_notifications(self.admin.id)

# Check if the rest of the notifications are sent in a batch
self.assertEqual(len(mail.outbox), 2)
self.assertIn('Summary of 10 Notifications', mail.outbox[1].subject)
self.assertIn('20 new notifications since', mail.outbox[1].subject)
self.assertIn('View all Notifications', mail.outbox[1].body)
self.assertNotIn('Test Notification', mail.outbox[1].body)

Dhanus3133 marked this conversation as resolved.
Show resolved Hide resolved
@override_settings(EMAIL_BATCH_INTERVAL=0)
def test_without_batch_email_notification(self):
self.notification_options.update(
{
'message': 'Notification title',
'type': 'default',
}
)
for _ in range(3):
notify.send(recipient=self.admin, **self.notification_options)

self.assertEqual(len(mail.outbox), 3)
Dhanus3133 marked this conversation as resolved.
Show resolved Hide resolved

def test_that_the_notification_is_only_sent_once_to_the_user(self):
first_org = self._create_org()
first_org.organization_id = first_org.id
Expand Down
2 changes: 0 additions & 2 deletions openwisp_notifications/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@ def normalize_unread_count(unread_count):


def send_notification_email(notification):

"""Send a single email notification"""

try:
subject = notification.email_subject
except NotificationRenderException:
Expand Down