Expertiza_Wiki - User contributions [en]

CSC/ECE 517 Spring 2016/Refactor response controller

2016-04-02T01:26:31Z

Ptrived: /* Instructions to test from GUI */

==E1614. Refactoring Response Controller==

This page provide the details of the changes done as part of the refactoring response controller project(E1614).

__TOC__

===A brief overview of Expertiza===
[http://expertiza.ncsu.edu/ Expertiza] project is a platform to create reusable learning objects through peer review. It is an open source project which uses [http://rubyonrails.org/ Ruby on Rails] framework.

===Project Statement===
The main aim of this project was to refactor the response_controller. The following tasks were completed as part of refactoring in this project:

* Moving variable declaration to right places.
* Removing unused variables.
* Fixing code duplication.
* Replacing if else block with switch statements.
* Remove the unreachable code.

===About Response Controller===
The Response controller is responsible for the CRUD(Create, Read, Update and Delete) operations on responses. The users can fill out a questionnaire such as review rubric or feedback on the partner's contribution. So the ResponseController handles the various operations on the responses, which are objects that Expertiza creates when you fill out a questionnaire.

===Changes done as part of refactoring===
====Moving variable declaration to right places====
There were two variables "msg" and "error_msg" in the create method which were declared and initialized to blank strings and then again assigned some string value before it was used in the method. So we moved the variable declaration to the place were it was used and removed the unnecessary variable declaration. Below if the code snippet for the changes done:
<pre>
def create
@map = ResponseMap.find(params[:id]) #assignment/review/metareview id is in params id

set_all_responses

#to save the response for ReviewResponseMap, a questionnaire_id is wrapped in the params
if params[:review][:questionnaire_id]
@questionnaire = Questionnaire.find(params[:review][:questionnaire_id])
@round = params[:review][:round]
else
@round=nil
end

# create the response
if params[:isSubmit].eql?('Yes')
is_submitted = true
else
is_submitted = false
end
@response = Response.create(:map_id => @map.id, :additional_comment => params[:review][:comments],:round => @round, :is_submitted => is_submitted)#,:version_num=>@version)

#Change the order for displaying questions for editing response views.
questions=sort_questions(@questionnaire.questions)

if params[:responses]
create_answers(params, questions)
end

#@map.save
msg = "Your response was successfully saved."
error_msg="Error"
@response.email();
redirect_to :controller => 'response', :action => 'saving', :id => @map.map_id, :return => params[:return], :msg => msg, :error_msg => error_msg, :save_options => params[:save_options]
end
</pre>

====Removing unused variables====
There were some unused variables in this controller. The edit method contained a variable "array_not_empty" variable which was not used anywhere. So we removed the unused variables.

====Fixing code duplication====
There was some duplicate code in the update method. So we removed the duplicate code and retained the code snippet inside the if block. Below is the modified code for the update method:
<pre>
#Update the response and answers when student "edit" existing response
def update
return unless action_allowed?

# the response to be updated
@response = Response.find(params[:id])

msg = ""
begin
@map = @response.map
@response.update_attribute('additional_comment', params[:review][:comments])
if @map.type=="ReviewResponseMap" && @response.round
@questionnaire = @map.questionnaire(@response.round)
elsif @map.type=="ReviewResponseMap"
@questionnaire = @map.questionnaire(nil)
else
@questionnaire = @map.questionnaire
end
questions = @questionnaire.questions.sort { |a,b| a.seq <=> b.seq }

questions=sort_questions(@questionnaire.questions)
create_answers(params,questions)
questions = @questionnaire.questions.sort { |a,b| a.seq <=> b.seq }

if !params[:responses].nil? # for some rubrics, there might be no questions but only file submission (Dr. Ayala's rubric)
params[:responses].each_pair do |k, v|
score = Answer.where(response_id: @response.id, question_id: questions[k.to_i].id).first
unless score
score = Answer.create(:response_id => @response.id, :question_id => questions[k.to_i].id, :answer => v[:score], :comments => v[:comment])
end
score.update_attribute('answer', v[:score])
score.update_attribute('comments', v[:comment])
end
end
if (params['isSubmit'] && (params['isSubmit'].eql?'Yes'))
# Update the submission flag.
@response.update_attribute('is_submitted',true)
else
@response.update_attribute('is_submitted',false)
end
rescue
msg = "Your response was not saved. Cause:189 #{$!}"
end
redirect_to :controller => 'response', :action => 'saving', :id => @map.map_id, :return => params[:return], :msg => msg, :save_options => params[:save_options]
end
</pre>

====Replacing if else block with switch statements====
The redirection method used the if-else statements. The if-else block was replaced with switch statements. Below if is the code snippet for the redirection method after the code changes:
<pre>
def redirection
flash[:error] = params[:error_msg] unless params[:error_msg] and params[:error_msg].empty?
flash[:note] = params[:msg] unless params[:msg] and params[:msg].empty?
@map = Response.find_by_map_id(params[:id])

case params[:return]
when "feedback"
redirect_to :controller => 'grades', :action => 'view_my_scores', :id => @map.reviewer.id
when "teammate"
redirect_to view_student_teams_path student_id: @map.reviewer.id
when "instructor"
redirect_to :controller => 'grades', :action => 'view', :id => @map.response_map.assignment.id
when "assignment_edit"
redirect_to controller: 'assignments', action: 'edit', id: @map.response_map.assignment.id
else
redirect_to :controller => 'student_review', :action => 'list', :id => @map.reviewer.id
end
end
</pre>

====Remove the unreachable code====
In the action_allowed? method of the controller there was a case for the "edit" case which was getting executed all the time making the second case block unreachable. Modified the code to use the proper case statements for "edit", "delete" and "update". Below is the code snippet for the action_allowed? method after the code changes:
<pre>
def action_allowed?
case params[:action]
# Deny access to anyone except reviewer & author's team
when 'edit' # If response has been submitted, no further editing allowed
response = Response.find(params[:id])
current_user_id?(response.map.reviewer.user_id)
if (response.is_submitted)
return false
end
when 'delete','update'
response = Response.find(params[:id])
current_user_id?(response.map.reviewer.user_id)
when 'view'
response = Response.find(params[:id])
map = response.map

# if it is a review response map, all the members of revieweee team should be able to view the reponse (can be done from heat map)
if map.is_a? ReviewResponseMap
reviewee_team = AssignmentTeam.find(map.reviewee_id)
current_user_id?(response.map.reviewer.user_id) || reviewee_team.has_user(current_user) || (['Administrator','Instructor','Teaching Assistant'].include? current_user.role.name)
else
current_user_id?(response.map.reviewer.user_id)
end
else
current_user
end
end
</pre>

===Instructions to test from GUI===
As the main motive of the project was refactoring the response controller, we list down below the steps to test the flows involved with response controller so as to make sure that nothing is broken due to changes done as part of refactoring and the functionality as expected.
====Testing review====
* Login as a student user.
* Click on the Assignments tab and Select the assignment you want to submit review for others work.
* In the assignments page click on others work and select one of the assignment for review.
* Click on Begin and then answer different questions which are presented in the review rubric.
* Once done with the review, click on Submit review to submit the review.

====Testing peer review====
* Login as a student user.
* Click on the Assignments tab and select the assignment you want to submit the teammate review.
* Click on the your team option.
* In the teammates page, click on the Review option against the user you want to submit the feedback.
* Fill out the questionnaire presented in the teammate review rubric.
* Once done with the responses, click on Submit review to submit the review for the teammate.

===References===

#[https://github.com/anuragshendge/expertiza Forked Repository for Expertiza Project]
#[http://wikis.lib.ncsu.edu/index.php/Expertiza Expertiza project documentation wiki]
#[https://github.com/expertiza/expertiza Expertiza on GitHub]

CSC/ECE 517 Spring 2016/Refactor response controller

2016-04-02T01:23:13Z

Ptrived:

==E1614. Refactoring Response Controller==

This page provide the details of the changes done as part of the refactoring response controller project(E1614).

__TOC__

===A brief overview of Expertiza===
[http://expertiza.ncsu.edu/ Expertiza] project is a platform to create reusable learning objects through peer review. It is an open source project which uses [http://rubyonrails.org/ Ruby on Rails] framework.

===Project Statement===
The main aim of this project was to refactor the response_controller. The following tasks were completed as part of refactoring in this project:

* Moving variable declaration to right places.
* Removing unused variables.
* Fixing code duplication.
* Replacing if else block with switch statements.
* Remove the unreachable code.

===About Response Controller===
The Response controller is responsible for the CRUD(Create, Read, Update and Delete) operations on responses. The users can fill out a questionnaire such as review rubric or feedback on the partner's contribution. So the ResponseController handles the various operations on the responses, which are objects that Expertiza creates when you fill out a questionnaire.

===Changes done as part of refactoring===
====Moving variable declaration to right places====
There were two variables "msg" and "error_msg" in the create method which were declared and initialized to blank strings and then again assigned some string value before it was used in the method. So we moved the variable declaration to the place were it was used and removed the unnecessary variable declaration. Below if the code snippet for the changes done:
<pre>
def create
@map = ResponseMap.find(params[:id]) #assignment/review/metareview id is in params id

set_all_responses

#to save the response for ReviewResponseMap, a questionnaire_id is wrapped in the params
if params[:review][:questionnaire_id]
@questionnaire = Questionnaire.find(params[:review][:questionnaire_id])
@round = params[:review][:round]
else
@round=nil
end

# create the response
if params[:isSubmit].eql?('Yes')
is_submitted = true
else
is_submitted = false
end
@response = Response.create(:map_id => @map.id, :additional_comment => params[:review][:comments],:round => @round, :is_submitted => is_submitted)#,:version_num=>@version)

#Change the order for displaying questions for editing response views.
questions=sort_questions(@questionnaire.questions)

if params[:responses]
create_answers(params, questions)
end

#@map.save
msg = "Your response was successfully saved."
error_msg="Error"
@response.email();
redirect_to :controller => 'response', :action => 'saving', :id => @map.map_id, :return => params[:return], :msg => msg, :error_msg => error_msg, :save_options => params[:save_options]
end
</pre>

====Removing unused variables====
There were some unused variables in this controller. The edit method contained a variable "array_not_empty" variable which was not used anywhere. So we removed the unused variables.

====Fixing code duplication====
There was some duplicate code in the update method. So we removed the duplicate code and retained the code snippet inside the if block. Below is the modified code for the update method:
<pre>
#Update the response and answers when student "edit" existing response
def update
return unless action_allowed?

# the response to be updated
@response = Response.find(params[:id])

msg = ""
begin
@map = @response.map
@response.update_attribute('additional_comment', params[:review][:comments])
if @map.type=="ReviewResponseMap" && @response.round
@questionnaire = @map.questionnaire(@response.round)
elsif @map.type=="ReviewResponseMap"
@questionnaire = @map.questionnaire(nil)
else
@questionnaire = @map.questionnaire
end
questions = @questionnaire.questions.sort { |a,b| a.seq <=> b.seq }

questions=sort_questions(@questionnaire.questions)
create_answers(params,questions)
questions = @questionnaire.questions.sort { |a,b| a.seq <=> b.seq }

if !params[:responses].nil? # for some rubrics, there might be no questions but only file submission (Dr. Ayala's rubric)
params[:responses].each_pair do |k, v|
score = Answer.where(response_id: @response.id, question_id: questions[k.to_i].id).first
unless score
score = Answer.create(:response_id => @response.id, :question_id => questions[k.to_i].id, :answer => v[:score], :comments => v[:comment])
end
score.update_attribute('answer', v[:score])
score.update_attribute('comments', v[:comment])
end
end
if (params['isSubmit'] && (params['isSubmit'].eql?'Yes'))
# Update the submission flag.
@response.update_attribute('is_submitted',true)
else
@response.update_attribute('is_submitted',false)
end
rescue
msg = "Your response was not saved. Cause:189 #{$!}"
end
redirect_to :controller => 'response', :action => 'saving', :id => @map.map_id, :return => params[:return], :msg => msg, :save_options => params[:save_options]
end
</pre>

====Replacing if else block with switch statements====
The redirection method used the if-else statements. The if-else block was replaced with switch statements. Below if is the code snippet for the redirection method after the code changes:
<pre>
def redirection
flash[:error] = params[:error_msg] unless params[:error_msg] and params[:error_msg].empty?
flash[:note] = params[:msg] unless params[:msg] and params[:msg].empty?
@map = Response.find_by_map_id(params[:id])

case params[:return]
when "feedback"
redirect_to :controller => 'grades', :action => 'view_my_scores', :id => @map.reviewer.id
when "teammate"
redirect_to view_student_teams_path student_id: @map.reviewer.id
when "instructor"
redirect_to :controller => 'grades', :action => 'view', :id => @map.response_map.assignment.id
when "assignment_edit"
redirect_to controller: 'assignments', action: 'edit', id: @map.response_map.assignment.id
else
redirect_to :controller => 'student_review', :action => 'list', :id => @map.reviewer.id
end
end
</pre>

====Remove the unreachable code====
In the action_allowed? method of the controller there was a case for the "edit" case which was getting executed all the time making the second case block unreachable. Modified the code to use the proper case statements for "edit", "delete" and "update". Below is the code snippet for the action_allowed? method after the code changes:
<pre>
def action_allowed?
case params[:action]
# Deny access to anyone except reviewer & author's team
when 'edit' # If response has been submitted, no further editing allowed
response = Response.find(params[:id])
current_user_id?(response.map.reviewer.user_id)
if (response.is_submitted)
return false
end
when 'delete','update'
response = Response.find(params[:id])
current_user_id?(response.map.reviewer.user_id)
when 'view'
response = Response.find(params[:id])
map = response.map

# if it is a review response map, all the members of revieweee team should be able to view the reponse (can be done from heat map)
if map.is_a? ReviewResponseMap
reviewee_team = AssignmentTeam.find(map.reviewee_id)
current_user_id?(response.map.reviewer.user_id) || reviewee_team.has_user(current_user) || (['Administrator','Instructor','Teaching Assistant'].include? current_user.role.name)
else
current_user_id?(response.map.reviewer.user_id)
end
else
current_user
end
end
</pre>

===Instructions to test from GUI===
As the main motive of the project was refactoring the response controller, we list down below the steps to test the flows involved with response controller so as to make sure that nothing is broken due to changes done as part of refactoring and the functionality as expected.
====Testing review====
* Login as a student user.
* Click on the Assignment and Select the assignment you want to submit review for others work.
* In the assignments page click on others work and select one of the assignment for review.
* Click on Begin and then answer different questions which are presented in the review rubric.
* Once done with the review, click on Submit review to submit the review.

=====Testing peer review=====
*

===References===

#[https://github.com/anuragshendge/expertiza Forked Repository for Expertiza Project]
#[http://wikis.lib.ncsu.edu/index.php/Expertiza Expertiza project documentation wiki]
#[https://github.com/expertiza/expertiza Expertiza on GitHub]

CSC/ECE 517 Spring 2016/Refactor response controller

2016-03-29T18:23:02Z

Ptrived: Created page with "==E1614. Refactoring Response Controller== This page provide the details of the changes done as part of the refactoring response controller project(E1614). __TOC__ ===A brief ..."

==E1614. Refactoring Response Controller==

This page provide the details of the changes done as part of the refactoring response controller project(E1614).

__TOC__

===A brief overview of Expertiza===
[http://expertiza.ncsu.edu/ Expertiza] project is a platform to create reusable learning objects through peer review. It is an open source project which uses [http://rubyonrails.org/ Ruby on Rails] framework.

===Project Statement===
The main aim of this project was to refactor the response_controller. The following tasks were completed as part of refactoring in this project:

* Moving variable declaration to right places.
* Removing unused variables.
* Fixing code duplication.
* Replacing if else block with switch statements.
* Remove the unreachable code.

===About Response Controller===
The Response controller is responsible for the CRUD(Create, Read, Update and Delete) operations on responses. The users can fill out a questionnaire such as review rubric or feedback on the partner's contribution. So the ResponseController handles the various operations on the responses, which are objects that Expertiza creates when you fill out a questionnaire.

===Changes done as part of refactoring===
====Moving variable declaration to right places====
There were two variables "msg" and "error_msg" in the create method which were declared and initialized to blank strings and then again assigned some string value before it was used in the method. So we moved the variable declaration to the place were it was used and removed the unnecessary variable declaration. Below if the code snippet for the changes done:
<pre>
def create
@map = ResponseMap.find(params[:id]) #assignment/review/metareview id is in params id

set_all_responses

#to save the response for ReviewResponseMap, a questionnaire_id is wrapped in the params
if params[:review][:questionnaire_id]
@questionnaire = Questionnaire.find(params[:review][:questionnaire_id])
@round = params[:review][:round]
else
@round=nil
end

# create the response
if params[:isSubmit].eql?('Yes')
is_submitted = true
else
is_submitted = false
end
@response = Response.create(:map_id => @map.id, :additional_comment => params[:review][:comments],:round => @round, :is_submitted => is_submitted)#,:version_num=>@version)

#Change the order for displaying questions for editing response views.
questions=sort_questions(@questionnaire.questions)

if params[:responses]
create_answers(params, questions)
end

#@map.save
msg = "Your response was successfully saved."
error_msg="Error"
@response.email();
redirect_to :controller => 'response', :action => 'saving', :id => @map.map_id, :return => params[:return], :msg => msg, :error_msg => error_msg, :save_options => params[:save_options]
end
</pre>

====Removing unused variables====
There were some unused variables in this controller. The edit method contained a variable "array_not_empty" variable which was not used anywhere. So we removed the unused variables.

====Fixing code duplication====
There was some duplicate code in the update method. So we removed the duplicate code and retained the code snippet inside the if block. Below is the modified code for the update method:
<pre>
#Update the response and answers when student "edit" existing response
def update
return unless action_allowed?

# the response to be updated
@response = Response.find(params[:id])

msg = ""
begin
@map = @response.map
@response.update_attribute('additional_comment', params[:review][:comments])
if @map.type=="ReviewResponseMap" && @response.round
@questionnaire = @map.questionnaire(@response.round)
elsif @map.type=="ReviewResponseMap"
@questionnaire = @map.questionnaire(nil)
else
@questionnaire = @map.questionnaire
end
questions = @questionnaire.questions.sort { |a,b| a.seq <=> b.seq }

questions=sort_questions(@questionnaire.questions)
create_answers(params,questions)
questions = @questionnaire.questions.sort { |a,b| a.seq <=> b.seq }

if !params[:responses].nil? # for some rubrics, there might be no questions but only file submission (Dr. Ayala's rubric)
params[:responses].each_pair do |k, v|
score = Answer.where(response_id: @response.id, question_id: questions[k.to_i].id).first
unless score
score = Answer.create(:response_id => @response.id, :question_id => questions[k.to_i].id, :answer => v[:score], :comments => v[:comment])
end
score.update_attribute('answer', v[:score])
score.update_attribute('comments', v[:comment])
end
end
if (params['isSubmit'] && (params['isSubmit'].eql?'Yes'))
# Update the submission flag.
@response.update_attribute('is_submitted',true)
else
@response.update_attribute('is_submitted',false)
end
rescue
msg = "Your response was not saved. Cause:189 #{$!}"
end
redirect_to :controller => 'response', :action => 'saving', :id => @map.map_id, :return => params[:return], :msg => msg, :save_options => params[:save_options]
end
</pre>

====Replacing if else block with switch statements====
The redirection method used the if-else statements. The if-else block was replaced with switch statements. Below if is the code snippet for the redirection method after the code changes:
<pre>
def redirection
flash[:error] = params[:error_msg] unless params[:error_msg] and params[:error_msg].empty?
flash[:note] = params[:msg] unless params[:msg] and params[:msg].empty?
@map = Response.find_by_map_id(params[:id])

case params[:return]
when "feedback"
redirect_to :controller => 'grades', :action => 'view_my_scores', :id => @map.reviewer.id
when "teammate"
redirect_to view_student_teams_path student_id: @map.reviewer.id
when "instructor"
redirect_to :controller => 'grades', :action => 'view', :id => @map.response_map.assignment.id
when "assignment_edit"
redirect_to controller: 'assignments', action: 'edit', id: @map.response_map.assignment.id
else
redirect_to :controller => 'student_review', :action => 'list', :id => @map.reviewer.id
end
end
</pre>

====Remove the unreachable code====
In the action_allowed? method of the controller there was a case for the "edit" case which was getting executed all the time making the second case block unreachable. Modified the code to use the proper case statements for "edit", "delete" and "update". Below is the code snippet for the action_allowed? method after the code changes:
<pre>
def action_allowed?
case params[:action]
# Deny access to anyone except reviewer & author's team
when 'edit' # If response has been submitted, no further editing allowed
response = Response.find(params[:id])
current_user_id?(response.map.reviewer.user_id)
if (response.is_submitted)
return false
end
when 'delete','update'
response = Response.find(params[:id])
current_user_id?(response.map.reviewer.user_id)
when 'view'
response = Response.find(params[:id])
map = response.map

# if it is a review response map, all the members of revieweee team should be able to view the reponse (can be done from heat map)
if map.is_a? ReviewResponseMap
reviewee_team = AssignmentTeam.find(map.reviewee_id)
current_user_id?(response.map.reviewer.user_id) || reviewee_team.has_user(current_user) || (['Administrator','Instructor','Teaching Assistant'].include? current_user.role.name)
else
current_user_id?(response.map.reviewer.user_id)
end
else
current_user
end
end
</pre>

===References===

#[https://github.com/anuragshendge/expertiza Forked Repository for Expertiza Project]
#[http://wikis.lib.ncsu.edu/index.php/Expertiza Expertiza project documentation wiki]
#[https://github.com/expertiza/expertiza Expertiza on GitHub]

CSC/ECE 517 Spring 2016

2016-03-29T18:22:51Z

Ptrived: /* Writing Assignment 2 */

==Writing Assignment 1==
*[[CSC/ECE 517 Spring 2016/Active Job]]

==Calibration Assignment Submissions==
*[[Calibration Assignment Submission (OmniAuth)]]
*[[Calibration Assignment Submission (Patch_verb)]]

==Writing Assignment 2==
*[[CSC/ECE 517 Spring 2016/Mozilla Implement HTML5 form validation]]

*[[CSC/ECE 517 Spring 2016/Functional Tests for Questionnaire Controller]]

*[[CSC/ECE 517 Spring 2016 / Expertiza Self-Review Feature]]

*[[CSC/ECE 517 Spring 2016/Refactor different question types from quiz feature]]

*[[CSC/ECE 517 Spring 2016/Implement private browsing]]

*[[CSC/ECE 517 Spring 2016/Write automated tests for WebDriver]]

*[[CSC/ECE 517 Spring 2016/Refactor sign_up_sheet_controller.rb and sign_up_topic.rb]]

*[[CSC/ECE 517 Spring 2016/Implement Common Parts of the CSSOM API]]

*[[CSC/ECE 517 Spring 2016/Functional tests for assignment creation function]]

*[[CSC/ECE 517 Spring 2016/E1604. Functional tests for Calibration function]]

*[[CSC/ECE 517 Spring 2016/Refactor and write unit tests for question type.rb]]

*[[CSC/ECE 517 Spring 2016/Refactor response controller]]

User:Ptrived

2016-03-23T17:14:52Z