E1981

README.md

@@ -1,3 +1,47 @@
+This is forked for course project in CSC517
+
+E1981. Student-generated questions added to rubric
+-----
+
+Mentor: Yashad Trivedi (ytrived@ncsu.edu)
+
+The idea:  Instructors make up rubrics in Expertiza. They can ask about anything that is relevant to all the projects that will be submitted. But sometimes students want specific advice on aspects of their work that may be different from the work or topics that other students are working on. It would surely be nice if they could ask reviewers to answer questions specific to their project.
+
+What needs to be done/changed:  In Expertiza, all kinds of rubrics and surveys are subclasses of Questionnaire. A Questionnaire can contain “questions” of several types (e.g., checkboxes, dropdowns, text boxes). You should add a new subclass of Questionnaire called, say, SupplementalReviewQuestionnaire (I know that’s awkward, but I can’t think of anything better).
+
+- An assignment could have a checkbox (on the Review Strategy tab of assignment creation) that, if and when checked, would enable assignment participants to create a supplemental review rubric.  I would suggest putting a button or link on the page that appears when a student clicks on “Your work”.  This button would take the student to the same page that an instructor lands on when creating a new rubric (select “Questionnaires” from the menu at the top, and then, e.g., “New private item” from the “Review” line).  The student could then create a review rubric just like the author does.
+
+- This rubric would be created on behalf of the author. You would have to add a field to the teams table so that the rubric could be stored in an AssignmentTeam. I would suggest that this field be called, supplemental_review_question_id (note the distinction between the class (SupplementalReviewQuestionnaire) and the instance of the questionnaire (SupplementalReviewQuestions)).
+
+- When a reviewer fills out a rubric, the ResponseController should display a set of rubrics, in order, on the same page. This set would normally consist of just a review rubric (a Response object), but in this case the set would have a review rubric and a supplemental review rubric.  That is, there would be two items in the set instead of one. And in general, the ResponseController should be written so that it always iterates through a set of rubrics, though in all other instances, there would be only one element (a single rubric) in this set.
+
+Student View: Then there is the question of how students would see feedback given on such a rubric. The “View” function for a rubric should display answers submitted for the SupplementalReviewQuestionnaire as well as the ReviewQuestionnaire.  And it probably makes sense to add another column to the “View scores” page (for both instructor and students) to report the scores that students gave on these questions (if indeed, any of them were scored questions).  I would not worry about including scores on student-generated questions in the overall score for the (regular) review, since doing so might encourage students to ask “easy” questions so that their reviewers would give them a lot of high scores.
+
+Note: This project has been done twice before.  Here are the links to the project from last fall.
+- http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1879_Student_Generated_Questions_Added_To_Rubric 
+- https://github.com/zyczyh/expertiza 
+- https://github.com/expertiza/expertiza/pull/1325 
+- https://youtu.be/3PUNknSbU-k 
+
+That implementation had several problems.
+- Design problems
+  - The review rubric is divided into “Regular Review Questions” and “Supplementary [Supplemental] Review Questions.”  There are several issues with this.
+    - Rubrics can already be divided into sections (cf. Program 2).  Adding an extra set of headers that no one could get rid of would make the rubric confusing.
+    - The method create_supplemental_review_questionnaire is in questionnaires_controller.  This is a bad choice, because it clutters questionnaires_controller.  Make supplemental_review_questionnaire a subclass of Questionnaire or ReviewQuestionnaire, and give it its own create method.
+    - There are four tests for whether there is a supplemental review questionnaire.  Checks like this disturb the flow of the method they are in, and mean that anyone who reads that method also has to know about student-generated “questions.”  When the reader of a class has to know about large numbers of extraneous items to understand the code, the code becomes unreadable.
+    - It would be really nice if “regular” and supplemental items could be mixed in the same rubric.  This could be done if supplemental questions are assigned a sequence number just like other questions.  Think about how this can cleanly be worked into the design (should Supplemental be a subclass of Question?). 
+  - Calculation as to whether an instructor can edit the page is done in controller code; it should be in model code.
+  - Messy pull request
+    - The pull request did not include the latest refactoring, and indeed, tried to revert several refactorings that were done while the project was in progress.
+    - The pull request included the first OSS project of one of the team members (E1850).  This is a show-stopper if we don’t want to merge that first project.
+
+It was also done in Spring 2018. Here is the link to that pull request. Although we rejected this pull request for the reasons below, there may still be some use to reading the code.
+
+- Use "SupplementalReviewQuestionnaire" instead of "srq" will be clearer for later developers.
+- In questionnaire_controller.rb Line 372: Questionnaire creation code can use `create` instead. And it is unnecessary to use the instance variable. Also, you can make values as parameters, instead of assigning them one by one.
+- There are a couple of DRY violations, but more importantly, response_controller checks in 9(!) places for "supplementary review questions."  Checking more than once or twice is a definite no-no.
+- Their tests are shallow - test irrelevant, unlikely-to-fail functionality.
+
 Expertiza
 =========
 

