feat: allow TUs to change their votes on running proposals

In addition, this patch brings in display for the vote decision
you cast on it. This is only viewable by the request user; your
vote is not being shared with others.

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

aurweb/models/tu_vote.py

@@ -54,3 +54,6 @@ class TUVote(Base):
                            "aurweb.models.tu_vote.DECISIONS"),
                 orig="TU_Votes.Decision",
                 params=(self.Decision,))
+
+    def display(self) -> str:
+        return DECISIONS.get(self.Decision)

aurweb/routers/trusted_user.py

@@ -2,6 +2,7 @@ import html
 import typing
 
 from http import HTTPStatus
+from typing import Any, Dict, Optional, Tuple
 
 from fastapi import APIRouter, Form, HTTPException, Request
 from fastapi.responses import RedirectResponse, Response
@@ -10,8 +11,9 @@ from sqlalchemy import and_, func, or_
 from aurweb import db, l10n, logging, models, time
 from aurweb.auth import creds, requires_auth
 from aurweb.exceptions import handle_form_exceptions
-from aurweb.models import User
+from aurweb.models import TUVote, TUVoteInfo, User
 from aurweb.models.account_type import TRUSTED_USER_AND_DEV_ID, TRUSTED_USER_ID
+from aurweb.models.tu_vote import DECISION_IDS, DECISIONS
 from aurweb.templates import make_context, make_variable_context, render_template
 
 router = APIRouter()
@@ -152,8 +154,10 @@ async def trusted_user_proposal(request: Request, proposal: int):
     context = await make_variable_context(request, "Trusted User")
     proposal = int(proposal)
 
+    with db.begin():
         voteinfo = db.query(models.TUVoteInfo).filter(
             models.TUVoteInfo.ID == proposal).first()
+
     if not voteinfo:
         raise HTTPException(status_code=HTTPStatus.NOT_FOUND)
 
@@ -166,13 +170,43 @@ async def trusted_user_proposal(request: Request, proposal: int):
         context["error"] = "Only Trusted Users are allowed to vote."
     if voteinfo.User == request.user.Username:
         context["error"] = "You cannot vote in an proposal about you."
-    elif vote is not None:
-        context["error"] = "You've already voted for this proposal."
 
-    context["vote"] = vote
     return render_proposal(request, context, proposal, voteinfo, voters, vote)
 
 
+def judge_decision(request: Request, context: Dict[str, Any],
+                   voteinfo: TUVoteInfo, vote: Optional[TUVote],
+                   decision_id: int) -> Tuple[HTTPStatus, str]:
+    """ Decide if a given decision by request is valid.
+
+    A non-HTTPStatus.OK status_code value indicates that judge_decision
+    ran into an error. context's error key is set when this happens.
+
+    :param request: FastAPI Request
+    :param context: FastAPI template context
+    :param voteinfo: TUVoteInfo instance
+    :param vote: TUVote instance
+    :param decision_id: YES_ID, NO_ID, or ABSTAIN_ID
+    :return: (status_code, old_decision) tuple
+    """
+    old_decision = None
+    status_code = HTTPStatus.OK
+    if not request.user.has_credential(creds.TU_VOTE):
+        context["error"] = "Only Trusted Users are allowed to vote."
+        status_code = HTTPStatus.UNAUTHORIZED
+    elif not voteinfo.is_running():
+        context["error"] = "Voting is closed for this proposal."
+        status_code = HTTPStatus.BAD_REQUEST
+    elif voteinfo.User == request.user.Username:
+        context["error"] = "You cannot vote in an proposal about you."
+        status_code = HTTPStatus.BAD_REQUEST
+    elif vote is not None:
+        if vote.Decision is not None and vote.Decision != decision_id:
+            old_decision = DECISIONS.get(vote.Decision)
+
+    return (status_code, old_decision)
+
+
 @router.post("/tu/{proposal}")
 @handle_form_exceptions
 @requires_auth
@@ -189,39 +223,40 @@ async def trusted_user_proposal_post(request: Request, proposal: int,
     if not voteinfo:
         raise HTTPException(status_code=HTTPStatus.NOT_FOUND)
 
+    if decision not in DECISION_IDS:
+        return Response("Invalid 'decision' value.",
+                        status_code=HTTPStatus.BAD_REQUEST)
+    decision_id = DECISION_IDS.get(decision)
+
     voters = db.query(models.User).join(models.TUVote).filter(
         models.TUVote.VoteID == voteinfo.ID)
     vote = db.query(models.TUVote).filter(
         and_(models.TUVote.UserID == request.user.ID,
              models.TUVote.VoteID == voteinfo.ID)).first()
 
-    status_code = HTTPStatus.OK
-    if not request.user.has_credential(creds.TU_VOTE):
-        context["error"] = "Only Trusted Users are allowed to vote."
-        status_code = HTTPStatus.UNAUTHORIZED
-    elif voteinfo.User == request.user.Username:
-        context["error"] = "You cannot vote in an proposal about you."
-        status_code = HTTPStatus.BAD_REQUEST
-    elif vote is not None:
-        context["error"] = "You've already voted for this proposal."
-        status_code = HTTPStatus.BAD_REQUEST
-
+    status_code, old_decision = judge_decision(
+        request, context, voteinfo, vote, decision_id)
     if status_code != HTTPStatus.OK:
         return render_proposal(request, context, proposal,
                                voteinfo, voters, vote,
                                status_code=status_code)
 
-    if decision in {"Yes", "No", "Abstain"}:
-        # Increment whichever decision was given to us.
-        setattr(voteinfo, decision, getattr(voteinfo, decision) + 1)
-    else:
-        return Response("Invalid 'decision' value.",
-                        status_code=HTTPStatus.BAD_REQUEST)
-
     with db.begin():
-        vote = db.create(models.TUVote, User=request.user, VoteInfo=voteinfo)
+        # If the decision was changed, decrement the old decision.
+        if old_decision is not None:
+            setattr(voteinfo, old_decision,
+                    getattr(voteinfo, old_decision) - 1)
+
+        # In all cases, increment the new decision.
+        setattr(voteinfo, decision, getattr(voteinfo, decision) + 1)
+
+        # Create the vote if doesn't exist yet.
+        if not vote:
+            vote = db.create(models.TUVote, User=request.user,
+                             VoteInfo=voteinfo, Decision=decision_id)
+        else:
+            vote.Decision = decision_id
 
-    context["error"] = "You've already voted for this proposal."
     return render_proposal(request, context, proposal, voteinfo, voters, vote)
 
 

po/aurweb.pot

@@ -2334,3 +2334,11 @@ msgid "This action will close any pending package requests "
 "related to it. If %sComments%s are omitted, a closure "
 "comment will be autogenerated."
 msgstr ""
+
+#: templates/partials/tu/proposal/details.html
+msgid "Your vote"
+msgstr ""
+
+#: templates/partials/tu/proposal/details.html
+msgid "You can change your vote while the proposal is still running."
+msgstr ""

templates/partials/tu/proposal/details.html

@@ -2,7 +2,11 @@
 
 {% if voteinfo.is_running() %}
     <p class="vote-running" style="font-weight: bold; color: red">
-        {% trans %}This vote is still running.{% endtrans %}
+        {% trans %}This vote is still running.{% endtrans %}<br />
+        {% if vote %}
+            {{ "You've already voted for this proposal." | tr }}
+            {{ "You can change your vote while the proposal is still running." | tr }}
+        {% endif %}
     </p>
 {% endif %}
 
@@ -39,6 +43,13 @@
         </strong>
     </div>
 
+    {% if vote and vote.Decision %}
+        <div class="field">
+            {{ "Your vote" | tr }}:
+            <strong>{{ vote.display() }}</strong>
+        </div>
+    {% endif %}
+
     {% if not voteinfo.is_running() %}
         <div class="field result">
             {{ "Result" | tr }}:

test/test_trusted_user_routes.py

@@ -1,3 +1,4 @@
+import html
 import re
 
 from http import HTTPStatus
@@ -11,7 +12,7 @@ from fastapi.testclient import TestClient
 
 from aurweb import config, db, filters, time
 from aurweb.models.account_type import DEVELOPER_ID, TRUSTED_USER_ID, AccountType
-from aurweb.models.tu_vote import YES_ID, TUVote
+from aurweb.models.tu_vote import ABSTAIN_ID, YES_ID, TUVote
 from aurweb.models.tu_voteinfo import TUVoteInfo
 from aurweb.models.user import User
 from aurweb.testing.requests import Request
@@ -586,16 +587,21 @@ def test_tu_running_proposal(client: TestClient,
             "/tu", params={"id": voteinfo.ID}, cookies=cookies)
     assert response.status_code == int(HTTPStatus.OK)
 
+    # Check that we're told we've voted.
+    # Check that our vote decision is displayed.
+    content = html.unescape(response.text)
+    assert "You've already voted for this proposal." in content
+    expected = "You can change your vote while the proposal is still running."
+    assert expected in content
+    assert "Your vote:" in content
+    assert "<strong>Yes</strong>" in content
+
     # Parse our new root.
     root = parse_root(response.text)
 
-    # Check that we no longer have a voting form.
+    # Check that we still have a voting form.
     form = root.xpath('//form[contains(@class, "action-form")]')
-    assert not form
-
-    # Check that we're told we've voted.
-    status = root.xpath('//span[contains(@class, "status")]/text()')[0]
-    assert status == "You've already voted for this proposal."
+    assert form is not None
 
 
 def test_tu_ended_proposal(client, proposal):
@@ -605,11 +611,11 @@ def test_tu_ended_proposal(client, proposal):
     with db.begin():
         voteinfo.End = ts - 5  # 5 seconds ago.
 
-    # Initiate an authenticated GET request to /tu/{proposal_id}.
-    proposal_id = voteinfo.ID
+    # Initiate an authenticated GET request to /tu/{voteinfo.ID}.
     cookies = {"AURSID": tu_user.login(Request(), "testPassword")}
+    endpoint = f"/tu/{voteinfo.ID}"
     with client as request:
-        response = request.get(f"/tu/{proposal_id}", cookies=cookies)
+        response = request.get(f"/tu/{voteinfo.ID}", cookies=cookies)
     assert response.status_code == int(HTTPStatus.OK)
 
     # Alright, now let's continue on to verifying some markup.
@@ -627,7 +633,7 @@ def test_tu_ended_proposal(client, proposal):
     result = result_node.xpath("./span/text()")[0]
     assert result.strip() == "unknown"
 
-    # Check that voting has ended.
+    # Check that the form is gone; voting has ended.
     form = root.xpath('//form[contains(@class, "action-form")]')
     assert not form
 
@@ -635,6 +641,19 @@ def test_tu_ended_proposal(client, proposal):
     status = root.xpath('//span[contains(@class, "status")]/text()')[0]
     assert status == "Voting is closed for this proposal."
 
+    # Perform a POST request and expect the same behavior.
+    data = {"decision": "Yes"}
+    with client as request:
+        resp = request.post(endpoint, data=data, cookies=cookies)
+    assert resp.status_code == HTTPStatus.BAD_REQUEST
+
+    # Repeat the same assertions as we did in the GET request.
+    root = parse_root(resp.text)
+    form = root.xpath('//form[contains(@class, "action-form")]')
+    assert not form
+    status = root.xpath('//span[contains(@class, "status")]/text()')[0]
+    assert status == "Voting is closed for this proposal."
+
 
 def test_tu_proposal_vote_not_found(client, tu_user):
     """ Test POST request to a missing vote. """
@@ -667,11 +686,13 @@ def test_tu_proposal_vote(client, proposal):
                     TUVote.User == tu_user).first()
     assert vote is not None
 
-    root = parse_root(response.text)
-
-    # Check that we're told we've voted.
-    status = root.xpath('//span[contains(@class, "status")]/text()')[0]
-    assert status == "You've already voted for this proposal."
+    # Assert that we've gotten the message that we've voted.
+    content = html.unescape(response.text)
+    assert "You've already voted for this proposal." in content
+    expected = "You can change your vote while the proposal is still running."
+    assert expected in content
+    assert "Your vote:" in content
+    assert "<strong>Yes</strong>" in content
 
 
 def test_tu_proposal_vote_unauthorized(
@@ -732,36 +753,6 @@ def test_tu_proposal_vote_cant_self_vote(client, proposal):
     assert status == "You cannot vote in an proposal about you."
 
 
-def test_tu_proposal_vote_already_voted(client, proposal):
-    tu_user, user, voteinfo = proposal
-
-    with db.begin():
-        db.create(TUVote, VoteInfo=voteinfo, User=tu_user)
-        voteinfo.Yes += 1
-        voteinfo.ActiveTUs += 1
-
-    cookies = {"AURSID": tu_user.login(Request(), "testPassword")}
-    with client as request:
-        data = {"decision": "Yes"}
-        response = request.post(f"/tu/{voteinfo.ID}", cookies=cookies,
-                                data=data, allow_redirects=False)
-    assert response.status_code == int(HTTPStatus.BAD_REQUEST)
-
-    root = parse_root(response.text)
-    status = root.xpath('//span[contains(@class, "status")]/text()')[0]
-    assert status == "You've already voted for this proposal."
-
-    with client as request:
-        data = {"decision": "Yes"}
-        response = request.get(f"/tu/{voteinfo.ID}", cookies=cookies,
-                               data=data, allow_redirects=False)
-    assert response.status_code == int(HTTPStatus.OK)
-
-    root = parse_root(response.text)
-    status = root.xpath('//span[contains(@class, "status")]/text()')[0]
-    assert status == "You've already voted for this proposal."
-
-
 def test_tu_proposal_vote_invalid_decision(client, proposal):
     tu_user, user, voteinfo = proposal
 
@@ -881,3 +872,32 @@ def test_tu_addvote_post_bylaws(client: TestClient, tu_user: User):
     with client as request:
         response = request.post("/addvote", cookies=cookies, data=data)
     assert response.status_code == int(HTTPStatus.SEE_OTHER)
+
+
+def test_tu_change_vote(client: TestClient, tu_user: User,
+                        proposal: TUVoteInfo):
+    _, _, voteinfo = proposal
+    cookies = {"AURSID": tu_user.login(Request(), "testPassword")}
+    endpoint = f"/tu/{voteinfo.ID}"
+
+    # First, make an Abstain vote.
+    data = {"decision": "Abstain"}
+    with client as request:
+        resp = request.post(endpoint, data=data, cookies=cookies)
+    assert resp.status_code == HTTPStatus.OK
+
+    vote = db.query(TUVote).filter(TUVote.VoteID == voteinfo.ID).first()
+    assert vote is not None
+    assert vote.Decision == ABSTAIN_ID
+    assert voteinfo.Abstain == 1
+
+    # Changed our mind! Vote yes, instead.
+    data = {"decision": "Yes"}
+    with client as request:
+        resp = request.post(endpoint, data=data, cookies=cookies)
+    assert resp.status_code == HTTPStatus.OK
+
+    # Expect that the records got changed correctly.
+    assert vote.Decision == YES_ID
+    assert voteinfo.Abstain == 0
+    assert voteinfo.Yes == 1