feat(FastAPI): add pkgbase comments (new, edit)

In PHP, this was implemented using an /rpc type 'get-comment-form'.
With FastAPI, we've decided to reorganize this into a non-RPC route:
`/pkgbase/{name}/comments/{id}/form`, rendered via the new
`templates/partials/packages/comment_form.html` template.

When the comment_form.html template is provided a `comment` object,
it will produce an edit comment form. Otherwise, it will produce a new
comment form.

A few new FastAPI routes have been introduced:

- GET `/pkgbase/{name}/comments/{id}/form`
    - Produces a JSON response based on {"form": "<form_markup>"}.
- POST `/pkgbase/{name}/comments'
    - Creates a new comment.
- POST `/pkgbase/{name}/comments/{id}`
    - Edits an existing comment.

In addition, some Javascript has been modified for our new routes.

Signed-off-by: Kevin Morris <kevr@0cost.org>

aurweb/packages/util.py

@@ -11,6 +11,7 @@ from aurweb import db
 from aurweb.models.official_provider import OFFICIAL_BASE, OfficialProvider
 from aurweb.models.package import Package
 from aurweb.models.package_base import PackageBase
+from aurweb.models.package_comment import PackageComment
 from aurweb.models.package_dependency import PackageDependency
 from aurweb.models.package_notification import PackageNotification
 from aurweb.models.package_relation import PackageRelation
@@ -121,6 +122,13 @@ def get_pkg_or_base(name: str, cls: Union[Package, PackageBase] = PackageBase):
     return instance
 
 
+def get_pkgbase_comment(pkgbase: PackageBase, id: int) -> PackageComment:
+    comment = pkgbase.comments.filter(PackageComment.ID == id).first()
+    if not comment:
+        raise HTTPException(status_code=int(HTTPStatus.NOT_FOUND))
+    return comment
+
+
 @register_filter("out_of_date")
 def out_of_date(packages: orm.Query) -> orm.Query:
     return packages.filter(PackageBase.OutOfDateTS.isnot(None))

aurweb/routers/packages.py

@@ -1,8 +1,9 @@
+from datetime import datetime
 from http import HTTPStatus
 from typing import Any, Dict
 
-from fastapi import APIRouter, Request, Response
+from fastapi import APIRouter, Form, HTTPException, Request, Response
-from fastapi.responses import RedirectResponse
+from fastapi.responses import JSONResponse, RedirectResponse
 from sqlalchemy import and_
 
 import aurweb.filters
@@ -11,9 +12,11 @@ import aurweb.models.package_keyword
 import aurweb.packages.util
 
 from aurweb import db
+from aurweb.auth import auth_required
 from aurweb.models.license import License
 from aurweb.models.package import Package
 from aurweb.models.package_base import PackageBase
+from aurweb.models.package_comment import PackageComment
 from aurweb.models.package_dependency import PackageDependency
 from aurweb.models.package_license import PackageLicense
 from aurweb.models.package_notification import PackageNotification
@@ -23,8 +26,9 @@ from aurweb.models.package_source import PackageSource
 from aurweb.models.package_vote import PackageVote
 from aurweb.models.relation_type import CONFLICTS_ID
 from aurweb.packages.search import PackageSearch
-from aurweb.packages.util import get_pkg_or_base, query_notified, query_voted
+from aurweb.packages.util import get_pkg_or_base, get_pkgbase_comment, query_notified, query_voted
-from aurweb.templates import make_context, render_template
+from aurweb.scripts.rendercomment import update_comment_render
+from aurweb.templates import make_context, render_raw_template, render_template
 
 router = APIRouter()
 
@@ -124,7 +128,9 @@ async def make_single_context(request: Request,
     context["pkgbase"] = pkgbase
     context["packages_count"] = pkgbase.packages.count()
     context["keywords"] = pkgbase.keywords
-    context["comments"] = pkgbase.comments
+    context["comments"] = pkgbase.comments.order_by(
+        PackageComment.CommentTS.desc()
+    )
     context["is_maintainer"] = (request.user.is_authenticated()
                                 and request.user.ID == pkgbase.MaintainerUID)
     context["notified"] = request.user.notified(pkgbase)
@@ -201,7 +207,94 @@ async def package_base(request: Request, name: str) -> Response:
 @router.get("/pkgbase/{name}/voters")
 async def package_base_voters(request: Request, name: str) -> Response:
     # Get the PackageBase.
-    pkgbase = get_pkgbase(name)
+    pkgbase = get_pkg_or_base(name, PackageBase)
     context = make_context(request, "Voters")
     context["pkgbase"] = pkgbase
     return render_template(request, "pkgbase/voters.html", context)
+
+
+@router.post("/pkgbase/{name}/comments")
+@auth_required(True)
+async def pkgbase_comments_post(
+        request: Request, name: str,
+        comment: str = Form(default=str()),
+        enable_notifications: bool = Form(default=False)):
+    """ Add a new comment. """
+    pkgbase = get_pkg_or_base(name, PackageBase)
+
+    if not comment:
+        raise HTTPException(status_code=int(HTTPStatus.EXPECTATION_FAILED))
+
+    # If the provided comment is different than the record's version,
+    # update the db record.
+    now = int(datetime.utcnow().timestamp())
+    with db.begin():
+        comment = db.create(PackageComment, User=request.user,
+                            PackageBase=pkgbase,
+                            Comments=comment, RenderedComment=str(),
+                            CommentTS=now)
+
+        if enable_notifications and not request.user.notified(pkgbase):
+            db.create(PackageNotification,
+                      User=request.user,
+                      PackageBase=pkgbase)
+    update_comment_render(comment.ID)
+
+    # Redirect to the pkgbase page.
+    return RedirectResponse(f"/pkgbase/{pkgbase.Name}#comment-{comment.ID}",
+                            status_code=int(HTTPStatus.SEE_OTHER))
+
+
+@router.get("/pkgbase/{name}/comments/{id}/form")
+@auth_required(True)
+async def pkgbase_comment_form(request: Request, name: str, id: int):
+    """ Produce a comment form for comment {id}. """
+    pkgbase = get_pkg_or_base(name, PackageBase)
+    comment = pkgbase.comments.filter(PackageComment.ID == id).first()
+    if not comment:
+        return JSONResponse({}, status_code=int(HTTPStatus.NOT_FOUND))
+
+    if not request.user.is_elevated() and request.user != comment.User:
+        return JSONResponse({}, status_code=int(HTTPStatus.UNAUTHORIZED))
+
+    context = await make_single_context(request, pkgbase)
+    context["comment"] = comment
+
+    form = render_raw_template(
+        request, "partials/packages/comment_form.html", context)
+    return JSONResponse({"form": form})
+
+
+@router.post("/pkgbase/{name}/comments/{id}")
+@auth_required(True)
+async def pkgbase_comment_post(
+        request: Request, name: str, id: int,
+        comment: str = Form(default=str()),
+        enable_notifications: bool = Form(default=False)):
+    pkgbase = get_pkg_or_base(name, PackageBase)
+    db_comment = get_pkgbase_comment(pkgbase, id)
+
+    if not comment:
+        raise HTTPException(status_code=int(HTTPStatus.EXPECTATION_FAILED))
+
+    # If the provided comment is different than the record's version,
+    # update the db record.
+    now = int(datetime.utcnow().timestamp())
+    if db_comment.Comments != comment:
+        with db.begin():
+            db_comment.Comments = comment
+            db_comment.Editor = request.user
+            db_comment.EditedTS = now
+
+            db_notif = request.user.notifications.filter(
+                PackageNotification.PackageBaseID == pkgbase.ID
+            ).first()
+            if enable_notifications and not db_notif:
+                db.create(PackageNotification,
+                          User=request.user,
+                          PackageBase=pkgbase)
+    update_comment_render(db_comment.ID)
+
+    # Redirect to the pkgbase page anchored to the updated comment.
+    return RedirectResponse(f"/pkgbase/{pkgbase.Name}#comment-{db_comment.ID}",
+                            status_code=int(HTTPStatus.SEE_OTHER))

templates/partials/packages/comment.html

@@ -16,26 +16,38 @@
     )
     | safe
     }}
-    {% if is_maintainer %}
+    {% if comment.Editor %}
-    <form class="delete-comment-form" method="post" action="/pkgbase/{{ pkgname }}/">
+        {% set edited_on = comment.EditedTS | dt | as_timezone(timezone) %}
-        <fieldset style="display:inline;">
+        <span class="edited">
-            <input type="hidden" name="action" value="do_DeleteComment" />
+            ({{ "edited on %s by %s" | tr
-            <input type="hidden" name="comment_id" value="{{ comment.ID }}"/>
+            | format(edited_on.strftime('%Y-%m-%d %H:%M'),
-            <input type="hidden" name="return_to" value="/pkgbase/{{ pkgname }}/"/>
+            '<a href="/account/%s">%s</a>' | format(
-            <input type="image" class="delete-comment" src="/images/x.min.svg" width="11" height="11" alt="{{ 'Delete comment' | tr }}" title="{{ 'Delete comment' | tr }}" name="submit" value="1" />
+            comment.Editor.Username, comment.Editor.Username))
-        </fieldset>
+            | safe
-    </form>
+            }})
-    <a href="/pkgbase/{{ pkgname }}/edit-comment/?comment_id={{ comment.ID }}" class="edit-comment" title="Edit comment"><img src="/images/pencil.min.svg" alt="Edit comment" width="11" height="11"></a>
+        </span>
+    {% endif %}
+    {% if request.user.is_elevated() or pkgbase.Maintainer == request.user %}
+        <form class="delete-comment-form" method="post" action="/pkgbase/{{ name }}/">
+            <fieldset style="display:inline;">
+                <input type="hidden" name="action" value="do_DeleteComment" />
+                <input type="hidden" name="comment_id" value="{{ comment.ID }}"/>
+                <input type="hidden" name="return_to" value="/pkgbase/{{ name }}/"/>
+                <input type="image" class="delete-comment" src="/images/x.min.svg" width="11" height="11" alt="{{ 'Delete comment' | tr }}" title="{{ 'Delete comment' | tr }}" name="submit" value="1" />
+            </fieldset>
+        </form>
+        <a href="/pkgbase/{{ pkgname }}/edit-comment/?comment_id={{ comment.ID }}" class="edit-comment" title="Edit comment"><img src="/images/pencil.min.svg" alt="Edit comment" width="11" height="11"></a>
+
+        <form class="pin-comment-form" method="post" action="/pkgbase/{{ name }}/">
+            <fieldset style="display:inline;">
+                <input type="hidden" name="action" value="do_PinComment"/>
+                <input type="hidden" name="comment_id" value="{{ comment.ID }}"/>
+                <input type="hidden" name="package_base" value="{{ pkgbase.ID }}"/>
+                <input type="hidden" name="return_to" value="/pkgbase/{{ name }}/"/>
+                <input type="image" class="pin-comment" src="/images/pin.min.svg" width="11" height="11" alt="{{ 'Pin comment' | tr }}" title="{{ 'Pin comment' | tr }}" name="submit" value="1"/>
+            </fieldset>
+        </form>
     {% endif %}
-    <form class="pin-comment-form" method="post" action="/pkgbase/{{ pkgname }}/">
-        <fieldset style="display:inline;">
-            <input type="hidden" name="action" value="do_PinComment"/>
-            <input type="hidden" name="comment_id" value="{{ comment.ID }}"/>
-            <input type="hidden" name="package_base" value="{{ pkgbase_id }}"/>
-            <input type="hidden" name="return_to" value="/pkgbase/{{ pkgname }}/"/>
-            <input type="image" class="pin-comment" src="/images/pin.min.svg" width="11" height="11" alt="{{ 'Pin comment' | tr }}" title="{{ 'Pin comment' | tr }}" name="submit" value="1"/>
-        </fieldset>
-    </form>
 </h4>
 <div id="comment-{{ comment.ID }}-content" class="article-content">
     <div>

templates/partials/packages/comment_form.html

@@ -0,0 +1,46 @@
+{# `action` is assigned the proper route to use for the form action.
+When `comment` is provided (PackageComment), we display an edit form
+for the comment. Otherwise, we display a new form.
+
+Routes:
+    new comment  - /pkgbase/{name}/comments
+    edit comment - /pkgbase/{name}/comments/{id}
+#}
+{% set action = "/pkgbase/%s/comments" | format(pkgbase.Name) %}
+{% if comment %}
+    {% set action = "/pkgbase/%s/comments/%d" | format(pkgbase.Name, comment.ID) %}
+{% endif %}
+
+<form action="{{ action }}" method="post">
+    <fieldset>
+        <p>
+            {{ "Git commit identifiers referencing commits in the AUR package "
+            "repository and URLs are converted to links automatically." | tr }}
+            {{ "%sMarkdown syntax%s is partially supported." | tr
+            | format('<a href="https://daringfireball.net/projects/markdown/syntax">',
+                     "</a>")
+            | safe }}
+        </p>
+        <p>
+        <textarea id="id_comment"
+                  name="comment"
+                  cols="80"
+                  rows="10"
+        >{% if comment %}{{ comment.Comments or '' }}{% endif %}</textarea>
+        </p>
+        <p>
+            <button type="submit" class="button">
+                {{ ("Save" if comment else "Add Comment") | tr }}
+            </button>
+            {% if comment and not request.user.notified(pkgbase) %}
+                <span class="comment-enable-notifications">
+                    <input type="checkbox" name="enable_notifications"
+                           id="id_enable_notifications" />
+                    <label for="id_enable_notifications">
+                        {{ "Enable notifications" | tr }}
+                    </label>
+                </span>
+            {% endif %}
+        </p>
+    </fieldset>
+</form>

templates/partials/packages/comments.html

@@ -8,44 +8,7 @@
 {% if request.user.is_authenticated() %}
 <div id="generic-form" class="box">
     <h2>Add Comment</h2>
-    <form action="/pkgbase/{{ pkgname }}/" method="post">
+    {% include "partials/packages/comment_form.html" %}
-        <fieldset>
-            <div>
-                <input type="hidden" name="action" value="do_AddComment"/>
-                <input type="hidden" name="ID" value="{{ pkgbase_id }}"/>
-            </div>
-            <p>
-                {{
-                "Git commit identifiers referencing commits in the AUR package"
-                " repository and URLs are converted to links automatically."
-                | tr
-                }}
-                {{
-                "%sMarkdown Syntax%s is partially supported."
-                | tr
-                | format('<a href="https://daringfireball.net/projects/markdown/syntax">', '</a>')
-                | safe
-                }}
-            </p>
-            <p>
-                <textarea id="id_comment" name="comment" cols="80" rows="10"></textarea>
-            </p>
-            <p>
-                <input type="submit" value="{{ 'Add Comment' | tr }}"/>
-                {% if not notifications_enabled %}
-                    <span class="comment-enable-notifications">
-                        <input id="id_enable_notifications"
-                               type="checkbox"
-                               name="enable_notifications"
-                        />
-                        <label for="id_enable_notifications">
-                            {{ "Enable notifications" | tr }}
-                        </label>
-                    </span>
-                {% endif %}
-            </p>
-        </fieldset>
-    </form>
 </div>
 {% endif %}
 
@@ -99,29 +62,28 @@ function handleEditCommentClick(event) {
     // The div class="article-content" which contains the comment
     const edit_form = parent_element.nextElementSibling;
 
-    const params = new URLSearchParams({
+    const url = "/pkgbase/{{ pkgbase.Name }}/comments/" + comment_id + "/form";
-        type: "get-comment-form",
-        arg: comment_id,
-        base_id: {{ pkgbase.ID }},
-        pkgbase_name: {{ pkgbase.Name }}
-    });
-
-    const url = '/rpc?' + params.toString();
 
     add_busy_indicator(event.target);
 
     fetch(url, {
-        method: 'GET'
+        method: 'GET',
+        credentials: 'same-origin'
+    })
+    .then(function(response) {
+        if (!response.ok) {
+            throw Error(response.statusText);
+        }
+        return response.json();
     })
-    .then(function(response) { return response.json(); })
     .then(function(data) {
         remove_busy_indicator(event.target);
-        if (data.success) {
+        edit_form.innerHTML = data.form;
-            edit_form.innerHTML = data.form;
+        edit_form.querySelector('textarea').focus();
-            edit_form.querySelector('textarea').focus();
+    })
-        } else {
+    .catch(function(error) {
-            alert(data.error);
+        remove_busy_indicator(event.target);
-        }
+        console.error(error);
     });
 }
 

test/test_packages_routes.py

@@ -73,6 +73,7 @@ def setup():
         PackageVote.__tablename__,
         PackageNotification.__tablename__,
         PackageComaintainer.__tablename__,
+        PackageComment.__tablename__,
         OfficialProvider.__tablename__
     )
 
@@ -930,3 +931,135 @@ def test_pkgbase_voters(client: TestClient, maintainer: User, package: Package):
     root = parse_root(resp.text)
     rows = root.xpath('//div[@class="box"]//ul/li')
     assert len(rows) == 1
+
+
+def test_pkgbase_comment_not_found(client: TestClient, maintainer: User,
+                                   package: Package):
+    cookies = {"AURSID": maintainer.login(Request(), "testPassword")}
+    comment_id = 12345  # A non-existing comment.
+    endpoint = f"/pkgbase/{package.PackageBase.Name}/comments/{comment_id}"
+    with client as request:
+        resp = request.post(endpoint, data={
+            "comment": "Failure"
+        }, cookies=cookies)
+    assert resp.status_code == int(HTTPStatus.NOT_FOUND)
+
+
+def test_pkgbase_comment_form_unauthorized(client: TestClient, user: User,
+                                           maintainer: User, package: Package):
+    now = int(datetime.utcnow().timestamp())
+    with db.begin():
+        comment = db.create(PackageComment, PackageBase=package.PackageBase,
+                            User=maintainer, Comments="Test",
+                            RenderedComment=str(), CommentTS=now)
+
+    cookies = {"AURSID": user.login(Request(), "testPassword")}
+    pkgbasename = package.PackageBase.Name
+    endpoint = f"/pkgbase/{pkgbasename}/comments/{comment.ID}/form"
+    with client as request:
+        resp = request.get(endpoint, cookies=cookies)
+    assert resp.status_code == int(HTTPStatus.UNAUTHORIZED)
+
+
+def test_pkgbase_comment_form_not_found(client: TestClient, maintainer: User,
+                                        package: Package):
+    cookies = {"AURSID": maintainer.login(Request(), "testPassword")}
+    comment_id = 12345  # A non-existing comment.
+    pkgbasename = package.PackageBase.Name
+    endpoint = f"/pkgbase/{pkgbasename}/comments/{comment_id}/form"
+    with client as request:
+        resp = request.get(endpoint, cookies=cookies)
+    assert resp.status_code == int(HTTPStatus.NOT_FOUND)
+
+
+def test_pkgbase_comments_missing_comment(client: TestClient, maintainer: User,
+                                          package: Package):
+    cookies = {"AURSID": maintainer.login(Request(), "testPassword")}
+    endpoint = f"/pkgbase/{package.PackageBase.Name}/comments"
+    with client as request:
+        resp = request.post(endpoint, cookies=cookies)
+    assert resp.status_code == int(HTTPStatus.EXPECTATION_FAILED)
+
+
+def test_pkgbase_comments(client: TestClient, maintainer: User,
+                          package: Package):
+    cookies = {"AURSID": maintainer.login(Request(), "testPassword")}
+    pkgbasename = package.PackageBase.Name
+    endpoint = f"/pkgbase/{pkgbasename}/comments"
+    with client as request:
+        resp = request.post(endpoint, data={
+            "comment": "Test comment.",
+            "enable_notifications": True
+        }, cookies=cookies)
+    assert resp.status_code == int(HTTPStatus.SEE_OTHER)
+
+    expected_prefix = f"/pkgbase/{pkgbasename}"
+    prefix_len = len(expected_prefix)
+    assert resp.headers.get("location")[:prefix_len] == expected_prefix
+
+    with client as request:
+        resp = request.get(resp.headers.get("location"))
+    assert resp.status_code == int(HTTPStatus.OK)
+
+    root = parse_root(resp.text)
+    headers = root.xpath('//h4[@class="comment-header"]')
+    bodies = root.xpath('//div[@class="article-content"]/div/p')
+
+    assert len(headers) == 1
+    assert len(bodies) == 1
+
+    assert bodies[0].text.strip() == "Test comment."
+
+    # Clear up the PackageNotification. This doubles as testing
+    # that the notification was created and clears it up so we can
+    # test enabling it during edit.
+    pkgbase = package.PackageBase
+    db_notif = pkgbase.notifications.filter(
+        PackageNotification.UserID == maintainer.ID
+    ).first()
+    with db.begin():
+        db.session.delete(db_notif)
+
+    # Now, let's edit the comment we just created.
+    comment_id = int(headers[0].attrib["id"].split("-")[-1])
+    endpoint = f"/pkgbase/{pkgbasename}/comments/{comment_id}"
+    with client as request:
+        resp = request.post(endpoint, data={
+            "comment": "Edited comment.",
+            "enable_notifications": True
+        }, cookies=cookies)
+    assert resp.status_code == int(HTTPStatus.SEE_OTHER)
+
+    with client as request:
+        resp = request.get(resp.headers.get("location"))
+    assert resp.status_code == int(HTTPStatus.OK)
+
+    root = parse_root(resp.text)
+    headers = root.xpath('//h4[@class="comment-header"]')
+    bodies = root.xpath('//div[@class="article-content"]/div/p')
+
+    assert len(headers) == 1
+    assert len(bodies) == 1
+
+    assert bodies[0].text.strip() == "Edited comment."
+
+    # Ensure that a notification was created.
+    db_notif = pkgbase.notifications.filter(
+        PackageNotification.UserID == maintainer.ID
+    ).first()
+    assert db_notif is not None
+
+    # Don't supply a comment; should return EXPECTATION_FAILED.
+    with client as request:
+        fail_resp = request.post(endpoint, cookies=cookies)
+    assert fail_resp.status_code == int(HTTPStatus.EXPECTATION_FAILED)
+
+    # Now, test the form route, which should return form markup
+    # via JSON.
+    endpoint = f"{endpoint}/form"
+    with client as request:
+        resp = request.get(endpoint, cookies=cookies)
+    assert resp.status_code == int(HTTPStatus.OK)
+
+    data = resp.json()
+    assert "form" in data