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

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

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/controller/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.