Expertiza_Wiki - User contributions [en]

CSC/ECE 517 Spring 2016/Write automated tests for WebDriver

2016-04-28T04:01:34Z

Skunapa: /* Design pattern */

{{student editor|course = [[Wikipedia:Wiki_Ed/North_Carolina_State_University/Object-Oriented_Design_and_Development_(Spring_2016)]] }}

== Write automated tests for the WebDriver server ==

ServoServo is an open source prototype web browser layout engine being developed by Mozilla, and it is written in Rust language. It is currently developed on 64bit OS X, 64bit Linux, Android, and Gonk (Firefox OS). The main idea is to create a highly parallel environment, in which different components can be handled by fine grained, isolated tasks. The different components can be rendering, HTML parsing, etc.

== Background information: ==

=== Python: ===
Python is a widely used high-level, general-purpose, interpreted, dynamic programming language. Python supports multiple programming paradigms, including object oriented, imperative and functional programming or procedural styles. It features a dynamic type system and automatic memory management and has a large and comprehensive standard library. Modules for creating graphical user interfaces, connecting to relational databases, pseudorandom number generators, arithmetic with arbitrary precision decimals,[77] manipulating regular expressions, and doing unit testing are also included.

=== Servo: ===
Servo is a project to develop a new Web browser engine to help mitigate vulnerabilities associated with incorrect memory management and data races. Servo is written in Rust, a new language designed specifically with Servo's requirements in mind. Rust provides a task-parallel infrastructure and a strong type system that enforces memory safety and data race freedom.

Servo is focused both on supporting a full Web browser, through the use of the purely HTML-based user interface Browser.html and on creating a solid, embeddable engine. Although Servo was originally a research project, it was implemented with the goal of having production-quality code and is in the process of shipping several of its components in the Firefox browser.

[[File:ServoArchitecture.JPG]]

The above diagram denotes several of the tasks that are associated with building the Servo web browser. Each of the boxes represents a task that is implemented in Rust and the lines indicates relationships between the tasks.

=== WebDriver: ===
The WebDriver specification defines a communication protocol for automating interactions with a web browser that would traditionally require a human being (eg. clicking the mouse on a particular link; typing text into an input field; etc.) This protocol is being implemented by major browsers such as Firefox, Edge, and Chrome, and Servo also provides a WebDriver server. The goal of this project is both to submit a broad range of automated client tests using the Python WebDriver API that can be run against multiple browsers, and also to report on how many of the tests Servo currently passes.

== Scope ==
===Initial Steps===
* Compile Servo and ensure that it runs on <code>tests/html/about-mozilla.html</code>
* To run a simple webdriver script using the python client against Servo.
* Create a<code>mach</code> command (<code>test-webdriver</code>) in <code>python/servo/testing_commands.py</code> that will import each python file in the directory and execute the <code>test</code> method contained within it by invoking the Servo process before running each test, and killing it after each test finishes.
* Create a <code>webdriver</code> directory under <code>tests</code> where the forthcoming tests will reside.
* Writing an automated test for loading a particular URL, asserting that the resulting page is the expected url.
===Subsequent Steps===
* Writing tests that exercise the lower-level command API (eg. self.send_command("POST", "url", body)) to test server capabilities:
* Writing a test with a simple get to a url that exists on the server, ensure that we get the right response type
* Sending a non-string as the URL and ensuring that we get the right kind of error message.
* Trying other kinds of non-conforming messages e.g. ones with extra fields (should be ignored) or no url field (should return the correct type of error)
* Testing that we are actually loading the correct page. This will require the use of some other webdriver functions e.g. to get an element's text, or take a screenshot
* Testing that Get always navigates the top level browsing context (by navigating to a page with an iframe, setting the current bc to that iframe, and ensuring that it is not just the iframe that is navigated).
* Trying Get with pages that return non-200 responses.
* Loading a page that loads slower than the timeout, and ensure we get the correct error message

== Design ==
Design patterns are not applicable as our task involved writing automated tests in python. However, the Implementation section below provides details of the steps the way it was implemented.

We have implemented the test cases based on the input from the Mozilla team and there were several major tasks that were implemented.

1. Execute tests on a standalone HTTP server: In this we wanted to test the functionality if a web page opens as expected on the wed driver engine. We have implemented this by giving a URL which opens a web page whose actual URL is different when the page opens. We matched the web page expected with the one that is opened to successfully execute the test case.

2.

== Implementation ==
===Initial Steps===
The following steps were followed to meet the project requirements as per this github page.

=== Step 1: ===
We created a new <code>mach</code> command (<code>test-webdriver</code>) in <code>python/servo/testing_commands.py</code> file. It imports each python file in the directory and executes the <code>test</code> method contained within it.

=== Step 2: ===

We created tests in test.py and test1.py for loading a particular URL, asserting that the resulting page is the expected url.

===Subsequent Steps===
===Step 1:===
We would create 3 different test.py files for each of the different tests that would reside in tests/webdriver folder.

===Step 2:===
Creating a ServoProcess that would be used by the test.py files to start a servo session.

===Step 3:===
Catching exceptions when the tests fail and displaying them in proper format based on the kind of different error.

== Testing ==
Following are the steps to run the automated tests for WebDriver:
git clone <nowiki>https://github.com/krunal3103/servo</nowiki>
Note: Follow the steps listed in the Readme.md file of this link to clone and build servo on your local machine.
cd servo

./mach test-webdriver
You will see that all tests pass as expected.

== Pull Request ==
Here is our [https://github.com/servo/servo/pull/10113 pull request]. In the link you can see all code snippets and commits as well as integration test progression information.

CSC/ECE 517 Spring 2016/Write automated tests for WebDriver

2016-04-28T00:58:43Z

Skunapa: /* Servo: */

{{student editor|course = [[Wikipedia:Wiki_Ed/North_Carolina_State_University/Object-Oriented_Design_and_Development_(Spring_2016)]] }}

== Write automated tests for the WebDriver server ==

ServoServo is an open source prototype web browser layout engine being developed by Mozilla, and it is written in Rust language. It is currently developed on 64bit OS X, 64bit Linux, Android, and Gonk (Firefox OS). The main idea is to create a highly parallel environment, in which different components can be handled by fine grained, isolated tasks. The different components can be rendering, HTML parsing, etc.

== Background information: ==

=== Python: ===
Python is a widely used high-level, general-purpose, interpreted, dynamic programming language. Python supports multiple programming paradigms, including object oriented, imperative and functional programming or procedural styles. It features a dynamic type system and automatic memory management and has a large and comprehensive standard library. Modules for creating graphical user interfaces, connecting to relational databases, pseudorandom number generators, arithmetic with arbitrary precision decimals,[77] manipulating regular expressions, and doing unit testing are also included.

=== Servo: ===
Servo is a project to develop a new Web browser engine to help mitigate vulnerabilities associated with incorrect memory management and data races. Servo is written in Rust, a new language designed specifically with Servo's requirements in mind. Rust provides a task-parallel infrastructure and a strong type system that enforces memory safety and data race freedom.

Servo is focused both on supporting a full Web browser, through the use of the purely HTML-based user interface Browser.html and on creating a solid, embeddable engine. Although Servo was originally a research project, it was implemented with the goal of having production-quality code and is in the process of shipping several of its components in the Firefox browser.

[[File:ServoArchitecture.JPG]]

The above diagram denotes several of the tasks that are associated with building the Servo web browser. Each of the boxes represents a task that is implemented in Rust and the lines indicates relationships between the tasks.

=== WebDriver: ===
The WebDriver specification defines a communication protocol for automating interactions with a web browser that would traditionally require a human being (eg. clicking the mouse on a particular link; typing text into an input field; etc.) This protocol is being implemented by major browsers such as Firefox, Edge, and Chrome, and Servo also provides a WebDriver server. The goal of this project is both to submit a broad range of automated client tests using the Python WebDriver API that can be run against multiple browsers, and also to report on how many of the tests Servo currently passes.

== Scope ==
===Initial Steps===
* Compile Servo and ensure that it runs on <code>tests/html/about-mozilla.html</code>
* To run a simple webdriver script using the python client against Servo.
* Create a<code>mach</code> command (<code>test-webdriver</code>) in <code>python/servo/testing_commands.py</code> that will import each python file in the directory and execute the <code>test</code> method contained within it by invoking the Servo process before running each test, and killing it after each test finishes.
* Create a <code>webdriver</code> directory under <code>tests</code> where the forthcoming tests will reside.
* Writing an automated test for loading a particular URL, asserting that the resulting page is the expected url.
===Subsequent Steps===
* Writing tests that exercise the lower-level command API (eg. self.send_command("POST", "url", body)) to test server capabilities:
* Writing a test with a simple get to a url that exists on the server, ensure that we get the right response type
* Sending a non-string as the URL and ensuring that we get the right kind of error message.
* Trying other kinds of non-conforming messages e.g. ones with extra fields (should be ignored) or no url field (should return the correct type of error)
* Testing that we are actually loading the correct page. This will require the use of some other webdriver functions e.g. to get an element's text, or take a screenshot
* Testing that Get always navigates the top level browsing context (by navigating to a page with an iframe, setting the current bc to that iframe, and ensuring that it is not just the iframe that is navigated).
* Trying Get with pages that return non-200 responses.
* Loading a page that loads slower than the timeout, and ensure we get the correct error message

== Design pattern ==
Design patterns are not applicable as our task involved writing automated tests in python. However, the Implementation section below provides details of the steps the way it was implemented.

== Implementation ==
===Initial Steps===
The following steps were followed to meet the project requirements as per this github page.

=== Step 1: ===
We created a new <code>mach</code> command (<code>test-webdriver</code>) in <code>python/servo/testing_commands.py</code> file. It imports each python file in the directory and executes the <code>test</code> method contained within it.

=== Step 2: ===

We created tests in test.py and test1.py for loading a particular URL, asserting that the resulting page is the expected url.

===Subsequent Steps===
===Step 1:===
We would create 3 different test.py files for each of the different tests that would reside in tests/webdriver folder.

===Step 2:===
Creating a ServoProcess that would be used by the test.py files to start a servo session.

===Step 3:===
Catching exceptions when the tests fail and displaying them in proper format based on the kind of different error.

== Testing ==
Following are the steps to run the automated tests for WebDriver:
git clone <nowiki>https://github.com/krunal3103/servo</nowiki>
Note: Follow the steps listed in the Readme.md file of this link to clone and build servo on your local machine.
cd servo

./mach test-webdriver
You will see that all tests pass as expected.

== Pull Request ==
Here is our [https://github.com/servo/servo/pull/10113 pull request]. In the link you can see all code snippets and commits as well as integration test progression information.

CSC/ECE 517 Spring 2016/Write automated tests for WebDriver

2016-04-28T00:54:17Z

Skunapa: /* Servo: */

{{student editor|course = [[Wikipedia:Wiki_Ed/North_Carolina_State_University/Object-Oriented_Design_and_Development_(Spring_2016)]] }}

== Write automated tests for the WebDriver server ==

ServoServo is an open source prototype web browser layout engine being developed by Mozilla, and it is written in Rust language. It is currently developed on 64bit OS X, 64bit Linux, Android, and Gonk (Firefox OS). The main idea is to create a highly parallel environment, in which different components can be handled by fine grained, isolated tasks. The different components can be rendering, HTML parsing, etc.

== Background information: ==

=== Python: ===
Python is a widely used high-level, general-purpose, interpreted, dynamic programming language. Python supports multiple programming paradigms, including object oriented, imperative and functional programming or procedural styles. It features a dynamic type system and automatic memory management and has a large and comprehensive standard library. Modules for creating graphical user interfaces, connecting to relational databases, pseudorandom number generators, arithmetic with arbitrary precision decimals,[77] manipulating regular expressions, and doing unit testing are also included.

=== Servo: ===
[[File:ServoArchitecture.JPG]]

=== WebDriver: ===
The WebDriver specification defines a communication protocol for automating interactions with a web browser that would traditionally require a human being (eg. clicking the mouse on a particular link; typing text into an input field; etc.) This protocol is being implemented by major browsers such as Firefox, Edge, and Chrome, and Servo also provides a WebDriver server. The goal of this project is both to submit a broad range of automated client tests using the Python WebDriver API that can be run against multiple browsers, and also to report on how many of the tests Servo currently passes.

== Scope ==
===Initial Steps===
* Compile Servo and ensure that it runs on <code>tests/html/about-mozilla.html</code>
* To run a simple webdriver script using the python client against Servo.
* Create a<code>mach</code> command (<code>test-webdriver</code>) in <code>python/servo/testing_commands.py</code> that will import each python file in the directory and execute the <code>test</code> method contained within it by invoking the Servo process before running each test, and killing it after each test finishes.
* Create a <code>webdriver</code> directory under <code>tests</code> where the forthcoming tests will reside.
* Writing an automated test for loading a particular URL, asserting that the resulting page is the expected url.
===Subsequent Steps===
* Writing tests that exercise the lower-level command API (eg. self.send_command("POST", "url", body)) to test server capabilities:
* Writing a test with a simple get to a url that exists on the server, ensure that we get the right response type
* Sending a non-string as the URL and ensuring that we get the right kind of error message.
* Trying other kinds of non-conforming messages e.g. ones with extra fields (should be ignored) or no url field (should return the correct type of error)
* Testing that we are actually loading the correct page. This will require the use of some other webdriver functions e.g. to get an element's text, or take a screenshot
* Testing that Get always navigates the top level browsing context (by navigating to a page with an iframe, setting the current bc to that iframe, and ensuring that it is not just the iframe that is navigated).
* Trying Get with pages that return non-200 responses.
* Loading a page that loads slower than the timeout, and ensure we get the correct error message

== Design pattern ==
Design patterns are not applicable as our task involved writing automated tests in python. However, the Implementation section below provides details of the steps the way it was implemented.

== Implementation ==
===Initial Steps===
The following steps were followed to meet the project requirements as per this github page.

=== Step 1: ===
We created a new <code>mach</code> command (<code>test-webdriver</code>) in <code>python/servo/testing_commands.py</code> file. It imports each python file in the directory and executes the <code>test</code> method contained within it.

=== Step 2: ===

We created tests in test.py and test1.py for loading a particular URL, asserting that the resulting page is the expected url.

===Subsequent Steps===
===Step 1:===
We would create 3 different test.py files for each of the different tests that would reside in tests/webdriver folder.

===Step 2:===
Creating a ServoProcess that would be used by the test.py files to start a servo session.

===Step 3:===
Catching exceptions when the tests fail and displaying them in proper format based on the kind of different error.

== Testing ==
Following are the steps to run the automated tests for WebDriver:
git clone <nowiki>https://github.com/krunal3103/servo</nowiki>
Note: Follow the steps listed in the Readme.md file of this link to clone and build servo on your local machine.
cd servo

./mach test-webdriver
You will see that all tests pass as expected.

== Pull Request ==
Here is our [https://github.com/servo/servo/pull/10113 pull request]. In the link you can see all code snippets and commits as well as integration test progression information.

CSC/ECE 517 Spring 2016/Write automated tests for WebDriver

2016-04-28T00:54:01Z

Skunapa: /* Background information: */

{{student editor|course = [[Wikipedia:Wiki_Ed/North_Carolina_State_University/Object-Oriented_Design_and_Development_(Spring_2016)]] }}

== Write automated tests for the WebDriver server ==

ServoServo is an open source prototype web browser layout engine being developed by Mozilla, and it is written in Rust language. It is currently developed on 64bit OS X, 64bit Linux, Android, and Gonk (Firefox OS). The main idea is to create a highly parallel environment, in which different components can be handled by fine grained, isolated tasks. The different components can be rendering, HTML parsing, etc.

== Background information: ==

=== Python: ===
Python is a widely used high-level, general-purpose, interpreted, dynamic programming language. Python supports multiple programming paradigms, including object oriented, imperative and functional programming or procedural styles. It features a dynamic type system and automatic memory management and has a large and comprehensive standard library. Modules for creating graphical user interfaces, connecting to relational databases, pseudorandom number generators, arithmetic with arbitrary precision decimals,[77] manipulating regular expressions, and doing unit testing are also included.

=== Servo: ===
[[File:ServoArchitecture.PNG]]

=== WebDriver: ===
The WebDriver specification defines a communication protocol for automating interactions with a web browser that would traditionally require a human being (eg. clicking the mouse on a particular link; typing text into an input field; etc.) This protocol is being implemented by major browsers such as Firefox, Edge, and Chrome, and Servo also provides a WebDriver server. The goal of this project is both to submit a broad range of automated client tests using the Python WebDriver API that can be run against multiple browsers, and also to report on how many of the tests Servo currently passes.

== Scope ==
===Initial Steps===
* Compile Servo and ensure that it runs on <code>tests/html/about-mozilla.html</code>
* To run a simple webdriver script using the python client against Servo.
* Create a<code>mach</code> command (<code>test-webdriver</code>) in <code>python/servo/testing_commands.py</code> that will import each python file in the directory and execute the <code>test</code> method contained within it by invoking the Servo process before running each test, and killing it after each test finishes.
* Create a <code>webdriver</code> directory under <code>tests</code> where the forthcoming tests will reside.
* Writing an automated test for loading a particular URL, asserting that the resulting page is the expected url.
===Subsequent Steps===
* Writing tests that exercise the lower-level command API (eg. self.send_command("POST", "url", body)) to test server capabilities:
* Writing a test with a simple get to a url that exists on the server, ensure that we get the right response type
* Sending a non-string as the URL and ensuring that we get the right kind of error message.
* Trying other kinds of non-conforming messages e.g. ones with extra fields (should be ignored) or no url field (should return the correct type of error)
* Testing that we are actually loading the correct page. This will require the use of some other webdriver functions e.g. to get an element's text, or take a screenshot
* Testing that Get always navigates the top level browsing context (by navigating to a page with an iframe, setting the current bc to that iframe, and ensuring that it is not just the iframe that is navigated).
* Trying Get with pages that return non-200 responses.
* Loading a page that loads slower than the timeout, and ensure we get the correct error message

== Design pattern ==
Design patterns are not applicable as our task involved writing automated tests in python. However, the Implementation section below provides details of the steps the way it was implemented.

== Implementation ==
===Initial Steps===
The following steps were followed to meet the project requirements as per this github page.

=== Step 1: ===
We created a new <code>mach</code> command (<code>test-webdriver</code>) in <code>python/servo/testing_commands.py</code> file. It imports each python file in the directory and executes the <code>test</code> method contained within it.

=== Step 2: ===

We created tests in test.py and test1.py for loading a particular URL, asserting that the resulting page is the expected url.

===Subsequent Steps===
===Step 1:===
We would create 3 different test.py files for each of the different tests that would reside in tests/webdriver folder.

===Step 2:===
Creating a ServoProcess that would be used by the test.py files to start a servo session.

===Step 3:===
Catching exceptions when the tests fail and displaying them in proper format based on the kind of different error.

== Testing ==
Following are the steps to run the automated tests for WebDriver:
git clone <nowiki>https://github.com/krunal3103/servo</nowiki>
Note: Follow the steps listed in the Readme.md file of this link to clone and build servo on your local machine.
cd servo

./mach test-webdriver
You will see that all tests pass as expected.

== Pull Request ==
Here is our [https://github.com/servo/servo/pull/10113 pull request]. In the link you can see all code snippets and commits as well as integration test progression information.

File:ServoArchitecture.JPG

2016-04-28T00:50:45Z

Skunapa:

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-04-01T22:10:57Z

Skunapa:

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

=='''Expertiza '''==

The Expertiza is a software project to create reusable learning objects through peer review. It is a Ruby on Rails based application which can be run on Windows, Linux and Mac OS X.

Main Features of Expertiza are:
* Allows students to work in groups to improve others learning experiences
* The work done by students/team is subjeted to multiple reviews , minimizing plagiarism
* Reviewing is done by studnets, which allows instructor/TA to spend less time in grading
* Large classes can also be handled easily with the help of Expertiza

=='''About Review mapping controller'''==

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the team for a submission , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Problem Statement'''===

The main aim of this project is to

* Refactor reporting method(response_report) which is big
* Use more efficient function sample instead of shuffle for random number selection
* Remove unused and assigned variables
* Simplify Automatic_review_mapping_strategy

=='''Code Improvements'''==

==='''Unused variables and arguments'''===

There are unused variables in the methods which use the stack unnecessarily. So, it is better to remove the unused variables or at the least indicate that a variable is unused.

For suppose when both keys and values are not used in a hash but are given as arguments, then the unused variables can be indicated by adding a "_" infront of the name or replace the unused variable with "_" to represent it as unused variable but allow them in arguments.

<pre>
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
</pre>
<pre>
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

In the above case teams_hash should consist of a hash with both keys and values but the sorting is done based on values. So the key is replaced with a "_" so that the user may deem it unused in the implementation of the process.

==='''Use sample instead of shuffle'''===

When sample is used, the elements in an array are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle. Also by using shuffle[0] we are shuffling all the elements in the array and then picking the first element instead of picking a single element randomly which is more efficient. The following are the couple of places where shuffle[0] was used and is replaced by sample.

<pre>
Previous Code:

assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

==='''Cyclomatic complexity of automatic_review_mapping_strategy method'''===

The method automatic_review_mapping_strategy handles the number of reviews assigned to a individual and also the number of reviews that can be done with in a team. The method is very long and has many nested if statements due to which the complexity of the method is very high. Instead of a single method handling all the parts of the strategy, it is divided into several parts due to which the code is more readable and also the complexity of code is shared by each method.

The method first checks the number of participants that are in an assignment and the number of teams that are present. It then sets the values for the maximum number of reviews that are possible and also the minimum number that are required. Then it assigns the reviews to each of the teams randomly when a request for review is made by a participant. At last it checks if the participant has the minimum number of reviews required after allotting the reviews. If not, it assigns more reviews to valid teams so that the minimum requirement is met.

The part of the code that is moved out of automatic_review_mapping_strategy as peer_review_strategy:

<pre>
private
def peer_review_strategy(teams, num_participants, student_review_num, participants, participants_hash)
iterator = 0
teams.each do |team|
selected_participants = Array.new
if !team.equal? teams.last
#need to even out the # of reviews for teams
while selected_participants.size < num_reviews_per_team
num_participants_this_team = TeamsUser.where(team_id: team.id).size
#If there are some submitters or reviewers in this team, they are not treated as normal participants.
#They should be removed from 'num_participants_this_team'
TeamsUser.where(team_id: team.id).each do |team_user|
temp_participant = Participant.where(user_id: team_user.user_id, parent_id: assignment_id).first
num_participants_this_team -= 1 if temp_participant.can_review == false or temp_participant.can_submit == false
end
#if all outstanding participants are already in selected_participants, just break the loop.
break if selected_participants.size == participants.size - num_participants_this_team

# generate random number
if iterator == 0
rand_num = rand(0..num_participants-1)
else
min_value = participants_hash.values.min
#get the temp array including indices of participants, each participant has minimum review number in hash table.
participants_with_min_assigned_reviews = Array.new
participants.each do |participant|
participants_with_min_assigned_reviews << participants.index(participant) if participants_hash[participant.id] == min_value
end
#if participants_with_min_assigned_reviews is blank
if_condition_1 = participants_with_min_assigned_reviews.empty?
#or only one element in participants_with_min_assigned_reviews, prohibit one student to review his/her own artifact
if_condition_2 = (participants_with_min_assigned_reviews.size == 1 and TeamsUser.exists?(team_id: team.id, user_id: participants[participants_with_min_assigned_reviews[0]].user_id))
if if_condition_1 or if_condition_2
#use original method to get random number
rand_num = rand(0..num_participants-1)
else
#rand_num should be the position of this participant in original array
rand_num = participants_with_min_assigned_reviews[rand(0..participants_with_min_assigned_reviews.size-1)]
end
end
# prohibit one student to review his/her own artifact
next if TeamsUser.exists?(team_id: team.id, user_id: participants[rand_num].user_id)

if_condition_1 = (participants_hash[participants[rand_num].id] < student_review_num)
if_condition_2 = (!selected_participants.include? participants[rand_num].id)
if if_condition_1 and if_condition_2
# selected_participants cannot include duplicate num
selected_participants << participants[rand_num].id
participants_hash[participants[rand_num].id] += 1
end
# remove students who have already been assigned enough num of reviews out of participants array
participants.each do |participant|
if participants_hash[participant.id] == student_review_num
participants.delete_at(rand_num)
num_participants -= 1
end
end
end
else
#review num for last team can be different from other teams.
#prohibit one student to review his/her own artifact and selected_participants cannot include duplicate num
participants.each do |participant|
# avoid last team receives too many peer reviews
if !TeamsUser.exists?(team_id: team.id, user_id: participant.user_id) and selected_participants.size < num_reviews_per_team
selected_participants << participant.id
participants_hash[participant.id] += 1
end
end
end

begin
selected_participants.each {|index| ReviewResponseMap.where(:reviewee_id => team.id, :reviewer_id => index, :reviewed_object_id => assignment_id).first_or_create}
rescue
flash[:error] = "Automatic assignment of reviewer failed."
end
iterator += 1
end
end
</pre>

The method is made private so that it can only be called with in the controller and cannot directly be called through a view.

The complexity of the original method reduced after breaking it and can be easily readable now.

<pre>
def automatic_review_mapping_strategy(assignment_id, participants, teams, student_review_num=0, submission_review_num=0)
participants_hash = {}
participants.each { |participant| participants_hash[participant.id] = 0 }
#calculate reviewers for each team
num_participants = participants.size
if student_review_num != 0 and submission_review_num == 0
num_reviews_per_team = (participants.size * student_review_num * 1.0 / teams.size).round
exact_num_of_review_needed = participants.size * student_review_num
elsif student_review_num == 0 and submission_review_num != 0
num_reviews_per_team = submission_review_num
student_review_num = (teams.size * submission_review_num * 1.0 / participants.size).round
exact_num_of_review_needed = teams.size * submission_review_num
end
#Exception detection: If instructor want to assign too many reviews done by each student, there will be an error msg.
if student_review_num >= teams.size
flash[:error] = 'You cannot set the number of reviews done by each student to be greater than or equal to total number of teams [or "participants" if it is an individual assignment].'
end

peer_review_strategy(teams, num_participants, student_review_num, participants, participants_hash)

# after assigning peer reviews for each team, if there are still some peer reviewers not obtain enough peer review, just assign them to valid teams
if ReviewResponseMap.where(["reviewed_object_id = ? and created_at > ? and calibrate_to = ?", assignment_id, @@time_create_last_review_mapping_record, 0]).size < exact_num_of_review_needed
participants_with_insufficient_review_num = Array.new
participants_hash.each do |participant_id, review_num|
participants_with_insufficient_review_num << participant_id if review_num < student_review_num
end
unsorted_teams_hash = {}
ReviewResponseMap.where(["reviewed_object_id = ? and calibrate_to = ?", assignment_id, 0]).each do |response_map|
unless unsorted_teams_hash.has_key? (response_map.reviewee_id)
unsorted_teams_hash[response_map.reviewee_id] = 1
else
unsorted_teams_hash[response_map.reviewee_id] += 1
end
end
teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
participants_with_insufficient_review_num.each do |participant_id|
teams_hash.each do |team_id, num_review_received|
unless TeamsUser.exists?(team_id: team_id, user_id: Participant.find(participant_id).user_id)
ReviewResponseMap.where(:reviewee_id => team_id, :reviewer_id => participant_id, :reviewed_object_id => assignment_id).first_or_create
teams_hash[team_id] += 1
teams_hash = teams_hash.sort_by{|_, v| v}.to_h
break
end
end
end
end
@@time_create_last_review_mapping_record = ReviewResponseMap.where(reviewed_object_id: assignment_id).last.created_at
end
</pre>

'''4. Moving code specific to models to models instead of controller'''

The method response_report contains code which generates reports based on the input of one of the three of 'ReviewResponseMap', 'FeedbackResponseMap', 'TeammateReviewResponseMap' and 'Calibration'. Calibration is a special case and should be handled in the controller itself. But the other three are models that are subclasses of ResponseMap model. The report is generated for each of the three by calling the ResponseMap model and obtaining the values by querying the database. Moving the code to the models and calling the methods in the model through the controller makes more sense than writing the code in controller.

The following is the code snippet from the original method which contains the calls to ResponseMap model:

<pre>
case @type
when "ReviewResponseMap"
if params[:user].nil?
# This is not a search, so find all reviewers for this assignment
response_maps_with_distinct_participant_id = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ? and calibrate_to = ?", @id, @type, 0])
@reviewers = []
response_maps_with_distinct_participant_id.each do |reviewer_id_from_response_map|
@reviewers << (AssignmentParticipant.find(reviewer_id_from_response_map.reviewer_id))
end
@reviewers = Participant.sort_by_name(@reviewers)
else
# This is a search, so find reviewers by user's full name
user = User.select("DISTINCT id").where(["fullname LIKE ?", '%'+params[:user][:fullname]+'%'])
@reviewers = AssignmentParticipant.where(["user_id IN (?) and parent_id = ?", user, @assignment.id])
end
# @review_scores[reveiwer_id][reviewee_id] = score for assignments not using vary_rubric_by_rounds feature
# @review_scores[reviewer_id][round][reviewee_id] = score for assignments using vary_rubric_by_rounds feature
@review_scores = @assignment.compute_reviews_hash
@avg_and_ranges= @assignment.compute_avg_and_ranges_hash
when "FeedbackResponseMap"
#Example query
#SELECT distinct reviewer_id FROM response_maps where type = 'FeedbackResponseMap' and
#reviewed_object_id in (select id from responses where
#map_id in (select id from response_maps where reviewed_object_id = 722 and type = 'ReviewResponseMap'))
@review_response_map_ids = ResponseMap.select("id").where(["reviewed_object_id = ? and type = ?", @id, 'ReviewResponseMap'])
@response_ids = Response.select("id").where(["map_id IN (?)", @review_response_map_ids])
@reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id IN (?) and type = ?", @response_ids, @type])
when "TeammateReviewResponseMap"
#Example query
#SELECT distinct reviewer_id FROM response_maps where type = 'TeammateReviewResponseMap' and reviewed_object_id = 711
@reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ?", @id, 'TeammateReviewResponseMap'])
end
end
</pre>

The same code after moving the methods to their respective models looks as follows:

<pre>
case @type
when "ReviewResponseMap"
@review_user= params[:user]
#If review response is required call review_response_report method in review_response_map model
@reviewers = ReviewResponseMap.review_response_report(@id, @assignment,@type, @review_user)
@review_scores = @assignment.compute_reviews_hash
@avg_and_ranges= @assignment.compute_avg_and_ranges_hash
when "FeedbackResponseMap"
#If review report for feedback is required call feedback_response_report method in feedback_review_response_map model
@reviewers = FeedbackResponseMap.feedback_response_report(@id, @type)
when "TeammateReviewResponseMap"
#If review report for teammate is required call teammate_response_report method in teammate_review_response_map model
@reviewers = TeammateReviewResponseMap.teammate_response_report(@id)
end
end
</pre>
=='''Testing UI'''==

'''Peer review information:'''
* Instructor Login:
Username: Instructor6
Password: password
* Student login:
Username: Student6384
Password: password

'''Steps for testing UI:'''
# Login as an instructor (Using Instructor6 will help you import the participants from other assignments).
# Navigate to "Manage->Assignments".
# Click on "New Public Assignment" for creating a new assignment.
# Create a new assignment by providing assignment name, selecting a course, submission directory (Give any name) and description URL.
# Select "has teams?" and provide the team size. Click on create to create a new assignment.
## After that, click on review strategy and limit the number of reviews per submission.
## Click on "Due dates" and update date for submission and review. Adjust the review date and time in order to test the reviews.
## Click on "save". A new assignment is created and it can be viewed in "Manage->Assignments" section.
# In order to add participants, there are two methods to add students to the assignment.
## Click on "add participants" against the assignment. Enter the user login to add the student. Add atleast 6 participants so that the review mapping can be seen.
## Click on any previous assignment "add participants" and export the students list (I used wiki assignment).
## Click on your assignment "add participants" and import the students using the export file.
# Go back to "Assignments" section and click against "create teams".
## After clicking on "create teams", Click on "create teams" in the directed page.
### Teams can be formed either manually or automatically.
# Login as a student to submit the links for assignment (Valid hyperlink must be provided in the submission link). Add submission for atleast 5 students in order to check the automatic review mapping. (Password for student is password)
# After submitting the links for some students, Log in as an instructor to change the assignment phase to review phase.
## To change the review phase period, Go to assignments section and click on edit. Click on due dates and change the review due date.
# Now, login as a student and go to others work. Click on "Request a submission to review" and check whether a review is assigned automatically. If no assignments are submitted, then this cannot be tested.
# For teammate review report, author feedback report and review report, click against "review report" and all the review reports can be seen by selecting it in the drop down menu.

==References==

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/harsha1007/expertiza Forked repository for the project]
#[http://expertiza.ncsu.edu/ Expertiza Main Page]

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-04-01T22:10:05Z

Skunapa:

E1615. Refactoring the Review Mapping Controller

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

=='''Expertiza '''==

The Expertiza is a software project to create reusable learning objects through peer review. It is a Ruby on Rails based application which can be run on Windows, Linux and Mac OS X.

Main Features of Expertiza are:
* Allows students to work in groups to improve others learning experiences
* The work done by students/team is subjeted to multiple reviews , minimizing plagiarism
* Reviewing is done by studnets, which allows instructor/TA to spend less time in grading
* Large classes can also be handled easily with the help of Expertiza

=='''About Review mapping controller'''==

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the team for a submission , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Problem Statement'''===

The main aim of this project is to

* Refactor reporting method(response_report) which is big
* Use more efficient function sample instead of shuffle for random number selection
* Remove unused and assigned variables
* Simplify Automatic_review_mapping_strategy

=='''Code Improvements'''==

==='''Unused variables and arguments'''===

There are unused variables in the methods which use the stack unnecessarily. So, it is better to remove the unused variables or at the least indicate that a variable is unused.

For suppose when both keys and values are not used in a hash but are given as arguments, then the unused variables can be indicated by adding a "_" infront of the name or replace the unused variable with "_" to represent it as unused variable but allow them in arguments.

<pre>
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
</pre>
<pre>
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

In the above case teams_hash should consist of a hash with both keys and values but the sorting is done based on values. So the key is replaced with a "_" so that the user may deem it unused in the implementation of the process.

==='''Use sample instead of shuffle'''===

When sample is used, the elements in an array are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle. Also by using shuffle[0] we are shuffling all the elements in the array and then picking the first element instead of picking a single element randomly which is more efficient. The following are the couple of places where shuffle[0] was used and is replaced by sample.

<pre>
Previous Code:

assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

==='''Cyclomatic complexity of automatic_review_mapping_strategy method'''===

The method automatic_review_mapping_strategy handles the number of reviews assigned to a individual and also the number of reviews that can be done with in a team. The method is very long and has many nested if statements due to which the complexity of the method is very high. Instead of a single method handling all the parts of the strategy, it is divided into several parts due to which the code is more readable and also the complexity of code is shared by each method.

The method first checks the number of participants that are in an assignment and the number of teams that are present. It then sets the values for the maximum number of reviews that are possible and also the minimum number that are required. Then it assigns the reviews to each of the teams randomly when a request for review is made by a participant. At last it checks if the participant has the minimum number of reviews required after allotting the reviews. If not, it assigns more reviews to valid teams so that the minimum requirement is met.

The part of the code that is moved out of automatic_review_mapping_strategy as peer_review_strategy:

<pre>
private
def peer_review_strategy(teams, num_participants, student_review_num, participants, participants_hash)
iterator = 0
teams.each do |team|
selected_participants = Array.new
if !team.equal? teams.last
#need to even out the # of reviews for teams
while selected_participants.size < num_reviews_per_team
num_participants_this_team = TeamsUser.where(team_id: team.id).size
#If there are some submitters or reviewers in this team, they are not treated as normal participants.
#They should be removed from 'num_participants_this_team'
TeamsUser.where(team_id: team.id).each do |team_user|
temp_participant = Participant.where(user_id: team_user.user_id, parent_id: assignment_id).first
num_participants_this_team -= 1 if temp_participant.can_review == false or temp_participant.can_submit == false
end
#if all outstanding participants are already in selected_participants, just break the loop.
break if selected_participants.size == participants.size - num_participants_this_team

# generate random number
if iterator == 0
rand_num = rand(0..num_participants-1)
else
min_value = participants_hash.values.min
#get the temp array including indices of participants, each participant has minimum review number in hash table.
participants_with_min_assigned_reviews = Array.new
participants.each do |participant|
participants_with_min_assigned_reviews << participants.index(participant) if participants_hash[participant.id] == min_value
end
#if participants_with_min_assigned_reviews is blank
if_condition_1 = participants_with_min_assigned_reviews.empty?
#or only one element in participants_with_min_assigned_reviews, prohibit one student to review his/her own artifact
if_condition_2 = (participants_with_min_assigned_reviews.size == 1 and TeamsUser.exists?(team_id: team.id, user_id: participants[participants_with_min_assigned_reviews[0]].user_id))
if if_condition_1 or if_condition_2
#use original method to get random number
rand_num = rand(0..num_participants-1)
else
#rand_num should be the position of this participant in original array
rand_num = participants_with_min_assigned_reviews[rand(0..participants_with_min_assigned_reviews.size-1)]
end
end
# prohibit one student to review his/her own artifact
next if TeamsUser.exists?(team_id: team.id, user_id: participants[rand_num].user_id)

if_condition_1 = (participants_hash[participants[rand_num].id] < student_review_num)
if_condition_2 = (!selected_participants.include? participants[rand_num].id)
if if_condition_1 and if_condition_2
# selected_participants cannot include duplicate num
selected_participants << participants[rand_num].id
participants_hash[participants[rand_num].id] += 1
end
# remove students who have already been assigned enough num of reviews out of participants array
participants.each do |participant|
if participants_hash[participant.id] == student_review_num
participants.delete_at(rand_num)
num_participants -= 1
end
end
end
else
#review num for last team can be different from other teams.
#prohibit one student to review his/her own artifact and selected_participants cannot include duplicate num
participants.each do |participant|
# avoid last team receives too many peer reviews
if !TeamsUser.exists?(team_id: team.id, user_id: participant.user_id) and selected_participants.size < num_reviews_per_team
selected_participants << participant.id
participants_hash[participant.id] += 1
end
end
end

begin
selected_participants.each {|index| ReviewResponseMap.where(:reviewee_id => team.id, :reviewer_id => index, :reviewed_object_id => assignment_id).first_or_create}
rescue
flash[:error] = "Automatic assignment of reviewer failed."
end
iterator += 1
end
end
</pre>

The method is made private so that it can only be called with in the controller and cannot directly be called through a view.

The complexity of the original method reduced after breaking it and can be easily readable now.

<pre>
def automatic_review_mapping_strategy(assignment_id, participants, teams, student_review_num=0, submission_review_num=0)
participants_hash = {}
participants.each { |participant| participants_hash[participant.id] = 0 }
#calculate reviewers for each team
num_participants = participants.size
if student_review_num != 0 and submission_review_num == 0
num_reviews_per_team = (participants.size * student_review_num * 1.0 / teams.size).round
exact_num_of_review_needed = participants.size * student_review_num
elsif student_review_num == 0 and submission_review_num != 0
num_reviews_per_team = submission_review_num
student_review_num = (teams.size * submission_review_num * 1.0 / participants.size).round
exact_num_of_review_needed = teams.size * submission_review_num
end
#Exception detection: If instructor want to assign too many reviews done by each student, there will be an error msg.
if student_review_num >= teams.size
flash[:error] = 'You cannot set the number of reviews done by each student to be greater than or equal to total number of teams [or "participants" if it is an individual assignment].'
end

peer_review_strategy(teams, num_participants, student_review_num, participants, participants_hash)

# after assigning peer reviews for each team, if there are still some peer reviewers not obtain enough peer review, just assign them to valid teams
if ReviewResponseMap.where(["reviewed_object_id = ? and created_at > ? and calibrate_to = ?", assignment_id, @@time_create_last_review_mapping_record, 0]).size < exact_num_of_review_needed
participants_with_insufficient_review_num = Array.new
participants_hash.each do |participant_id, review_num|
participants_with_insufficient_review_num << participant_id if review_num < student_review_num
end
unsorted_teams_hash = {}
ReviewResponseMap.where(["reviewed_object_id = ? and calibrate_to = ?", assignment_id, 0]).each do |response_map|
unless unsorted_teams_hash.has_key? (response_map.reviewee_id)
unsorted_teams_hash[response_map.reviewee_id] = 1
else
unsorted_teams_hash[response_map.reviewee_id] += 1
end
end
teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
participants_with_insufficient_review_num.each do |participant_id|
teams_hash.each do |team_id, num_review_received|
unless TeamsUser.exists?(team_id: team_id, user_id: Participant.find(participant_id).user_id)
ReviewResponseMap.where(:reviewee_id => team_id, :reviewer_id => participant_id, :reviewed_object_id => assignment_id).first_or_create
teams_hash[team_id] += 1
teams_hash = teams_hash.sort_by{|_, v| v}.to_h
break
end
end
end
end
@@time_create_last_review_mapping_record = ReviewResponseMap.where(reviewed_object_id: assignment_id).last.created_at
end
</pre>

'''4. Moving code specific to models to models instead of controller'''

The method response_report contains code which generates reports based on the input of one of the three of 'ReviewResponseMap', 'FeedbackResponseMap', 'TeammateReviewResponseMap' and 'Calibration'. Calibration is a special case and should be handled in the controller itself. But the other three are models that are subclasses of ResponseMap model. The report is generated for each of the three by calling the ResponseMap model and obtaining the values by querying the database. Moving the code to the models and calling the methods in the model through the controller makes more sense than writing the code in controller.

The following is the code snippet from the original method which contains the calls to ResponseMap model:

<pre>
case @type
when "ReviewResponseMap"
if params[:user].nil?
# This is not a search, so find all reviewers for this assignment
response_maps_with_distinct_participant_id = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ? and calibrate_to = ?", @id, @type, 0])
@reviewers = []
response_maps_with_distinct_participant_id.each do |reviewer_id_from_response_map|
@reviewers << (AssignmentParticipant.find(reviewer_id_from_response_map.reviewer_id))
end
@reviewers = Participant.sort_by_name(@reviewers)
else
# This is a search, so find reviewers by user's full name
user = User.select("DISTINCT id").where(["fullname LIKE ?", '%'+params[:user][:fullname]+'%'])
@reviewers = AssignmentParticipant.where(["user_id IN (?) and parent_id = ?", user, @assignment.id])
end
# @review_scores[reveiwer_id][reviewee_id] = score for assignments not using vary_rubric_by_rounds feature
# @review_scores[reviewer_id][round][reviewee_id] = score for assignments using vary_rubric_by_rounds feature
@review_scores = @assignment.compute_reviews_hash
@avg_and_ranges= @assignment.compute_avg_and_ranges_hash
when "FeedbackResponseMap"
#Example query
#SELECT distinct reviewer_id FROM response_maps where type = 'FeedbackResponseMap' and
#reviewed_object_id in (select id from responses where
#map_id in (select id from response_maps where reviewed_object_id = 722 and type = 'ReviewResponseMap'))
@review_response_map_ids = ResponseMap.select("id").where(["reviewed_object_id = ? and type = ?", @id, 'ReviewResponseMap'])
@response_ids = Response.select("id").where(["map_id IN (?)", @review_response_map_ids])
@reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id IN (?) and type = ?", @response_ids, @type])
when "TeammateReviewResponseMap"
#Example query
#SELECT distinct reviewer_id FROM response_maps where type = 'TeammateReviewResponseMap' and reviewed_object_id = 711
@reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ?", @id, 'TeammateReviewResponseMap'])
end
end
</pre>

The same code after moving the methods to their respective models looks as follows:

<pre>
case @type
when "ReviewResponseMap"
@review_user= params[:user]
#If review response is required call review_response_report method in review_response_map model
@reviewers = ReviewResponseMap.review_response_report(@id, @assignment,@type, @review_user)
@review_scores = @assignment.compute_reviews_hash
@avg_and_ranges= @assignment.compute_avg_and_ranges_hash
when "FeedbackResponseMap"
#If review report for feedback is required call feedback_response_report method in feedback_review_response_map model
@reviewers = FeedbackResponseMap.feedback_response_report(@id, @type)
when "TeammateReviewResponseMap"
#If review report for teammate is required call teammate_response_report method in teammate_review_response_map model
@reviewers = TeammateReviewResponseMap.teammate_response_report(@id)
end
end
</pre>
=='''Testing UI'''==

'''Peer review information:'''
* Instructor Login:
Username: Instructor6
Password: password
* Student login:
Username: Student6384
Password: password

'''Steps for testing UI:'''
# Login as an instructor (Using Instructor6 will help you import the participants from other assignments).
# Navigate to "Manage->Assignments".
# Click on "New Public Assignment" for creating a new assignment.
# Create a new assignment by providing assignment name, selecting a course, submission directory (Give any name) and description URL.
# Select "has teams?" and provide the team size. Click on create to create a new assignment.
## After that, click on review strategy and limit the number of reviews per submission.
## Click on "Due dates" and update date for submission and review. Adjust the review date and time in order to test the reviews.
## Click on "save". A new assignment is created and it can be viewed in "Manage->Assignments" section.
# In order to add participants, there are two methods to add students to the assignment.
## Click on "add participants" against the assignment. Enter the user login to add the student. Add atleast 6 participants so that the review mapping can be seen.
## Click on any previous assignment "add participants" and export the students list (I used wiki assignment).
## Click on your assignment "add participants" and import the students using the export file.
# Go back to "Assignments" section and click against "create teams".
## After clicking on "create teams", Click on "create teams" in the directed page.
### Teams can be formed either manually or automatically.
# Login as a student to submit the links for assignment (Valid hyperlink must be provided in the submission link). Add submission for atleast 5 students in order to check the automatic review mapping. (Password for student is password)
# After submitting the links for some students, Log in as an instructor to change the assignment phase to review phase.
## To change the review phase period, Go to assignments section and click on edit. Click on due dates and change the review due date.
# Now, login as a student and go to others work. Click on "Request a submission to review" and check whether a review is assigned automatically. If no assignments are submitted, then this cannot be tested.
# For teammate review report, author feedback report and review report, click against "review report" and all the review reports can be seen by selecting it in the drop down menu.

==References==

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/harsha1007/expertiza Forked repository for the project]
#[http://expertiza.ncsu.edu/ Expertiza Main Page]

Talk:/CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-04-01T22:07:49Z

Skunapa: moved Talk:/CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb to Talk:CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

#REDIRECT [[Talk:CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb]]

Talk:CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-04-01T22:07:49Z

Skunapa: moved Talk:/CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb to Talk:CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

/CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-04-01T22:07:49Z

Skunapa: moved /CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb to CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

#REDIRECT [[CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb]]

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-04-01T22:07:49Z

Skunapa: moved /CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb to CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

==E1615. Refactoring the Review Mapping Controller==

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

__TOC__

=='''Expertiza '''==

The Expertiza is a software project to create reusable learning objects through peer review. It is a Ruby on Rails based application which can be run on Windows, Linux and Mac OS X.

Main Features of Expertiza are:
* Allows students to work in groups to improve others learning experiences
* The work done by students/team is subjeted to multiple reviews , minimizing plagiarism
* Reviewing is done by studnets, which allows instructor/TA to spend less time in grading
* Large classes can also be handled easily with the help of Expertiza

=='''About Review mapping controller'''==

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the team for a submission , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Problem Statement'''===

The main aim of this project is to

* Refactor reporting method(response_report) which is big
* Use more efficient function sample instead of shuffle for random number selection
* Remove unused and assigned variables
* Simplify Automatic_review_mapping_strategy

=='''Code Improvements'''==

==='''Unused variables and arguments'''===

There are unused variables in the methods which use the stack unnecessarily. So, it is better to remove the unused variables or at the least indicate that a variable is unused.

For suppose when both keys and values are not used in a hash but are given as arguments, then the unused variables can be indicated by adding a "_" infront of the name or replace the unused variable with "_" to represent it as unused variable but allow them in arguments.

<pre>
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
</pre>
<pre>
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

In the above case teams_hash should consist of a hash with both keys and values but the sorting is done based on values. So the key is replaced with a "_" so that the user may deem it unused in the implementation of the process.

==='''Use sample instead of shuffle'''===

When sample is used, the elements in an array are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle. Also by using shuffle[0] we are shuffling all the elements in the array and then picking the first element instead of picking a single element randomly which is more efficient. The following are the couple of places where shuffle[0] was used and is replaced by sample.

<pre>
Previous Code:

assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

==='''Cyclomatic complexity of automatic_review_mapping_strategy method'''===

The method automatic_review_mapping_strategy handles the number of reviews assigned to a individual and also the number of reviews that can be done with in a team. The method is very long and has many nested if statements due to which the complexity of the method is very high. Instead of a single method handling all the parts of the strategy, it is divided into several parts due to which the code is more readable and also the complexity of code is shared by each method.

The method first checks the number of participants that are in an assignment and the number of teams that are present. It then sets the values for the maximum number of reviews that are possible and also the minimum number that are required. Then it assigns the reviews to each of the teams randomly when a request for review is made by a participant. At last it checks if the participant has the minimum number of reviews required after allotting the reviews. If not, it assigns more reviews to valid teams so that the minimum requirement is met.

The part of the code that is moved out of automatic_review_mapping_strategy as peer_review_strategy:

<pre>
private
def peer_review_strategy(teams, num_participants, student_review_num, participants, participants_hash)
iterator = 0
teams.each do |team|
selected_participants = Array.new
if !team.equal? teams.last
#need to even out the # of reviews for teams
while selected_participants.size < num_reviews_per_team
num_participants_this_team = TeamsUser.where(team_id: team.id).size
#If there are some submitters or reviewers in this team, they are not treated as normal participants.
#They should be removed from 'num_participants_this_team'
TeamsUser.where(team_id: team.id).each do |team_user|
temp_participant = Participant.where(user_id: team_user.user_id, parent_id: assignment_id).first
num_participants_this_team -= 1 if temp_participant.can_review == false or temp_participant.can_submit == false
end
#if all outstanding participants are already in selected_participants, just break the loop.
break if selected_participants.size == participants.size - num_participants_this_team

# generate random number
if iterator == 0
rand_num = rand(0..num_participants-1)
else
min_value = participants_hash.values.min
#get the temp array including indices of participants, each participant has minimum review number in hash table.
participants_with_min_assigned_reviews = Array.new
participants.each do |participant|
participants_with_min_assigned_reviews << participants.index(participant) if participants_hash[participant.id] == min_value
end
#if participants_with_min_assigned_reviews is blank
if_condition_1 = participants_with_min_assigned_reviews.empty?
#or only one element in participants_with_min_assigned_reviews, prohibit one student to review his/her own artifact
if_condition_2 = (participants_with_min_assigned_reviews.size == 1 and TeamsUser.exists?(team_id: team.id, user_id: participants[participants_with_min_assigned_reviews[0]].user_id))
if if_condition_1 or if_condition_2
#use original method to get random number
rand_num = rand(0..num_participants-1)
else
#rand_num should be the position of this participant in original array
rand_num = participants_with_min_assigned_reviews[rand(0..participants_with_min_assigned_reviews.size-1)]
end
end
# prohibit one student to review his/her own artifact
next if TeamsUser.exists?(team_id: team.id, user_id: participants[rand_num].user_id)

if_condition_1 = (participants_hash[participants[rand_num].id] < student_review_num)
if_condition_2 = (!selected_participants.include? participants[rand_num].id)
if if_condition_1 and if_condition_2
# selected_participants cannot include duplicate num
selected_participants << participants[rand_num].id
participants_hash[participants[rand_num].id] += 1
end
# remove students who have already been assigned enough num of reviews out of participants array
participants.each do |participant|
if participants_hash[participant.id] == student_review_num
participants.delete_at(rand_num)
num_participants -= 1
end
end
end
else
#review num for last team can be different from other teams.
#prohibit one student to review his/her own artifact and selected_participants cannot include duplicate num
participants.each do |participant|
# avoid last team receives too many peer reviews
if !TeamsUser.exists?(team_id: team.id, user_id: participant.user_id) and selected_participants.size < num_reviews_per_team
selected_participants << participant.id
participants_hash[participant.id] += 1
end
end
end

begin
selected_participants.each {|index| ReviewResponseMap.where(:reviewee_id => team.id, :reviewer_id => index, :reviewed_object_id => assignment_id).first_or_create}
rescue
flash[:error] = "Automatic assignment of reviewer failed."
end
iterator += 1
end
end
</pre>

The method is made private so that it can only be called with in the controller and cannot directly be called through a view.

The complexity of the original method reduced after breaking it and can be easily readable now.

<pre>
def automatic_review_mapping_strategy(assignment_id, participants, teams, student_review_num=0, submission_review_num=0)
participants_hash = {}
participants.each { |participant| participants_hash[participant.id] = 0 }
#calculate reviewers for each team
num_participants = participants.size
if student_review_num != 0 and submission_review_num == 0
num_reviews_per_team = (participants.size * student_review_num * 1.0 / teams.size).round
exact_num_of_review_needed = participants.size * student_review_num
elsif student_review_num == 0 and submission_review_num != 0
num_reviews_per_team = submission_review_num
student_review_num = (teams.size * submission_review_num * 1.0 / participants.size).round
exact_num_of_review_needed = teams.size * submission_review_num
end
#Exception detection: If instructor want to assign too many reviews done by each student, there will be an error msg.
if student_review_num >= teams.size
flash[:error] = 'You cannot set the number of reviews done by each student to be greater than or equal to total number of teams [or "participants" if it is an individual assignment].'
end

peer_review_strategy(teams, num_participants, student_review_num, participants, participants_hash)

# after assigning peer reviews for each team, if there are still some peer reviewers not obtain enough peer review, just assign them to valid teams
if ReviewResponseMap.where(["reviewed_object_id = ? and created_at > ? and calibrate_to = ?", assignment_id, @@time_create_last_review_mapping_record, 0]).size < exact_num_of_review_needed
participants_with_insufficient_review_num = Array.new
participants_hash.each do |participant_id, review_num|
participants_with_insufficient_review_num << participant_id if review_num < student_review_num
end
unsorted_teams_hash = {}
ReviewResponseMap.where(["reviewed_object_id = ? and calibrate_to = ?", assignment_id, 0]).each do |response_map|
unless unsorted_teams_hash.has_key? (response_map.reviewee_id)
unsorted_teams_hash[response_map.reviewee_id] = 1
else
unsorted_teams_hash[response_map.reviewee_id] += 1
end
end
teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
participants_with_insufficient_review_num.each do |participant_id|
teams_hash.each do |team_id, num_review_received|
unless TeamsUser.exists?(team_id: team_id, user_id: Participant.find(participant_id).user_id)
ReviewResponseMap.where(:reviewee_id => team_id, :reviewer_id => participant_id, :reviewed_object_id => assignment_id).first_or_create
teams_hash[team_id] += 1
teams_hash = teams_hash.sort_by{|_, v| v}.to_h
break
end
end
end
end
@@time_create_last_review_mapping_record = ReviewResponseMap.where(reviewed_object_id: assignment_id).last.created_at
end
</pre>

'''4. Moving code specific to models to models instead of controller'''

The method response_report contains code which generates reports based on the input of one of the three of 'ReviewResponseMap', 'FeedbackResponseMap', 'TeammateReviewResponseMap' and 'Calibration'. Calibration is a special case and should be handled in the controller itself. But the other three are models that are subclasses of ResponseMap model. The report is generated for each of the three by calling the ResponseMap model and obtaining the values by querying the database. Moving the code to the models and calling the methods in the model through the controller makes more sense than writing the code in controller.

The following is the code snippet from the original method which contains the calls to ResponseMap model:

<pre>
case @type
when "ReviewResponseMap"
if params[:user].nil?
# This is not a search, so find all reviewers for this assignment
response_maps_with_distinct_participant_id = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ? and calibrate_to = ?", @id, @type, 0])
@reviewers = []
response_maps_with_distinct_participant_id.each do |reviewer_id_from_response_map|
@reviewers << (AssignmentParticipant.find(reviewer_id_from_response_map.reviewer_id))
end
@reviewers = Participant.sort_by_name(@reviewers)
else
# This is a search, so find reviewers by user's full name
user = User.select("DISTINCT id").where(["fullname LIKE ?", '%'+params[:user][:fullname]+'%'])
@reviewers = AssignmentParticipant.where(["user_id IN (?) and parent_id = ?", user, @assignment.id])
end
# @review_scores[reveiwer_id][reviewee_id] = score for assignments not using vary_rubric_by_rounds feature
# @review_scores[reviewer_id][round][reviewee_id] = score for assignments using vary_rubric_by_rounds feature
@review_scores = @assignment.compute_reviews_hash
@avg_and_ranges= @assignment.compute_avg_and_ranges_hash
when "FeedbackResponseMap"
#Example query
#SELECT distinct reviewer_id FROM response_maps where type = 'FeedbackResponseMap' and
#reviewed_object_id in (select id from responses where
#map_id in (select id from response_maps where reviewed_object_id = 722 and type = 'ReviewResponseMap'))
@review_response_map_ids = ResponseMap.select("id").where(["reviewed_object_id = ? and type = ?", @id, 'ReviewResponseMap'])
@response_ids = Response.select("id").where(["map_id IN (?)", @review_response_map_ids])
@reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id IN (?) and type = ?", @response_ids, @type])
when "TeammateReviewResponseMap"
#Example query
#SELECT distinct reviewer_id FROM response_maps where type = 'TeammateReviewResponseMap' and reviewed_object_id = 711
@reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ?", @id, 'TeammateReviewResponseMap'])
end
end
</pre>

The same code after moving the methods to their respective models looks as follows:

<pre>
case @type
when "ReviewResponseMap"
@review_user= params[:user]
#If review response is required call review_response_report method in review_response_map model
@reviewers = ReviewResponseMap.review_response_report(@id, @assignment,@type, @review_user)
@review_scores = @assignment.compute_reviews_hash
@avg_and_ranges= @assignment.compute_avg_and_ranges_hash
when "FeedbackResponseMap"
#If review report for feedback is required call feedback_response_report method in feedback_review_response_map model
@reviewers = FeedbackResponseMap.feedback_response_report(@id, @type)
when "TeammateReviewResponseMap"
#If review report for teammate is required call teammate_response_report method in teammate_review_response_map model
@reviewers = TeammateReviewResponseMap.teammate_response_report(@id)
end
end
</pre>
=='''Testing UI'''==

'''Peer review information:'''
* Instructor Login:
Username: Instructor6
Password: password
* Student login:
Username: Student6384
Password: password

'''Steps for testing UI:'''
# Login as an instructor (Using Instructor6 will help you import the participants from other assignments).
# Navigate to "Manage->Assignments".
# Click on "New Public Assignment" for creating a new assignment.
# Create a new assignment by providing assignment name, selecting a course, submission directory (Give any name) and description URL.
# Select "has teams?" and provide the team size. Click on create to create a new assignment.
## After that, click on review strategy and limit the number of reviews per submission.
## Click on "Due dates" and update date for submission and review. Adjust the review date and time in order to test the reviews.
## Click on "save". A new assignment is created and it can be viewed in "Manage->Assignments" section.
# In order to add participants, there are two methods to add students to the assignment.
## Click on "add participants" against the assignment. Enter the user login to add the student. Add atleast 6 participants so that the review mapping can be seen.
## Click on any previous assignment "add participants" and export the students list (I used wiki assignment).
## Click on your assignment "add participants" and import the students using the export file.
# Go back to "Assignments" section and click against "create teams".
## After clicking on "create teams", Click on "create teams" in the directed page.
### Teams can be formed either manually or automatically.
# Login as a student to submit the links for assignment (Valid hyperlink must be provided in the submission link). Add submission for atleast 5 students in order to check the automatic review mapping. (Password for student is password)
# After submitting the links for some students, Log in as an instructor to change the assignment phase to review phase.
## To change the review phase period, Go to assignments section and click on edit. Click on due dates and change the review due date.
# Now, login as a student and go to others work. Click on "Request a submission to review" and check whether a review is assigned automatically. If no assignments are submitted, then this cannot be tested.
# For teammate review report, author feedback report and review report, click against "review report" and all the review reports can be seen by selecting it in the drop down menu.

==References==

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/harsha1007/expertiza Forked repository for the project]
#[http://expertiza.ncsu.edu/ Expertiza Main Page]

Talk:Http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-04-01T22:07:35Z

Skunapa: moved Talk:Http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb to Talk:/CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

#REDIRECT [[Talk:/CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb]]

Talk:CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-04-01T22:07:35Z

Skunapa: moved Talk:Http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb to Talk:/CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

Http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-04-01T22:07:34Z

Skunapa: moved Http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb to /CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

#REDIRECT [[/CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb]]

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-04-01T22:07:34Z

Skunapa: moved Http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb to /CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

User talk:Skunapa

2016-04-01T22:07:08Z

Skunapa: moved User talk:Skunapa to Talk:Http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

#REDIRECT [[Talk:Http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb]]

Talk:CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-04-01T22:07:08Z

Skunapa: moved User talk:Skunapa to Talk:Http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

User:Skunapa

2016-04-01T22:07:08Z

Skunapa: moved User:Skunapa to Http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

#REDIRECT [[Http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb]]

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-04-01T22:07:08Z

Skunapa: moved User:Skunapa to Http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-04-01T17:16:35Z

Skunapa: /* Code Improvements */

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-03-30T20:29:15Z

Skunapa:

==E1615. Refactoring the Review Mapping Controller==

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

__TOC__

=='''Expertiza '''==

The Expertiza is a software project to create reusable learning objects through peer review. It is a Ruby on Rails based application which can be run on Windows, Linux and Mac OS X.

Main Features of Expertiza are:
* Allows students to work in groups to improve others learning experiences
* The work done by students/team is subjeted to multiple reviews , minimizing plagiarism
* Reviewing is done by studnets, which allows instructor/TA to spend less time in grading
* Large classes can also be handled easily with the help of Expertiza

=='''About Review mapping controller'''==

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the team for a submission , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Problem Statement'''===

The main aim of this project is to

* Refactor reporting method(response_report) which is big
* Use more efficient function sample instead of shuffle for random number selection
* Remove unused and assigned variables
* Simplify Automatic_review_mapping_strategy

=='''Code Improvements'''==

==='''Unused variables and arguments'''===

There are unused variables in the methods which use the stack unnecessarily. So, it is better to remove the unused variables or at the least indicate that a variable is unused.

For suppose when both keys and values are not used in a hash but are given as arguments, then the unused variables can be indicated by adding a "_" infront of the name or replace the unused variable with "_" to represent it as unused variable but allow them in arguments.

<pre>
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
</pre>
<pre>
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

In the above case teams_hash should consist of a hash with both keys and values but the sorting is done based on values. So the key is replaced with a "_" so that the user may deem it unused in the implementation of the process.

==='''Use sample instead of shuffle'''===

When sample is used, the elements in an array are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle. Also by using shuffle[0] we are shuffling all the elements in the array and then picking the first element instead of picking a single element randomly which is more efficient. The following are the couple of places where shuffle[0] was used and is replaced by sample.

<pre>
Previous Code:

assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

==='''Cyclomatic complexity of automatic_review_mapping_strategy method'''===

The method automatic_review_mapping_strategy handles the number of reviews assigned to a individual and also the number of reviews that can be done with in a team. The method is very long and has many nested if statements due to which the complexity of the method is very high. Instead of a single method handling all the parts of the strategy, it is divided into several parts due to which the code is more readable and also the complexity of code is shared by each method.

The method first checks the number of participants that are in an assignment and the number of teams that are present. It then sets the values for the maximum number of reviews that are possible and also the minimum number that are required. Then it assigns the reviews to each of the teams randomly when a request for review is made by a participant. At last it checks if the participant has the minimum number of reviews required after allotting the reviews. If not, it assigns more reviews to valid teams so that the minimum requirement is met.

The part of the code that is moved out of automatic_review_mapping_strategy as peer_review_strategy:

<pre>
private
def peer_review_strategy(teams, num_participants, student_review_num, participants, participants_hash)
iterator = 0
teams.each do |team|
selected_participants = Array.new
if !team.equal? teams.last
#need to even out the # of reviews for teams
while selected_participants.size < num_reviews_per_team
num_participants_this_team = TeamsUser.where(team_id: team.id).size
#If there are some submitters or reviewers in this team, they are not treated as normal participants.
#They should be removed from 'num_participants_this_team'
TeamsUser.where(team_id: team.id).each do |team_user|
temp_participant = Participant.where(user_id: team_user.user_id, parent_id: assignment_id).first
num_participants_this_team -= 1 if temp_participant.can_review == false or temp_participant.can_submit == false
end
#if all outstanding participants are already in selected_participants, just break the loop.
break if selected_participants.size == participants.size - num_participants_this_team

# generate random number
if iterator == 0
rand_num = rand(0..num_participants-1)
else
min_value = participants_hash.values.min
#get the temp array including indices of participants, each participant has minimum review number in hash table.
participants_with_min_assigned_reviews = Array.new
participants.each do |participant|
participants_with_min_assigned_reviews << participants.index(participant) if participants_hash[participant.id] == min_value
end
#if participants_with_min_assigned_reviews is blank
if_condition_1 = participants_with_min_assigned_reviews.empty?
#or only one element in participants_with_min_assigned_reviews, prohibit one student to review his/her own artifact
if_condition_2 = (participants_with_min_assigned_reviews.size == 1 and TeamsUser.exists?(team_id: team.id, user_id: participants[participants_with_min_assigned_reviews[0]].user_id))
if if_condition_1 or if_condition_2
#use original method to get random number
rand_num = rand(0..num_participants-1)
else
#rand_num should be the position of this participant in original array
rand_num = participants_with_min_assigned_reviews[rand(0..participants_with_min_assigned_reviews.size-1)]
end
end
# prohibit one student to review his/her own artifact
next if TeamsUser.exists?(team_id: team.id, user_id: participants[rand_num].user_id)

if_condition_1 = (participants_hash[participants[rand_num].id] < student_review_num)
if_condition_2 = (!selected_participants.include? participants[rand_num].id)
if if_condition_1 and if_condition_2
# selected_participants cannot include duplicate num
selected_participants << participants[rand_num].id
participants_hash[participants[rand_num].id] += 1
end
# remove students who have already been assigned enough num of reviews out of participants array
participants.each do |participant|
if participants_hash[participant.id] == student_review_num
participants.delete_at(rand_num)
num_participants -= 1
end
end
end
else
#review num for last team can be different from other teams.
#prohibit one student to review his/her own artifact and selected_participants cannot include duplicate num
participants.each do |participant|
# avoid last team receives too many peer reviews
if !TeamsUser.exists?(team_id: team.id, user_id: participant.user_id) and selected_participants.size < num_reviews_per_team
selected_participants << participant.id
participants_hash[participant.id] += 1
end
end
end

begin
selected_participants.each {|index| ReviewResponseMap.where(:reviewee_id => team.id, :reviewer_id => index, :reviewed_object_id => assignment_id).first_or_create}
rescue
flash[:error] = "Automatic assignment of reviewer failed."
end
iterator += 1
end
end
</pre>

The method is made private so that it can only be called with in the controller and cannot directly be called through a view.

The complexity of the original method reduced after breaking it and can be easily readable now.

<pre>
def automatic_review_mapping_strategy(assignment_id, participants, teams, student_review_num=0, submission_review_num=0)
participants_hash = {}
participants.each { |participant| participants_hash[participant.id] = 0 }
#calculate reviewers for each team
num_participants = participants.size
if student_review_num != 0 and submission_review_num == 0
num_reviews_per_team = (participants.size * student_review_num * 1.0 / teams.size).round
exact_num_of_review_needed = participants.size * student_review_num
elsif student_review_num == 0 and submission_review_num != 0
num_reviews_per_team = submission_review_num
student_review_num = (teams.size * submission_review_num * 1.0 / participants.size).round
exact_num_of_review_needed = teams.size * submission_review_num
end
#Exception detection: If instructor want to assign too many reviews done by each student, there will be an error msg.
if student_review_num >= teams.size
flash[:error] = 'You cannot set the number of reviews done by each student to be greater than or equal to total number of teams [or "participants" if it is an individual assignment].'
end

peer_review_strategy(teams, num_participants, student_review_num, participants, participants_hash)

# after assigning peer reviews for each team, if there are still some peer reviewers not obtain enough peer review, just assign them to valid teams
if ReviewResponseMap.where(["reviewed_object_id = ? and created_at > ? and calibrate_to = ?", assignment_id, @@time_create_last_review_mapping_record, 0]).size < exact_num_of_review_needed
participants_with_insufficient_review_num = Array.new
participants_hash.each do |participant_id, review_num|
participants_with_insufficient_review_num << participant_id if review_num < student_review_num
end
unsorted_teams_hash = {}
ReviewResponseMap.where(["reviewed_object_id = ? and calibrate_to = ?", assignment_id, 0]).each do |response_map|
unless unsorted_teams_hash.has_key? (response_map.reviewee_id)
unsorted_teams_hash[response_map.reviewee_id] = 1
else
unsorted_teams_hash[response_map.reviewee_id] += 1
end
end
teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
participants_with_insufficient_review_num.each do |participant_id|
teams_hash.each do |team_id, num_review_received|
unless TeamsUser.exists?(team_id: team_id, user_id: Participant.find(participant_id).user_id)
ReviewResponseMap.where(:reviewee_id => team_id, :reviewer_id => participant_id, :reviewed_object_id => assignment_id).first_or_create
teams_hash[team_id] += 1
teams_hash = teams_hash.sort_by{|_, v| v}.to_h
break
end
end
end
end
@@time_create_last_review_mapping_record = ReviewResponseMap.where(reviewed_object_id: assignment_id).last.created_at
end
</pre>

'''4. Moving code specific to models to models instead of controller'''

The method response_report contains code which generates reports based on the input of one of the three of 'ReviewResponseMap', 'FeedbackResponseMap', 'TeammateReviewResponseMap' and 'Calibration'. Calibration is a special case and should be handled in the controller itself. But the other three are models that are subclasses of ResponseMap model. The report is generated for each of the three by calling the ResponseMap model and obtaining the values by querying the database. Moving the code to the models and calling the methods in the model through the controller makes more sense than writing the code in controller.

The following is the code snippet from the original method which contains the calls to ResponseMap model:

<pre>
case @type
when "ReviewResponseMap"
if params[:user].nil?
# This is not a search, so find all reviewers for this assignment
response_maps_with_distinct_participant_id = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ? and calibrate_to = ?", @id, @type, 0])
@reviewers = []
response_maps_with_distinct_participant_id.each do |reviewer_id_from_response_map|
@reviewers << (AssignmentParticipant.find(reviewer_id_from_response_map.reviewer_id))
end
@reviewers = Participant.sort_by_name(@reviewers)
else
# This is a search, so find reviewers by user's full name
user = User.select("DISTINCT id").where(["fullname LIKE ?", '%'+params[:user][:fullname]+'%'])
@reviewers = AssignmentParticipant.where(["user_id IN (?) and parent_id = ?", user, @assignment.id])
end
# @review_scores[reveiwer_id][reviewee_id] = score for assignments not using vary_rubric_by_rounds feature
# @review_scores[reviewer_id][round][reviewee_id] = score for assignments using vary_rubric_by_rounds feature
@review_scores = @assignment.compute_reviews_hash
@avg_and_ranges= @assignment.compute_avg_and_ranges_hash
when "FeedbackResponseMap"
#Example query
#SELECT distinct reviewer_id FROM response_maps where type = 'FeedbackResponseMap' and
#reviewed_object_id in (select id from responses where
#map_id in (select id from response_maps where reviewed_object_id = 722 and type = 'ReviewResponseMap'))
@review_response_map_ids = ResponseMap.select("id").where(["reviewed_object_id = ? and type = ?", @id, 'ReviewResponseMap'])
@response_ids = Response.select("id").where(["map_id IN (?)", @review_response_map_ids])
@reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id IN (?) and type = ?", @response_ids, @type])
when "TeammateReviewResponseMap"
#Example query
#SELECT distinct reviewer_id FROM response_maps where type = 'TeammateReviewResponseMap' and reviewed_object_id = 711
@reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ?", @id, 'TeammateReviewResponseMap'])
end
end
</pre>

The same code after moving the methods to their respective models looks as follows:

<pre>
case @type
when "ReviewResponseMap"
@review_user= params[:user]
#If review response is required call review_response_report method in review_response_map model
@reviewers = ReviewResponseMap.review_response_report(@id, @assignment,@type, @review_user)
@review_scores = @assignment.compute_reviews_hash
@avg_and_ranges= @assignment.compute_avg_and_ranges_hash
when "FeedbackResponseMap"
#If review report for feedback is required call feedback_response_report method in feedback_review_response_map model
@reviewers = FeedbackResponseMap.feedback_response_report(@id, @type)
when "TeammateReviewResponseMap"
#If review report for teammate is required call teammate_response_report method in teammate_review_response_map model
@reviewers = TeammateReviewResponseMap.teammate_response_report(@id)
end
end
</pre>

==References==

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/harsha1007/expertiza Forked repository for the project]
#[http://expertiza.ncsu.edu/ Expertiza Main Page]

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-03-30T20:28:39Z

Skunapa:

==E1615. Refactoring the Review Mapping Controller==

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

__TOC__

=='''Expertiza '''==

The Expertiza is a software project to create reusable learning objects through peer review. It is a Ruby on Rails based application which can be run on Windows, Linux and Mac OS X.

Main Features of Expertiza are:
* Allows students to work in groups to improve others learning experiences
* The work done by students/team is subjeted to multiple reviews , minimizing plagiarism
* Reviewing is done by studnets, which allows instructor/TA to spend less time in grading
* Large classes can also be handled easily with the help of Expertiza

=='''About Review mapping controller'''==

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the team for a submission , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Problem Statement'''===

The main aim of this project is to

* Refactor reporting method)(response_report) which is big
* Use more efficient function sample instead of shuffle for random number selection
* Remove unused and assigned variables
* Simplify Automatic_review_mapping_strategy

=='''Code Improvements'''==

==='''Unused variables and arguments'''===

There are unused variables in the methods which use the stack unnecessarily. So, it is better to remove the unused variables or at the least indicate that a variable is unused.

For suppose when both keys and values are not used in a hash but are given as arguments, then the unused variables can be indicated by adding a "_" infront of the name or replace the unused variable with "_" to represent it as unused variable but allow them in arguments.

<pre>
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
</pre>
<pre>
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

In the above case teams_hash should consist of a hash with both keys and values but the sorting is done based on values. So the key is replaced with a "_" so that the user may deem it unused in the implementation of the process.

==='''Use sample instead of shuffle'''===

When sample is used, the elements in an array are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle. Also by using shuffle[0] we are shuffling all the elements in the array and then picking the first element instead of picking a single element randomly which is more efficient. The following are the couple of places where shuffle[0] was used and is replaced by sample.

<pre>
Previous Code:

assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

==='''Cyclomatic complexity of automatic_review_mapping_strategy method'''===

The method automatic_review_mapping_strategy handles the number of reviews assigned to a individual and also the number of reviews that can be done with in a team. The method is very long and has many nested if statements due to which the complexity of the method is very high. Instead of a single method handling all the parts of the strategy, it is divided into several parts due to which the code is more readable and also the complexity of code is shared by each method.

The method first checks the number of participants that are in an assignment and the number of teams that are present. It then sets the values for the maximum number of reviews that are possible and also the minimum number that are required. Then it assigns the reviews to each of the teams randomly when a request for review is made by a participant. At last it checks if the participant has the minimum number of reviews required after allotting the reviews. If not, it assigns more reviews to valid teams so that the minimum requirement is met.

The part of the code that is moved out of automatic_review_mapping_strategy as peer_review_strategy:

<pre>
private
def peer_review_strategy(teams, num_participants, student_review_num, participants, participants_hash)
iterator = 0
teams.each do |team|
selected_participants = Array.new
if !team.equal? teams.last
#need to even out the # of reviews for teams
while selected_participants.size < num_reviews_per_team
num_participants_this_team = TeamsUser.where(team_id: team.id).size
#If there are some submitters or reviewers in this team, they are not treated as normal participants.
#They should be removed from 'num_participants_this_team'
TeamsUser.where(team_id: team.id).each do |team_user|
temp_participant = Participant.where(user_id: team_user.user_id, parent_id: assignment_id).first
num_participants_this_team -= 1 if temp_participant.can_review == false or temp_participant.can_submit == false
end
#if all outstanding participants are already in selected_participants, just break the loop.
break if selected_participants.size == participants.size - num_participants_this_team

# generate random number
if iterator == 0
rand_num = rand(0..num_participants-1)
else
min_value = participants_hash.values.min
#get the temp array including indices of participants, each participant has minimum review number in hash table.
participants_with_min_assigned_reviews = Array.new
participants.each do |participant|
participants_with_min_assigned_reviews << participants.index(participant) if participants_hash[participant.id] == min_value
end
#if participants_with_min_assigned_reviews is blank
if_condition_1 = participants_with_min_assigned_reviews.empty?
#or only one element in participants_with_min_assigned_reviews, prohibit one student to review his/her own artifact
if_condition_2 = (participants_with_min_assigned_reviews.size == 1 and TeamsUser.exists?(team_id: team.id, user_id: participants[participants_with_min_assigned_reviews[0]].user_id))
if if_condition_1 or if_condition_2
#use original method to get random number
rand_num = rand(0..num_participants-1)
else
#rand_num should be the position of this participant in original array
rand_num = participants_with_min_assigned_reviews[rand(0..participants_with_min_assigned_reviews.size-1)]
end
end
# prohibit one student to review his/her own artifact
next if TeamsUser.exists?(team_id: team.id, user_id: participants[rand_num].user_id)

if_condition_1 = (participants_hash[participants[rand_num].id] < student_review_num)
if_condition_2 = (!selected_participants.include? participants[rand_num].id)
if if_condition_1 and if_condition_2
# selected_participants cannot include duplicate num
selected_participants << participants[rand_num].id
participants_hash[participants[rand_num].id] += 1
end
# remove students who have already been assigned enough num of reviews out of participants array
participants.each do |participant|
if participants_hash[participant.id] == student_review_num
participants.delete_at(rand_num)
num_participants -= 1
end
end
end
else
#review num for last team can be different from other teams.
#prohibit one student to review his/her own artifact and selected_participants cannot include duplicate num
participants.each do |participant|
# avoid last team receives too many peer reviews
if !TeamsUser.exists?(team_id: team.id, user_id: participant.user_id) and selected_participants.size < num_reviews_per_team
selected_participants << participant.id
participants_hash[participant.id] += 1
end
end
end

begin
selected_participants.each {|index| ReviewResponseMap.where(:reviewee_id => team.id, :reviewer_id => index, :reviewed_object_id => assignment_id).first_or_create}
rescue
flash[:error] = "Automatic assignment of reviewer failed."
end
iterator += 1
end
end
</pre>

The method is made private so that it can only be called with in the controller and cannot directly be called through a view.

The complexity of the original method reduced after breaking it and can be easily readable now.

<pre>
def automatic_review_mapping_strategy(assignment_id, participants, teams, student_review_num=0, submission_review_num=0)
participants_hash = {}
participants.each { |participant| participants_hash[participant.id] = 0 }
#calculate reviewers for each team
num_participants = participants.size
if student_review_num != 0 and submission_review_num == 0
num_reviews_per_team = (participants.size * student_review_num * 1.0 / teams.size).round
exact_num_of_review_needed = participants.size * student_review_num
elsif student_review_num == 0 and submission_review_num != 0
num_reviews_per_team = submission_review_num
student_review_num = (teams.size * submission_review_num * 1.0 / participants.size).round
exact_num_of_review_needed = teams.size * submission_review_num
end
#Exception detection: If instructor want to assign too many reviews done by each student, there will be an error msg.
if student_review_num >= teams.size
flash[:error] = 'You cannot set the number of reviews done by each student to be greater than or equal to total number of teams [or "participants" if it is an individual assignment].'
end

peer_review_strategy(teams, num_participants, student_review_num, participants, participants_hash)

# after assigning peer reviews for each team, if there are still some peer reviewers not obtain enough peer review, just assign them to valid teams
if ReviewResponseMap.where(["reviewed_object_id = ? and created_at > ? and calibrate_to = ?", assignment_id, @@time_create_last_review_mapping_record, 0]).size < exact_num_of_review_needed
participants_with_insufficient_review_num = Array.new
participants_hash.each do |participant_id, review_num|
participants_with_insufficient_review_num << participant_id if review_num < student_review_num
end
unsorted_teams_hash = {}
ReviewResponseMap.where(["reviewed_object_id = ? and calibrate_to = ?", assignment_id, 0]).each do |response_map|
unless unsorted_teams_hash.has_key? (response_map.reviewee_id)
unsorted_teams_hash[response_map.reviewee_id] = 1
else
unsorted_teams_hash[response_map.reviewee_id] += 1
end
end
teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
participants_with_insufficient_review_num.each do |participant_id|
teams_hash.each do |team_id, num_review_received|
unless TeamsUser.exists?(team_id: team_id, user_id: Participant.find(participant_id).user_id)
ReviewResponseMap.where(:reviewee_id => team_id, :reviewer_id => participant_id, :reviewed_object_id => assignment_id).first_or_create
teams_hash[team_id] += 1
teams_hash = teams_hash.sort_by{|_, v| v}.to_h
break
end
end
end
end
@@time_create_last_review_mapping_record = ReviewResponseMap.where(reviewed_object_id: assignment_id).last.created_at
end
</pre>

'''4. Moving code specific to models to models instead of controller'''

The method response_report contains code which generates reports based on the input of one of the three of 'ReviewResponseMap', 'FeedbackResponseMap', 'TeammateReviewResponseMap' and 'Calibration'. Calibration is a special case and should be handled in the controller itself. But the other three are models that are subclasses of ResponseMap model. The report is generated for each of the three by calling the ResponseMap model and obtaining the values by querying the database. Moving the code to the models and calling the methods in the model through the controller makes more sense than writing the code in controller.

The following is the code snippet from the original method which contains the calls to ResponseMap model:

<pre>
case @type
when "ReviewResponseMap"
if params[:user].nil?
# This is not a search, so find all reviewers for this assignment
response_maps_with_distinct_participant_id = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ? and calibrate_to = ?", @id, @type, 0])
@reviewers = []
response_maps_with_distinct_participant_id.each do |reviewer_id_from_response_map|
@reviewers << (AssignmentParticipant.find(reviewer_id_from_response_map.reviewer_id))
end
@reviewers = Participant.sort_by_name(@reviewers)
else
# This is a search, so find reviewers by user's full name
user = User.select("DISTINCT id").where(["fullname LIKE ?", '%'+params[:user][:fullname]+'%'])
@reviewers = AssignmentParticipant.where(["user_id IN (?) and parent_id = ?", user, @assignment.id])
end
# @review_scores[reveiwer_id][reviewee_id] = score for assignments not using vary_rubric_by_rounds feature
# @review_scores[reviewer_id][round][reviewee_id] = score for assignments using vary_rubric_by_rounds feature
@review_scores = @assignment.compute_reviews_hash
@avg_and_ranges= @assignment.compute_avg_and_ranges_hash
when "FeedbackResponseMap"
#Example query
#SELECT distinct reviewer_id FROM response_maps where type = 'FeedbackResponseMap' and
#reviewed_object_id in (select id from responses where
#map_id in (select id from response_maps where reviewed_object_id = 722 and type = 'ReviewResponseMap'))
@review_response_map_ids = ResponseMap.select("id").where(["reviewed_object_id = ? and type = ?", @id, 'ReviewResponseMap'])
@response_ids = Response.select("id").where(["map_id IN (?)", @review_response_map_ids])
@reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id IN (?) and type = ?", @response_ids, @type])
when "TeammateReviewResponseMap"
#Example query
#SELECT distinct reviewer_id FROM response_maps where type = 'TeammateReviewResponseMap' and reviewed_object_id = 711
@reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ?", @id, 'TeammateReviewResponseMap'])
end
end
</pre>

The same code after moving the methods to their respective models looks as follows:

<pre>
case @type
when "ReviewResponseMap"
@review_user= params[:user]
#If review response is required call review_response_report method in review_response_map model
@reviewers = ReviewResponseMap.review_response_report(@id, @assignment,@type, @review_user)
@review_scores = @assignment.compute_reviews_hash
@avg_and_ranges= @assignment.compute_avg_and_ranges_hash
when "FeedbackResponseMap"
#If review report for feedback is required call feedback_response_report method in feedback_review_response_map model
@reviewers = FeedbackResponseMap.feedback_response_report(@id, @type)
when "TeammateReviewResponseMap"
#If review report for teammate is required call teammate_response_report method in teammate_review_response_map model
@reviewers = TeammateReviewResponseMap.teammate_response_report(@id)
end
end
</pre>

==References==

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/harsha1007/expertiza Forked repository for the project]
#[http://expertiza.ncsu.edu/ Expertiza Main Page]

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-03-24T03:33:39Z

Skunapa: /* Code Improvements */

==E1615. Refactoring the Review Mapping Controller==

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

__TOC__

==='''About Review mapping controller'''===

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the team for a submission , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Code Improvements'''===

'''1. Unused variables and arguments'''

There are unused variables in the methods which use the stack unnecessarily. So, it is better to remove the unused variables or at the least indicate that a variable is unused.

For suppose when both keys and values are not used in a hash but are given as arguments, then the unused variables can be indicated by adding a "_" infront of the name or replaced with "_" to represent it as unused variable but allow them arguments.

<pre>
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
</pre>
<pre>
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

In the above case teams_hash should consist of a hash with both keys and values but the sorting is done based on values. So the key is replaced with a "_" so that the user may deem it unused in the implementation of the process.

'''2. Use sample instead of shuffle'''

When sample is used, the elements in an array are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle. Also by using shuffle[0] we are shuffling all the elements in the array and then picking the first element instead of picking a single element randomly which is more efficient. The following are the couple of places where shuffle[0] was used and is replaced by sample.

<pre>
Previous Code:

assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

'''3. Cyclomatic complexity of automatic_review_mapping_strategy method'''

The method automatic_review_mapping_strategy handles the number of reviews assigned to a individual and also the number of reviews that can be done with in a team. The method is very long and has many nested if statements due to which the complexity of the method is very high. Instead of a single method handling all the parts of the strategy, it is divided into several parts due to which the code is more readable and also the complexity of code is shared by each method.

The method first checks the number of participants that are in an assignment and the number of teams that are present. It then sets the values for the maximum number of reviews that are possible and also the minimum number that are required. Then it assigns the reviews to each of the teams randomly when a request for review is made by a participant. At last it checks if the participant has the minimum number of reviews required after allotting the reviews. If not, it assigns more reviews to valid teams so that the minimum requirement is met.

The part of the code that is moved out of automatic_review_mapping_strategy as peer_review_strategy:

<pre>
private
def peer_review_strategy(teams, num_participants, student_review_num, participants, participants_hash)
iterator = 0
teams.each do |team|
selected_participants = Array.new
if !team.equal? teams.last
#need to even out the # of reviews for teams
while selected_participants.size < num_reviews_per_team
num_participants_this_team = TeamsUser.where(team_id: team.id).size
#If there are some submitters or reviewers in this team, they are not treated as normal participants.
#They should be removed from 'num_participants_this_team'
TeamsUser.where(team_id: team.id).each do |team_user|
temp_participant = Participant.where(user_id: team_user.user_id, parent_id: assignment_id).first
num_participants_this_team -= 1 if temp_participant.can_review == false or temp_participant.can_submit == false
end
#if all outstanding participants are already in selected_participants, just break the loop.
break if selected_participants.size == participants.size - num_participants_this_team

# generate random number
if iterator == 0
rand_num = rand(0..num_participants-1)
else
min_value = participants_hash.values.min
#get the temp array including indices of participants, each participant has minimum review number in hash table.
participants_with_min_assigned_reviews = Array.new
participants.each do |participant|
participants_with_min_assigned_reviews << participants.index(participant) if participants_hash[participant.id] == min_value
end
#if participants_with_min_assigned_reviews is blank
if_condition_1 = participants_with_min_assigned_reviews.empty?
#or only one element in participants_with_min_assigned_reviews, prohibit one student to review his/her own artifact
if_condition_2 = (participants_with_min_assigned_reviews.size == 1 and TeamsUser.exists?(team_id: team.id, user_id: participants[participants_with_min_assigned_reviews[0]].user_id))
if if_condition_1 or if_condition_2
#use original method to get random number
rand_num = rand(0..num_participants-1)
else
#rand_num should be the position of this participant in original array
rand_num = participants_with_min_assigned_reviews[rand(0..participants_with_min_assigned_reviews.size-1)]
end
end
# prohibit one student to review his/her own artifact
next if TeamsUser.exists?(team_id: team.id, user_id: participants[rand_num].user_id)

if_condition_1 = (participants_hash[participants[rand_num].id] < student_review_num)
if_condition_2 = (!selected_participants.include? participants[rand_num].id)
if if_condition_1 and if_condition_2
# selected_participants cannot include duplicate num
selected_participants << participants[rand_num].id
participants_hash[participants[rand_num].id] += 1
end
# remove students who have already been assigned enough num of reviews out of participants array
participants.each do |participant|
if participants_hash[participant.id] == student_review_num
participants.delete_at(rand_num)
num_participants -= 1
end
end
end
else
#review num for last team can be different from other teams.
#prohibit one student to review his/her own artifact and selected_participants cannot include duplicate num
participants.each do |participant|
# avoid last team receives too many peer reviews
if !TeamsUser.exists?(team_id: team.id, user_id: participant.user_id) and selected_participants.size < num_reviews_per_team
selected_participants << participant.id
participants_hash[participant.id] += 1
end
end
end

begin
selected_participants.each {|index| ReviewResponseMap.where(:reviewee_id => team.id, :reviewer_id => index, :reviewed_object_id => assignment_id).first_or_create}
rescue
flash[:error] = "Automatic assignment of reviewer failed."
end
iterator += 1
end
end
</pre>

The method is made private so that it can only be called with in the controller and cannot directly be called through a view.

The complexity of the original method reduced after breaking it and can be easily readable now.

<pre>
def automatic_review_mapping_strategy(assignment_id, participants, teams, student_review_num=0, submission_review_num=0)
participants_hash = {}
participants.each { |participant| participants_hash[participant.id] = 0 }
#calculate reviewers for each team
num_participants = participants.size
if student_review_num != 0 and submission_review_num == 0
num_reviews_per_team = (participants.size * student_review_num * 1.0 / teams.size).round
exact_num_of_review_needed = participants.size * student_review_num
elsif student_review_num == 0 and submission_review_num != 0
num_reviews_per_team = submission_review_num
student_review_num = (teams.size * submission_review_num * 1.0 / participants.size).round
exact_num_of_review_needed = teams.size * submission_review_num
end
#Exception detection: If instructor want to assign too many reviews done by each student, there will be an error msg.
if student_review_num >= teams.size
flash[:error] = 'You cannot set the number of reviews done by each student to be greater than or equal to total number of teams [or "participants" if it is an individual assignment].'
end

peer_review_strategy(teams, num_participants, student_review_num, participants, participants_hash)

# after assigning peer reviews for each team, if there are still some peer reviewers not obtain enough peer review, just assign them to valid teams
if ReviewResponseMap.where(["reviewed_object_id = ? and created_at > ? and calibrate_to = ?", assignment_id, @@time_create_last_review_mapping_record, 0]).size < exact_num_of_review_needed
participants_with_insufficient_review_num = Array.new
participants_hash.each do |participant_id, review_num|
participants_with_insufficient_review_num << participant_id if review_num < student_review_num
end
unsorted_teams_hash = {}
ReviewResponseMap.where(["reviewed_object_id = ? and calibrate_to = ?", assignment_id, 0]).each do |response_map|
unless unsorted_teams_hash.has_key? (response_map.reviewee_id)
unsorted_teams_hash[response_map.reviewee_id] = 1
else
unsorted_teams_hash[response_map.reviewee_id] += 1
end
end
teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
participants_with_insufficient_review_num.each do |participant_id|
teams_hash.each do |team_id, num_review_received|
unless TeamsUser.exists?(team_id: team_id, user_id: Participant.find(participant_id).user_id)
ReviewResponseMap.where(:reviewee_id => team_id, :reviewer_id => participant_id, :reviewed_object_id => assignment_id).first_or_create
teams_hash[team_id] += 1
teams_hash = teams_hash.sort_by{|_, v| v}.to_h
break
end
end
end
end
@@time_create_last_review_mapping_record = ReviewResponseMap.where(reviewed_object_id: assignment_id).last.created_at
end
</pre>

'''4. Moving code specific to models to models instead of controller'''

The method response_report contains code which generates reports based on the input of one of the three of 'ReviewResponseMap', 'FeedbackResponseMap', 'TeammateReviewResponseMap' and 'Calibration'. Calibration is a special case and should be handled in the controller itself. But the other three are models that are subclasses of ResponseMap model. The report is generated for each of the three by calling the ResponseMap model and obtaining the values by querying the database. Moving the code to the models and calling the methods in the model through the controller makes more sense than writing the code in controller.

The following is the code snippet from the original method which contains the calls to ResponseMap model:

<pre>
case @type
when "ReviewResponseMap"
if params[:user].nil?
# This is not a search, so find all reviewers for this assignment
response_maps_with_distinct_participant_id = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ? and calibrate_to = ?", @id, @type, 0])
@reviewers = []
response_maps_with_distinct_participant_id.each do |reviewer_id_from_response_map|
@reviewers << (AssignmentParticipant.find(reviewer_id_from_response_map.reviewer_id))
end
@reviewers = Participant.sort_by_name(@reviewers)
else
# This is a search, so find reviewers by user's full name
user = User.select("DISTINCT id").where(["fullname LIKE ?", '%'+params[:user][:fullname]+'%'])
@reviewers = AssignmentParticipant.where(["user_id IN (?) and parent_id = ?", user, @assignment.id])
end
# @review_scores[reveiwer_id][reviewee_id] = score for assignments not using vary_rubric_by_rounds feature
# @review_scores[reviewer_id][round][reviewee_id] = score for assignments using vary_rubric_by_rounds feature
@review_scores = @assignment.compute_reviews_hash
@avg_and_ranges= @assignment.compute_avg_and_ranges_hash
when "FeedbackResponseMap"
#Example query
#SELECT distinct reviewer_id FROM response_maps where type = 'FeedbackResponseMap' and
#reviewed_object_id in (select id from responses where
#map_id in (select id from response_maps where reviewed_object_id = 722 and type = 'ReviewResponseMap'))
@review_response_map_ids = ResponseMap.select("id").where(["reviewed_object_id = ? and type = ?", @id, 'ReviewResponseMap'])
@response_ids = Response.select("id").where(["map_id IN (?)", @review_response_map_ids])
@reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id IN (?) and type = ?", @response_ids, @type])
when "TeammateReviewResponseMap"
#Example query
#SELECT distinct reviewer_id FROM response_maps where type = 'TeammateReviewResponseMap' and reviewed_object_id = 711
@reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ?", @id, 'TeammateReviewResponseMap'])
end
end
</pre>

The same code after moving the methods to their respective models looks as follows:

<pre>
case @type
when "ReviewResponseMap"
@review_user= params[:user]
#If review response is required call review_response_report method in review_response_map model
@reviewers = ReviewResponseMap.review_response_report(@id, @assignment,@type, @review_user)
@review_scores = @assignment.compute_reviews_hash
@avg_and_ranges= @assignment.compute_avg_and_ranges_hash
when "FeedbackResponseMap"
#If review report for feedback is required call feedback_response_report method in feedback_review_response_map model
@reviewers = FeedbackResponseMap.feedback_response_report(@id, @type)
when "TeammateReviewResponseMap"
#If review report for teammate is required call teammate_response_report method in teammate_review_response_map model
@reviewers = TeammateReviewResponseMap.teammate_response_report(@id)
end
end
</pre>

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/harsha1007/expertiza Forked repository for the project]
#[http://expertiza.ncsu.edu/ Expertiza Main Page]

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-03-24T03:29:55Z

Skunapa: /* Code Improvements */

==E1615. Refactoring the Review Mapping Controller==

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

__TOC__

==='''About Review mapping controller'''===

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the team for a submission , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Code Improvements'''===

'''1. Unused variables and arguments'''

There are unused variables in the methods which use the stack unnecessarily. So, it is better to remove the unused variables or at the least indicate that a variable is unused.

For suppose when both keys and values are not used in a hash but are given as arguments, then the unused variables can be indicated by adding a "_" infront of the name or replaced with "_" to represent it as unused variable but allow them arguments.

<pre>
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
</pre>
<pre>
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

In the above case teams_hash should consist of a hash with both keys and values but the sorting is done based on values. So the key is replaced with a "_" so that the user may deem it unused in the implementation of the process.

'''2. Use sample instead of shuffle'''

When sample is used, the elements in an array are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle. Also by using shuffle[0] we are shuffling all the elements in the array and then picking the first element instead of picking a single element randomly which is more efficient. The following are the couple of places where shuffle[0] was used and is replaced by sample.

<pre>
Previous Code:

assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

'''3. Cyclomatic complexity of automatic_review_mapping_strategy method'''

The method automatic_review_mapping_strategy handles the number of reviews assigned to a individual and also the number of reviews that can be done with in a team. The method is very long and has many nested if statements due to which the complexity of the method is very high. Instead of a single method handling all the parts of the strategy, it is divided into several parts due to which the code is more readable and also the complexity of code is shared by each method.

The method first checks the number of participants that are in an assignment and the number of teams that are present. It then sets the values for the maximum number of reviews that are possible and also the minimum number that are required. Then it assigns the reviews to each of the teams randomly when a request for review is made by a participant. At last it checks if the participant has the minimum number of reviews required after allotting the reviews. If not, it assigns more reviews to valid teams so that the minimum requirement is met.

The part of the method which assigns reviews to teams automatically is divided into a separate method called peer_review_strategy.

'''4. Moving code specific to models to models instead of controller'''

The method response_report contains code which generates reports based on the input of one of the three of 'ReviewResponseMap', 'FeedbackResponseMap', 'TeammateReviewResponseMap' and 'Calibration'. Calibration is a special case and should be handled in the controller itself. But the other three are models that are subclasses of ResponseMap model. The report is generated for each of the three by calling the ResponseMap model and obtaining the values by querying the database. Moving the code to the models and calling the methods in the model through the controller makes more sense than writing the code in controller.

The following is the code snippet from the original method which contains the calls to ResponseMap model:

<pre>
case @type
when "ReviewResponseMap"
if params[:user].nil?
# This is not a search, so find all reviewers for this assignment
response_maps_with_distinct_participant_id = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ? and calibrate_to = ?", @id, @type, 0])
@reviewers = []
response_maps_with_distinct_participant_id.each do |reviewer_id_from_response_map|
@reviewers << (AssignmentParticipant.find(reviewer_id_from_response_map.reviewer_id))
end
@reviewers = Participant.sort_by_name(@reviewers)
else
# This is a search, so find reviewers by user's full name
user = User.select("DISTINCT id").where(["fullname LIKE ?", '%'+params[:user][:fullname]+'%'])
@reviewers = AssignmentParticipant.where(["user_id IN (?) and parent_id = ?", user, @assignment.id])
end
# @review_scores[reveiwer_id][reviewee_id] = score for assignments not using vary_rubric_by_rounds feature
# @review_scores[reviewer_id][round][reviewee_id] = score for assignments using vary_rubric_by_rounds feature
@review_scores = @assignment.compute_reviews_hash
@avg_and_ranges= @assignment.compute_avg_and_ranges_hash
when "FeedbackResponseMap"
#Example query
#SELECT distinct reviewer_id FROM response_maps where type = 'FeedbackResponseMap' and
#reviewed_object_id in (select id from responses where
#map_id in (select id from response_maps where reviewed_object_id = 722 and type = 'ReviewResponseMap'))
@review_response_map_ids = ResponseMap.select("id").where(["reviewed_object_id = ? and type = ?", @id, 'ReviewResponseMap'])
@response_ids = Response.select("id").where(["map_id IN (?)", @review_response_map_ids])
@reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id IN (?) and type = ?", @response_ids, @type])
when "TeammateReviewResponseMap"
#Example query
#SELECT distinct reviewer_id FROM response_maps where type = 'TeammateReviewResponseMap' and reviewed_object_id = 711
@reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ?", @id, 'TeammateReviewResponseMap'])
end
end
</pre>

The same code after moving the methods to their respective models looks as follows:

<pre>
case @type
when "ReviewResponseMap"
@review_user= params[:user]
#If review response is required call review_response_report method in review_response_map model
@reviewers = ReviewResponseMap.review_response_report(@id, @assignment,@type, @review_user)
@review_scores = @assignment.compute_reviews_hash
@avg_and_ranges= @assignment.compute_avg_and_ranges_hash
when "FeedbackResponseMap"
#If review report for feedback is required call feedback_response_report method in feedback_review_response_map model
@reviewers = FeedbackResponseMap.feedback_response_report(@id, @type)
when "TeammateReviewResponseMap"
#If review report for teammate is required call teammate_response_report method in teammate_review_response_map model
@reviewers = TeammateReviewResponseMap.teammate_response_report(@id)
end
end
</pre>

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/harsha1007/expertiza Forked repository for the project]
#[http://expertiza.ncsu.edu/ Expertiza Main Page]

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-03-24T03:29:44Z

Skunapa: /* Code Improvements */

==E1615. Refactoring the Review Mapping Controller==

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

__TOC__

==='''About Review mapping controller'''===

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the team for a submission , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Code Improvements'''===

'''1. Unused variables and arguments'''

There are unused variables in the methods which use the stack unnecessarily. So, it is better to remove the unused variables or at the least indicate that a variable is unused.

For suppose when both keys and values are not used in a hash but are given as arguments, then the unused variables can be indicated by adding a "_" infront of the name or replaced with "_" to represent it as unused variable but allow them arguments.

<pre>
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
</pre>
<pre>
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

In the above case teams_hash should consist of a hash with both keys and values but the sorting is done based on values. So the key is replaced with a "_" so that the user may deem it unused in the implementation of the process.

'''2. Use sample instead of shuffle'''

When sample is used, the elements in an array are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle. Also by using shuffle[0] we are shuffling all the elements in the array and then picking the first element instead of picking a single element randomly which is more efficient. The following are the couple of places where shuffle[0] was used and is replaced by sample.

<pre>

Previous Code:

assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

'''3. Cyclomatic complexity of automatic_review_mapping_strategy method'''

The method automatic_review_mapping_strategy handles the number of reviews assigned to a individual and also the number of reviews that can be done with in a team. The method is very long and has many nested if statements due to which the complexity of the method is very high. Instead of a single method handling all the parts of the strategy, it is divided into several parts due to which the code is more readable and also the complexity of code is shared by each method.

The method first checks the number of participants that are in an assignment and the number of teams that are present. It then sets the values for the maximum number of reviews that are possible and also the minimum number that are required. Then it assigns the reviews to each of the teams randomly when a request for review is made by a participant. At last it checks if the participant has the minimum number of reviews required after allotting the reviews. If not, it assigns more reviews to valid teams so that the minimum requirement is met.

The part of the method which assigns reviews to teams automatically is divided into a separate method called peer_review_strategy.

'''4. Moving code specific to models to models instead of controller'''

The method response_report contains code which generates reports based on the input of one of the three of 'ReviewResponseMap', 'FeedbackResponseMap', 'TeammateReviewResponseMap' and 'Calibration'. Calibration is a special case and should be handled in the controller itself. But the other three are models that are subclasses of ResponseMap model. The report is generated for each of the three by calling the ResponseMap model and obtaining the values by querying the database. Moving the code to the models and calling the methods in the model through the controller makes more sense than writing the code in controller.

The following is the code snippet from the original method which contains the calls to ResponseMap model:

<pre>
case @type
when "ReviewResponseMap"
if params[:user].nil?
# This is not a search, so find all reviewers for this assignment
response_maps_with_distinct_participant_id = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ? and calibrate_to = ?", @id, @type, 0])
@reviewers = []
response_maps_with_distinct_participant_id.each do |reviewer_id_from_response_map|
@reviewers << (AssignmentParticipant.find(reviewer_id_from_response_map.reviewer_id))
end
@reviewers = Participant.sort_by_name(@reviewers)
else
# This is a search, so find reviewers by user's full name
user = User.select("DISTINCT id").where(["fullname LIKE ?", '%'+params[:user][:fullname]+'%'])
@reviewers = AssignmentParticipant.where(["user_id IN (?) and parent_id = ?", user, @assignment.id])
end
# @review_scores[reveiwer_id][reviewee_id] = score for assignments not using vary_rubric_by_rounds feature
# @review_scores[reviewer_id][round][reviewee_id] = score for assignments using vary_rubric_by_rounds feature
@review_scores = @assignment.compute_reviews_hash
@avg_and_ranges= @assignment.compute_avg_and_ranges_hash
when "FeedbackResponseMap"
#Example query
#SELECT distinct reviewer_id FROM response_maps where type = 'FeedbackResponseMap' and
#reviewed_object_id in (select id from responses where
#map_id in (select id from response_maps where reviewed_object_id = 722 and type = 'ReviewResponseMap'))
@review_response_map_ids = ResponseMap.select("id").where(["reviewed_object_id = ? and type = ?", @id, 'ReviewResponseMap'])
@response_ids = Response.select("id").where(["map_id IN (?)", @review_response_map_ids])
@reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id IN (?) and type = ?", @response_ids, @type])
when "TeammateReviewResponseMap"
#Example query
#SELECT distinct reviewer_id FROM response_maps where type = 'TeammateReviewResponseMap' and reviewed_object_id = 711
@reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ?", @id, 'TeammateReviewResponseMap'])
end
end
</pre>

The same code after moving the methods to their respective models looks as follows:

<pre>
case @type
when "ReviewResponseMap"
@review_user= params[:user]
#If review response is required call review_response_report method in review_response_map model
@reviewers = ReviewResponseMap.review_response_report(@id, @assignment,@type, @review_user)
@review_scores = @assignment.compute_reviews_hash
@avg_and_ranges= @assignment.compute_avg_and_ranges_hash
when "FeedbackResponseMap"
#If review report for feedback is required call feedback_response_report method in feedback_review_response_map model
@reviewers = FeedbackResponseMap.feedback_response_report(@id, @type)
when "TeammateReviewResponseMap"
#If review report for teammate is required call teammate_response_report method in teammate_review_response_map model
@reviewers = TeammateReviewResponseMap.teammate_response_report(@id)
end
end
</pre>

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/harsha1007/expertiza Forked repository for the project]
#[http://expertiza.ncsu.edu/ Expertiza Main Page]

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-03-24T03:24:23Z

Skunapa: /* Code Improvements */

==E1615. Refactoring the Review Mapping Controller==

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

__TOC__

==='''About Review mapping controller'''===

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the team for a submission , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Code Improvements'''===

'''1. Unused variables and arguments'''

There are unused variables in the methods which use the stack unnecessarily. So, it is better to remove the unused variables or at the least indicate that a variable is unused.

For suppose when both keys and values are not used in a hash but are given as arguments, then the unused variables can be indicated by adding a "_" infront of the name or replaced with "_" to represent it as unused variable but allow them arguments.

<pre>
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
</pre>
<pre>
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

In the above case teams_hash should consist of a hash with both keys and values but the sorting is done based on values. So the key is replaced with a "_" so that the user may deem it unused in the implementation of the process.

'''2. Use sample instead of shuffle'''

When sample is used, the elements in an array are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle. Also by using shuffle[0] we are shuffling all the elements in the array and then picking the first element instead of picking a single element randomly which is more efficient. The following are the couple of places where shuffle[0] was used and is replaced by sample.

<pre>

Previous Code:
assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

'''3. Cyclomatic complexity of automatic_review_mapping_strategy method'''

The method automatic_review_mapping_strategy handles the number of reviews assigned to a individual and also the number of reviews that can be done with in a team. The method is very long and has many nested if statements due to which the complexity of the method is very high. Instead of a single method handling all the parts of the strategy, it is divided into several parts due to which the code is more readable and also the complexity of code is shared by each method.

The method first checks the number of participants that are in an assignment and the number of teams that are present. It then sets the values for the maximum number of reviews that are possible and also the minimum number that are required. Then it assigns the reviews to each of the teams randomly when a request for review is made by a participant. At last it checks if the participant has the minimum number of reviews required after allotting the reviews. If not, it assigns more reviews to valid teams so that the minimum requirement is met.

The part of the method which assigns reviews to teams automatically is divided into a separate method called peer_review_strategy.

'''4. Moving code specific to models to models instead of controller'''

The method response_report contains code which generates reports based on the input of one of the three of 'ReviewResponseMap', 'FeedbackResponseMap', 'TeammateReviewResponseMap' and 'Calibration'. Calibration is a special case and should be handled in the controller itself. But the other three are models that are subclasses of ResponseMap model. The report is generated for each of the three by calling the ResponseMap model and obtaining the values by querying the database. Moving the code to the models and calling the methods in the model through the controller makes more sense than writing the code in controller.

The following is the code snippet from the original method which contains the calls to ResponseMap model:

<pre>
case @type
when "ReviewResponseMap"
if params[:user].nil?
# This is not a search, so find all reviewers for this assignment
response_maps_with_distinct_participant_id = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ? and calibrate_to = ?", @id, @type, 0])
@reviewers = []
response_maps_with_distinct_participant_id.each do |reviewer_id_from_response_map|
@reviewers << (AssignmentParticipant.find(reviewer_id_from_response_map.reviewer_id))
end
@reviewers = Participant.sort_by_name(@reviewers)
else
# This is a search, so find reviewers by user's full name
user = User.select("DISTINCT id").where(["fullname LIKE ?", '%'+params[:user][:fullname]+'%'])
@reviewers = AssignmentParticipant.where(["user_id IN (?) and parent_id = ?", user, @assignment.id])
end
# @review_scores[reveiwer_id][reviewee_id] = score for assignments not using vary_rubric_by_rounds feature
# @review_scores[reviewer_id][round][reviewee_id] = score for assignments using vary_rubric_by_rounds feature
@review_scores = @assignment.compute_reviews_hash
@avg_and_ranges= @assignment.compute_avg_and_ranges_hash
when "FeedbackResponseMap"
#Example query
#SELECT distinct reviewer_id FROM response_maps where type = 'FeedbackResponseMap' and
#reviewed_object_id in (select id from responses where
#map_id in (select id from response_maps where reviewed_object_id = 722 and type = 'ReviewResponseMap'))
@review_response_map_ids = ResponseMap.select("id").where(["reviewed_object_id = ? and type = ?", @id, 'ReviewResponseMap'])
@response_ids = Response.select("id").where(["map_id IN (?)", @review_response_map_ids])
@reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id IN (?) and type = ?", @response_ids, @type])
when "TeammateReviewResponseMap"
#Example query
#SELECT distinct reviewer_id FROM response_maps where type = 'TeammateReviewResponseMap' and reviewed_object_id = 711
@reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ?", @id, 'TeammateReviewResponseMap'])
end
end
</pre>

The same code after moving the methods to their respective models looks as follows:

<pre>
case @type
when "ReviewResponseMap"
@review_user= params[:user]
#If review response is required call review_response_report method in review_response_map model
@reviewers = ReviewResponseMap.review_response_report(@id, @assignment,@type, @review_user)
@review_scores = @assignment.compute_reviews_hash
@avg_and_ranges= @assignment.compute_avg_and_ranges_hash
when "FeedbackResponseMap"
#If review report for feedback is required call feedback_response_report method in feedback_review_response_map model
@reviewers = FeedbackResponseMap.feedback_response_report(@id, @type)
when "TeammateReviewResponseMap"
#If review report for teammate is required call teammate_response_report method in teammate_review_response_map model
@reviewers = TeammateReviewResponseMap.teammate_response_report(@id)
end
end
</pre>

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/harsha1007/expertiza Forked repository for the project]
#[http://expertiza.ncsu.edu/ Expertiza Main Page]

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-03-24T03:22:47Z

Skunapa: /* Code Improvements */

==E1615. Refactoring the Review Mapping Controller==

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

__TOC__

==='''About Review mapping controller'''===

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the team for a submission , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Code Improvements'''===

