Reimplemented StudentQuizzesController #112
Conversation
| review_response_maps.each do |response_map| | ||
| reviewee_team = Team.find(response_map.reviewee_id) | ||
|
|
||
| next unless reviewee_team.parent_id == assignment_id |
There was a problem hiding this comment.
Why do we need to cycle through all the response maps? Couldn't we just do a find to find the response map with the right parent?
| redirect_to action: :fetch_available_quizzes_for_reviewer, assignment_id: params[:assignment_id], questionnaire_id: response.response_map.reviewed_object_id, map_id: map.id | ||
| end | ||
| else | ||
| flash[:error] = 'You have already taken this quiz, below are the records for your responses.' |
There was a problem hiding this comment.
"below is the record of your responses."
app/models/question.rb
Outdated
| validates :seq, presence: true, numericality: true # sequence must be numeric | ||
| validates :txt, length: { minimum: 0, allow_nil: false, message: "can't be nil" } # user must define text content for a question | ||
| validates :question_type, presence: true # user must define type for a question | ||
| validates :break_before, presence: true | ||
|
|
||
| QUESTION_TYPES = { | ||
| 'MultipleChoiceCheckbox' => :calculate_score_for_checkbox_question, |
There was a problem hiding this comment.
"MultipleChocieTextbox" --> "Checkbox"
"MultipleChoiceRadio" --> "MultipleChoice"
app/models/question.rb
Outdated
| validates :seq, presence: true, numericality: true # sequence must be numeric | ||
| validates :txt, length: { minimum: 0, allow_nil: false, message: "can't be nil" } # user must define text content for a question | ||
| validates :question_type, presence: true # user must define type for a question | ||
| validates :break_before, presence: true | ||
|
|
||
| QUESTION_TYPES = { | ||
| 'MultipleChoiceCheckbox' => :calculate_score_for_checkbox_question, | ||
| 'TrueFalse' => :calculate_score_for_truefalse_question, |
There was a problem hiding this comment.
Method name is too complicated. Couldn't there just be a score method implemented in Response, that could be overridden where necessary?
-Ed
| @quiz_score = @response.aggregate_questionnaire_score # Use the score calculated by Response model | ||
| end | ||
|
|
||
| # Fetches quizzes for a given assignment that a reviewer has not yet started. |
There was a problem hiding this comment.
A better name for this method would be "unattempted_quizzes".
| # Submits the quiz response and calculates the score. | ||
| def submit_quiz | ||
| map = ResponseMap.find(params[:map_id]) | ||
| # Check if there is any response for this map_id. This is to prevent student from taking the same quiz twice. |
There was a problem hiding this comment.
You are embedding a POLICY into this mechanism. Bad design! Let the instructor decide whether someone can take a quiz over, e.g., to improve their score.
| flash[:error] = 'Please answer every question.' | ||
| redirect_to action: :fetch_available_quizzes_for_reviewer, assignment_id: params[:assignment_id], questionnaire_id: response.response_map.reviewed_object_id, map_id: map.id | ||
| end | ||
| else |
There was a problem hiding this comment.
This is another POLICY that belongs elsewhere. Did you ever skip a question on a test? Why don't you want to let other students do that?
No description provided.