From 9e3c2e15eab02da6aa6b3c170ac6de5d6e9de916 Mon Sep 17 00:00:00 2001 From: Kevin Morris Date: Mon, 7 Mar 2022 23:13:08 -0800 Subject: [PATCH] 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 --- aurweb/models/tu_vote.py | 3 + aurweb/routers/trusted_user.py | 87 ++++++++++----- po/aurweb.pot | 8 ++ templates/partials/tu/proposal/details.html | 13 ++- test/test_trusted_user_routes.py | 112 ++++++++++++-------- 5 files changed, 150 insertions(+), 73 deletions(-) diff --git a/aurweb/models/tu_vote.py b/aurweb/models/tu_vote.py index cd486b4d..22fefedb 100644 --- a/aurweb/models/tu_vote.py +++ b/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) diff --git a/aurweb/routers/trusted_user.py b/aurweb/routers/trusted_user.py index 53bcecb7..02d3a5b7 100644 --- a/aurweb/routers/trusted_user.py +++ b/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) - voteinfo = db.query(models.TUVoteInfo).filter( - models.TUVoteInfo.ID == proposal).first() + 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) diff --git a/po/aurweb.pot b/po/aurweb.pot index bec1b672..aa9f4eb2 100644 --- a/po/aurweb.pot +++ b/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 "" diff --git a/templates/partials/tu/proposal/details.html b/templates/partials/tu/proposal/details.html index f7a55148..e4b64629 100644 --- a/templates/partials/tu/proposal/details.html +++ b/templates/partials/tu/proposal/details.html @@ -2,7 +2,11 @@ {% if voteinfo.is_running() %}

- {% trans %}This vote is still running.{% endtrans %} + {% trans %}This vote is still running.{% endtrans %}
+ {% if vote %} + {{ "You've already voted for this proposal." | tr }} + {{ "You can change your vote while the proposal is still running." | tr }} + {% endif %}

{% endif %} @@ -39,6 +43,13 @@ + {% if vote and vote.Decision %} +
+ {{ "Your vote" | tr }}: + {{ vote.display() }} +
+ {% endif %} + {% if not voteinfo.is_running() %}
{{ "Result" | tr }}: diff --git a/test/test_trusted_user_routes.py b/test/test_trusted_user_routes.py index 99884cf6..372d864b 100644 --- a/test/test_trusted_user_routes.py +++ b/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 "Yes" 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 "Yes" 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