'''1. Unused variables and arguments'''

There are unused variables in the methods which use the stack unnecessarily. So, it is better to remove the unused variables or at the least indicate that a variable is unused.

For suppose when both keys and values are not used in a hash but are given as arguments, then the unused variables can be indicated by adding a "_" infront of the name or replaced with "_" to represent it as unused variable but allow them arguments.

<pre>
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
</pre>
<pre>
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

In the above case teams_hash should consist of a hash with both keys and values but the sorting is done based on values. So the key is replaced with a "_" so that the user may deem it unused in the implementation of the process.

'''2. Use sample instead of shuffle'''

When sample is used, the elements in an array are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle. Also by using shuffle[0] we are shuffling all the elements in the array and then picking the first element instead of picking a single element randomly which is more efficient. The following are the couple of places where shuffle[0] was used and is replaced by sample.

<pre>

Previous Code:
assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

'''3. Cyclomatic complexity of automatic_review_mapping_strategy method'''

The method automatic_review_mapping_strategy handles the number of reviews assigned to a individual and also the number of reviews that can be done with in a team. The method is very long and has many nested if statements due to which the complexity of the method is very high. Instead of a single method handling all the parts of the strategy, it is divided into several parts due to which the code is more readable and also the complexity of code is shared by each method.

The method first checks the number of participants that are in an assignment and the number of teams that are present. It then sets the values for the maximum number of reviews that are possible and also the minimum number that are required. Then it assigns the reviews to each of the teams randomly when a request for review is made by a participant. At last it checks if the participant has the minimum number of reviews required after allotting the reviews. If not, it assigns more reviews to valid teams so that the minimum requirement is met.

