Merge lp:~widelands-dev/widelands-website/better_notification_mail into lp:widelands-website

[kaputtnik]

Fixes bug 1736263

Do not send emails to the one who

- creates a new topic
- replied to a topic
- uploaded a new map

Reworked the content of send emails. For the wiki it contains now the user who edited an article and a link to the last changeset, which shows the changes, e.g. https://wl.widelands.org/wiki/history/Main%20Page/changeset/94/. This will fix bug 340920

Removed filter 'custom_date' from wiki feeds, to get a normal date format.

This should may be tested on the alpha site.

[lp:~widelands-dev/widelands-website/better_notification_mail]

493. By kaputtnik on 2017-12-12

small css fix for the documentation

[kaputtnik (franku)]

I found a bug in this code which needs fixing.

review: Needs Fixing

[lp:~widelands-dev/widelands-website/better_notification_mail]

494. By kaputtnik on 2018-01-04

fixed a failure in wlmaps; removed django-messages related templates because those are self handled by django-messages

[kaputtnik (franku)]

I had forgotten to update the packages of the virtual environment the last time. Did this now. The content of email send by django-messages are defined by django-messages itself, so the old templates (templates/notification/messages_*.txt) are obsolete and i have removed them in this branch.

django messages uses this template:

http://bazaar.launchpad.net/~widelands-dev/widelands-website/trunk/view/head:/templates/django_messages/new_message.html

review: Needs Resubmitting

[GunChleoc (gunchleoc)]

Code LGTM as far as I can tell.

I might be available sometime next week to do some testing.

[lp:~widelands-dev/widelands-website/better_notification_mail]

495. By kaputtnik on 2018-01-14

reverted latest changes

496. By kaputtnik on 2018-01-14

fixed a failure in wlmaps

497. By kaputtnik on 2018-01-14

merged with trunk

[kaputtnik (franku)]

I've made a failure recently by updating django-messages to a wrong version.

https://wl.widelands.org/forum/topic/4196/

I fixed it in trunk and also in this branch, which means we need to keep the templates in

templates/notification/messages_*/full_text

[SirVer (sirver)]

lgtm. not tested though.

review: Approve

[kaputtnik (franku)]

Some parts aren't fine. E.g. the email subject when replying to a pm.

review: Needs Fixing

[lp:~widelands-dev/widelands-website/better_notification_mail]

498. By kaputtnik on 2018-02-04

unified email messsages

[kaputtnik (franku)]

alpha site is running with database django_18 now, so only the 8 registered people in this database may get emails and not the people from the old alpha database.

review: Needs Resubmitting

[lp:~widelands-dev/widelands-website/better_notification_mail]

499. By kaputtnik on 2018-02-04

reduced upper space for email of observed article changed

[kaputtnik (franku)]

Ooops, sorry for spamming you with emails. I just forgot that ./manage.py emit_notices must be called by hand on the alpha site and wrote post after post, wondering that no emails get send :-S

[lp:~widelands-dev/widelands-website/better_notification_mail]

500. By kaputtnik on 2018-02-05

remove leading newlines from email body

[GunChleoc (gunchleoc)]

I have added a few posts to the alpha site and uploaded a map.

[GunChleoc (gunchleoc)]

PM to myself just says "by GunChleoc" in the subject. It should read "X sent you a message on Widelands" or something.

[lp:~widelands-dev/widelands-website/better_notification_mail]

501. By kaputtnik on 2018-02-15

Added files to overwrite default short.txt

[kaputtnik (franku)]

Thanks for testing :-)

The subject on private messages should work now. I forgot that django_messages ships his own short.txt (used for the subject).

I send you a PM reply and a new PM. If this is ok it can go in, imho.

Shut off the alpha site for now.

[GunChleoc (gunchleoc)]

There's a typo in the subject now: was send -> was sent

I got the 2 notifications for the messages.

[lp:~widelands-dev/widelands-website/better_notification_mail]

502. By kaputtnik on 2018-02-15

fixed a typo

503. By kaputtnik on 2018-02-20

merged with trunk

[kaputtnik (franku)]

Merged and deployed

=== modified file 'media/css/documentation.css'
--- media/css/documentation.css	2016-10-18 09:43:58 +0000
+++ media/css/documentation.css	2018-02-22 06:34:38 +0000
@@ -1,3 +1,7 @@
+table.field-list{
+	border-collapse: separate;
+}
+
 .sphinx a.headerlink {
 	padding: 0 4px 0 4px;
 	visibility: hidden;

=== modified file 'notification/models.py'
--- notification/models.py	2017-05-07 09:51:15 +0000
+++ notification/models.py	2018-02-22 06:34:38 +0000
@@ -40,7 +40,9 @@
 class NoticeType(models.Model):
 
     label = models.CharField(_('label'), max_length=40)
-    display = models.CharField(_('display'), max_length=50)
+    display = models.CharField(_('display'),
+                               max_length=50,
+                               help_text=_('Used as subject when sending emails.'))
     description = models.CharField(_('description'), max_length=100)
 
     # by default only on for media with sensitivity less than or equal to this
@@ -101,14 +103,22 @@
         setting.save()
         return setting
 
+
 def should_send(user, notice_type, medium):
     return get_notification_setting(user, notice_type, medium).send
 
-def get_observers_for(notice_type):
-    """ Returns the list of users which wants to get a message (email) for this
+
+def get_observers_for(notice_type, excl_user=None):
+    """Returns the list of users which wants to get a message (email) for this
     type of notice."""
-    settings = NoticeSetting.objects.filter(notice_type__label=notice_type).filter(send=True)
-    return [s.user for s in NoticeSetting.objects.filter(notice_type__label=notice_type).filter(send=True)]
+    query = NoticeSetting.objects.filter(
+            notice_type__label=notice_type, send=True)
+
+    if excl_user:
+        query = query.exclude(user=excl_user)
+
+    return [notice_setting.user for notice_setting in query]
+
 
 class NoticeQueueBatch(models.Model):
     """A queued notice.
@@ -244,6 +254,8 @@
                 'user': user,
                 'notices_url': notices_url,
                 'current_site': current_site,
+                'subject': notice_type.display,
+                'description': notice_type.description,
             })
             context.update(extra_context)
 
@@ -254,10 +266,14 @@
             subject = ''.join(render_to_string('notification/email_subject.txt', {
                 'message': messages['short.txt'],
             }, context).splitlines())
-
+            
+            # Strip leading newlines. Make writing the email templates easier:
+            # Each linebreak in the templates results in a linebreak in the emails
+            # If the first line in a template contains only template tags the 
+            # email will contain an empty line at the top.
             body = render_to_string('notification/email_body.txt', {
                 'message': messages['full.txt'],
-            }, context)
+            }, context).lstrip()
 
             if should_send(user, notice_type, '1') and user.email:  # Email
                 recipients.append(user.email)

=== modified file 'pybb/views.py'
--- pybb/views.py	2017-11-26 10:57:35 +0000
+++ pybb/views.py	2018-02-22 06:34:38 +0000
@@ -178,14 +178,20 @@
         if notification:
             if not topic:
                 # Inform subscribers of a new topic
-                notification.send(notification.get_observers_for('forum_new_topic'), 'forum_new_topic',
-                                  {'topic': post.topic, 'post': post, 'user': post.topic.user}, queue = True)
+                subscribers = notification.get_observers_for('forum_new_topic',
+                                                             excl_user=request.user)
+                notification.send(subscribers, 'forum_new_topic',
+                                  {'topic': post.topic,
+                                   'post': post,
+                                   'user': post.topic.user
+                                   },
+                                  queue = True)
                 # Topics author is subscriber for all new posts in his topic
                 post.topic.subscribers.add(request.user)
 
             else:
                 # Send mails about a new post to topic subscribers
-                notification.send(post.topic.subscribers.all(), 'forum_new_post',
+                notification.send(post.topic.subscribers.exclude(username=post.user), 'forum_new_post',
                                   {'post': post, 'topic': topic, 'user': post.user}, queue = True)
 
         return HttpResponseRedirect(post.get_absolute_url())

=== modified file 'templates/notification/email_body.txt'
--- templates/notification/email_body.txt	2017-04-17 14:34:48 +0000
+++ templates/notification/email_body.txt	2018-02-22 06:34:38 +0000
@@ -1,5 +1,4 @@
-{% load i18n %}{% blocktrans %}You have received the following notice from {{ current_site }}:
-
+{% load i18n %}{% blocktrans %}
 {{ message }}
 To change how you receive notifications, please go to {{ notices_url }}.
 {% endblocktrans %}

=== modified file 'templates/notification/email_subject.txt'
--- templates/notification/email_subject.txt	2009-02-20 16:46:21 +0000
+++ templates/notification/email_subject.txt	2018-02-22 06:34:38 +0000
@@ -1,1 +1,6 @@
-{% load i18n %}{% blocktrans %}[Widelands.org] {{ message }}{% endblocktrans %}
+{% load i18n %}
+{% if message|length > 1 %}
+{% blocktrans %}{{ message }}{% endblocktrans %}
+{% else %}
+{% blocktrans %}{{ subject }}{% endblocktrans %}
+{% endif %}

=== modified file 'templates/notification/forum_new_post/full.txt'
--- templates/notification/forum_new_post/full.txt	2015-03-16 07:00:43 +0000
+++ templates/notification/forum_new_post/full.txt	2018-02-22 06:34:38 +0000
@@ -2,9 +2,9 @@
 
 "{{ user }}" wrote:
 
-{{ post.body_text }}
+{{ post.body }}
 {% blocktrans with post.get_absolute_url as post_url and topic.get_absolute_url as topic_url %} 
-–––––––––––
+-------------------------
 Link to post: http://{{ current_site }}{{ post_url }}
 Link to topic: http://{{ current_site }}{{ topic_url }}
 {% endblocktrans %}
\ No newline at end of file

=== modified file 'templates/notification/forum_new_topic/full.txt'
--- templates/notification/forum_new_topic/full.txt	2010-06-10 12:13:43 +0000
+++ templates/notification/forum_new_topic/full.txt	2018-02-22 06:34:38 +0000
@@ -1,7 +1,9 @@
-{% load i18n %}{% blocktrans with topic.get_absolute_url as topic_url %}The Forum topic {{ topic }} has been created by {{ user }}.
-
-http://{{ current_site }}{{ topic_url }}
+{% load i18n %}{% blocktrans with topic.get_absolute_url as topic_url and post.body as txt %}The Forum topic "{{ topic }}" has been created by {{ user }}.
+
+{{ user }} wrote:
+
+{{ txt }}
 {% endblocktrans %}
-{{ topic }}:
 
-{{ post.body_text }}
+-------------------------
+Link to topic: http://{{ current_site }}{{ topic_url }}
\ No newline at end of file

=== modified file 'templates/notification/maps_new_map/full.txt'
--- templates/notification/maps_new_map/full.txt	2017-04-23 21:00:01 +0000
+++ templates/notification/maps_new_map/full.txt	2018-02-22 06:34:38 +0000
@@ -1,7 +1,8 @@
-{% load i18n %}A new map was uploaded to the Website by {{ user }}:
+{% load i18n %}A new map has been uploaded to the Website by {{ user }}:
 {% blocktrans %}
 Mapname: {{ mapname }}
 Description: {{ uploader_comment }}
-–––––––––––
+
+-------------------------
 Link to map: http://{{ current_site }}{{ url }}
-{% endblocktrans %}
\ No newline at end of file
+{% endblocktrans %}

=== modified file 'templates/notification/messages_received/full.txt'
--- templates/notification/messages_received/full.txt	2009-03-17 12:57:21 +0000
+++ templates/notification/messages_received/full.txt	2018-02-22 06:34:38 +0000
@@ -4,7 +4,6 @@
 
 {{ message_body }}
 
-http://{{ current_site }}{{ message_url }}
-
-reply directly: 
-http://{{ current_site }}/messages/reply/{{ message_id }}/{% endblocktrans %}
+-------------------------
+Link to Message: http://{{ current_site }}{{ message_url }}
+Reply directly: http://{{ current_site }}/messages/reply/{{ message_id }}/{% endblocktrans %}

=== added file 'templates/notification/messages_received/short.txt'
--- templates/notification/messages_received/short.txt	1970-01-01 00:00:00 +0000
+++ templates/notification/messages_received/short.txt	2018-02-22 06:34:38 +0000
@@ -0,0 +1,1 @@
+{% load i18n %}{% blocktrans with message.sender as message_sender %}User {{ message_sender }} has sent you a message{% endblocktrans %}
\ No newline at end of file

=== removed file 'templates/notification/messages_received/short.txt'
--- templates/notification/messages_received/short.txt	2009-02-26 11:32:18 +0000
+++ templates/notification/messages_received/short.txt	1970-01-01 00:00:00 +0000
@@ -1,1 +0,0 @@
-{% load i18n %}{% blocktrans with message.sender as message_sender %}{{ notice }} by {{ message_sender }}{% endblocktrans %}
\ No newline at end of file

=== modified file 'templates/notification/messages_reply_received/full.txt'
--- templates/notification/messages_reply_received/full.txt	2009-03-17 12:57:21 +0000
+++ templates/notification/messages_reply_received/full.txt	2018-02-22 06:34:38 +0000
@@ -4,7 +4,7 @@
 
 {{ message_body }}
 
-http://{{ current_site }}{{ message_url }}
-
-reply directly: 
-http://{{ current_site }}/messages/reply/{{ message_id }}/{% endblocktrans %}
+-------------------------
+Link to message: http://{{ current_site }}{{ message_url }}
+Reply directly: http://{{ current_site }}/messages/reply/{{ message_id }}/
+{% endblocktrans %}

=== added file 'templates/notification/messages_reply_received/short.txt'
--- templates/notification/messages_reply_received/short.txt	1970-01-01 00:00:00 +0000
+++ templates/notification/messages_reply_received/short.txt	2018-02-22 06:34:38 +0000
@@ -0,0 +1,1 @@
+{% load i18n %}{% blocktrans with message.sender as message_sender %}{{ message_sender }} has replied to a message{% endblocktrans %}
\ No newline at end of file

=== removed file 'templates/notification/messages_reply_received/short.txt'
--- templates/notification/messages_reply_received/short.txt	2009-02-26 11:32:18 +0000
+++ templates/notification/messages_reply_received/short.txt	1970-01-01 00:00:00 +0000
@@ -1,1 +0,0 @@
-{% load i18n %}{% blocktrans with message.sender as message_sender %}{{ notice }} by {{ message_sender }}{% endblocktrans %}
\ No newline at end of file

=== modified file 'templates/notification/short.txt'
--- templates/notification/short.txt	2009-02-20 16:46:21 +0000
+++ templates/notification/short.txt	2018-02-22 06:34:38 +0000
@@ -1,1 +1,1 @@
-{% load i18n %}{% blocktrans %}{{ notice }}{% endblocktrans %}
\ No newline at end of file
+{% load i18n %}{% blocktrans %}{{ notice }}{% endblocktrans %}

=== removed directory 'templates/notification/wiki_article_edited'
=== removed file 'templates/notification/wiki_article_edited/full.txt'
--- templates/notification/wiki_article_edited/full.txt	2009-02-20 10:11:49 +0000
+++ templates/notification/wiki_article_edited/full.txt	1970-01-01 00:00:00 +0000
@@ -1,4 +0,0 @@
-{% load i18n %}{% blocktrans with article.get_absolute_url as article_url %}The wiki article {{ article }} has been edited by {{ user }}.
-
-http://{{ current_site }}{{ article_url }}
-{% endblocktrans %}

=== modified file 'templates/notification/wiki_observed_article_changed/full.txt'
--- templates/notification/wiki_observed_article_changed/full.txt	2009-02-20 10:11:49 +0000
+++ templates/notification/wiki_observed_article_changed/full.txt	2018-02-22 06:34:38 +0000
@@ -1,4 +1,8 @@
-{% load i18n %}{% blocktrans with observed.get_absolute_url as article_url %}The article {{ observed }} that you observe has changed.
+{% load i18n %}{% url 'wiki_changeset' article rev as diff_url %}{% url 'wiki_article' article as article_url %}{% blocktrans %}
+The article "{{ article }}" that you observe has been edited by {{ editor }}.
+Comment for this revision: "{{ rev_comment }}"
 
-http://{{ current_site }}{{ article_url }}
+-------------------------
+A diff is available at: http://{{ current_site }}{{ diff_url }}
+Link to article: http://{{ current_site }}{{ article_url }}
 {% endblocktrans %}
\ No newline at end of file

=== modified file 'templates/notification/wiki_revision_reverted/full.txt'
--- templates/notification/wiki_revision_reverted/full.txt	2009-02-20 10:11:49 +0000
+++ templates/notification/wiki_revision_reverted/full.txt	2018-02-22 06:34:38 +0000
@@ -1,5 +1,6 @@
 {% load i18n %}
-{% blocktrans with article.get_absolute_url as article_url %}Your revision {{ revision }} on {{ article }} has been reverted.
+{% blocktrans with article.get_absolute_url as article_url %}Your revision {{ revision }} on "{{ article }}" has been reverted.
 
-http://{{ current_site }}{{ article_url }}
-{% endblocktrans %}
\ No newline at end of file
+--------------------------------------
+Link to article: http://{{ current_site }}{{ article_url }}
+{% endblocktrans %}

=== modified file 'templates/wiki/feeds/history_description.html'
--- templates/wiki/feeds/history_description.html	2016-04-30 09:43:53 +0000
+++ templates/wiki/feeds/history_description.html	2018-02-22 06:34:38 +0000
@@ -1,4 +1,4 @@
-{% load i18n custom_date %}
+{% load i18n %}
 
-{% trans "edited by user" %} {{ obj.editor.username }} {% trans "at"%} {{ obj.modified|custom_date:obj.editor }}<br>
+{% trans "edited by user" %} {{ obj.editor.username }} {% trans "at"%} {{ obj.modified }}<br>
 {{ obj.comment }}

=== modified file 'templates/wiki/feeds/history_title.html'
--- templates/wiki/feeds/history_title.html	2016-04-30 09:43:53 +0000
+++ templates/wiki/feeds/history_title.html	2018-02-22 06:34:38 +0000
@@ -1,3 +1,2 @@
-{%load custom_date %}
 
-{{ obj.article.title }} - {{ obj.modified|custom_date:obj.editor }}
+{{ obj.article.title }} - {{ obj.modified }}

=== modified file 'wiki/models.py'
--- wiki/models.py	2017-12-01 14:06:15 +0000
+++ wiki/models.py	2018-02-22 06:34:38 +0000
@@ -133,13 +133,6 @@
         return self.filter(revision__gt=int(revision))
 
 
-class NonRevertedChangeSetManager(ChangeSetManager):
-
-    def get_default_queryset(self):
-        super(PublishedBookManager, self).get_queryset().filter(
-            reverted=False)
-
-
 class ChangeSet(models.Model):
     """A report of an older version of some Article."""
 
@@ -165,7 +158,6 @@
     reverted = models.BooleanField(_(u"Reverted Revision"), default=False)
 
     objects = ChangeSetManager()
-    non_reverted_objects = NonRevertedChangeSetManager()
 
     class Meta:
         verbose_name = _(u'Change set')
@@ -177,16 +169,17 @@
     def __unicode__(self):
         return u'#%s' % self.revision
 
-    @models.permalink
     def get_absolute_url(self):
         if self.article.group is None:
-            return ('wiki_changeset', (),
-                    {'title': self.article.title,
-                     'revision': self.revision})
-        return ('wiki_changeset', (),
-                {'group_slug': self.article.group.slug,
-                 'title': self.article.title,
-                 'revision': self.revision})
+            return reverse('wiki_changeset', kwargs={
+                'title': self.article.title,
+                'revision': self.revision
+            })
+        return reverse('wiki_changeset', kwargs={
+            'group_slug': self.article.group.slug,
+            'title': self.article.title,
+            'revision': self.revision,
+        })
 
     def is_anonymous_change(self):
         return self.editor is None
@@ -240,31 +233,9 @@
                     article=self.article).latest().revision + 1
             except self.DoesNotExist:
                 self.revision = 1
