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: The first step is to write controllers for the Questionnaires and Questions, which should be responsible for creating, modifying, and deleting questions and each controller should only be responsible for minimal interdependency.
  2. Fix bugs in the existing functionality: Delete questionnaire option was not working
    1. Redirect to the view page after creation of a questionnaire
    2. Update method not working for updating parameters of the questionnaire like name and visibility.
    3. Update method not working on questionnaire if it has zero questions.
  3. Implementing CRUD operations for each controller: Each controller should have only CRUD operations as far as possible. The QuestionnairesController should have methods for creating, updating, deleting, and viewing questionnaires. Similarly, the QuestionsController should have methods for creating, updating, deleting, and viewing questions.
  4. Improving the clarity and conciseness of code: The code should be written in a clean and concise manner. Methods with identical names that perform different functionalities should be renamed for clarity. Functions or functionality that are not clear should be commented on or removed. Any loops or methods that can be refactored for better performance should be addressed e.g. delete all questions that belong to a questionnaire
  5. Discarding unused or unclear functionality: Any unused or unclear functionality should be removed from the controllers. This will help to reduce complexity and make the code easier to maintain.-
  6. Writing tests for the two controllers: Tests should be written for both the Questionnaire Controller and the Questions Controller. The tests should cover at least 80% of the code, and tools like Rubocop and Code Climate should be used to verify code smells.

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.

  1. Separating the responsibilities of the QuestionnairesController and QuestionsController: Initially the add_new_questions method present in questionnaire controller was responsible for adding a new question for the given questionnaire. As questions controller is responsible for creating a question, we defined a create method in questions controller which is responsible for the adding a new question for a given questionnaire. Since, to link a question to a questionnaire, we passed questionnaire_id using params and linked a new question created with the help of create method in questions controller to the required questionnaire and thus separated their responsibilities.
  2. 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.
    2. 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.
    3. Redirect to View page after creating a questionnaire instead of edit questionnaire page.
  3. 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.
  4. Reimplement CRUD functionality and create JSON endpoints in QuestionsController: We reimplemented the following methods to create the required endpoints.
    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:

Proposed Changes

  • Solution: - Separating the responsibilities of the Questionnaire Controller and Questions Controller:

Initially the add_new_questions method present in questionnaire controller was responsible for adding a new question for the given questionnaire. As questions controller is responsible for creating a question, we defined a create method in questions controller which is responsible for the adding a new question for a given questionnaire. Since, to link a question to a questionnaire, we passed questionnaire id using params and linked a new question created with the help of create method in questions controller to the required questionnaire and thus separated their responsibilities.

  • Solution:- Fix bugs in the existing functionality:

1. Delete questionnaire functionality - added get confirm in routes of tree display which was missing 2. Redirect to view page after creating questionnaire - Redirect to view page after the questionnaire is created, action: view in the create function 3. Save all questions - kept a if check to avoid nil class error

  • Solution: - Implementing CRUD operations for each controller:

After separating the functionalities of the two controller, the CRUD operations for each controller are :- 1. Questionnaire Controller -

2. Question Controller -

  • Checked functioning of all JSON end points of all the methods using postman -

1. Image below shows working of index method of questionnaire controller

2. Image below shows working of show method of questionnaire controller

3. Image below shows working of create method of questionnaire controller

4. Image below shows working of destroy method of questionnaire controller

5. Image below shows working of update method of questionnaire controller

6. Image below shows working of copy method of questionnaire controller

7. Image below shows working of toggle_access method of questionnaire controller

8. Image below shows working of index method of question controller

9. Image below shows working of show method of question controller

10. Image below shows working of create method of question controller

11. Image below shows working of destroy method of question controller

12. Image below shows working of update method of question controller

13. Image below shows working of types method of question controller

  • Solution:- Improving the clarity and conciseness of code:

1. add_new_questions -> create method

add_new_questions method defined in questionnaire controller was used for creating questions. Hence, was renamed as create while defining the CRUD operations for question controller

2. save_all_questions -> update method

save_all_questions method in questionnaire controller was used for updating the questionnaire. Hence it was renamed to update and fixed.

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


  • Solution:- Discarding unused or unclear functionality:

Removing unused or unclear functionality- After checking all the existing functionalities following code were unused or unclear and hence were removed from the reimplemented controller code.

  • create_questionnaire method of questionnaire controller -

Create method defined in the questionnaire controller creates a new controller hence the method create_questionnaire was unused.

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


  • save method, delete_question method, save_new_question method and save method in questionnaire controller

question associated to a controller is deleted with the help of destroy method in question controller, hence the delete_question method is unused. All the save methods use is unclear.

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
 # delete questions from a questionnaire
 # @param [Object] questionnaire_id
 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
 # Handles questions whose wording changed as a result of the edit
 # @param [Object] questionnaire_id
 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
  • update method in questionnaire controller

Update method is not used for updating the questionnaire controller and hence is unused

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
  • create method in questions controller

Since add_new_questions method is use for adding questions, create method of questions controller is unused. Since, we modified each controller with the CRUD functionality add_new_question was first moved from questionnaire controller to question controller and then was changed to the new create method in the questions controller. def create

   @question = Question.new(question_params[:question])
   if @question.save
     flash[:notice] = 'The question was successfully created.'
     redirect_to action: 'list'
   else
     render action: 'new'
   end
 end
  • Solution:- Writing tests for the two controllers:

1. Questionnaire Controller

2. Question Controller


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 Shah, and Aditya Srivastava. Our project mentor was Ankur Mundra (amundra@ncsu.edu)