Expertiza_Wiki - User contributions [en]

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-11-07T03:29:47Z

Vravi2: /* Tests */

= E1551. Refactoring response_controller.rb =
== Expertiza ==

'''About Expertiza'''
Expertiza is an open source project accomplished by NCState students. It was developed in Ruby on Rails. It has general features like forming teams, registering for topics in a project, uploading the final deliveries. After completing the project there is an option for submitting reviews as well and updating the reviews. There is an option for submitting new quizzes as well.

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

*To test the responsecontroller login as user:instructor6 with password:password
*Go to assignments tab and choose any assignment from the assignments listed.
*From there click on others work and choose any review among the reviews listed
*Click on view or begin to start reviewing.
*You can also test response controller in similar way by logging as student student2064 and password:password.

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
;Solution summary
:Renamed <code>latestResponseVersion</code> to <code>set_all_responses</code>.
==== Solution ====
Examining <code>latestResponseVersion</code> we were able to determine that it did the following two things:
# Set the <code>@prev</code> variable to a relation of responses that correspond to the map id of the current response being created or edited.
# Set the <code>@review_scores</code> variable to an Array version if the contents of <code>@prev</code>.
Since the purpose of the code seemed to be to set variables, we renamed it to <code>set_all_responses</code>.
We also simplified the method by using ActiveRecord::Relation's <code>to_a</code> to convert the relation to an array.
There was some duplicate code in the <code>scores</code> function which we replaced with a call the <code>set_all_responses</code>.

=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
'''Solution summary'''
: The method <code>get_scores</code> was renamed to <code>scores</code>.

=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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.
The ResponseController controller has an overall problem in that it knows too much about the subclasses of <code>ResponseMap</code>. There is some major refactoring that needs to occur to clean up this controller's coupling with <code>ResponseMap</code>'s subclasses. We made an attempt to do this, but it was just to large a change to accomplish in the time allotted.

