-
Notifications
You must be signed in to change notification settings - Fork 129
New/course flow #54
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: new/course-flow
Are you sure you want to change the base?
New/course flow #54
Conversation
martindolores
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 was able to pull this down and test using curl. I can confirm when inputting a user id that doesn't directly have a course map, it will return a course map that has no associated user id to it. Integration tests are also passing :)
I do have a quick question though!
app/api/courseflow/course_map_api.rb
Outdated
| if course_map.nil? | ||
| course_map = CourseMap.find_by(userId: nil) | ||
| end |
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.
What happens if there is multiple course_maps records with no userId associated to it? How do we know which default course map to grab?
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 have asked Nebula on this matter. He told me that there would only be 1 default course map template for all users regardless of their course/major, so in all cases that would be the one selected for users who do not have a course map associated with their userId.
samindiii
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.
Hello! Great job on your work :) I've gone through the code and suggested some changes! Feel free to reach out to me if you have any questions. Please remember to tag me or message me when you've made changes! Thanks :)
martindolores
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.
Great work 🤘
|
LGTM. |
Description
Added a fallback mechanism for course maps, allowing users without personal course maps to see a template/default course map. When a user requests their course map but doesn't have one configured, the system will automatically return the template course map (defined with userId=null)
Fixes # (issue)
Type of change
How Has This Been Tested?
Tests have been added that cover the following scenarios:
Checklist: