You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by br...@apache.org on 2019/11/14 23:02:36 UTC
[allura] branch db/multiple_notification_fix created (now e0ae8ce)
This is an automated email from the ASF dual-hosted git repository.
brondsem pushed a change to branch db/multiple_notification_fix
in repository https://gitbox.apache.org/repos/asf/allura.git.
at e0ae8ce [#8339] make sure to preserve cookie portions for notifications, even if they don't qualify for the current pageview
This branch includes the following new commits:
new e0ae8ce [#8339] make sure to preserve cookie portions for notifications, even if they don't qualify for the current pageview
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[allura] 01/01: [#8339] make sure to preserve cookie portions for
notifications, even if they don't qualify for the current pageview
Posted by br...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
brondsem pushed a commit to branch db/multiple_notification_fix
in repository https://gitbox.apache.org/repos/asf/allura.git
commit e0ae8ce642bd2f23d787e647c92cd6f92e1a3c33
Author: Dave Brondsema <da...@brondsema.net>
AuthorDate: Thu Nov 14 18:02:00 2019 -0500
[#8339] make sure to preserve cookie portions for notifications, even if they don't qualify for the current pageview
---
Allura/allura/lib/plugin.py | 34 +++++++++++++++++++---------------
Allura/allura/model/notification.py | 2 +-
Allura/allura/tests/test_plugin.py | 12 ++++++------
3 files changed, 26 insertions(+), 22 deletions(-)
diff --git a/Allura/allura/lib/plugin.py b/Allura/allura/lib/plugin.py
index c95df68..d3c3318 100644
--- a/Allura/allura/lib/plugin.py
+++ b/Allura/allura/lib/plugin.py
@@ -1480,21 +1480,20 @@ class ThemeProvider(object):
def _get_site_notification(self, url='', user=None, tool_name='', site_notification_cookie_value=''):
from allura.model.notification import SiteNotification
- existing_cookie_info = {}
+ cookie_info = {}
try:
for existing_cookie_chunk in site_notification_cookie_value.split('_'):
note_id, views, closed = existing_cookie_chunk.split('-')
views = asint(views)
closed = asbool(closed)
- existing_cookie_info[note_id] = (views, closed)
+ cookie_info[note_id] = (views, closed)
except ValueError:
# ignore any weird cookie data
pass
- set_cookie_chunks = []
- notes = SiteNotification.actives()
+ active_notes = SiteNotification.actives()
note_to_show = None
- for note in notes:
+ for note in active_notes:
if note.user_role and (not user or user.is_anonymous()):
continue
if note.user_role:
@@ -1507,7 +1506,7 @@ class ThemeProvider(object):
if note.page_tool_type and tool_name.lower() != note.page_tool_type.lower():
continue
- views_closed = existing_cookie_info.get(str(note._id))
+ views_closed = cookie_info.get(str(note._id))
if views_closed:
views, closed = views_closed
else:
@@ -1515,20 +1514,25 @@ class ThemeProvider(object):
closed = False
if closed or note.impressions > 0 and views > note.impressions:
- # preserve info about this in the cookie (so it won't be forgotten, and thus be displayed again)
- set_cookie_chunks.append('-'.join(map(str, [note._id, views, closed])))
- # but stop this loop iteration since the notification shouldn't be shown again
continue
- # this notification is ok to show, so if there's not one picked already, this is the one.
- if not note_to_show:
- views += 1
- note_to_show = note
+ # this notification is ok to show, so this is the one.
+ views += 1
+ note_to_show = note
+ cookie_info[str(note._id)] = (views, closed)
+ break
- set_cookie_chunks.append('-'.join(map(str, [note._id, views, closed])))
+ # remove any extraneous cookie chunks so it doesn't accumulate over time into a too-large cookie
+ for note_id in list(cookie_info.keys()):
+ if note_id not in [str(n._id) for n in active_notes]:
+ del cookie_info[note_id]
if note_to_show:
- return note_to_show, '_'.join(set_cookie_chunks)
+ cookie_chunks = []
+ for note_id, views_closed in cookie_info.iteritems():
+ cookie_chunks.append('{}-{}-{}'.format(note_id, views_closed[0], views_closed[1]))
+ set_cookie_value = '_'.join(sorted(cookie_chunks))
+ return note_to_show, set_cookie_value
def get_site_notification(self):
from tg import request, response
diff --git a/Allura/allura/model/notification.py b/Allura/allura/model/notification.py
index 410a71c..1e1a707 100644
--- a/Allura/allura/model/notification.py
+++ b/Allura/allura/model/notification.py
@@ -736,4 +736,4 @@ class SiteNotification(MappedClass):
@classmethod
def actives(cls):
- return cls.query.find({'active': True}).sort('_id', -1)
+ return cls.query.find({'active': True}).sort('_id', -1).all()
diff --git a/Allura/allura/tests/test_plugin.py b/Allura/allura/tests/test_plugin.py
index 7d889d0..c3ae26d 100644
--- a/Allura/allura/tests/test_plugin.py
+++ b/Allura/allura/tests/test_plugin.py
@@ -534,17 +534,17 @@ class TestThemeProvider_notifications(object):
@patch('allura.model.notification.SiteNotification')
def test_get__site_notification_multiple(self, SiteNotification):
- note1 = MagicMock()
+ note1 = MagicMock(name='note1')
note1._id = 'test1'
note1.user_role = None
note1.page_regex = 'this-will-not-match'
note1.page_tool_type = None
- note2 = MagicMock()
+ note2 = MagicMock(name='note2')
note2._id = 'test2'
note2.user_role = None
note2.page_regex = None
note2.page_tool_type = None
- note3 = MagicMock()
+ note3 = MagicMock(name='note3')
note3._id = 'test3'
note3.user_role = None
note3.page_regex = None
@@ -554,8 +554,8 @@ class TestThemeProvider_notifications(object):
assert isinstance(get_note, tuple)
assert len(get_note) == 2
- assert get_note[0] is note2
- assert_equal(get_note[1], 'test2-1-False_test3-0-False')
+ assert_equal(get_note[0], note2)
+ assert_equal(get_note[1], 'test2-1-False')
# and with a cookie set
get_note = ThemeProvider()._get_site_notification(
@@ -564,7 +564,7 @@ class TestThemeProvider_notifications(object):
assert isinstance(get_note, tuple)
assert len(get_note) == 2
- assert get_note[0] is note3
+ assert_equal(get_note[0], note3)
assert_equal(get_note[1], 'test2-3-True_test3-1-False')
@patch('allura.model.notification.SiteNotification')