CSC/ECE 517 Fall 2018 E1836 Refactor quiz questionnaires controller.rb

E1836. Refactoring quiz_questionnaires_controller.rb

This page provides a description of the Expertiza based OSS project.

About Expertiza

Expertiza is an open source project based on Ruby on Rails framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.

Problem Statement

The following tasks were accomplished in this project:

Created quiz_questionnaire_controller.rb
Moved view_quiz, new_quiz, create_quiz_questionnaire, edit_quiz and update_quiz from questionnaires_controller.rb, to quiz_questionnaires_controller.rb, and renamed it as view, new, create, edit and update respectively.
Refactored the the long methods in questionnaires_controller.rb, such as create and create_questionnaire, update quiz, valid quiz, etc.
Replaced switch statements in questionnaires_controller.rb with subclass methods.
Created models for the subclasses.
Removed hardcoded parameters, such as save_choice method.
Appropriate tests were written to test the code.

Implementation

Problem 1: Create quiz_questionnaire_controller.rb and move some of the methods in the questionnaires_controller.rb to the new file created.

Solution: The methods view_quiz, new_quiz, create_quiz, edit_quiz and update_quiz in the questionnaires_controller.rb were the methods identified and moved into the quiz_questionnaire_controller.rb. They were respectively renamed as view, new, create, edit and update. The implementation of the newly created quiz_questionnaire_controller.rb can be seen below.

class QuizQuestionnairesController < QuestionnairesController

   #=========================================================================================================
   # Separate methods for quiz questionnaire
   #=========================================================================================================

   # View a quiz questionnaire
   def view
     @questionnaire = Questionnaire.find(params[:id])
     @participant = Participant.find(params[:pid]) # creating an instance variable since it needs to be sent to submitted_content/edit
     render :view
   end

   # define a new quiz questionnaire
   # method invoked by the view
   def new
     valid_request = true
     @assignment_id = params[:aid] # creating an instance variable to hold the assignment id
     @participant_id = params[:pid] # creating an instance variable to hold the participant id
     assignment = Assignment.find(@assignment_id)
     if !assignment.require_quiz? # flash error if this assignment does not require quiz
       flash[:error] = "This assignment does not support the quizzing feature."
       valid_request = false
     else
       team = AssignmentParticipant.find(@participant_id).team

       if team.nil? # flash error if this current participant does not have a team
         flash[:error] = "You should create or join a team first."
         valid_request = false
       else
         if assignment.topics? && team.topic.nil? # flash error if this assignment has topic but current team does not have a topic
           flash[:error] = "Your team should have a topic."
           valid_request = false
         end
       end
     end

     if valid_request && Questionnaire::QUESTIONNAIRE_TYPES.include?(params[:model])
       @questionnaire = Object.const_get(params[:model]).new
       @questionnaire.private = params[:private]
       @questionnaire.min_question_score = 0
       @questionnaire.max_question_score = 1

       render :new
     else
       redirect_to controller: 'submitted_content', action: 'view', id: params[:pid]
     end
   end

   # seperate method for creating a quiz questionnaire because of differences in permission
   def create
     valid = QuizQuestionnaire.valid
     if valid.eql?("valid")
       @questionnaire = Object.const_get(params[:questionnaire][:type]).new(questionnaire_params)

       # TODO: check for Quiz Questionnaire?
       if @questionnaire.type == "QuizQuestionnaire" # checking if it is a quiz questionnaire
         participant_id = params[:pid] # creating a local variable to send as parameter to submitted content if it is a quiz questionnaire
         @questionnaire.min_question_score = 0
         @questionnaire.max_question_score = 1
         author_team = AssignmentTeam.team(Participant.find(participant_id))

         @questionnaire.instructor_id = author_team.id # for a team assignment, set the instructor id to the team_id

         @successful_create = true
         save

         save_choices @questionnaire.id

         flash[:note] = "The quiz was successfully created." if @successful_create == true
         redirect_to controller: 'submitted_content', action: 'edit', id: participant_id
       else # if it is not a quiz questionnaire
         @questionnaire.instructor_id = Ta.get_my_instructor(session[:user].id) if session[:user].role.name == "Teaching Assistant"
         save

         redirect_to controller: 'tree_display', action: 'list'
       end
     else
       flash[:error] = valid.to_s
       redirect_to :back
     end
   end

   # edit a quiz questionnaire
   def edit
     @questionnaire = Questionnaire.find(params[:id])
     if !@questionnaire.taken_by_anyone?
       render :edit
     else
       flash[:error] = "Your quiz has been taken by some other students, you cannot edit it anymore."
       redirect_to controller: 'submitted_content', action: 'view', id: params[:pid]
     end
   end
   # save an updated quiz questionnaire to the database
   def update
     @questionnaire = Questionnaire.find(params[:id])
     if @questionnaire.nil?
       redirect_to controller: 'submitted_content', action: 'view', id: params[:pid]
       return
     end
     if params['save'] && params[:question].try(:keys)
       @questionnaire.update_attributes(questionnaire_params)

       for qid in params[:question].keys
         @question = Question.find(qid)
         @question.txt = params[:question][qid.to_sym][:txt]
         @question.save

         @quiz_question_choices = QuizQuestionChoice.where(question_id: qid)
         i = 1
         for quiz_question_choice in @quiz_question_choices
           type = @question.type
           id = @question.id
           QuizQuestionnaire.change_question_types(quiz_question_choice, type, id, i)
           i += 1
         end
       end
     end
     redirect_to controller: 'submitted_content', action: 'view', id: params[:pid]
   end

 end