The part of the method which assigns reviews to teams automatically is divided into a separate method called peer_review_strategy.

'''4. Moving code specific to models to models instead of controller'''

The method response_report contains code which generates reports based on the input of one of the three of 'ReviewResponseMap', 'FeedbackResponseMap', 'TeammateReviewResponseMap' and 'Calibration'. Calibration is a special case and should be handled in the controller itself. But the other three are models that are subclasses of ResponseMap model. The report is generated for each of the three by calling the ResponseMap model and obtaining the values by querying the database. Moving the code to the models and calling the methods in the model through the controller makes more sense than writing the code in controller.

The following is the code snippet from the original method which contains the calls to ResponseMap model:

<pre>
case @type
when "ReviewResponseMap"
if params[:user].nil?
# This is not a search, so find all reviewers for this assignment
response_maps_with_distinct_participant_id = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ? and calibrate_to = ?", @id, @type, 0])
@reviewers = []
response_maps_with_distinct_participant_id.each do |reviewer_id_from_response_map|
@reviewers << (AssignmentParticipant.find(reviewer_id_from_response_map.reviewer_id))
end
@reviewers = Participant.sort_by_name(@reviewers)
else
# This is a search, so find reviewers by user's full name
user = User.select("DISTINCT id").where(["fullname LIKE ?", '%'+params[:user][:fullname]+'%'])
@reviewers = AssignmentParticipant.where(["user_id IN (?) and parent_id = ?", user, @assignment.id])
end
# @review_scores[reveiwer_id][reviewee_id] = score for assignments not using vary_rubric_by_rounds feature
# @review_scores[reviewer_id][round][reviewee_id] = score for assignments using vary_rubric_by_rounds feature
@review_scores = @assignment.compute_reviews_hash
@avg_and_ranges= @assignment.compute_avg_and_ranges_hash
when "FeedbackResponseMap"
#Example query
#SELECT distinct reviewer_id FROM response_maps where type = 'FeedbackResponseMap' and
#reviewed_object_id in (select id from responses where
#map_id in (select id from response_maps where reviewed_object_id = 722 and type = 'ReviewResponseMap'))
@review_response_map_ids = ResponseMap.select("id").where(["reviewed_object_id = ? and type = ?", @id, 'ReviewResponseMap'])
@response_ids = Response.select("id").where(["map_id IN (?)", @review_response_map_ids])
@reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id IN (?) and type = ?", @response_ids, @type])
when "TeammateReviewResponseMap"
#Example query
#SELECT distinct reviewer_id FROM response_maps where type = 'TeammateReviewResponseMap' and reviewed_object_id = 711
@reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ?", @id, 'TeammateReviewResponseMap'])
when "Calibration"
participant = AssignmentParticipant.where(parent_id: params[:id], user_id: session[:user].id).first rescue nil
if participant.nil?
participant = AssignmentParticipant.create(parent_id: params[:id], user_id: session[:user].id, can_submit: 1, can_review: 1, can_take_quiz: 1, handle: 'handle')
end
@assignment = Assignment.find(params[:id])
@review_questionnaire_ids = ReviewQuestionnaire.select("id")
@assignment_questionnaire = AssignmentQuestionnaire.where(["assignment_id = ? and questionnaire_id IN (?)", params[:id], @review_questionnaire_ids]).first
@questions = @assignment_questionnaire.questionnaire.questions.select{|q| q.type == 'Criterion' or q.type == 'Scale'}
@calibration_response_maps = ReviewResponseMap.where(["reviewed_object_id = ? and calibrate_to = ?", params[:id], 1])
@review_response_map_ids = ReviewResponseMap.select('id').where(["reviewed_object_id = ? and calibrate_to = ?", params[:id], 0])
@responses = Response.where(["map_id IN (?)", @review_response_map_ids])
end
end
</pre>

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/harsha1007/expertiza Forked repository for the project]
#[http://expertiza.ncsu.edu/ Expertiza Main Page]

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-03-24T03:07:20Z

Skunapa: /* Code Improvements */

==E1615. Refactoring the Review Mapping Controller==

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

__TOC__

==='''About Review mapping controller'''===

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the team for a submission , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Code Improvements'''===

'''1. Unused variables and arguments'''

There are unused variables in the methods which use the stack unnecessarily. So, it is better to remove the unused variables or at the least indicate that a variable is unused.

For suppose when both keys and values are not used in a hash but are given as arguments, then the unused variables can be indicated by adding a "_" infront of the name or replaced with "_" to represent it as unused variable but allow them arguments.

<pre>
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
</pre>
<pre>
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

In the above case teams_hash should consist of a hash with both keys and values but the sorting is done based on values. So the key is replaced with a "_" so that the user may deem it unused in the implementation of the process.

'''2. Use sample instead of shuffle'''

When sample is used, the elements in an array are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle. Also by using shuffle[0] we are shuffling all the elements in the array and then picking the first element instead of picking a single element randomly which is more efficient. The following are the couple of places where shuffle[0] was used and is replaced by sample.

<pre>

Previous Code:
assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

'''3. Cyclomatic complexity of automatic_review_mapping_strategy method'''

The method automatic_review_mapping_strategy handles the number of reviews assigned to a individual and also the number of reviews that can be done with in a team. The method is very long and has many nested if statements due to which the complexity of the method is very high. Instead of a single method handling all the parts of the strategy, it is divided into several parts due to which the code is more readable and also the complexity of code is shared by each method.

The method first checks the number of participants that are in an assignment and the number of teams that are present. It then sets the values for the maximum number of reviews that are possible and also the minimum number that are required. Then it assigns the reviews to each of the teams randomly when a request for review is made by a participant. At last it checks if the participant has the minimum number of reviews required after allotting the reviews. If not, it assigns more reviews to valid teams so that the minimum requirement is met.

The part of the method which assigns reviews to teams automatically is divided into a separate method called peer_review_strategy.

'''4. Moving code specific to models to models instead of controller'''

The method response_report contains code which generates

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/harsha1007/expertiza Forked repository for the project]
#[http://expertiza.ncsu.edu/ Expertiza Main Page]

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-03-24T02:39:55Z

Skunapa: /* Code Improvements */

==E1615. Refactoring the Review Mapping Controller==

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

__TOC__

==='''About Review mapping controller'''===

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the team for a submission , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Code Improvements'''===

'''1. Unused variables and arguments'''

There are unused variables in the methods which use the stack unnecessarily. So, it is better to remove the unused variables or at the least indicate that a variable is unused.

For suppose when both keys and values are not used in a hash but are given as arguments, then the unused variables can be indicated by adding a "_" infront of the name or replaced with "_" to represent it as unused variable but allow them arguments.

<pre>
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
</pre>
<pre>
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

In the above case teams_hash should consist of a hash with both keys and values but the sorting is done based on values. So the key is replaced with a "_" so that the user may deem it unused in the implementation of the process.

'''2. Use sample instead of shuffle'''

When sample is used, the elements in an array are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle. Also by using shuffle[0] we are shuffling all the elements in the array and then picking the first element instead of picking a single element randomly which is more efficient. The following are the couple of places where shuffle[0] was used and is replaced by sample.

<pre>

Previous Code:
assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/harsha1007/expertiza Forked repository for the project]
#[http://expertiza.ncsu.edu/ Expertiza Main Page]

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-03-24T02:39:44Z

Skunapa: /* Code Improvements */

==E1615. Refactoring the Review Mapping Controller==

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

__TOC__

==='''About Review mapping controller'''===

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the team for a submission , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Code Improvements'''===

'''1. Unused variables and arguments'''

There are unused variables in the methods which use the stack unnecessarily. So, it is better to remove the unused variables or at the least indicate that a variable is unused.

For suppose when both keys and values are not used in a hash but are given as arguments, then the unused variables can be indicated by adding a "_" infront of the name or replaced with "_" to represent it as unused variable but allow them arguments.

<pre>
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
</pre>
<pre>
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

In the above case teams_hash should consist of a hash with both keys and values but the sorting is done based on values. So the key is replaced with a "_" so that the user may deem it unused in the implementation of the process.

'''2. Use sample instead of shuffle'''

When sample is used, the elements in an array are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle. Also by using shuffle[0] we are shuffling all the elements in the array and then picking the first element instead of picking a single element randomly which is more efficient. The following are the couple of places where shuffle[0] is used and is replaced by sample.

<pre>

Previous Code:
assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/harsha1007/expertiza Forked repository for the project]
#[http://expertiza.ncsu.edu/ Expertiza Main Page]

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-03-24T02:39:04Z

Skunapa: /* Code Improvements */

==E1615. Refactoring the Review Mapping Controller==

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

__TOC__

==='''About Review mapping controller'''===

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the team for a submission , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Code Improvements'''===

'''1. Unused variables and arguments'''

There are unused variables in the methods which use the stack unnecessarily. So, it is better to remove the unused variables or at the least indicate that a variable is unused.

For suppose when both keys and values are not used in a hash but are given as arguments, then the unused variables can be indicated by adding a "_" infront of the name or replaced with "_" to represent it as unused variable but allow them arguments.

<pre>
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
</pre>
<pre>
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

In the above case teams_hash should consist of a hash with both keys and values but the sorting is done based on values. So the key is replaced with a "_" so that the user may deem it unused in the implementation of the process.

'''2. Use sample instead of shuffle'''

When sample is used, the elements in an array are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle. Also by using shuffle[0] we are shuffling all the elements in the array and then picking the first element instead of picking a single element randomly which is more efficient.

<pre>

Previous Code:
assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/harsha1007/expertiza Forked repository for the project]
#[http://expertiza.ncsu.edu/ Expertiza Main Page]

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-03-24T02:37:07Z

Skunapa: /* Code Improvements */

==E1615. Refactoring the Review Mapping Controller==

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

__TOC__

==='''About Review mapping controller'''===

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the team for a submission , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Code Improvements'''===

'''1. Unused variables and arguments'''

There are unused variables in the methods which use the stack unnecessarily. So, it is better to remove the unused variables or at the least indicate that a variable is unused.

For suppose when both keys and values are not used in a hash but are given as arguments, then the unused variables can be indicated by adding a "_" infront of the name or replaced with "_" to represent it as unused variable but allow them arguments.

<pre>
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
</pre>
<pre>
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

In the above case teams_hash should consist of a hash with both keys and values but the sorting is done based on values. So the key is replaced with a "_" so that the user may deem it unused in the implementation of the process.

'''2. Use sample instead of shuffle'''

When sample is used, the elements are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle.

<pre>

Previous Code:
assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/harsha1007/expertiza Forked repository for the project]
#[http://expertiza.ncsu.edu/ Expertiza Main Page]

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-03-24T02:36:10Z

Skunapa: /* Code Improvements */

==E1615. Refactoring the Review Mapping Controller==

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

__TOC__

==='''About Review mapping controller'''===

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the team for a submission , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Code Improvements'''===

1. Unused variables and arguments

There are unused variables in the methods which use the stack unnecessarily. So, it is better to remove the unused variables or at the least indicate that a variable is unused.

For suppose when both keys and values are not used in a hash but are given as arguments, then the unused variables can be indicated by adding a "_" infront of the name or replaced with "_" to represent it as unused variable but allow them arguments.

<pre>
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
</pre>
<pre>
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

In the above case teams_hash should consist of a hash with both keys and values but the sorting is done based on values. So the key is replaced with a "_" so that the user may deem it unused in the implementation of the process.

2. Use sample instead of shuffle

When sample is used, the elements are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle.

<pre>

Previous Code:
assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/harsha1007/expertiza Forked repository for the project]
#[http://expertiza.ncsu.edu/ Expertiza Main Page]

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-03-24T02:34:12Z

Skunapa: /* Code Improvements */

==E1615. Refactoring the Review Mapping Controller==

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

__TOC__

==='''About Review mapping controller'''===

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the team for a submission , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Code Improvements'''===

1. Unused variables and arguments: There are unused variables in the methods which use the stack unnecessarily. So, it is better to remove the unused variables or at the least indicate that a variable is unused.

For suppose when both keys and values are not used in a hash but are given as arguments, then the unused variables can be indicated by adding a "_" infront of the name or replaced with "_" to represent it as unused variable but allow them arguments.

<pre>
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
</pre>
<pre>
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

In the above case teams_hash should consist of a hash with both keys and values but the sorting is done based on values. So the key is replaced with a "_" so that the user may deem it unused in the implementation of the process.

2. Use sample instead of shuffle
When sample is used, the elements are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle.

<pre>

Previous Code:
assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/harsha1007/expertiza Forked repository for the project]
#[http://expertiza.ncsu.edu/ Expertiza Main Page]

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-03-24T02:30:08Z

Skunapa: /* Code Improvements */

==E1615. Refactoring the Review Mapping Controller==

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

__TOC__

==='''About Review mapping controller'''===

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the team for a submission , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Code Improvements'''===

1. Unused variables and arguments: There are unused variables in the methods. If there are unused variables in the methods, it uses stack unnecessarily. So, it is better to remove the unused variables.

