Skip to content
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

Certs <-- has and belongs to many --> Courses #706

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

masukomi
Copy link
Collaborator

@masukomi masukomi commented Jun 4, 2018

After speaking with @kgf it was decided that there should be a HABTM association between courses and certs, because a course can convey multiple certs.

This PR adds that functionality.

On a related note: I don't really understand why a you associated a cert with courses instead of a course with certs. I changed the cert form to support multiple courses. It doesn't feel quite right but i believe that it is, because there are multiple courses that could, for example, convey the same cert. For example the long First Responder / First Aid course includes CPR training but you could also have a short CPR course that conveyed the same CPR cert.

That being said, it feels wrong that you can't choose certs when creating / editing a course.

@kgf
Copy link
Member

kgf commented Jun 5, 2018

Amazingly enough, I am open to renaming the models if it would make it easier for people to understand.

There is a person. A certification is that person completing some set of hurdles, such as taking a class and a test. An example of this would be a drivers license. They studied the book and then took the driving test. They are now licensed/certified to drive a car. This certification will expire. It is for one course.
A long first aid course with CPR gives two certs; First aid and CPR. It needs to work this way, because they expire on different timelines.
You aree quite correct that it does get complicated.

@kgf
Copy link
Member

kgf commented Jul 5, 2018

@masukomi Would you be willing to freshen up this PR ? It did not pass the tests...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants