[#8339] allow multiple site notifications to be active at once
diff --git a/Allura/allura/lib/plugin.py b/Allura/allura/lib/plugin.py
index 00d6128..c95df68 100644
--- a/Allura/allura/lib/plugin.py
+++ b/Allura/allura/lib/plugin.py
@@ -1480,33 +1480,55 @@
def _get_site_notification(self, url='', user=None, tool_name='', site_notification_cookie_value=''):
from allura.model.notification import SiteNotification
- note = SiteNotification.current()
- if note is None:
- return None
- if note.user_role and (not user or user.is_anonymous()):
- return None
- if note.user_role:
- projects = user.my_projects_by_role_name(note.user_role)
- if len(projects) == 0 or len(projects) == 1 and projects[0].is_user_project:
- return None
+ existing_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)
+ except ValueError:
+ # ignore any weird cookie data
+ pass
- if note.page_regex and re.search(note.page_regex, url) is None:
- return None
- if note.page_tool_type and tool_name.lower() != note.page_tool_type.lower():
- return None
+ set_cookie_chunks = []
+ notes = SiteNotification.actives()
+ note_to_show = None
+ for note in notes:
+ if note.user_role and (not user or user.is_anonymous()):
+ continue
+ if note.user_role:
+ projects = user.my_projects_by_role_name(note.user_role)
+ if len(projects) == 0 or len(projects) == 1 and projects[0].is_user_project:
+ continue
- cookie = site_notification_cookie_value.split('-')
- if len(cookie) == 3 and cookie[0] == str(note._id):
- views = asint(cookie[1]) + 1
- closed = asbool(cookie[2])
- else:
- views = 1
- closed = False
- if closed or note.impressions > 0 and views > note.impressions:
- return None
+ if note.page_regex and re.search(note.page_regex, url) is None:
+ continue
+ if note.page_tool_type and tool_name.lower() != note.page_tool_type.lower():
+ continue
- set_cookie = '-'.join(map(str, [note._id, views, closed]))
- return note, set_cookie
+ views_closed = existing_cookie_info.get(str(note._id))
+ if views_closed:
+ views, closed = views_closed
+ else:
+ views = 0
+ 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
+
+ set_cookie_chunks.append('-'.join(map(str, [note._id, views, closed])))
+
+ if note_to_show:
+ return note_to_show, '_'.join(set_cookie_chunks)
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 080ef05..410a71c 100644
--- a/Allura/allura/model/notification.py
+++ b/Allura/allura/model/notification.py
@@ -735,6 +735,5 @@
)
@classmethod
- def current(cls):
- note = cls.query.find({'active': True}).sort('_id', -1).limit(1).first()
- return note
\ No newline at end of file
+ def actives(cls):
+ return cls.query.find({'active': True}).sort('_id', -1)
diff --git a/Allura/allura/public/nf/js/allura-base.js b/Allura/allura/public/nf/js/allura-base.js
index 21857b4..66600cd 100644
--- a/Allura/allura/public/nf/js/allura-base.js
+++ b/Allura/allura/public/nf/js/allura-base.js
@@ -208,13 +208,15 @@
}
});
- var SN_ID=0, SN_VIEWS=1, SN_CLOSED=2;
$('#site-notification .btn-close').click(function(e) {
var $note = $(this).parent();
$note.hide();
- var status = $.cookie('site-notification').split('-');
- status[SN_CLOSED] = 'true';
- $.cookie('site-notification', status.join('-'), {
+ var note_id = $note.attr('data-notification-id');
+ var cookie = $.cookie('site-notification');
+ // change e.g. "5dc2f69f07ae3175c7c21972-5-False" to "5dc2f69f07ae3175c7c21972-5-True" to mark as closed
+ // cookie may have multiple id-num-bool sets in it
+ cookie = cookie.replace(new RegExp(note_id + '-([0-9]+)-False'), note_id + '-$1-True');
+ $.cookie('site-notification', cookie, {
expires: 365,
path: '/'
});
diff --git a/Allura/allura/templates/site_admin_site_notifications_list.html b/Allura/allura/templates/site_admin_site_notifications_list.html
index e795c94..b289c35 100644
--- a/Allura/allura/templates/site_admin_site_notifications_list.html
+++ b/Allura/allura/templates/site_admin_site_notifications_list.html
@@ -23,6 +23,10 @@
{% block content %}
<div class="grid-23"><a href="/nf/admin">Back to Site Admin Home</a></div>
<div class="grid-23"><a href="new">Create a new notification</a></div>
+ <div class="grid-23">
+ If there are multiple active notifications whose criteria match for a visitor, the most recent one will be used. After that notification has been closed explicitly or automatically reached its limit, the next one can appear.
+ <br><br>
+ </div>
{{c.page_size.display(limit=limit, page=page_url, count=count)}}
<table id="site_notifications">
<thead>
diff --git a/Allura/allura/tests/functional/test_site_admin.py b/Allura/allura/tests/functional/test_site_admin.py
index 4d691f3..d7eebc6 100644
--- a/Allura/allura/tests/functional/test_site_admin.py
+++ b/Allura/allura/tests/functional/test_site_admin.py
@@ -170,6 +170,9 @@
task_name='allura.tests.functional.test_site_admin.test_task'))
assert json.loads(r.body)['doc'] == 'test_task doc string'
+
+class TestSiteAdminNotifications(TestController):
+
def test_site_notifications_access(self):
self.app.get('/nf/admin/site_notifications', extra_environ=dict(
username='test_user'), status=403)
diff --git a/Allura/allura/tests/model/test_notification.py b/Allura/allura/tests/model/test_notification.py
index 57eefd2..d154bd7 100644
--- a/Allura/allura/tests/model/test_notification.py
+++ b/Allura/allura/tests/model/test_notification.py
@@ -507,8 +507,8 @@
def _clear_subscriptions():
- M.Mailbox.query.remove({})
+ M.Mailbox.query.remove({})
def _clear_notifications():
- M.Notification.query.remove({})
+ M.Notification.query.remove({})
diff --git a/Allura/allura/tests/test_plugin.py b/Allura/allura/tests/test_plugin.py
index 6c582b1..7d889d0 100644
--- a/Allura/allura/tests/test_plugin.py
+++ b/Allura/allura/tests/test_plugin.py
@@ -251,121 +251,6 @@
class TestThemeProvider(object):
- @patch('allura.lib.plugin.c', MagicMock())
- @patch('allura.model.notification.SiteNotification')
- @patch('tg.response')
- @patch('tg.request')
- def test_get_site_notification_no_note(self, request, response, SiteNotification):
- SiteNotification.current.return_value = None
- assert_is_none(ThemeProvider().get_site_notification())
- assert not response.set_cookie.called
-
- @patch('allura.lib.plugin.c', MagicMock())
- @patch('allura.model.notification.SiteNotification')
- @patch('tg.response')
- @patch('tg.request')
- def test_get_site_notification_closed(self, request, response, SiteNotification):
- SiteNotification.current.return_value._id = 'deadbeef'
- SiteNotification.current.return_value.user_role = None
- SiteNotification.current.return_value.page_regex = None
- SiteNotification.current.return_value.page_tool_type = None
- request.cookies = {'site-notification': 'deadbeef-1-true'}
- assert_is_none(ThemeProvider().get_site_notification())
- assert not response.set_cookie.called
-
- @patch('allura.lib.plugin.c', MagicMock())
- @patch('allura.model.notification.SiteNotification')
- @patch('tg.response')
- @patch('tg.request')
- def test_get_site_notification_impressions_over(self, request, response, SiteNotification):
- note = SiteNotification.current.return_value
- note._id = 'deadbeef'
- note.impressions = 2
- note.user_role = None
- note.page_regex = None
- note.page_tool_type = None
- request.cookies = {'site-notification': 'deadbeef-3-false'}
- assert_is_none(ThemeProvider().get_site_notification())
- assert not response.set_cookie.called
-
- @patch('allura.lib.plugin.c', MagicMock())
- @patch('allura.model.notification.SiteNotification')
- @patch('tg.response')
- @patch('tg.request')
- def test_get_site_notification_impressions_under(self, request, response, SiteNotification):
- note = SiteNotification.current.return_value
- note._id = 'deadbeef'
- note.impressions = 2
- note.user_role = None
- note.page_regex = None
- note.page_tool_type = None
- request.cookies = {'site-notification': 'deadbeef-1-false'}
- assert_is(ThemeProvider().get_site_notification(), note)
- response.set_cookie.assert_called_once_with(
- 'site-notification', 'deadbeef-2-False', max_age=dt.timedelta(days=365))
-
- @patch('allura.lib.plugin.c', MagicMock())
- @patch('allura.model.notification.SiteNotification')
- @patch('tg.response')
- @patch('tg.request')
- def test_get_site_notification_impressions_persistent(self, request, response, SiteNotification):
- note = SiteNotification.current.return_value
- note._id = 'deadbeef'
- note.impressions = 0
- note.user_role = None
- note.page_regex = None
- note.page_tool_type = None
- request.cookies = {'site-notification': 'deadbeef-1000-false'}
- assert_is(ThemeProvider().get_site_notification(), note)
-
- @patch('allura.lib.plugin.c', MagicMock())
- @patch('allura.model.notification.SiteNotification')
- @patch('tg.response')
- @patch('tg.request')
- def test_get_site_notification_new_notification(self, request, response, SiteNotification):
- note = SiteNotification.current.return_value
- note._id = 'deadbeef'
- note.impressions = 1
- note.user_role = None
- note.page_regex = None
- note.page_tool_type = None
- request.cookies = {'site-notification': '0ddba11-1000-true'}
- assert_is(ThemeProvider().get_site_notification(), note)
- response.set_cookie.assert_called_once_with(
- 'site-notification', 'deadbeef-1-False', max_age=dt.timedelta(days=365))
-
- @patch('allura.lib.plugin.c', MagicMock())
- @patch('allura.model.notification.SiteNotification')
- @patch('tg.response')
- @patch('tg.request')
- def test_get_site_notification_no_cookie(self, request, response, SiteNotification):
- note = SiteNotification.current.return_value
- note._id = 'deadbeef'
- note.impressions = 0
- note.user_role = None
- note.page_regex = None
- note.page_tool_type = None
- request.cookies = {}
- assert_is(ThemeProvider().get_site_notification(), note)
- response.set_cookie.assert_called_once_with(
- 'site-notification', 'deadbeef-1-False', max_age=dt.timedelta(days=365))
-
- @patch('allura.lib.plugin.c', MagicMock())
- @patch('allura.model.notification.SiteNotification')
- @patch('tg.response')
- @patch('tg.request')
- def test_get_site_notification_bad_cookie(self, request, response, SiteNotification):
- note = SiteNotification.current.return_value
- note._id = 'deadbeef'
- note.impressions = 0
- note.user_role = None
- note.page_regex = None
- note.page_tool_type = None
- request.cookies = {'site-notification': 'deadbeef-1000-true-bad'}
- assert_is(ThemeProvider().get_site_notification(), note)
- response.set_cookie.assert_called_once_with(
- 'site-notification', 'deadbeef-1-False', max_age=dt.timedelta(days=365))
-
@patch('allura.app.g')
@patch('allura.lib.plugin.g')
def test_app_icon_str(self, plugin_g, app_g):
@@ -395,15 +280,142 @@
g.theme_href.return_value)
g.theme_href.assert_called_with('images/testapp_24.png')
+
+class TestThemeProvider_notifications(object):
+
+ @patch('allura.lib.plugin.c', MagicMock())
+ @patch('allura.model.notification.SiteNotification')
+ @patch('tg.response')
+ @patch('tg.request')
+ def test_get_site_notification_no_note(self, request, response, SiteNotification):
+ SiteNotification.actives.return_value = []
+ assert_is_none(ThemeProvider().get_site_notification())
+ assert not response.set_cookie.called
+
+ @patch('allura.lib.plugin.c', MagicMock())
+ @patch('allura.model.notification.SiteNotification')
+ @patch('tg.response')
+ @patch('tg.request')
+ def test_get_site_notification_closed(self, request, response, SiteNotification):
+ note = MagicMock()
+ note._id = 'deadbeef'
+ note.user_role = None
+ note.page_regex = None
+ note.page_tool_type = None
+ SiteNotification.actives.return_value = [note]
+ request.cookies = {'site-notification': 'deadbeef-1-true'}
+ assert_is_none(ThemeProvider().get_site_notification())
+ assert not response.set_cookie.called
+
+ @patch('allura.lib.plugin.c', MagicMock())
+ @patch('allura.model.notification.SiteNotification')
+ @patch('tg.response')
+ @patch('tg.request')
+ def test_get_site_notification_impressions_over(self, request, response, SiteNotification):
+ note = MagicMock()
+ note._id = 'deadbeef'
+ note.impressions = 2
+ note.user_role = None
+ note.page_regex = None
+ note.page_tool_type = None
+ SiteNotification.actives.return_value = [note]
+ request.cookies = {'site-notification': 'deadbeef-3-false'}
+ assert_is_none(ThemeProvider().get_site_notification())
+ assert not response.set_cookie.called
+
+ @patch('allura.lib.plugin.c', MagicMock())
+ @patch('allura.model.notification.SiteNotification')
+ @patch('tg.response')
+ @patch('tg.request')
+ def test_get_site_notification_impressions_under(self, request, response, SiteNotification):
+ note = MagicMock()
+ note._id = 'deadbeef'
+ note.impressions = 2
+ note.user_role = None
+ note.page_regex = None
+ note.page_tool_type = None
+ SiteNotification.actives.return_value = [note]
+ request.cookies = {'site-notification': 'deadbeef-1-false'}
+ assert_is(ThemeProvider().get_site_notification(), note)
+ response.set_cookie.assert_called_once_with(
+ 'site-notification', 'deadbeef-2-False', max_age=dt.timedelta(days=365))
+
+ @patch('allura.lib.plugin.c', MagicMock())
+ @patch('allura.model.notification.SiteNotification')
+ @patch('tg.response')
+ @patch('tg.request')
+ def test_get_site_notification_impressions_persistent(self, request, response, SiteNotification):
+ note = MagicMock()
+ note._id = 'deadbeef'
+ note.impressions = 0
+ note.user_role = None
+ note.page_regex = None
+ note.page_tool_type = None
+ SiteNotification.actives.return_value = [note]
+ request.cookies = {'site-notification': 'deadbeef-1000-false'}
+ assert_is(ThemeProvider().get_site_notification(), note)
+
+ @patch('allura.lib.plugin.c', MagicMock())
+ @patch('allura.model.notification.SiteNotification')
+ @patch('tg.response')
+ @patch('tg.request')
+ def test_get_site_notification_new_notification(self, request, response, SiteNotification):
+ note = MagicMock()
+ note._id = 'deadbeef'
+ note.impressions = 1
+ note.user_role = None
+ note.page_regex = None
+ note.page_tool_type = None
+ SiteNotification.actives.return_value = [note]
+ request.cookies = {'site-notification': '0ddba11-1000-true'}
+ assert_is(ThemeProvider().get_site_notification(), note)
+ response.set_cookie.assert_called_once_with(
+ 'site-notification', 'deadbeef-1-False', max_age=dt.timedelta(days=365))
+
+ @patch('allura.lib.plugin.c', MagicMock())
+ @patch('allura.model.notification.SiteNotification')
+ @patch('tg.response')
+ @patch('tg.request')
+ def test_get_site_notification_no_cookie(self, request, response, SiteNotification):
+ note = MagicMock()
+ note._id = 'deadbeef'
+ note.impressions = 0
+ note.user_role = None
+ note.page_regex = None
+ note.page_tool_type = None
+ SiteNotification.actives.return_value = [note]
+ request.cookies = {}
+ assert_is(ThemeProvider().get_site_notification(), note)
+ response.set_cookie.assert_called_once_with(
+ 'site-notification', 'deadbeef-1-False', max_age=dt.timedelta(days=365))
+
+ @patch('allura.lib.plugin.c', MagicMock())
+ @patch('allura.model.notification.SiteNotification')
+ @patch('tg.response')
+ @patch('tg.request')
+ def test_get_site_notification_bad_cookie(self, request, response, SiteNotification):
+ note = MagicMock()
+ note._id = 'deadbeef'
+ note.impressions = 0
+ note.user_role = None
+ note.page_regex = None
+ note.page_tool_type = None
+ SiteNotification.actives.return_value = [note]
+ request.cookies = {'site-notification': 'deadbeef-1000-true-bad'}
+ assert_is(ThemeProvider().get_site_notification(), note)
+ response.set_cookie.assert_called_once_with(
+ 'site-notification', 'deadbeef-1-False', max_age=dt.timedelta(days=365))
+
@patch('allura.lib.plugin.c')
@patch('allura.model.notification.SiteNotification')
@patch('tg.response', MagicMock())
@patch('tg.request', MagicMock())
def test_get_site_notification_with_role(self, SiteNotification, c):
- note = SiteNotification.current.return_value
+ note = MagicMock()
note.user_role = 'Test'
note.page_regex = None
note.page_tool_type = None
+ SiteNotification.actives.return_value = [note]
projects = c.user.my_projects_by_role_name
c.user.is_anonymous.return_value = True
@@ -428,10 +440,11 @@
@patch('tg.response', MagicMock())
@patch('tg.request', MagicMock())
def test_get_site_notification_without_role(self, SiteNotification):
- note = SiteNotification.current.return_value
+ note = MagicMock()
note.user_role = None
note.page_regex = None
note.page_tool_type = None
+ SiteNotification.actives.return_value = [note]
assert_is(ThemeProvider().get_site_notification(), note)
@patch('allura.lib.plugin.c', MagicMock())
@@ -440,10 +453,11 @@
@patch('tg.response', MagicMock())
@patch('tg.request', MagicMock())
def test_get_site_notification_with_page_regex(self, SiteNotification, search):
- note = SiteNotification.current.return_value
+ note = MagicMock()
note.user_role = None
note.page_regex = 'test'
note.page_tool_type = None
+ SiteNotification.actives.return_value = [note]
search.return_value = True
assert_is(ThemeProvider().get_site_notification(), note)
@@ -456,12 +470,12 @@
@patch('tg.response', MagicMock())
@patch('tg.request', MagicMock())
def test_get_site_notification_with_page_tool_type(self, SiteNotification, c):
- note = SiteNotification.current.return_value
+ note = MagicMock()
note.user_role = None
note.page_regex = None
- c.app = Mock()
note.page_tool_type.lower.return_value = 'test1'
-
+ SiteNotification.actives.return_value = [note]
+ c.app = Mock()
c.app.config.tool_name.lower.return_value = 'test1'
assert_is(ThemeProvider().get_site_notification(), note)
@@ -476,11 +490,12 @@
@patch('allura.model.notification.SiteNotification')
@patch('tg.response', MagicMock())
def test_get_site_notification_with_page_tool_type_page_regex(self, SiteNotification, request, c):
- note = SiteNotification.current.return_value
+ note = MagicMock()
note.user_role = None
note.page_regex = 'test'
- c.app = Mock()
note.page_tool_type.lower.return_value = 'test1'
+ SiteNotification.actives.return_value = [note]
+ c.app = Mock()
request.path_qs = 'ttt'
c.app.config.tool_name.lower.return_value = 'test2'
@@ -504,33 +519,68 @@
@patch('allura.model.notification.SiteNotification')
def test_get__site_notification(self, SiteNotification):
- note = SiteNotification.current.return_value
- note._id = 'test_id'
+ note = MagicMock()
+ note._id = 'testid'
note.user_role = None
note.page_regex = None
note.page_tool_type = None
+ SiteNotification.actives.return_value = [note]
get_note = ThemeProvider()._get_site_notification()
assert isinstance(get_note, tuple)
assert len(get_note) == 2
assert get_note[0] is note
- assert get_note[1] == 'test_id-1-False'
+ assert get_note[1] == 'testid-1-False'
@patch('allura.model.notification.SiteNotification')
- @patch('tg.request')
- def test_get_site_notifications_with_api_cookie(self, request, SiteNotification):
- note = SiteNotification.current.return_value
- note._id = 'test_id'
+ def test_get__site_notification_multiple(self, SiteNotification):
+ note1 = MagicMock()
+ note1._id = 'test1'
+ note1.user_role = None
+ note1.page_regex = 'this-will-not-match'
+ note1.page_tool_type = None
+ note2 = MagicMock()
+ note2._id = 'test2'
+ note2.user_role = None
+ note2.page_regex = None
+ note2.page_tool_type = None
+ note3 = MagicMock()
+ note3._id = 'test3'
+ note3.user_role = None
+ note3.page_regex = None
+ note3.page_tool_type = None
+ SiteNotification.actives.return_value = [note1, note2, note3]
+ get_note = ThemeProvider()._get_site_notification()
+
+ 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')
+
+ # and with a cookie set
+ get_note = ThemeProvider()._get_site_notification(
+ site_notification_cookie_value='test2-3-True_test3-0-False'
+ )
+
+ assert isinstance(get_note, tuple)
+ assert len(get_note) == 2
+ assert get_note[0] is note3
+ assert_equal(get_note[1], 'test2-3-True_test3-1-False')
+
+ @patch('allura.model.notification.SiteNotification')
+ def test_get_site_notifications_with_api_cookie(self, SiteNotification):
+ note = MagicMock()
+ note._id = 'testid'
note.user_role = None
note.page_regex = None
note.page_tool_type = None
- request.cookies = {}
+ SiteNotification.actives.return_value = [note]
get_note = ThemeProvider()._get_site_notification(
- site_notification_cookie_value='test_id-1-False'
+ site_notification_cookie_value='testid-1-False'
)
assert get_note[0] is note
- assert get_note[1] == 'test_id-2-False'
+ assert get_note[1] == 'testid-2-False'
class TestLocalAuthenticationProvider(object):
diff --git a/Allura/docs/getting_started/administration.rst b/Allura/docs/getting_started/administration.rst
index 6c35f6f..76469b7 100644
--- a/Allura/docs/getting_started/administration.rst
+++ b/Allura/docs/getting_started/administration.rst
@@ -333,8 +333,8 @@
the current page is matching regex :code:`(Home|browse_pages)` and app
tool type is :code:`wiki`. An "Impressions" value of 0 will show the
notification indefinitely (until closed). The notification content can contain
-HTML. Only the most recent active notification will be shown.
-"User Role", "Page Regex" and "Page Type" are optional.
+HTML. The most recent notification that is active and matches for the visitor
+will be shown. "User Role", "Page Regex" and "Page Type" are optional.
.. _delete-projects: