gatekeep

[shaeespring]

Add gatekeep requirements to follow ComputerScienceHouse/Constitution#260

[Mstrodl]

Would it be better to, in addition to the result, expose the expected and actual counts for each thing? That way things like vote could explain to you why you're ineligible to vote.

You may also want to add a banner to the conditional dashboard page telling you you're ineligible to vote.

Also I don't see a check for the date here... Presumably you only need to meet these requirements by week 6? Sometimes we need to do elections or spending presentations early in the year.

Lastly... How does this interact with members who were on coop in the fall? Are they just ineligible to vote until they do a bunch of meetings? That might be a problem with the amendment itself though. Would it be better to reuse the "house meetings missed" logic for the dashboard of conditional? Unfortunately that doesn't fix the other requirements, but it gets the house meeting one dealt with.

[shaeespring]

Lastly... How does this interact with members who were on coop in the fall? Are they just ineligible to vote until they do a bunch of meetings? That might be a problem with the amendment itself though. Would it be better to reuse the "house meetings missed" logic for the dashboard of conditional? Unfortunately that doesn't fix the other requirements, but it gets the house meeting one dealt with.

Correct, they are ineligible to vote until the following semester, where for the first 6 weeks, everyone will be able to vote, and then ALL members must fulfill the requirements again if they want to vote during the semester

Unfortunately, the way the constitution is worded, currently in order to vote you must have six house meetings. That amendment is currently in the process of being ratified (hopefully). The way I wrote the gatekeep function is based upon the assumption that that amendment would fail, and I will update it in the future

[shaeespring]

Also I don't see a check for the date here... Presumably you only need to meet these requirements by week 6? Sometimes we need to do elections or spending presentations early in the year.

This will be implemented into vote via a toggle. Doing it here would spread false information that a member has passed gatekeep when they haven't... Would it be better to add that maybe to the dashboard in "member statistics"?

[tallen42]

Correct, they are ineligible to vote until the following semester, where for the first 6 weeks, everyone will be able to vote, and then ALL members must fulfill the requirements again if they want to vote during the semester

Conditional does not return to 0 HM attendances, seminars, and directorships for spring semester. Your current code would allow anyone who has passed gatekeep at any point in the operating year, not just the current semester.

[Mstrodl]

Whoops that put my thing in as a review rather than a comment... Oh well.

[Mstrodl]

This will be implemented into vote via a toggle. Doing it here would spread false information that a member has passed gatekeep when they haven't... Would it be better to add that maybe to the dashboard in "member statistics"?

To me, this seems like it's a lot of room for human error... What happens when some votes have the switch flipped and others don't? I think conditional has the brains to know when an intro eval has happened, so you could probably key off of that?

If you want a separate line item for passing the requirements I'd say that's fine, but I do think you should also have SOME way to expose whether or not you can vote right now... This would account for things like coop, gatekeep, activeness, etc.

[shaeespring]

Conditional does not return to 0 HM attendances, seminars, and directorships for spring semester. Your current code would allow anyone who has passed gatekeep at any point in the operating year, not just the current semester.

Huge! Thank you for that catch because I never would have seen that. Thank you

[shaeespring]

To me, this seems like it's a lot of room for human error... What happens when some votes have the switch flipped and others don't? I think conditional has the brains to know when an intro eval has happened, so you could probably key off of that?

This has to happen, otherwise all votes would require gatekeep, even if a member wants to make a vote about what type of cheese is best for grilled cheese. That member might want all people to vote, not just voting active members. The toggle will default to on though, since most votes will require gatekeep

If you want a separate line item for passing the requirements I'd say that's fine, but I do think you should also have SOME way to expose whether or not you can vote right now... This would account for things like coop, gatekeep, activeness, etc.

I'm looking at how conditional gives member the "voting" tag right now. It's literally a mess I will organize that code better (it's in dashboard.html). Basically, it says "if voting" which connects to the get_voting_members function (and returns that same voting list that the "voting members" shows) if a member is in that list, it gives them the voting tag, otherwise it gives them a "non-voting" tag. If they have questions about why they should look at the constitution

[Mstrodl]

This has to happen, otherwise all votes would require gatekeep, even if a member wants to make a vote about what type of cheese is best for grilled cheese. That member might want all people to vote, not just voting active members. The toggle will default to on though, since most votes will require gatekeep
Vote already requires people to not be active and not on coop... Is that going to change?

[shaeespring]

Vote already requires people to not be active and not on coop... Is that going to change?

That's outside the scope of this PR. Alumni members are not my concern. I'm adding implementation for specifically active members following ComputerScienceHouse/Constitution#260.

[cecilialau6776]

I think limiting access to this call is better done with a service account - we already have ones for services like drink and selfservice, and you could do a check on the username/email/whatever.

Personally I don't really understand why this call is being limited to vote - it would be pretty easy to pull the relevant information from LDAP + scrape /spring_evals to get the information anyway.

Regardless, I think giving vote a service account to give it authentication is a more robust solution for access control to API calls.

[tallen42]

It would be "pretty easy" to do that, but the number of people that will do that is very little (historically probably 0), considering we all rely on our Evals Director to run the queries year after year.
Having an accessible endpoint would make it significantly more convenient, and I would imagine people would actually start looking at it.

As far as the service account vs token is concerned, a token is fine... It's "more robust" but if we are hard-coding a username in as a check, that completely 180's the robustness of it. The way this works, the token could even be shared as one shared secret/configmap in OKD, and then we can mount the same secret across multiple Deployments, so it can be rotated once in all the places.

[cecilialau6776]

Oh, I didn't consider that it could be shared as a secret/configmap in OKD. Good enough, I guess.

[goosenotduck]

In what application is the /gatekeep endpoint used? I feel like that could also dictate if an account or token makes more sense

[cecilialau6776]

Vote

[goosenotduck]

Token feels fine then tbh

[cecilialau6776]

Everything else looks good to me (in hindsight i should've just done a review lol mb)

[cecilialau6776]

Oh, I didn't consider that it could be shared as a secret/configmap in OKD. Good enough, I guess.

[goosenotduck]

Looks good 👍

[costowell]

LGTM 👍

[goosenotduck]

#410 has been merged, all set 👍

[sentry-io]

Issues attributed to commits in this pull request

This pull request was merged and Sentry observed the following issues:

• ‼️ ImportError: cannot import name 'gatekeep_status'... in development