-
Notifications
You must be signed in to change notification settings - Fork 6
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
allow student to update graduation year on 360 #944
Conversation
There’s one policy issue with the Registrar for this ability for a student to change the cohort with which they wish to be associated. Its okay to pursue this option, however, before making it live a procedural conversation with Advising and the Registrar should be made. I can start that if this looks like a promising feature.
As a backup option, could the feature instead prompt a message to the Registar indicating the student wants to make a change, or redirect the student to the Registrar’s required form - if that becomes important in the process map for data confirmation for the system of record?
On 07/11/23, 09:52, "Amos Cha" ***@***.******@***.***>> wrote:
** CAUTION: External Email: Do not click links or open attachments unless you recognize sender and know content is safe.**
@amos-cha requested changes on this pull request.
________________________________
In Gordon360/Services/ProfileService.cs<#944 (comment)>:
+ /// <summary>
+ /// update planned graduation
+ /// </summary>
+ /// <param name="username">The username for the user whose planned graduation year is to be updated</param>
+ /// <param name="newPlannedGraduationYear">The new year to update the user's planned graduation year to</param>
+ public async Task<StudentProfileViewModel> UpdatePlannedGraduationYearAsync(string username, string newPlannedGraduationYear)
+ {
+ var profile = GetStudentProfileByUsername(username);
+ if (profile == null)
+ {
+ throw new ResourceNotFoundException() { ExceptionMessage = "The account was not found" };
+ }
+
+ // Update value in cached data
+
+ var student = _context.CUSTOM_PROFILE.FirstOrDefault(x => x.username == username);
⬇️ Suggested change
- var student = _context.CUSTOM_PROFILE.FirstOrDefault(x => x.username == username);
+ var student = _context.CUSTOM_PROFILE.Find(username);
If I'm not mistaken custom_profile has a primary key of ad_username
________________________________
In Gordon360/Services/ProfileService.cs<#944 (comment)>:
+ return profile;
+ }
+
+ /// <summary>
+ /// privacy setting of mobile phone.
+ /// </summary>
+ /// <param name="username">AD Username</param>
+ /// <param name="value">Y or N</param>
+ public async Task UpdatePlannedGraduationYearPrivacyAsync(string username, string value)
+ {
+ var account = _accountService.GetAccountByUsername(username);
+ // Update value in cached data
+ var student = _context.CUSTOM_PROFILE.FirstOrDefault(x => x.username == username);
+ if (student != null)
+ {
+ student.IsPlannedGradYearPrivate = (value == "Y" ? 1 : 0);
⬇️ Suggested change
- student.IsPlannedGradYearPrivate = (value == "Y" ? 1 : 0);
+ student.IsPlannedGradYearPrivate = value == "Y"
________________________________
In Gordon360/Services/ProfileService.cs<#944 (comment)>:
+ var profile = GetStudentProfileByUsername(username);
+ if (profile == null)
+ {
+ throw new ResourceNotFoundException() { ExceptionMessage = "The account was not found" };
+ }
⬇️ Suggested change
- var profile = GetStudentProfileByUsername(username);
- if (profile == null)
- {
- throw new ResourceNotFoundException() { ExceptionMessage = "The account was not found" };
- }
+ var profile = GetStudentProfileByUsername(username);
+ if (profile == null)
+ {
+ throw new ResourceNotFoundException() { ExceptionMessage = "The account was not found" };
+ }
Why do we get the get the profile here if we don't use it? Also I don't know how this table is scripted if at all, but in the event a student doesn't have a CUSTOM_PROFILE row, what happens?
—
Reply to this email directly, view it on GitHub<#944 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AKLQOYTLPANVIQVMBQWUICDXPVLBXANCNFSM6AAAAAAZ7NT6J4>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
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.
lgtm
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.
fix the conflicts and i can re-approve
f5d4930
to
445ff64
Compare
All requested changes have been resolved
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.
LGTM
IMPORTANT NOTE: The column PlannedGradYear (varchar(4), null) in the CUSTOM_PROFILE table in CCT was added in the Train Database, before merging to production it is needed to check the need for adding it to Prod Database |
Already added in Prod database by @amos-cha |
Fixes #730
This PR is for allowing student to update graduation year on 360.