CSC/ECE 517 Fall 2016/E1642. Refactor review response map.rb: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
No edit summary
mNo edit summary
 
(30 intermediate revisions by 3 users not shown)
Line 1: Line 1:
== Peer Review Information ==
==Introduction==
For testing the changes made, the following credentials are recommended:
* Instructor Login: '''username:''' ''instructor6'' '''password:''' ''password''
* Student Login: '''username:''' ''student11'' '''password:''' ''password''
* Student Login: '''username:''' ''student1862'' '''password:''' ''password''
The above users are suggested for testing because there are a few users which would lead to exceptions upon login for unknown reasons completely unrelated to our work.
 
 
=='''Introduction to Expertiza'''==
===Background===
===Background===


[http://expertiza.ncsu.edu/ Expertiza] is an open source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.
[http://expertiza.ncsu.edu/ Expertiza] is an open source project based on [http://rubyonrails.org/ Ruby on Rails] framework. It is essentially an educational tool that allows an instructor to create and modify assignments, add topics to assignments, grade students,create evaluation rubrics, etc. Students use Expertiza for submission of their assignments, forming teams, reviewing other students, choosing topics etc. Expertiza also has a bidding feature where students can bid for different topics for an assignment and then it assigns topics to students depending on their priorities and also automatically forms teams.
 


=== Problem Statement ===
=== Problem Statement ===
The following were the tasks identified to accomplish through this project. These tasks are part of the review_response_map.rb, assignment.rb and  metareview_response_maps.
Following are the tasks which need to be completed as a part of our project.  
*'''P1: ''' Code Climate shows import method is complex, because of lots of checks. This method can be fixed by adding private methods for raising import error. However, you need to check if this functionality is called anywhere.  
*'''P1: ''' Code Climate shows import method is complex, because of lots of checks. This method can be fixed by adding private methods for raising import error. However, you need to check if this functionality is called anywhere.  
*'''P2: ''' Get_assessments_round_for method can be renamed to get_responses_for_team_round. Team_id private variable is not needed.
*'''P2: ''' Get_assessments_round_for method can be renamed to get_responses_for_team_round. Team_id private variable is not needed.
Line 24: Line 15:
*'''P8: ''' Write missing unit tests for existing methods.
*'''P8: ''' Write missing unit tests for existing methods.


== Reason for Refractor==
The purpose of this work is to rewrite the code of seven file for several reason listed below.


== Files Modified in this Project ==
== Files Modified in this Project ==
*review_response_map.rb
* review_response_map.rb
*assignment.rb   
* assignment.rb   
*metareview_response_maps
* on_the_fly_calc.rb
 
* vm_question_response.rb
* factories.rb
* review_response_map_spec.rb (created)


== Solutions implemented and Delivered ==
== Solutions implemented and Delivered ==
Line 38: Line 29:


==='''P2: ''' ===
==='''P2: ''' ===
*Get_assessments_round_for Method is refactored to get_responses_for_team_round  as required. In order to test from UI follow the steps:
*Get_assessments_round_for Method is refactored to get_responses_for_team_round  as required.  
*Login as instructor6
We do not need a separate private variable for  Team_id as we can directly use Team.id for the same.  
*Click on Manage... tab and then on the Assignments section
[[File:P2.png|frame|upright|center|Get_assessments_round_for refactoring examples]]
*Go on Wikipedia contribution assignment and then click on the View scores button.(This will call the above method)
 
*Then for any one team, click on Alternate View (This is another call to the method via a different route.)
==='''P3: ''' ===
 
There is no need for the inner iteration as for every review for a particular round, there is only one metareview. The second iteration was redundant hence is removed.  


[[File:P2.png|frame|upright|center|Get_assessments_round_for refactoring examples]]
[[File:P3.png|frame|upright|center|Second Iteration Removed]]


==='''P4: ''' ===
==='''P4: ''' ===
Line 51: Line 44:


This function returns a map containing the final versions of a reviewer. The function has duplicate code for scenario with varying rubric and non varying rubric. We moved this code into a new function with method name prepare_review_response. As we removed the duplicate code, this will reduce the Assignment Branch Condition size.
This function returns a map containing the final versions of a reviewer. The function has duplicate code for scenario with varying rubric and non varying rubric. We moved this code into a new function with method name prepare_review_response. As we removed the duplicate code, this will reduce the Assignment Branch Condition size.
[[File:P4.png|frame|upright|center|Assignments section refactoring examples(code in the red is the "before" version and green is "after" editing version)]]
==='''P6: ''' ===
[http://api.rubyonrails.org/classes/ActiveRecord/FinderMethods.html#method-i-find_by find_by] method can be used instead of where.first. Although here it wasn't used since the lines of code that used where.first were redundant and were removed to solve other problems. But its application can found on this page http://api.rubyonrails.org/classes/ActiveRecord/FinderMethods.html#method-i-find_by
==='''P7: ''' ===
Over long lines can be hard to read in a small screen. Hence proper use of refactoring the lines can save the overhead of reading and understanding long lines of code. In this problem we have wrapped the overlong lines. In an IDE such as RubyMine we can use custom methods such as "Use Soft Wraps in editor" to ease the process.
==='''P8: ''' ===
There were no tests present for review_response_map. New unit tests were added to test the functionalities and the test coverage result shows that the coverage is now 71.43 %
New Added Code
require 'rails_helper'
describe 'ReviewResponseMap' do
  before(:each) do
    create(:participant)
    @assignment_questionnaire= create(:assignment_questionnaire)
    @review_response=create(:review_response_map)
  end
  describe "#validity" do
    it "should have a reviewee_id, reviewed_object_id" do
      expect(@review_response.reviewee_id).to be_instance_of(Fixnum)
      expect(@review_response.reviewed_object_id).to be_instance_of(Fixnum)
    end
    it "should return the questionnaire" do
      review_questionnaire=@review_response.questionnaire 1
      expect(review_questionnaire).to be_instance_of(ReviewQuestionnaire)
      expect(review_questionnaire.id).to be(1)
    end
    it "should return title" do
      expect(@review_response.get_title).to eql "Review"
    end
    it "should return export_fields" do
      export_fields=ReviewResponseMap.export_fields nil
      expect(export_fields.length).to be(2)
    end
  end
  describe "#final_versions_from_reviewer method" do
    #reviewer giver three reviews, result must have final review
    it "should return final version of review  when assignment has non varying rubric" do
      create(:response_1)
      create(:response_1)
      create(:response_1)
      map=ReviewResponseMap.final_versions_from_reviewer(1)
      expect(map).to be_truthy
      expect(map[:review][:questionnaire_id]).to be(1)
      expect(map[:review][:response_ids].length).to be(1)
      expect(map[:review][:response_ids][0]).to be(3)
    end
   
#reviewer giver two reviews, result must have final review
    it "should return final version of review when assignment has varying rubric" do
      @assignment_questionnaire2= create(:assignment_questionnaire)
      @assignment_questionnaire.update(used_in_round:1)
      @assignment_questionnaire2.update(used_in_round:2)
      create(:response_1)
      create(:response_1)
      map=ReviewResponseMap.final_versions_from_reviewer(1)
      expect(map).to be_truthy
      expect(map["review round1".to_sym][:questionnaire_id]).to be(1)
      expect(map["review round1".to_sym][:response_ids].length).to be(1)
      expect(map["review round1".to_sym][:response_ids][0]).to be(2)
    end
  end
  describe "#get_responses_for_team_round method" do
#There are 2 reviewers for a team and both have given their reviews for round 1
    it "should return correct number of reponses per round for a team" do
      @review1 = create(:response_1)
      @review2 = create(:response_1)
      @responsemap2= create(:response_map_review)
      @review2.update(response_map: @responsemap2 )
      @team = build(:assignment_team)
      @team.id=1
      expect(ReviewResponseMap.get_responses_for_team_round(@team,1).size).to eq(2)
    end
#There are 2 reviewers for a team but one reviewer has given review for round1 and another one for round2
    it "should return only those reponses which are related to a particular round" do
      @review1 = create(:response_1)
      @review2 = create(:response_1)
      @responsemap2= create(:response_map_review)
      @review2.update(response_map: @responsemap2 )
      @review2.update(round: 2)
      @team = build(:assignment_team)
      @team.id=1
      expect(ReviewResponseMap.get_responses_for_team_round(@team,1).size).to eq(1)
    end
  end
  describe "#Test for the metareview_response_maps method" do
# There is 1 metareview each for the reviews given by a particular reviewer in round 1 and round 2
    it "should return correct number of metareviews for a particular reviewer" do
      @review1 = create(:response_1)
      @metareview1=create(:response_map_metareview)
      @metareview1.update(reviewed_object_id: @review1.id)
      @review2 = create(:response_1)
      @review2.update(round: 2)
      @metareview2=create(:response_map_metareview)
      @metareview2.update(reviewed_object_id: @review2.id)
      @response_map=@review1.response_map
      expect(@response_map.metareview_response_maps.size).to eq(2)
    end
  end
end
==Testing==
New Test Cases were created and the test coverage results are as shown.
[[File:P8.png|frame|upright|center|Test coverage increased to 71.43% from 0]]
==UI Testing==
===P2 UI TESTING===
This functionality is used for viewing assignment scores for different teams and a detailed analysis for the same.


In order to test from UI follow the steps:
In order to test from UI follow the steps:


* Login as '''instructor6'''
*Login as instructor6
* Click on Manage -> tab ->Assignments section
*Click on Manage... tab and then on the Assignments section
* Click on '''view review report'''
*Go on Wikipedia contribution assignment and then click on the View scores button.(This will call the above method)
* Click on '''review summary'''
*Then for any one team, click on Alternate View (This is another call to the method via a different route.)
 
===P4 UI TESTING===
This functionality is used to check the final review summary of a assignment


In order to test from UI follow the steps:


[[File:P4.png|frame|upright|center|Assignments section refactoring examples]]
* Login as instructor6
* Click on Manage -> tab ->Assignments section
* Click on view review report for any assignment
* Click on reviewer summary for any reviewer

Latest revision as of 15:07, 10 November 2016

Introduction

Background

Expertiza is an open source project based on Ruby on Rails framework. It is essentially an educational tool that allows an instructor to create and modify assignments, add topics to assignments, grade students,create evaluation rubrics, etc. Students use Expertiza for submission of their assignments, forming teams, reviewing other students, choosing topics etc. Expertiza also has a bidding feature where students can bid for different topics for an assignment and then it assigns topics to students depending on their priorities and also automatically forms teams.

Problem Statement

Following are the tasks which need to be completed as a part of our project.

  • P1: Code Climate shows import method is complex, because of lots of checks. This method can be fixed by adding private methods for raising import error. However, you need to check if this functionality is called anywhere.
  • P2: Get_assessments_round_for method can be renamed to get_responses_for_team_round. Team_id private variable is not needed.
  • P3: metareview_response_maps rename, refactoring can be done. No need to do second iteration.
  • P4: Assignment Branch Condition size for final_versions_from_reviewer is too high.
  • P5: Assignment Branch Condition size for get_assessments_round_for is too high, try to get rid of the nested if.
  • P6: Use find_by instead of where.first, e.g. line 60, 65, etc.
  • P7: Wrap the over-long lines: 58, 62 and 181.
  • P8: Write missing unit tests for existing methods.


Files Modified in this Project

  • review_response_map.rb
  • assignment.rb
  • on_the_fly_calc.rb
  • vm_question_response.rb
  • factories.rb
  • review_response_map_spec.rb (created)

Solutions implemented and Delivered

P1:

We could find a call to the import method from 4 relevant places in the importFile method in the ImportFileController. Of them 2 are for different models and the other 2 are having different number of parameters. Therefore we concluded that the import method in the review_response_map.rb is not called from anywhere and so we dropped the method.

P2:

  • Get_assessments_round_for Method is refactored to get_responses_for_team_round as required.

We do not need a separate private variable for Team_id as we can directly use Team.id for the same.

Get_assessments_round_for refactoring examples

P3:

There is no need for the inner iteration as for every review for a particular round, there is only one metareview. The second iteration was redundant hence is removed.

Second Iteration Removed

P4:

Assignment Branch Condition size for final_versions_from_reviewer is too high:

This function returns a map containing the final versions of a reviewer. The function has duplicate code for scenario with varying rubric and non varying rubric. We moved this code into a new function with method name prepare_review_response. As we removed the duplicate code, this will reduce the Assignment Branch Condition size.

Assignments section refactoring examples(code in the red is the "before" version and green is "after" editing version)


P6:

find_by method can be used instead of where.first. Although here it wasn't used since the lines of code that used where.first were redundant and were removed to solve other problems. But its application can found on this page http://api.rubyonrails.org/classes/ActiveRecord/FinderMethods.html#method-i-find_by

P7:

Over long lines can be hard to read in a small screen. Hence proper use of refactoring the lines can save the overhead of reading and understanding long lines of code. In this problem we have wrapped the overlong lines. In an IDE such as RubyMine we can use custom methods such as "Use Soft Wraps in editor" to ease the process.


P8:

There were no tests present for review_response_map. New unit tests were added to test the functionalities and the test coverage result shows that the coverage is now 71.43 %

New Added Code

require 'rails_helper'
describe 'ReviewResponseMap' do
 before(:each) do
   create(:participant)
   @assignment_questionnaire= create(:assignment_questionnaire)
   @review_response=create(:review_response_map)
 end
 describe "#validity" do
   it "should have a reviewee_id, reviewed_object_id" do
     expect(@review_response.reviewee_id).to be_instance_of(Fixnum)
     expect(@review_response.reviewed_object_id).to be_instance_of(Fixnum)
   end
   it "should return the questionnaire" do
     review_questionnaire=@review_response.questionnaire 1
     expect(review_questionnaire).to be_instance_of(ReviewQuestionnaire)
     expect(review_questionnaire.id).to be(1)
   end
   it "should return title" do
     expect(@review_response.get_title).to eql "Review"
   end
   it "should return export_fields" do
     export_fields=ReviewResponseMap.export_fields nil
     expect(export_fields.length).to be(2)
   end
 end
 describe "#final_versions_from_reviewer method" do
   #reviewer giver three reviews, result must have final review
   it "should return final version of review  when assignment has non varying rubric" do
     create(:response_1)
     create(:response_1)
     create(:response_1)
     map=ReviewResponseMap.final_versions_from_reviewer(1)
     expect(map).to be_truthy
     expect(map[:review][:questionnaire_id]).to be(1)
     expect(map[:review][:response_ids].length).to be(1)
     expect(map[:review][:response_ids][0]).to be(3)
   end
   
#reviewer giver two reviews, result must have final review
   it "should return final version of review when assignment has varying rubric" do
     @assignment_questionnaire2= create(:assignment_questionnaire)
     @assignment_questionnaire.update(used_in_round:1)
     @assignment_questionnaire2.update(used_in_round:2)
     create(:response_1)
     create(:response_1)
     map=ReviewResponseMap.final_versions_from_reviewer(1)
     expect(map).to be_truthy
     expect(map["review round1".to_sym][:questionnaire_id]).to be(1)
     expect(map["review round1".to_sym][:response_ids].length).to be(1)
     expect(map["review round1".to_sym][:response_ids][0]).to be(2)
   end
 end


 describe "#get_responses_for_team_round method" do
#There are 2 reviewers for a team and both have given their reviews for round 1
   it "should return correct number of reponses per round for a team" do
     @review1 = create(:response_1)
     @review2 = create(:response_1)
     @responsemap2= create(:response_map_review)
     @review2.update(response_map: @responsemap2 )
     @team = build(:assignment_team)
     @team.id=1
     expect(ReviewResponseMap.get_responses_for_team_round(@team,1).size).to eq(2)
   end
#There are 2 reviewers for a team but one reviewer has given review for round1 and another one for round2
   it "should return only those reponses which are related to a particular round" do
     @review1 = create(:response_1)
     @review2 = create(:response_1)
     @responsemap2= create(:response_map_review)
     @review2.update(response_map: @responsemap2 )
     @review2.update(round: 2)
     @team = build(:assignment_team)
     @team.id=1
     expect(ReviewResponseMap.get_responses_for_team_round(@team,1).size).to eq(1)
   end
 end
 describe "#Test for the metareview_response_maps method" do
# There is 1 metareview each for the reviews given by a particular reviewer in round 1 and round 2
   it "should return correct number of metareviews for a particular reviewer" do
     @review1 = create(:response_1)
     @metareview1=create(:response_map_metareview)
     @metareview1.update(reviewed_object_id: @review1.id)
     @review2 = create(:response_1)
     @review2.update(round: 2)
     @metareview2=create(:response_map_metareview)
     @metareview2.update(reviewed_object_id: @review2.id)
     @response_map=@review1.response_map
     expect(@response_map.metareview_response_maps.size).to eq(2)
   end
 end
end

Testing

New Test Cases were created and the test coverage results are as shown.

Test coverage increased to 71.43% from 0

UI Testing

P2 UI TESTING

This functionality is used for viewing assignment scores for different teams and a detailed analysis for the same.

In order to test from UI follow the steps:

  • Login as instructor6
  • Click on Manage... tab and then on the Assignments section
  • Go on Wikipedia contribution assignment and then click on the View scores button.(This will call the above method)
  • Then for any one team, click on Alternate View (This is another call to the method via a different route.)

P4 UI TESTING

This functionality is used to check the final review summary of a assignment

In order to test from UI follow the steps:

  • Login as instructor6
  • Click on Manage -> tab ->Assignments section
  • Click on view review report for any assignment
  • Click on reviewer summary for any reviewer