From ff2ee47cbe10c5c41ff5d097675f78e484d0d4d7 Mon Sep 17 00:00:00 2001 From: IssaIan Date: Tue, 14 May 2019 15:52:23 +0300 Subject: [PATCH] bg(Fix notifications):Fix notifications - User can get notifications existing before unsubscribing from in app notifications [finishes #165952876] --- .../migrations/0001_initial.py | 10 +- .../migrations/0002_usernotification_user.py | 23 ---- authors/apps/appnotifications/models.py | 8 +- authors/apps/appnotifications/serializers.py | 4 +- .../tests/test_notifications.py | 12 -- authors/apps/appnotifications/views.py | 8 +- authors/utils/notification_handlers.py | 123 +++++++++--------- 7 files changed, 72 insertions(+), 116 deletions(-) delete mode 100644 authors/apps/appnotifications/migrations/0002_usernotification_user.py diff --git a/authors/apps/appnotifications/migrations/0001_initial.py b/authors/apps/appnotifications/migrations/0001_initial.py index acd0155..bff3032 100644 --- a/authors/apps/appnotifications/migrations/0001_initial.py +++ b/authors/apps/appnotifications/migrations/0001_initial.py @@ -1,6 +1,8 @@ -# Generated by Django 2.2 on 2019-05-10 10:13 +# Generated by Django 2.2 on 2019-05-14 13:13 +from django.conf import settings from django.db import migrations, models +import django.db.models.deletion class Migration(migrations.Migration): @@ -8,6 +10,7 @@ class Migration(migrations.Migration): initial = True dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), ] operations = [ @@ -15,8 +18,9 @@ class Migration(migrations.Migration): name='UserNotification', fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('email_notifications_subscription', models.BooleanField(default=True)), - ('in_app_notifications_subscription', models.BooleanField(default=True)), + ('email_notifications', models.BooleanField(default=True)), + ('in_app_notifications', models.BooleanField(default=True)), + ('user', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='notification_preferences', to=settings.AUTH_USER_MODEL)), ], ), ] diff --git a/authors/apps/appnotifications/migrations/0002_usernotification_user.py b/authors/apps/appnotifications/migrations/0002_usernotification_user.py deleted file mode 100644 index 13696d3..0000000 --- a/authors/apps/appnotifications/migrations/0002_usernotification_user.py +++ /dev/null @@ -1,23 +0,0 @@ -# Generated by Django 2.2 on 2019-05-10 10:13 - -from django.conf import settings -from django.db import migrations, models -import django.db.models.deletion - - -class Migration(migrations.Migration): - - initial = True - - dependencies = [ - migrations.swappable_dependency(settings.AUTH_USER_MODEL), - ('appnotifications', '0001_initial'), - ] - - operations = [ - migrations.AddField( - model_name='usernotification', - name='user', - field=models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='notification_preferences', to=settings.AUTH_USER_MODEL), - ), - ] diff --git a/authors/apps/appnotifications/models.py b/authors/apps/appnotifications/models.py index 5bf25ba..2da8f56 100644 --- a/authors/apps/appnotifications/models.py +++ b/authors/apps/appnotifications/models.py @@ -16,8 +16,8 @@ class UserNotification(models.Model): user = models.OneToOneField(User, on_delete=models.CASCADE, unique=True, related_name='notification_preferences') - email_notifications_subscription = models.BooleanField(default=True) - in_app_notifications_subscription = models.BooleanField(default=True) + email_notifications = models.BooleanField(default=True) + in_app_notifications = models.BooleanField(default=True) @receiver(post_save, sender=User) @@ -28,7 +28,7 @@ def setup_notification_permissions(sender, **kwargs): if created: data = { 'user': instance, - 'email_notifications_subscription': True, - 'in_app_notifications_subscription': True + 'email_notifications': True, + 'in_app_notifications': True } UserNotification.objects.create(**data) diff --git a/authors/apps/appnotifications/serializers.py b/authors/apps/appnotifications/serializers.py index 64e1f40..4371041 100644 --- a/authors/apps/appnotifications/serializers.py +++ b/authors/apps/appnotifications/serializers.py @@ -12,8 +12,8 @@ class Subscription(serializers.ModelSerializer): """ class Meta: model = UserNotification - fields = ('email_notifications_subscription', - 'in_app_notifications_subscription') + fields = ('email_notifications', + 'in_app_notifications') class NotificationSerializer(serializers.ModelSerializer): diff --git a/authors/apps/appnotifications/tests/test_notifications.py b/authors/apps/appnotifications/tests/test_notifications.py index 47db605..7f5c27e 100644 --- a/authors/apps/appnotifications/tests/test_notifications.py +++ b/authors/apps/appnotifications/tests/test_notifications.py @@ -72,18 +72,6 @@ def test_notification_from_comments(self): response = self.client.get(self.notification_url) self.assertEqual(response.status_code, status.HTTP_200_OK) - def test_get_notifications_while_unsubscribed(self): - """ - Test getting notifications while user has unsubscribed - """ - self.is_authenticated("jim@gmail.com", "@Us3r.com") - self.client.patch(self.subscribe_unsubscribe_url, { - 'email_notifications_subscription': 'false', - 'in_app_notifications_subscription': 'false'}) - response = self.client.get(self.notification_url) - self.assertEqual(response.data['message'], - 'You are not subscribed to in app notifications') - def test_delete_notifications(self): """ Tests successful deletion of notifications diff --git a/authors/apps/appnotifications/views.py b/authors/apps/appnotifications/views.py index 02477d9..8d7b290 100644 --- a/authors/apps/appnotifications/views.py +++ b/authors/apps/appnotifications/views.py @@ -23,7 +23,6 @@ class SubscribeUnsubscribeAPIView(RetrieveUpdateAPIView): def update(self, request, *args, **kwargs): user = UserNotification.objects.get(user=request.user) - request.user.notifications.mark_all_as_deleted() serializer_data = request.data serializer = self.serializer_class( instance=user, data=serializer_data, partial=True) @@ -60,12 +59,7 @@ def get(self, request, *args, **kwargs): serializer = self.serializer_class( notifications, many=True ) - user = request.user.notification_preferences - if user.in_app_notifications_subscription is False: - resp = { - "message": "You are not subscribed to in app notifications" - } - elif notifications.count() == 0: + if notifications.count() == 0: resp = { "message": "You have no new notifications" } diff --git a/authors/utils/notification_handlers.py b/authors/utils/notification_handlers.py index 1088342..27ff6d3 100644 --- a/authors/utils/notification_handlers.py +++ b/authors/utils/notification_handlers.py @@ -22,93 +22,86 @@ def create_article_handler(sender, instance, created, **kwargs): title = instance.title article_author = instance.author.profile followers = article_author.followers_list() + description = "{} posted an article '{}' on {}".format( + article_author, + title.upper(), + instance.created_at.strftime('%d-%B-%Y %H:%M')) if not followers: return for user in followers: - - url = reverse( - "articles:articles", args=[instance.slug]) - url = f"{settings.DOMAIN}{url}" - notify.send( - article_author, - recipient=user.user, - description="{} posted an article '{}' on {}".format( + if user.user.notification_preferences.in_app_notifications: + url = reverse( + "articles:articles", args=[instance.slug]) + url = f"{settings.DOMAIN}{url}" + notify.send( article_author, - title.upper(), - instance.created_at.strftime('%d-%B-%Y %H:%M')), - verb=verbs.ARTICLE_CREATION, - action_object=instance, - resource_url=url - ) + recipient=user.user, + description=description, + verb=verbs.ARTICLE_CREATION, + action_object=instance, + resource_url=url + ) + if user.user.notification_preferences.email_notifications: + email_notification_handler(user, description) -def create_email_notification_handler(sender, instance, created, **kwargs): +def comment_handler(sender, instance, created, **kwargs): """ - notification handler for emails. + notification handler for comments """ - user = instance.recipient - recipient = User.objects.get(email=user) - send = recipient.notification_preferences.email_notifications_subscription - if send is False: - return - description = instance.description - instance.emailed = True - token, created = Token.objects.get_or_create(user=user) + recipients = [] + comment_author = instance.author + article = instance.article + favourited = Favorite.objects.filter(article=article) + description = "{} posted a comment to {} on {}".format( + comment_author.username, + article.title, + instance.created_at.strftime('%d-%B-%Y %H:%M')) + url = reverse( + "articles:articles", args=[article.slug]) + resource_url = f"{settings.DOMAIN}{url}" + for user in favourited: + email = user.user + recipients.append(email) + + if email.notification_preferences.in_app_notifications: + notify.send(comment_author, + recipient=recipients, + description=description, + target=article or instance, + verb=verbs.COMMENT_CREATED, + action_object=instance, + resource_url=resource_url + ) + if email.notification_preferences.email_notifications: + email_notification_handler(user, description) + + +def email_notification_handler(user, description): + token, created = Token.objects.get_or_create(user=user.user) if not created: token.created = timezone.now() token.save() url = reverse( "notifications:opt_out_link", args=[token]) opt_out_link = f'{settings.DOMAIN}{url}' - try: - resource_url = instance.data['resource_url'] - except TypeError: - resource_url = f"{settings.DOMAIN}/api/articles" + resource_url = url = f"{settings.DOMAIN}{url}" - html_content = render_to_string('notification_template.html', context={ - "opt_out_link": opt_out_link, - "username": recipient.username, - "description": description, - "resource_url": resource_url - }) + html_content = render_to_string( + 'notification_template.html', context={ + "opt_out_link": opt_out_link, + "username": user.user.username, + "description": description, + "resource_url": resource_url + }) send_mail( "User Notification", '', settings.EMAIL_HOST_USER, - [recipient.email], + [user.user.email], html_message=html_content) -def comment_handler(sender, instance, created, **kwargs): - """ - notification handler for comments - """ - recipients = [] - comment_author = instance.author - article = instance.article - favourited = Favorite.objects.filter(article=article) - for user in favourited: - email = user.user - recipients.append(email) - description_string = "{} posted a comment to {} on {}" - url = reverse( - "articles:articles", args=[article.slug]) - resource_url = f"{settings.DOMAIN}{url}" - - notify.send(comment_author, - recipient=recipients, - description=description_string.format( - comment_author.username, - article.title, - instance.created_at.strftime('%d-%B-%Y %H:%M')), - target=article or instance, - verb=verbs.COMMENT_CREATED, - action_object=instance, - resource_url=resource_url - ) - - -post_save.connect(create_email_notification_handler, sender=Notification) post_save.connect(create_article_handler, sender=Article) post_save.connect(comment_handler, sender=Comment)