CSC/ECE 517 Spring 2023 - E2321. Reimplement QuestionnairesController and QuestionsController

From Expertiza_Wiki
Jump to navigation Jump to search

E2321. Reimplement QuestionnairesController and QuestionsController

Problem Statement

The questionnaire is the superclass for all kinds of questionnaires and rubrics. Rubrics are used for evaluating submissions and teammate contributions, as well as taking quizzes and surveys, and all of these are subclasses of the Questionnaire class. In Expertiza, various types of questionnaires can be created, such as the Survey Questionnaire, Author Feedback Questionnaire, Bookmark Rating Questionnaire, Metareview Questionnaire, Quiz Questionnaire, Review Questionnaire, and Teammate Review Questionnaire. Each of these questionnaires can have zero or more questions, which are represented by the Question class. Typically, a questionnaire is associated with an assignment using the Assignment class.

The Questionnaire Controller is responsible for performing CRUD (Create, Read, Update, Delete) operations, such as copying and viewing questionnaires. On the other hand, the Questions Controller is responsible for creating, modifying, and deleting individual questions within a questionnaire. Questions can come in various types, such as checkboxes, multiple choice, text boxes, and more. However, each question object will always belong to a particular questionnaire. However, the current implementation suffers from several issues, such as mixing the responsibilities of the two controllers, using methods with identical names for different functionalities, and unclear or unused functionalities. Therefore, to improve the code quality and functionality, the two controllers should be implemented separately and adhere to the CRUD operations as much as possible. Additionally, the code needs to be refactored and optimized for better performance, and unused or unclear methods should be discarded. Finally, comprehensive tests should be written to ensure that the code is functioning correctly and free of code smells.

Goals of this Project

The goals of this Project are:

  1. Separating the responsibilities of the QuestionnairesController and QuestionsController
  2. Fix bugs in the existing functionality
  3. Implementing CRUD operations for each controller
  4. Discarding unused or unclear functionality
  5. Writing tests for the two controllers

By achieving these goals, both the controllers can have their basic CRUD functionalities as well as it will also ensure that the code readability and functionality of the code for the two controllers is increased. It will also help in resolving the existing bugs which will increase correctness of the code.

Implementation

A considerable amount of time and effort was necessary to review, understand, and re-implement the previous implementation of QuestionsController and QuestionnairesController. The team started with understanding how the Questionnaires and Questions module work by playing around with the Expertiza website. Once we had understanding of the flow, we started understanding the existing code written in the controllers. At first glance, it was visible that the QuestionnairesController was handling all the CRUD operations of Questions as well. We identified what functionalities did not belong in the QuestionnairesController and which redundant code had to be removed. The issues that we have addressed in our reimplementation project are listed as follows.

  • Separating the responsibilities of the QuestionnairesController and QuestionsController: The code snippets below are part of the original implementation of the QuestionnairesController. However, according to design principles, the QuestionsController should be responsible to handle the CRUD operations for questions. We reimplemented the CRUD operations and separated the responsibilities of each controller.
  def add_new_questions
    questionnaire_id = params[:id] unless params[:id].nil?
    # If the questionnaire is being used in the active period of an assignment, delete existing responses before adding new questions
    if AnswerHelper.check_and_delete_responses(questionnaire_id)
      flash[:success] = 'You have successfully added a new question. Any existing reviews for the questionnaire have been deleted!'
    else
      flash[:success] = 'You have successfully added a new question.'
    end

    num_of_existed_questions = Questionnaire.find(questionnaire_id).questions.size
    ((num_of_existed_questions + 1)..(num_of_existed_questions + params[:question][:total_num].to_i)).each do |i|
      question = Object.const_get(params[:question][:type]).create(txt: '', questionnaire_id: questionnaire_id, seq: i, type: params[:question][:type], break_before: true)
      if question.is_a? ScoredQuestion
        question.weight = params[:question][:weight]
        question.max_label = 'Strongly agree'
        question.min_label = 'Strongly disagree'
      end

      question.size = '50, 3' if question.is_a? Criterion
      question.size = '50, 3' if question.is_a? Cake
      question.alternatives = '0|1|2|3|4|5' if question.is_a? Dropdown
      question.size = '60, 5' if question.is_a? TextArea
      question.size = '30' if question.is_a? TextField

      begin
        question.save
      rescue StandardError
        flash[:error] = $ERROR_INFO
      end
    end
    redirect_to edit_questionnaire_path(questionnaire_id.to_sym)
  end
  • add_new_questions method in the questionnaire controller was responsible for creating questionnaire however it is a basic functionality of questions controller, hence was remove from the questionnaire's controller.
def update
    # If 'Add' or 'Edit/View advice' is clicked, redirect appropriately
    if params[:add_new_questions]
      # redirect_to action: 'add_new_questions', id: params.permit(:id)[:id], question: params.permit(:new_question)[:new_question]
      nested_keys = params[:new_question].keys
      permitted_params = params.permit(:id, :new_question => nested_keys)
      redirect_to action: 'add_new_questions', id: permitted_params[:id], question: permitted_params[:new_question]
    elsif params[:view_advice]
      redirect_to controller: 'advice', action: 'edit_advice', id: params[:id]
    else
      @questionnaire = Questionnaire.find(params[:id])
      begin
        # Save questionnaire information
        @questionnaire.update_attributes(questionnaire_params)

        # Save all questions
        unless params[:question].nil?
          params[:question].each_pair do |k, v|
            @question = Question.find(k)
            # example of 'v' value
            # {"seq"=>"1.0", "txt"=>"WOW", "weight"=>"1", "size"=>"50,3", "max_label"=>"Strong agree", "min_label"=>"Not agree"}
            v.each_pair do |key, value|
              @question.send(key + '=', value) unless @question.send(key) == value
            end
            @question.save
          end
        end
        flash[:success] = 'The questionnaire has been successfully updated!'
      rescue StandardError
        flash[:error] = $ERROR_INFO
      end
      redirect_to action: 'edit', id: @questionnaire.id.to_s.to_sym
    end
  end
  • update method in the questionnaire controller was updating both questionnaire and questions controller. As we want to separate the responsibility of both the controllers, there are two separate method in each controller updating the respective controllers.
  • Discard unused functionality and redundant code: The following code snippets were unused in the original implementation so we discarded them.

QUESTIONNAIRES CONTROLLER

  def create_questionnaire
    @questionnaire = Object.const_get(params[:questionnaire][:type]).new(questionnaire_params)
    # Create Quiz content has been moved to Quiz Questionnaire Controller
    if @questionnaire.type != 'QuizQuestionnaire' # checking if it is 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
  end
  • Questionnaire is being created with the help of create method, create_questionnaire is a redundant method.
  def save_all_questions
    questionnaire_id = params[:id]
    begin
      if params[:save]
        params[:question].each_pair do |k, v|
          @question = Question.find(k)
          # example of 'v' value
          # {"seq"=>"1.0", "txt"=>"WOW", "weight"=>"1", "size"=>"50,3", "max_label"=>"Strong agree", "min_label"=>"Not agree"}
          v.each_pair do |key, value|
            @question.send(key + '=', value) unless @question.send(key) == value
          end

          @question.save
          flash[:success] = 'All questions have been successfully saved!'
        end
      end
    rescue StandardError
      flash[:error] = $ERROR_INFO
    end

    if params[:view_advice]
      redirect_to controller: 'advice', action: 'edit_advice', id: params[:id]
    elsif questionnaire_id
      redirect_to edit_questionnaire_path(questionnaire_id.to_sym)
    end
  end
  • save_all_questions method was used in place of update method in questionnaire controller, that is update in questionnaire was being performed with the help of this method. While implementing basic CRUD, it is no longer needed.
def action_allowed?
    case params[:action]
    when 'edit'
      @questionnaire = Questionnaire.find(params[:id])
      current_user_has_admin_privileges? ||
        (current_user_is_a?('Instructor') && current_user_id?(@questionnaire.try(:instructor_id))) ||
        (current_user_is_a?('Teaching Assistant') && session[:user].instructor_id == @questionnaire.try(:instructor_id))
    else
      current_user_has_student_privileges?
    end
  end
  • Authorisation has been removed in the new implementation, so it is no longer needed. action_allowed method used to allow certain actions based on the session created by the login credentials.
def new
    @questionnaire = Object.const_get(params[:model].split.join).new if Questionnaire::QUESTIONNAIRE_TYPES.include? params[:model].split.join
  rescue StandardError
    flash[:error] = $ERROR_INFO
  end
def edit
    @questionnaire = Questionnaire.find(params[:id])
    redirect_to Questionnaire if @questionnaire.nil?
    session[:return_to] = request.original_url
  end
  • new and edit methods are no longer needed as the front-end is not rendered using rails but will be implemented using React in the further implementations, so they are discarded.
def add_new_questions
    questionnaire_id = params[:id] unless params[:id].nil?
    # If the questionnaire is being used in the active period of an assignment, delete existing responses before adding new questions
    if AnswerHelper.check_and_delete_responses(questionnaire_id)
      flash[:success] = 'You have successfully added a new question. Any existing reviews for the questionnaire have been deleted!'
    else
      flash[:success] = 'You have successfully added a new question.'
    end

    num_of_existed_questions = Questionnaire.find(questionnaire_id).questions.size
    ((num_of_existed_questions + 1)..(num_of_existed_questions + params[:question][:total_num].to_i)).each do |i|
      question = Object.const_get(params[:question][:type]).create(txt: '', questionnaire_id: questionnaire_id, seq: i, type: params[:question][:type], break_before: true)
      if question.is_a? ScoredQuestion
        question.weight = params[:question][:weight]
        question.max_label = 'Strongly agree'
        question.min_label = 'Strongly disagree'
      end

      question.size = '50, 3' if question.is_a? Criterion
      question.size = '50, 3' if question.is_a? Cake
      question.alternatives = '0|1|2|3|4|5' if question.is_a? Dropdown
      question.size = '60, 5' if question.is_a? TextArea
      question.size = '30' if question.is_a? TextField

      begin
        question.save
      rescue StandardError
        flash[:error] = $ERROR_INFO
      end
    end
    redirect_to edit_questionnaire_path(questionnaire_id.to_sym)
  end
  • add_new_questions method is now part of responsibility of questions controller as create method.
def save_all_questions
    questionnaire_id = params[:id]
    begin
      if params[:save]
        params[:question].each_pair do |k, v|
          @question = Question.find(k)
          # example of 'v' value
          # {"seq"=>"1.0", "txt"=>"WOW", "weight"=>"1", "size"=>"50,3", "max_label"=>"Strong agree", "min_label"=>"Not agree"}
          v.each_pair do |key, value|
            @question.send(key + '=', value) unless @question.send(key) == value
          end

          @question.save
          flash[:success] = 'All questions have been successfully saved!'
        end
      end
    rescue StandardError
      flash[:error] = $ERROR_INFO
    end

    if params[:view_advice]
      redirect_to controller: 'advice', action: 'edit_advice', id: params[:id]
    elsif questionnaire_id
      redirect_to edit_questionnaire_path(questionnaire_id.to_sym)
    end
  end
 def save
    @questionnaire.save!
    save_questions @questionnaire.id unless @questionnaire.id.nil? || @questionnaire.id <= 0
    undo_link("Questionnaire \"#{@questionnaire.name}\" has been updated successfully. ")
  end

  # save questions that have been added to a questionnaire
  def save_new_questions(questionnaire_id)
    if params[:new_question]
      # The new_question array contains all the new questions
      # that should be saved to the database
      params[:new_question].keys.each_with_index do |question_key, index|
        q = Question.new
        q.txt = params[:new_question][question_key]
        q.questionnaire_id = questionnaire_id
        q.type = params[:question_type][question_key][:type]
        q.seq = question_key.to_i
        if @questionnaire.type == 'QuizQuestionnaire'
          # using the weight user enters when creating quiz
          weight_key = "question_#{index + 1}"
          q.weight = params[:question_weights][weight_key.to_sym]
        end
        q.save unless q.txt.strip.empty?
      end
    end
  end
def save_questions(questionnaire_id)
    delete_questions questionnaire_id
    save_new_questions questionnaire_id

    if params[:question]
      params[:question].keys.each do |question_key|
        if params[:question][question_key][:txt].strip.empty?
          # question text is empty, delete the question
          Question.delete(question_key)
        else
          # Update existing question.
          question = Question.find(question_key)
          Rails.logger.info(question.errors.messages.inspect) unless question.update_attributes(params[:question][question_key])
        end
      end
    end
  end
  • save_all_questions, save, save_new_questions, and save_questions don't belong in the QuestionnairesController since they are responsibility of the QuestionsController. These methods are discarded and reimplemented as CRUD operations in QuestionsController.
def delete_questions(questionnaire_id)
    # Deletes any questions that, as a result of the edit, are no longer in the questionnaire
    questions = Question.where('questionnaire_id = ?', questionnaire_id)
    @deleted_questions = []
    questions.each do |question|
      should_delete = true
      unless question_params.nil?
        params[:question].each_key do |question_key|
          should_delete = false if question_key.to_s == question.id.to_s
        end
      end

      next unless should_delete

      question.question_advices.each(&:destroy)
      # keep track of the deleted questions
      @deleted_questions.push(question)
      question.destroy
    end
  end
  • destroy method in the QuestionsController was responsible for deleting the questions, so delete_questions is an unused method.

QUESTIONS CONTROLLER

 def action_allowed?
    current_user_has_ta_privileges?
  end
  • Authorisation is not required while creating CRUD end points.
def new
    @question = Question.new
  end
def edit
    @question = Question.find(params[:id])
  end
  • new and edit methods are no longer needed as the front-end is not rendered using rails but will be implemented using React in the further implementations, so they are discarded.
  • Minor Bug fixes in the existing code: The following bugs in the existing code needed to be fixed.
  1. User was not able to delete Questionnaire: After careful debugging, we found that a path for delete questionnaire was not defined in the routes.rb file.
  1. Nil Class Error while saving empty questionnaire: We kept a check in the save_all_questions method to handle the error and allow the questionnaire to be saved empty.
  • Fix the update method in QuestionnairesController: The original implementation of the method could not update the attributes of the questionnaire and it was used only to update the questions of that questionnaire. We reimplemented the method to update both the questionnaire attributes and each question if a change was made to them.

Initial code for the Questions and Questionnaires controller can be referred here -

Code was reimplemented to create basic CRUD operations after the methods of the controllers were fixed for the bugs. Then the methods were converted to basic CRUD operations which are described below.

  • Reimplement CRUD functionality and create JSON endpoints in QuestionsController: We reimplemented the following methods to create the required endpoints in questions_controller.rb.
  1. index: This is a GET endpoint that responds with a list of all questions with their attributes.
  2. show: This is a GET endpoint that accepts a question id and responds with the attributes of that question if it exists.
  3. create: This is a POST endpoint that accepts the question parameters and creates a new question.
  4. destroy: This is a DELETE endpoint that accepts a question ID and deletes that question.
  5. update: This is a PUT endpoint that accepts a question ID and parameters, and, updates that question with the given parameters.
  6. types: This is a GET endpoint that responds with the list of question types.
  • Reimplement CRUD functionality and create JSON endpoints in QuestionnairesController: We reimplemented the following methods to create the required endpoints in questionnaires_controller.rb.
  1. index: This is a GET endpoint that responds with a list of all questionnaires with their attributes.
  2. show: This is a GET endpoint that accepts a questionnaire id and responds with the attributes of that questionnaire if it exists.
  3. create: This is a POST endpoint that accepts the questionnaire parameters and creates a new questionnaire.
  4. destroy: This is a DELETE endpoint that accepts a questionnaire ID and deletes that questionnaire.
  5. update: This is a PUT endpoint that accepts a questionnaire ID and parameters, and, updates that questionnaire with the given parameters.
  6. copy: This is a POST endpoint that accepts a questionnaire ID and creates a new questionnaire (copy) with the same attributes of that questionnaire.
  7. toggle_access: This is a GET endpoint that changes the access from public to private and vice versa.

Testing Methodology

Our work was to reimplement the QuestionsController.rb and QuestionnairesController.rb and create JSON endpoints for each CRUD operation. Since all the operations had dependencies on other Models and Helper functions which are not included in the reimplementation-back-end, it is not possible for us to use RSpec as the testing tool. To test each endpoint, we have used Postman to send a request to each endpoint. The video with the testing plan is available at https://bit.ly/E2321

Using Postman to test endpoints:
We tested each route of the QuestionsController and QuestionnairesController to test whether the expected response is obtained. We made valid requests by passing the right parameters to each endpoint to check whether each method behaves as expected. We then made requests with incorrect number of parameters, invalid parameter types and values to check whether the endpoint throws a status 422 Invalid Request as the response. This method of testing via postman helped us ensure that each API endpoint handles the valid and invalid requests and behaves as intended.

For automated testing, these Rspec files https://github.com/Vineet2311/reimplementation-back-end/blob/main/spec/requests/api/v1/questionnaires_spec.rb and https://github.com/Vineet2311/reimplementation-back-end/blob/main/spec/requests/api/v1/questions_spec.rb can be used in the future implementations when all the models and helper classes needed have been added to the repository. These test cases ensures that the API returns the expected responses for valid and invalid requests.

Relevant Links

Contributors

This feature was created as part of Dr. Edward Gehringer's "CSC/ECE 517: Object-Oriented Design and Development" class, Spring 2023. The contributors were: Vineet Vimal Chheda, Rohan Jigarbhai Shah, and Aditya Srivastava. Our project mentor was Ankur Mundra (amundra@ncsu.edu)