Problem 2: Refactor the long methods in questionnaires_controller.rb, such as create and create_questionnaire, update quiz, valid quiz, etc.

Solution:

a. Deleted the create_questionnaire method and replaced the call to it in create_quiz_questionnaire with the code of create_questionnaire and is later moved to quiz_questionnaires_controller.rb and the code snippet below is used.

def create
     valid = QuizQuestionnaire.valid
     if valid.eql?("valid")
       @questionnaire = Object.const_get(params[:questionnaire][:type]).new(questionnaire_params)

       # TODO: check for Quiz Questionnaire?
       if @questionnaire.type == "QuizQuestionnaire" # checking if it is a quiz questionnaire
         participant_id = params[:pid] # creating a local variable to send as parameter to submitted content if it is a quiz questionnaire
         @questionnaire.min_question_score = 0
         @questionnaire.max_question_score = 1
         author_team = AssignmentTeam.team(Participant.find(participant_id))

         @questionnaire.instructor_id = author_team.id # for a team assignment, set the instructor id to the team_id

         @successful_create = true
         save

         save_choices @questionnaire.id

         flash[:note] = "The quiz was successfully created." if @successful_create == true
         redirect_to controller: 'submitted_content', action: 'edit', id: participant_id
       else # if it is not a quiz questionnaire
         @questionnaire.instructor_id = Ta.get_my_instructor(session[:user].id) if session[:user].role.name == "Teaching Assistant"
         save

         redirect_to controller: 'tree_display', action: 'list'
       end
     else
       flash[:error] = valid.to_s
       redirect_to :back
     end
   end

b. Moved the large block of for and if statements into method called change_question_types in the quiz_questionnaire model.

def change_question_types(quiz_question_choice, type, id, i)

	if type == "MultipleChoiceCheckbox"

            if params[:quiz_question_choices][id.to_s][type][i.to_s]
              quiz_question_choice.update_attributes(iscorrect: params[:quiz_question_choices][id.to_s][type][i.to_s][:iscorrect], txt: params[:quiz_question_choices][id.to_s][type][i.to_s][:txt])

            else
              quiz_question_choice.update_attributes(iscorrect: '0', txt: params[:quiz_question_choices][quiz_question_choice.id.to_s][:txt])
            end
          end

          if type == "MultipleChoiceRadio"

            if params[:quiz_question_choices][id.to_s][type][:correctindex] == i.to_s
              quiz_question_choice.update_attributes(iscorrect: '1', txt: params[:quiz_question_choices][id.to_s][type][i.to_s][:txt])

            else
              quiz_question_choice.update_attributes(iscorrect: '0', txt: params[:quiz_question_choices][id.to_s][type][i.to_s][:txt])
            end
          end

          if type == "TrueFalse"

            if params[:quiz_question_choices][id.to_s][type][1.to_s][:iscorrect] == "True" # the statement is correct

              if quiz_question_choice.txt == "True"
                quiz_question_choice.update_attributes(iscorrect: '1') # the statement is correct so "True" is the right answer

              else
                quiz_question_choice.update_attributes(iscorrect: '0')
              end

            else # the statement is not correct

              if quiz_question_choice.txt == "True"
                quiz_question_choice.update_attributes(iscorrect: '0')

              else
                quiz_question_choice.update_attributes(iscorrect: '1') # the statement is not correct so "False" is the right answer
              end
            end
          end
  end

