Refactor les transactions pour pas raiser dedans

zds/forum/views.py

@@ -202,12 +202,12 @@ class TopicNew(CreateView, SingleObjectMixin):
 
     @method_decorator(login_required)
     @method_decorator(can_write_and_read_now)
-    @method_decorator(transaction.atomic)
     def dispatch(self, request, *args, **kwargs):
-        self.object = self.get_object()
-        if not self.object.can_read(request.user):
-            raise PermissionDenied
-        return super(TopicNew, self).dispatch(request, *args, **kwargs)
+        with transaction.atomic():
+            self.object = self.get_object()
+            if self.object.can_read(request.user):
+                return super(TopicNew, self).dispatch(request, *args, **kwargs)
+        raise PermissionDenied
 
     def get_object(self, queryset=None):
         try:
@@ -456,19 +456,18 @@ class PostNew(CreatePostView):
 
     @method_decorator(login_required)
     @method_decorator(can_write_and_read_now)
-    @method_decorator(transaction.atomic)
     def dispatch(self, request, *args, **kwargs):
-        self.object = self.get_object()
-        if not self.object.forum.can_read(request.user):
-            raise PermissionDenied
-        if self.object.is_locked:
-            raise PermissionDenied
-        if self.object.antispam(request.user):
-            raise PermissionDenied
-        self.posts = Post.objects.filter(topic=self.object) \
-                         .prefetch_related() \
-                         .order_by('-position')[:settings.ZDS_APP['forum']['posts_per_page']]
-        return super(PostNew, self).dispatch(request, *args, **kwargs)
+        with transaction.atomic():
+            self.object = self.get_object()
+            can_read = self.object.forum.can_read(request.user)
+            not_locked = not self.object.is_locked
+            not_spamming = not self.object.antispam(request.user)
+            if can_read and not_locked and not_spamming:
+                self.posts = Post.objects.filter(topic=self.object) \
+                                 .prefetch_related() \
+                                 .order_by('-position')[:settings.ZDS_APP['forum']['posts_per_page']]
+                return super(PostNew, self).dispatch(request, *args, **kwargs)
+        raise PermissionDenied
 
     def create_forum(self, form_class, **kwargs):
         form = form_class(self.object, self.request.user, initial=kwargs)
@@ -499,16 +498,15 @@ class PostEdit(UpdateView, SinglePostObjectMixin, PostEditMixin):
 
     @method_decorator(login_required)
     @method_decorator(can_write_and_read_now)
-    @method_decorator(transaction.atomic)
     def dispatch(self, request, *args, **kwargs):
-        self.object = self.get_object()
-        if not self.object.topic.forum.can_read(request.user):
-            raise PermissionDenied
-        if self.object.author != request.user and not request.user.has_perm('forum.change_post'):
-            raise PermissionDenied
-        if not self.object.is_visible and not request.user.has_perm('forum.change_post'):
-            raise PermissionDenied
-        return super(PostEdit, self).dispatch(request, *args, **kwargs)
+        with transaction.atomic():
+            self.object = self.get_object()
+            is_author = self.object.author == request.user
+            can_read = self.object.topic.forum.can_read(request.user)
+            is_visible = self.object.is_visible
+            if can_read and ((is_author and is_visible) or request.user.has_perm('forum.change_post')):
+                return super(PostEdit, self).dispatch(request, *args, **kwargs)
+        raise PermissionDenied
 
     def get(self, request, *args, **kwargs):
         if self.object.author != request.user and request.user.has_perm('forum.change_post'):
@@ -578,16 +576,15 @@ class PostSignal(UpdateView, SinglePostObjectMixin, PostEditMixin):
 
     @method_decorator(login_required)
     @method_decorator(can_write_and_read_now)
-    @method_decorator(transaction.atomic)
     def dispatch(self, request, *args, **kwargs):
-        self.object = self.get_object()
-
-        if not self.object.topic.forum.can_read(request.user):
-            raise PermissionDenied
-        if not self.object.is_visible and not request.user.has_perm('forum.change_post'):
-            raise PermissionDenied
-
-        return super(PostSignal, self).dispatch(request, *args, **kwargs)
+        with transaction.atomic():
+            self.object = self.get_object()
+            can_read = self.object.topic.forum.can_read(request.user)
+            is_visible = self.object.is_visible
+            can_edit = request.user.has_perm('forum.change_post')
+            if can_read and (is_visible or can_edit):
+                return super(PostSignal, self).dispatch(request, *args, **kwargs)
+        raise PermissionDenied
 
     def post(self, request, *args, **kwargs):
         if 'signal_message' in request.POST:

zds/notification/models.py

@@ -4,15 +4,15 @@
 from django.contrib.auth.models import User
 from django.contrib.contenttypes.fields import GenericForeignKey
 from django.contrib.contenttypes.models import ContentType
-from django.core.exceptions import ObjectDoesNotExist, MultipleObjectsReturned
 from django.core.mail import EmailMultiAlternatives
 from django.db import models, IntegrityError, transaction
 from django.template.loader import render_to_string
 from django.utils.translation import ugettext_lazy as _
-
 from django.conf import settings
-from zds.forum.models import Topic
+from django.core.exceptions import ObjectDoesNotExist
+
 from zds.member.models import Profile
+from zds.forum.models import Topic
 from zds.notification.managers import NotificationManager, SubscriptionManager, TopicFollowedManager, \
     TopicAnswerSubscriptionManager, NewTopicSubscriptionManager
 from zds.utils.misc import convert_camel_to_underscore
@@ -106,7 +106,7 @@ def send_email(self, notification):
         try:
             msg.send()
         except SMTPException:
-            pass
+            LOG.error('Failed sending mail for %s', self, exc_info=True)
 
     @staticmethod
     def has_read_permission(request):
@@ -131,19 +131,20 @@ def send_notification(self, content=None, send_email=True, sender=None):
         assert hasattr(self, 'get_notification_url')
         assert hasattr(self, 'get_notification_title')
         assert hasattr(self, 'send_email')
-        with transaction.atomic():
-            if self.last_notification is None or self.last_notification.is_read:
+
+        if self.last_notification is None or self.last_notification.is_read:
+            with transaction.atomic():
+                notifications = list(Notification.objects.filter(subscription=self))
+                if len(notifications) > 1:
+                    LOG.error('Found %s notifications for %s', len(notifications), self, exc_info=True)
+                    Notification.objects.filter(pk__in=[n.pk for n in notifications[1:]]).delete()
+                    LOG.info('Duplicates deleted.')
+
                 # If there isn't a notification yet or the last one is read, we generate a new one.
                 try:
                     notification = Notification.objects.get(subscription=self)
                 except Notification.DoesNotExist:
                     notification = Notification(subscription=self, content_object=content, sender=sender)
-                except MultipleObjectsReturned:
-                    notifications = list(Notification.objects.filter(subscription=self))
-                    LOG.error('found %s notifications for %s', len(notifications), self, exc_info=True)
-                    Notification.objects.filter(pk__in=[n.pk for n in notifications[1:]]).delete()
-                    LOG.info('removed doubly.')
-                    notification = notifications[0]
                 notification.content_object = content
                 notification.sender = sender
                 notification.url = self.get_notification_url(content)
@@ -156,11 +157,11 @@ def send_notification(self, content=None, send_email=True, sender=None):
 
                 if send_email and self.by_email:
                     self.send_email(notification)
-            elif self.last_notification is not None:
-                # Update last notification if the new content is older (case of unreading answer)
-                if not self.last_notification.is_read and self.last_notification.pubdate > content.pubdate:
-                    self.last_notification.content_object = content
-                    self.last_notification.save()
+        elif self.last_notification is not None and not self.last_notification.is_read:
+            # Update last notification if the new content is older (marking answer as unread)
+            if self.last_notification.pubdate > content.pubdate:
+                self.last_notification.content_object = content
+                self.last_notification.save()
 
     def mark_notification_read(self):
         """
@@ -210,27 +211,28 @@ def mark_notification_read(self, content):
         if content is None:
             raise ValueError('Object content of notification must be defined')
 
-        with transaction.atomic():
-            content_notification_type = ContentType.objects.get_for_model(content)
-            notifications = list(Notification.objects.filter(subscription=self,
-                                                             content_type__pk=content_notification_type.pk,
-                                                             object_id=content.pk, is_read=False))
-            # handles cases where a same subscription lead to several notifications
-            if not notifications:
-                LOG.debug('nothing to mark as read')
-                return
-            elif len(notifications) > 1:
-                LOG.warning('%s notifications were find for %s/%s', len(notifications), content.type, content.title)
-                for notif in notifications[1:]:
-                    notif.delete()
-
-            notification = notifications[0]
-            notification.subscription = self
-            notification.is_read = True
-            try:
+        try:
+            with transaction.atomic():
+                content_notification_type = ContentType.objects.get_for_model(content)
+                notifications = list(Notification.objects.filter(subscription=self,
+                                                                 content_type__pk=content_notification_type.pk,
+                                                                 object_id=content.pk, is_read=False))
+                # handles cases where a same subscription lead to several notifications
+                if not notifications:
+                    LOG.debug('nothing to mark as read')
+                    return
+                elif len(notifications) > 1:
+                    LOG.warning('%s notifications found for %s/%s', len(notifications), content.type, content.title)
+                    for notif in notifications[1:]:
+                        notif.delete()
+
+                notification = notifications[0]
+                notification.subscription = self
+                notification.is_read = True
+
                 notification.save()
-            except IntegrityError:
-                LOG.exception('Could not save %s', notification)
+        except IntegrityError:
+            LOG.exception('Could not save %s', notification)
 
 
 class AnswerSubscription(Subscription):

zds/tutorialv2/api/tests.py

@@ -138,6 +138,7 @@ def test_get_content_reaction_voters(self):
         equal_reaction.like += 1
         equal_reaction.dislike += 1
         equal_reaction.save()
+
         CommentVote.objects.create(user=profile.user, comment=equal_reaction, positive=True)
         CommentVote.objects.create(user=profile2.user, comment=equal_reaction, positive=False)
 

zds/tutorialv2/views/published.py

@@ -1002,7 +1002,6 @@ def perform_follow(user_to_follow, user):
     def perform_follow_by_email(user_to_follow, user):
         return NewPublicationSubscription.objects.toggle_follow(user_to_follow, user, True).is_active
 
-    @method_decorator(transaction.atomic)
     def post(self, request, *args, **kwargs):
         response = {}
 
@@ -1016,11 +1015,13 @@ def post(self, request, *args, **kwargs):
         if user_to_follow == request.user:
             raise PermissionDenied
 
-        if 'follow' in request.POST:
-            response['follow'] = self.perform_follow(user_to_follow, request.user)
-            response['subscriberCount'] = NewPublicationSubscription.objects.get_subscriptions(user_to_follow).count()
-        elif 'email' in request.POST:
-            response['email'] = self.perform_follow_by_email(user_to_follow, request.user)
+        with transaction.atomic():
+            if 'follow' in request.POST:
+                response['follow'] = self.perform_follow(user_to_follow, request.user)
+                response['subscriberCount'] = NewPublicationSubscription.objects \
+                    .get_subscriptions(user_to_follow).count()
+            elif 'email' in request.POST:
+                response['email'] = self.perform_follow_by_email(user_to_follow, request.user)
 
         if request.is_ajax():
             return HttpResponse(json_handler.dumps(response), content_type='application/json')

zds/tutorialv2/views/validations.py

@@ -827,16 +827,15 @@ def get_form_kwargs(self):
         return kwargs
 
     def form_valid(self, form):
-
         db_object = self.object
         versioned = self.versioned_object
         self.success_url = versioned.get_absolute_url_online()
 
         if not db_object.sha_picked:
-            raise PermissionDenied("Retirer des billets choisis quelque chose qui n'y est pas")
+            raise PermissionDenied('Impossible de retirer des billets choisis un billet pas choisi.')
 
         if db_object.sha_picked != form.cleaned_data['version']:
-            raise PermissionDenied("Retirer des billets choisis quelque chose qui n'y est pas")
+            raise PermissionDenied('Impossible de retirer des billets choisis un billet pas choisi.')
 
         db_object.sha_picked = None
         db_object.save()