When both keys and values are not used, but given as arguments, then the used variables can be added "_" or replaced with "_" to represent it as unused variable but allowed in the arguments.

<pre>
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
</pre>
<pre>
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

2. Use sample instead of shuffle
When sample is used, the elements are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle.

<pre>

Previous Code:
assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/harsha1007/expertiza Forked repository for the project]
#[http://expertiza.ncsu.edu/ Expertiza Main Page]

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-03-24T02:29:28Z

Skunapa: /* About Review mapping controller */

==E1615. Refactoring the Review Mapping Controller==

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

__TOC__

==='''About Review mapping controller'''===

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the team for a submission , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Code Improvements'''===

1. Unused variables and arguments: There are unused variables in the methods. If there are unused variables in the methods, it uses stack unnecessarily. So, it is better to remove the unused variables.

When both keys and values are not used, but given as arguments, then the used variables can be added "_" or replaced with "_" to represent it as unused variable but allowed in the arguments.

<pre>
At line 533 and line 539
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

2. Use sample instead of shuffle
When sample is used, the elements are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle.

<pre>

Previous Code:
assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/harsha1007/expertiza Forked repository for the project]
#[http://expertiza.ncsu.edu/ Expertiza Main Page]

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-03-24T02:27:46Z

Skunapa:

==E1615. Refactoring the Review Mapping Controller==

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

__TOC__

==='''About Review mapping controller'''===

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the submission team , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Code Improvements'''===

1. Unused variables and arguments: There are unused variables in the methods. If there are unused variables in the methods, it uses stack unnecessarily. So, it is better to remove the unused variables.

When both keys and values are not used, but given as arguments, then the used variables can be added "_" or replaced with "_" to represent it as unused variable but allowed in the arguments.

<pre>
At line 533 and line 539
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

2. Use sample instead of shuffle
When sample is used, the elements are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle.

<pre>

Previous Code:
assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/harsha1007/expertiza Forked repository for the project]
#[http://expertiza.ncsu.edu/ Expertiza Main Page]

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-03-24T02:26:31Z

Skunapa: /* About Review mapping controller */

==E1615. Refactoring the Review Mapping Controller==

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

__TOC__

==='''About Review mapping controller'''===

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the submission team , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Code Improvements'''===

1. Unused variables and arguments: There are unused variables in the methods. If there are unused variables in the methods, it uses stack unnecessarily. So, it is better to remove the unused variables.

When both keys and values are not used, but given as arguments, then the used variables can be added "_" or replaced with "_" to represent it as unused variable but allowed in the arguments.

<pre>
At line 533 and line 539
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

2. Use sample instead of shuffle
When sample is used, the elements are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle.

<pre>

Previous Code:
assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/WintersLt/expertiza GitHub Project Repository Fork]
#[http://expertiza.ncsu.edu/ The live Expertiza website]
#[http://bit.ly/myexpertiza Demo link]
#[http://wikis.lib.ncsu.edu/index.php/Expertiza Expertiza project documentation wiki]
#[https://relishapp.com/rspec Rspec Documentation]
#Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-03-24T02:25:42Z

Skunapa:

==E1615. Refactoring the Review Mapping Controller==

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

__TOC__

==='''About Review mapping controller'''===

/* When a reviewerreport reviews, assigning reviews and quizzes to teams. It also contains methods to assign reviews to participants automatically*/ Ignore this

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the submission team , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Code Improvements'''===

1. Unused variables and arguments: There are unused variables in the methods. If there are unused variables in the methods, it uses stack unnecessarily. So, it is better to remove the unused variables.

When both keys and values are not used, but given as arguments, then the used variables can be added "_" or replaced with "_" to represent it as unused variable but allowed in the arguments.

<pre>
At line 533 and line 539
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

2. Use sample instead of shuffle
When sample is used, the elements are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle.

<pre>

Previous Code:
assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/WintersLt/expertiza GitHub Project Repository Fork]
#[http://expertiza.ncsu.edu/ The live Expertiza website]
#[http://bit.ly/myexpertiza Demo link]
#[http://wikis.lib.ncsu.edu/index.php/Expertiza Expertiza project documentation wiki]
#[https://relishapp.com/rspec Rspec Documentation]
#Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-03-24T02:25:10Z

Skunapa:

==E1615. Refactoring the Review Mapping Controller==

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

__TOC__

=='''Wiki write up'''==

About Review mapping controller

/* When a reviewerreport reviews, assigning reviews and quizzes to teams. It also contains methods to assign reviews to participants automatically*/ Ignore this

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the submission team , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

==='''Code Improvements'''===

1. Unused variables and arguments: There are unused variables in the methods. If there are unused variables in the methods, it uses stack unnecessarily. So, it is better to remove the unused variables.

When both keys and values are not used, but given as arguments, then the used variables can be added "_" or replaced with "_" to represent it as unused variable but allowed in the arguments.

<pre>
At line 533 and line 539
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>

2. Use sample instead of shuffle
When sample is used, the elements are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle.

<pre>

Previous Code:
assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/WintersLt/expertiza GitHub Project Repository Fork]
#[http://expertiza.ncsu.edu/ The live Expertiza website]
#[http://bit.ly/myexpertiza Demo link]
#[http://wikis.lib.ncsu.edu/index.php/Expertiza Expertiza project documentation wiki]
#[https://relishapp.com/rspec Rspec Documentation]
#Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-03-23T21:07:27Z

Skunapa:

==E1553. Refactoring the Review Mapping Controller==

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.

__TOC__

===Introduction to Expertiza===

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

===Problem Statement===
The following tasks were accomplished in this project:

* Improved the clarity of code by improving the variable and parameter names.
* Long character strings were taken and given appropriate names.
* Handled pagination by a separate helper module, which can be used by multiple controllers.
* Implemented action_allowed for access_control to prevent unauthorized access of methods.
* Prevented displaying of all versions for all users and tables when a user views the index page.
* Added missing CRUD methods to Versions Controller
* Added RSPEC testcases for testing changes done in Versions Controller

===About Versions Controller===
This class manages different versions of reviews. If a reviewer reviews a submission, and after that, the author revises the submission, the next time the reviewer does a review (s)he will create a new version. Sometimes it’s necessary to find the current version of a review; sometimes it’s necessary to find all versions. Similarly, a user may want to delete the current version of a review, or all versions of a review.
Pagination of versions helps the user to view a subset of versions at a time. Considering the huge number of versions in the system, it is very useful to have a pagination mechanism and a filtering mechanism which can be applied on the whole set of versions. The idea is to display the versions in an ordered, comprehensible and logical manner. In Expertiza the gem ‘will_paginate’ is used to achieve pagination.

===Current Implementation===

=====Functionality=====
* Any user irrespective of his/ her privileges can view all the versions.
::The versions which a particular user can view should be restricted based on the privileges of the user. For instance, only a user with Administrator privileges should be able to view all the versions in the system. However, that is not the case now. Every user can view all the versions irrespective of whether the user is a student or an administrator.
* Any user can delete any version
::The versions which a particular user can delete should be restricted based on the privileges of the user. For instance, a student should not be allowed to delete any version. According to the current implementation any user can delete any version in the system.
* Filtering of versions were restricted to the current user
::The filtering options on versions were restricted to the current user. Sometimes a user might want to view versions associated with other users. For instance, an instructor might want to view the list of versions created by a particular student. This is not possible with the current implementation.

=====Drawbacks and Solutions=====
* '''Problem 1''': The method paginate_list is doing more than one thing.
::The method paginate_list was building a complex search criteria based on the input params, getting the list of versions from the Database matching this search criteria and then calling the Page API. All these tasks in a single method made it difficult to understand.
<pre>
# For filtering the versions list with proper search and pagination.
def paginate_list(id, user_id, item_type, event, datetime)
# Set up the search criteria
criteria = ''
criteria = criteria + "id = #{id} AND " if id && id.to_i > 0
if current_user_role? == 'Super-Administrator'
criteria = criteria + "whodunnit = #{user_id} AND " if user_id && user_id.to_i > 0
end
criteria = criteria + "whodunnit = #{current_user.try(:id)} AND " if current_user.try(:id) && current_user.try(:id).to_i > 0
criteria = criteria + "item_type = '#{item_type}' AND " if item_type && !(item_type.eql? 'Any')
criteria = criteria + "event = '#{event}' AND " if event && !(event.eql? 'Any')
criteria = criteria + "created_at >= '#{time_to_string(params[:start_time])}' AND "
criteria = criteria + "created_at <= '#{time_to_string(params[:end_time])}' AND "

if current_role == 'Instructor' || current_role == 'Administrator'

end

# Remove the last ' AND '
criteria = criteria[0..-5]

versions = Version.page(params[:page]).order('id').per_page(25).where(criteria)
versions
end
</pre>
* '''Solution''': The implementation has been changed in such a way that the versions which a user is allowed to see depends on the privileges of the user. The approach we have taken is as follows:
**An administrator can see all the versions
**An instructor can see all the versions created by him and other users who are in his course or are participants in the assignments he creates.
**A TA can see all the versions created by him and other users who are in the course for which he/ she assists.
**A Student can see all the versions created by him/ her.
* '''Problem 2''': The search criteria created in the method paginate_list was difficult to comprehend.
::The code which builds the search criteria in the method paginate_list uses many string literals and conditions and is hardly intuitive. The programmer will have to spend some time to understand what the code is really doing.
* '''Solution''': The implementation has been changed. A student is not allowed to delete any versions now. Other types of users, for instance administrators, instructors and TAs are allowed to delete only the versions they are authorized to view.
* '''Problem 3''': The paginate method can be moved to a helper class.
::VersionsController is not the only component which require to paginate items. There are other components too. For instance, the UsersController has to paginate the list of users. Hence the Paginate method can be moved to a helper class which can be accessed by other components as well.
* '''Solution''': The filtering options has also been enhanced. The current user can now choose as part of the version search filter any user from a list of users if the current user is authorized to see the versions created by that user.

===New Implementation===
*The method paginate_list has been split into 2 methods now.
** BuildSearchCriteria – as the name suggests the sole purpose of this method is to build a search criteria based on the input search filters when the current user initiates a search in versions.
** paginate_list – this method will call the paginate API.
:First the search criteria is built, then the criteria is applied to versions in the database to get all versions which matches the criteria and then the retrieved versions are paginated.
<pre>
# pagination.
def paginate_list(versions)
paginate(versions, VERSIONS_PER_PAGE);
end

def BuildSearchCriteria(id, user_id, item_type, event)
# Set up the search criteria
search_criteria = ''
search_criteria = search_criteria + add_id_filter_if_valid(id).to_s
if current_user_role? == 'Super-Administrator'
search_criteria = search_criteria + add_user_filter_for_super_admin(user_id).to_s
end
search_criteria = search_criteria + add_user_filter
search_criteria = search_criteria + add_version_type_filter(item_type).to_s
search_criteria = search_criteria + add_event_filter(event).to_s
search_criteria = search_criteria + add_date_time_filter
search_criteria
end
</pre>
* The string literals and conditions in the method paginate_list were replaced with methods with intuitive names so that the programmer can understand the code more easily. We also removed an empty if clause and a redundant statement.
<pre>
def add_id_filter_if_valid (id)
"id = #{id} AND " if id && id.to_i > 0
end

def add_user_filter_for_super_admin (user_id)
"whodunnit = #{user_id} AND " if user_id && user_id.to_i > 0
end

def add_user_filter
"whodunnit = #{current_user.try(:id)} AND " if current_user.try(:id) && current_user.try(:id).to_i > 0
end

def add_event_filter (event)
"event = '#{event}' AND " if event && !(event.eql? 'Any')
end

def add_date_time_filter
"created_at >= '#{time_to_string(params[:start_time])}' AND " +
"created_at <= '#{time_to_string(params[:end_time])}'"
end

def add_version_type_filter (version_type)
"item_type = '#{version_type}' AND " if version_type && !(version_type.eql? 'Any')
end
</pre>
* The paginate method has been moved to the helper class Pagination_Helper. This new method can be now reused by the different components like UsersController etc. The method receives two parameters, first the list to paginate and second the number of items to be displayed in a page.

<pre>
module PaginationHelper

def paginate (items, number_of_items_per_page)
items.page(params[:page]).per_page(number_of_items_per_page)
end

end
</pre>

===Code improvements===
* Introduced a constant VERSIONS_PER_PAGE and assigned the value 25 to it. The pagination algorithm for VersionsController displays at most 25 versions in a page. The existing implementation uses the value 25 straight in the code and there are few problems associated with such an approach.
** It is not easy to understand what 25 is unless the programmer takes a close look at the code.
** In case if the value 25 is used at more than one places and in future a new requirement comes to show at most 30 versions in a page, all the values will have to be modified. It is not very DRY.
* The VersionsController was overriding AccessHelper - action_allowed? method to return true in all the cases. This was violating the whole purpose of the method action_allowed?. The purpose of this method is to determine whether the user who is triggering a CRUD operation is allowed to do so. So when the current user invokes a CRUD operation, the action_allowed? method is invoked first and if the method returns true the CRUD operation is triggered or else the user is intimated with a message and gracefully exited. Hence, when the action_allowed? method is overridden to return true always, it results in providing unauthorized access to certain users.

<pre>
def action_allowed?
true
end
</pre>

:With the new implementation the AccessHelper - action_allowed? method has been modified in such a way that unauthorized access is prevented. As per the new algorithm, 'new', 'create', 'edit', 'update' cannot be invoked by any user. These operations can be accessed only by ‘papertrail’ gem. Only an ‘Administrator’ or ‘Super-Administrator’ can call 'destroy_all' method. All the other methods are accessible to ‘Administrator’, ‘Super-Administrator’, ‘Instructor’, ‘Teaching Assistant’ and ‘Student’.

<pre>
def action_allowed?
case params[:action]
when 'new', 'create', 'edit', 'update'
#Modifications can only be done by papertrail
return false
when 'destroy_all'
['Super-Administrator',
'Administrator'].include? current_role_name
else
#Allow all others
['Super-Administrator',
'Administrator',
'Instructor',
'Teaching Assistant',
'Student'].include? current_role_name
end
end
</pre>

===Automated Testing using RSPEC===
The current version of expertiza did not have any test for VersionsController. Using the test driven development(TDD) approach, we have added an exhaustive set of RSPEC tests for VersionsController, to test all the modifications we have done to the code of the controller class. The tests use double and stub features of rspec-rails gem, to fake the log in by different users - Administrator, Instructor, Student etc. The tests can be executed "rpec spec" command as shown below.
<pre>
user-expertiza $rspec spec
.
.
.
Finished in 5.39 seconds (files took 25.33 seconds to load)
66 examples, 0 failures

Randomized with seed 19254
.
.
</pre>

===Testing from UI===
Following are a few testcases with respectto our code changes that can be tried from UI:
1. To go to versions index page, type in the following url after logging in:
http://152.46.16.81:3000/versions

2. After logging in as student/instructor or admin : Try accessing the new, create, edit, update actions. These actions are not allowed to any of the users.
http://152.46.16.81:3000/versions/new
This calls the new action. In the current production version of expertiza, it is unhandled and application gives a default 404 page.

3. Another feature that can be tested from UI is Pagination. Try searching for a user's versions and see if the results are paginated or not. Search here:
http://152.46.16.81:3000/versions/search

4. Visit the same URL as step 3, you should see only the students under that instructor in the users dropdown.

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/WintersLt/expertiza GitHub Project Repository Fork]
#[http://expertiza.ncsu.edu/ The live Expertiza website]
#[http://bit.ly/myexpertiza Demo link]
#[http://wikis.lib.ncsu.edu/index.php/Expertiza Expertiza project documentation wiki]
#[https://relishapp.com/rspec Rspec Documentation]
#Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-03-23T21:05:47Z

Skunapa:

==E1553. Refactoring the Versions Controller==

This page provides a description of the Expertiza based OSS project.

__TOC__

===Introduction to Expertiza===

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

===Problem Statement===
The following tasks were accomplished in this project:

* Improved the clarity of code by improving the variable and parameter names.
* Long character strings were taken and given appropriate names.
* Handled pagination by a separate helper module, which can be used by multiple controllers.
* Implemented action_allowed for access_control to prevent unauthorized access of methods.
* Prevented displaying of all versions for all users and tables when a user views the index page.
* Added missing CRUD methods to Versions Controller
* Added RSPEC testcases for testing changes done in Versions Controller

===About Versions Controller===
This class manages different versions of reviews. If a reviewer reviews a submission, and after that, the author revises the submission, the next time the reviewer does a review (s)he will create a new version. Sometimes it’s necessary to find the current version of a review; sometimes it’s necessary to find all versions. Similarly, a user may want to delete the current version of a review, or all versions of a review.
Pagination of versions helps the user to view a subset of versions at a time. Considering the huge number of versions in the system, it is very useful to have a pagination mechanism and a filtering mechanism which can be applied on the whole set of versions. The idea is to display the versions in an ordered, comprehensible and logical manner. In Expertiza the gem ‘will_paginate’ is used to achieve pagination.

===Current Implementation===

=====Functionality=====
* Any user irrespective of his/ her privileges can view all the versions.
::The versions which a particular user can view should be restricted based on the privileges of the user. For instance, only a user with Administrator privileges should be able to view all the versions in the system. However, that is not the case now. Every user can view all the versions irrespective of whether the user is a student or an administrator.
* Any user can delete any version
::The versions which a particular user can delete should be restricted based on the privileges of the user. For instance, a student should not be allowed to delete any version. According to the current implementation any user can delete any version in the system.
* Filtering of versions were restricted to the current user
::The filtering options on versions were restricted to the current user. Sometimes a user might want to view versions associated with other users. For instance, an instructor might want to view the list of versions created by a particular student. This is not possible with the current implementation.

=====Drawbacks and Solutions=====
* '''Problem 1''': The method paginate_list is doing more than one thing.
::The method paginate_list was building a complex search criteria based on the input params, getting the list of versions from the Database matching this search criteria and then calling the Page API. All these tasks in a single method made it difficult to understand.
<pre>
# For filtering the versions list with proper search and pagination.
def paginate_list(id, user_id, item_type, event, datetime)
# Set up the search criteria
criteria = ''
criteria = criteria + "id = #{id} AND " if id && id.to_i > 0
if current_user_role? == 'Super-Administrator'
criteria = criteria + "whodunnit = #{user_id} AND " if user_id && user_id.to_i > 0
end
criteria = criteria + "whodunnit = #{current_user.try(:id)} AND " if current_user.try(:id) && current_user.try(:id).to_i > 0
criteria = criteria + "item_type = '#{item_type}' AND " if item_type && !(item_type.eql? 'Any')
criteria = criteria + "event = '#{event}' AND " if event && !(event.eql? 'Any')
criteria = criteria + "created_at >= '#{time_to_string(params[:start_time])}' AND "
criteria = criteria + "created_at <= '#{time_to_string(params[:end_time])}' AND "

if current_role == 'Instructor' || current_role == 'Administrator'

end

# Remove the last ' AND '
criteria = criteria[0..-5]

versions = Version.page(params[:page]).order('id').per_page(25).where(criteria)
versions
end
</pre>
* '''Solution''': The implementation has been changed in such a way that the versions which a user is allowed to see depends on the privileges of the user. The approach we have taken is as follows:
**An administrator can see all the versions
**An instructor can see all the versions created by him and other users who are in his course or are participants in the assignments he creates.
**A TA can see all the versions created by him and other users who are in the course for which he/ she assists.
**A Student can see all the versions created by him/ her.
* '''Problem 2''': The search criteria created in the method paginate_list was difficult to comprehend.
::The code which builds the search criteria in the method paginate_list uses many string literals and conditions and is hardly intuitive. The programmer will have to spend some time to understand what the code is really doing.
* '''Solution''': The implementation has been changed. A student is not allowed to delete any versions now. Other types of users, for instance administrators, instructors and TAs are allowed to delete only the versions they are authorized to view.
* '''Problem 3''': The paginate method can be moved to a helper class.
::VersionsController is not the only component which require to paginate items. There are other components too. For instance, the UsersController has to paginate the list of users. Hence the Paginate method can be moved to a helper class which can be accessed by other components as well.
* '''Solution''': The filtering options has also been enhanced. The current user can now choose as part of the version search filter any user from a list of users if the current user is authorized to see the versions created by that user.

===New Implementation===
*The method paginate_list has been split into 2 methods now.
** BuildSearchCriteria – as the name suggests the sole purpose of this method is to build a search criteria based on the input search filters when the current user initiates a search in versions.
** paginate_list – this method will call the paginate API.
:First the search criteria is built, then the criteria is applied to versions in the database to get all versions which matches the criteria and then the retrieved versions are paginated.
<pre>
# pagination.
def paginate_list(versions)
paginate(versions, VERSIONS_PER_PAGE);
end

def BuildSearchCriteria(id, user_id, item_type, event)
# Set up the search criteria
search_criteria = ''
search_criteria = search_criteria + add_id_filter_if_valid(id).to_s
if current_user_role? == 'Super-Administrator'
search_criteria = search_criteria + add_user_filter_for_super_admin(user_id).to_s
end
search_criteria = search_criteria + add_user_filter
search_criteria = search_criteria + add_version_type_filter(item_type).to_s
search_criteria = search_criteria + add_event_filter(event).to_s
search_criteria = search_criteria + add_date_time_filter
search_criteria
end
</pre>
* The string literals and conditions in the method paginate_list were replaced with methods with intuitive names so that the programmer can understand the code more easily. We also removed an empty if clause and a redundant statement.
<pre>
def add_id_filter_if_valid (id)
"id = #{id} AND " if id && id.to_i > 0
end

def add_user_filter_for_super_admin (user_id)
"whodunnit = #{user_id} AND " if user_id && user_id.to_i > 0
end

def add_user_filter
"whodunnit = #{current_user.try(:id)} AND " if current_user.try(:id) && current_user.try(:id).to_i > 0
end

def add_event_filter (event)
"event = '#{event}' AND " if event && !(event.eql? 'Any')
end

def add_date_time_filter
"created_at >= '#{time_to_string(params[:start_time])}' AND " +
"created_at <= '#{time_to_string(params[:end_time])}'"
end

def add_version_type_filter (version_type)
"item_type = '#{version_type}' AND " if version_type && !(version_type.eql? 'Any')
end
</pre>
* The paginate method has been moved to the helper class Pagination_Helper. This new method can be now reused by the different components like UsersController etc. The method receives two parameters, first the list to paginate and second the number of items to be displayed in a page.

<pre>
module PaginationHelper

def paginate (items, number_of_items_per_page)
items.page(params[:page]).per_page(number_of_items_per_page)
end

end
</pre>

===Code improvements===
* Introduced a constant VERSIONS_PER_PAGE and assigned the value 25 to it. The pagination algorithm for VersionsController displays at most 25 versions in a page. The existing implementation uses the value 25 straight in the code and there are few problems associated with such an approach.
** It is not easy to understand what 25 is unless the programmer takes a close look at the code.
** In case if the value 25 is used at more than one places and in future a new requirement comes to show at most 30 versions in a page, all the values will have to be modified. It is not very DRY.
* The VersionsController was overriding AccessHelper - action_allowed? method to return true in all the cases. This was violating the whole purpose of the method action_allowed?. The purpose of this method is to determine whether the user who is triggering a CRUD operation is allowed to do so. So when the current user invokes a CRUD operation, the action_allowed? method is invoked first and if the method returns true the CRUD operation is triggered or else the user is intimated with a message and gracefully exited. Hence, when the action_allowed? method is overridden to return true always, it results in providing unauthorized access to certain users.

<pre>
def action_allowed?
true
end
</pre>

:With the new implementation the AccessHelper - action_allowed? method has been modified in such a way that unauthorized access is prevented. As per the new algorithm, 'new', 'create', 'edit', 'update' cannot be invoked by any user. These operations can be accessed only by ‘papertrail’ gem. Only an ‘Administrator’ or ‘Super-Administrator’ can call 'destroy_all' method. All the other methods are accessible to ‘Administrator’, ‘Super-Administrator’, ‘Instructor’, ‘Teaching Assistant’ and ‘Student’.

<pre>
def action_allowed?
case params[:action]
when 'new', 'create', 'edit', 'update'
#Modifications can only be done by papertrail
return false
when 'destroy_all'
['Super-Administrator',
'Administrator'].include? current_role_name
else
#Allow all others
['Super-Administrator',
'Administrator',
'Instructor',
'Teaching Assistant',
'Student'].include? current_role_name
end
end
</pre>

===Automated Testing using RSPEC===
The current version of expertiza did not have any test for VersionsController. Using the test driven development(TDD) approach, we have added an exhaustive set of RSPEC tests for VersionsController, to test all the modifications we have done to the code of the controller class. The tests use double and stub features of rspec-rails gem, to fake the log in by different users - Administrator, Instructor, Student etc. The tests can be executed "rpec spec" command as shown below.
<pre>
user-expertiza $rspec spec
.
.
.
Finished in 5.39 seconds (files took 25.33 seconds to load)
66 examples, 0 failures

Randomized with seed 19254
.
.
</pre>

===Testing from UI===
Following are a few testcases with respectto our code changes that can be tried from UI:
1. To go to versions index page, type in the following url after logging in:
http://152.46.16.81:3000/versions

2. After logging in as student/instructor or admin : Try accessing the new, create, edit, update actions. These actions are not allowed to any of the users.
http://152.46.16.81:3000/versions/new
This calls the new action. In the current production version of expertiza, it is unhandled and application gives a default 404 page.

