widelands-dev team mailing list archive

[kaputtnik <somal@xxxxxxxx>]

kaputtnik has proposed merging lp:~widelands-dev/widelands-website/better_notification_mail into lp:widelands-website.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #340920 in Widelands Website: "wiki change notification mail is very sparse"
  https://bugs.launchpad.net/widelands-website/+bug/340920
  Bug #1736263 in Widelands Website: "notification to user of user's posts"
  https://bugs.launchpad.net/widelands-website/+bug/1736263

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands-website/better_notification_mail/+merge/335028

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.
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands-website/better_notification_mail into lp:widelands-website.

=== modified file 'notification/models.py'
--- notification/models.py	2017-05-07 09:51:15 +0000
+++ notification/models.py	2017-12-11 13:12:44 +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)
 

=== modified file 'pybb/views.py'
--- pybb/views.py	2017-11-26 10:57:35 +0000
+++ pybb/views.py	2017-12-11 13:12:44 +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	2017-12-11 13:12:44 +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	2017-12-11 13:12:44 +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	2017-12-11 13:12:44 +0000
@@ -2,7 +2,7 @@
 
 "{{ 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 }}

=== 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	2017-12-11 13:12:44 +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 }}
+-------------------------
+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	2017-12-11 13:12:44 +0000
@@ -1,7 +1,7 @@
-{% 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 %}

=== 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

=== 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	2017-12-11 13:12:44 +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	2017-12-11 13:12:44 +0000
@@ -1,4 +1,10 @@
-{% load i18n %}{% blocktrans with observed.get_absolute_url as article_url %}The article {{ observed }} that you observe has changed.
-
-http://{{ current_site }}{{ article_url }}
+{% 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 }}"
+
+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	2017-12-11 13:12:44 +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
+{% 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	2017-12-11 13:12:44 +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	2017-12-11 13:12:44 +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	2017-12-11 13:12:44 +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	2017-12-11 13:12:44 +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	2017-12-11 13:12:44 +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=request.user), '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