=== Consistent authorization ===
;Problem definition
:Authorization needs to be checked in the <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, simplified, and documented with comments.
A review of the <code>get_content</code> method showed that it's purpose was to '''set''' various variables for use by the response views. As such, we renamed this method <code>set_content</code> to help clarify it's intent.
We also extracted methods for the more complicated variables into their own methods, making the <code>set_content</code> method easy to understand at a glance.

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

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Some functional tests are written for this class using Rspec. The test coverage has increased from 19% to 24.1%
;To check these tests.
*clone the github repository link provided above
*run the command rspec spec/controllers/response_controller_spec.rb

==References==
<references>

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-11-02T19:08:21Z

Vravi2: /* Tests */

= E1551. Refactoring response_controller.rb =
== Expertiza ==

'''About Expertiza'''
Expertiza is an open source project accomplished by NCState students. It was developed in Ruby on Rails. It has general features like forming teams, registering for topics in a project, uploading the final deliveries. After completing the project there is an option for submitting reviews as well and updating the reviews. There is an option for submitting new quizzes as well.

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

*To test the responsecontroller login as user:instructor6 with password:password
*Go to assignments tab and choose any assignment from the assignments listed.
*From there click on others work and choose any review among the reviews listed
*Click on view or begin to start reviewing.
*You can also test response controller in similar way by logging as student student2064 and password:password.

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
;Solution summary
:Renamed <code>latestResponseVersion</code> to <code>set_all_responses</code>.
==== Solution ====
Examining <code>latestResponseVersion</code> we were able to determine that it did the following two things:
# Set the <code>@prev</code> variable to a relation of responses that correspond to the map id of the current response being created or edited.
# Set the <code>@review_scores</code> variable to an Array version if the contents of <code>@prev</code>.
Since the purpose of the code seemed to be to set variables, we renamed it to <code>set_all_responses</code>.
We also simplified the method by using ActiveRecord::Relation's <code>to_a</code> to convert the relation to an array.
There was some duplicate code in the <code>scores</code> function which we replaced with a call the <code>set_all_responses</code>.

=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
'''Solution summary'''
: The method <code>get_scores</code> was renamed to <code>scores</code>.

=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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.
The ResponseController controller has an overall problem in that it knows too much about the subclasses of <code>ResponseMap</code>. There is some major refactoring that needs to occur to clean up this controller's coupling with <code>ResponseMap</code>'s subclasses. We made an attempt to do this, but it was just to large a change to accomplish in the time allotted.

=== Consistent authorization ===
;Problem definition
:Authorization needs to be checked in the <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, simplified, and documented with comments.
A review of the <code>get_content</code> method showed that it's purpose was to '''set''' various variables for use by the response views. As such, we renamed this method <code>set_content</code> to help clarify it's intent.
We also extracted methods for the more complicated variables into their own methods, making the <code>set_content</code> method easy to understand at a glance.

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

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Some functional tests are written for this class using Rspec
;To check these tests.
*clone the github repository link provided above
*run the command rspec spec/controllers/response_controller_spec.rb

==References==
<references>

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-11-02T19:07:57Z

Vravi2: /* ResponseController */

= E1551. Refactoring response_controller.rb =
== Expertiza ==

'''About Expertiza'''
Expertiza is an open source project accomplished by NCState students. It was developed in Ruby on Rails. It has general features like forming teams, registering for topics in a project, uploading the final deliveries. After completing the project there is an option for submitting reviews as well and updating the reviews. There is an option for submitting new quizzes as well.

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

*To test the responsecontroller login as user:instructor6 with password:password
*Go to assignments tab and choose any assignment from the assignments listed.
*From there click on others work and choose any review among the reviews listed
*Click on view or begin to start reviewing.
*You can also test response controller in similar way by logging as student student2064 and password:password.

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
;Solution summary
:Renamed <code>latestResponseVersion</code> to <code>set_all_responses</code>.
==== Solution ====
Examining <code>latestResponseVersion</code> we were able to determine that it did the following two things:
# Set the <code>@prev</code> variable to a relation of responses that correspond to the map id of the current response being created or edited.
# Set the <code>@review_scores</code> variable to an Array version if the contents of <code>@prev</code>.
Since the purpose of the code seemed to be to set variables, we renamed it to <code>set_all_responses</code>.
We also simplified the method by using ActiveRecord::Relation's <code>to_a</code> to convert the relation to an array.
There was some duplicate code in the <code>scores</code> function which we replaced with a call the <code>set_all_responses</code>.

=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
'''Solution summary'''
: The method <code>get_scores</code> was renamed to <code>scores</code>.

=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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.
The ResponseController controller has an overall problem in that it knows too much about the subclasses of <code>ResponseMap</code>. There is some major refactoring that needs to occur to clean up this controller's coupling with <code>ResponseMap</code>'s subclasses. We made an attempt to do this, but it was just to large a change to accomplish in the time allotted.

=== Consistent authorization ===
;Problem definition
:Authorization needs to be checked in the <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, simplified, and documented with comments.
A review of the <code>get_content</code> method showed that it's purpose was to '''set''' various variables for use by the response views. As such, we renamed this method <code>set_content</code> to help clarify it's intent.
We also extracted methods for the more complicated variables into their own methods, making the <code>set_content</code> method easy to understand at a glance.

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

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Some functional tests are written for this class using Rspec and Factorygirls.
;To check these tests.
*clone the github repository link provided above
*run the command rspec spec/controllers/response_controller_spec.rb

==References==
<references>

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-11-02T19:05:47Z

Vravi2: /* ResponseController */

= E1551. Refactoring response_controller.rb =
== Expertiza ==

'''About Expertiza'''
Expertiza is an open source project accomplished by NCState students. It was developed in Ruby on Rails. It has general features like forming teams, registering for topics in a project, uploading the final deliveries. After completing the project there is an option for submitting reviews as well and updating the reviews. There is an option for submitting new quizzes as well.

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

*To test the responsecontroller login as user:instructor6 with password:password
*Go to assignments tab and choose any assignment from the assignments listed.
*From there click on others work and choose any review among the reviews listed
*Click on view or begin to start reviewing.

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
;Solution summary
:Renamed <code>latestResponseVersion</code> to <code>set_all_responses</code>.
==== Solution ====
Examining <code>latestResponseVersion</code> we were able to determine that it did the following two things:
# Set the <code>@prev</code> variable to a relation of responses that correspond to the map id of the current response being created or edited.
# Set the <code>@review_scores</code> variable to an Array version if the contents of <code>@prev</code>.
Since the purpose of the code seemed to be to set variables, we renamed it to <code>set_all_responses</code>.
We also simplified the method by using ActiveRecord::Relation's <code>to_a</code> to convert the relation to an array.
There was some duplicate code in the <code>scores</code> function which we replaced with a call the <code>set_all_responses</code>.

=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
'''Solution summary'''
: The method <code>get_scores</code> was renamed to <code>scores</code>.

=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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.
The ResponseController controller has an overall problem in that it knows too much about the subclasses of <code>ResponseMap</code>. There is some major refactoring that needs to occur to clean up this controller's coupling with <code>ResponseMap</code>'s subclasses. We made an attempt to do this, but it was just to large a change to accomplish in the time allotted.

=== Consistent authorization ===
;Problem definition
:Authorization needs to be checked in the <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, simplified, and documented with comments.
A review of the <code>get_content</code> method showed that it's purpose was to '''set''' various variables for use by the response views. As such, we renamed this method <code>set_content</code> to help clarify it's intent.
We also extracted methods for the more complicated variables into their own methods, making the <code>set_content</code> method easy to understand at a glance.

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

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Some functional tests are written for this class using Rspec and Factorygirls.
;To check these tests.
*clone the github repository link provided above
*run the command rspec spec/controllers/response_controller_spec.rb

==References==
<references>

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-11-01T04:12:15Z

Vravi2: /* ResponseController */

= E1551. Refactoring response_controller.rb =
== Expertiza ==

'''About Expertiza'''
Expertiza is an open source project accomplished by NCState students. It was developed in Ruby on Rails. It has general features like forming teams, registering for topics in a project, uploading the final deliveries. After completing the project there is an option for submitting reviews as well and updating the reviews. There is an option for submitting new quizzes as well.

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

To test the responsecontroller login as user:instructor6 with password:password

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
;Solution summary
:Renamed <code>latestResponseVersion</code> to <code>set_all_responses</code>.
==== Solution ====
Examining <code>latestResponseVersion</code> we were able to determine that it did the following two things:
# Set the <code>@prev</code> variable to a relation of responses that correspond to the map id of the current response being created or edited.
# Set the <code>@review_scores</code> variable to an Array version if the contents of <code>@prev</code>.
Since the purpose of the code seemed to be to set variables, we renamed it to <code>set_all_responses</code>.
We also simplified the method by using ActiveRecord::Relation's <code>to_a</code> to convert the relation to an array.
There was some duplicate code in the <code>scores</code> function which we replaced with a call the <code>set_all_responses</code>.

=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
'''Solution summary'''
: The method <code>get_scores</code> was renamed to <code>scores</code>.

=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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.
The ResponseController controller has an overall problem in that it knows too much about the subclasses of <code>ResponseMap</code>. There is some major refactoring that needs to occur to clean up this controller's coupling with <code>ResponseMap</code>'s subclasses. We made an attempt to do this, but it was just to large a change to accomplish in the time allotted.

=== Consistent authorization ===
;Problem definition
:Authorization needs to be checked in the <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, simplified, and documented with comments.
A review of the <code>get_content</code> method showed that it's purpose was to '''set''' various variables for use by the response views. As such, we renamed this method <code>set_content</code> to help clarify it's intent.
We also extracted methods for the more complicated variables into their own methods, making the <code>set_content</code> method easy to understand at a glance.

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

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Some functional tests are written for this class using Rspec and Factorygirls.
;To check these tests.
*clone the github repository link provided above
*run the command rspec spec/controllers/response_controller_spec.rb

==References==
<references>

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-11-01T03:59:50Z

Vravi2:

= E1551. Refactoring response_controller.rb =
== Expertiza ==

'''About Expertiza'''
Expertiza is an open source project accomplished by NCState students. It was developed in Ruby on Rails. It has general features like forming teams, registering for topics in a project, uploading the final deliveries. After completing the project there is an option for submitting reviews as well and updating the reviews. There is an option for submitting new quizzes as well.

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

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
;Solution summary
:Renamed <code>latestResponseVersion</code> to <code>set_all_responses</code>.
==== Solution ====
Examining <code>latestResponseVersion</code> we were able to determine that it did the following two things:
# Set the <code>@prev</code> variable to a relation of responses that correspond to the map id of the current response being created or edited.
# Set the <code>@review_scores</code> variable to an Array version if the contents of <code>@prev</code>.
Since the purpose of the code seemed to be to set variables, we renamed it to <code>set_all_responses</code>.
We also simplified the method by using ActiveRecord::Relation's <code>to_a</code> to convert the relation to an array.
There was some duplicate code in the <code>scores</code> function which we replaced with a call the <code>set_all_responses</code>.

=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
'''Solution summary'''
: The method <code>get_scores</code> was renamed to <code>scores</code>.

=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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>
 

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Some functional tests are written for this class using Rspec and Factorygirls.
;To check these tests.
*clone the github repository link provided above
*run the command rspec spec/controllers/response_controller_spec.rb

==References==
<references>

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-11-01T03:59:35Z

Vravi2:

= E1551. Refactoring response_controller.rb =
== Expertiza ==

'''About Expertiza'''
Expertiza is an open source project accomplished by NCState students. It was developed in Ruby on Rails. It has general features like forming teams, registering for topics in a project, uploading the final deliveries. After completing the project there is an option for submitting reviews as well and updating the reviews. There is an option for submitting new quizzes as well.

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

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> 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
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
'''Solution summary'''
: The method <code>get_scores</code> was renamed to <code>scores</code>.

=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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>
 

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Some functional tests are written for this class using Rspec and Factorygirls.
;To check these tests.
*clone the github repository link provided above
*run the command rspec spec/controllers/response_controller_spec.rb

==References==
<references>

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-11-01T03:24:02Z

Vravi2: /* Tests */

= 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
:<code>latestResponseVersion</code> 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
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
'''Solution summary'''
: The method <code>get_scores</code> was renamed to <code>scores</code>.

=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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>
 

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Some functional tests are written for this class using Rspec and Factorygirls.
;To check these tests.
*clone the github repository link provided above
*run the command rspec spec/controllers/response_controller_spec.rb

==References==
<references>

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-11-01T03:23:44Z

Vravi2: /* Tests */

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-11-01T03:23:19Z

Vravi2: /* Tests */

= 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
:<code>latestResponseVersion</code> 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
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
'''Solution summary'''
: The method <code>get_scores</code> was renamed to <code>scores</code>.

=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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>
 

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Some functional tests are written for this class using Rspec and Factorygirls.
;To check these tests.
*clone the git repository
*run the command rspec spec/controllers/response_controller.rb

==References==
<references>

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-11-01T03:23:01Z

Vravi2: /* Tests */

= 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
:<code>latestResponseVersion</code> 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
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
'''Solution summary'''
: The method <code>get_scores</code> was renamed to <code>scores</code>.

=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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>
 

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Some functional tests are written for this class using Rspec and Factorygirls.
To check these tests.
*clone the git repository
*run the command rspec spec/controllers/response_controller.rb

==References==
<references>

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T21:23:26Z

Vravi2: /* references */

= 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
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
'''Solution summary'''
: The method <code>get_scores</code> was renamed to <code>scores</code>.

=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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>
 

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Some functional tests are written for this class.
==References==
<references>

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T21:23:08Z

Vravi2:

= 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
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
'''Solution summary'''
: The method <code>get_scores</code> was renamed to <code>scores</code>.

=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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>
 

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Some functional tests are written for this class.
==references==
<references>

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T21:22:45Z

Vravi2:

= 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
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
'''Solution summary'''
: The method <code>get_scores</code> was renamed to <code>scores</code>.

=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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>
 

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Some functional tests are written for this class.

<references>

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T21:22:26Z

Vravi2:

= 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
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
'''Solution summary'''
: The method <code>get_scores</code> was renamed to <code>scores</code>.

=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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>
 

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Some functional tests are written for this class.

==references==

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T21:22:02Z

Vravi2: /* Tests */

= 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
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
'''Solution summary'''
: The method <code>get_scores</code> was renamed to <code>scores</code>.

=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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>
 

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Some functional tests are written for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T21:20:37Z

Vravi2: /* Rename get_scores */

= 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
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
'''Solution summary'''
: The method <code>get_scores</code> was renamed to <code>scores</code>.

=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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>
 

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T21:20:14Z

Vravi2: /* Rename get_scores */

= 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
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
'''Solution summary'''
: The method <code>get_scores</code> was renamed to <code>score</code>.

=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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>
 

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T21:20:03Z

Vravi2: /* Rename get_scores */

= 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
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
Solution summary
: The method <code>get_scores</code> was renamed to <code>score</code>.

=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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>
 

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T21:17:17Z

Vravi2: /* Expertiza */

= 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
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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>
 

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T21:16:46Z

Vravi2: /* Project Description */

= E1551. Refactoring response_controller.rb =
== 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
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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>
 

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T21:16:32Z

Vravi2: /* ResponseController */

= E1551. Refactoring response_controller.rb =
== 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 ==

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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>
 

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T21:16:14Z

Vravi2: /* ResponseController */

= E1551. Refactoring response_controller.rb =
== 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.
What needs to be done:

== Project Description ==

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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>
 

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T21:16:00Z

Vravi2: /* ResponseController */

= E1551. Refactoring response_controller.rb =
== 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.
What needs to be done:

== Project Description ==

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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>
 

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T20:21:58Z

Vravi2: /* Comparison of Original and Refactored Code */

= E1551. Refactoring response_controller.rb =
== Expertiza ==
== ResponseController ==
The ResponseController manages Responses.

== Project Description ==

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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>
 

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T20:17:53Z

Vravi2: /* Comparison of Original and Refactored Code */

= E1551. Refactoring response_controller.rb =
== Expertiza ==
== ResponseController ==
The ResponseController manages Responses.

== Project Description ==

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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>
 

[[File:Code_climate_response.png]]
'''
Refactored Responsecontroller''' <ref name="Refactoredresponsecontroller>''Refactored responseController'' https://github.com/viswaraavi/expertiza</ref>
 

[[File:Code_climate_expertiza.png]]

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T20:16:55Z

Vravi2: /* Comparison of Original and Refactored Code */

= E1551. Refactoring response_controller.rb =
== Expertiza ==
== ResponseController ==
The ResponseController manages Responses.

== Project Description ==

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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>
 

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T20:15:46Z

Vravi2: /* Comparison of Original and Refactored Code */

= E1551. Refactoring response_controller.rb =
== Expertiza ==
== ResponseController ==
The ResponseController manages Responses.

== Project Description ==

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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>
 

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T20:14:26Z

Vravi2: /* Comparison of Original and Refactored Code */

= E1551. Refactoring response_controller.rb =
== Expertiza ==
== ResponseController ==
The ResponseController manages Responses.

== Project Description ==

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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 : 248
::Post Refactoring : 224

Code Complexity (Compared on Code Climate)

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

[[File:Code_climate_response.png]]

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

[[File:Code_climate_expertiza.png]]

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T20:10:49Z

Vravi2: /* Remove SQL queries */

= E1551. Refactoring response_controller.rb =
== Expertiza ==
== ResponseController ==
The ResponseController manages Responses.

== Project Description ==

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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 : 248
::Post Refactoring : 224

Code Complexity (Compared on Code Climate)

[[File:Code_climate_response.png]]

[[File:Code_climate_expertiza.png]]

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T20:07:10Z

Vravi2: /* Remove SQL queries */

= E1551. Refactoring response_controller.rb =
== Expertiza ==
== ResponseController ==
The ResponseController manages Responses.

== Project Description ==

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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 : 248
::Post Refactoring : 224

Code Complexity (Compared on Code Climate)

[[File:Code climate response.png|thumb|Image taken using codeclimate]]

[[File:Code climate expertiza.png|thumb|image taken using codeclimate]]

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

File:Code climate expertiza.png

2015-10-31T20:05:04Z

Vravi2:

File:Code climate response.png

2015-10-31T20:04:34Z

Vravi2:

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T20:04:01Z

Vravi2: /* Comparison of Original and Refactored Code */

= E1551. Refactoring response_controller.rb =
== Expertiza ==
== ResponseController ==
The ResponseController manages Responses.

== Project Description ==

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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.

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 : 248
::Post Refactoring : 224

Code Complexity (Compared on Code Climate)

[[File:Code climate response.png|thumb|Image taken using codeclimate]]

[[File:Code climate expertiza.png|thumb|image taken using codeclimate]]

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T19:56:19Z

Vravi2: /* Comparison of Original and Refactored Code */

= E1551. Refactoring response_controller.rb =
== Expertiza ==
== ResponseController ==
The ResponseController manages Responses.

== Project Description ==

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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 : 248
::Post Refactoring : 224

Code Complexity (Compared on Code Climate)

[[File:Codeclimate 1.png]]

[[File:codeclimate 2.png]]

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T19:56:02Z

Vravi2: /* Comparison of Original and Refactored Code */

= E1551. Refactoring response_controller.rb =
== Expertiza ==
== ResponseController ==
The ResponseController manages Responses.

== Project Description ==

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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 : 248
:Post Refactoring : 224

Code Complexity (Compared on Code Climate)

[[File:Codeclimate 1.png]]

[[File:codeclimate 2.png]]

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T19:55:42Z

Vravi2: /* Comparison of Original and Refactored Code */

= E1551. Refactoring response_controller.rb =
== Expertiza ==
== ResponseController ==
The ResponseController manages Responses.

== Project Description ==

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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 : 248
Post Refactoring : 224

Code Complexity (Compared on Code Climate)

[[File:Codeclimate 1.png]]

[[File:codeclimate 2.png]]

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T19:55:17Z

Vravi2:

= E1551. Refactoring response_controller.rb =
== Expertiza ==
== ResponseController ==
The ResponseController manages Responses.

== Project Description ==

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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 : 248
Post Refactoring : 224

Code Complexity (Compared on Code Climate)

[[File:Codeclimate 1.png]]

[[File:codeclimate 2.png]]

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T19:39:44Z

Vravi2: /* Delete rereview method */

= E1551. Refactoring response_controller.rb =
== Expertiza ==
== ResponseController ==
The ResponseController manages Responses.

== Project Description ==

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rereview</code> 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 <code>rereview</code> was deleted.

=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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.

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T19:38:41Z

Vravi2: /* Consistent authorization */

= E1551. Refactoring response_controller.rb =
== Expertiza ==
== ResponseController ==
The ResponseController manages Responses.

== Project Description ==

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rearview</code> 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 <code>rearview</code> was deleted.
=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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.

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T19:37:50Z

Vravi2: /* Consistent authorization */

= E1551. Refactoring response_controller.rb =
== Expertiza ==
== ResponseController ==
The ResponseController manages Responses.

== Project Description ==

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rearview</code> 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 <code>rearview</code> was deleted.
=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
:: Code before changing

<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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.

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T19:36:55Z

Vravi2: /* Consistent authorization */

= E1551. Refactoring response_controller.rb =
== Expertiza ==
== ResponseController ==
The ResponseController manages Responses.

== Project Description ==

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rearview</code> 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 <code>rearview</code> was deleted.
=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
:So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
: Code before changing
<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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.

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T19:35:56Z

Vravi2: /* Consistent authorization */

= E1551. Refactoring response_controller.rb =
== Expertiza ==
== ResponseController ==
The ResponseController manages Responses.

== Project Description ==

== Changes ==
=== Refactor latestResponseVersion method ===
;Problem definition
:<code>latestResponseVersion</code> is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
=== Rename get_scores ===
;Problem definition
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>.
=== Delete rereview method ===
;Problem definition
:The 100+-line method <code>rearview</code> 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 <code>rearview</code> was deleted.
=== DRY create and update ===
;Problem definition
:<code>create</code> and <code>update</code> 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 <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file.
;Solution summary
So the functionality is moved from <code>redirect_when_disallowed</code> to <code>action_allowed</code>. 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 <code>redirect_when_disallowed</code> method.
: Code before changing
<code>
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>
: Code after changing

<code>
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
</code>

=== Refactor get_content ===
;Problem definition
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does.
;Solution summary
:The <code>get_content</code> method was renamed <code>set_content</code>, 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.

== Tests ==
Contact: Ed Gehringer, efg@ncsu.edu
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.
What needs to be done:

Please write some functional tests for this class.

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T19:00:12Z

Vravi2: /* Consistent authorization */

CSC/ECE 517 Fall 2015/oss E1551 RGS

2015-10-31T18:52:46Z

Vravi2: