Packet 3.5 #208
Merged
Packet 3.5 #208
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Require that the image build succeeds in order to pass ci
Prevents linting failures from failing builds
* Update pylint to include pylint_quotes in readme and gulp. * Add docker build to travis to test that PRs don't break the build.
Creates /stats/packet/<packet_id>, which displays a graph of signatures over time for the given packet
Bumps [lodash](https://github.com/lodash/lodash) from 4.17.15 to 4.17.19. - [Release notes](https://github.com/lodash/lodash/releases) - [Commits](lodash/lodash@4.17.15...4.17.19) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
VivianNK
reviewed
Aug 21, 2020
mxmeinhold
approved these changes
Aug 21, 2020
There was a problem hiding this comment.
I'm gonna give this a thumbs up, but I'd like a lookover from @VivianNK and @galenguyer, since I have looked at or written everything in this PR before.
|
I'm personally ready to get this merged. I want to generate new packets tonight after Chairman's speech. |
|
Yolo-ops |
JoelEager
reviewed
Aug 23, 2020
Comment on lines
+52
to
+57
| @classmethod | ||
| def get_all(cls): | ||
| """ | ||
| Helper method to get all freshmen easily | ||
| """ | ||
| return cls.query.all() |
There was a problem hiding this comment.
Not sure if the couple of characters saved by this helper method is worth the extra level of abstraction/indirection. Not a big deal either way though.
Comment on lines
+10
to
+15
| @app.route('/admin/packets') | ||
| @log_cache | ||
| @packet_auth | ||
| @admin_auth | ||
| @before_request | ||
| @log_time |
There was a problem hiding this comment.
This is getting to be a lot of decorators. I'd consider refactoring the auth and logging ones to be function-style decorators so you can make things more compact and readable. Maybe something like this:
@app.route('/admin/packets')
@with_logs(TIME, CACHE)
@require_auth(ADMIN, PACKET)
@before_request
Comment on lines
+90
to
+93
| # Only allow evals to sync ldap | ||
| username = str(session['userinfo'].get('preferred_username', '')) | ||
| if not _ldap_is_member_of_group(ldap_get_member(username), 'eboard-evaluations'): | ||
| return 'Forbidden: not Evaluations Director', 403 |
There was a problem hiding this comment.
Shouldn't this be covered by the @admin_auth decorator?
| <div class="card mb-3"> | ||
| <div class="card-body" style="text-align: left;"> | ||
| <h5 class="card-text"> | ||
| I guess this is what you get when you trust a bunch of college kids. |
| INTRO_REALM = 'https://sso.csh.rit.edu/auth/realms/intro' | ||
|
|
||
|
|
||
| def before_request(func): |
There was a problem hiding this comment.
This decorator really needs a more descriptive name. Maybe @populate_session?
Comment on lines
+115
to
+253
|
|
||
|
|
||
| def sync_freshman(freshmen_list: dict): | ||
| freshmen_in_db = {freshman.rit_username: freshman for freshman in Freshman.query.all()} | ||
|
|
||
| for list_freshman in freshmen_list.values(): | ||
| if list_freshman.rit_username not in freshmen_in_db: | ||
| # This is a new freshman so add them to the DB | ||
| freshmen_in_db[list_freshman.rit_username] = Freshman(rit_username=list_freshman.rit_username, | ||
| name=list_freshman.name, | ||
| onfloor=list_freshman.onfloor) | ||
| db.session.add(freshmen_in_db[list_freshman.rit_username]) | ||
| else: | ||
| # This freshman is already in the DB so just update them | ||
| freshmen_in_db[list_freshman.rit_username].onfloor = list_freshman.onfloor | ||
| freshmen_in_db[list_freshman.rit_username].name = list_freshman.name | ||
|
|
||
| # Update all freshmen entries that represent people who are no longer freshmen | ||
| for freshman in filter(lambda freshman: freshman.rit_username not in freshmen_list, freshmen_in_db.values()): | ||
| freshman.onfloor = False | ||
|
|
||
| # Update the freshmen signatures of each open or future packet | ||
| for packet in Packet.query.filter(Packet.end > datetime.now()).all(): | ||
| # Handle the freshmen that are no longer onfloor | ||
| for fresh_sig in filter(lambda fresh_sig: not fresh_sig.freshman.onfloor, packet.fresh_signatures): | ||
| FreshSignature.query.filter_by(packet_id=fresh_sig.packet_id, | ||
| freshman_username=fresh_sig.freshman_username).delete() | ||
|
|
||
| # Add any new onfloor freshmen | ||
| # pylint: disable=cell-var-from-loop | ||
| current_fresh_sigs = set(map(lambda fresh_sig: fresh_sig.freshman_username, packet.fresh_signatures)) | ||
| for list_freshman in filter(lambda list_freshman: list_freshman.rit_username not in current_fresh_sigs and | ||
| list_freshman.onfloor and | ||
| list_freshman.rit_username != packet.freshman_username, | ||
| freshmen_list.values()): | ||
| db.session.add(FreshSignature(packet=packet, freshman=freshmen_in_db[list_freshman.rit_username])) | ||
|
|
||
| db.session.commit() | ||
|
|
||
|
|
||
| def create_new_packets(base_date: date, freshmen_list: dict): | ||
| packet_start_time = time(hour=19) | ||
| packet_end_time = time(hour=21) | ||
| start = datetime.combine(base_date, packet_start_time) | ||
| end = datetime.combine(base_date, packet_end_time) + timedelta(days=14) | ||
|
|
||
| print('Fetching data from LDAP...') | ||
| all_upper = list(filter( | ||
| lambda member: not ldap_is_intromember(member) and not ldap_is_on_coop(member), ldap_get_active_members())) | ||
|
|
||
| rtp = ldap_get_active_rtps() | ||
| three_da = ldap_get_3das() | ||
| webmaster = ldap_get_webmasters() | ||
| c_m = ldap_get_constitutional_maintainers() | ||
| drink = ldap_get_drink_admins() | ||
|
|
||
| # Packet starting notifications | ||
| packets_starting_notification(start) | ||
|
|
||
| # Create the new packets and the signatures for each freshman in the given CSV | ||
| print('Creating DB entries and sending emails...') | ||
| for freshman in Freshman.query.filter(Freshman.rit_username.in_(freshmen_list)).all(): | ||
| packet = Packet(freshman=freshman, start=start, end=end) | ||
| db.session.add(packet) | ||
| send_start_packet_mail(packet) | ||
| packet_starting_notification(packet) | ||
|
|
||
| for member in all_upper: | ||
| sig = UpperSignature(packet=packet, member=member.uid) | ||
| sig.eboard = ldap_get_eboard_role(member) | ||
| sig.active_rtp = member.uid in rtp | ||
| sig.three_da = member.uid in three_da | ||
| sig.webmaster = member.uid in webmaster | ||
| sig.c_m = member.uid in c_m | ||
| sig.drink_admin = member.uid in drink | ||
| db.session.add(sig) | ||
|
|
||
| for onfloor_freshman in Freshman.query.filter_by(onfloor=True).filter(Freshman.rit_username != | ||
| freshman.rit_username).all(): | ||
| db.session.add(FreshSignature(packet=packet, freshman=onfloor_freshman)) | ||
|
|
||
| db.session.commit() | ||
|
|
||
|
|
||
| def sync_with_ldap(): | ||
| print('Fetching data from LDAP...') | ||
| all_upper = {member.uid: member for member in filter( | ||
| lambda member: not ldap_is_intromember(member) and not ldap_is_on_coop(member), ldap_get_active_members())} | ||
|
|
||
| rtp = ldap_get_active_rtps() | ||
| three_da = ldap_get_3das() | ||
| webmaster = ldap_get_webmasters() | ||
| c_m = ldap_get_constitutional_maintainers() | ||
| drink = ldap_get_drink_admins() | ||
|
|
||
| print('Applying updates to the DB...') | ||
| for packet in Packet.query.filter(Packet.end > datetime.now()).all(): | ||
| # Update the role state of all UpperSignatures | ||
| for sig in filter(lambda sig: sig.member in all_upper, packet.upper_signatures): | ||
| sig.eboard = ldap_get_eboard_role(all_upper[sig.member]) | ||
| sig.active_rtp = sig.member in rtp | ||
| sig.three_da = sig.member in three_da | ||
| sig.webmaster = sig.member in webmaster | ||
| sig.c_m = sig.member in c_m | ||
| sig.drink_admin = sig.member in drink | ||
|
|
||
| # Migrate UpperSignatures that are from accounts that are not active anymore | ||
| for sig in filter(lambda sig: sig.member not in all_upper, packet.upper_signatures): | ||
| UpperSignature.query.filter_by(packet_id=packet.id, member=sig.member).delete() | ||
| if sig.signed: | ||
| sig = MiscSignature(packet=packet, member=sig.member) | ||
| db.session.add(sig) | ||
|
|
||
| # Migrate MiscSignatures that are from accounts that are now active members | ||
| for sig in filter(lambda sig: sig.member in all_upper, packet.misc_signatures): | ||
| MiscSignature.query.filter_by(packet_id=packet.id, member=sig.member).delete() | ||
| sig = UpperSignature(packet=packet, member=sig.member, signed=True) | ||
| sig.eboard = ldap_get_eboard_role(all_upper[sig.member]) | ||
| sig.active_rtp = sig.member in rtp | ||
| sig.three_da = sig.member in three_da | ||
| sig.webmaster = sig.member in webmaster | ||
| sig.c_m = sig.member in c_m | ||
| sig.drink_admin = sig.member in drink | ||
| db.session.add(sig) | ||
|
|
||
| # Create UpperSignatures for any new active members | ||
| # pylint: disable=cell-var-from-loop | ||
| upper_sigs = set(map(lambda sig: sig.member, packet.upper_signatures)) | ||
| for member in filter(lambda member: member not in upper_sigs, all_upper): | ||
| sig = UpperSignature(packet=packet, member=member) | ||
| sig.eboard = ldap_get_eboard_role(all_upper[sig.member]) | ||
| sig.active_rtp = sig.member in rtp | ||
| sig.three_da = sig.member in three_da | ||
| sig.webmaster = sig.member in webmaster | ||
| sig.c_m = sig.member in c_m | ||
| sig.drink_admin = sig.member in drink | ||
| db.session.add(sig) | ||
|
|
||
| db.session.commit() |
There was a problem hiding this comment.
These functions probably should be broken out into a separate file. My philosophy is that utils files are fine as long as they are short and only contain a couple things that don't really need their own home. Once they get to this length though it's time to start breaking some of the functions out by category.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This update adds: