From 5b2a2984ca9956a9573ec08665d6c959efe37743 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laur=C3=A9line=20Gu=C3=A9rin?= <zebuline@entrouvert.com>
Date: Mon, 14 Dec 2020 15:56:14 +0100
Subject: [PATCH 2/3] errors: list all errors on one page (#48926)

---
 tests/admin_pages/test_logged_errors.py       | 243 ++++++++++++++++++
 wcs/admin/logged_errors.py                    |  69 ++++-
 wcs/backoffice/studio.py                      |  32 ++-
 wcs/qommon/backoffice/listing.py              |   5 +-
 wcs/sql.py                                    |  12 +-
 .../wcs/backoffice/logged-errors.html         |   1 +
 6 files changed, 340 insertions(+), 22 deletions(-)
 create mode 100644 tests/admin_pages/test_logged_errors.py

diff --git a/tests/admin_pages/test_logged_errors.py b/tests/admin_pages/test_logged_errors.py
new file mode 100644
index 00000000..838e6ed3
--- /dev/null
+++ b/tests/admin_pages/test_logged_errors.py
@@ -0,0 +1,243 @@
+# -*- coding: utf-8 -*-
+
+import datetime
+
+import pytest
+
+from wcs.carddef import CardDef
+from wcs.formdef import FormDef
+from wcs.qommon.http_request import HTTPRequest
+from wcs.workflows import Workflow
+
+from utilities import get_app, login, create_temporary_pub, clean_temporary_pub
+from .test_all import create_superuser
+
+
+@pytest.fixture
+def pub(request):
+    pub = create_temporary_pub(sql_mode=True)
+    req = HTTPRequest(None, {'SCRIPT_NAME': '/', 'SERVER_NAME': 'example.net'})
+    pub.set_app_dir(req)
+    pub.cfg['identification'] = {'methods': ['password']}
+    pub.cfg['language'] = {'language': 'en'}
+    pub.write_cfg()
+    return pub
+
+
+def teardown_module(module):
+    clean_temporary_pub()
+
+
+def test_studio_home(pub):
+    create_superuser(pub)
+    app = login(get_app(pub))
+    resp = app.get('/backoffice/studio/')
+    assert 'logged-errors/' in resp.text
+
+
+def test_listing_paginations(pub):
+    pub.loggederror_class.wipe()
+
+    FormDef.wipe()
+    CardDef.wipe()
+    Workflow.wipe()
+
+    formdef = FormDef()
+    formdef.name = 'foo'
+    formdef.store()
+    formdef2 = FormDef()
+    formdef2.name = 'foo 2'
+    formdef2.store()
+    carddef = CardDef()
+    carddef.name = 'bar'
+    carddef.store()
+    carddef2 = CardDef()
+    carddef2.name = 'bar 2'
+    carddef2.store()
+    workflow = Workflow()
+    workflow.name = 'blah'
+    workflow.store()
+    workflow2 = Workflow()
+    workflow2.name = 'blah 2'
+    workflow2.store()
+
+    # FormDef errors
+    for i in range(0, 21):
+        error = pub.loggederror_class()
+        error.summary = 'FormDef Workflow Logged Error n°%s' % i
+        error.formdef_class = 'FormDef'
+        error.formdef_id = formdef.id
+        error.workflow_id = workflow.id
+        error.first_occurence_timestamp = datetime.datetime.now()
+        error.store()
+    error = pub.loggederror_class()
+    error.summary = 'FormDef 2 Workflow 2 Logged Error n°%s' % i
+    error.formdef_class = 'FormDef'
+    error.formdef_id = formdef2.id
+    error.workflow_id = workflow2.id
+    error.first_occurence_timestamp = datetime.datetime.now()
+    error.store()
+
+    # CardDef errors
+    for i in range(0, 21):
+        error = pub.loggederror_class()
+        error.summary = 'CardDef Workflow Logged Error n°%s' % i
+        error.formdef_class = 'CardDef'
+        error.formdef_id = carddef.id
+        error.workflow_id = workflow.id
+        error.first_occurence_timestamp = datetime.datetime.now()
+        error.store()
+    error = pub.loggederror_class()
+    error.summary = 'CardDef 2 Workflow 2 Logged Error n°%s' % i
+    error.formdef_class = 'CardDef'
+    error.formdef_id = carddef2.id
+    error.workflow_id = workflow2.id
+    error.first_occurence_timestamp = datetime.datetime.now()
+    error.store()
+
+    # workflow-only errors
+    for i in range(0, 21):
+        error = pub.loggederror_class()
+        error.summary = 'Workflow Logged Error n°%s' % i
+        error.workflow_id = workflow.id
+        error.first_occurence_timestamp = datetime.datetime.now()
+        error.store()
+    error = pub.loggederror_class()
+    error.summary = 'Workflow 2 Logged Error n°%s' % i
+    error.workflow_id = workflow2.id
+    error.first_occurence_timestamp = datetime.datetime.now()
+    error.store()
+
+    create_superuser(pub)
+    app = login(get_app(pub))
+
+    # all errors
+
+    # default pagination
+    resp = app.get('/backoffice/studio/logged-errors/')
+    assert '1-20/66' in resp.text
+    assert resp.text.count('Logged Error n°') == 20
+    resp = resp.click(href=r'\?offset=60')
+    assert '61-66/66' in resp.text
+    assert resp.text.count('Logged Error n°') == 6
+
+    # change pagination
+    resp = app.get('/backoffice/studio/logged-errors/?offset=0&limit=50')
+    assert '1-50/66' in resp.text
+    assert resp.text.count('Logged Error n°') == 50
+    resp = resp.click('<!--Next Page-->')
+    assert '51-66/66' in resp.text
+    assert resp.text.count('Logged Error n°') == 16
+
+    # formdef errors
+    resp = app.get('/backoffice/forms/%s/logged-errors/' % formdef.id)
+    assert '1-20/21' in resp.text
+    assert resp.text.count('FormDef Workflow Logged Error n°') == 20
+    resp = resp.click('<!--Next Page-->')
+    assert '21-21/21' in resp.text
+    assert resp.text.count('FormDef Workflow Logged Error n°') == 1
+
+    # carddef errors
+    resp = app.get('/backoffice/cards/%s/logged-errors/' % carddef.id)
+    assert '1-20/21' in resp.text
+    assert resp.text.count('CardDef Workflow Logged Error n°') == 20
+    resp = resp.click('<!--Next Page-->')
+    assert '21-21/21' in resp.text
+    assert resp.text.count('CardDef Workflow Logged Error n°') == 1
+
+    # workflows errors
+    resp = app.get('/backoffice/workflows/%s/logged-errors/' % workflow.id)
+    assert '1-20/63' in resp.text
+    assert resp.text.count('Workflow Logged Error n°') == 20
+    resp = resp.click(href=r'\?offset=60')
+    assert '61-63/63' in resp.text
+    assert resp.text.count('Workflow Logged Error n°') == 3
+
+
+def test_backoffice_access(pub):
+    pub.loggederror_class.wipe()
+
+    FormDef.wipe()
+    CardDef.wipe()
+    Workflow.wipe()
+
+    formdef = FormDef()
+    formdef.name = 'foo'
+    formdef.store()
+    carddef = CardDef()
+    carddef.name = 'bar'
+    carddef.store()
+    workflow = Workflow()
+    workflow.name = 'blah'
+    workflow.store()
+
+    # FormDef error
+    error1 = pub.loggederror_class()
+    error1.summary = 'LoggedError'
+    error1.formdef_class = 'FormDef'
+    error1.formdef_id = formdef.id
+    error1.workflow_id = workflow.id
+    error1.first_occurence_timestamp = datetime.datetime.now()
+    error1.store()
+
+    # CardDef error
+    error2 = pub.loggederror_class()
+    error2.summary = 'LoggedError'
+    error2.formdef_class = 'CardDef'
+    error2.formdef_id = carddef.id
+    error2.workflow_id = workflow.id
+    error2.first_occurence_timestamp = datetime.datetime.now()
+    error2.store()
+
+    # workflow-only error
+    error3 = pub.loggederror_class()
+    error3.summary = 'LoggedError'
+    error3.workflow_id = workflow.id
+    error3.first_occurence_timestamp = datetime.datetime.now()
+    error3.store()
+
+    create_superuser(pub)
+    app = login(get_app(pub))
+
+    # check section link are not displayed if user has no access right
+
+    # formdefs are not accessible to current user
+    pub.cfg['admin-permissions'] = {'forms': ['X']}
+    pub.write_cfg()
+    resp = app.get('/backoffice/studio/logged-errors/')
+    assert resp.text.count('LoggedError') == 2
+    assert '<a href="%s/">' % error1.id not in resp.text
+    assert '<a href="%s/">' % error2.id in resp.text
+    assert '<a href="%s/">' % error3.id in resp.text
+
+    # carddefs are not accessible to current user
+    pub.cfg['admin-permissions'] = {'cards': ['X']}
+    pub.write_cfg()
+    resp = app.get('/backoffice/studio/logged-errors/')
+    assert resp.text.count('LoggedError') == 2
+    assert '<a href="%s/">' % error1.id in resp.text
+    assert '<a href="%s/">' % error2.id not in resp.text
+    assert '<a href="%s/">' % error3.id in resp.text
+
+    # workflows are not accessible to current user
+    pub.cfg['admin-permissions'] = {'workflows': ['X']}
+    pub.write_cfg()
+    resp = app.get('/backoffice/studio/logged-errors/')
+    assert resp.text.count('LoggedError') == 2
+    assert '<a href="%s/">' % error1.id in resp.text
+    assert '<a href="%s/">' % error2.id in resp.text
+    assert '<a href="%s/">' % error3.id not in resp.text
+
+    # mix formdefs & workflows
+    pub.cfg['admin-permissions'] = {'forms': ['X'], 'workflows': ['X']}
+    pub.write_cfg()
+    resp = app.get('/backoffice/studio/logged-errors/')
+    assert resp.text.count('LoggedError') == 1
+    assert '<a href="%s/">' % error1.id not in resp.text
+    assert '<a href="%s/">' % error2.id in resp.text
+    assert '<a href="%s/">' % error3.id not in resp.text
+
+    # mix all
+    pub.cfg['admin-permissions'] = {'forms': ['X'], 'cards': ['X'], 'workflows': ['X']}
+    pub.write_cfg()
+    resp = app.get('/backoffice/studio/logged-errors/', status=403)
diff --git a/wcs/admin/logged_errors.py b/wcs/admin/logged_errors.py
index c0f25b68..40da5b81 100644
--- a/wcs/admin/logged_errors.py
+++ b/wcs/admin/logged_errors.py
@@ -17,12 +17,14 @@
 import re
 
 from django.utils.text import Truncator
-from quixote import get_response, get_publisher, redirect
+from quixote import get_response, get_publisher, redirect, get_request
 from quixote.directory import Directory
 from quixote.html import TemplateIO, htmltext
 from wcs.qommon import _, ngettext, N_, template
-from wcs.qommon import errors, get_cfg
-from wcs.qommon.misc import localstrftime
+from wcs.qommon import errors
+from wcs.qommon import misc
+from wcs.qommon.backoffice.listing import pagination_links
+from wcs.qommon.storage import Or, Equal, NotEqual, Null, NotNull
 
 
 class LoggedErrorDirectory(Directory):
@@ -113,24 +115,48 @@ class LoggedErrorsDirectory(Directory):
     _q_exports = ['']
 
     @classmethod
-    def get_errors(cls, formdef_class=None, formdef_id=None, workflow_id=None):
+    def get_errors(cls, offset, limit, formdef_class=None, formdef_id=None, workflow_id=None, with_total=False):
         errors = []
         if not get_publisher().loggederror_class:
-            return errors
+            return errors, 0
+
+        select_kwargs = {
+            'order_by': '-first_occurence_timestamp',
+            'limit': limit,
+            'offset': offset,
+        }
+        clauses = []
+
         if formdef_id and formdef_class:
-            errors = [
-                e for e in get_publisher().loggederror_class.get_with_indexed_value('formdef_id', formdef_id)
-                if e.formdef_class == formdef_class.__name__]
+            clauses = [Equal('formdef_id', formdef_id), Equal('formdef_class', formdef_class.__name__)]
         elif workflow_id:
-            errors = get_publisher().loggederror_class.get_with_indexed_value('workflow_id', workflow_id)
-        return list(errors)
+            clauses = [Equal('workflow_id', workflow_id)]
+        else:
+            # check permissions, exclude errors related to not accessible items
+            clauses = []
+            backoffice_root = get_publisher().get_backoffice_root()
+            if not backoffice_root.is_accessible('forms'):
+                clauses.append(Or([NotEqual('formdef_class', 'FormDef'), Null('formdef_class')]))
+            if not backoffice_root.is_accessible('cards'):
+                clauses.append(Or([NotEqual('formdef_class', 'CardDef'), Null('formdef_class')]))
+            if not backoffice_root.is_accessible('workflows'):
+                # exclude workflow-only errors
+                clauses.append(NotNull('formdef_class'))
+
+        errors = get_publisher().loggederror_class.select(clause=clauses, **select_kwargs)
+
+        count = 0
+        if with_total:
+            count = get_publisher().loggederror_class.count(clauses)
+
+        return list(errors), count
 
     @classmethod
     def errors_block(cls, formdef_class=None, formdef_id=None, workflow_id=None):
-        errors = cls.get_errors(formdef_class=formdef_class, formdef_id=formdef_id, workflow_id=workflow_id)
+        # select 3 + 1 last errors
+        errors = cls.get_errors(offset=0, limit=4, formdef_class=formdef_class, formdef_id=formdef_id, workflow_id=workflow_id)[0]
         if not errors:
             return ''
-        errors.sort(key=lambda x: x.id, reverse=True)
 
         r = TemplateIO(html=True)
         r += htmltext('<div class="bo-block logged-errors">')
@@ -160,12 +186,29 @@ class LoggedErrorsDirectory(Directory):
         self.workflow_id = workflow_id
 
     def _q_index(self):
+        backoffice_root = get_publisher().get_backoffice_root()
+        if not (backoffice_root.is_accessible('forms') or
+                backoffice_root.is_accessible('cards') or
+                backoffice_root.is_accessible('workflows')):
+            raise errors.AccessForbiddenError()
+
         get_response().breadcrumb.append(('logged-errors/', _('Logged Errors')))
         self.parent_dir.html_top(_('Logged Errors'))
+        limit = misc.get_int_or_400(
+            get_request().form.get('limit', get_publisher().get_site_option('default-page-size')) or 20)
+        offset = misc.get_int_or_400(get_request().form.get('offset', 0))
+        logged_errors, total_count = self.get_errors(
+            offset=offset, limit=limit,
+            formdef_class=self.formdef_class, formdef_id=self.formdef_id, workflow_id=self.workflow_id,
+            with_total=True)
+        links = ''
+        if get_publisher().is_using_postgresql():
+            links = pagination_links(offset, limit, total_count, load_js=False)
         return template.QommonTemplateResponse(
                 templates=['wcs/backoffice/logged-errors.html'],
                 context={
-                    'errors': self.get_errors(formdef_class=self.formdef_class, formdef_id=self.formdef_id, workflow_id=self.workflow_id),
+                    'errors': logged_errors,
+                    'pagination_links': links,
                 })
 
     def _q_lookup(self, component):
diff --git a/wcs/backoffice/studio.py b/wcs/backoffice/studio.py
index e49103d2..5e3f75a7 100644
--- a/wcs/backoffice/studio.py
+++ b/wcs/backoffice/studio.py
@@ -16,16 +16,38 @@
 
 from quixote import get_publisher
 from quixote.directory import Directory
-from ..qommon import _
-from ..qommon.backoffice.menu import html_top
-from ..qommon import template
+from quixote.html import TemplateIO, htmltext
+
+from wcs.admin.logged_errors import LoggedErrorsDirectory
+from wcs.qommon import _
+from wcs.qommon import template
+from wcs.qommon.backoffice.menu import html_top
+from wcs.qommon.form import get_response
 
 
 class StudioDirectory(Directory):
-    _q_exports = ['']
+    _q_exports = ['', ('logged-errors', 'logged_errors_dir')]
+
+    def __init__(self):
+        self.logged_errors_dir = LoggedErrorsDirectory(parent_dir=self)
+
+    def get_sidebar(self):
+        r = TemplateIO(html=True)
+        r += htmltext('<ul id="sidebar-actions">')
+        r += htmltext('<li><a href="logged-errors/">%s</a></li>') % _('Logged Errors')
+        r += htmltext('</ul>')
+        return r.getvalue()
+
+    def html_top(self, title):
+        return html_top('studio', title)
+
+    def _q_traverse(self, path):
+        get_response().breadcrumb.append(('studio/', _('Studio')))
+        return super()._q_traverse(path)
 
     def _q_index(self):
-        html_top('studio', _('Studio'))
+        self.html_top(_('Studio'))
+        get_response().filter['sidebar'] = self.get_sidebar()
         return template.QommonTemplateResponse(
                 templates=['wcs/backoffice/studio.html'],
                 context={})
diff --git a/wcs/qommon/backoffice/listing.py b/wcs/qommon/backoffice/listing.py
index 162c0c86..823d4b7d 100644
--- a/wcs/qommon/backoffice/listing.py
+++ b/wcs/qommon/backoffice/listing.py
@@ -21,9 +21,10 @@ from quixote import get_request, get_response
 from .. import _
 
 
-def pagination_links(offset, limit, total_count):
+def pagination_links(offset, limit, total_count, load_js=True):
     limit = limit or 10  # make sure a limit is set
-    get_response().add_javascript(['wcs.listing.js'])
+    if load_js:
+        get_response().add_javascript(['wcs.listing.js'])
     # pagination
     r = TemplateIO(html=True)
     r += htmltext('<div id="page-links">')
diff --git a/wcs/sql.py b/wcs/sql.py
index 6fd85d91..7b6e63f5 100644
--- a/wcs/sql.py
+++ b/wcs/sql.py
@@ -1185,7 +1185,7 @@ class SqlMixin(object):
 
     @classmethod
     @guard_postgres
-    def get_with_indexed_value(cls, index, value, ignore_errors = False):
+    def get_with_indexed_value(cls, index, value, ignore_errors=False, order_by=None, limit=None, offset=None):
         conn, cur = get_connection_and_cursor()
         sql_statement = '''SELECT %s
                              FROM %s
@@ -1194,7 +1194,15 @@ class SqlMixin(object):
                                               + cls.get_data_fields()),
                                     cls._table_name,
                                     index)
-        cur.execute(sql_statement, {'value': str(value)})
+        sql_statement += cls.get_order_by_clause(order_by)
+        parameters = {'value': str(value)}
+        if limit:
+            sql_statement += ' LIMIT %(limit)s'
+            parameters['limit'] = limit
+        if offset:
+            sql_statement += ' OFFSET %(offset)s'
+            parameters['offset'] = offset
+        cur.execute(sql_statement, parameters)
         try:
             while True:
                 row = cur.fetchone()
diff --git a/wcs/templates/wcs/backoffice/logged-errors.html b/wcs/templates/wcs/backoffice/logged-errors.html
index 60e9c2b4..cac56732 100644
--- a/wcs/templates/wcs/backoffice/logged-errors.html
+++ b/wcs/templates/wcs/backoffice/logged-errors.html
@@ -13,4 +13,5 @@
           </a><span class="badge">{{ error.occurences_count }}</span></li>
   {% endfor %}
 </ul>
+{{ pagination_links|safe }}
 {% endblock %}
-- 
2.28.0

