CSC/ECE 517 Spring 2019 - Project E1905. Refactor questionnaires controller.rb

From Expertiza_Wiki
Jump to navigation Jump to search

Description of the project

The questionnaires controller had several issues; this project aimed to address some of them, including:

  • Remove or move logic that should reside elsewhere, e.g. in a model class.
  • Remove logic and references to logic that is no longer being used.

Files modified in the project

  1. app/controllers/questionnaires_controller.rb
  2. app/models/questionnaire.rb
  3. app/views/questionnaires/_questionnaire.html.erb
  4. app/views/questionnaires/edit_questionnaire.html.erb (deleted)
  5. spec/controllers/questionnaires_controller_spec.rb
  6. spec/models/questionnaire_spec.rb

Issues and Improvements

The assign_instructor_id method

Problems

This method had two issues:

  1. Its name was misleading because it did not actually assign a value; it simply obtained a value from the user in the session.
  2. It was delving into implementation details of the User class.
Solution

The equivalent logic was already implemented in the User class's instructor_id method, so calls to the assign_instructor_id method were simply replaced with:

session[:user].instructor_id

A description in spec/controllers/questionnaires_controller_spec.rb was updated to reflect that assign_instructor_id was no longer in scope.

The export and import methods and edit_questionnaires.html.erb

Problems

These methods were actually no longer being used, because the functionality they provided was moved to the export_file and import_file routes.

Solution
  • The logic was triggered by the presence of the parameters import or export in the save_all_questions action. Code that caused these parameters to be present had been commented out of _questionnaire.html.erb and edit_questionnaire.html.erb in the questionnaire views. The latter file was no longer reachable, as there was no reference to an edit_questionnaire action in config/routes.rb or any direct references to it in the questionnaires controller itself. Thus the commented-out code was removed from _questionnaire.html.erb and the edit_questionnaire.html.erb file was deleted.
  • The logic referencing the parameters and methods was removed from the save_all_questions method.

The 'copy' and 'copy_questionnaire_details' methods

Problems
  1. There were two methods copy and copy_questionnaire_details present in app/controllers/questionnaire_controller.rb which implemented just one functionality of making a copy of a particular questionnaire.
  2. There was no need of two methods in the controller itself. Also, some parts of what was happening needed to be in the model.
Solution
  • Nothing special was happening in the copy_questionnaire_details method. Also, it needed to be placed into the model because of the nature of things happening inside it.
  • So, now we have one method copy in the controller, which calls the copy_questionnaire_details method, which is present in the Questionnaire model. The instructor_id as well as the params are passed as arguments to the copy_questionnaire_details method.
  • The other methods that the older copy_questionnaire_details methods used to call, like assign_instructor_id were removed in the changes above and they are now included in the method in the model itself.
  • The Exception (if it occurs while saving the Questionnaire object) is being handled in the copy method of the controller so that the user can be redirected easily.

Making code DRYer

Problems
  1. There were parts of the code, specifically when QuizQuestionChoice was created, which was copied throughout the questionnaires_controller.rb.
Solution
  • To make the code DRYer, the repeating code sequences were added into a function called create_quiz_question_choice.
  • Calls were made to this method wherever QuizQuestionChoice was made.

Test Plan

There is no means of testing the removal of dead code, except to run the existing suite of test cases:

rspec spec

Removal of the assign_instructor_id method can be tested by running test cases for the questionnaires controller, since existing test cases rely on the instructor ID being retrieved correctly:

rspec spec/controllers/questionnaires_controller_spec.rb

Testing the method create_quiz_question_choice can likewise be done with this command. The logic associated with getting the instructor ID from a user is tested using

rspec spec/models/user_spec.rb

The additions to the questionnaires model can be tested using

rspec spec/models/questionnaire_spec.rb

Note: If you are having difficulties running the rspec command, try using

bundle exec rspec

instead. This will use the gems specified in Gemfile that were installed via bundle install.

Also, tests were included for the code movement from the questionnaires controller to the questionnaire model. Checks were being made for database calls in the controller rspec method, the same format was followed while writing tests for the moved methods.