Review: Approve

Instead of sending an email immediately and for each post, how about adding a 
django command that sends an email if there are hidden posts every night. That 
way even us admins would not be drowned by the SPAM and we could delete 
accordingly.

one suggestion in the code, otherwise code lgtm.

Diff comments:

> 
> === modified file 'pybb/forms.py'
> --- pybb/forms.py     2016-10-03 14:10:48 +0000
> +++ pybb/forms.py     2016-10-24 20:00:09 +0000
> @@ -63,20 +67,48 @@
>              topic_is_new = False
>              topic = self.topic
>  
> +        # Check for spam
> +        # TODO: This is currently a simple keyword search. Maybe add akismet 
> check here
> +        hidden = False
> +        text = self.cleaned_data['body']
> +        if any(x in text.lower() for x in settings.ANTI_SPAM_BODY):
> +            hidden = True
> +
> +        if re.search(INTERN_PHONE_NR, text):
> +            hidden = True
> +        
> +        if topic_is_new:
> +            text = self.cleaned_data['name']
> +            if any(x in text.lower() for x in settings.ANTI_SPAM_TOPIC):
> +                hidden = True
> +            if re.search(INTERN_PHONE_NR, text):
> +                hidden = True
> +
>          post = Post(topic=topic, user=self.user, user_ip=self.ip,
>                      markup=self.cleaned_data['markup'],
> -                    body=self.cleaned_data['body'])
> +                    body=self.cleaned_data['body'], hidden=hidden)
>          post.save(*args, **kwargs)
>  
>          if pybb_settings.ATTACHMENT_ENABLE:
>              self.save_attachment(post, self.cleaned_data['attachment'])
>  
> -        if topic_is_new:
> -            send(User.objects.all(), "forum_new_topic",
> -                {'topic': topic, 'post':post, 'user':topic.user})
> +        if not hidden:
> +            if topic_is_new:
> +                send(User.objects.all(), "forum_new_topic",
> +                    {'topic': topic, 'post':post, 'user':topic.user})
> +            else:
> +                send(self.topic.subscribers.all(), "forum_new_post",
> +                    {'post':post, 'topic':topic, 'user':post.user})
>          else:
> -            send(self.topic.subscribers.all(), "forum_new_post",
> -                {'post':post, 'topic':topic, 'user':post.user})
> +            # Inform admins of a hidden post

pull out into a separate method? the right drift makes this hard to read and it 
drowns out the rest of the logic in thes method.

> +            recipients = [addr[1] for addr in settings.ADMINS]
> +            message = '\n'.join(['Hidden post:',
> +                                 'Topic name: ' + topic.name,
> +                                 'Post body: ' + post.body,
> +                                 'Admin page: http://'+ 
> Site.objects.get_current().domain + '/admin/login/?next=/admin/pybb/post/'+ 
> str(post.id)])
> +            send_mail('A post was hidden by spam check', message, 
> '[email protected]',
> +                      recipients, fail_silently=False)
> +
>          return post
>  
>  


-- 
https://code.launchpad.net/~widelands-dev/widelands-website/anti_spam_3/+merge/309167
Your team Widelands Developers is subscribed to branch lp:widelands-website.

_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp

Reply via email to