entrouvert
/
combo
16
1
Fork 0
Code Issues Pull Requests 12 Releases Wiki Activity

notifications: improve internal API (#22390) 

* (user, external_id) is now unique, no need to user .first() / .last()
* new filter .find(user, id), .visible(user), new queryset actions .ack() and .forget()
* new property notification.visible to know if a notification is shown
* when notify() is called on an existing notification, it's unacked
* notify's parameters url, body and origin now default to '', serializer
has been updated
This commit is contained in:
Benjamin Dauvergne 2018-03-16 12:35:28 +01:00
parent 64c767c5ca
commit f2719b3a68
4 changed files with 208 additions and 112 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
combo/apps/notifications/api_views.py
View File

@ -14,7 +14,7 @@
# You should have received a copy of the GNU Affero General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
from rest_framework import authentication, serializers, permissions, status
from rest_framework import serializers, permissions, status
from rest_framework.generics import GenericAPIView
from rest_framework.response import Response
@ -23,10 +23,10 @@ from .models import Notification
class NotificationSerializer(serializers.Serializer):
summary = serializers.CharField(required=True, allow_blank=False, max_length=140)
id = serializers.SlugField(required=False, allow_null=True)
body = serializers.CharField(required=False, allow_blank=False)
url = serializers.URLField(required=False, allow_blank=True)
origin = serializers.CharField(required=False, allow_blank=True)
id = serializers.CharField(required=False, allow_null=True)
body = serializers.CharField(allow_blank=False, default='')
url = serializers.URLField(allow_blank=True, default='')
origin = serializers.CharField(allow_blank=True, default='')
start_timestamp = serializers.DateTimeField(required=False, allow_null=True)
end_timestamp = serializers.DateTimeField(required=False, allow_null=True)
duration = serializers.IntegerField(required=False, allow_null=True, min_value=0)
@ -42,30 +42,33 @@ class Add(GenericAPIView):
response = {'err': 1, 'err_desc': serializer.errors}
return Response(response, status.HTTP_400_BAD_REQUEST)
data = serializer.validated_data
notification_id, created = Notification.notify(
user=request.user,
summary=data['summary'],
id=data.get('id'),
body=data.get('body'),
url=data.get('url'),
origin=data.get('origin'),
start_timestamp=data.get('start_timestamp'),
end_timestamp=data.get('end_timestamp'),
duration=data.get('duration')
)
response = {'err': 0, 'data': {'id': notification_id, 'created': created}}
return Response(response)
try:
notification = Notification.notify(
user=request.user,
summary=data['summary'],
id=data.get('id'),
body=data.get('body'),
url=data.get('url'),
origin=data.get('origin'),
start_timestamp=data.get('start_timestamp'),
end_timestamp=data.get('end_timestamp'),
duration=data.get('duration')
)
except ValueError as e:
response = {'err': 1, 'err_desc': {'id': [unicode(e)]}}
return Response(response, status.HTTP_400_BAD_REQUEST)
else:
response = {'err': 0, 'data': {'id': notification.public_id}}
return Response(response)
add = Add.as_view()
class Ack(GenericAPIView):
authentication_classes = (authentication.SessionAuthentication,)
permission_classes = (permissions.IsAuthenticated,)
def get(self, request, notification_id):
Notification.ack(request.user, notification_id)
def get(self, request, notification_id, *args, **kwargs):
Notification.objects.find(request.user, notification_id).ack()
return Response({'err': 0})
ack = Ack.as_view()
@ -75,7 +78,7 @@ class Forget(GenericAPIView):
permission_classes = (permissions.IsAuthenticated,)
def get(self, request, notification_id, *args, **kwargs):
Notification.forget(request.user, notification_id)
Notification.objects.find(request.user, notification_id).forget()
return Response({'err': 0})
forget = Forget.as_view()

21
combo/apps/notifications/migrations/0004_auto_20180316_1026.py Normal file
View File

@ -0,0 +1,21 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.11 on 2018-03-16 10:26
from __future__ import unicode_literals
from django.conf import settings
from django.db import migrations
class Migration(migrations.Migration):
dependencies = [
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
('notifications', '0003_notification_origin'),
]
operations = [
migrations.AlterUniqueTogether(
name='notification',
unique_together=set([('user', 'external_id')]),
),
]

130
combo/apps/notifications/models.py
View File

@ -14,29 +14,53 @@
# You should have received a copy of the GNU Affero General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
import re
from django.conf import settings
from django.db import models
from django.utils.translation import ugettext_lazy as _
from django.utils.timezone import now, localtime, timedelta
from django.utils.timezone import now, timedelta
from django.db.models import Q
from django.db.models.query import QuerySet
from combo.data.models import CellBase
from combo.data.library import register_cell_class
class NotificationManager(models.Manager):
def filter_by_id(self, id):
class NotificationQuerySet(QuerySet):
def find(self, user, id):
qs = self.filter(user=user)
try:
int(id)
except (ValueError, TypeError):
search_id = Q(external_id=id)
else:
search_id = Q(pk=id) | Q(external_id=id)
return self.filter(search_id)
return qs.filter(search_id)
def ack(self):
self.update(acked=True)
def visible(self, user):
n = now()
qs = self.filter(user=user,
start_timestamp__lte=n,
end_timestamp__gt=n)
return qs.order_by('-start_timestamp')
def new(self):
return self.filter(acked=False)
def forget(self):
past_end_timestamp = now() - timedelta(seconds=5)
self.update(
end_timestamp=past_end_timestamp, acked=True)
class Notification(models.Model):
objects = NotificationManager()
ID_RE = r'^[\w-]+:[\w]+$'
objects = NotificationQuerySet.as_manager()
user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE)
summary = models.CharField(_('Label'), max_length=140)
@ -48,18 +72,23 @@ class Notification(models.Model):
acked = models.BooleanField(_('Acked'), default=False)
external_id = models.SlugField(_('External identifier'), null=True)
class Meta:
verbose_name = _('Notification')
unique_together = (
('user', 'external_id'),
)
def __unicode__(self):
return self.summary
@property
def public_id(self):
return self.external_id or str(self.pk)
@classmethod
def notify(cls, user, summary, id=None, body=None, url=None, origin=None,
start_timestamp=None, duration=None, end_timestamp=None):
def notify(cls, user, summary, id=None, body='', url='', origin='',
start_timestamp=None, duration=None, end_timestamp=None):
'''
Create a new notification:
Notification.notify(user, 'summary') -> id
@ -68,43 +97,60 @@ class Notification(models.Model):
Renew an existing notification, or create a new one, with an external_id:
Notification.notify(user, 'summary', id='id')
'''
created = False
# get ...
notification = Notification.objects.filter_by_id(id).filter(user=user).first() if id else None
if not notification: # ... or create
notification = Notification(user=user, summary=summary,
body=body or '', url=url or '',
external_id=id)
created = True
notification.summary = summary
notification.body = body or ''
notification.url = url or ''
notification.origin = origin or ''
notification.start_timestamp = start_timestamp or now()
start_timestamp = start_timestamp or now()
if duration:
if not isinstance(duration, timedelta):
duration = timedelta(seconds=duration)
notification.end_timestamp = notification.start_timestamp + duration
end_timestamp = start_timestamp + duration
else:
notification.end_timestamp = end_timestamp or notification.start_timestamp + timedelta(3)
end_timestamp = end_timestamp or start_timestamp + timedelta(days=3)
notification.save()
defaults = {
'summary': summary,
'body': body,
'url': url,
'origin': origin,
'start_timestamp': start_timestamp,
'end_timestamp': end_timestamp,
'acked': False,
}
if notification.external_id is None:
notification_id = '%s' % notification.pk
try:
pk = int(id)
# id is maybe an implicit id
notification = Notification.objects.get(pk=pk)
Notification.objects.filter(pk=pk).update(**defaults)
return notification
except (ValueError, TypeError, Notification.DoesNotExist):
pass
if id:
try:
id = unicode(id)
except Exception as e:
raise ValueError('id must be convertible to unicode', e)
if not re.match(cls.ID_RE, id):
raise ValueError('id must match regular expression %s' % cls.ID_RE)
notification, created = Notification.objects.update_or_create(
user=user, external_id=id,
defaults=defaults)
else:
notification_id = notification.external_id
return notification_id, created
notification = Notification.objects.create(user=user, **defaults)
return notification
@classmethod
def ack(cls, user, id):
Notification.objects.filter_by_id(id).filter(user=user).update(acked=True)
@property
def visible(self):
return self.end_timestamp > now()
@classmethod
def forget(cls, user, id):
past = now() - timedelta(seconds=5)
Notification.objects.filter_by_id(id).filter(user=user).update(end_timestamp=past,
acked=True)
def forget(self):
self.end_timestamp = now() - timedelta(seconds=5)
self.acked = True
self.save(update_fields=['end_timestamp', 'acked'])
def ack(self):
self.acked = True
self.save(update_fields=['acked'])
@register_cell_class
@ -125,18 +171,16 @@ class NotificationsCell(CellBase):
extra_context = super(NotificationsCell, self).get_cell_extra_context(context)
user = getattr(context.get('request'), 'user', None)
if user and user.is_authenticated():
extra_context['notifications'] = Notification.objects.filter(user=user,
start_timestamp__lte=now(), end_timestamp__gt=now()).order_by('-start_timestamp')
extra_context['new_notifications'] = extra_context['notifications'].filter(acked=False)
qs = Notification.objects.visible(user)
extra_context['notifications'] = qs
extra_context['new_notifications'] = qs.new()
return extra_context
def get_badge(self, context):
user = getattr(context.get('request'), 'user', None)
if not user or not user.is_authenticated():
return
notifs = Notification.objects.filter(user=user, start_timestamp__lte=now(),
end_timestamp__gt=now())
new = notifs.filter(acked=False).count()
if not new:
new_count = Notification.objects.visible(user).new().count()
if not new_count:
return
return {'badge': str(new)}
return {'badge': str(new_count)}

120
tests/test_notification.py
View File

@ -16,6 +16,7 @@ pytestmark = pytest.mark.django_db
client = Client()
@pytest.fixture
def user():
try:
@ -26,6 +27,7 @@ def user():
admin.save()
return admin
@pytest.fixture
def user2():
try:
@ -34,49 +36,47 @@ def user2():
admin2 = User.objects.create_user('admin2', email=None, password='admin2')
return admin2
def login(username='admin', password='admin'):
resp = client.post('/login/', {'username': username, 'password': password})
assert resp.status_code == 302
def test_notification_api(user, user2):
id_notifoo, created = Notification.notify(user, 'notifoo')
assert created
assert Notification.objects.all().count() == 1
noti = Notification.objects.filter_by_id(id_notifoo).filter(user=user).first()
assert noti.pk == int(id_notifoo)
assert noti.summary == 'notifoo'
assert noti.body == ''
assert noti.url == ''
assert noti.external_id is None
assert noti.end_timestamp - noti.start_timestamp == timedelta(3)
assert noti.acked is False
Notification.ack(user, id_notifoo)
noti = Notification.objects.filter_by_id(id_notifoo).filter(user=user).first()
assert noti.acked is True
notification = Notification.notify(user, 'notifoo')
assert Notification.objects.count() == 1
assert notification.summary == 'notifoo'
assert notification.body == ''
assert notification.url == ''
assert notification.origin == ''
assert notification.external_id is None
assert notification.end_timestamp - notification.start_timestamp == timedelta(3)
assert notification.acked is False
Notification.objects.visible(user).ack()
assert Notification.objects.get().acked is True
Notification.notify(user, 'notirefoo', id=id_notifoo)
assert Notification.objects.all().count() == 1
noti = Notification.objects.filter_by_id(id_notifoo).filter(user=user).first()
assert noti.pk == int(id_notifoo)
assert noti.summary == 'notirefoo'
Notification.notify(user, 'notirefoo', id=str(notification.pk))
assert Notification.objects.count() == 1
assert Notification.objects.get().summary == 'notirefoo'
# we updated the notification, it's un-acked
assert Notification.objects.get().acked is False
Notification.notify(user, 'notirefoo', id=id_notifoo, duration=3600)
noti = Notification.objects.filter_by_id(id_notifoo).filter(user=user).first()
Notification.notify(user, 'notirefoo', id=str(notification.pk), duration=3600)
noti = Notification.objects.get()
assert noti.end_timestamp - noti.start_timestamp == timedelta(seconds=3600)
notification, created = Notification.notify(user, 'notibar', id='notibar')
assert created
assert Notification.objects.all().count() == 2
notification, created = Notification.notify(user, 'notirebar', id='notibar')
assert not created
assert Notification.objects.all().count() == 2
notification = Notification.notify(user, 'notibar', id='ns:notibar')
assert Notification.objects.count() == 2
notification = Notification.notify(user, 'notirebar', id='ns:notibar')
assert Notification.objects.count() == 2
notification = Notification.notify(user2, 'notiother')
notification.forget()
assert Notification.objects.filter(user=user2).count() == 1
notification = Notification.objects.filter(user=user2).get()
assert notification.end_timestamp < now()
assert notification.acked is True
id2, created = Notification.notify(user2, 'notiother')
Notification.forget(user2, id2)
noti = Notification.objects.filter_by_id(id2).filter(user=user2).first()
assert noti.end_timestamp < now()
assert noti.acked is True
def test_notification_cell(user, user2):
page = Page(title='notif', slug='test_notification_cell', template_name='standard')
@ -92,36 +92,36 @@ def test_notification_cell(user, user2):
assert cell.is_visible(context['request'].user) is True
assert cell.get_badge(context) is None
id_noti1, created = Notification.notify(user, 'notibar')
id_noti2, created = Notification.notify(user, 'notifoo')
notification1 = Notification.notify(user, 'notibar')
notification2 = Notification.notify(user, 'notifoo')
content = cell.render(context)
assert 'notibar' in content
assert 'notifoo' in content
assert cell.get_badge(context) == {'badge': '2'}
Notification.forget(user, id_noti2)
notification2.forget()
content = cell.render(context)
assert 'notibar' in content
assert 'notifoo' not in content
assert cell.get_badge(context) == {'badge': '1'}
Notification.notify(user, 'notirebar', id=id_noti1)
Notification.notify(user, 'notirebar', id=str(notification1.pk))
content = cell.render(context)
assert 'notirebar' in content
assert 'notibar' not in content
Notification.notify(user, 'notiurl', id=id_noti1, url='https://www.example.net/')
Notification.notify(user, 'notiurl', id=str(notification1.pk), url='https://www.example.net/')
content = cell.render(context)
assert 'notiurl' in content
assert 'https://www.example.net/' in content
ackme, created = Notification.notify(user, 'ackme')
Notification.ack(user, id=ackme)
notification3 = Notification.notify(user, 'ackme')
notification3.ack()
content = cell.render(context)
assert 'acked' in content
assert cell.get_badge(context) == {'badge': '1'}
Notification.ack(user, id=id_noti1)
notification1.ack()
content = cell.render(context)
assert cell.get_badge(context) is None
@ -136,6 +136,7 @@ def test_notification_cell(user, user2):
assert 'notiother' in content
assert cell.get_badge(context) == {'badge': '1'}
def test_notification_ws(user):
def notify(data, check_id, count):
@ -143,14 +144,14 @@ def test_notification_ws(user):
content_type='application/json')
assert resp.status_code == 200
result = json.loads(resp.content)
assert result == {'data': {'id': check_id, 'created': True}, 'err': 0}
assert result == {'data': {'id': check_id}, 'err': 0}
assert Notification.objects.filter(user=user).count() == count
return Notification.objects.filter_by_id(check_id).last()
return Notification.objects.find(user, check_id).get()
login()
notify({'summary': 'foo'}, '1', 1)
notify({'summary': 'bar'}, '2', 2)
notify({'summary': 'bar', 'id': 'noti3'}, 'noti3', 3)
notify({'summary': 'bar', 'id': 'ns:noti3'}, 'ns:noti3', 3)
notif = {
'summary': 'bar',
'url': 'http://www.example.net',
@ -179,16 +180,17 @@ def test_notification_ws(user):
resp = client.get(reverse('api-notification-ack', kwargs={'notification_id': '6'}))
assert resp.status_code == 200
assert Notification.objects.filter(acked=True).count() == 1
assert Notification.objects.filter(acked=True).first().public_id() == '6'
assert Notification.objects.filter(acked=True).get().public_id == '6'
resp = client.get(reverse('api-notification-forget', kwargs={'notification_id': '5'}))
assert resp.status_code == 200
assert Notification.objects.filter(acked=True).count() == 2
notif = Notification.objects.filter_by_id('5').filter(user=user).first()
assert notif.public_id() == '5'
notif = Notification.objects.find(user, '5').get()
assert notif.public_id == '5'
assert notif.acked is True
assert notif.end_timestamp < now()
def test_notification_ws_badrequest(user):
def check_error(data, message):
@ -212,6 +214,7 @@ def test_notification_ws_badrequest(user):
check_error({'summary': 'ok', 'duration': 4.01}, 'valid integer is required')
check_error({'summary': 'ok', 'duration': -4}, 'greater than')
def test_notification_ws_deny():
assert client.post(reverse('api-notification-add'),
json.dumps({'summary': 'ok'}),
@ -221,9 +224,34 @@ def test_notification_ws_deny():
assert client.get(reverse('api-notification-forget',
kwargs={'notification_id': '1'})).status_code == 403
def test_notification_ws_check_urls():
assert reverse('api-notification-add') == '/api/notification/add/'
assert reverse('api-notification-ack',
kwargs={'notification_id': 'noti1'}) == '/api/notification/ack/noti1/'
assert reverse('api-notification-forget',
kwargs={'notification_id': 'noti1'}) == '/api/notification/forget/noti1/'
def test_notification_id_and_origin(user):
login()
def notify(data):
resp = client.post(reverse('api-notification-add'), json.dumps(data),
content_type='application/json')
return json.loads(resp.content)
result = notify({'summary': 'foo', 'id': '1'})
assert result['err'] == 1
notification = Notification.notify(user, 'foo')
result = notify({'summary': 'foo', 'id': str(notification.id)})
assert result['err'] == 0
result = notify({'summary': 'foo', 'id': 'foo'})
assert result['err'] == 1
result = notify({'summary': 'foo', 'id': 'foo:foo', 'origin': 'bar'})
assert result['err'] == 0