CSC/ECE 517 Spring 2019 - Project E1905. Refactor questionnaires controller.rb: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
No edit summary
Line 50: Line 50:
* 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.
* 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.
* 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 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' methods of the controller so that the user can be redirected easily.
* 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.

Revision as of 00:23, 26 March 2019

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