Validation for section with participant type #44271
Conversation
|
my top-level feedback is that the UI error messages sound a bit too technical to be understood by the user. Are these strings coming from the spec? if not, can we get some input from Tess? Of course, making better error messages could mean more work (e.g. to differentiate between "this course is only available for teachers" vs "students") |
| if participant_type_changed? && persisted? | ||
| errors.add(:participant_type, "can not be update once set.") | ||
| end | ||
| end |
| elsif participant_type == SharedCourseConstants::PARTICIPANT_AUDIENCE.teacher | ||
| return user.teacher? | ||
| elsif participant_type == SharedCourseConstants::PARTICIPANT_AUDIENCE.student | ||
| return true #if participant_type is student let anyone join |
There was a problem hiding this comment.
This comment looks off because it is saying what the code is doing (pretty clear in this case) instead of why it is doing it. It seems like the point of this comment is to remind people that you do not actually have to exactly match the participant type in order to be eligible, is that correct? if so, I would suggest removing this comment and augmenting the method-level comment to say something like: "participant does not have to exactly match the participant type to join the section, for example facilitators can join any section and anyone can join a student section."
https://blog.codinghorror.com/code-tells-you-how-comments-tell-you-why/
|
I'm going to merge this now. I have asked for Tess input on the error strings but since nobody can set a section to a participant type other than student right now nobody should see them yet. |
Adds a bunch of validation and conditions on the new participant_type field for sections:
Links
Eng Plan
Product Spec
Testing story