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:37 UTC
[allura] 01/01: [#8339] make sure to preserve cookie portions for
notifications, even if they don't qualify for the current pageview
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')