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
Scholarship dropdowns set course #28278
Conversation
def self.update_or_create(user, application_year, scholarship_status) | ||
scholarship_info = Pd::ScholarshipInfo.find_by(user: user, application_year: application_year) || Pd::ScholarshipInfo.new(user: user) | ||
scholarship_info.update(scholarship_status: scholarship_status) | ||
def self.update_or_create(user, application_year, course, scholarship_status) |
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.
At a certain number of parameters, a hash might be better? Not sure we're there yet, though...
@@ -65,11 +65,13 @@ def update_user_school_info! | |||
end | |||
|
|||
def update_scholarship_status(scholarship_status) | |||
Pd::ScholarshipInfo.update_or_create(user, application_year, scholarship_status) | |||
Pd::ScholarshipInfo.update_or_create(user, application_year, course_name, scholarship_status) |
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.
Is course_name
a user-friendly string (e.g. "CS Discoveries"
) or a short code (e.g. "csd"
)?
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.
user-friendly, so it matches the course
we store for workshops.
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.
but based on your feedback here I'm going to switch to the simplfied course code ('csd')
Part of PLC-165
Now that we have added the
course
column toScholarshipInfo
, the existing scholarship dropdowns need to set it.