-
Notifications
You must be signed in to change notification settings - Fork 5
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
Spike: Support multiple teachers per class #456
base: main
Are you sure you want to change the base?
Conversation
@@ -1,6 +1,6 @@ | |||
# frozen_string_literal: true | |||
|
|||
class ClassMember | |||
module ClassMember |
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'm being curios, why do switch class here to a module?
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.
Because we renamed the class_member table in the database to class_student, and also renamed the ClassMember model/class to ClassStudent. So we don't really care about creating instances of ClassMember anymore, just in using the methods it provides. These methods in some cases were taking account of both teachers and students, even though instances of the old ClassMember model could only represent students, which was confusing, hence the change.
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.
Thanks for open anwer👏
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.
Approved👍
Status
Points for consideration:
What's changed?
Description of what's been done - bullets are often best
Steps to perform after deploying to production
If the production environment requires any extra work after this PR has been deployed detail it here. This could be running a Rake task, a migration, or upgrading a Gem. That kind of thing.