From f420dd98acdc5aca0152a5b4841643ee07ceeff8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89lie=20Bouttier?= Date: Thu, 4 Sep 2014 20:12:38 -0700 Subject: [PATCH] users can disable notifications --- accounts/forms.py | 2 +- .../migrations/0003_user_notifications.py | 20 ++++++++++++++++ accounts/models.py | 12 ++++++++++ accounts/tests.py | 3 +++ templates/base_project.html | 2 +- templates/tracker/issue_details.html | 6 ++--- tracker/notifications.py | 14 +++++++---- tracker/tests.py | 22 ++++++++++++++---- tracker/views.py | 23 ++++++++++++------- 9 files changed, 82 insertions(+), 22 deletions(-) create mode 100644 accounts/migrations/0003_user_notifications.py diff --git a/accounts/forms.py b/accounts/forms.py index 026b12d..6d2b23e 100644 --- a/accounts/forms.py +++ b/accounts/forms.py @@ -7,7 +7,7 @@ from accounts.models import * __all__ = ['UserForm', 'UserFormWithoutUsername', 'ProfileForm', 'GroupForm', 'TeamForm'] -user_fields=['first_name', 'last_name', 'email'] +user_fields=['first_name', 'last_name', 'email', 'notifications'] UserForm = modelform_factory(User, fields=['username']+user_fields+['is_superuser']) diff --git a/accounts/migrations/0003_user_notifications.py b/accounts/migrations/0003_user_notifications.py new file mode 100644 index 0000000..a0df08c --- /dev/null +++ b/accounts/migrations/0003_user_notifications.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('accounts', '0002_auto_20140905_0229'), + ] + + operations = [ + migrations.AddField( + model_name='user', + name='notifications', + field=models.IntegerField(choices=[(0, 'Never'), (1, 'Ignore my actions'), (2, 'Always')], default=1), + preserve_default=True, + ), + ] diff --git a/accounts/models.py b/accounts/models.py index 5f2913c..b586026 100644 --- a/accounts/models.py +++ b/accounts/models.py @@ -14,6 +14,18 @@ class User(AbstractUser): class Meta: ordering = ['username'] + NOTIFICATIONS_NEVER = 0 # do not change: used as boolean value + NOTIFICATIONS_OTHERS = 1 + NOTIFICATIONS_ALWAYS = 2 + NOTIFICATIONS_CHOICES = ( + (NOTIFICATIONS_NEVER, 'Never'), + (NOTIFICATIONS_OTHERS, 'Ignore my actions'), + (NOTIFICATIONS_ALWAYS, 'Always'), + ) + + notifications = models.IntegerField(choices=NOTIFICATIONS_CHOICES, + default=NOTIFICATIONS_OTHERS) + @property def teams(self): query = Q(groups__in=self.groups.all()) | Q(users=self) diff --git a/accounts/tests.py b/accounts/tests.py index 9c0da4d..1b454a1 100644 --- a/accounts/tests.py +++ b/accounts/tests.py @@ -20,6 +20,7 @@ class TestViews(TestCase): self.assertEqual(response.status_code, 200) response = self.client.post(reverse('profile'), { 'first_name': 'newfirstname', + 'notifications': User.NOTIFICATIONS_OTHERS, }, follow=True) self.assertRedirects(response, reverse('profile')) self.assertContains(response, 'successfully') @@ -46,6 +47,7 @@ class TestViews(TestCase): user_count = User.objects.count() response = self.client.post(reverse('add-user'), { 'username': 'newuser', + 'notifications': User.NOTIFICATIONS_OTHERS, }) self.assertEqual(User.objects.count(), user_count + 1) user = User.objects.get(username='newuser') @@ -63,6 +65,7 @@ class TestViews(TestCase): response = self.client.post(reverse('edit-user', args=[user.id]), { 'username': user.username, 'first_name': 'newfirstname', + 'notifications': User.NOTIFICATIONS_OTHERS, }) self.assertRedirects(response, reverse('show-user', args=[user.id])) user = User.objects.get(pk=user.pk) diff --git a/templates/base_project.html b/templates/base_project.html index 9b9b11a..5bbbcf4 100644 --- a/templates/base_project.html +++ b/templates/base_project.html @@ -20,7 +20,7 @@ {% endcomment %} {% if request.user.is_authenticated %} -{% if request.user.email and request.user in project.subscribers.all %} +{% if request.user.email and request.user.notifications and request.user in project.subscribers.all %}
  • Unwatch
    • {% else %}
  • Watch
    • diff --git a/templates/tracker/issue_details.html b/templates/tracker/issue_details.html index d9afa86..7f0df19 100644 --- a/templates/tracker/issue_details.html +++ b/templates/tracker/issue_details.html @@ -193,15 +193,13 @@
    - {% if request.user.email and request.user in project.subscribers.all %} + {% if request.user.email and request.user.notifications and request.user in project.subscribers.all %} Subscribed to the project - {% else %} - {% if request.user in issue.subscribers.all %} + {% elif request.user.notifications and request.user in issue.subscribers.all %}  Unsubscribe {% else %}  Subscribe {% endif %} - {% endif %}
    {% endif %} diff --git a/tracker/notifications.py b/tracker/notifications.py index 8d02f05..92df90f 100644 --- a/tracker/notifications.py +++ b/tracker/notifications.py @@ -9,6 +9,8 @@ if 'djcelery' in settings.INSTALLED_APPS: else: from django.core.mail import send_mass_mail +from accounts.models import User + __all__ = [ 'notify_new_issue', 'notify_new_comment', @@ -31,9 +33,11 @@ def notify_new_issue(issue): data = [] for dest in dests: - if dest == issue.author: + if dest.notifications == User.NOTIFICATIONS_NEVER: + continue + if dest == issue.author \ + and dest.notifications == User.NOTIFICATIONS_OTHERS: continue - dest_addr = dest.email if not dest_addr: continue @@ -87,9 +91,11 @@ def notify_event(event, template): for dest in dests: - if dest == event.author: + if dest.notifications == User.NOTIFICATIONS_NEVER: + continue + if dest == issue.author \ + and dest.notifications == User.NOTIFICATIONS_OTHERS: continue - dest_addr = dest.email if not dest_addr: continue diff --git a/tracker/tests.py b/tracker/tests.py index cfcf8e8..b97479a 100644 --- a/tracker/tests.py +++ b/tracker/tests.py @@ -127,9 +127,16 @@ class TestViews(TestCase): response = self.client.get(reverse('unsubscribe-project', args=[project.name]), follow=True) self.assertRedirects(response, reverse('list-issue', args=[project.name])) self.assertContains(response, 'not subscribed to this project') - response = self.client.get(reverse('subscribe-project', args=[project.name])) - self.assertRedirects(response, reverse('profile')) + response = self.client.get(reverse('subscribe-project', args=[project.name]), follow=True) + self.assertRedirects(response, reverse('list-issue', args=[project.name])) + self.assertContains(response, 'must set an email') user.email = 'user@example.com' + user.notifications = User.NOTIFICATIONS_NEVER + user.save() + response = self.client.get(reverse('subscribe-project', args=[project.name]), follow=True) + self.assertRedirects(response, reverse('list-issue', args=[project.name])) + self.assertContains(response, 'must enable notifications') + user.notifications = User.NOTIFICATIONS_OTHERS user.save() response = self.client.get(reverse('subscribe-project', args=[project.name])) self.assertRedirects(response, reverse('list-issue', args=[project.name])) @@ -343,9 +350,16 @@ class TestViews(TestCase): response = self.client.get(reverse('unsubscribe-issue', args=[project.name, issue.id]), follow=True) self.assertRedirects(response, reverse('show-issue', args=[project.name, issue.id])) self.assertContains(response, 'not subscribed to this issue') - response = self.client.get(reverse('subscribe-issue', args=[project.name, issue.id])) - self.assertRedirects(response, reverse('profile')) + response = self.client.get(reverse('subscribe-issue', args=[project.name, issue.id]), follow=True) + self.assertRedirects(response, reverse('show-issue', args=[project.name, issue.id])) + self.assertContains(response, 'must set an email') user.email = 'user@example.com' + user.notifications = User.NOTIFICATIONS_NEVER + user.save() + response = self.client.get(reverse('subscribe-issue', args=[project.name, issue.id]), follow=True) + self.assertRedirects(response, reverse('show-issue', args=[project.name, issue.id])) + self.assertContains(response, 'must enable notifications') + user.notifications = User.NOTIFICATIONS_OTHERS user.save() response = self.client.get(reverse('subscribe-issue', args=[project.name, issue.id])) self.assertRedirects(response, reverse('show-issue', args=[project.name, issue.id])) diff --git a/tracker/views.py b/tracker/views.py index 5dd7ea3..3e9c918 100644 --- a/tracker/views.py +++ b/tracker/views.py @@ -4,6 +4,7 @@ from django.core.exceptions import ObjectDoesNotExist, PermissionDenied from django.contrib.auth.decorators import login_required from django.views.decorators.http import require_http_methods from django.conf import settings +from django.core.urlresolvers import reverse from tracker.forms import * from tracker.models import * @@ -130,10 +131,14 @@ def project_delete(request, project): def project_subscribe(request, project): if not request.user.email: - messages.error(request, 'You must set an email address in order to receive notifications.') - return redirect('profile') - - if project.subscribers.filter(pk=request.user.pk).exists(): + messages.error(request, 'You must set an email address in your ' + 'profile in order ' + 'to watch this project.') + elif request.user.notifications == User.NOTIFICATIONS_NEVER: + messages.error(request, 'You must enable notifications in your ' + 'profile in order ' + 'to watch this project.') + elif project.subscribers.filter(pk=request.user.pk).exists(): messages.warning(request, 'You are already subscribed to this project.') else: @@ -544,10 +549,12 @@ def issue_subscribe(request, project, issue): issue = get_object_or_404(Issue, project=project, id=issue) if not request.user.email: - messages.error(request, 'You must set an email address in order to receive notifications.') - return redirect('profile') - - if issue.subscribers.filter(pk=request.user.pk).exists(): + messages.error(request, 'You must set an email address in your ' + 'profile to subscribe.') + elif request.user.notifications == User.NOTIFICATIONS_NEVER: + messages.error(request, 'You must enable notifications in your ' + 'profile to subscribe.') + elif issue.subscribers.filter(pk=request.user.pk).exists(): messages.warning(request, 'You are already subscribed to this issue.') else: issue.subscribers.add(request.user)