app/controllers/assessment360_controller.rb

@@ -160,8 +160,9 @@ def populate_hash_for_all_students_all_reviews(assignment,
   # The peer review score is taken from the questions for the assignment
   def find_peer_review_score(user_id, assignment_id)
     participant = AssignmentParticipant.find_by(user_id: user_id, parent_id: assignment_id)
+    team_id = TeamsUser.team_id(participant.parent_id, participant.user_id)
     assignment = participant.assignment
-    questions = retrieve_questions assignment.questionnaires, assignment_id
+    questions = retrieve_questions assignment.questionnaires, assignment_id, team_id 
 
     participant.scores(questions)
   end

app/controllers/grades_controller.rb

@@ -64,7 +64,7 @@ def view_my_scores
     return if redirect_when_disallowed
     @assignment = @participant.assignment
     questionnaires = @assignment.questionnaires
-    @questions = retrieve_questions questionnaires, @assignment.id
+    @questions = retrieve_questions questionnaires, @assignment.id, @team_id
     # @pscore has the newest versions of response for each response map, and only one for each response map (unless it is vary rubric by round)
     @pscore = @participant.scores(@questions)
     make_chart
@@ -86,7 +86,7 @@ def view_team
     @team = @participant.team
     @team_id = @team.id
     questionnaires = @assignment.questionnaires
-    @questions = retrieve_questions questionnaires, @assignment.id
+    @questions = retrieve_questions questionnaires, @assignment.id, @team_id
     @pscore = @participant.scores(@questions)
     @vmlist = []
 
@@ -107,6 +107,17 @@ def view_team
       end
       vm = VmQuestionResponse.new(questionnaire, @assignment, @round)
       vmquestions = questionnaire.questions
+
+      # add supplementary questions
+      supplementary_review_questionnaire_id = Team.get_supplementary_review_questionnaire_id_of_team(@team_id)
+      unless supplementary_review_questionnaire_id.nil?
+        supplementary_review_questionnaire = Questionnaire.find(supplementary_review_questionnaire_id)
+        unless supplementary_review_questionnaire.nil?
+          supplementary_review_questions = supplementary_review_questionnaire.questions
+          vmquestions += supplementary_review_questions
+        end
+      end
+
       vm.add_questions(vmquestions)
       vm.add_team_members(@team)
       vm.add_reviews(@participant, @team, @assignment.varying_rubrics_by_round?)

app/controllers/questionnaires_controller.rb

@@ -10,11 +10,16 @@ class QuestionnairesController < ApplicationController
   def action_allowed?
     if params[:action] == "edit"
       @questionnaire = Questionnaire.find(params[:id])
+      unless @questionnaire.nil?
+        if @questionnaire.type == "SupplementaryReviewQuestionnaire"
+          return ['Student'].include? current_role_name
+        end
+      end
       (['Super-Administrator',
         'Administrator'].include? current_role_name) ||
           ((['Instructor'].include? current_role_name) && current_user_id?(@questionnaire.try(:instructor_id))) ||
           ((['Teaching Assistant'].include? current_role_name) && session[:user].instructor_id == @questionnaire.try(:instructor_id))
-
+          
     else
       ['Super-Administrator',
        'Administrator',

app/controllers/response_controller.rb

@@ -28,7 +28,7 @@ def view_allowed?(map, user_id)
     if map.is_a? ReviewResponseMap
       reviewee_team = AssignmentTeam.find(map.reviewee_id)
       return current_user_id?(user_id) || reviewee_team.user?(current_user) || current_user.role.name == 'Administrator' ||
-        (current_user.role.name == 'Instructor' and assignment.instructor_id == current_user.id) ||
+          (current_user.role.name == 'Instructor' and assignment.instructor_id == current_user.id) ||
           (current_user.role.name == 'Teaching Assistant' and TaMapping.exists?(ta_id: current_user.id, course_id: assignment.course.id))
     else
       current_user_id?(user_id)
@@ -69,7 +69,12 @@ def edit
     @questions.each do |question|
       @review_scores << Answer.where(response_id: @response.response_id, question_id: question.id).first
     end
-    @questionnaire = set_questionnaire
+    unless @supplementary_review_questionnaire.nil?
+      @supplementary_review_questions.each do |question|
+        @review_scores << Answer.where(response_id: @response.response_id, question_id: question.id).first
+      end
+    end
+    set_questionnaire
     render action: 'response'
   end
 
@@ -84,8 +89,12 @@ def update
     begin
       @map = @response.map
       @response.update_attribute('additional_comment', params[:review][:comments])
-      @questionnaire = set_questionnaire
+      set_questionnaire
       questions = sort_questions(@questionnaire.questions)
+      unless @supplementary_review_questionnaire.nil?
+        supplementary_review_questions = sort_questions(@supplementary_review_questionnaire.questions)
+        questions += supplementary_review_questions
+      end
       create_answers(params, questions) unless params[:responses].nil? # for some rubrics, there might be no questions but only file submission (Dr. Ayala's rubric)
       @response.update_attribute('is_submitted', true) if params['isSubmit'] && params['isSubmit'] == 'Yes'
       @response.notify_instructor_on_difference if (@map.is_a? ReviewResponseMap) && @response.is_submitted && @response.significant_difference?
@@ -144,6 +153,10 @@ def create
     if params[:review][:questionnaire_id]
       @questionnaire = Questionnaire.find(params[:review][:questionnaire_id])
       @round = params[:review][:round]
+      @supplementary_review_questionnaire_id = Team.get_supplementary_review_questionnaire_id_of_team(@map.reviewee_id)
+      unless @supplementary_review_questionnaire_id.nil?
+        @supplementary_review_questionnaire = Questionnaire.find(@supplementary_review_questionnaire_id)
+      end
     else
       @round = nil
     end
@@ -154,10 +167,10 @@ def create
     @response = Response.where(map_id: @map.id, round: @round.to_i).order(created_at: :desc).first
     if @response.nil?
       @response = Response.create(
-        map_id: @map.id,
-        additional_comment: params[:review][:comments],
-        round: @round.to_i,
-        is_submitted: is_submitted
+          map_id: @map.id,
+          additional_comment: params[:review][:comments],
+          round: @round.to_i,
+          is_submitted: is_submitted
       )
     end
     was_submitted = @response.is_submitted
@@ -166,6 +179,10 @@ def create
     # :version_num=>@version)
     # Change the order for displaying questions for editing response views.
     questions = sort_questions(@questionnaire.questions)
+    unless @supplementary_review_questionnaire.nil?
+      supplementary_review_questions = sort_questions(@supplementary_review_questionnaire.questions)
+      questions += supplementary_review_questions
+    end
     create_answers(params, questions) if params[:responses]
     msg = "Your response was successfully saved."
     error_msg = ""
@@ -253,6 +270,9 @@ def set_content(new_response = false)
     new_response ? set_questionnaire_for_new_response : set_questionnaire
     set_dropdown_or_scale
     @questions = sort_questions(@questionnaire.questions)
+    unless @supplementary_review_questionnaire.nil?
+      @supplementary_review_questions = sort_questions(@supplementary_review_questionnaire.questions)
+    end
     @min = @questionnaire.min_question_score
     @max = @questionnaire.max_question_score
   end
@@ -282,13 +302,16 @@ def set_questionnaire_for_new_response
       reviewees_topic = SignedUpTeam.topic_id_by_team_id(@contributor.id)
       @current_round = @assignment.number_of_current_round(reviewees_topic)
       @questionnaire = @map.questionnaire(@current_round)
-    when
-      "MetareviewResponseMap",
-      "TeammateReviewResponseMap",
-      "FeedbackResponseMap",
-      "CourseSurveyResponseMap",
-      "AssignmentSurveyResponseMap",
-      "GlobalSurveyResponseMap"
+      @supplementary_review_questionnaire_id = Team.get_supplementary_review_questionnaire_id_of_team(@contributor.id)
+      unless @supplementary_review_questionnaire_id.nil?
+        @supplementary_review_questionnaire = Questionnaire.find(@supplementary_review_questionnaire_id)
+      end
+    when "MetareviewResponseMap",
+        "TeammateReviewResponseMap",
+        "FeedbackResponseMap",
+        "CourseSurveyResponseMap",
+        "AssignmentSurveyResponseMap",
+        "GlobalSurveyResponseMap"
       @questionnaire = @map.questionnaire
     end
   end
@@ -297,8 +320,8 @@ def scores
     @review_scores = []
     @questions.each do |question|
       @review_scores << Answer.where(
-        response_id: @response.id,
-        question_id:  question.id
+          response_id: @response.id,
+          question_id:  question.id
       ).first
     end
   end
@@ -308,13 +331,17 @@ def set_questionnaire
     # we can find the questionnaire from the question_id in answers
     answer = @response.scores.first
     @questionnaire = @response.questionnaire_by_answer(answer)
+    @supplementary_review_questionnaire_id = Team.get_supplementary_review_questionnaire_id_of_team(@map.contributor.id)
+    unless @supplementary_review_questionnaire_id.nil?
+      @supplementary_review_questionnaire = Questionnaire.find(@supplementary_review_questionnaire_id)
+    end
   end
 
   # checks if the questionnaire is nil and opens drop down or rating accordingly
   def set_dropdown_or_scale
     use_dropdown = AssignmentQuestionnaire.where(assignment_id: @assignment.try(:id),
                                                  questionnaire_id: @questionnaire.try(:id))
-                                          .first.try(:dropdown)
+                       .first.try(:dropdown)
     @dropdown_or_scale = (use_dropdown ? 'dropdown' : 'scale')
   end
 

app/controllers/supplementary_review_questionnaires_controller.rb

@@ -0,0 +1,24 @@
+class SupplementaryReviewQuestionnairesController < QuestionnairesController
+
+  def create_supplementary_review_questionnaire
+        @participant = AssignmentParticipant.find(params[:id])
+        @team = Team.find(@participant.team.id)
+        if @team.supplementary_review_questionnaire_id.nil?
+          @questionnaire = Questionnaire.create(private: false, name: "supplementary_review_questionnaire_" + @team.id.to_s,
+            instructor_id: @team.id, min_question_score: 0, max_question_score: 5, type: "SupplementaryReviewQuestionnaire", display_type: "Review",
+            instruction_loc: Questionnaire::DEFAULT_QUESTIONNAIRE_URL)
+
+          if @questionnaire.save
+            @team.supplementary_review_questionnaire_id = @questionnaire.id
+            @team.save
+            flash[:success] = 'You have successfully created a rubric!'
+          else
+            flash[:error] = $ERROR_INFO
+          end
+        else
+          @questionnaire = Questionnaire.find(@team.supplementary_review_questionnaire_id)
+        end
+        redirect_to controller: 'supplementary_review_questionnaires', action: 'edit', id: @questionnaire.id
+    end
+
+end

app/helpers/grades_helper.rb

@@ -105,7 +105,7 @@ def underlined?(score)
     return "underlined" if score.comment.present?
   end
 
-  def retrieve_questions(questionnaires, assignment_id)
+  def retrieve_questions(questionnaires, assignment_id, team_id = nil)
     questions = {}
     questionnaires.each do |questionnaire|
       round = AssignmentQuestionnaire.where(assignment_id: assignment_id, questionnaire_id: questionnaire.id).first.used_in_round
@@ -115,6 +115,18 @@ def retrieve_questions(questionnaires, assignment_id)
                                questionnaire.symbol
                              end
       questions[questionnaire_symbol] = questionnaire.questions
+      # add supplementary questions
+      unless team_id.nil?
+        supplementary_review_questionnaire_id = Team.get_supplementary_review_questionnaire_id_of_team(team_id)
+        unless supplementary_review_questionnaire_id.nil?
+          supplementary_review_questionnaire = Questionnaire.find(supplementary_review_questionnaire_id)
+          unless supplementary_review_questionnaire.nil?
+            supplementary_review_questions = supplementary_review_questionnaire.questions
+            questions[questionnaire_symbol] += supplementary_review_questions
+          end
+        end
+      end
+
     end
     questions
   end

app/models/questionnaire.rb

@@ -28,7 +28,9 @@ class Questionnaire < ActiveRecord::Base
                          'Course SurveyQuestionnaire',
                          'CourseSurveyQuestionnaire',
                          'BookmarkratingQuestionnaire',
-                         'QuizQuestionnaire'].freeze
+                         'QuizQuestionnaire',
+                         'SupplementaryReviewQuestionnaire'].freeze
+
   has_paper_trail
 
   def get_weighted_score(assignment, scores)

app/models/response.rb

@@ -240,6 +240,14 @@ def construct_review_response code, self_id, show_tags = nil, current_user = nil
       questionnaire = self.questionnaire_by_answer(answers.first)
       questionnaire_max = questionnaire.max_question_score
       questions = questionnaire.questions.sort_by(&:seq)
+      unless self.map.nil? || self.map.contributor.nil? 
+        supplementary_review_questionnaire_id = Team.get_supplementary_review_questionnaire_id_of_team(self.map.contributor.id)
+      end
+      unless supplementary_review_questionnaire_id.nil?
+        supplementary_review_questionnaire = Questionnaire.find(supplementary_review_questionnaire_id)
+        supplementary_review_questions = supplementary_review_questionnaire.questions.sort_by(&:seq)
+        questions += supplementary_review_questions
+      end
       # get the tag settings this questionnaire
       tag_prompt_deployments = show_tags ? TagPromptDeployment.where(questionnaire_id: questionnaire.id, assignment_id: self.map.assignment.id) : nil
       code = add_table_rows questionnaire_max, questions, answers, code, tag_prompt_deployments, current_user
@@ -277,3 +285,4 @@ def add_table_rows questionnaire_max, questions, answers, code, tag_prompt_deplo
     code
   end
 end
+

app/models/supplementary_review_questionnaire.rb

@@ -0,0 +1,2 @@
+class SupplementaryReviewQuestionnaire < Questionnaire
+end