Skip to content

New Year Flow #143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 18, 2017
Merged

New Year Flow #143

merged 3 commits into from
Jun 18, 2017

Conversation

mbillow
Copy link
Member

@mbillow mbillow commented Jun 17, 2017

The evaluations director now has a simple, easy to follow flow for clearing the active group, removing everyone from their rooms, pruning the current students group, and then filling out the housing roster.

Fixes #140

@mbillow mbillow added this to the Conditional 1.5 milestone Jun 17, 2017
@mbillow mbillow self-assigned this Jun 17, 2017
@@ -11,6 +11,7 @@
from conditional.util.ldap import ldap_get_member
from conditional.util.ldap import ldap_get_roomnumber
from conditional.util.ldap import ldap_get_current_students
from conditional.util.ldap import _ldap_add_member_to_group as ldap_add_member_to_group
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're going to make this method public, let's remove the _ instead of aliasing it.

@@ -118,6 +119,8 @@ def change_room_numbers(rmnumber):
account = ldap_get_member(occupant)
account.roomNumber = rmnumber
log.info('api', action='%s assigned to room %s' % (occupant, rmnumber))
ldap_add_member_to_group(account, "active")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively to the above comment, let's provide an API for toggling active status.

@@ -35,6 +35,7 @@
from conditional.util.ldap import ldap_set_housingpoints
from conditional.util.ldap import ldap_get_active_members
from conditional.util.ldap import ldap_get_member
from conditional.util.ldap import ldap_get_current_students
from conditional.util.ldap import _ldap_add_member_to_group as ldap_add_member_to_group
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above with the private methods, although it's not technically part of this PR.

@@ -549,7 +550,7 @@ def introductory_project_submit():
def get_member(uid):
log = logger.new(user_name=request.headers.get("x-webauth-user"),
request_id=str(uuid.uuid4()))
log.info('api', action='submit introductory project results')
log.info('api', action="get %s's information" & uid)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you want "get {}'s information".format(uid) here. Or at least the & should be a %.


# Clear the active group.
for account in members:
log.info('api', action='remove %s from active status' % account.uid)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, please use Python 3 string formatting.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once this is merged I am going to be re-doing all of our logging...

});
} else if (this.uid) {
if ($('#rem-' + this.uid).is(":visible")) {
fetch(this.endpoints.current + this.uid, {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above. In addition, if you want the statements following to only be executed in the event of a successful request, they should be enclosed in a .then() on this promise.

var userRow = $('#row-' + this.uid)[0];
userRow.style.setProperty("text-decoration", "line-through");
} else {
fetch(this.endpoints.current + this.uid, {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

<p>Is it that time of year already? Before we get started, please <strong>be careful and ready everything before continuing</strong>. Some actions taken on the next few steps can be detrimental if followed in the middle of the year. Once you are ready to proceed, click 'Begin' below.</p>
<a href="#" data-module="newYear" data-step="welcome" class="btn btn-primary btn-new-next center-block"><span class="glyphicon glyphicon-ok"></span> Begin</a>
</div>
<div id="new-clear" style="display: none;">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you added a .hidden class, please use that instead of inline style here.

<p>This is the scary part. In order to get everything ready for next year, we need to clear everyone from Active status and then clear the housing board. Once we get a fresh start, we will forward you onto the housing page to fill out the new housing board.</p>
<a href="#" data-module="newYear" data-step="clear" class="btn btn-primary btn-new-next center-block"><span class="glyphicon glyphicon-erase"></span> Clear Active Group and Housing Board</a>
</div>
<div id="new-current" style="display: none;">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

</table>
<a href="#" data-module="newYear" data-step="current" class="btn btn-primary btn-new-next center-block"><span class="glyphicon glyphicon-ok"></span> Next Step</a>
</div>
<div id="new-housing" style="display: none;">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

@mbillow mbillow merged commit 163b39a into ComputerScienceHouse:develop Jun 18, 2017
@mbillow mbillow deleted the new-year branch June 18, 2017 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants