Annual Position Review Button by johnlolonga19 · Pull Request #662 · BCStudentSoftwareDevTeam/lsf

johnlolonga19 · 2026-08-03T18:15:54Z

Fixes issue #618

The Labor Office should be able to initiate the annual position review process for every department from the Manage Departments page.
This workflow requests departments to review their position descriptions and submit any necessary updates before the next academic year.

Additions

A working "Annual Position Review" button on the Manage Departments page. Clicking it opens a confirmation window explaining what's about to happen.
An "Edit Email Template" option in that confirmation window, so an admin can review or change the wording of the email before sending it.
A "Submit Request" button that actually sends the request. Once sent, every active department's supervisors and Labor Coordinators receive an email asking them to review their positions for the current academic year.
A confirmation message on screen after sending, showing how many departments the request went out to.
The system now keeps a record of when a review was requested for each department and academic year, and who requested it, so re-sending a request updates that record instead of creating duplicates.

Changes

annualPositionReview.html — the previously-stubbed confirmation modal (empty onclick="" on every button) is now fully wired: Submit Request calls the new JS, Edit Email Template links to emailTemplates linked to this template
manageDepartments.html loads annualPositionReview.js; the trigger button now carries data-academic-year so the JS knows which academic year is selected.
manageDepartments() view now also passes the chosenAY Term object into the template context (previously only chosenAY.termName was passed), needed for the data-academic-year attribute above.
emailTemplates.js added prefillFromQueryParams() so the Email Templates editor can be linked with Recipient/Form Type/Action pre-selected (used by the Edit Email Template link above)
The confirmation page and its buttons (Cancel, Edit Email Template, Submit Request) previously did nothing when clicked they're now fully functional.
The email template editor page can now be opened directly from the confirmation window with the right template already selected, instead of requiring the admin to fill it manually.
Added the test suite for annualPositionReview.py

Testing

ALWAYS_SEND_MAIL was set to False in local secret_config.yaml (untracked, not part of this PR) so local testing doesn't attempt a real SMTP send.
To submit an annual position review, on the sidebar menu, click Admin
Then click the " Manage Department " that will take you to the page
On the page click "Annual Position Review" and then " Submit Request"
A message will show that the request has been sent to x departments out of y.
Two ways to edit email template through Manage Department and Manage Email Template:
- To edit the email template inside the, click on "Edit Email Template" that will prefill the Position Review components
- Or inside Manage Email Template, fill the fields for the Position Review Email and will allow you to edit
- And you can save the changes or discard

…ed logic file for annual position review

… email template page

…eview btn AND annualPositionReview.html handles the confirmation

BhushanSah · 2026-08-04T18:33:26Z

+        supervisors, laborCoordinators = getSupervisors(department)
+        recipients = {person["email"] for person in supervisors + laborCoordinators if person["email"]}
+        if not recipients:
+            continue


Should we still create or update the PositionReview record for this department even if there are no email recipients?

The PR description says the system keeps a record of when a review was requested for each department and academic year. Right now, departments with no supervisors/coordinators are skipped before the PositionReview record is created, so those departments will have no record of the request.

BhushanSah · 2026-08-04T18:33:46Z

