From faa60e85d25a6da4e2c33b7a05f6043ca56cf1e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20P=C3=A9ters?= Date: Sun, 28 Feb 2016 11:58:12 +0100 Subject: [PATCH] backoffice: add advisory locking to formdata (#10075) --- tests/test_backoffice_pages.py | 53 +++++++++++++++++++++++++++ tests/test_sessions.py | 73 +++++++++++++++++++++++++++++++++++++ wcs/backoffice/management.py | 13 +++++-- wcs/forms/backoffice.py | 17 +++++++-- wcs/forms/common.py | 41 ++++++++++++++++++++- wcs/publisher.py | 9 +++++ wcs/qommon/static/css/dc2/admin.css | 17 +++++++++ wcs/sessions.py | 47 +++++++++++++++++++++++- 8 files changed, 261 insertions(+), 9 deletions(-) diff --git a/tests/test_backoffice_pages.py b/tests/test_backoffice_pages.py index de713ef..8d7e30f 100644 --- a/tests/test_backoffice_pages.py +++ b/tests/test_backoffice_pages.py @@ -1715,3 +1715,56 @@ def test_backoffice_resume_folded(pub): resp = resp.form.submit('button_commentable') resp = resp.follow() assert '

Summary

' in resp.body + +def test_backoffice_advisory_lock(pub): + pub.session_manager.session_class.wipe() + create_superuser(pub) + create_environment(pub) + + second_user = pub.user_class(name='foobar') + second_user.roles = Role.keys() + second_user.store() + account = PasswordAccount(id='foobar') + account.set_password('foobar') + account.user_id = second_user.id + account.store() + + app = login(get_app(pub)) + resp = app.get('/backoffice/management/form-title/') + first_link = re.findall('data-link="(\d+/)?"', resp.body)[0] + assert not 'advisory-lock' in resp.body + + app2 = login(get_app(pub), username='foobar', password='foobar') + resp = app2.get('/backoffice/management/form-title/') + assert not 'advisory-lock' in resp.body + + resp = app.get('/backoffice/management/form-title/' + first_link) + resp = app2.get('/backoffice/management/form-title/') + assert 'advisory-lock' in resp.body + resp = app.get('/backoffice/management/form-title/') + assert not 'advisory-lock' in resp.body + + if pub.is_using_postgresql(): + # check global view + resp = app2.get('/backoffice/management/listing?limit=100') + assert 'advisory-lock' in resp.body + resp = app.get('/backoffice/management/listing?limit=100') + assert not 'advisory-lock' in resp.body + + resp = app.get('/backoffice/management/form-title/' + first_link) + assert not 'Be warned this form is also being' in resp.body + assert len(resp.forms) == 1 + resp = app2.get('/backoffice/management/form-title/' + first_link) + assert 'Be warned this form is also being' in resp.body + assert len(resp.forms) == 0 + # revisit with first user, no change + resp = app.get('/backoffice/management/form-title/' + first_link) + assert not 'Be warned this form is also being' in resp.body + # back to second + resp = app2.get('/backoffice/management/form-title/' + first_link) + assert 'Be warned this form is also being' in resp.body + resp = resp.click('(unlock actions)') + resp = resp.follow() + assert 'Be warned this form is also being' in resp.body + assert not '(unlock actions)' in resp.body + assert len(resp.forms) == 1 diff --git a/tests/test_sessions.py b/tests/test_sessions.py index e42458b..5cdcf13 100644 --- a/tests/test_sessions.py +++ b/tests/test_sessions.py @@ -6,6 +6,7 @@ import pytest from quixote import cleanup from wcs.qommon.ident.password_accounts import PasswordAccount +from wcs.qommon.http_request import HTTPRequest from utilities import create_temporary_pub, clean_temporary_pub, get_app, login @@ -29,6 +30,12 @@ def pub(request): pub.write_cfg() return pub +@pytest.fixture +def http_request(pub): + req = HTTPRequest(None, {}) + req.language = None + pub._set_request(req) + @pytest.fixture def user(pub): @@ -71,3 +78,69 @@ def test_session_expire(pub, user, app): session.set_expire(time.time() - 1) session.store() assert 'Logout' not in app.get('/') + +def test_sessions_visiting_objects(pub, http_request): + manager = pub.session_manager_class() + # check it starts with nothing + assert len(pub.get_visited_objects()) == 0 + + # mark two visits + session1 = manager.session_class(id='session1') + session1.user = 'FOO' + session1.mark_visited_object('formdata-foobar-1') + session1.mark_visited_object('formdata-foobar-2') + session1.store() + assert len(pub.get_visited_objects()) == 2 + assert set([x[0] for x in pub.get_object_visitors('formdata-foobar-2')]) == set(['FOO']) + + # mark a visit as being in the past + session1.visiting_objects['formdata-foobar-1'] = time.time() - 35*60 + session1.store() + assert len(pub.get_visited_objects()) == 1 + + # check older visits are automatically removed + session1 = manager.session_class.get('session1') + assert len(session1.visiting_objects.keys()) == 2 + session1.mark_visited_object('formdata-foobar-2') + assert len(session1.visiting_objects.keys()) == 1 + session1.store() + assert len(pub.get_visited_objects()) == 1 + assert pub.get_visited_objects() == ['formdata-foobar-2'] + + # check with a second session + session1.mark_visited_object('formdata-foobar-1') + session1.mark_visited_object('formdata-foobar-2') + session1.store() + assert len(pub.get_visited_objects()) == 2 + + # mark a visit as being in the past + session1.visiting_objects['formdata-foobar-1'] = time.time() - 35*60 + session1.store() + assert len(pub.get_visited_objects()) == 1 + + # check older visits are automatically removed + session1 = manager.session_class.get('session1') + assert len(session1.visiting_objects.keys()) == 2 + session1.mark_visited_object('formdata-foobar-2') + assert len(session1.visiting_objects.keys()) == 1 + session1.store() + assert len(pub.get_visited_objects()) == 1 + assert pub.get_visited_objects() == ['formdata-foobar-2'] + + # check with a second session + session2 = manager.session_class(id='session2') + session2.user = 'BAR' + session2.store() + assert len(pub.get_visited_objects()) == 1 + session2.mark_visited_object('formdata-foobar-2') + session2.store() + assert len(pub.get_visited_objects()) == 1 + session2.mark_visited_object('formdata-foobar-3') + session2.store() + assert len(pub.get_visited_objects()) == 2 + + assert pub.get_visited_objects(exclude_user='BAR') == ['formdata-foobar-2'] + + # check visitors + assert set([x[0] for x in pub.get_object_visitors('formdata-foobar-2')]) == set(['FOO', 'BAR']) + assert set([x[0] for x in pub.get_object_visitors('formdata-foobar-1')]) == set([]) diff --git a/wcs/backoffice/management.py b/wcs/backoffice/management.py index 5571a8b..aade59e 100644 --- a/wcs/backoffice/management.py +++ b/wcs/backoffice/management.py @@ -687,6 +687,7 @@ class ManagementDirectory(Directory): r = TemplateIO(html=True) r += htmltext('') r += htmltext('') + r += htmltext('') # lock if include_submission_channel: r += htmltext('') % _('Channel') r += htmltext('') % _('Form') @@ -698,13 +699,19 @@ class ManagementDirectory(Directory): r += htmltext('') r += htmltext('') workflows = {} + visited_objects = get_publisher().get_visited_objects(exclude_user=get_session().user) for formdata in formdatas: if not formdata.formdef.workflow_id in workflows: workflows[formdata.formdef.workflow_id] = formdata.formdef.workflow - r += htmltext('' % ( - formdata.formdef.workflow.id, - formdata.status, + + classes = ['status-%s-%s' % (formdata.formdef.workflow.id, formdata.status)] + object_key = 'formdata-%s-%s' % (formdata.formdef.url_name, formdata.id) + if object_key in visited_objects: + classes.append('advisory-lock') + r += htmltext('' % ( + ' '.join(classes), formdata.get_url(backoffice=True))) + r += htmltext('') # lock if include_submission_channel: r += htmltext('') % formdata.get_submission_channel_label() r += htmltext('') % formdata.formdef.name diff --git a/wcs/forms/backoffice.py b/wcs/forms/backoffice.py index c95e877..af0b4c4 100644 --- a/wcs/forms/backoffice.py +++ b/wcs/forms/backoffice.py @@ -68,6 +68,7 @@ class FormDefUI(object): r += htmltext('
%s%s
%s%s
') r += htmltext('') + r += htmltext('') # lock r += htmltext('') r += htmltext('') for f in fields: @@ -75,6 +76,7 @@ class FormDefUI(object): r += htmltext('') r += htmltext('') + r += htmltext('') # lock for f in fields: field_sort_key = None if getattr(f, 'fake', False): @@ -206,17 +208,24 @@ class FormDefUI(object): else: url_action = '' root_url = get_publisher().get_root_url() + visited_objects = get_publisher().get_visited_objects(exclude_user=get_session().user) for i, filled in enumerate(items): + classes = ['status-%s-%s' % (filled.formdef.workflow.id, filled.status)] if i%2: - style = 'even' + classes.append('even') else: - style = 'odd' + classes.append('odd') + + object_key = 'formdata-%s-%s' % (filled.formdef.url_name, filled.id) + if object_key in visited_objects: + classes.append('advisory-lock') + link = str(filled.id) + '/' data = ' data-link="%s"' % link if filled.anonymised: data += ' data-anonymised="true"' - r += htmltext('' % (filled.formdef.workflow.id, - filled.status, style, data)) + r += htmltext('' % (' '.join(classes), data)) + r += htmltext('') # lock for i, f in enumerate(fields): if f.type == 'id': r += htmltext('') % (link, url_action, diff --git a/wcs/forms/common.py b/wcs/forms/common.py index c9d4add..7280ea1 100644 --- a/wcs/forms/common.py +++ b/wcs/forms/common.py @@ -466,6 +466,13 @@ class FormStatusPage(Directory): def status(self): + if get_request().get_query() == 'unlock': + # mark user as active visitor of the object, then redirect to self, + # the unlocked form will appear. + object_key = 'formdata-%s-%s' % (self.formdef.url_name, self.filled.id) + get_session().mark_visited_object(object_key) + return redirect('./#lock-notice') + user = self.check_receiver() form = None @@ -522,7 +529,37 @@ class FormStatusPage(Directory): r += self.history() if form: - r += form.render() + object_key = 'formdata-%s-%s' % (self.formdef.url_name, self.filled.id) + all_visitors = get_publisher().get_object_visitors(object_key) + visitors = [x for x in all_visitors if x[0] != get_session().user] + me_in_visitors = bool(get_session().user in [x[0] for x in all_visitors]) + if visitors: + current_timestamp = time.time() + visitor_users = [] + for visitor_id, visitor_timestamp in visitors: + try: + visitor_name = get_publisher().user_class.get(visitor_id).display_name + except KeyError: + continue + minutes_ago = int((current_timestamp - visitor_timestamp) / 60) + if minutes_ago < 1: + time_ago = _('less than a minute ago') + else: + time_ago = _('less than %s minutes ago') % (minutes_ago + 1) + visitor_users.append('%s (%s)' % (visitor_name, time_ago)) + if visitor_users: + r += htmltext('

') + r += _('Be warned this form is also being looked at by: ' + '%s.') % ', '.join(visitor_users) + r += ' ' + r += htmltext('

') + if not me_in_visitors: + r += htmltext('

%s

' + ) % _('(unlock actions)') + r += htmltext('
') + if not visitors or me_in_visitors: + r += form.render() + get_session().mark_visited_object(object_key) if (self.filled.get_status() and self.filled.get_status().backoffice_info_text) or ( form and any((getattr(button, 'backoffice_info_text', None) @@ -654,6 +691,8 @@ class FormStatusPage(Directory): f.edited_data = self.filled f.edit_action_id = action_id f.action_url = 'wfedit-%s' % action_id + get_session().mark_visited_object('formdata-%s-%s' % ( + self.formdef.url_name, self.filled.id)) get_response().breadcrumb = get_response().breadcrumb[:-1] get_response().breadcrumb.append((f.action_url, _('Edit'))) return f._q_index() diff --git a/wcs/publisher.py b/wcs/publisher.py index 3bd905f..87fd1b3 100644 --- a/wcs/publisher.py +++ b/wcs/publisher.py @@ -214,6 +214,15 @@ class WcsPublisher(StubWcsPublisher): request.response.iframe_mode = True return QommonPublisher.try_publish(self, request) + def get_object_visitors(self, object_key): + session_manager = self.session_manager_class() + return session_manager.session_class.get_object_visitors(object_key) + + def get_visited_objects(self, exclude_user=None): + session_manager = self.session_manager_class() + return session_manager.session_class.get_visited_objects( + exclude_user=exclude_user) + def initialize_sql(self): import sql sql.get_connection(new=True) diff --git a/wcs/qommon/static/css/dc2/admin.css b/wcs/qommon/static/css/dc2/admin.css index 6f13e10..ff0ad18 100644 --- a/wcs/qommon/static/css/dc2/admin.css +++ b/wcs/qommon/static/css/dc2/admin.css @@ -339,6 +339,10 @@ span.activity-done { margin-bottom: 1em; } +#listing thead td { + background: white; +} + ul.biglist, table#listing { -webkit-transition: opacity 500ms ease-out; -moz-transition: opacity 500ms ease-out; @@ -412,6 +416,15 @@ div.page-nav span.page-count-label { margin: 0 1ex; } +table.main tr.advisory-lock { + opacity: 0.5; +} + +tr.advisory-lock td:first-child::before { + font-family: FontAwesome; + content: "\f023"; /* lock */ +} + div.bo-block ul.biglist li.user-is-admin strong a { border-left: 5px solid #0099ff; } @@ -1083,6 +1096,10 @@ a#filter-settings { background: #FFFFA0; } +.infonotice p.action { + text-align: right; +} + div.WidgetDict div.content div.StringWidget { width: 25%; } diff --git a/wcs/sessions.py b/wcs/sessions.py index 7b7bc41..68efa26 100644 --- a/wcs/sessions.py +++ b/wcs/sessions.py @@ -15,6 +15,7 @@ # along with this program; if not, see . import random +import time import qommon.sessions from qommon.sessions import Session @@ -24,9 +25,11 @@ class BasicSession(Session): anonymous_key = None magictokens = None anonymous_formdata_keys = None + visiting_objects = None def has_info(self): - return self.anonymous_formdata_keys or self.anonymous_key or self.magictokens or Session.has_info(self) + return (self.anonymous_formdata_keys or self.anonymous_key or + self.magictokens or self.visiting_objects or Session.has_info(self)) is_dirty = has_info def get_anonymous_key(self, generate = False): @@ -63,5 +66,47 @@ class BasicSession(Session): formdata_key = '%s-%s' % (formdata.formdef.id, formdata.id) return formdata_key in self.anonymous_formdata_keys + def mark_visited_object(self, key): + if not self.visiting_objects: + self.visiting_objects = {} + # first clean older objects + current_timestamp = time.time() + for object_key, object_timestamp in self.visiting_objects.items(): + if object_timestamp < (current_timestamp - 30*60): + del self.visiting_objects[object_key] + self.visiting_objects[key] = current_timestamp + + @classmethod + def get_visited_objects(cls, exclude_user=None): + # return the list of visited objects + current_timestamp = time.time() + visited_objects = {} + for session in cls.select(): + if session.user and session.user == exclude_user: + continue + visiting_objects = getattr(session, 'visiting_objects', None) + if not visiting_objects: + continue + for object_key, object_timestamp in visiting_objects.items(): + if object_timestamp > (current_timestamp - 30*60): + visited_objects[object_key] = True + return visited_objects.keys() + + @classmethod + def get_object_visitors(cls, object_key): + '''return tuples of (user_id, last_visit_timestamp)''' + current_timestamp = time.time() + visitors = {} + for session in cls.select(): + visiting_objects = getattr(session, 'visiting_objects', None) + if not visiting_objects: + continue + object_timestamp = visiting_objects.get(object_key) + if not object_timestamp: + continue + if object_timestamp > (current_timestamp - 30*60): + visitors[session.user] = max(object_timestamp, visitors.get(session.user, 0)) + return visitors.items() + qommon.sessions.BasicSession = BasicSession StorageSessionManager = qommon.sessions.StorageSessionManager -- 2.7.0
%s