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 <kevr@0cost.org>
This commit is contained in:
Kevin Morris 2021-09-10 13:28:11 -07:00
parent c164abe256
commit 99482f9962
No known key found for this signature in database
GPG key ID: F7E46DED420788F3
11 changed files with 341 additions and 17 deletions

View file

@ -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")

18
aurweb/defaults.py Normal file
View file

@ -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

View file

@ -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)

View file

@ -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)

View file

@ -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
}

View file

@ -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

115
templates/requests.html Normal file
View file

@ -0,0 +1,115 @@
{% extends "partials/layout.html" %}
{% set singular = "%d package request found." %}
{% set plural = "%d package requests found." %}
{% block pageContent %}
<div id="pkglist-results" class="box">
{% if not total %}
<p>{{ "No requests matched your search criteria." | tr }}</p>
{% else %}
{% include "partials/widgets/pager.html" %}
<table class="results">
<thead>
<tr>
<th>{{ "Package" | tr }}</th>
<th>{{ "Type" | tr }}</th>
<th>{{ "Comments" | tr }}</th>
<th>{{ "Filed by" | tr }}</th>
<th>{{ "Date" | tr }}</th>
<th>{{ "Status" | tr }}</th>
</tr>
</thead>
<tbody>
{% for result in results %}
<tr>
<td>
{# Package #}
<a href="/pkgbase/{{ result.PackageBaseName }}">
{{ result.PackageBaseName }}
</a>
</td>
{# Type #}
<td>
{{ 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 %}
</td>
{# Comments #}
<td class="wrap">{{ result.Comments }}</td>
<td>
{# Filed by #}
<a href="/account/{{ result.User.Username }}">
{{ result.User.Username }}
</a>
</td>
{% 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 %}
<td
{% if due %}
class="flagged"
{% endif %}
>
{# Date #}
{% set date = result.RequestTS | dt | as_timezone(timezone) %}
{{ date.strftime("%Y-%m-%d %H:%M") }}
</td>
<td>
{# 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. #}
<a href="/pkgbase/{{ result.PackageBaseName }}/{{ action }}?{{ temp_q | urlencode }}">
{{ "Accept" | tr }}
</a>
{% endif %}
<br />
{% endif %}
<a href="/requests/{{ result.ID }}/close">
{{ "Close" | tr }}
</a>
{% else %}
{{ result.status_display() }}
{% endif %}
</td>
</tr>
{% endfor %}
</tbody>
</table>
{% include "partials/widgets/pager.html" %}
{% endif %}
</div>
{% endblock %}

14
test/test_defaults.py Normal file
View file

@ -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

View file

@ -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.

View file

@ -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

View file

@ -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