From c3e395753cd36c329eedd798da9a975deeffb588 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 12 Nov 2018 15:33:14 -0500 Subject: [PATCH 01/11] Add pagination to AuditLog.get_all_events --- atst/domain/audit_log.py | 9 +++++---- atst/domain/common/query.py | 35 ++++++++++++++++++++++++++++++++++ tests/domain/test_audit_log.py | 9 +++++++++ 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/atst/domain/audit_log.py b/atst/domain/audit_log.py index 238cff2e..4e7abca9 100644 --- a/atst/domain/audit_log.py +++ b/atst/domain/audit_log.py @@ -8,8 +8,9 @@ class AuditEventQuery(Query): model = AuditEvent @classmethod - def get_all(cls): - return db.session.query(cls.model).order_by(cls.model.time_created.desc()).all() + def get_all(cls, pagination): + query = db.session.query(cls.model).order_by(cls.model.time_created.desc()) + return cls.paginate(query, pagination) class AuditLog(object): @@ -28,11 +29,11 @@ class AuditLog(object): return cls._log(resource=resource, action=action) @classmethod - def get_all_events(cls, user): + def get_all_events(cls, user, pagination=None): Authorization.check_atat_permission( user, Permissions.VIEW_AUDIT_LOG, "view audit log" ) - return AuditEventQuery.get_all() + return AuditEventQuery.get_all(pagination) @classmethod def _resource_type(cls, resource): diff --git a/atst/domain/common/query.py b/atst/domain/common/query.py index 4f55d6c0..8f554268 100644 --- a/atst/domain/common/query.py +++ b/atst/domain/common/query.py @@ -5,6 +5,37 @@ from atst.domain.exceptions import NotFoundError from atst.database import db +class Paginator(object): + """ + Uses the Flask-SQLAlchemy extension's pagination method to paginate + a query set. + + Also acts as a proxy object so that the results of the query set can be iterated + over without needing to call `.items`. + """ + + def __init__(self, query_set): + self.query_set = query_set + + @classmethod + def paginate(cls, query, pagination=None): + if pagination is not None: + return cls( + query.paginate(page=pagination["page"], per_page=pagination["per_page"]) + ) + else: + return query.all() + + def __getattr__(self, name): + return getattr(self.query_set, name) + + def __iter__(self): + return self.items.__iter__() + + def __len__(self): + return self.items.__len__() + + class Query(object): model = None @@ -35,3 +66,7 @@ class Query(object): db.session.add(resource) db.session.commit() return resource + + @classmethod + def paginate(cls, query, pagination): + return Paginator.paginate(query, pagination) diff --git a/tests/domain/test_audit_log.py b/tests/domain/test_audit_log.py index 1a2d95f7..387da03c 100644 --- a/tests/domain/test_audit_log.py +++ b/tests/domain/test_audit_log.py @@ -22,3 +22,12 @@ def test_non_admin_cannot_view_audit_log(developer): def test_ccpo_can_iview_audit_log(ccpo): AuditLog.get_all_events(ccpo) + + +def test_paginate_audit_log(ccpo): + user = UserFactory.create() + for _ in range(100): + AuditLog.log_system_event(user, action="create") + + events = AuditLog.get_all_events(ccpo, pagination={"per_page": 25, "page": 2}) + assert len(events) == 25 From 5b0383bde3013e55b4ee5519bc91637926bff1f3 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 12 Nov 2018 15:33:33 -0500 Subject: [PATCH 02/11] Simple pagination for /activity-history --- atst/routes/__init__.py | 10 +++++++++- templates/audit_log.html | 8 ++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index 780ff0c3..1885f5c7 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -121,9 +121,17 @@ def logout(): return redirect(url_for(".root")) +def pagination_info(request, default_page=1, default_per_page=50): + return { + "page": int(request.args.get("page", default_page)), + "per_page": int(request.args.get("per_page", default_per_page)), + } + + @bp.route("/activity-history") def activity_history(): - audit_events = AuditLog.get_all_events(g.current_user) + pagination = pagination_info(request) + audit_events = AuditLog.get_all_events(g.current_user, pagination) return render_template("audit_log.html", audit_events=audit_events) diff --git a/templates/audit_log.html b/templates/audit_log.html index 7010c0ad..b5c8fc1d 100644 --- a/templates/audit_log.html +++ b/templates/audit_log.html @@ -40,5 +40,13 @@ + {% for i in range(1, audit_events.pages + 1) %} + {% if i == audit_events.page %} + {{ i }} + {% else %} + {{ i }} + {% endif %} + {% endfor %} + {% endblock %} From 4244ecf9b7c4b442a958854565e41873d8c0d111 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 12 Nov 2018 16:45:55 -0500 Subject: [PATCH 03/11] Rename pagination args to pagination_opts --- atst/domain/audit_log.py | 8 ++++---- atst/domain/common/query.py | 12 +++++++----- atst/routes/__init__.py | 6 +++--- tests/domain/test_audit_log.py | 2 +- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/atst/domain/audit_log.py b/atst/domain/audit_log.py index 4e7abca9..b5f8522c 100644 --- a/atst/domain/audit_log.py +++ b/atst/domain/audit_log.py @@ -8,9 +8,9 @@ class AuditEventQuery(Query): model = AuditEvent @classmethod - def get_all(cls, pagination): + def get_all(cls, pagination_opts): query = db.session.query(cls.model).order_by(cls.model.time_created.desc()) - return cls.paginate(query, pagination) + return cls.paginate(query, pagination_opts) class AuditLog(object): @@ -29,11 +29,11 @@ class AuditLog(object): return cls._log(resource=resource, action=action) @classmethod - def get_all_events(cls, user, pagination=None): + def get_all_events(cls, user, pagination_opts=None): Authorization.check_atat_permission( user, Permissions.VIEW_AUDIT_LOG, "view audit log" ) - return AuditEventQuery.get_all(pagination) + return AuditEventQuery.get_all(pagination_opts) @classmethod def _resource_type(cls, resource): diff --git a/atst/domain/common/query.py b/atst/domain/common/query.py index 8f554268..07db46ef 100644 --- a/atst/domain/common/query.py +++ b/atst/domain/common/query.py @@ -18,10 +18,12 @@ class Paginator(object): self.query_set = query_set @classmethod - def paginate(cls, query, pagination=None): - if pagination is not None: + def paginate(cls, query, pagination_opts=None): + if pagination_opts is not None: return cls( - query.paginate(page=pagination["page"], per_page=pagination["per_page"]) + query.paginate( + page=pagination_opts["page"], per_page=pagination_opts["per_page"] + ) ) else: return query.all() @@ -68,5 +70,5 @@ class Query(object): return resource @classmethod - def paginate(cls, query, pagination): - return Paginator.paginate(query, pagination) + def paginate(cls, query, pagination_opts): + return Paginator.paginate(query, pagination_opts) diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index 1885f5c7..e6b2b57a 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -121,7 +121,7 @@ def logout(): return redirect(url_for(".root")) -def pagination_info(request, default_page=1, default_per_page=50): +def get_pagination_opts(request, default_page=1, default_per_page=50): return { "page": int(request.args.get("page", default_page)), "per_page": int(request.args.get("per_page", default_per_page)), @@ -130,8 +130,8 @@ def pagination_info(request, default_page=1, default_per_page=50): @bp.route("/activity-history") def activity_history(): - pagination = pagination_info(request) - audit_events = AuditLog.get_all_events(g.current_user, pagination) + pagination_opts = get_pagination_opts(request) + audit_events = AuditLog.get_all_events(g.current_user, pagination_opts) return render_template("audit_log.html", audit_events=audit_events) diff --git a/tests/domain/test_audit_log.py b/tests/domain/test_audit_log.py index 387da03c..ab03c48c 100644 --- a/tests/domain/test_audit_log.py +++ b/tests/domain/test_audit_log.py @@ -29,5 +29,5 @@ def test_paginate_audit_log(ccpo): for _ in range(100): AuditLog.log_system_event(user, action="create") - events = AuditLog.get_all_events(ccpo, pagination={"per_page": 25, "page": 2}) + events = AuditLog.get_all_events(ccpo, pagination_opts={"per_page": 25, "page": 2}) assert len(events) == 25 From 7753167b3fd9ebbd672dce2bd327627d4f9c2ebd Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 13 Nov 2018 14:37:50 -0500 Subject: [PATCH 04/11] WIP styling --- styles/components/_audit_log.scss | 12 ++++++++++++ templates/audit_log.html | 21 +++++++++++++-------- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/styles/components/_audit_log.scss b/styles/components/_audit_log.scss index 212ebb83..2cea4260 100644 --- a/styles/components/_audit_log.scss +++ b/styles/components/_audit_log.scss @@ -20,3 +20,15 @@ margin-bottom: $gap; } } + +.pagination { + width: 800px; + display: flex; + flex-direction: row; + align-content: space-between; + margin: auto; +} + +.page { + margin: auto; +} diff --git a/templates/audit_log.html b/templates/audit_log.html index b5c8fc1d..76afa6e3 100644 --- a/templates/audit_log.html +++ b/templates/audit_log.html @@ -40,13 +40,18 @@ - {% for i in range(1, audit_events.pages + 1) %} - {% if i == audit_events.page %} - {{ i }} - {% else %} - {{ i }} - {% endif %} - {% endfor %} - + {% set page_route = 'atst.activity_history' %} + {% set pagination = audit_events %} + {% endblock %} From ccb76f3601fe59c6e0176f299d31a8a2ff4d062c Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 13 Nov 2018 17:06:29 -0500 Subject: [PATCH 05/11] Show relevant pages --- atst/filters.py | 14 ++++++++++ atst/routes/__init__.py | 2 +- templates/audit_log.html | 58 +++++++++++++++++++++++++++++++--------- 3 files changed, 60 insertions(+), 14 deletions(-) diff --git a/atst/filters.py b/atst/filters.py index c9aa682f..03526529 100644 --- a/atst/filters.py +++ b/atst/filters.py @@ -76,6 +76,19 @@ def dateFromString(value, formatter="%m/%Y"): return datetime.datetime.strptime(value, formatter) +def pageWindow(pagination, size=2): + page = pagination.page + num_pages = pagination.pages + + over = max(0, page + size - num_pages) + under = min(0, page - size - 1) + + return ( + max(1, (page - size) - over), + min(num_pages, (page + size) - under) + ) + + def register_filters(app): app.jinja_env.filters["iconSvg"] = iconSvg app.jinja_env.filters["dollars"] = dollars @@ -87,3 +100,4 @@ def register_filters(app): app.jinja_env.filters["renderList"] = renderList app.jinja_env.filters["formattedDate"] = formattedDate app.jinja_env.filters["dateFromString"] = dateFromString + app.jinja_env.filters["pageWindow"] = pageWindow diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index e6b2b57a..c53c6a88 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -124,7 +124,7 @@ def logout(): def get_pagination_opts(request, default_page=1, default_per_page=50): return { "page": int(request.args.get("page", default_page)), - "per_page": int(request.args.get("per_page", default_per_page)), + "per_page": int(request.args.get("perPage", default_per_page)), } diff --git a/templates/audit_log.html b/templates/audit_log.html index 76afa6e3..696368ff 100644 --- a/templates/audit_log.html +++ b/templates/audit_log.html @@ -40,18 +40,50 @@ - {% set page_route = 'atst.activity_history' %} - {% set pagination = audit_events %} + {% macro Page(pagination, route, i, label=None) -%} + {% set label = label or i %} - + {% if i == pagination.page %} + + {% else %} + {{ label }} + {% endif%} + {%- endmacro %} + + {% macro Pagination(pagination, route) -%} + {% set first %} + {{ Page(pagination, route, 1, label="first") }} + {{ Page(pagination, route, pagination.page - 1, label="prev") }} + {% endset %} + + {% set last %} + {{ Page(pagination, route, pagination.page + 1, label="next") }} + {{ Page(pagination, route, pagination.pages, label="last") }} + {% endset %} + + + {%- endmacro %} + {{ Pagination(audit_events, 'atst.activity_history') }} {% endblock %} From 3555cae78c49a15e96549f54066bf27ac05e5685 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 13 Nov 2018 17:12:28 -0500 Subject: [PATCH 06/11] Give Pagination its own component --- templates/audit_log.html | 46 +--------------------------- templates/components/pagination.html | 45 +++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 45 deletions(-) create mode 100644 templates/components/pagination.html diff --git a/templates/audit_log.html b/templates/audit_log.html index 696368ff..8455276f 100644 --- a/templates/audit_log.html +++ b/templates/audit_log.html @@ -1,4 +1,5 @@ {% extends "base.html" %} +{% from "components/pagination.html" import Pagination %} {% block content %} @@ -40,50 +41,5 @@ - {% macro Page(pagination, route, i, label=None) -%} - {% set label = label or i %} - - {% if i == pagination.page %} - - {% else %} - {{ label }} - {% endif%} - {%- endmacro %} - - {% macro Pagination(pagination, route) -%} - {% set first %} - {{ Page(pagination, route, 1, label="first") }} - {{ Page(pagination, route, pagination.page - 1, label="prev") }} - {% endset %} - - {% set last %} - {{ Page(pagination, route, pagination.page + 1, label="next") }} - {{ Page(pagination, route, pagination.pages, label="last") }} - {% endset %} - - - {%- endmacro %} {{ Pagination(audit_events, 'atst.activity_history') }} {% endblock %} diff --git a/templates/components/pagination.html b/templates/components/pagination.html new file mode 100644 index 00000000..9f45c44e --- /dev/null +++ b/templates/components/pagination.html @@ -0,0 +1,45 @@ +{% macro Page(pagination, route, i, label=None) -%} + {% set label = label or i %} + + {% if i == pagination.page %} + + {% else %} + {{ label }} + {% endif%} +{%- endmacro %} + +{% macro Pagination(pagination, route) -%} + {% set first %} + {{ Page(pagination, route, 1, label="first") }} + {{ Page(pagination, route, pagination.page - 1, label="prev") }} + {% endset %} + + {% set last %} + {{ Page(pagination, route, pagination.page + 1, label="next") }} + {{ Page(pagination, route, pagination.pages, label="last") }} + {% endset %} + + +{%- endmacro %} From d0c923bf9bf90daac6cae277869477d5be0f9d11 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 13 Nov 2018 17:21:42 -0500 Subject: [PATCH 07/11] Formatting --- atst/filters.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/atst/filters.py b/atst/filters.py index 03526529..c172a3df 100644 --- a/atst/filters.py +++ b/atst/filters.py @@ -83,10 +83,7 @@ def pageWindow(pagination, size=2): over = max(0, page + size - num_pages) under = min(0, page - size - 1) - return ( - max(1, (page - size) - over), - min(num_pages, (page + size) - under) - ) + return (max(1, (page - size) - over), min(num_pages, (page + size) - under)) def register_filters(app): From 50b57507ed2983e99459867d213d12a701c27dff Mon Sep 17 00:00:00 2001 From: richard-dds Date: Wed, 14 Nov 2018 15:29:51 -0500 Subject: [PATCH 08/11] Styling tweaks --- styles/components/_audit_log.scss | 5 +++-- templates/components/pagination.html | 33 +++++++++++++++++++--------- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/styles/components/_audit_log.scss b/styles/components/_audit_log.scss index 2cea4260..624714a1 100644 --- a/styles/components/_audit_log.scss +++ b/styles/components/_audit_log.scss @@ -22,13 +22,14 @@ } .pagination { - width: 800px; + width: 80%; display: flex; flex-direction: row; - align-content: space-between; margin: auto; } .page { margin: auto; + flex-grow: 1; + max-width: 8%; } diff --git a/templates/components/pagination.html b/templates/components/pagination.html index 9f45c44e..ebca6b3a 100644 --- a/templates/components/pagination.html +++ b/templates/components/pagination.html @@ -1,11 +1,17 @@ -{% macro Page(pagination, route, i, label=None) -%} +{% macro Page(pagination, route, i, label=None, disabled=False) -%} {% set label = label or i %} - {% if i == pagination.page %} - + {% set button_class = "page usa-button " %} + + {% if disabled %} + {% set button_class = button_class + "usa-button-disabled" %} + {% elif i == pagination.page %} + {% set button_class = button_class + "usa-button-primary" %} {% else %} - {{ label }} - {% endif%} + {% set button_class = button_class + "usa-button-secondary" %} + {% endif %} + + {{ label }} {%- endmacro %} {% macro Pagination(pagination, route) -%} @@ -23,23 +29,30 @@ {% if pagination.page == 1 %} {% set max_page = [pagination.pages, 5] | min %} - + {{ Page(pagination, route, 1, label="first", disabled=True) }} + {{ Page(pagination, route, pagination.page - 1, label="prev", disabled=True) }} {% for i in range(1, max_page + 1) %} {{ Page(pagination, route, i) }} {% endfor %} - {{ last }} + {{ Page(pagination, route, pagination.page + 1, label="next") }} + {{ Page(pagination, route, pagination.pages, label="last") }} {% elif pagination.page == pagination.pages %} - {{ first }} + {{ Page(pagination, route, 1, label="first") }} + {{ Page(pagination, route, pagination.page - 1, label="prev") }} {% for i in range(pagination.pages - 4, pagination.pages + 1) %} {{ Page(pagination, route, i) }} {% endfor %} + {{ Page(pagination, route, pagination.page + 1, label="next", disabled=True) }} + {{ Page(pagination, route, pagination.pages, label="last", disabled=True) }} {% else %} {% set window = pagination | pageWindow %} - {{ first }} + {{ Page(pagination, route, 1, label="first", disabled=True) }} + {{ Page(pagination, route, pagination.page - 1, label="prev", disabled=True) }} {% for i in range(window.0, window.1 + 1) %} {{ Page(pagination, route, i) }} {% endfor %} - {{ last }} + {{ Page(pagination, route, pagination.page + 1, label="next") }} + {{ Page(pagination, route, pagination.pages, label="last") }} {% endif %} {%- endmacro %} From ee86ea2b1ad0712131070db838bd110e038e1d83 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Wed, 14 Nov 2018 15:35:36 -0500 Subject: [PATCH 09/11] Show 100 entries per page --- atst/routes/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index c53c6a88..9fd690a9 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -121,7 +121,7 @@ def logout(): return redirect(url_for(".root")) -def get_pagination_opts(request, default_page=1, default_per_page=50): +def get_pagination_opts(request, default_page=1, default_per_page=100): return { "page": int(request.args.get("page", default_page)), "per_page": int(request.args.get("perPage", default_per_page)), From 5c7907efe513e3772dd013e71d9a12b5d87cc681 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Wed, 14 Nov 2018 17:09:13 -0500 Subject: [PATCH 10/11] Fix bug preventing 'disabled' style from rendering --- templates/components/pagination.html | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/templates/components/pagination.html b/templates/components/pagination.html index ebca6b3a..5b1c84a9 100644 --- a/templates/components/pagination.html +++ b/templates/components/pagination.html @@ -11,20 +11,10 @@ {% set button_class = button_class + "usa-button-secondary" %} {% endif %} - {{ label }} + {{ label }} {%- endmacro %} {% macro Pagination(pagination, route) -%} - {% set first %} - {{ Page(pagination, route, 1, label="first") }} - {{ Page(pagination, route, pagination.page - 1, label="prev") }} - {% endset %} - - {% set last %} - {{ Page(pagination, route, pagination.page + 1, label="next") }} - {{ Page(pagination, route, pagination.pages, label="last") }} - {% endset %} -