Projet

Général

Profil

0004-notifications-improve-internal-API-22390.patch

Benjamin Dauvergne, 18 mars 2018 22:13

Télécharger (19,7 ko)

Voir les différences:

Subject: [PATCH 4/8] 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
 combo/apps/notifications/api_views.py              |  17 ++-
 .../migrations/0004_auto_20180316_1026.py          |  21 ++++
 combo/apps/notifications/models.py                 | 126 ++++++++++++++-------
 tests/test_notification.py                         |  90 ++++++++-------
 4 files changed, 159 insertions(+), 95 deletions(-)
 create mode 100644 combo/apps/notifications/migrations/0004_auto_20180316_1026.py
combo/apps/notifications/api_views.py
24 24
class NotificationSerializer(serializers.Serializer):
25 25
    summary = serializers.CharField(required=True, allow_blank=False, max_length=140)
26 26
    id = serializers.SlugField(required=False, allow_null=True)
27
    body = serializers.CharField(required=False, allow_blank=False)
28
    url = serializers.URLField(required=False, allow_blank=True)
29
    origin = serializers.CharField(required=False, allow_blank=True)
27
    body = serializers.CharField(allow_blank=False, default='')
28
    url = serializers.URLField(allow_blank=True, default='')
29
    origin = serializers.CharField(allow_blank=True, default='')
30 30
    start_timestamp = serializers.DateTimeField(required=False, allow_null=True)
31 31
    end_timestamp = serializers.DateTimeField(required=False, allow_null=True)
32 32
    duration = serializers.IntegerField(required=False, allow_null=True, min_value=0)
......
43 43
            return Response(response, status.HTTP_400_BAD_REQUEST)
44 44
        data = serializer.validated_data
45 45

  
46
        notification_id, created = Notification.notify(
46
        notification, created = Notification.notify(
47 47
            user=request.user,
48 48
            summary=data['summary'],
49 49
            id=data.get('id'),
......
54 54
            end_timestamp=data.get('end_timestamp'),
55 55
            duration=data.get('duration')
56 56
        )
57
        response = {'err': 0, 'data': {'id': notification_id, 'created': created}}
57
        response = {'err': 0, 'data': {'id': notification.public_id, 'created': created}}
58 58
        return Response(response)
59 59

  
60 60
add = Add.as_view()
61 61

  
62 62

  
63 63
class Ack(GenericAPIView):
64
    authentication_classes = (authentication.SessionAuthentication,)
65 64
    permission_classes = (permissions.IsAuthenticated,)
66 65

  
67
    def get(self, request, notification_id):
68
        Notification.ack(request.user, notification_id)
66
    def get(self, request, notification_id, *args, **kwargs):
67
        Notification.objects.find(request.user, notification_id).ack()
69 68
        return Response({'err': 0})
70 69

  
71 70
ack = Ack.as_view()
......
75 74
    permission_classes = (permissions.IsAuthenticated,)
76 75

  
77 76
    def get(self, request, notification_id, *args, **kwargs):
78
        Notification.forget(request.user, notification_id)
77
        Notification.objects.find(request.user, notification_id).forget()
79 78
        return Response({'err': 0})
80 79

  
81 80
forget = Forget.as_view()
combo/apps/notifications/migrations/0004_auto_20180316_1026.py
1
# -*- coding: utf-8 -*-
2
# Generated by Django 1.11.11 on 2018-03-16 10:26
3
from __future__ import unicode_literals
4

  
5
from django.conf import settings
6
from django.db import migrations
7

  
8

  
9
class Migration(migrations.Migration):
10

  
11
    dependencies = [
12
        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
13
        ('notifications', '0003_notification_origin'),
14
    ]
15

  
16
    operations = [
17
        migrations.AlterUniqueTogether(
18
            name='notification',
19
            unique_together=set([('user', 'external_id')]),
20
        ),
21
    ]
combo/apps/notifications/models.py
17 17
from django.conf import settings
18 18
from django.db import models
19 19
from django.utils.translation import ugettext_lazy as _
20
from django.utils.timezone import now, localtime, timedelta
20
from django.utils.timezone import now, timedelta
21 21
from django.db.models import Q
22
from django.db.models.query import QuerySet
22 23

  
23 24
from combo.data.models import CellBase
24 25
from combo.data.library import register_cell_class
25 26

  
26 27

  
27
class NotificationManager(models.Manager):
28
    def filter_by_id(self, id):
28
class NotificationQuerySet(QuerySet):
29
    def find(self, user, id):
30
        qs = self.filter(user=user)
29 31
        try:
30 32
            int(id)
31 33
        except (ValueError, TypeError):
32 34
            search_id = Q(external_id=id)
33 35
        else:
34 36
            search_id = Q(pk=id) | Q(external_id=id)
35
        return self.filter(search_id)
37
        return qs.filter(search_id)
38

  
39
    def ack(self):
40
        self.update(acked=True)
41

  
42
    def visible(self, user):
43
        n = now()
44
        qs = self.filter(user=user,
45
                         start_timestamp__lte=n,
46
                         end_timestamp__gt=n)
47
        return qs.order_by('-start_timestamp')
48

  
49
    def new(self):
50
        return self.filter(acked=False)
51

  
52
    def forget(self):
53
        past_end_timestamp = now() - timedelta(seconds=5)
54
        self.update(
55
            end_timestamp=past_end_timestamp, acked=True)
36 56

  
37 57

  
38 58
class Notification(models.Model):
39
    objects = NotificationManager()
59
    objects = NotificationQuerySet.as_manager()
40 60

  
41 61
    user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE)
42 62
    summary = models.CharField(_('Label'), max_length=140)
......
50 70

  
51 71
    class Meta:
52 72
        verbose_name = _('Notification')
73
        unique_together = (
74
            ('user', 'external_id'),
75
        )
53 76

  
54 77
    def __unicode__(self):
55 78
        return self.summary
56 79

  
80
    @property
57 81
    def public_id(self):
58 82
        return self.external_id or str(self.pk)
59 83

  
60 84
    @classmethod
61
    def notify(cls, user, summary, id=None, body=None, url=None, origin=None,
62
           start_timestamp=None, duration=None, end_timestamp=None):
85
    def notify(cls, user, summary, id=None, body='', url='', origin='',
86
               start_timestamp=None, duration=None, end_timestamp=None):
63 87
        '''
64 88
        Create a new notification:
65 89
            Notification.notify(user, 'summary') -> id
......
68 92
        Renew an existing notification, or create a new one, with an external_id:
69 93
            Notification.notify(user, 'summary', id='id')
70 94
        '''
71
        created = False
72
        # get ...
73
        notification = Notification.objects.filter_by_id(id).filter(user=user).first() if id else None
74
        if not notification: # ... or create
75
            notification = Notification(user=user, summary=summary,
76
                                        body=body or '', url=url or '',
77
                                        external_id=id)
78
            created = True
79
        notification.summary = summary
80
        notification.body = body or ''
81
        notification.url = url or ''
82
        notification.origin = origin or ''
83
        notification.start_timestamp = start_timestamp or now()
95
        start_timestamp = start_timestamp or now()
96

  
84 97
        if duration:
85 98
            if not isinstance(duration, timedelta):
86 99
                duration = timedelta(seconds=duration)
87
            notification.end_timestamp = notification.start_timestamp + duration
100
            end_timestamp = start_timestamp + duration
88 101
        else:
89
            notification.end_timestamp = end_timestamp or notification.start_timestamp + timedelta(3)
102
            end_timestamp = end_timestamp or start_timestamp + timedelta(days=3)
103

  
104
        defaults = {
105
            'summary': summary,
106
            'body': body,
107
            'url': url,
108
            'origin': origin,
109
            'start_timestamp': start_timestamp,
110
            'end_timestamp': end_timestamp,
111
            'acked': False,
112
        }
90 113

  
91
        notification.save()
92

  
93
        if notification.external_id is None:
94
            notification_id = '%s' % notification.pk
114
        try:
115
            pk = int(id)
116
            # id is maybe an implicit id
117
            notification = Notification.objects.get(pk=pk)
118
            Notification.objects.filter(pk=pk).update(**defaults)
119
            return notification, False
120
        except (ValueError, TypeError, Notification.DoesNotExist):
121
            pass
122

  
123
        if id:
124
            try:
125
                id = unicode(id)
126
            except Exception as e:
127
                raise ValueError('id must be convertible to unicode', e)
128
            notification, created = Notification.objects.update_or_create(
129
                user=user, external_id=id,
130
                defaults=defaults)
95 131
        else:
96
            notification_id = notification.external_id
97
        return notification_id, created
132
            notification = Notification.objects.create(user=user, **defaults)
133
            created = True
134
        return notification, created
98 135

  
99
    @classmethod
100
    def ack(cls, user, id):
101
        Notification.objects.filter_by_id(id).filter(user=user).update(acked=True)
136
    @property
137
    def visible(self):
138
        return self.end_timestamp > now()
102 139

  
103
    @classmethod
104
    def forget(cls, user, id):
105
        past = now() - timedelta(seconds=5)
106
        Notification.objects.filter_by_id(id).filter(user=user).update(end_timestamp=past,
107
                                                                       acked=True)
140
    def forget(self):
141
        self.end_timestamp = now() - timedelta(seconds=5)
142
        self.acked = True
143
        self.save(update_fields=['end_timestamp', 'acked'])
144

  
145
    def ack(self):
146
        self.acked = True
147
        self.save(update_fields=['acked'])
108 148

  
109 149

  
110 150
@register_cell_class
......
125 165
        extra_context = super(NotificationsCell, self).get_cell_extra_context(context)
126 166
        user = getattr(context.get('request'), 'user', None)
127 167
        if user and user.is_authenticated():
128
            extra_context['notifications'] = Notification.objects.filter(user=user,
129
                    start_timestamp__lte=now(), end_timestamp__gt=now()).order_by('-start_timestamp')
130
            extra_context['new_notifications'] = extra_context['notifications'].filter(acked=False)
168
            qs = Notification.objects.visible(user)
169
            extra_context['notifications'] = qs
170
            extra_context['new_notifications'] = qs.new()
131 171
        return extra_context
132 172

  
133 173
    def get_badge(self, context):
134 174
        user = getattr(context.get('request'), 'user', None)
135 175
        if not user or not user.is_authenticated():
136 176
            return
137
        notifs = Notification.objects.filter(user=user, start_timestamp__lte=now(),
138
                                             end_timestamp__gt=now())
139
        new = notifs.filter(acked=False).count()
140
        if not new:
177
        new_count = Notification.objects.visible(user).new().count()
178
        if not new_count:
141 179
            return
142
        return {'badge': str(new)}
180
        return {'badge': str(new_count)}
tests/test_notification.py
16 16

  
17 17
client = Client()
18 18

  
19

  
19 20
@pytest.fixture
20 21
def user():
21 22
    try:
......
26 27
    admin.save()
27 28
    return admin
28 29

  
30

  
29 31
@pytest.fixture
30 32
def user2():
31 33
    try:
......
34 36
        admin2 = User.objects.create_user('admin2', email=None, password='admin2')
35 37
    return admin2
36 38

  
39

  
37 40
def login(username='admin', password='admin'):
38 41
    resp = client.post('/login/', {'username': username, 'password': password})
39 42
    assert resp.status_code == 302
40 43

  
41 44

  
42 45
def test_notification_api(user, user2):
43
    id_notifoo, created = Notification.notify(user, 'notifoo')
46
    notification, created = Notification.notify(user, 'notifoo')
