From 99482f9962de96de0d5b248f0ee99cdd15a6a740 Mon Sep 17 00:00:00 2001 From: Kevin Morris Date: Fri, 10 Sep 2021 13:28:11 -0700 Subject: [PATCH] feat(FastAPI): added /requests (get) route Introduces `aurweb.defaults` and `aurweb.filters`. `aurweb.filters` is a location developers can put their additional Jinja2 filters and/or functions. We should slowly move all of our filters over here, where it makes sense. `aurweb.defaults` is a new module which hosts some default constants and utility functions, starting with offsets (O) and per page values (PP). As far as the new GET /requests is concerned, we match up here to PHP's implementation, with some minor improvements: Improvements: * PP on this page is now configurable: 50 (default), 100, or 250. * Example: `https://localhost:8444/requests?PP=250` Modifications: * The pagination is a bit different, but serves the exact same purpose. * "Last" no longer goes to an empty page. * Closes: https://gitlab.archlinux.org/archlinux/aurweb/-/issues/14 Signed-off-by: Kevin Morris --- aurweb/auth.py | 15 +++++ aurweb/defaults.py | 18 ++++++ aurweb/filters.py | 14 ++++- aurweb/routers/packages.py | 40 ++++++++++-- aurweb/templates.py | 15 +++++ setup.cfg | 19 +++--- templates/requests.html | 115 +++++++++++++++++++++++++++++++++++ test/test_defaults.py | 14 +++++ test/test_packages_routes.py | 84 ++++++++++++++++++++++++- test/test_templates.py | 16 ++++- test/test_util.py | 8 ++- 11 files changed, 341 insertions(+), 17 deletions(-) create mode 100644 aurweb/defaults.py create mode 100644 templates/requests.html create mode 100644 test/test_defaults.py diff --git a/aurweb/auth.py b/aurweb/auth.py index d1a9d9cb..21d31081 100644 --- a/aurweb/auth.py +++ b/aurweb/auth.py @@ -31,8 +31,23 @@ class StubQuery: class AnonymousUser: + """ A stubbed User class used when an unauthenticated User + makes a request against FastAPI. """ # Stub attributes used to mimic a real user. ID = 0 + + class AccountType: + """ A stubbed AccountType static class. In here, we use an ID + and AccountType which do not exist in our constant records. + All records primary keys (AccountType.ID) should be non-zero, + so using a zero here means that we'll never match against a + real AccountType. """ + ID = 0 + AccountType = "Anonymous" + + # AccountTypeID == AccountType.ID; assign a stubbed column. + AccountTypeID = AccountType.ID + LangPreference = aurweb.config.get("options", "default_lang") Timezone = aurweb.config.get("options", "default_timezone") diff --git a/aurweb/defaults.py b/aurweb/defaults.py new file mode 100644 index 00000000..c2568d05 --- /dev/null +++ b/aurweb/defaults.py @@ -0,0 +1,18 @@ +""" Constant default values centralized in one place. """ + +# Default [O]ffset +O = 0 + +# Default [P]er [P]age +PP = 50 + +# A whitelist of valid PP values +PP_WHITELIST = {50, 100, 250} + + +def fallback_pp(per_page: int) -> int: + """ If `per_page` is a valid value in PP_WHITELIST, return it. + Otherwise, return defaults.PP. """ + if per_page not in PP_WHITELIST: + return PP + return per_page diff --git a/aurweb/filters.py b/aurweb/filters.py index bb56c656..f9f56b5d 100644 --- a/aurweb/filters.py +++ b/aurweb/filters.py @@ -4,8 +4,8 @@ import paginate from jinja2 import pass_context -from aurweb import util -from aurweb.templates import register_filter +from aurweb import config, util +from aurweb.templates import register_filter, register_function @register_filter("pager_nav") @@ -48,3 +48,13 @@ def pager_nav(context: Dict[str, Any], symbol_previous="‹ Previous", symbol_next="Next ›", symbol_last="Last »") + + +@register_function("config_getint") +def config_getint(section: str, key: str) -> int: + return config.getint(section, key) + + +@register_function("round") +def do_round(f: float) -> int: + return round(f) diff --git a/aurweb/routers/packages.py b/aurweb/routers/packages.py index 385d91db..5751a3ee 100644 --- a/aurweb/routers/packages.py +++ b/aurweb/routers/packages.py @@ -2,17 +2,18 @@ from datetime import datetime from http import HTTPStatus from typing import Any, Dict -from fastapi import APIRouter, Form, HTTPException, Request, Response +from fastapi import APIRouter, Form, HTTPException, Query, Request, Response from fastapi.responses import JSONResponse, RedirectResponse -from sqlalchemy import and_ +from sqlalchemy import and_, case import aurweb.filters import aurweb.models.package_comment import aurweb.models.package_keyword import aurweb.packages.util -from aurweb import db, l10n -from aurweb.auth import auth_required +from aurweb import db, defaults, l10n +from aurweb.auth import account_type_required, auth_required +from aurweb.models.account_type import DEVELOPER, TRUSTED_USER, TRUSTED_USER_AND_DEV from aurweb.models.license import License from aurweb.models.package import Package from aurweb.models.package_base import PackageBase @@ -22,10 +23,11 @@ from aurweb.models.package_dependency import PackageDependency from aurweb.models.package_license import PackageLicense from aurweb.models.package_notification import PackageNotification from aurweb.models.package_relation import PackageRelation -from aurweb.models.package_request import PackageRequest +from aurweb.models.package_request import PENDING_ID, PackageRequest from aurweb.models.package_source import PackageSource from aurweb.models.package_vote import PackageVote from aurweb.models.relation_type import CONFLICTS_ID +from aurweb.models.request_type import RequestType from aurweb.models.user import User from aurweb.packages.search import PackageSearch from aurweb.packages.util import get_pkg_or_base, get_pkgbase_comment, query_notified, query_voted @@ -535,3 +537,31 @@ async def package_base_comaintainers_post( return RedirectResponse(f"/pkgbase/{pkgbase.Name}", status_code=int(HTTPStatus.SEE_OTHER)) + + +@router.get("/requests") +@account_type_required({TRUSTED_USER, DEVELOPER, TRUSTED_USER_AND_DEV}) +@auth_required(True, redirect="/") +async def requests(request: Request, + O: int = Query(default=defaults.O), + PP: int = Query(default=defaults.PP)): + context = make_context(request, "Requests") + + context["q"] = dict(request.query_params) + context["O"] = O + context["PP"] = PP + + # A PackageRequest query, with left inner joined User and RequestType. + query = db.query(PackageRequest).join( + User, PackageRequest.UsersID == User.ID + ).join(RequestType) + + context["total"] = query.count() + context["results"] = query.order_by( + # Order primarily by the Status column being PENDING_ID, + # and secondarily by RequestTS; both in descending order. + case([(PackageRequest.Status == PENDING_ID, 1)], else_=0).desc(), + PackageRequest.RequestTS.desc() + ).limit(PP).offset(O).all() + + return render_template(request, "requests.html", context) diff --git a/aurweb/templates.py b/aurweb/templates.py index ef020bdf..2301cfe2 100644 --- a/aurweb/templates.py +++ b/aurweb/templates.py @@ -71,6 +71,20 @@ def register_filter(name: str) -> Callable: return decorator +def register_function(name: str) -> Callable: + """ A decorator that can be used to register a function. + """ + def decorator(func): + @functools.wraps(func) + def wrapper(*args, **kwargs): + return func(*args, **kwargs) + if name in _env.globals: + raise KeyError(f"Jinja already has a function named '{name}'") + _env.globals[name] = wrapper + return wrapper + return decorator + + def make_context(request: Request, title: str, next: str = None): """ Create a context for a jinja2 TemplateResponse. """ @@ -83,6 +97,7 @@ def make_context(request: Request, title: str, next: str = None): "timezones": time.SUPPORTED_TIMEZONES, "title": title, "now": datetime.now(tz=zoneinfo.ZoneInfo(timezone)), + "utcnow": int(datetime.utcnow().timestamp()), "config": aurweb.config, "next": next if next else request.url.path } diff --git a/setup.cfg b/setup.cfg index 4f2bdf7d..cec1bcf5 100644 --- a/setup.cfg +++ b/setup.cfg @@ -6,17 +6,22 @@ ignore = E741, W503 max-line-length = 127 max-complexity = 10 -# aurweb/routers/accounts.py # Ignore some unavoidable flake8 warnings; we know this is against -# pycodestyle, but some of the existing codebase uses `I` variables, +# PEP8, but some of the existing codebase uses `I` variables, # so specifically silence warnings about it in pre-defined files. +# # In E741, the 'I', 'O', 'l' are ambiguous variable names. # Our current implementation uses these variables through HTTP # and the FastAPI form specification wants them named as such. -# In C901's case, our process_account_form function is way too -# complex for PEP (too many if statements). However, we need to -# process these anyways, and making it any more complex would -# just add confusion to the implementation. +# +# With {W503,W504}, PEP8 does not want us to break lines before +# or after a binary operator. We have many scripts that already +# do this, so we're ignoring it here. +ignore = E741, W503, W504 + +# aurweb/routers/accounts.py +# Ignore over-reaching complexity. +# TODO: This should actually be addressed so we do not ignore C901. # # test/test_ssh_pub_key.py # E501 is detected due to our >127 width test constant. Ignore it. @@ -24,7 +29,7 @@ max-complexity = 10 # Anything like this should be questioned. # per-file-ignores = - aurweb/routers/accounts.py:E741,C901 + aurweb/routers/accounts.py:C901 test/test_ssh_pub_key.py:E501 aurweb/routers/packages.py:E741 diff --git a/templates/requests.html b/templates/requests.html new file mode 100644 index 00000000..a9017e2f --- /dev/null +++ b/templates/requests.html @@ -0,0 +1,115 @@ +{% extends "partials/layout.html" %} + +{% set singular = "%d package request found." %} +{% set plural = "%d package requests found." %} + +{% block pageContent %} +
+ {% if not total %} +

{{ "No requests matched your search criteria." | tr }}

+ {% else %} + {% include "partials/widgets/pager.html" %} + + + + + + + + + + + + + {% for result in results %} + + + {# Type #} + + {# Comments #} + + + {% set idle_time = config_getint("options", "request_idle_time") %} + {% set time_delta = (utcnow - result.RequestTS) | int %} + + {% set due = result.Status == 0 and time_delta > idle_time %} + + + + {% endfor %} + +
{{ "Package" | tr }}{{ "Type" | tr }}{{ "Comments" | tr }}{{ "Filed by" | tr }}{{ "Date" | tr }}{{ "Status" | tr }}
+ {# Package #} + + {{ result.PackageBaseName }} + + + {{ result.RequestType.name_display() }} + {# If the RequestType is a merge and request.MergeBaseName is valid... #} + {% if result.RequestType.ID == 3 and result.MergeBaseName %} + ({{ result.MergeBaseName }}) + {% endif %} + {{ result.Comments }} + {# Filed by #} + + {{ result.User.Username }} + + + {# Date #} + {% set date = result.RequestTS | dt | as_timezone(timezone) %} + {{ date.strftime("%Y-%m-%d %H:%M") }} + + {# Status #} + {% if result.Status == 0 %} + {% set temp_q = { "next": "/requests" } %} + + {% if result.RequestType.ID == 1 %} + {% set action = "delete" %} + {% elif result.RequestType.ID == 2 %} + {% set action = "disown" %} + {% elif result.RequestType.ID == 3 %} + {% set action = "merge" %} + {# Add the 'via' url query parameter. #} + {% set temp_q = temp_q | extend_query( + ["via", result.ID], + ["into", result.MergeBaseName] + ) %} + {% endif %} + + {% if request.user.is_elevated() %} + {% if result.RequestType.ID == 2 and not due %} + {% set time_left = idle_time - time_delta %} + {% if time_left > 48 * 3600 %} + {% set n = round(time_left / (24 * 3600)) %} + {% set time_left_fmt = (n | tn("~%d day left", "~%d days left") | format(n)) %} + {% elif time_left > 3600 %} + {% set n = round(time_left / 3600) %} + {% set time_left_fmt = (n | tn("~%d hour left", "~%d hours left") | format(n)) %} + {% else %} + {% set time_left_fmt = ("<1 hour left" | tr) %} + {% endif %} + {{ "Locked" | tr }} + ({{ time_left_fmt }}) + {% else %} + {# Only elevated users (TU or Dev) are allowed to accept requests. #} + + {{ "Accept" | tr }} + + {% endif %} +
+ {% endif %} + + {{ "Close" | tr }} + + {% else %} + {{ result.status_display() }} + {% endif %} +
+ {% include "partials/widgets/pager.html" %} + {% endif %} +
+{% endblock %} diff --git a/test/test_defaults.py b/test/test_defaults.py new file mode 100644 index 00000000..4803fb5a --- /dev/null +++ b/test/test_defaults.py @@ -0,0 +1,14 @@ +from aurweb import defaults + + +def test_fallback_pp(): + assert defaults.fallback_pp(75) == defaults.PP + assert defaults.fallback_pp(100) == 100 + + +def test_pp(): + assert defaults.PP == 50 + + +def test_o(): + assert defaults.O == 0 diff --git a/test/test_packages_routes.py b/test/test_packages_routes.py index f1c20067..a25fcb7e 100644 --- a/test/test_packages_routes.py +++ b/test/test_packages_routes.py @@ -8,7 +8,7 @@ import pytest from fastapi.testclient import TestClient -from aurweb import asgi, db +from aurweb import asgi, db, defaults from aurweb.models.account_type import USER_ID, AccountType from aurweb.models.dependency_type import DependencyType from aurweb.models.official_provider import OfficialProvider @@ -74,6 +74,7 @@ def setup(): PackageNotification.__tablename__, PackageComaintainer.__tablename__, PackageComment.__tablename__, + PackageRequest.__tablename__, OfficialProvider.__tablename__ ) @@ -108,6 +109,18 @@ def maintainer() -> User: yield maintainer +@pytest.fixture +def tu_user(): + tu_type = db.query(AccountType, + AccountType.AccountType == "Trusted User").first() + with db.begin(): + tu_user = db.create(User, Username="test_tu", + Email="test_tu@example.org", + RealName="Test TU", Passwd="testPassword", + AccountType=tu_type) + yield tu_user + + @pytest.fixture def package(maintainer: User) -> Package: """ Yield a Package created by user. """ @@ -160,6 +173,25 @@ def packages(maintainer: User) -> List[Package]: yield packages_ +@pytest.fixture +def requests(user: User, packages: List[Package]) -> List[PackageRequest]: + pkgreqs = [] + deletion_type = db.query(RequestType).filter( + RequestType.ID == DELETION_ID + ).first() + with db.begin(): + for i in range(55): + pkgreq = db.create(PackageRequest, + RequestType=deletion_type, + User=user, + PackageBase=packages[i].PackageBase, + PackageBaseName=packages[i].Name, + Comments=f"Deletion request for pkg_{i}", + ClosureComment=str()) + pkgreqs.append(pkgreq) + yield pkgreqs + + def test_package_not_found(client: TestClient): with client as request: resp = request.get("/packages/not_found") @@ -1304,3 +1336,53 @@ def test_pkgbase_comaintainers(client: TestClient, user: User, root = parse_root(resp.text) users = root.xpath('//textarea[@id="id_users"]')[0] assert users is not None and users.text is None + + +def test_requests_unauthorized(client: TestClient, + maintainer: User, + tu_user: User, + packages: List[Package], + requests: List[PackageRequest]): + cookies = {"AURSID": maintainer.login(Request(), "testPassword")} + with client as request: + resp = request.get("/requests", cookies=cookies, allow_redirects=False) + assert resp.status_code == int(HTTPStatus.SEE_OTHER) + + +def test_requests(client: TestClient, + maintainer: User, + tu_user: User, + packages: List[Package], + requests: List[PackageRequest]): + cookies = {"AURSID": tu_user.login(Request(), "testPassword")} + with client as request: + resp = request.get("/requests", params={ + # Pass in url query parameters O, SeB and SB to exercise + # their paths inside of the pager_nav used in this request. + "O": 0, # Page 1 + "SeB": "nd", + "SB": "n" + }, cookies=cookies) + assert resp.status_code == int(HTTPStatus.OK) + + assert "Next ›" in resp.text + assert "Last »" in resp.text + + root = parse_root(resp.text) + # We have 55 requests, our defaults.PP is 50, so expect we have 50 rows. + rows = root.xpath('//table[@class="results"]/tbody/tr') + assert len(rows) == defaults.PP + + # Request page 2 of the requests page. + with client as request: + resp = request.get("/requests", params={ + "O": 50 # Page 2 + }, cookies=cookies) + assert resp.status_code == int(HTTPStatus.OK) + + assert "‹ Previous" in resp.text + assert "« First" in resp.text + + root = parse_root(resp.text) + rows = root.xpath('//table[@class="results"]/tbody/tr') + assert len(rows) == 5 # There are five records left on the second page. diff --git a/test/test_templates.py b/test/test_templates.py index b6aa2055..86fbf611 100644 --- a/test/test_templates.py +++ b/test/test_templates.py @@ -1,6 +1,6 @@ import pytest -from aurweb.templates import register_filter +from aurweb.templates import register_filter, register_function @register_filter("func") @@ -8,6 +8,11 @@ def func(): pass +@register_function("function") +def function(): + pass + + def test_register_filter_exists_key_error(): """ Most instances of register_filter are tested through module imports or template renders, so we only test failures here. """ @@ -15,3 +20,12 @@ def test_register_filter_exists_key_error(): @register_filter("func") def some_func(): pass + + +def test_register_function_exists_key_error(): + """ Most instances of register_filter are tested through module + imports or template renders, so we only test failures here. """ + with pytest.raises(KeyError): + @register_function("function") + def some_func(): + pass diff --git a/test/test_util.py b/test/test_util.py index 0cc45409..99b77a78 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -1,7 +1,7 @@ from datetime import datetime from zoneinfo import ZoneInfo -from aurweb import util +from aurweb import filters, util def test_timestamp_to_datetime(): @@ -34,3 +34,9 @@ def test_to_qs(): query = {"a": "b", "c": [1, 2, 3]} qs = util.to_qs(query) assert qs == "a=b&c=1&c=2&c=3" + + +def test_round(): + assert filters.do_round(1.3) == 1 + assert filters.do_round(1.5) == 2 + assert filters.do_round(2.0) == 2