c. Replaced large block of for and if statements in update with a call to the new change_question_types method.


     for quiz_question_choice in @quiz_question_choices
           type = @question.type
           id = @question.id
           QuizQuestionnaire.change_question_types(quiz_question_choice, type, id, i)
           i += 1
         end

d. Moved export, import, copy_questionnaire_details, and assign_instructor_id to questionaire model.

  def copy_questionnaire_details(questions, orig_questionnaire, id)
    self.instructor_id = self.assign_instructor_id()
    self.name = 'Copy of ' + orig_questionnaire.name

    begin
      self.created_at = Time.now
      self.save!
      questions.each do |question|
        new_question = question.dup
        new_question.questionnaire_id = id
        new_question.size = '50,3' if (new_question.is_a? Criterion or new_question.is_a? TextResponse) and new_question.size.nil?
        new_question.save!
        advices = QuestionAdvice.where(question_id: question.id)
        next if advices.empty?
        advices.each do |advice|
          new_advice = advice.dup
          new_advice.question_id = new_question.id
          new_advice.save!
        end
      end

      pFolder = TreeFolder.find_by(name: question.set_display_type(display_type))
      parent = FolderNode.find_by(node_object_id: pFolder.id)
      QuestionnaireNode.find_or_create_by(parent_id: parent.id, node_object_id: id)
      undo_link("Copy of questionnaire #{orig_questionnaire.name} has been created successfully.")
      redirect_to controller: 'questionnaires', action: 'view', id: @questionnaire.id
    rescue StandardError
      flash[:error] = 'The questionnaire was not able to be copied. Please check the original course for missing information.' + $ERROR_INFO
      redirect_to action: 'list', controller: 'tree_display'
    end
  end

  def export
    @questionnaire = Questionnaire.find(params[:id])
    csv_data = QuestionnaireHelper.create_questionnaire_csv @questionnaire, session[:user].name
    send_data csv_data,
              type: 'text/csv; charset=iso-8859-1; header=present',
              disposition: "attachment; filename=questionnaires.csv"
  end

  def import
    @questionnaire = Questionnaire.find(params[:id])
    file = params['csv']
    @questionnaire.questions << QuestionnaireHelper.get_questions_from_csv(@questionnaire, file)
  end

  def assign_instructor_id
    # if the user to copy the questionnaire is a TA, the instructor should be the owner instead of the TA
    if session[:user].role.name != "Teaching Assistant"
      session[:user].id
    else # for TA we need to get his instructor id and by default add it to his course for which he is the TA
      Ta.get_my_instructor(session[:user].id)
    end
  end

3: Replace switch statements with subclasses methods and create models for the subclasses.

Solution: Moved the switch-case statement from create in questionaire_controller.rb into a method called set_display_type in questionaire model. The following is the code snippet that shows the same.


  def set_dispay_type(display_type)

      case display_type

      when 'Review'
	display_type = 'Review'

      when 'Metareview'
	display_type = 'Metareview'

      when 'AuthorFeedback'
        display_type = 'Author%Feedback'

      when 'CourseSurvey'
        display_type = 'Course%Survey'

      when 'TeammateReview'
        display_type = 'Teammate%Review'

      when 'GlobalSurvey'
        display_type = 'Global%Survey'

      when 'AssignmentSurvey'
        display_type = 'Assignment%Survey'

      when 'Bookmarkrating'
	display_type = 'Bookmarkrating'
      end
  end

Problem 4: Remove hardcoded parameters, such as save_choice method

Solution: Hardcoded parameters are removed and necessary constants are defined.

Test Plans

Because the purpose of our project was to better arrange our code, it follows that we also needed to rearrange the tests for that code. We first divided the tests for questionnaires_controller into two test files, questionnaires_controller_spec and quiz_questionnaires_controller_spec and separated the tests for CRUD operations of quiz_questionnaires_controller into the quiz_questionnaires_controller_spec, renaming them from view_quiz, create_quiz and the like to view and create. After this separation of CRUD operation testing, we tested the functionality that we moved out of the controllers into the model including the validation method for the quiz questionnaire. We did not include tests to cover the existing model functions, nor did we test the new set_display_type or change_question_types functions as they are setter methods, which, according to our testing instruction are not worth testing.

The setup for testing was particularly troublesome for the deployment of this project. The tests did not run properly in RubyMine, so it was difficult to ensure they were set up properly, and none of the members of our group have any expertise in rspec testing.