+
         super(ChangeSet, self).save(*args, **kwargs)
 
-    def display_diff(self):
-        """Returns a HTML representation of the diff."""
-
-        # well, it *will* be the old content
-        old_content = self.article.content
-
-        # newer non-reverted revisions of this article, starting from this
-        newer_changesets = ChangeSet.non_reverted_objects.filter(
-            article=self.article,
-            revision__gte=self.revision)
-
-        # apply all patches to get the content of this revision
-        for i, changeset in enumerate(newer_changesets):
-            patches = dmp.patch_fromText(changeset.content_diff)
-            if len(newer_changesets) == i + 1:
-                # we need to compare with the next revision after the change
-                next_rev_content = old_content
-            old_content = dmp.patch_apply(patches, old_content)[0]
-
-        diffs = dmp.diff_main(old_content, next_rev_content)
-        dmp.diff_cleanupSemantic(diffs)
-        return dmp.diff_prettyHtml(diffs)
-
     def get_content(self):
         """Returns the content of this revision."""
         content = self.article.content
@@ -283,6 +254,3 @@
         diffs = dmp.diff_main(other_content, self.get_content())
         dmp.diff_cleanupSemantic(diffs)
         return dmp.diff_prettyHtml(diffs)
-
-if notification is not None:
-    signals.post_save.connect(notification.handle_observations, sender=Article)

=== modified file 'wiki/views.py'
--- wiki/views.py	2017-12-03 11:43:47 +0000
+++ wiki/views.py	2018-02-22 06:34:38 +0000
@@ -286,6 +286,18 @@
                 form.group = group
 
             new_article, changeset = form.save()
+            
+            if notification and not changeset.reverted:
+                # Get observers for this article and exclude current editor
+                items = notification.ObservedItem.objects.all_for(
+                    new_article, 'post_save').exclude(user=request.user).iterator()
+                users = [o.user for o in items]
+                notification.send(users, 'wiki_observed_article_changed',
+                                  {'editor': request.user,
+                                   'rev': changeset.revision,
+                                   'rev_comment': changeset.comment,
+                                   'article': new_article})
+
 
             return redirect(new_article)
 

=== modified file 'wlmaps/models.py'
--- wlmaps/models.py	2017-09-12 18:04:34 +0000
+++ wlmaps/models.py	2018-02-22 06:34:38 +0000
@@ -63,9 +63,15 @@
 
         map = super(Map, self).save(*args, **kwargs)
 
-        # Send notifications only on new maps, not when updating fields, e.g. nr_downloads
+        # Send notifications only on new maps, not when updating fields, e.g.
+        # nr_downloads
         if notification and is_new:
-            notification.send(notification.get_observers_for('maps_new_map'), 'maps_new_map',
-                              {'mapname': self.name, 'url': self.get_absolute_url(), 'user': self.uploader, 'uploader_comment': self.uploader_comment}, queue=True)
+            notification.send(notification.get_observers_for('maps_new_map', excl_user=self.uploader), 'maps_new_map',
+                              {'mapname': self.name,
+                               'url': self.get_absolute_url(),
+                               'user': self.uploader,
+                               'uploader_comment': self.uploader_comment
+                               },
+                              queue=True)
 
-        return map    
+        return map