-
Notifications
You must be signed in to change notification settings - Fork 0
527 student reasons #550
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: development
Are you sure you want to change the base?
527 student reasons #550
Conversation
ojmakinde
left a comment
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.
ojmakinde
left a comment
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.
I ran the branch, created a new LSF, got the student to accept it, and while it was pending, I changed the number of hours to 20, triggering an overload. After doing that, an email was sent, and when I clicked the email link, I got this bug:
This is also what the email looked like, just to ensure I was following the correct instructions:
|
Also, why don't you put information regarding the overload in the email? It's a bit bland and doesn't provide any information about the role or what's being adjusted from first glance |
This was how it was set up already... I just used the pre-existing template. |
| if fieldName == "department": | ||
| oldDept = Department.get(Department.ORG == oldValue) | ||
| newDept = Department.get(Department.ORG == newValue) | ||
| oldValue = oldDept.DEPT_NAME | ||
| newValue = newDept.DEPT_NAME |
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.
This is strange. If we assume there can only be one adjusted field, then this department change would not lead to an overload. Since you're treating adjusted change like there's only one possible change, you're overwriting the old value and new value that I would assume to be the hours. This implementation is a bit strange and we should talk about it
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.
Still unresolved
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.
I think the adjusted change allows for multiple adjustments, not just one. Also, if the adjustment happens to be only the department then that will be treated as a regular adjustment and not an overload adjustment.
… into 527StudentReasons



Summary of Changes
This update improves the Labor Status Form adjustment workflow by enhancing email communication and making adjustment details visible to users.
Key Changes
Added a new email template for labor status form adjustments, including a link for supervisors/students to review and add adjustment reasons.
Updated the adjustment form UI so users can now see which fields were changed compared to the original submitted form.
Fixed the issue where adjustment emails were previously empty — the email now correctly includes the message: “Labor Status Form Adjusted” along with the adjustment link.
How to Test
Switch to the 527StudentReasons branch.
Create a Labor Status Form.
Approve the form.
Make an Adjustment to the approved form.
Check your email — you should now receive a populated adjustment email containing:
“Labor Status Form Adjusted”
A link to review/add adjustment reasons
(Previously, this email was blank.)
Expected Results
Adjustment email is no longer empty.
The email includes the correct template and link.
The adjustment form clearly displays old vs. new values as well as an option for the student to add their reason for wanting an adjustment.