From 896f420d527d2a6ecab763e8c63b0ff25837ed96 Mon Sep 17 00:00:00 2001 From: Ben Cotton Date: May 17 2021 16:42:08 +0000 Subject: Make the FPCA+1 requirement used by FESCo optional Many thanks to @pingou for teaching me how to do the tests and for writing many of them. --- diff --git a/alembic/versions/523f7d5c58d7_make_fpca_1_optional.py b/alembic/versions/523f7d5c58d7_make_fpca_1_optional.py new file mode 100644 index 0000000..4e2e2b3 --- /dev/null +++ b/alembic/versions/523f7d5c58d7_make_fpca_1_optional.py @@ -0,0 +1,28 @@ +"""Make FPCA+1 optional + +Revision ID: 523f7d5c58d7 +Revises: 5ecdd55b4af4 +Create Date: 2021-05-07 15:13:07.218758 + +""" + +# revision identifiers, used by Alembic. +revision = '523f7d5c58d7' +down_revision = '5ecdd55b4af4' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + """Add the FPCA+1 requirement option to elections""" + op.add_column( + 'requires_plusone', + sa.Column(sa.Boolean, default=False) + ) + +def downgrade(): + """Drop the FPCA+1 requirement option""" + op.remove_column( + 'requires_plusone' + ) diff --git a/fedora_elections/admin.py b/fedora_elections/admin.py index 062ac9a..c10788b 100644 --- a/fedora_elections/admin.py +++ b/fedora_elections/admin.py @@ -88,6 +88,7 @@ def admin_new_election(): url_badge=form.url_badge.data, voting_type=form.voting_type.data, max_votes=form.max_votes.data, + requires_plusone=form.requires_plusone.data, candidates_are_fasusers=int(form.candidates_are_fasusers.data), fas_user=flask.g.fas_user.username, ) diff --git a/fedora_elections/elections.py b/fedora_elections/elections.py index 8ed8d35..6039341 100644 --- a/fedora_elections/elections.py +++ b/fedora_elections/elections.py @@ -50,13 +50,6 @@ def login_required(f): elif not flask.g.fas_user.cla_done: flask.flash("You must sign the CLA to vote", "error") return safe_redirect_back() - else: - user_groups = OIDC.user_getfield("groups") - if len(user_groups) == 0: - flask.flash( - "You need to be in one another group than CLA to vote", "error" - ) - return safe_redirect_back() return f(*args, **kwargs) @@ -98,8 +91,17 @@ def vote(election_alias): if not isinstance(election, models.Election): return election + user_groups = OIDC.user_getfield("groups") + + # Check if this election requires FPCA +1 + if election.requires_plusone and len(user_groups) == 0: + flask.flash( + "You need to be in one another group than CLA to vote", "error" + ) + return safe_redirect_back() + + # Or if the election is restricted to specific groups if election.legal_voters_list: - user_groups = OIDC.user_getfield("groups") if len(set(user_groups).intersection(set(election.legal_voters_list))) == 0: flask.flash( "You are not among the groups that are allowed to vote " diff --git a/fedora_elections/forms.py b/fedora_elections/forms.py index 4cd3640..8227286 100644 --- a/fedora_elections/forms.py +++ b/fedora_elections/forms.py @@ -91,6 +91,8 @@ class ElectionForm(FlaskForm): ], ) + requires_plusone = wtforms.BooleanField("Election requires FPCA+1") + lgl_voters = wtforms.TextField( "Legal voters groups", [wtforms.validators.optional()] ) diff --git a/fedora_elections/models.py b/fedora_elections/models.py index 87503b1..e1983f3 100644 --- a/fedora_elections/models.py +++ b/fedora_elections/models.py @@ -90,6 +90,7 @@ class Election(BASE): max_votes = sa.Column(sa.Integer, nullable=True) candidates_are_fasusers = sa.Column(sa.Integer, nullable=False, default=0) fas_user = sa.Column(sa.Unicode(50), nullable=False) + requires_plusone = sa.Column(sa.Boolean, default=False) def to_json(self): """ Return a json representation of this object. """ diff --git a/fedora_elections/templates/_formhelpers.html b/fedora_elections/templates/_formhelpers.html index 1d093c7..058968c 100644 --- a/fedora_elections/templates/_formhelpers.html +++ b/fedora_elections/templates/_formhelpers.html @@ -107,6 +107,7 @@ {{ render_bootstrap_checkbox_in_row(form.candidates_are_fasusers) }} {{ render_bootstrap_checkbox_in_row(form.embargoed) }} {{ render_bootstrap_textfield_in_row(form.url_badge) }} + {{ render_bootstrap_checkbox_in_row(form.requires_plusone, after="Require membership in any non-FPCA group") }} {{ render_bootstrap_textfield_in_row(form.lgl_voters, after="FAS groups allowed to vote on this election (CLA-done is always required)") }} {{ render_bootstrap_textfield_in_row(form.admin_grp, after="FAS groups allowed to view the result despite the embargo") }} diff --git a/tests/test_flask_admin.py b/tests/test_flask_admin.py index d4d9c53..b627143 100644 --- a/tests/test_flask_admin.py +++ b/tests/test_flask_admin.py @@ -427,6 +427,84 @@ class FlaskAdmintests(ModelFlasktests): output_text, ) + def test_admin_new_election_requires_plusone(self): + """ Test the admin_new_election function. """ + self.setup_db() + + user = FakeUser( + fedora_elections.APP.config["FEDORA_ELECTIONS_ADMIN_GROUP"], + username="toshio", + ) + + with user_set(fedora_elections.APP, user, oidc_id_token="foobar"): + with patch( + "fedora_elections.OIDC.user_getfield", + MagicMock(return_value=["elections"]), + ): + output = self.app.get("/admin/new") + self.assertEqual(output.status_code, 200) + + csrf_token = self.get_csrf(output=output) + + # Test that we get the plusone field + # Probably not strictly necessary, but worth including + # for now since it's a new field + output_text = output.get_data(as_text=True) + self.assertIn( + 'input id="requires_plusone" ' + 'name="requires_plusone" type="checkbox" ', + output_text, + ) + + # All good + data = { + "alias": "new_election2", + "shortdesc": "new election2 shortdesc", + "description": "new election2 description", + "voting_type": "simple", + "url": "https://fedoraproject.org", + "start_date": TODAY + timedelta(days=2), + "end_date": TODAY + timedelta(days=4), + "seats_elected": 2, + "candidates_are_fasusers": False, + "embargoed": True, + "admin_grp": "testers, , sysadmin-main,,", + "lgl_voters": "testers, packager,,,", + "csrf_token": csrf_token, + "requires_plusone": True + } + + with mock_sends(NewElectionV1): + output = self.app.post( + "/admin/new", data=data, follow_redirects=True + ) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertTrue('Election "new_election2" added' in output_text) + self.assertTrue("There are no candidates." in output_text) + self.assertIn( + 'input class="form-control" id="admin_grp" ' + 'name="admin_grp" type="text" ' + 'value="sysadmin-main, testers">', + output_text, + ) + self.assertIn( + 'input class="form-control" id="lgl_voters" ' + 'name="lgl_voters" type="text" ' + 'value="packager, testers">', + output_text, + ) + self.assertIn( + 'input class="form-control" id="max_votes" ' + 'name="max_votes" type="text" ' + 'value="">', + output_text, + ) + + obj = fedora_elections.models.Election.get(self.session, "new_election2") + self.assertNotEqual(obj, None) + self.assertEqual(obj.requires_plusone, True) + def test_admin_edit_election(self): """ Test the admin_edit_election function. """ user = FakeUser( diff --git a/tests/test_flask_elections.py b/tests/test_flask_elections.py index caced31..8fef0b4 100644 --- a/tests/test_flask_elections.py +++ b/tests/test_flask_elections.py @@ -56,8 +56,30 @@ class FlaskElectionstests(ModelFlasktests): output = self.app.get("/vote/test_election", follow_redirects=True) output_text = output.get_data(as_text=True) - self.assertTrue( - "The election, test_election, does not exist." in output_text + self.assertIn( + "The election, test_election, does not exist.", output_text + ) + + def test_vote_require_fpca(self): + """ Test the vote function. """ + self.setup_db() + obj = fedora_elections.models.Election.get(self.session, "test_election3") + self.assertNotEqual(obj, None) + obj.requires_plusone = True + self.session.add(obj) + self.session.commit() + + user = FakeUser([], username="pingou") + with user_set(fedora_elections.APP, user, oidc_id_token="foobar"): + with patch( + "fedora_elections.OIDC.user_getfield", MagicMock(return_value=[]) + ): + + output = self.app.get("/vote/test_election3", follow_redirects=True) + output_text = output.get_data(as_text=True) + self.assertIn( + "You need to be in one another group than " + "CLA to vote", output_text ) def test_vote(self): @@ -90,13 +112,6 @@ class FlaskElectionstests(ModelFlasktests): output = self.app.get("/vote/test_election") self.assertEqual(output.status_code, 302) - output = self.app.get("/vote/test_election", follow_redirects=True) - output_text = output.get_data(as_text=True) - self.assertTrue( - "You need to be in one another group than " - "CLA to vote" in output_text - ) - user = FakeUser(["packager"], username="pingou") with user_set(fedora_elections.APP, user, oidc_id_token="foobar"): with patch(