+*/
+  var academicYear = $('[data-target="#annualPositionModal"]').data('academic-year');
+
+  $.ajax({


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.

BhushanSah · 2026-08-04T18:34:50Z

+
+    try:
+        rsp = request.get_json()
+        result = sendAnnualPositionReviewRequests(int(rsp['academicYear']), currentUser)


Can we validate rsp and academicYear before calling int()?

If the request body is missing, not JSON, or does not include academicYear, this will raise an exception and return {"Success": false} with a 200 response. It would be cleaner to return a 400 with a clear message for bad input.

BhushanSah · 2026-08-04T18:35:17Z

+from app.logic.getSupervisors import getSupervisors
+
+
+def sendMail(mail, message: Message):


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?

i just got rid of annualPositionReview.py and move both the sendAnnualPositionReviewRequests() and sendMail() into emailHandler.py so that i can just use the function as send() to remove the duplicate

BhushanSah · 2026-08-04T18:35:45Z

+        with mainDB.atomic() as transaction:
+            Department.update(isActive=False).where(Department.isActive == True).execute()
+
+


This logic test is helpful. Can we also add a route-level test for /admin/manageDepartments/annualPositionReview?

I think we should test that non-admin users get 403 and that missing or invalid academicYear input returns a clear error. That would cover the endpoint behavior, not just the helper function.

BhushanSah · 2026-08-04T18:39:47Z

-                <button type="button" class="btn btn-primary" onclick="">Submit Request</button>
+                <button type="button" class="btn btn-secondary" data-dismiss="modal">Cancel</button>
+                <a class="btn btn-info" href="/admin/emailTemplates?audience=Department&formType=Position%20Review&action=Annual%20Request">Edit Email Template</a>
+                <button type="button" class="btn btn-primary" onclick="submitAnnualPositionReview()">Submit Request</button>


Can we disable this Submit Request button while the AJAX request is running?

Since this sends emails to all active departments, a double-click could send the same email request more than once. The PositionReview table avoids duplicate records, but the emails could still go out multiple times.

NYABUTOA · 2026-08-04T19:32:31Z

+    for department in departments:
+        supervisors, laborCoordinators = getSupervisors(department)
+        recipients = {person["email"] for person in supervisors + laborCoordinators if person["email"]}
+        if not recipients:


when a department has no supervisors/coordinators with an email, it's skipped with zero logging. No way to tell from logs which departments were skipped or why.

NYABUTOA · 2026-08-04T19:42:58Z

+      var category, msg;
+      if (response["Success"]) {
+        category = "success";
+        msg = "Position review requests sent to " + response["sentCount"] + " of " + response["departmentCount"] + " departments.";
+      } else {
+        category = "danger";
+        msg = "Something went wrong sending the Annual Position Review requests.";
+      }


the UI only shows "Success" or "Something went wrong," so an admin can't tell which departments were reached before a failure.

ArtemKurasov

The code is good overall, but I saw several print statements in the code. You should consider deleting them if they serve no purpose other than debugging

ArtemKurasov · 2026-08-04T19:58:22Z

+        result = sendAnnualPositionReviewRequests(int(rsp['academicYear']), currentUser)
+        return jsonify({"Success": True, **result})
+    except Exception as e:
+        print(e)


Do we need this print statement (line 114)?

…eview-John

created a branch off of dep_portal_ad_ManageDepartments and then add…

643d58e

…ed logic file for annual position review

johnlolonga19 self-assigned this Aug 3, 2026

johnlolonga19 added 6 commits August 3, 2026 16:40

added Annual positionn review email template

8bbf144

Add sendAnnualPositionReviewRequests logic for Annual Position Review

5b76836

Add POST route to trigger Annual Position Review requests

3361ecb

add js to anchor the request button and emailTemplates.js link to the…

f9091b0

… email template page

manageDepartments.html: handle functionality of the Annual Position r…

338844b

…eview btn AND annualPositionReview.html handles the confirmation

added the test suite for the logic

9a31d84

BhushanSah requested changes Aug 4, 2026

View reviewed changes

ArtemKurasov self-requested a review August 4, 2026 19:23

NYABUTOA reviewed Aug 4, 2026

View reviewed changes

ArtemKurasov requested changes Aug 4, 2026

View reviewed changes

johnlolonga19 added 5 commits August 4, 2026 16:46

fixed some requested changes

6fb8246

Merge branch 'dep_portal_ad_ManageDepartments' into annual-position-r…

667112d

…eview-John

fixed requested change to validate rsp and academicYear

73eaeb0

removed the duplicate fuction

60a30f8

ressolve changes

d088351

Navigation Menu

Uh oh!

Annual Position Review Button - #662

Annual Position Review Button#662
johnlolonga19 wants to merge 12 commits into
dep_portal_ad_ManageDepartmentsfrom
annual-position-review-John

johnlolonga19 commented Aug 3, 2026 •

edited

Loading

Uh oh!

BhushanSah Aug 4, 2026

Uh oh!

BhushanSah Aug 4, 2026

Uh oh!

BhushanSah Aug 4, 2026

Uh oh!

BhushanSah Aug 4, 2026

Uh oh!

johnlolonga19 Aug 4, 2026

Uh oh!

BhushanSah Aug 4, 2026

Uh oh!

BhushanSah Aug 4, 2026

Uh oh!

NYABUTOA Aug 4, 2026

Uh oh!

NYABUTOA Aug 4, 2026

Uh oh!

ArtemKurasov left a comment

Uh oh!

Uh oh!

ArtemKurasov Aug 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		from app.logic.getSupervisors import getSupervisors


		def sendMail(mail, message: Message):

		with mainDB.atomic() as transaction:
		Department.update(isActive=False).where(Department.isActive == True).execute()

Uh oh!

Conversation

johnlolonga19 commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Additions

Changes

Testing

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArtemKurasov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

johnlolonga19 commented Aug 3, 2026 •

edited

Loading