CSC/ECE 517 Spring 2021 - E2103. Refactor response controller.rb

This page details project documentation for the CSC/ECE 517 Spring 2021, "E2103 refactor response_controller.rb" project.

Background

A response is the object that is created when someone fills out a review rubric, such as when one writes a review, gives feedback to a reviewer, or fills out a survey. Responses to the individual rubric items are kept in Answer objects; each Answer object has a response_id to say what Response it is part of. Since response_controller needs to work with many kinds of responses, its code is pretty general. It is not the worst controller in the system, but it would be much clearer if its method names were more descriptive of what they do.

Mentor

Ed Gehringer, efg@ncsu.edu

Team Members

• Zach Parks (zpparks@ncsu.edu)
• Jatin Pramod Chinchkar (jchinch@ncsu.edu)
• Abhinav Kashyap (akashya3)

Issues

• def assign_instance_vars This name could be more specific.
• def scores This should be a model method (in response.rb)! It is all calculation.
• def new This method contains a complicated condition that determines whether the submission has been updated since the last time it was reviewed. If it has not, then the reviewer can edit his/her previous review. If there has been an update, then the reviewer gets a new review form to “update” the review. It would make sense to have a model method that tests whether there has been a review since the last file or link was submitted. Then the code here would just call that function. It would be a lot clearer what the new method is doing. set_content(new_response = false) also plays a role in this calculation. Perhaps this method should also be included in the refactoring, for the sake of clarity of the resulting code.
• def set_questionnaire Badly named; what kind of questionnaire and why? The name should be a lot clearer.
• def set_questionnaire_for_new_response Badly named, no comments, not at all clear. The logic is not like set_questionnaire. Rename, refactor for clarity, and add comments as appropriate.
• def show_calibration_results_for_student This method makes about five db accesses. Can it be broken into 2 methods, with the business logic moved to response.rb?

Refactorization and Code Modifications

Below is a list of code refactor tasks and issues addressed by this team in this project. We have also included any pertinent details, reasonings, comments, warnings, etc., corresponding to each task.

METHOD: def assign_instance_vars

This method was poorly named and commented. This method was called within the controller methods Edit and New, and is used to set any instance variable objects that the controller will need during these actions. Due to this functionality, it was renamed to assign_action_parameters to better clarify that it was assigning parameters for the controller actions.

METHOD: def scores

This method appears to have been created to replace the code at lines 71-74, however these particular lines of code are never repeated, and any calculation or business logic regarding scoresd should be (and is) in the model, the method scores is not necessary. The method scores was deleted as part of this refactor.

METHOD: def new

Note: The current version of this refactor does work - however, there are plans to refactor this further to meet CodeClimate requirements.

The reason this method was complicated and needed refactoring was due to the fact that when a response is being created, that particular Response object must be created when the User first begins a new Response (so do the Answer object(s)). A new response also has to be created when there have not been any reviews for a particular current round, and also when a reviewee has updated their submission after the most recent review in the current round. So far, this method was refactored by moving much of the logic to the Response model in the method populate_new_response. In this method, references to a response map and the current round (if any) of a response are used to determine whether a previously-created or a new response object is used in this new review.

METHOD: def set_questionnaire

The main issue with this method was the naming and poorly commented code, making the purpose and functionality of the method obscure. This method is called whenever a user is editing or viewing a response - in this case, the controller already has access to the particular Response object in question and thus this method is used to get a reference to the questionnaire corresponding to the Response object and question id. Due to the functionality and because it requires access to a Response object already, this method was renamed to get_questionnaire_from_response. Comments were added to clarify its functionality.

METHOD: def set_questionnaire_for_new_response

Again, the main issue with this method was the name and poorly commented code. This method has similar, but not the same, functionality as the set_questionnaire (get_questionnaire_from_response) method, except this method is called when a user is creating a new Response object, thus the controller does not have access to the Response object in question. In this case, it is possible to get a reference to the appropriate questionnaire by using the ResponseMap object instead of the Response object. This method was subsequently renamed to get_questionnaire_from_response_map and the code was commented to clarify its functionality.

METHOD: def show_calibration_results_for_student

In this method previously review_response_map_id and calibration_response_map_id were used to get the review_response_map and calibration_response_map rom ReviewResponseMap.rb model accessing its database table. Further using 0th index values from both the maps the function was getting a assignment object. After that the function was using calling AssignmentQuestionnaire.rb model to get the set of questionnaire for the responses. The function had too many database calls and logic which should not be implemented into a controller. Therefore we created a method calibration_results_info Response.rb which would calculate the the required review_response_map, calibration_response_map , question objects keeping the logic and entire database access into the model.

Manually Testing Details

Due to the size and structure of Expertiza, we thought it would be appropriate to provide some guidance on how to manually test the main refactors in this project, the new method and the show_calibration_results_for_student method.

• def new

In order to make a new response, we suggest logging in as (or impersonating) one of the following students:

• student7602

• student7605

• student7607

Once signed in or impersonating one of the student accounts listed above, navigate to the Program 1 assignment, and click on it. Now, click on Others work, and request a new submission to review. From here, click begin to see that a new response form is generated. The below pictures show these steps.

• def show_calibration_results_for_student

In order to show calibration results from the perspective of a student, we suggest signing (or impersonating) as the following student(s): student7605

Once signed in, navigate to the Program 1 assignment, and click on it. Again, click on Others work, and click on Show calibration results. Now, the calibration results for this review response will be visible to the user.

In order to create more calibration results, it is required to log into the instructor account and complete more calibration reviews for the Program 1 assignment and subsequently log into the corresponding student account(s) and repeat the steps above.

Ruby RSpec Testing

To run the tests :

  rspec spec/models/response_spec.rb

Changes made to response_spec.rb: