-
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 all commits
643d58e
8bbf144
5b76836
3361ecb
f9091b0
338844b
9a31d84
6fb8246
667112d
73eaeb0
60a30f8
d088351
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 |
|---|---|---|
| @@ -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']: | ||
|
|
||
| # 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 * | ||
| 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) { | ||
| $("#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.
Do we need this print statement (line 114)?