-
Notifications
You must be signed in to change notification settings - Fork 0
Annual Position Review Button #662
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
base: dep_portal_ad_ManageDepartments
Are you sure you want to change the base?
Changes from 14 commits
643d58e
8bbf144
5b76836
3361ecb
f9091b0
338844b
9a31d84
6fb8246
667112d
73eaeb0
60a30f8
d088351
fd6e4c5
48d3a54
f6366b3
80aa951
b92ba12
f0f122e
829421e
9279efc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,8 @@ | |
| from app.models.allocation import * | ||
| from app.models.laborStatusForm import * | ||
|
|
||
| from app.logic.manageDepartments import * | ||
| from app.logic.manageDepartments import * | ||
| from app.logic.emailHandler import sendAnnualPositionReviewRequests | ||
|
|
||
|
|
||
|
|
||
|
|
@@ -67,6 +68,7 @@ def manageDepartments(academicYear = None): | |
| currentAY = currentAY, | ||
| previousAY = previousAY, | ||
| nextAY = nextAY, | ||
| chosenAY = chosenAY, | ||
| academicYear = chosenAY.termName, | ||
| breakHoursByDepartment = breakHoursByDepartment, | ||
| allocationStatus = allocationStatus | ||
|
|
@@ -93,6 +95,34 @@ def complianceStatusCheck(): | |
|
|
||
|
|
||
|
|
||
| @admin.route('/admin/manageDepartments/annualPositionReview', methods=['POST']) | ||
| def annualPositionReviewRequest(): | ||
| """ | ||
| Sends an Annual Position Review request email to every active department's | ||
| Labor Coordinators and supervisors for the selected academic year, and | ||
| records the request. Triggered from the Manage Departments page. | ||
| """ | ||
| currentUser = require_login() | ||
| if not currentUser or not currentUser.isLaborAdmin: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we also need to give access to isLaborDepartmentStudent as well I have checked the controllers and we do give access. |
||
| return jsonify({"Success": False}), 403 | ||
|
|
||
| rsp = request.get_json(silent=True) | ||
| if not rsp or "academicYear" not in rsp: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. from line 110 to 122 can be consolidate into two try and except with the first try doing academic year and second try doing result and returning success jsonify. |
||
| return jsonify({"Success": False, "message": "Request must include academicYear."}), 400 | ||
|
|
||
| try: | ||
| academicYear = int(rsp["academicYear"]) | ||
| except (TypeError, ValueError): | ||
| return jsonify({"Success": False, "message": "academicYear must be a valid integer."}), 400 | ||
|
|
||
| try: | ||
| result = sendAnnualPositionReviewRequests(academicYear, currentUser) | ||
| return jsonify({"Success": True, **result}) | ||
| except Exception: | ||
| return jsonify({"Success": False}) | ||
|
|
||
|
|
||
|
|
||
| @admin.route('/admin/manageDepartments/<org>/<account>/allocationReview', methods=['GET']) | ||
| def allocationReview(org=None, account=None): | ||
| """ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| from datetime import datetime | ||
|
|
||
| from flask_mail import Mail, Message | ||
|
|
||
| from app import app | ||
| from app.models.department import Department | ||
| from app.models.emailTemplate import EmailTemplate | ||
| from app.models.positionReview import PositionReview | ||
| from app.models.term import Term | ||
| from app.logic.getSupervisors import getSupervisors | ||
|
|
||
|
|
||
| def sendMail(mail, message: Message): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This sendMail function looks very similar to the existing emailHandler.send logic. Can we reuse the existing email sending helper or extract the shared behavior so we do not have two versions of the same mail override / reply_to / testing behavior?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i just got rid of |
||
| if app.config['ENV'] == 'production' or app.config['ALWAYS_SEND_MAIL']: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. emailHandler page has a emailhandler class but it runs on id, that means you will have to expand the emailhandler to be flexible. This means emailhandler can have a second optional parameter and turn the id parameter into a default too. For the second parameter turn it to default and have a condition in the init so that when one of the parameter is added the class knows what to create. This means you won't need this file while you incorporate sendannualpositionreviewrequests into the emailhandler function which is good as we would be doing similar with allocation request |
||
|
|
||
| # If we have set an override address | ||
| if app.config['MAIL_OVERRIDE_ALL']: | ||
| message.html = "<b>Original message intended for {}.</b><br>".format(", ".join(message.recipients)) + message.html | ||
| message.recipients = [app.config['MAIL_OVERRIDE_ALL']] | ||
|
|
||
| message.reply_to = app.config["REPLY_TO_ADDRESS"] | ||
| mail.send(message) | ||
|
|
||
| elif app.config['ENV'] == 'testing': | ||
| pass | ||
| else: | ||
| print("ENV: {}. Email not sent to {}, subject '{}'.".format(app.config['ENV'], message.recipients, message.subject)) | ||
|
|
||
|
|
||
| def sendAnnualPositionReviewRequests(academicYearTermCode, requestingUser): | ||
| """ | ||
| Sends an Annual Position Review request email to every active department's | ||
| Labor Coordinators and supervisors, and records that the request was made | ||
| for the given academic year. | ||
| """ | ||
| mail = Mail(app) | ||
| term = Term.get(Term.termCode == academicYearTermCode) | ||
| template = EmailTemplate.get(EmailTemplate.purpose == "Annual Position Review Request") | ||
| departments = Department.select().where(Department.isActive == True) | ||
|
|
||
| sentCount = 0 | ||
| for department in departments: | ||
| # A review is considered "requested" for every active department as soon | ||
| # as this runs, whether or not there's currently anyone to email - a | ||
| # department with no supervisors/coordinators assigned is itself worth | ||
| # surfacing, not skipping the department. | ||
| existingReview = PositionReview.get_or_none( | ||
| PositionReview.academicYear == term, | ||
| PositionReview.department == department | ||
| ) | ||
| if existingReview: | ||
| existingReview.requestedOn = datetime.now() | ||
| existingReview.requestedBy = requestingUser | ||
| existingReview.save() | ||
| else: | ||
| PositionReview.create( | ||
| academicYear=term, | ||
| department=department, | ||
| requestedOn=datetime.now(), | ||
| requestedBy=requestingUser | ||
| ) | ||
|
|
||
| supervisors, laborCoordinators = getSupervisors(department) | ||
| recipients = {person["email"] for person in supervisors + laborCoordinators if person["email"]} | ||
| if not recipients: | ||
| continue | ||
|
|
||
| subject = template.subject.replace("@@AcademicYear@@", term.termName) | ||
| body = template.body.replace("@@Department@@", department.DEPT_NAME).replace("@@AcademicYear@@", term.termName) | ||
|
|
||
| message = Message(subject, recipients=list(recipients)) | ||
| message.html = body | ||
| sendMail(mail, message) | ||
|
|
||
| sentCount += 1 | ||
|
|
||
| return {"sentCount": sentCount, "departmentCount": departments.count()} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| from app.models import * | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should try to work off from positionhistory model |
||
| from app.models.department import Department | ||
| from app.models.term import Term | ||
| from app.models.user import User | ||
|
|
||
|
|
||
| class PositionReview(baseModel): | ||
| academicYear = ForeignKeyField(Term) | ||
| department = ForeignKeyField(Department) | ||
| requestedOn = DateTimeField() | ||
| requestedBy = ForeignKeyField(User) | ||
|
|
||
| class Meta: | ||
| indexes = ( (('academicYear', 'department'), True), ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| function submitAnnualPositionReview(button) { | ||
| /* | ||
| POSTs the Annual Position Review request for the currently selected academic year. | ||
| Sends a review request email to every active department's Labor Coordinators and | ||
| supervisors, then shows a success/failure flash message. | ||
|
|
||
| RETURNS: None | ||
| */ | ||
| var academicYear = $('[data-target="#annualPositionModal"]').data('academic-year'); | ||
|
|
||
| // Disable immediately so a double-click can't fire this request twice - | ||
| // PositionReview dedupes the record, but the emails would still go out | ||
| // more than once. Re-enabled in complete regardless of outcome so a retry | ||
| // after a failure is possible without reloading the page. | ||
| $(button).prop("disabled", true); | ||
|
|
||
| $.ajax({ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add an error callback here? Right now, if the server returns 403, 500, or the request fails, the modal will likely stay open and the user will not get a clear message. The success handler handles {"Success": false}, but it does not handle actual AJAX errors. |
||
| method: "POST", | ||
| url: "/admin/manageDepartments/annualPositionReview", | ||
| dataType: "json", | ||
| contentType: "application/json", | ||
| data: JSON.stringify({"academicYear": academicYear}), | ||
| success: function(response) { | ||
| $("#annualPositionModal").modal("hide"); | ||
|
|
||
| if (response["Success"]) { | ||
| flashMessage("success", "Position review requests sent to " + response["sentCount"] + " of " + response["departmentCount"] + " departments."); | ||
| } else { | ||
| flashMessage("danger", "Something went wrong sending the Annual Position Review requests."); | ||
| } | ||
| }, | ||
| error: function(jqXHR) { | ||
| // Covers cases success: never sees - a 403 (not a labor admin), a 500, | ||
| // or the request failing outright. Leaves the modal open so the admin | ||
| // can retry instead of silently doing nothing. | ||
| var msg = jqXHR.status === 403 | ||
| ? "You don't have permission to send Annual Position Review requests." | ||
| : "Something went wrong sending the Annual Position Review requests."; | ||
| flashMessage("danger", msg); | ||
| }, | ||
| complete: function() { | ||
| $(button).prop("disabled", false); | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| function flashMessage(category, msg) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is a flash message function in base.js |
||
| $("#flash_container").html('<div class="alert alert-'+ category +'" role="alert" id="flasher">'+msg+'</div>'); | ||
| $("#flasher").delay(3000).fadeOut(); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using g.openterm will create a lot of bugs in case user have two open term, if the position review is for next ay while the openterm is for this ay