diff --git a/aurweb/models/tu_vote.py b/aurweb/models/tu_vote.py index efb23b19..22fefedb 100644 --- a/aurweb/models/tu_vote.py +++ b/aurweb/models/tu_vote.py @@ -6,6 +6,17 @@ from aurweb.models.declarative import Base from aurweb.models.tu_voteinfo import TUVoteInfo as _TUVoteInfo from aurweb.models.user import User as _User +YES_ID = 1 +NO_ID = 2 +ABSTAIN_ID = 3 + +DECISIONS = { + YES_ID: "Yes", + NO_ID: "No", + ABSTAIN_ID: "Abstain", +} +DECISION_IDS = {v: k for k, v in DECISIONS.items()} + class TUVote(Base): __table__ = schema.TU_Votes @@ -29,10 +40,20 @@ class TUVote(Base): raise IntegrityError( statement="Foreign key VoteID cannot be null.", orig="TU_Votes.VoteID", - params=("NULL")) + params=("NULL",)) if not self.User and not self.UserID: raise IntegrityError( statement="Foreign key UserID cannot be null.", orig="TU_Votes.UserID", - params=("NULL")) + params=("NULL",)) + + if self.Decision is None or self.Decision not in DECISIONS: + raise IntegrityError( + statement=("Decision column must be a valid key in " + "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 cbe3e47d..0d714aea 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/aurweb/schema.py b/aurweb/schema.py index d2644541..3fc2356b 100644 --- a/aurweb/schema.py +++ b/aurweb/schema.py @@ -406,6 +406,7 @@ TU_Votes = Table( 'TU_Votes', metadata, Column('VoteID', ForeignKey('TU_VoteInfo.ID', ondelete='CASCADE'), nullable=False), Column('UserID', ForeignKey('Users.ID', ondelete='CASCADE'), nullable=False), + Column('Decision', TINYINT(unsigned=True)), mysql_engine='InnoDB', ) diff --git a/migrations/versions/84cd90073dde_relocate_vote_decisions_into_tuvote.py b/migrations/versions/84cd90073dde_relocate_vote_decisions_into_tuvote.py new file mode 100644 index 00000000..19be49f8 --- /dev/null +++ b/migrations/versions/84cd90073dde_relocate_vote_decisions_into_tuvote.py @@ -0,0 +1,61 @@ +"""relocate vote decisions into TUVote + +Revision ID: 84cd90073dde +Revises: d64e5571bc8d +Create Date: 2022-03-07 18:02:28.791103 + +This migration allows us to give Trusted Users the ability to +modify a vote they made on a proposal. Previously, the decision +was tracked purely through TU_VoteInfo records, which removes +tracking of what decisions users made. This blocks us from being +able to modify votes, because we can't update the TU_VoteInfo +Yes, No or Abstain columns properly. + +""" +import sqlalchemy as sa + +from alembic import op +from sqlalchemy.dialects.mysql import TINYINT + +from aurweb import db, logging, time +from aurweb.models import TUVoteInfo + +logger = logging.get_logger("alembic") + +# revision identifiers, used by Alembic. +revision = '84cd90073dde' +down_revision = 'd64e5571bc8d' +branch_labels = None +depends_on = None + +TABLE = "TU_Votes" + + +def upgrade(): + decision = sa.Column("Decision", TINYINT(unsigned=True)) + op.add_column(TABLE, decision) + + # For each proposal which is running at the time this migration + # is applied, eradicate all related votes. We will restart the votes. + # In addition, reset the Submitted and End columns based on the current + # timestamp; essentially resetting the proposal's state. + utcnow = time.utcnow() + running_proposals = db.query(TUVoteInfo).filter(TUVoteInfo.End > utcnow) + with db.begin(): + for proposal in running_proposals: + logger.info(f"Resetting proposal with ID {proposal.ID}: " + "Yes = 0, No = 0, Abstain = 0, deleted vote records") + + # Reset the Submitted and End columns. + length = proposal.End - proposal.Submitted + proposal.Submitted = utcnow + proposal.End = utcnow + length + + proposal.Yes = proposal.No = proposal.Abstain = 0 + db.delete_all(proposal.tu_votes) + logger.info(f"Proposal time range was reset: Submitted = " + f"{proposal.Submitted} and End = {proposal.End}") + + +def downgrade(): + op.drop_column(TABLE, "Decision") 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 a5c4c5e8..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 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 @@ -173,7 +174,7 @@ def test_tu_empty_index(client, tu_user): assert len(tables) == 0 -def test_tu_index(client, tu_user): +def test_tu_index(client: TestClient, tu_user: User): ts = time.utcnow() # Create some test votes: (Agenda, Start, End). @@ -197,7 +198,8 @@ def test_tu_index(client, tu_user): vote_record = vote_records[1] vote_record.Yes += 1 vote_record.ActiveTUs += 1 - db.create(TUVote, VoteInfo=vote_record, User=tu_user) + db.create(TUVote, VoteInfo=vote_record, User=tu_user, + Decision=YES_ID) cookies = {"AURSID": tu_user.login(Request(), "testPassword")} with client as request: @@ -462,8 +464,8 @@ def test_tu_index_last_votes(client: TestClient, tu_user: User, tu_user2: User, Submitter=tu_user) # Create a vote on it from tu_user. - db.create(TUVote, VoteInfo=voteinfo, User=tu_user) - db.create(TUVote, VoteInfo=voteinfo, User=tu_user2) + db.create(TUVote, VoteInfo=voteinfo, User=tu_user, Decision=YES_ID) + db.create(TUVote, VoteInfo=voteinfo, User=tu_user2, Decision=YES_ID) # Now, check that tu_user got populated in the .last-votes table. cookies = {"AURSID": tu_user.login(Request(), "testPassword")} @@ -575,7 +577,7 @@ def test_tu_running_proposal(client: TestClient, # Create a vote. with db.begin(): - db.create(TUVote, VoteInfo=voteinfo, User=tu_user) + db.create(TUVote, VoteInfo=voteinfo, User=tu_user, Decision=YES_ID) voteinfo.ActiveTUs += 1 voteinfo.Yes += 1 @@ -585,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): @@ -604,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. @@ -626,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 @@ -634,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. """ @@ -666,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( @@ -731,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 @@ -880,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 diff --git a/test/test_tu_vote.py b/test/test_tu_vote.py index 91d73ecb..3956e07e 100644 --- a/test/test_tu_vote.py +++ b/test/test_tu_vote.py @@ -4,7 +4,7 @@ from sqlalchemy.exc import IntegrityError from aurweb import db, time from aurweb.models.account_type import TRUSTED_USER_ID -from aurweb.models.tu_vote import TUVote +from aurweb.models.tu_vote import YES_ID, TUVote from aurweb.models.tu_voteinfo import TUVoteInfo from aurweb.models.user import User @@ -36,10 +36,12 @@ def tu_voteinfo(user: User) -> TUVoteInfo: def test_tu_vote_creation(user: User, tu_voteinfo: TUVoteInfo): with db.begin(): - tu_vote = db.create(TUVote, User=user, VoteInfo=tu_voteinfo) + tu_vote = db.create(TUVote, User=user, VoteInfo=tu_voteinfo, + Decision=YES_ID) assert tu_vote.VoteInfo == tu_voteinfo assert tu_vote.User == user + assert tu_vote.Decision == YES_ID assert tu_vote in user.tu_votes assert tu_vote in tu_voteinfo.tu_votes @@ -52,3 +54,12 @@ def test_tu_vote_null_user_raises_exception(tu_voteinfo: TUVoteInfo): def test_tu_vote_null_voteinfo_raises_exception(user: User): with pytest.raises(IntegrityError): TUVote(User=user) + + +def test_tu_vote_bad_decision_raises_exception(user: User, + tu_voteinfo: TUVoteInfo): + with pytest.raises(IntegrityError): + TUVote(User=user, VoteInfo=tu_voteinfo, Decision=None) + + with pytest.raises(IntegrityError): + TUVote(User=user, VoteInfo=tu_voteinfo, Decision=0) diff --git a/test/test_tuvotereminder.py b/test/test_tuvotereminder.py index a54c52a4..82ee9111 100644 --- a/test/test_tuvotereminder.py +++ b/test/test_tuvotereminder.py @@ -5,6 +5,7 @@ import pytest from aurweb import config, db, time from aurweb.models import TUVote, TUVoteInfo, User from aurweb.models.account_type import TRUSTED_USER_ID +from aurweb.models.tu_vote import YES_ID from aurweb.scripts import tuvotereminder as reminder from aurweb.testing.email import Email @@ -13,7 +14,8 @@ aur_location = config.get("options", "aur_location") def create_vote(user: User, voteinfo: TUVoteInfo) -> TUVote: with db.begin(): - vote = db.create(TUVote, User=user, VoteID=voteinfo.ID) + vote = db.create(TUVote, User=user, VoteID=voteinfo.ID, + Decision=YES_ID) return vote