CSC/ECE 517 Fall 2015/oss E1551 RGS

E1551. Refactoring response_controller.rb

Expertiza

About Expertiza

ResponseController

What it does: response_controller.rb manages Responses. A Response is what is generated any time a user fills out a rubric--any rubric. This includes review rubrics, author-feedback rubrics, teammate-review rubrics, quizzes, and surveys. Responses come in versions. Any time an author revises work, and the reviewer comes back to review it, a new Response object is generated. This Response object points to a particular ReponseMap, which tells who the reviewer is, which team is the reviewee, and what the reviewed entity is.

Project Description

What needs to be done:

Changes

Refactor latestResponseVersion method

Problem definition

latestResponseVersion is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.

Solution

Rename get_scores

Problem definition

get_scores has a Java-like name. It should just be scores.

Solution summary

The method get_scores was renamed to scores.

Delete rereview method

Problem definition

The 100+-line method rereview does not seem to be used anymore. The second-round review can be invoked in the same way as the first-round review. Remove it.

Solution summary

The method rereview was deleted.

DRY create and update

Problem definition

create and update use completely different code. Factor the common parts out into a partial, thereby simplifying the methods.

Solution summary

There was not much actual overlap in these two methods. Minimal changes.

Consistent authorization

Problem definition

Authorization needs to be checked in the action_allowed method instead of in redirect_when_disallowed at the bottom of the file.

Solution summary

So the functionality is moved from redirect_when_disallowed to action_allowed. And also it is made sure that no one other than the author of a review (or another team member, in the case of author feedback) can edit it and then removed the redirect_when_disallowed method.

Code before changing

def action_allowed?
   current_user
 end

def redirect_when_disallowed(response)
   # For author feedback, participants need to be able to read feedback submitted by other teammates.
   # If response is anything but author feedback, only the person who wrote feedback should be able to see it.
   if response.map.read_attribute(:type) == 'FeedbackResponseMap' && response.map.assignment.team_assignment?
     team = response.map.reviewer.team
     unless team.has_user session[:user]
       redirect_to '/denied?reason=You are not on the team that wrote this feedback'
     else
       return false
     end
     response.map.read_attribute(:type)
   end
   !current_user_id?(response.map.reviewer.user_id)
 end

Code after changing

def action_allowed?
   case params[:action]
   when 'view','edit','delete','update'
     response = Response.find(params[:id])
      if response.map.read_attribute(:type) == 'FeedbackResponseMap' && response.map.assignment.team_assignment?
       team = response.map.reviewer.team
       unless team.has_user session[:user]
         redirect_to '/denied?reason=You are not on the team that wrote this feedback'
       else
         return false
       end
       response.map.read_attribute(:type)
     end
     current_user_id?(response.map.reviewer.user_id)
   else
     current_user
   end
 end

Refactor get_content

Problem definition

get_content is a complex method. It should be renamed to content and simplified. Comments should be added explaining what it does.

Solution summary

The get_content method was renamed set_content, simplified, and documented with comments.

Remove SQL queries

Problem definition

This class contains SQL queries. Please change them to Active Record commands.

Solution summary

No SQL queries were identified. No changes.

Comparison of Original and Refactored Code

Major refactoring revolved around changing method names according to rails convention, using helper methods for avoiding duplication of code in controller methods.

Duplications in Code

Original duplications : 172

Post Refactoring : 21

Code Complexity (Compared on Code Climate)

Original ResponseController<ref name="originalresponsecontroller>Original responseController https://github.com/expertiza/expertiza</ref>

Refactored Responsecontroller <ref name="Refactoredresponsecontroller>Refactored responseController https://github.com/viswaraavi/expertiza</ref>

Tests

Some functional tests are written for this class.

References

<references>