44 47
    assert created
45
    assert Notification.objects.all().count() == 1
46
    noti = Notification.objects.filter_by_id(id_notifoo).filter(user=user).first()
47
    assert noti.pk == int(id_notifoo)
48
    assert noti.summary == 'notifoo'
49
    assert noti.body == ''
50
    assert noti.url == ''
51
    assert noti.external_id is None
52
    assert noti.end_timestamp - noti.start_timestamp == timedelta(3)
53
    assert noti.acked is False
54
    Notification.ack(user, id_notifoo)
55
    noti = Notification.objects.filter_by_id(id_notifoo).filter(user=user).first()
56
    assert noti.acked is True
57

  
58
    Notification.notify(user, 'notirefoo', id=id_notifoo)
59
    assert Notification.objects.all().count() == 1
60
    noti = Notification.objects.filter_by_id(id_notifoo).filter(user=user).first()
61
    assert noti.pk == int(id_notifoo)
62
    assert noti.summary == 'notirefoo'
63

  
64
    Notification.notify(user, 'notirefoo', id=id_notifoo, duration=3600)
65
    noti = Notification.objects.filter_by_id(id_notifoo).filter(user=user).first()
48
    assert Notification.objects.count() == 1
49
    assert notification.summary == 'notifoo'
50
    assert notification.body == ''
51
    assert notification.url == ''
52
    assert notification.origin == ''
53
    assert notification.external_id is None
54
    assert notification.end_timestamp - notification.start_timestamp == timedelta(3)
55
    assert notification.acked is False
56
    Notification.objects.visible(user).ack()
57
    assert Notification.objects.get().acked is True
58

  
59
    Notification.notify(user, 'notirefoo', id=str(notification.pk))
60
    assert Notification.objects.count() == 1
61
    assert Notification.objects.get().summary == 'notirefoo'
62
    # we updated the notification, it's un-acked
63
    assert Notification.objects.get().acked is False
64

  
65
    Notification.notify(user, 'notirefoo', id=str(notification.pk), duration=3600)
66
    noti = Notification.objects.get()
66 67
    assert noti.end_timestamp - noti.start_timestamp == timedelta(seconds=3600)
67 68

  
68 69
    notification, created = Notification.notify(user, 'notibar', id='notibar')
69
    assert created
70
    assert Notification.objects.all().count() == 2
70
    assert Notification.objects.count() == 2
71 71
    notification, created = Notification.notify(user, 'notirebar', id='notibar')
72 72
    assert not created
73
    assert Notification.objects.all().count() == 2
73
    assert Notification.objects.count() == 2
74

  
75
    notification, created = Notification.notify(user2, 'notiother')
76
    notification.forget()
77
    assert Notification.objects.filter(user=user2).count() == 1
78
    notification = Notification.objects.filter(user=user2).get()
79
    assert notification.end_timestamp < now()
80
    assert notification.acked is True
74 81

  
75
    id2, created = Notification.notify(user2, 'notiother')
76
    Notification.forget(user2, id2)
77
    noti = Notification.objects.filter_by_id(id2).filter(user=user2).first()
78
    assert noti.end_timestamp < now()
79
    assert noti.acked is True
80 82

  
81 83
def test_notification_cell(user, user2):
82 84
    page = Page(title='notif', slug='test_notification_cell', template_name='standard')
......
92 94
    assert cell.is_visible(context['request'].user) is True
93 95
    assert cell.get_badge(context) is None
94 96

  
95
    id_noti1, created = Notification.notify(user, 'notibar')
96
    id_noti2, created = Notification.notify(user, 'notifoo')
97
    notification1, created = Notification.notify(user, 'notibar')
98
    notification2, created = Notification.notify(user, 'notifoo')
97 99
    content = cell.render(context)
98 100
    assert 'notibar' in content
99 101
    assert 'notifoo' in content
100 102
    assert cell.get_badge(context) == {'badge': '2'}
101 103

  
102
    Notification.forget(user, id_noti2)
104
    notification2.forget()
103 105
    content = cell.render(context)
104 106
    assert 'notibar' in content
105 107
    assert 'notifoo' not in content
106 108
    assert cell.get_badge(context) == {'badge': '1'}
107 109

  
108
    Notification.notify(user, 'notirebar', id=id_noti1)
110
    Notification.notify(user, 'notirebar', id=str(notification1.pk))
109 111
    content = cell.render(context)
110 112
    assert 'notirebar' in content
111 113
    assert 'notibar' not in content
112 114

  
113
    Notification.notify(user, 'notiurl', id=id_noti1, url='https://www.example.net/')
115
    Notification.notify(user, 'notiurl', id=str(notification1.pk), url='https://www.example.net/')
114 116
    content = cell.render(context)
115 117
    assert 'notiurl' in content
116 118
    assert 'https://www.example.net/' in content
117 119

  
118
    ackme, created = Notification.notify(user, 'ackme')
119
    Notification.ack(user, id=ackme)
120
    notification3, created = Notification.notify(user, 'ackme')
121
    notification3.ack()
120 122
    content = cell.render(context)
121 123
    assert 'acked' in content
122 124
    assert cell.get_badge(context) == {'badge': '1'}
123 125

  
124
    Notification.ack(user, id=id_noti1)
126
    notification1.ack()
125 127
    content = cell.render(context)
126 128
    assert cell.get_badge(context) is None
127 129

  
......
136 138
    assert 'notiother' in content
137 139
    assert cell.get_badge(context) == {'badge': '1'}
138 140

  
141

  
139 142
def test_notification_ws(user):
140 143

  
141 144
    def notify(data, check_id, count):
......
145 148
        result = json.loads(resp.content)
146 149
        assert result == {'data': {'id': check_id, 'created': True}, 'err': 0}
147 150
        assert Notification.objects.filter(user=user).count() == count
148
        return Notification.objects.filter_by_id(check_id).last()
151
        return Notification.objects.find(user, check_id).get()
149 152

  
150 153
    login()
151 154
    notify({'summary': 'foo'}, '1', 1)
......
179 182
    resp = client.get(reverse('api-notification-ack', kwargs={'notification_id': '6'}))
180 183
    assert resp.status_code == 200
181 184
    assert Notification.objects.filter(acked=True).count() == 1
182
    assert Notification.objects.filter(acked=True).first().public_id() == '6'
185
    assert Notification.objects.filter(acked=True).get().public_id == '6'
183 186

  
184 187
    resp = client.get(reverse('api-notification-forget', kwargs={'notification_id': '5'}))
185 188
    assert resp.status_code == 200
186 189
    assert Notification.objects.filter(acked=True).count() == 2
187
    notif = Notification.objects.filter_by_id('5').filter(user=user).first()
188
    assert notif.public_id() == '5'
190
    notif = Notification.objects.find(user, '5').get()
191
    assert notif.public_id == '5'
189 192
    assert notif.acked is True
190 193
    assert notif.end_timestamp < now()
191 194

  
195

  
192 196
def test_notification_ws_badrequest(user):
193 197

  
194 198
    def check_error(data, message):
......
212 216
    check_error({'summary': 'ok', 'duration': 4.01}, 'valid integer is required')
213 217
    check_error({'summary': 'ok', 'duration': -4}, 'greater than')
214 218

  
219

  
215 220
def test_notification_ws_deny():
216 221
    assert client.post(reverse('api-notification-add'),
217 222
                       json.dumps({'summary': 'ok'}),
......
221 226
    assert client.get(reverse('api-notification-forget',
222 227
                              kwargs={'notification_id': '1'})).status_code == 403
223 228

  
229

  
224 230
def test_notification_ws_check_urls():
225 231
    assert reverse('api-notification-add') == '/api/notification/add/'
226 232
    assert reverse('api-notification-ack',
227
-