3. Another feature that can be tested from UI is Pagination. Try searching for a user's versions and see if the results are paginated or not. Search here:
http://152.46.16.81:3000/versions/search

4. Visit the same URL as step 3, you should see only the students under that instructor in the users dropdown.

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/WintersLt/expertiza GitHub Project Repository Fork]
#[http://expertiza.ncsu.edu/ The live Expertiza website]
#[http://bit.ly/myexpertiza Demo link]
#[http://wikis.lib.ncsu.edu/index.php/Expertiza Expertiza project documentation wiki]
#[https://relishapp.com/rspec Rspec Documentation]
#Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-02-07T04:56:23Z

Skunapa:

{{Infobox software
| name =
| logo = 
| author =
| developer =
| released = 
| latest release version =
| latest release date = 
| status =
| programming language =
| operating system =
| license =
| website = {{URL|example.org}}
}}
Devise is a [https://en.wikipedia.org/wiki/Rack_(web_server_interface) rack] based full-fledged authentication system for [https://en.wikipedia.org/wiki/Ruby_on_Rails Rails]. It is a complete [https://en.wikipedia.org/wiki/Model-view-controller MVC] solution meaning it can support various models, views and controllers as part of its code and can be used be directly by developers. Devise is simple to use and starts up with a couple of commands but it is also highly customizable. Devise saves a lot of time and effort as many applications require user registration and authentication mechanisms which are difficult to develop from scratch.

=='''History'''==
Devise is first introduced in January 2010 by [https://github.com/plataformatec Plataformatec], a company which builds web and mobile applications. Devise is one of the few authentication systems which support rack based applications and hence can support Rails 3 and up as they are completely rack based. The latest version of Devise available is v3.5.3 and it is up to date with Rails 5 beta 2.
===Warden===
Devise gem is built on top of a rack application called [https://github.com/hassox/warden/wiki Warden] which is used to verify the identity of logged in user using a session string. In Warden, the id which is a primary key of a user is somehow stored to match it later with the logged in user. Warden also provides restricted access to guest users depending on the functional requirements of an application. 
Since Warden does not know of the existence of the Rails application, it cannot provide any helper methods, controller classes, views etc. This is where Devise comes into play and can integrate with Rails seemlessly. Devise interacts often with Warden using Strategy design pattern for [https://en.wikipedia.org/wiki/Encryption encrypting] passwords, [https://en.wikipedia.org/wiki/Basic_access_authentication HTTP Authentication] etc.

=='''Installation'''==
Although Devise is very useful and reduces the amount of effort in developing authentication mechanisms significantly, it requires a good understanding of the Rails framework. Hence it is advised for beginners to not use Devise. 
There are several commands that are required for the successful installation of Devise. They are listed out below:
*Add the devise gem to your gemfile.
gem 'devise'
*Run bundle command to install the gem.
bundle install
*You need to run the generator next which will install an initializer and creates all the configuration files.
rails generate devise:install
*Now you can add Devise to any of your models using the generator. This will create a class with the model name given and routes etc. The model will be configured with default Devise modules. The config/routes.rb file will be configured to point to the Devise controller.
rails generate devise user //Assuming that the model name is user
*Next, add any configuration changes that are required and then run:
rake db:migrate
*The following step will create Devise views but is optional. Devise has views for every generic operation like Login or SignUp and can be used directly instead of creating custom views.
rails generate devise:views users
* There are also many routes that are defined in config/routes.rb with a line like:
devise_for :users

=='''Modules'''==
When a devise generator is invoked a model class is created in app/models for you modify for your specific application requirements. This is the place where many important configuration changes are specified. Perhaps the most important are the Devise modules which provides essential functionalities like enhanced securtiy. 
There are 10 modules listed on the official page of Devise by Plataformatec. These modules are features that are contained in Devise and can be used by the developers depending on the use-cases or requirements of their application. Below is the list of modules:
*Database Authenticable
*Omniauthable
*Confirmable
*Recoverable
*Registerable
*Rememberable
*Trackable
*Timeoutable
*Validatable
*Lockable
Information regarding each module is listed in README as well as the [http://devise.plataformatec.com.br/ Plataformatec] website. The modules are included in an application in this way:
<pre>
class User < ActiveRecord::Base
devise :database_authenticable, :omniauthable, :confirmable, :rememberable,
:trackable, :timeoutable, :lockable
end
</pre>
In addition to the classes generated Devise also generates a [https://en.wikipedia.org/wiki/Data_migration#Database_migration database migration] in which fields related to the functionalities of these modules are added. Each field is related to a specific module and hence when a module is not require some of the fields may be removed from the migration to the [https://en.wikipedia.org/wiki/Database database]. Also most of these modules have specific [https://en.wikipedia.org/wiki/Form_(HTML) forms] and view associated with them. The forms are used by an end user to type in his/her information which will then be sent to the Devise controllers.

=='''Methods'''==
There are many classes in Devise which include models, controllers, helpers, views, routes etc. But much of the functionality offered by Devise is exposed via simple helper methods. Some of the most important methods which can be used in building our own application are:
 
*'''authenticate_user! :''' This method is used to check whether a user is logged in before he/she attempts to perform a specific set of controller actions. authenticate_user! may be called with before_action as shown below to ensure the user is logged in before performing any of the operations.
before_action :authenticate_user!
If only some of the actions need authentication and some do not, we can use except clause so that only some actions are blocked as guest and others are accessible. The code with except clause is as below:
before_action :authenticate_user! except [:index, :show]
In the above example index and show are two controller actions associated with operations which do not require user authentication and can be browsed as a guest.Authenticate user may also be used with in a controller action so that it is application only for that specification. We need to use before_filter instead of before_action to achieve this purpose.
<pre>
class EndUserBaseController < ApplicationController
before_filter :authentication_user!
end
</pre>
In this example the application will authenticate a user only if he is trying to perform an action associated with the EndUserBaseController. If in any of the above cases a user is not logged in the application backs off and redirects to its sign-in page.
*'''current_user :''' current_user method is used to return the model class corresponding to the user who is currently signed in. For example, if you are building a messaging application, you may retrieve all the sent messages of a user as:
<pre>
class SentMessagesController < ApplicationController
before_filter :authentication_user!
def index
@sent_messages = current_user.sent_messages.all
end
end
</pre>
Notice how authenticate_user! is used before checking the messages of the current user so as to ensure that the user is signed in before checking his/her messages.
*'''user_signed_in? :''' As the name suggests, user_signed_in? method is used to check whether a user is signed in. This is useful when you want to show two different pages depending on whether a user has logged in or not. For example, when a user is logged in you want to show him/her an option to Logout otherwise you want to show Register or Login options.
<pre>
<% if user_signed_in? %>
<li><%- link_to "Logout", destroy_user_session_path, method :delete %></li>
<% else %>
<li><%- link_to "Sign Up", new_user_registration_path %></li>
<li><%- link_to "Login", new_user_session_path %></li>
<% end %>
</pre>
We need to use method :delete to logout so that Devise will only logout when a HTTP delete request is made by user and does not accidentally logout because of a malicious link that automatically logs out the user.
*'''sign_in(@user) and sign_out(@user) :''' These methods are used to login(sign_in(@user)) or logout(sign_out(@user)) a newly created or existing user.
*'''user_session :''' This method returns metadata about the user that is currently logged in.
The methods that are most frequently used by developers are current_user and user_signed_in? which are present as helper methods. Also if the methods are to be referred to an Admin then replace user in each method with admin i.e. current_user becomes current_admin etc.

=='''See Also'''==
=='''References'''==

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-02-07T04:47:51Z

Skunapa:

{{Infobox software
| name =
| logo = 
| author =
| developer =
| released = 
| latest release version =
| latest release date = 
| status =
| programming language =
| operating system =
| license =
| website = {{URL|example.org}}
}}
Devise is a [https://en.wikipedia.org/wiki/Rack_(web_server_interface) rack] based full-fledged authentication system for Rails. It is a complete MVC solution meaning it can support various models, views and controllers as part of its code and can be used be directly by developers. Devise is simple to use and starts up with a couple of commands but it is also highly customizable. Devise saves a lot of time and effort as many applications require user registration and authentication mechanisms which are difficult to develop from scratch.

=='''History'''==
Devise is first introduced in January 2010 by Plataformatec, a company which builds web and mobile applications. Devise is one of the few authentication systems which support rack based applications and hence can support Rails 3 and up as they are completely rack based. The latest version of Devise available is v3.5.3 and it is up to date with Rails 5 beta 2.
===Warden===
Devise gem is built on top of a rack application called Warden which is used to verify the identity of logged in user using a session string. In Warden, the id which is a primary key of a user is somehow stored to match it later with the logged in user. Warden also provides restricted access to guest users depending on the functional requirements of an application. 
Since Warden does not know of the existence of the Rails application, it cannot provide any helper methods, controller classes, views etc. This is where Devise comes into play and can integrate with Rails seemlessly. Devise interacts often with Warden using Strategy design pattern for encrypting passwords, HTTP Authentication etc.

=='''Installation'''==
Although Devise is very useful and reduces the amount of effort in developing authentication mechanisms significantly, it requires a good understanding of the Rails framework. Hence it is advised for beginners to not use Devise. 
There are several commands that are required for the successful installation of Devise. They are listed out below:
*Add the devise gem to your gemfile.
gem 'devise'
*Run bundle command to install the gem.
bundle install
*You need to run the generator next which will install an initializer and creates all the configuration files.
rails generate devise:install
*Now you can add Devise to any of your models using the generator. This will create a class with the model name given and routes etc. The model will be configured with default Devise modules. The config/routes.rb file will be configured to point to the Devise controller.
rails generate devise user //Assuming that the model name is user
*Next, add any configuration changes that are required and then run:
rake db:migrate
*The following step will create Devise views but is optional. Devise has views for every generic operation like Login or SignUp and can be used directly instead of creating custom views.
rails generate devise:views users
* There are also many routes that are defined in config/routes.rb with a line like:
devise_for :users

=='''Modules'''==
When a devise generator is invoked a model class is created in app/models for you modify for your specific application requirements. This is the place where many important configuration changes are specified. Perhaps the most important are the Devise modules which provides essential functionalities like enhanced securtiy. 
There are 10 modules listed on the official page of Devise by Plataformatec. These modules are features that are contained in Devise and can be used by the developers depending on the use-cases or requirements of their application. Below is the list of modules:
*Database Authenticable
*Omniauthable
*Confirmable
*Recoverable
*Registerable
*Rememberable
*Trackable
*Timeoutable
*Validatable
*Lockable
Information regarding each module is listed in README as well as the [http://devise.plataformatec.com.br/ Plataformatec] website. The modules are included in an application in this way:
<pre>
class User < ActiveRecord::Base
devise :database_authenticable, :omniauthable, :confirmable, :rememberable,
:trackable, :timeoutable, :lockable
end
</pre>
In addition to the classes generated Devise also generates a database migration in which fields related to the functionalities of these modules are added. Each field is related to a specific module and hence when a module is not require some of the fields may be removed from the migration to the database. Also most of these modules have specific forms and view associated with them. The forms are used by an end user to type in his/her information which will then be sent to the Devise controllers.

=='''Methods'''==
There are many classes in Devise which include models, controllers, helpers, views, routes etc. But much of the functionality offered by Devise is exposed via simple helper methods. Some of the most important methods which can be used in building our own application are:
 
*'''authenticate_user! :''' This method is used to check whether a user is logged in before he/she attempts to perform a specific set of controller actions. authenticate_user! may be called with before_action as shown below to ensure the user is logged in before performing any of the operations.
before_action :authenticate_user!
If only some of the actions need authentication and some do not, we can use except clause so that only some actions are blocked as guest and others are accessible. The code with except clause is as below:
before_action :authenticate_user! except [:index, :show]
In the above example index and show are two controller actions associated with operations which do not require user authentication and can be browsed as a guest.Authenticate user may also be used with in a controller action so that it is application only for that specification. We need to use before_filter instead of before_action to achieve this purpose.
<pre>
class EndUserBaseController < ApplicationController
before_filter :authentication_user!
end
</pre>
In this example the application will authenticate a user only if he is trying to perform an action associated with the EndUserBaseController. If in any of the above cases a user is not logged in the application backs off and redirects to its sign-in page.
*'''current_user :''' current_user method is used to return the model class corresponding to the user who is currently signed in. For example, if you are building a messaging application, you may retrieve all the sent messages of a user as:
<pre>
class SentMessagesController < ApplicationController
before_filter :authentication_user!
def index
@sent_messages = current_user.sent_messages.all
end
end
</pre>
Notice how authenticate_user! is used before checking the messages of the current user so as to ensure that the user is signed in before checking his/her messages.
*'''user_signed_in? :''' As the name suggests, user_signed_in? method is used to check whether a user is signed in. This is useful when you want to show two different pages depending on whether a user has logged in or not. For example, when a user is logged in you want to show him/her an option to Logout otherwise you want to show Register or Login options.
<pre>
<% if user_signed_in? %>
<li><%- link_to "Logout", destroy_user_session_path, method :delete %></li>
<% else %>
<li><%- link_to "Sign Up", new_user_registration_path %></li>
<li><%- link_to "Login", new_user_session_path %></li>
<% end %>
</pre>
We need to use method :delete to logout so that Devise will only logout when a HTTP delete request is made by user and does not accidentally logout because of a malicious link that automatically logs out the user.
*'''sign_in(@user) and sign_out(@user) :''' These methods are used to login(sign_in(@user)) or logout(sign_out(@user)) a newly created or existing user.
*'''user_session :''' This method returns metadata about the user that is currently logged in.
The methods that are most frequently used by developers are current_user and user_signed_in? which are present as helper methods. Also if the methods are to be referred to an Admin then replace user in each method with admin i.e. current_user becomes current_admin etc.

=='''See Also'''==
=='''References'''==

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-02-07T04:45:46Z

Skunapa:

{{Infobox software
| name =
| logo = 
| author =
| developer =
| released = 
| latest release version =
| latest release date = 
| status =
| programming language =
| operating system =
| license =
| website = {{URL|example.org}}
}}
Devise is a [[Rack (web server interface)|rack]] based full-fledged authentication system for Rails. It is a complete MVC solution meaning it can support various models, views and controllers as part of its code and can be used be directly by developers. Devise is simple to use and starts up with a couple of commands but it is also highly customizable. Devise saves a lot of time and effort as many applications require user registration and authentication mechanisms which are difficult to develop from scratch.

=='''History'''==
Devise is first introduced in January 2010 by Plataformatec, a company which builds web and mobile applications. Devise is one of the few authentication systems which support rack based applications and hence can support Rails 3 and up as they are completely rack based. The latest version of Devise available is v3.5.3 and it is up to date with Rails 5 beta 2.
===Warden===
Devise gem is built on top of a rack application called Warden which is used to verify the identity of logged in user using a session string. In Warden, the id which is a primary key of a user is somehow stored to match it later with the logged in user. Warden also provides restricted access to guest users depending on the functional requirements of an application. 
Since Warden does not know of the existence of the Rails application, it cannot provide any helper methods, controller classes, views etc. This is where Devise comes into play and can integrate with Rails seemlessly. Devise interacts often with Warden using Strategy design pattern for encrypting passwords, HTTP Authentication etc.

=='''Installation'''==
Although Devise is very useful and reduces the amount of effort in developing authentication mechanisms significantly, it requires a good understanding of the Rails framework. Hence it is advised for beginners to not use Devise. 
There are several commands that are required for the successful installation of Devise. They are listed out below:
*Add the devise gem to your gemfile.
gem 'devise'
*Run bundle command to install the gem.
bundle install
*You need to run the generator next which will install an initializer and creates all the configuration files.
rails generate devise:install
*Now you can add Devise to any of your models using the generator. This will create a class with the model name given and routes etc. The model will be configured with default Devise modules. The config/routes.rb file will be configured to point to the Devise controller.
rails generate devise user //Assuming that the model name is user
*Next, add any configuration changes that are required and then run:
rake db:migrate
*The following step will create Devise views but is optional. Devise has views for every generic operation like Login or SignUp and can be used directly instead of creating custom views.
rails generate devise:views users
* There are also many routes that are defined in config/routes.rb with a line like:
devise_for :users

=='''Modules'''==
When a devise generator is invoked a model class is created in app/models for you modify for your specific application requirements. This is the place where many important configuration changes are specified. Perhaps the most important are the Devise modules which provides essential functionalities like enhanced securtiy. 
There are 10 modules listed on the official page of Devise by Plataformatec. These modules are features that are contained in Devise and can be used by the developers depending on the use-cases or requirements of their application. Below is the list of modules:
*Database Authenticable
*Omniauthable
*Confirmable
*Recoverable
*Registerable
*Rememberable
*Trackable
*Timeoutable
*Validatable
*Lockable
Information regarding each module is listed in README as well as the [http://devise.plataformatec.com.br/ Plataformatec] website. The modules are included in an application in this way:
<pre>
class User < ActiveRecord::Base
devise :database_authenticable, :omniauthable, :confirmable, :rememberable,
:trackable, :timeoutable, :lockable
end
</pre>
In addition to the classes generated Devise also generates a database migration in which fields related to the functionalities of these modules are added. Each field is related to a specific module and hence when a module is not require some of the fields may be removed from the migration to the database. Also most of these modules have specific forms and view associated with them. The forms are used by an end user to type in his/her information which will then be sent to the Devise controllers.

=='''Methods'''==
There are many classes in Devise which include models, controllers, helpers, views, routes etc. But much of the functionality offered by Devise is exposed via simple helper methods. Some of the most important methods which can be used in building our own application are:
 
*'''authenticate_user! :''' This method is used to check whether a user is logged in before he/she attempts to perform a specific set of controller actions. authenticate_user! may be called with before_action as shown below to ensure the user is logged in before performing any of the operations.
before_action :authenticate_user!
If only some of the actions need authentication and some do not, we can use except clause so that only some actions are blocked as guest and others are accessible. The code with except clause is as below:
before_action :authenticate_user! except [:index, :show]
In the above example index and show are two controller actions associated with operations which do not require user authentication and can be browsed as a guest.Authenticate user may also be used with in a controller action so that it is application only for that specification. We need to use before_filter instead of before_action to achieve this purpose.
<pre>
class EndUserBaseController < ApplicationController
before_filter :authentication_user!
end
</pre>
In this example the application will authenticate a user only if he is trying to perform an action associated with the EndUserBaseController. If in any of the above cases a user is not logged in the application backs off and redirects to its sign-in page.
*'''current_user :''' current_user method is used to return the model class corresponding to the user who is currently signed in. For example, if you are building a messaging application, you may retrieve all the sent messages of a user as:
<pre>
class SentMessagesController < ApplicationController
before_filter :authentication_user!
def index
@sent_messages = current_user.sent_messages.all
end
end
</pre>
Notice how authenticate_user! is used before checking the messages of the current user so as to ensure that the user is signed in before checking his/her messages.
*'''user_signed_in? :''' As the name suggests, user_signed_in? method is used to check whether a user is signed in. This is useful when you want to show two different pages depending on whether a user has logged in or not. For example, when a user is logged in you want to show him/her an option to Logout otherwise you want to show Register or Login options.
<pre>
<% if user_signed_in? %>
<li><%- link_to "Logout", destroy_user_session_path, method :delete %></li>
<% else %>
<li><%- link_to "Sign Up", new_user_registration_path %></li>
<li><%- link_to "Login", new_user_session_path %></li>
<% end %>
</pre>
We need to use method :delete to logout so that Devise will only logout when a HTTP delete request is made by user and does not accidentally logout because of a malicious link that automatically logs out the user.
*'''sign_in(@user) and sign_out(@user) :''' These methods are used to login(sign_in(@user)) or logout(sign_out(@user)) a newly created or existing user.
*'''user_session :''' This method returns metadata about the user that is currently logged in.
The methods that are most frequently used by developers are current_user and user_signed_in? which are present as helper methods. Also if the methods are to be referred to an Admin then replace user in each method with admin i.e. current_user becomes current_admin etc.

=='''See Also'''==
=='''References'''==

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-02-07T04:44:33Z

Skunapa:

{{Infobox software
| name =
| logo = 
| author =
| developer =
| released = 
| latest release version =
| latest release date = 
| status =
| programming language =
| operating system =
| license =
| website = {{URL|example.org}}
}}
Devise is a [[Rack_(web_server_interface)|rack]] based full-fledged authentication system for Rails. It is a complete MVC solution meaning it can support various models, views and controllers as part of its code and can be used be directly by developers. Devise is simple to use and starts up with a couple of commands but it is also highly customizable. Devise saves a lot of time and effort as many applications require user registration and authentication mechanisms which are difficult to develop from scratch.

=='''History'''==
Devise is first introduced in January 2010 by Plataformatec, a company which builds web and mobile applications. Devise is one of the few authentication systems which support rack based applications and hence can support Rails 3 and up as they are completely rack based. The latest version of Devise available is v3.5.3 and it is up to date with Rails 5 beta 2.
===Warden===
Devise gem is built on top of a rack application called Warden which is used to verify the identity of logged in user using a session string. In Warden, the id which is a primary key of a user is somehow stored to match it later with the logged in user. Warden also provides restricted access to guest users depending on the functional requirements of an application. 
Since Warden does not know of the existence of the Rails application, it cannot provide any helper methods, controller classes, views etc. This is where Devise comes into play and can integrate with Rails seemlessly. Devise interacts often with Warden using Strategy design pattern for encrypting passwords, HTTP Authentication etc.

=='''Installation'''==
Although Devise is very useful and reduces the amount of effort in developing authentication mechanisms significantly, it requires a good understanding of the Rails framework. Hence it is advised for beginners to not use Devise. 
There are several commands that are required for the successful installation of Devise. They are listed out below:
*Add the devise gem to your gemfile.
gem 'devise'
*Run bundle command to install the gem.
bundle install
*You need to run the generator next which will install an initializer and creates all the configuration files.
rails generate devise:install
*Now you can add Devise to any of your models using the generator. This will create a class with the model name given and routes etc. The model will be configured with default Devise modules. The config/routes.rb file will be configured to point to the Devise controller.
rails generate devise user //Assuming that the model name is user
*Next, add any configuration changes that are required and then run:
rake db:migrate
*The following step will create Devise views but is optional. Devise has views for every generic operation like Login or SignUp and can be used directly instead of creating custom views.
rails generate devise:views users
* There are also many routes that are defined in config/routes.rb with a line like:
devise_for :users

=='''Modules'''==
When a devise generator is invoked a model class is created in app/models for you modify for your specific application requirements. This is the place where many important configuration changes are specified. Perhaps the most important are the Devise modules which provides essential functionalities like enhanced securtiy. 
There are 10 modules listed on the official page of Devise by Plataformatec. These modules are features that are contained in Devise and can be used by the developers depending on the use-cases or requirements of their application. Below is the list of modules:
*Database Authenticable
*Omniauthable
*Confirmable
*Recoverable
*Registerable
*Rememberable
*Trackable
*Timeoutable
*Validatable
*Lockable
Information regarding each module is listed in README as well as the [http://devise.plataformatec.com.br/ Plataformatec] website. The modules are included in an application in this way:
<pre>
class User < ActiveRecord::Base
devise :database_authenticable, :omniauthable, :confirmable, :rememberable,
:trackable, :timeoutable, :lockable
end
</pre>
In addition to the classes generated Devise also generates a database migration in which fields related to the functionalities of these modules are added. Each field is related to a specific module and hence when a module is not require some of the fields may be removed from the migration to the database. Also most of these modules have specific forms and view associated with them. The forms are used by an end user to type in his/her information which will then be sent to the Devise controllers.

=='''Methods'''==
There are many classes in Devise which include models, controllers, helpers, views, routes etc. But much of the functionality offered by Devise is exposed via simple helper methods. Some of the most important methods which can be used in building our own application are:
 
*'''authenticate_user! :''' This method is used to check whether a user is logged in before he/she attempts to perform a specific set of controller actions. authenticate_user! may be called with before_action as shown below to ensure the user is logged in before performing any of the operations.
before_action :authenticate_user!
If only some of the actions need authentication and some do not, we can use except clause so that only some actions are blocked as guest and others are accessible. The code with except clause is as below:
before_action :authenticate_user! except [:index, :show]
In the above example index and show are two controller actions associated with operations which do not require user authentication and can be browsed as a guest.Authenticate user may also be used with in a controller action so that it is application only for that specification. We need to use before_filter instead of before_action to achieve this purpose.
<pre>
class EndUserBaseController < ApplicationController
before_filter :authentication_user!
end
</pre>
In this example the application will authenticate a user only if he is trying to perform an action associated with the EndUserBaseController. If in any of the above cases a user is not logged in the application backs off and redirects to its sign-in page.
*'''current_user :''' current_user method is used to return the model class corresponding to the user who is currently signed in. For example, if you are building a messaging application, you may retrieve all the sent messages of a user as:
<pre>
class SentMessagesController < ApplicationController
before_filter :authentication_user!
def index
@sent_messages = current_user.sent_messages.all
end
end
</pre>
Notice how authenticate_user! is used before checking the messages of the current user so as to ensure that the user is signed in before checking his/her messages.
*'''user_signed_in? :''' As the name suggests, user_signed_in? method is used to check whether a user is signed in. This is useful when you want to show two different pages depending on whether a user has logged in or not. For example, when a user is logged in you want to show him/her an option to Logout otherwise you want to show Register or Login options.
<pre>
<% if user_signed_in? %>
<li><%- link_to "Logout", destroy_user_session_path, method :delete %></li>
<% else %>
<li><%- link_to "Sign Up", new_user_registration_path %></li>
<li><%- link_to "Login", new_user_session_path %></li>
<% end %>
</pre>
We need to use method :delete to logout so that Devise will only logout when a HTTP delete request is made by user and does not accidentally logout because of a malicious link that automatically logs out the user.
*'''sign_in(@user) and sign_out(@user) :''' These methods are used to login(sign_in(@user)) or logout(sign_out(@user)) a newly created or existing user.
*'''user_session :''' This method returns metadata about the user that is currently logged in.
The methods that are most frequently used by developers are current_user and user_signed_in? which are present as helper methods. Also if the methods are to be referred to an Admin then replace user in each method with admin i.e. current_user becomes current_admin etc.

=='''See Also'''==
=='''References'''==

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-02-07T04:39:43Z

Skunapa:

{{Infobox software
| name =
| logo = 
| author =
| developer =
| released = 
| latest release version =
| latest release date = 
| status =
| programming language =
| operating system =
| license =
| website = {{URL|example.org}}
}}
Devise is a rack based full-fledged authentication system for Rails. It is a complete MVC solution meaning it can support various models, views and controllers as part of its code and can be used be directly by developers. Devise is simple to use and starts up with a couple of commands but it is also highly customizable. Devise saves a lot of time and effort as many applications require user registration and authentication mechanisms which are difficult to develop from scratch.

=='''History'''==
Devise is first introduced in January 2010 by Plataformatec, a company which builds web and mobile applications. Devise is one of the few authentication systems which support rack based applications and hence can support Rails 3 and up as they are completely rack based. The latest version of Devise available is v3.5.3 and it is up to date with Rails 5 beta 2.
===Warden===
Devise gem is built on top of a rack application called Warden which is used to verify the identity of logged in user using a session string. In Warden, the id which is a primary key of a user is somehow stored to match it later with the logged in user. Warden also provides restricted access to guest users depending on the functional requirements of an application. 
Since Warden does not know of the existence of the Rails application, it cannot provide any helper methods, controller classes, views etc. This is where Devise comes into play and can integrate with Rails seemlessly. Devise interacts often with Warden using Strategy design pattern for encrypting passwords, HTTP Authentication etc.

=='''Installation'''==
Although Devise is very useful and reduces the amount of effort in developing authentication mechanisms significantly, it requires a good understanding of the Rails framework. Hence it is advised for beginners to not use Devise. 
There are several commands that are required for the successful installation of Devise. They are listed out below:
*Add the devise gem to your gemfile.
gem 'devise'
*Run bundle command to install the gem.
bundle install
*You need to run the generator next which will install an initializer and creates all the configuration files.
rails generate devise:install
*Now you can add Devise to any of your models using the generator. This will create a class with the model name given and routes etc. The model will be configured with default Devise modules. The config/routes.rb file will be configured to point to the Devise controller.
rails generate devise user //Assuming that the model name is user
*Next, add any configuration changes that are required and then run:
rake db:migrate
*The following step will create Devise views but is optional. Devise has views for every generic operation like Login or SignUp and can be used directly instead of creating custom views.
rails generate devise:views users
* There are also many routes that are defined in config/routes.rb with a line like:
devise_for :users

=='''Modules'''==
When a devise generator is invoked a model class is created in app/models for you modify for your specific application requirements. This is the place where many important configuration changes are specified. Perhaps the most important are the Devise modules which provides essential functionalities like enhanced securtiy. 
There are 10 modules listed on the official page of Devise by Plataformatec. These modules are features that are contained in Devise and can be used by the developers depending on the use-cases or requirements of their application. Below is the list of modules:
*Database Authenticable
*Omniauthable
*Confirmable
*Recoverable
*Registerable
*Rememberable
*Trackable
*Timeoutable
*Validatable
*Lockable
Information regarding each module is listed in README as well as the [http://devise.plataformatec.com.br/ Plataformatec] website. The modules are included in an application in this way:
<pre>
class User < ActiveRecord::Base
devise :database_authenticable, :omniauthable, :confirmable, :rememberable,
:trackable, :timeoutable, :lockable
end
</pre>
In addition to the classes generated Devise also generates a database migration in which fields related to the functionalities of these modules are added. Each field is related to a specific module and hence when a module is not require some of the fields may be removed from the migration to the database. Also most of these modules have specific forms and view associated with them. The forms are used by an end user to type in his/her information which will then be sent to the Devise controllers.

=='''Methods'''==
There are many classes in Devise which include models, controllers, helpers, views, routes etc. But much of the functionality offered by Devise is exposed via simple helper methods. Some of the most important methods which can be used in building our own application are:
 
*'''authenticate_user! :''' This method is used to check whether a user is logged in before he/she attempts to perform a specific set of controller actions. authenticate_user! may be called with before_action as shown below to ensure the user is logged in before performing any of the operations.
before_action :authenticate_user!
If only some of the actions need authentication and some do not, we can use except clause so that only some actions are blocked as guest and others are accessible. The code with except clause is as below:
before_action :authenticate_user! except [:index, :show]
In the above example index and show are two controller actions associated with operations which do not require user authentication and can be browsed as a guest.Authenticate user may also be used with in a controller action so that it is application only for that specification. We need to use before_filter instead of before_action to achieve this purpose.
<pre>
class EndUserBaseController < ApplicationController
before_filter :authentication_user!
end
</pre>
In this example the application will authenticate a user only if he is trying to perform an action associated with the EndUserBaseController. If in any of the above cases a user is not logged in the application backs off and redirects to its sign-in page.
*'''current_user :''' current_user method is used to return the model class corresponding to the user who is currently signed in. For example, if you are building a messaging application, you may retrieve all the sent messages of a user as:
<pre>
class SentMessagesController < ApplicationController
before_filter :authentication_user!
def index
@sent_messages = current_user.sent_messages.all
end
end
</pre>
Notice how authenticate_user! is used before checking the messages of the current user so as to ensure that the user is signed in before checking his/her messages.
*'''user_signed_in? :''' As the name suggests, user_signed_in? method is used to check whether a user is signed in. This is useful when you want to show two different pages depending on whether a user has logged in or not. For example, when a user is logged in you want to show him/her an option to Logout otherwise you want to show Register or Login options.
<pre>
<% if user_signed_in? %>
<li><%- link_to "Logout", destroy_user_session_path, method :delete %></li>
<% else %>
<li><%- link_to "Sign Up", new_user_registration_path %></li>
<li><%- link_to "Login", new_user_session_path %></li>
<% end %>
</pre>
We need to use method :delete to logout so that Devise will only logout when a HTTP delete request is made by user and does not accidentally logout because of a malicious link that automatically logs out the user.
*'''sign_in(@user) and sign_out(@user) :''' These methods are used to login(sign_in(@user)) or logout(sign_out(@user)) a newly created or existing user.
*'''user_session :''' This method returns metadata about the user that is currently logged in.
The methods that are most frequently used by developers are current_user and user_signed_in? which are present as helper methods. Also if the methods are to be referred to an Admin then replace user in each method with admin i.e. current_user becomes current_admin etc.

=='''See Also'''==
=='''References'''==

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-02-07T04:37:27Z

Skunapa:

{{Infobox software
| name =
| logo = 
| logo alt =
| screenshot = 
| caption =
| screenshot alt =
| collapsible =
| author =
| developer =
| released = 
| discontinued =
| latest release version =
| latest release date = 
| latest preview version =
| latest preview date = 
| status =
| programming language =
| operating system =
| platform =
| size =
| language =
| language count = 
| language footnote =
| genre =
| license =
| alexa =
| website = {{URL|example.org}}
| standard =
| AsOf =
}}
Devise is a rack based full-fledged authentication system for Rails. It is a complete MVC solution meaning it can support various models, views and controllers as part of its code and can be used be directly by developers. Devise is simple to use and starts up with a couple of commands but it is also highly customizable. Devise saves a lot of time and effort as many applications require user registration and authentication mechanisms which are difficult to develop from scratch.

=='''History'''==
Devise is first introduced in January 2010 by Plataformatec, a company which builds web and mobile applications. Devise is one of the few authentication systems which support rack based applications and hence can support Rails 3 and up as they are completely rack based. The latest version of Devise available is v3.5.3 and it is up to date with Rails 5 beta 2.
===Warden===
Devise gem is built on top of a rack application called Warden which is used to verify the identity of logged in user using a session string. In Warden, the id which is a primary key of a user is somehow stored to match it later with the logged in user. Warden also provides restricted access to guest users depending on the functional requirements of an application. 
Since Warden does not know of the existence of the Rails application, it cannot provide any helper methods, controller classes, views etc. This is where Devise comes into play and can integrate with Rails seemlessly. Devise interacts often with Warden using Strategy design pattern for encrypting passwords, HTTP Authentication etc.

=='''Installation'''==
Although Devise is very useful and reduces the amount of effort in developing authentication mechanisms significantly, it requires a good understanding of the Rails framework. Hence it is advised for beginners to not use Devise. 
There are several commands that are required for the successful installation of Devise. They are listed out below:
*Add the devise gem to your gemfile.
gem 'devise'
*Run bundle command to install the gem.
bundle install
*You need to run the generator next which will install an initializer and creates all the configuration files.
rails generate devise:install
*Now you can add Devise to any of your models using the generator. This will create a class with the model name given and routes etc. The model will be configured with default Devise modules. The config/routes.rb file will be configured to point to the Devise controller.
rails generate devise user //Assuming that the model name is user
*Next, add any configuration changes that are required and then run:
rake db:migrate
*The following step will create Devise views but is optional. Devise has views for every generic operation like Login or SignUp and can be used directly instead of creating custom views.
rails generate devise:views users
* There are also many routes that are defined in config/routes.rb with a line like:
devise_for :users

=='''Modules'''==
When a devise generator is invoked a model class is created in app/models for you modify for your specific application requirements. This is the place where many important configuration changes are specified. Perhaps the most important are the Devise modules which provides essential functionalities like enhanced securtiy. 
There are 10 modules listed on the official page of Devise by Plataformatec. These modules are features that are contained in Devise and can be used by the developers depending on the use-cases or requirements of their application. Below is the list of modules:
*Database Authenticable
*Omniauthable
*Confirmable
*Recoverable
*Registerable
*Rememberable
*Trackable
*Timeoutable
*Validatable
*Lockable
Information regarding each module is listed in README as well as the [http://devise.plataformatec.com.br/ Plataformatec] website. The modules are included in an application in this way:
<pre>
class User < ActiveRecord::Base
devise :database_authenticable, :omniauthable, :confirmable, :rememberable,
:trackable, :timeoutable, :lockable
end
</pre>
In addition to the classes generated Devise also generates a database migration in which fields related to the functionalities of these modules are added. Each field is related to a specific module and hence when a module is not require some of the fields may be removed from the migration to the database. Also most of these modules have specific forms and view associated with them. The forms are used by an end user to type in his/her information which will then be sent to the Devise controllers.

=='''Methods'''==
There are many classes in Devise which include models, controllers, helpers, views, routes etc. But much of the functionality offered by Devise is exposed via simple helper methods. Some of the most important methods which can be used in building our own application are:
 
*'''authenticate_user! :''' This method is used to check whether a user is logged in before he/she attempts to perform a specific set of controller actions. authenticate_user! may be called with before_action as shown below to ensure the user is logged in before performing any of the operations.
before_action :authenticate_user!
If only some of the actions need authentication and some do not, we can use except clause so that only some actions are blocked as guest and others are accessible. The code with except clause is as below:
before_action :authenticate_user! except [:index, :show]
In the above example index and show are two controller actions associated with operations which do not require user authentication and can be browsed as a guest.Authenticate user may also be used with in a controller action so that it is application only for that specification. We need to use before_filter instead of before_action to achieve this purpose.
<pre>
class EndUserBaseController < ApplicationController
before_filter :authentication_user!
end
</pre>
In this example the application will authenticate a user only if he is trying to perform an action associated with the EndUserBaseController. If in any of the above cases a user is not logged in the application backs off and redirects to its sign-in page.
*'''current_user :''' current_user method is used to return the model class corresponding to the user who is currently signed in. For example, if you are building a messaging application, you may retrieve all the sent messages of a user as:
<pre>
class SentMessagesController < ApplicationController
before_filter :authentication_user!
def index
@sent_messages = current_user.sent_messages.all
end
end
</pre>
Notice how authenticate_user! is used before checking the messages of the current user so as to ensure that the user is signed in before checking his/her messages.
*'''user_signed_in? :''' As the name suggests, user_signed_in? method is used to check whether a user is signed in. This is useful when you want to show two different pages depending on whether a user has logged in or not. For example, when a user is logged in you want to show him/her an option to Logout otherwise you want to show Register or Login options.
<pre>
<% if user_signed_in? %>
<li><%- link_to "Logout", destroy_user_session_path, method :delete %></li>
<% else %>
<li><%- link_to "Sign Up", new_user_registration_path %></li>
<li><%- link_to "Login", new_user_session_path %></li>
<% end %>
</pre>
We need to use method :delete to logout so that Devise will only logout when a HTTP delete request is made by user and does not accidentally logout because of a malicious link that automatically logs out the user.
*'''sign_in(@user) and sign_out(@user) :''' These methods are used to login(sign_in(@user)) or logout(sign_out(@user)) a newly created or existing user.
*'''user_session :''' This method returns metadata about the user that is currently logged in.
The methods that are most frequently used by developers are current_user and user_signed_in? which are present as helper methods. Also if the methods are to be referred to an Admin then replace user in each method with admin i.e. current_user becomes current_admin etc.

=='''See Also'''==
=='''References'''==

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-02-07T04:35:36Z

Skunapa:

Devise is a rack based full-fledged authentication system for Rails. It is a complete MVC solution meaning it can support various models, views and controllers as part of its code and can be used be directly by developers. Devise is simple to use and starts up with a couple of commands but it is also highly customizable. Devise saves a lot of time and effort as many applications require user registration and authentication mechanisms which are difficult to develop from scratch.

=='''History'''==
Devise is first introduced in January 2010 by Plataformatec, a company which builds web and mobile applications. Devise is one of the few authentication systems which support rack based applications and hence can support Rails 3 and up as they are completely rack based. The latest version of Devise available is v3.5.3 and it is up to date with Rails 5 beta 2.
===Warden===
Devise gem is built on top of a rack application called Warden which is used to verify the identity of logged in user using a session string. In Warden, the id which is a primary key of a user is somehow stored to match it later with the logged in user. Warden also provides restricted access to guest users depending on the functional requirements of an application. 
Since Warden does not know of the existence of the Rails application, it cannot provide any helper methods, controller classes, views etc. This is where Devise comes into play and can integrate with Rails seemlessly. Devise interacts often with Warden using Strategy design pattern for encrypting passwords, HTTP Authentication etc.

=='''Installation'''==
Although Devise is very useful and reduces the amount of effort in developing authentication mechanisms significantly, it requires a good understanding of the Rails framework. Hence it is advised for beginners to not use Devise. 
There are several commands that are required for the successful installation of Devise. They are listed out below:
*Add the devise gem to your gemfile.
gem 'devise'
*Run bundle command to install the gem.
bundle install
*You need to run the generator next which will install an initializer and creates all the configuration files.
rails generate devise:install
*Now you can add Devise to any of your models using the generator. This will create a class with the model name given and routes etc. The model will be configured with default Devise modules. The config/routes.rb file will be configured to point to the Devise controller.
rails generate devise user //Assuming that the model name is user
*Next, add any configuration changes that are required and then run:
rake db:migrate
*The following step will create Devise views but is optional. Devise has views for every generic operation like Login or SignUp and can be used directly instead of creating custom views.
rails generate devise:views users
* There are also many routes that are defined in config/routes.rb with a line like:
devise_for :users

=='''Modules'''==
When a devise generator is invoked a model class is created in app/models for you modify for your specific application requirements. This is the place where many important configuration changes are specified. Perhaps the most important are the Devise modules which provides essential functionalities like enhanced securtiy. 
There are 10 modules listed on the official page of Devise by Plataformatec. These modules are features that are contained in Devise and can be used by the developers depending on the use-cases or requirements of their application. Below is the list of modules:
*Database Authenticable
*Omniauthable
*Confirmable
*Recoverable
*Registerable
*Rememberable
*Trackable
*Timeoutable
*Validatable
*Lockable
Information regarding each module is listed in README as well as the [http://devise.plataformatec.com.br/ Plataformatec] website. The modules are included in an application in this way:
<pre>
class User < ActiveRecord::Base
devise :database_authenticable, :omniauthable, :confirmable, :rememberable,
:trackable, :timeoutable, :lockable
end
</pre>
In addition to the classes generated Devise also generates a database migration in which fields related to the functionalities of these modules are added. Each field is related to a specific module and hence when a module is not require some of the fields may be removed from the migration to the database. Also most of these modules have specific forms and view associated with them. The forms are used by an end user to type in his/her information which will then be sent to the Devise controllers.

=='''Methods'''==
There are many classes in Devise which include models, controllers, helpers, views, routes etc. But much of the functionality offered by Devise is exposed via simple helper methods. Some of the most important methods which can be used in building our own application are:
 
*'''authenticate_user! :''' This method is used to check whether a user is logged in before he/she attempts to perform a specific set of controller actions. authenticate_user! may be called with before_action as shown below to ensure the user is logged in before performing any of the operations.
before_action :authenticate_user!
If only some of the actions need authentication and some do not, we can use except clause so that only some actions are blocked as guest and others are accessible. The code with except clause is as below:
before_action :authenticate_user! except [:index, :show]
In the above example index and show are two controller actions associated with operations which do not require user authentication and can be browsed as a guest.Authenticate user may also be used with in a controller action so that it is application only for that specification. We need to use before_filter instead of before_action to achieve this purpose.
<pre>
class EndUserBaseController < ApplicationController
before_filter :authentication_user!
end
</pre>
In this example the application will authenticate a user only if he is trying to perform an action associated with the EndUserBaseController. If in any of the above cases a user is not logged in the application backs off and redirects to its sign-in page.
*'''current_user :''' current_user method is used to return the model class corresponding to the user who is currently signed in. For example, if you are building a messaging application, you may retrieve all the sent messages of a user as:
<pre>
class SentMessagesController < ApplicationController
before_filter :authentication_user!
def index
@sent_messages = current_user.sent_messages.all
end
end
</pre>
Notice how authenticate_user! is used before checking the messages of the current user so as to ensure that the user is signed in before checking his/her messages.
*'''user_signed_in? :''' As the name suggests, user_signed_in? method is used to check whether a user is signed in. This is useful when you want to show two different pages depending on whether a user has logged in or not. For example, when a user is logged in you want to show him/her an option to Logout otherwise you want to show Register or Login options.
<pre>
<% if user_signed_in? %>
<li><%- link_to "Logout", destroy_user_session_path, method :delete %></li>
<% else %>
<li><%- link_to "Sign Up", new_user_registration_path %></li>
<li><%- link_to "Login", new_user_session_path %></li>
<% end %>
</pre>
We need to use method :delete to logout so that Devise will only logout when a HTTP delete request is made by user and does not accidentally logout because of a malicious link that automatically logs out the user.
*'''sign_in(@user) and sign_out(@user) :''' These methods are used to login(sign_in(@user)) or logout(sign_out(@user)) a newly created or existing user.
*'''user_session :''' This method returns metadata about the user that is currently logged in.
The methods that are most frequently used by developers are current_user and user_signed_in? which are present as helper methods. Also if the methods are to be referred to an Admin then replace user in each method with admin i.e. current_user becomes current_admin etc.

=='''See Also'''==
=='''References'''==

CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb

2016-02-07T04:35:21Z

Skunapa:

{{Devise}}
Devise is a rack based full-fledged authentication system for Rails. It is a complete MVC solution meaning it can support various models, views and controllers as part of its code and can be used be directly by developers. Devise is simple to use and starts up with a couple of commands but it is also highly customizable. Devise saves a lot of time and effort as many applications require user registration and authentication mechanisms which are difficult to develop from scratch.

=='''History'''==
Devise is first introduced in January 2010 by Plataformatec, a company which builds web and mobile applications. Devise is one of the few authentication systems which support rack based applications and hence can support Rails 3 and up as they are completely rack based. The latest version of Devise available is v3.5.3 and it is up to date with Rails 5 beta 2.
===Warden===
Devise gem is built on top of a rack application called Warden which is used to verify the identity of logged in user using a session string. In Warden, the id which is a primary key of a user is somehow stored to match it later with the logged in user. Warden also provides restricted access to guest users depending on the functional requirements of an application. 
Since Warden does not know of the existence of the Rails application, it cannot provide any helper methods, controller classes, views etc. This is where Devise comes into play and can integrate with Rails seemlessly. Devise interacts often with Warden using Strategy design pattern for encrypting passwords, HTTP Authentication etc.

=='''Installation'''==
Although Devise is very useful and reduces the amount of effort in developing authentication mechanisms significantly, it requires a good understanding of the Rails framework. Hence it is advised for beginners to not use Devise. 
There are several commands that are required for the successful installation of Devise. They are listed out below:
*Add the devise gem to your gemfile.
gem 'devise'
*Run bundle command to install the gem.
bundle install
*You need to run the generator next which will install an initializer and creates all the configuration files.
rails generate devise:install
*Now you can add Devise to any of your models using the generator. This will create a class with the model name given and routes etc. The model will be configured with default Devise modules. The config/routes.rb file will be configured to point to the Devise controller.
rails generate devise user //Assuming that the model name is user
*Next, add any configuration changes that are required and then run:
rake db:migrate
*The following step will create Devise views but is optional. Devise has views for every generic operation like Login or SignUp and can be used directly instead of creating custom views.
rails generate devise:views users
* There are also many routes that are defined in config/routes.rb with a line like:
devise_for :users

=='''Modules'''==
When a devise generator is invoked a model class is created in app/models for you modify for your specific application requirements. This is the place where many important configuration changes are specified. Perhaps the most important are the Devise modules which provides essential functionalities like enhanced securtiy. 
There are 10 modules listed on the official page of Devise by Plataformatec. These modules are features that are contained in Devise and can be used by the developers depending on the use-cases or requirements of their application. Below is the list of modules:
*Database Authenticable
*Omniauthable
*Confirmable
*Recoverable
*Registerable
*Rememberable
*Trackable
*Timeoutable
*Validatable
*Lockable
Information regarding each module is listed in README as well as the [http://devise.plataformatec.com.br/ Plataformatec] website. The modules are included in an application in this way:
<pre>
class User < ActiveRecord::Base
devise :database_authenticable, :omniauthable, :confirmable, :rememberable,
:trackable, :timeoutable, :lockable
end
</pre>
In addition to the classes generated Devise also generates a database migration in which fields related to the functionalities of these modules are added. Each field is related to a specific module and hence when a module is not require some of the fields may be removed from the migration to the database. Also most of these modules have specific forms and view associated with them. The forms are used by an end user to type in his/her information which will then be sent to the Devise controllers.

=='''Methods'''==
There are many classes in Devise which include models, controllers, helpers, views, routes etc. But much of the functionality offered by Devise is exposed via simple helper methods. Some of the most important methods which can be used in building our own application are:
 
*'''authenticate_user! :''' This method is used to check whether a user is logged in before he/she attempts to perform a specific set of controller actions. authenticate_user! may be called with before_action as shown below to ensure the user is logged in before performing any of the operations.
before_action :authenticate_user!
If only some of the actions need authentication and some do not, we can use except clause so that only some actions are blocked as guest and others are accessible. The code with except clause is as below:
before_action :authenticate_user! except [:index, :show]
In the above example index and show are two controller actions associated with operations which do not require user authentication and can be browsed as a guest.Authenticate user may also be used with in a controller action so that it is application only for that specification. We need to use before_filter instead of before_action to achieve this purpose.
<pre>
class EndUserBaseController < ApplicationController
before_filter :authentication_user!
end
</pre>
In this example the application will authenticate a user only if he is trying to perform an action associated with the EndUserBaseController. If in any of the above cases a user is not logged in the application backs off and redirects to its sign-in page.
*'''current_user :''' current_user method is used to return the model class corresponding to the user who is currently signed in. For example, if you are building a messaging application, you may retrieve all the sent messages of a user as:
<pre>
class SentMessagesController < ApplicationController
before_filter :authentication_user!
def index
@sent_messages = current_user.sent_messages.all
end
end
</pre>
Notice how authenticate_user! is used before checking the messages of the current user so as to ensure that the user is signed in before checking his/her messages.
*'''user_signed_in? :''' As the name suggests, user_signed_in? method is used to check whether a user is signed in. This is useful when you want to show two different pages depending on whether a user has logged in or not. For example, when a user is logged in you want to show him/her an option to Logout otherwise you want to show Register or Login options.
<pre>
<% if user_signed_in? %>
<li><%- link_to "Logout", destroy_user_session_path, method :delete %></li>
<% else %>
<li><%- link_to "Sign Up", new_user_registration_path %></li>
<li><%- link_to "Login", new_user_session_path %></li>
<% end %>
</pre>
We need to use method :delete to logout so that Devise will only logout when a HTTP delete request is made by user and does not accidentally logout because of a malicious link that automatically logs out the user.
*'''sign_in(@user) and sign_out(@user) :''' These methods are used to login(sign_in(@user)) or logout(sign_out(@user)) a newly created or existing user.
*'''user_session :''' This method returns metadata about the user that is currently logged in.
The methods that are most frequently used by developers are current_user and user_signed_in? which are present as helper methods. Also if the methods are to be referred to an Admin then replace user in each method with admin i.e. current_user becomes current_admin etc.

=='''See Also'''==
=='''References'''==