Expertiza_Wiki - User contributions [en]

CSC/ECE 517 Fall 2019 - E1944. Refactor review mapping controller

2019-11-18T17:52:33Z

Ysoodin: /* Links for Demo Videos */

This wiki page is for the description of changes made under E1944 OSS assignment for Fall 2019, CSC/ECE 517.
sd
== About 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.

== Description of the project ==

The focus of the project is on a controller named ReviewMappingController and the primary goal is to make changes to the internal structure of the controller to make it easier to read and cheaper to maintain without changing its observable behavior. This can be achieved through refactoring some of the more complex methods, modifying some of the language to make it more Ruby friendly, removing redundant code, etc.

The number of routes directed to review_mapping_controller is illustrated here and since it is intertwined with a lot of other parts in the code base, it is quite imperative that this code needs to be refactored.

Link to the Pull Request Submitted: [https://github.com/expertiza/expertiza/pull/1538]

Link to the Repository [https://github.com/VivekReddy98/expertiza/]

[[File:complexity.png]]

== Work-Plan Followed ==
[[File:FlowchartOODD.jpg]]
 
We were tasked to refactor the review_mapping_controller.rb and solve any cascading issues or bugs we could find. We followed the above work plan to complete this task. There were many times when all the Rspec and Cucumber tests passed locally but ran into an issue when we uploaded the changes on GitHub. Prompt feedback from the TRAVIS CI helped us recognize the issue. Then we went on local machine and followed the whole process of refactoring again. In this way, we covered every refactoring we did and ensured that the TRAVIS CI get passed with minimum issues on the code-climate.

== Files modified/created in the current project ==

1. review_mapping_controller.rb 
2. review_mapping_controller_spec.rb 
3. assign_quiz_controller.rb 
4. assign_quiz_controller_spec.rb 
5. review_response_map_controller.rb 
6. review_response_map_controller_spec.rb 
7. routes.rb 
8. Other views & partials associated affected by these changes 

=== ReviewMappingController ===

This controller will map the submissions made by the teams to the students for facilitating peer-reviewing. A couple of '''long''' and '''complex methods''' such as '''peer_review_strategy''' and '''automatic_review_mapping''' were refactored from this controller along with the '''removal''' of some '''non-related methods''' such as '''add_calibration''' and '''assign_quiz_dynamically'''. Variable names have been changed and code has been modularized and helper methods were separated from the important methods into a module and were included in the class.

Test Cases were created for the newly created controllers such as assign_quiz_controller etc.

=== review_mapping_controller_spec.rb ===

After refactoring the '''Review_Mapping_Controller.rb''', there were some tests still present in the spec file of this controller. So, we '''removed such tests''' from the '''review_mapping_controller_spec.rb''' to the appropriate spec file.

=== AssignQuizController ===

'''assign_quiz_dynamically''' (Quizzes are also stored in the Assignment table) is not a seemingly/semantically related task to review mapping. Hence this was moved into a separate controller.

=== assign_quiz_controller_spec.rb ===

Tests related to assign_quiz_controller were moved into this file.

=== ReviewResponseMapController.rb ===

'''Add_Calibration''' is a nuanced method and has seemingly '''different functionality''' than '''Review Mapping''' Controller. Methods having a different purpose than review_mapping or helping review_mapping should not be present in this controller. So we '''moved''' this method '''into''' a '''separate controller'''.

=== review_response_map_controller_spec.rb ===

Tests related to review_response_map were moved into this file.

=== routes.rb ===

New routes were added to newly created controllers.

<pre>
assign_quiz_dynamically_assign_quiz_index POST /assign_quiz/assign_quiz_dynamically(.:format) assign_quiz#assign_quiz_dynamically
</pre>

=== views/partials ===

Routes were changed in the views and partials.

==== Modified View Files: ====
app/views/student_quizzes/_set_dynamic_quiz.html.erb 
app/views/assignments/edit/_calibration.html.erb 

== Details of the changes made==
1. Changed 'instructor = build(:instructor)' to ‘@instructor = build(:instructor, id: 1)’. 
       *Removed code redundancy from review_mapping_controller_spec.rb. Two variables were being initialized containing the same value. One was in the before(:each) loop and other was being called in first three test cases. 
       *Replaced the one in the before(:each) loop by @instructor = build(:instructor, id: 1) and used @instructor class variable, wherever required. 

2. Changed :i_dont_care to :no_particular_topic 
       *:i_dont_care was used in the /app/views/student_review/_set_dynamic_review.html.erb as a flag to store if student is interested in any particular topic or doesn't care which topic to review. 
       *It was also used in review_mapping_controller.rb to check if student has selected any particular topic. 
       *Since, name :i_dont_care was very difficult to understand, we replaced it with something logical such as :no_particular_topic. It gives hint about what the symbol stores. 

3. Removed cascading effects of above change from features spec 
       *Above changes caused ./spec/features/review_assignment_spec.rb this feature test to fail. 
       *This spec has used the above symbol to check if the list of available topics collapse or not after selecting the I don't care option. before: [[File:3bef.jpg]] after: [[File:3aft.jpg]] 

4. Created a variable named ‘allowed_actions’ in method choose_case(action_in_params) 
       *Switch case in above method from review_mapping_controller.rb contained all the actions having the same output for around 70% of cases. 
       *Hence, replaced the switch statements and initialized a list with those switch cases. 
       *Even the space complexity increased, the tradeoff got balanced because if someone has to change some actions, he will have to just add or remove the action name from the allowed_actions list. 
before: [[File:4bef.jpg]] after: [[File:4aft.jpg]] 

5. Refactored Peer_review_strategy by using a helper method gen_random_participant_id 
       *A random participant_id is generated from the possible pool of candidates but the code block for that is kind of a query, i.e. it does not change or set anything. 
       *And it is equally complex enough to confuse the reader. So this has been put into a helper method with an expressive name to increase readability. 
<pre>
## Helper Method for generating a random participant which is to be used in peer_review_strategy method.
def gen_random_participant_id(iterator, participants_hash, num_participants, participants)
if iterator.zero?
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 = []
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
no_particpants = 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
participant_is_owner = (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))
rand_num = if no_particpants or participant_is_owner
# use original method to get random number
rand(0..num_participants - 1)
else
# rand_num should be the position of this participant in original array
participants_with_min_assigned_reviews[rand(0..participants_with_min_assigned_reviews.size - 1)]
end
end
return rand_num
end
</pre>
6. Refactored automatic_review_mapping by using a helper method check_num_reviews_args 
       *Parameters such as num_reviews_per_student, num_calibrated_artifacts etc passed are first verified to check if they are in acceptable range or pattern 
       *To increase readability, the not-so good looking sets of if-else statements have been moved into check_num_reviews_args method. 
<pre>
# Helper Method to check num_reviews_per_student and num_reviews_per_submission arguments passed in by params hash.
def check_num_reviews_args(num_reviews_per_student, num_reviews_per_submission, teams)
has_error_not_raised = true
# check for exit paths first
if num_reviews_per_student == 0 and num_reviews_per_submission == 0
flash[:error] = "Please choose either the number of reviews per student or the number of reviewers per team (student)."
has_error_not_raised = false
elsif num_reviews_per_student != 0 and num_reviews_per_submission != 0
flash[:error] = "Please choose either the number of reviews per student or the number of reviewers per team (student), not both."
has_error_not_raised = false
elsif num_reviews_per_student >= teams.size
# Exception detection: If instructor want to assign too many reviews done
# by each student, there will be an error msg.
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].'
has_error_not_raised = false
end
end
</pre>
7. Modularized helper methods into a module and was mixed in the ReviewMappingController Class. 
       *Only some of the methods written in the class have external usage i.e called by another controllers, views etc. 
       *The other methods are just helpers and as such moved into a Helper method module and mixed in the class 
Before Modularization
<pre>
class ReviewMappingController < ApplicationController
...................
...................
512 Lines & 25 Methods Defined (After moving a few methods into separate controllers)
...................
...................
end
</pre>
After Modularization
<pre>
module Helper_methods
...................
5 Methods and 170 lines
...................
end

class ReviewMappingController < ApplicationController
include Helper_methods
...................
340 Lines & 20 Methods (All of those are used elsewhere directly in the application)
...................
end
</pre>

8. Abided to the principles of Magic Tricks of testing and did not test any internally used methods, The other tests are written were already following this principle. 
       *Internally used methods were not tested 
       *Tests for newly created controllers have been moved into a separate spec files. 

9. Isolated AssignQuizDynamically method into a separate controller as the functionality was not related to ReviewMappingController. 
       *This method assigns a quiz(Stored as an assignment object) to the participant. 
       *This method is not related to ReviewMapping functionality, so it was made into a new controller. 

10. Associated Specs/routes/views/partials have been modified to adapt the change in controllers. 
       *The controller paths present in the views/partials have to be changed to not to break the functionality 
       *The controller paths in views/partials/routes have been changed for the newly created controller of AssignQuizDynamically 

View File Affected by the creation of AssinQuizController
[[File:View.png]]

11. student_review_num sounds like a number or an id associated with a student review, while it actually stores the number of reviews that a student can perform. So it is renamed num_reviews_per_student. 
12. submission_review_num sounds like a number or an id associated with a submission review, while it actually stores the total number of reviews that can be performed on a single submission. So it is renamed num_reviews_per_submission. 
13. calibrated_artifacts_num sounds like a number or an id associated with the calibrated artifacts, while it actually stores the number of calibrated artifacts. So it is renamed num_calibrated_artifacts. Similarly, uncalibrated_artifacts_num is renamed num_uncalibrated_artifacts. 
14. participants_hash is not an appropriate name for a hash whose keys are participant ids and values are number of reviews performed by corresponding participants. So it is renamed num_reviews_by_participant_hash. 
15. Extracted a method make_review_strategy (from automatic_review_mapping_strategy) that returns a review_strategy based on the values of num_reviews_per_submission and num_reviews_per_submission. 
16. add_calibration is a method that changes the attribute of a ReviewResponseMap and has little to do with Review Mapping. So it is now put in a separate controller named ReviewResponseMapController. 
17. The name add_reviewer may lead the reader to think that the method adds a reviewer to a collection of reviewers (e.g. a list of reviewers). Changing the name to assign_reviewer_manually informs the reader that the method assigns a reviewer (to a submission) and hence improves the readability of the code.

== Test Plan ==

=== Rspec Unit Tests ===

Since this is a Refactoring Project, We made sure that the changes made did not break any functionality.

[[File:testplan.png]]

Note: Tests have been run for three controllers (one existing and two new).

=== Capybara Integration and Functional Tests ===

As the controller routes have been modified in the routes.rb and the other view files, there are potential chances of failures in Integration tests.

However, no such failure has been reported by Travis build.

[[File:Travis CI.png]]

[[File:FeaturesCI.png]]

== Code Coverage ==

Code Coverage for Controllers section climbed up.

[https://coveralls.io/builds/26607665] # Link for the COVERALLS stats of our pull request.

[[File:CCC1.png]]

== Project Mentor ==

Ramya Vijayakumar (rvijaya4@ncsu.edu)

== Team Members ==

Yaswanth Soodini (ysoodin@ncsu.edu)

Saurabh Shingte (svshingt@ncsu.edu)

Vivek Karri (vkarri@ncsu.edu)

== Links for Demo Videos ==

Assign Reviewer Manually Demo[https://drive.google.com/file/d/1o9vJQ7fCwk0hwHJdErwpg1z_XjsN_XRT/view?usp=sharing]

Regression and Unit Testing of Controllers Demo[https://youtu.be/XejKHQKtpuA]

Automatic Assignment of Reviewer Demo [https://drive.google.com/file/d/17wpo7mTDenLvEcgubVflLtF_vmLb623a/view?usp=sharing]

CSC/ECE 517 Fall 2019 - E1986. Allow reviewers to bid on what to review

2019-11-16T04:03:41Z

Ysoodin:

== '''Why Should a Student bid for Reviewing Submissions?''' ==

If the choice of which submission to review is left to student itself, he/she would choose to review those topics which are related to their project or whatever they are highly interested in. This would benefit both the reviewer and reviewer because

From a Student's Perspective:

*Because he/she does not have to spend a lot of time understanding what is going on.
*The Student would be more focused on giving the review and hence judgment made tend to be more objective than subjective.
*This would improve the quality of reviews and will also be helpful for the reviewer.
*By implementing the color-coding feature, the student at any point knows which topic/s are more likely to get and he/she optimizes between "what they want" and "what they can get"
*Also, Bidding in Fun !!!!!!!

From the Instructor's Standpoint:

*Because opinions expressed in the reviews are a result of informed decisions, the inference made from the Machine Learning models trained on these data sets would make more sense.

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

=== Currently Deployed Implementation ===

When an assignment participant wants to review others’ work, they will be asked to choose an assignment topic. The participant will be allowed to choose from among those assignment topics that have not been already assigned to 10 other participants. If they choose the ‘I do not care about the topic I review’ option, a random topic will be chosen from all the available topics and will be assigned to them. That is, the reviews are being allocated on a First Come First Serve basis.

=== Previous Work on this issue ===

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review '''E1856''']

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Spring_2019_-_Project_E1928._Allow_reviewers_to_bid_on_what_to_review '''E1928''']

=== What’s Deeply wrong in those implementations? ===

To understand what's wrong, first consider the differences between Teams Bidding for Assignments and Students Bidding for Reviews. Both of these are Matching or resource Allocation Problems.

The former can be modelled as a one-to-one matching problem. (i.e team and assignment has one-to-one correspondence).

[[File:Teams_Assignments_2.png |center|]]

However, Students and Reviews have many-to-many relationship (a student can choose multiple submissions for review and a submission can be given to multiple students for review)

[[File:Teams_Assignments.png |center|]]

The Mathematical formulation is itself wrong in E1856 and E1928 and they have used the below shown diagram to represent the relationship.

[[File:Teams_Assignments_3.png |center|]]

=== Why does the difference is the representation matter? ===

Since they have modeled the problem on the same lines, they have used the same version of the Gale-Shapley or Top Trading Cycles Algorithm used for one-to-many or one-to-one approach.
Famous Problems Dealt on these lines.

[https://en.wikipedia.org/wiki/Stable_marriage_problem '''Stable Marriage Problem'''] (One-to-One)

[https://www.jstor.org/stable/3132114?seq=1&cid=pdf-reference#references_tab_content '''School Choice Problem'''] (One-to-Many)

=== Other Implementation pitfalls observed ===

'''E1928'''

*Once the bidding for review topics is done, the selections need to be saved to the database which is not happening and when the page is refreshed, the UI does not retain the bids.
*The button responsible for running the algorithm cannot be checked and it is a hunk-like icon.
*The button appears multiple times on the page.
*The algorithm needs to be implemented in the web service which should be ideally be used from the lottery controller which is not they have tried to implement. The entire code is written in Ruby.
*The algorithm needs to skip the review topic that the user has worked on. This Code already exists for topic and again written in the new implementation that needs to be integrated into the existing one and hence needs refactoring.
*Proper tests need to be written.

== '''What needs to be done?''' ==

The participants should be able to bid for topics to review in the same way they bid for topics to work on.

=== Approach for matching students with topics ===

We have a set of students S = {S1, ..., Sn}, and a set of topics T = {T1, ..., Tm}. Each student must get exactly qS topics (The threshold in our case is qS = 4, it’s up to the student to decide how many of these they want to actually review.), while each topic must be assigned to at least qT reviewers. Students will submit a list of preferences over the set of topics in linear order. If they submit the preferences for only a certain topics, the rest of the topics will be appended to their preference list in a random order. The topics shall have (possibly different) linear order preference over the set of students according to the timestamps of their time of bid and the total number of topics they have bid for. Students with lower timestamps and who bid for lower number of topics will be given higher preference.

To begin, we will try to obtain a matching with an approximately equal number of students reviewing each topic, which we call uniform distribution. If no uniform stable matching exists, our mechanism will nonetheless return a stable matching, provided one exists for a given profile of preferences.

The average number of reviewers assigned to each course will be

[[File:Math_eq2.png]]

where n is the number of students, and m is the number of courses.

Clearly, this number is not necessarily an integer. Let us assume that

[[File:eq2.png]]

where

[[File:eq1.png]]

Matching 𝝁 is a mapping which assigns exactly qS different topics to each student in Sand at least qT different students to each topic in T. We denote the set of topics assigned by matching 𝜇 to the student si and the set of students who were assigned the topic tj as 𝜇(si) and 𝜇(tj),respectively.

Blocking Pair (Defined according to our problem): A pair (s, t) is called a blocking pair, if the student is associated with the topic (If he/she is in the group which works on topic t). A Matching Set M is considered Stable if it does not have any blocking pairs.

The matching is (pairwise) stable if there are no blocking pairs.

Mechanism:

First we calculate k, p|, and p|. The goal is for each topic to be assigned to either p| and p| students, and to assign exactly qS courses.

The algorithm:

* Step 1:
Each topic proposes to accept the first pI students in its preference list. Each student accepts no more than qS proposals according to his/her preferences, rejecting the rest.
* Step k:
Each course that has z < pI students proposes to accept pI - z students it has not yet proposed to. Each student accepts no more than qS proposals according to his/her preferences, rejecting the others.

The algorithm stops when every topic that has not reached the maximum quota pI has proposed acceptance to every student.

=== Functional Requirements===
*Each participant should be able to submit a list of preferences in decreasing preference order for the topics they would like to review. The list may contain any number of topics.
*During bidding, the topics should be color-coded according to the number of participants who are contending for each topic. The colors should range from green to red with green representing a topic with a low number of bids and red representing a topic with a high number of bids.
*There should be a deadline before which all participants must submit their list of preferences.
*After the deadline, the course instructor should be able to run the bidding algorithm after which each participant will be assigned a list of topics to review. The number of topics assigned to a participant should be minimum(4,k) where ‘k’ is the number of topics in their preference list.
*Once the topics to review have been assigned, the participants should be able to see those topics they have been assigned and give feedback to the submission for each topic.
*If a participant has given feedback to less than 4 topics and chooses to review a submission, the topic with the minimum number of current reviewers will be assigned to that participant.

=== Non-functional Requirements ===

*There must exist a web service that takes the participants and their respective preference lists as input runs a bidding algorithm like top trading cycles and returns a map of the participants and the list of topics assigned to each participant.
*This web service must either be called from the LotteryController or a new controller can be created on the same (if Lottery Controller becomes too complex).

=== Implementing the Web Service ===
*1) Python
*2) Flask (Minimal MVC Framework for Python Backend)
*3) JSON as the Data Serialization Format.
*4) Deployment (AWS or VCL)
*5) Apache Web Server or GUnicorn or Default Flask Server.

=== (Potential) Addition/Modification of files in expertiza ===
*1) Lottery Controller
*2) Review Mapping Controller
*3) A New model to store the bidding information entered by the student.
*4) Unit Test Files Associated
*5) Views/Routes Associated

== '''Test Plan''' ==

*Ensuring that the algorithm results in stable matchings. The concept of stability is defined above.
*Check whether the UI reflects the bids made by the user.
*Testing that the color-coding feature is working and appropriate.
*Check if a user is assigned 4 topics for review, even though he might have bid for any number of topics.
*Regression testing if the code is added in the lottery controller. If new model/controller is added then relevant tests will be included based on the proposal acceptance.
*Tests to ensure that each review topic gets minimum number of reviews.
*Tests to check whether the web service works as expected.

== Project Mentor ==

Ramya Vijayakumar (rvijaya4@ncsu.edu)

== Team Members ==

Yaswanth Soodini (ysoodin@ncsu.edu)

Sai Shruthi Madhuri Kara (skara2@ncsu.edu)

Ayushi Rungta (arungta@ncsu.edu)

Vivek Karri (vkarri@ncsu.edu)

== Important references ==

[https://github.com/peerlogic/IntelligentAssignment/blob/master/app/app.py InteligentAssignment]

[http://ices.gmu.edu/wp-content/uploads/2015/02/Many-to-Many-Matching-Problem-with-Quotas-by-Freer-and-Titova.pdf Many-to-Many-Matching-Problem-with-Quotas-by-Freer-and-Titova]

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Spring_2019_-_Project_E1928._Allow_reviewers_to_bid_on_what_to_review Project_E1928._Allow_reviewers_to_bid_on_what_to_review]

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review E1856_Allow_reviewers_to_bid_on_what_to_review]

CSC/ECE 517 Fall 2019 - E1986. Allow reviewers to bid on what to review

2019-11-16T04:03:12Z

Ysoodin:

== '''Why Should a Student bid for Reviewing Submissions?''' ==

If the choice of which submission to review is left to student itself, he/she would choose to review those topics which are related to their project or whatever they are highly interested in. This would benefit both the reviewer and reviewer because

From a Student's Perspective:

*Because he/she does not have to spend a lot of time understanding what is going on.
*The Student would be more focused on giving the review and hence judgment made tend to be more objective than subjective.
*This would improve the quality of reviews and will also be helpful for the reviewer.
*By implementing the color-coding feature, the student at any point knows which topic/s are more likely to get and he/she optimizes between "what they want" and "what they can get"
*Also, Bidding in Fun !!!!!!!

From the Instructor's Standpoint:

*Because opinions expressed in the reviews are a result of informed decisions, the inference made from the Machine Learning models trained on these data sets would make more sense.

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

=== Currently Deployed Implementation ===

When an assignment participant wants to review others’ work, they will be asked to choose an assignment topic. The participant will be allowed to choose from among those assignment topics that have not been already assigned to 10 other participants. If they choose the ‘I do not care about the topic I review’ option, a random topic will be chosen from all the available topics and will be assigned to them. That is, the reviews are being allocated on a First Come First Serve basis.

=== Previous Work on this issue ===

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review '''E1856''']

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Spring_2019_-_Project_E1928._Allow_reviewers_to_bid_on_what_to_review '''E1928''']

=== What’s Deeply wrong in those implementations? ===

To understand what's wrong, first consider the differences between Teams Bidding for Assignments and Students Bidding for Reviews. Both of these are Matching or resource Allocation Problems.

The former can be modelled as a one-to-one matching problem. (i.e team and assignment has one-to-one correspondence).

[[File:Teams_Assignments_2.png |center|]]

However, Students and Reviews have many-to-many relationship (a student can choose multiple submissions for review and a submission can be given to multiple students for review)

[[File:Teams_Assignments.png |center|]]

The Mathematical formulation is itself wrong in E1856 and E1928 and they have used the below shown diagram to represent the relationship.

[[File:Teams_Assignments_3.png |center|]]

=== Why does the difference is the representation matter? ===

Since they have modeled the problem on the same lines, they have used the same version of the Gale-Shapley or Top Trading Cycles Algorithm used for one-to-many or one-to-one approach.
Famous Problems Dealt on these lines.

[https://en.wikipedia.org/wiki/Stable_marriage_problem '''Stable Marriage Problem'''] (One-to-One)

[https://www.jstor.org/stable/3132114?seq=1&cid=pdf-reference#references_tab_content '''School Choice Problem'''] (One-to-Many)

=== Other Implementation pitfalls observed ===

'''E1928'''

*Once the bidding for review topics is done, the selections need to be saved to the database which is not happening and when the page is refreshed, the UI does not retain the bids.
*The button responsible for running the algorithm cannot be checked and it is a hunk-like icon.
*The button appears multiple times on the page.
*The algorithm needs to be implemented in the web service which should be ideally be used from the lottery controller which is not they have tried to implement. The entire code is written in Ruby.
*The algorithm needs to skip the review topic that the user has worked on. This Code already exists for topic and again written in the new implementation that needs to be integrated into the existing one and hence needs refactoring.
*Proper tests need to be written.

== '''What needs to be done?''' ==

The participants should be able to bid for topics to review in the same way they bid for topics to work on.

=== Approach for matching students with topics ===

We have a set of students S = {S1, ..., Sn}, and a set of topics T = {T1, ..., Tm}. Each student must get exactly qS topics (The threshold in our case is qS = 4, it’s up to the student to decide how many of these they want to actually review.), while each topic must be assigned to at least qT reviewers. Students will submit a list of preferences over the set of topics in linear order. If they submit the preferences for only a certain topics, the rest of the topics will be appended to their preference list in a random order. The topics shall have (possibly different) linear order preference over the set of students according to the timestamps of their time of bid and the total number of topics they have bid for. Students with lower timestamps and who bid for lower number of topics will be given higher preference.

To begin, we will try to obtain a matching with an approximately equal number of students reviewing each topic, which we call uniform distribution. If no uniform stable matching exists, our mechanism will nonetheless return a stable matching, provided one exists for a given profile of preferences.

The average number of reviewers assigned to each course will be

[[File:Math_eq2.png]]

where n is the number of students, and m is the number of courses.

Clearly, this number is not necessarily an integer. Let us assume that

[[File:eq2.png]]

where

[[File:eq1.png]]

Matching 𝝁 is a mapping which assigns exactly qS different topics to each student in Sand at least qT different students to each topic in T. We denote the set of topics assigned by matching 𝜇 to the student si and the set of students who were assigned the topic tj as 𝜇(si) and 𝜇(tj),respectively.

Blocking Pair (Defined according to our problem): A pair (s, t) is called a blocking pair, if the student is associated with the topic (If he/she is in the group which works on topic t). A Matching Set M is considered Stable if it does not have any blocking pairs.

The matching is (pairwise) stable if there are no blocking pairs.

Mechanism:

First we calculate k, p|, and p|. The goal is for each topic to be assigned to either p| and p| students, and to assign exactly qS courses.

The algorithm:

Step 1:
Each topic proposes to accept the first pI students in its preference list. Each student accepts no more than qS proposals according to his/her preferences, rejecting the rest.
Step k:
Each course that has z < pI students proposes to accept pI - z students it has not yet proposed to. Each student accepts no more than qS proposals according to his/her preferences, rejecting the others.

The algorithm stops when every topic that has not reached the maximum quota pI has proposed acceptance to every student.

=== Functional Requirements===
*Each participant should be able to submit a list of preferences in decreasing preference order for the topics they would like to review. The list may contain any number of topics.
*During bidding, the topics should be color-coded according to the number of participants who are contending for each topic. The colors should range from green to red with green representing a topic with a low number of bids and red representing a topic with a high number of bids.
*There should be a deadline before which all participants must submit their list of preferences.
*After the deadline, the course instructor should be able to run the bidding algorithm after which each participant will be assigned a list of topics to review. The number of topics assigned to a participant should be minimum(4,k) where ‘k’ is the number of topics in their preference list.
*Once the topics to review have been assigned, the participants should be able to see those topics they have been assigned and give feedback to the submission for each topic.
*If a participant has given feedback to less than 4 topics and chooses to review a submission, the topic with the minimum number of current reviewers will be assigned to that participant.

=== Non-functional Requirements ===

*There must exist a web service that takes the participants and their respective preference lists as input runs a bidding algorithm like top trading cycles and returns a map of the participants and the list of topics assigned to each participant.
*This web service must either be called from the LotteryController or a new controller can be created on the same (if Lottery Controller becomes too complex).

=== Implementing the Web Service ===
*1) Python
*2) Flask (Minimal MVC Framework for Python Backend)
*3) JSON as the Data Serialization Format.
*4) Deployment (AWS or VCL)
*5) Apache Web Server or GUnicorn or Default Flask Server.

=== (Potential) Addition/Modification of files in expertiza ===
*1) Lottery Controller
*2) Review Mapping Controller
*3) A New model to store the bidding information entered by the student.
*4) Unit Test Files Associated
*5) Views/Routes Associated

== '''Test Plan''' ==

*Ensuring that the algorithm results in stable matchings. The concept of stability is defined above.
*Check whether the UI reflects the bids made by the user.
*Testing that the color-coding feature is working and appropriate.
*Check if a user is assigned 4 topics for review, even though he might have bid for any number of topics.
*Regression testing if the code is added in the lottery controller. If new model/controller is added then relevant tests will be included based on the proposal acceptance.
*Tests to ensure that each review topic gets minimum number of reviews.
*Tests to check whether the web service works as expected.

== Project Mentor ==

Ramya Vijayakumar (rvijaya4@ncsu.edu)

== Team Members ==

Yaswanth Soodini (ysoodin@ncsu.edu)

Sai Shruthi Madhuri Kara (skara2@ncsu.edu)

Ayushi Rungta (arungta@ncsu.edu)

Vivek Karri (vkarri@ncsu.edu)

== Important references ==

[https://github.com/peerlogic/IntelligentAssignment/blob/master/app/app.py InteligentAssignment]

[http://ices.gmu.edu/wp-content/uploads/2015/02/Many-to-Many-Matching-Problem-with-Quotas-by-Freer-and-Titova.pdf Many-to-Many-Matching-Problem-with-Quotas-by-Freer-and-Titova]

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Spring_2019_-_Project_E1928._Allow_reviewers_to_bid_on_what_to_review Project_E1928._Allow_reviewers_to_bid_on_what_to_review]

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review E1856_Allow_reviewers_to_bid_on_what_to_review]

CSC/ECE 517 Fall 2019 - E1986. Allow reviewers to bid on what to review

2019-11-16T04:02:25Z

Ysoodin:

== '''Why Should a Student bid for Reviewing Submissions?''' ==

If the choice of which submission to review is left to student itself, he/she would choose to review those topics which are related to their project or whatever they are highly interested in. This would benefit both the reviewer and reviewer because

From a Student's Perspective:

*Because he/she does not have to spend a lot of time understanding what is going on.
*The Student would be more focused on giving the review and hence judgment made tend to be more objective than subjective.
*This would improve the quality of reviews and will also be helpful for the reviewer.
*By implementing the color-coding feature, the student at any point knows which topic/s are more likely to get and he/she optimizes between "what they want" and "what they can get"
*Also, Bidding in Fun !!!!!!!

From the Instructor's Standpoint:

*Because opinions expressed in the reviews are a result of informed decisions, the inference made from the Machine Learning models trained on these data sets would make more sense.

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

=== Currently Deployed Implementation ===

When an assignment participant wants to review others’ work, they will be asked to choose an assignment topic. The participant will be allowed to choose from among those assignment topics that have not been already assigned to 10 other participants. If they choose the ‘I do not care about the topic I review’ option, a random topic will be chosen from all the available topics and will be assigned to them. That is, the reviews are being allocated on a First Come First Serve basis.

=== Previous Work on this issue ===

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review '''E1856''']

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Spring_2019_-_Project_E1928._Allow_reviewers_to_bid_on_what_to_review '''E1928''']

=== What’s Deeply wrong in those implementations? ===

To understand what's wrong, first consider the differences between Teams Bidding for Assignments and Students Bidding for Reviews. Both of these are Matching or resource Allocation Problems.

The former can be modelled as a one-to-one matching problem. (i.e team and assignment has one-to-one correspondence).

[[File:Teams_Assignments_2.png |center|]]

However, Students and Reviews have many-to-many relationship (a student can choose multiple submissions for review and a submission can be given to multiple students for review)

[[File:Teams_Assignments.png |center|]]

The Mathematical formulation is itself wrong in E1856 and E1928 and they have used the below shown diagram to represent the relationship.

[[File:Teams_Assignments_3.png |center|]]

=== Why does the difference is the representation matter? ===

Since they have modeled the problem on the same lines, they have used the same version of the Gale-Shapley or Top Trading Cycles Algorithm used for one-to-many or one-to-one approach.
Famous Problems Dealt on these lines.

[https://en.wikipedia.org/wiki/Stable_marriage_problem '''Stable Marriage Problem'''] (One-to-One)

[https://www.jstor.org/stable/3132114?seq=1&cid=pdf-reference#references_tab_content '''School Choice Problem'''] (One-to-Many)

=== Other Implementation pitfalls observed ===

'''E1928'''

*Once the bidding for review topics is done, the selections need to be saved to the database which is not happening and when the page is refreshed, the UI does not retain the bids.
*The button responsible for running the algorithm cannot be checked and it is a hunk-like icon.
*The button appears multiple times on the page.
*The algorithm needs to be implemented in the web service which should be ideally be used from the lottery controller which is not they have tried to implement. The entire code is written in Ruby.
*The algorithm needs to skip the review topic that the user has worked on. This Code already exists for topic and again written in the new implementation that needs to be integrated into the existing one and hence needs refactoring.
*Proper tests need to be written.

== '''What needs to be done?''' ==

The participants should be able to bid for topics to review in the same way they bid for topics to work on.

=== Approach for matching students with topics ===

We have a set of students S = {S1, ..., Sn}, and a set of topics T = {T1, ..., Tm}. Each student must get exactly qS topics (The threshold in our case is qS = 4, it’s up to the student to decide how many of these they want to actually review.), while each topic must be assigned to at least qT reviewers. Students will submit a list of preferences over the set of topics in linear order. If they submit the preferences for only a certain topics, the rest of the topics will be appended to their preference list in a random order. The topics shall have (possibly different) linear order preference over the set of students according to the timestamps of their time of bid and the total number of topics they have bid for. Students with lower timestamps and who bid for lower number of topics will be given higher preference.

To begin, we will try to obtain a matching with an approximately equal number of students reviewing each topic, which we call uniform distribution. If no uniform stable matching exists, our mechanism will nonetheless return a stable matching, provided one exists for a given profile of preferences.

The average number of reviewers assigned to each course will be

[[File:eq2.png]]

where n is the number of students, and m is the number of courses.

Clearly, this number is not necessarily an integer. Let us assume that

[[File:Math_eq2.png]]

where

[[File:eq1.png]]

Matching 𝝁 is a mapping which assigns exactly qS different topics to each student in Sand at least qT different students to each topic in T. We denote the set of topics assigned by matching 𝜇 to the student si and the set of students who were assigned the topic tj as 𝜇(si) and 𝜇(tj),respectively.

Blocking Pair (Defined according to our problem): A pair (s, t) is called a blocking pair, if the student is associated with the topic (If he/she is in the group which works on topic t). A Matching Set M is considered Stable if it does not have any blocking pairs.

The matching is (pairwise) stable if there are no blocking pairs.

Mechanism:

First we calculate k, p|, and p|. The goal is for each topic to be assigned to either p| and p| students, and to assign exactly qS courses.

The algorithm:

Step 1:
Each topic proposes to accept the first pI students in its preference list. Each student accepts no more than qS proposals according to his/her preferences, rejecting the rest.
Step k:
Each course that has z < pI students proposes to accept pI - z students it has not yet proposed to. Each student accepts no more than qS proposals according to his/her preferences, rejecting the others.

The algorithm stops when every topic that has not reached the maximum quota pI has proposed acceptance to every student.

=== Functional Requirements===
*Each participant should be able to submit a list of preferences in decreasing preference order for the topics they would like to review. The list may contain any number of topics.
*During bidding, the topics should be color-coded according to the number of participants who are contending for each topic. The colors should range from green to red with green representing a topic with a low number of bids and red representing a topic with a high number of bids.
*There should be a deadline before which all participants must submit their list of preferences.
*After the deadline, the course instructor should be able to run the bidding algorithm after which each participant will be assigned a list of topics to review. The number of topics assigned to a participant should be minimum(4,k) where ‘k’ is the number of topics in their preference list.
*Once the topics to review have been assigned, the participants should be able to see those topics they have been assigned and give feedback to the submission for each topic.
*If a participant has given feedback to less than 4 topics and chooses to review a submission, the topic with the minimum number of current reviewers will be assigned to that participant.

=== Non-functional Requirements ===

*There must exist a web service that takes the participants and their respective preference lists as input runs a bidding algorithm like top trading cycles and returns a map of the participants and the list of topics assigned to each participant.
*This web service must either be called from the LotteryController or a new controller can be created on the same (if Lottery Controller becomes too complex).

=== Implementing the Web Service ===
*1) Python
*2) Flask (Minimal MVC Framework for Python Backend)
*3) JSON as the Data Serialization Format.
*4) Deployment (AWS or VCL)
*5) Apache Web Server or GUnicorn or Default Flask Server.

=== (Potential) Addition/Modification of files in expertiza ===
*1) Lottery Controller
*2) Review Mapping Controller
*3) A New model to store the bidding information entered by the student.
*4) Unit Test Files Associated
*5) Views/Routes Associated

== '''Test Plan''' ==

*Ensuring that the algorithm results in stable matchings. The concept of stability is defined above.
*Check whether the UI reflects the bids made by the user.
*Testing that the color-coding feature is working and appropriate.
*Check if a user is assigned 4 topics for review, even though he might have bid for any number of topics.
*Regression testing if the code is added in the lottery controller. If new model/controller is added then relevant tests will be included based on the proposal acceptance.
*Tests to ensure that each review topic gets minimum number of reviews.
*Tests to check whether the web service works as expected.

== Project Mentor ==

Ramya Vijayakumar (rvijaya4@ncsu.edu)

== Team Members ==

Yaswanth Soodini (ysoodin@ncsu.edu)

Sai Shruthi Madhuri Kara (skara2@ncsu.edu)

Ayushi Rungta (arungta@ncsu.edu)

Vivek Karri (vkarri@ncsu.edu)

== Important references ==

[https://github.com/peerlogic/IntelligentAssignment/blob/master/app/app.py InteligentAssignment]

[http://ices.gmu.edu/wp-content/uploads/2015/02/Many-to-Many-Matching-Problem-with-Quotas-by-Freer-and-Titova.pdf Many-to-Many-Matching-Problem-with-Quotas-by-Freer-and-Titova]

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Spring_2019_-_Project_E1928._Allow_reviewers_to_bid_on_what_to_review Project_E1928._Allow_reviewers_to_bid_on_what_to_review]

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review E1856_Allow_reviewers_to_bid_on_what_to_review]

CSC/ECE 517 Fall 2019 - E1986. Allow reviewers to bid on what to review

2019-11-16T04:01:24Z

Ysoodin:

== '''Why Should a Student bid for Reviewing Submissions?''' ==

If the choice of which submission to review is left to student itself, he/she would choose to review those topics which are related to their project or whatever they are highly interested in. This would benefit both the reviewer and reviewer because

From a Student's Perspective:

*Because he/she does not have to spend a lot of time understanding what is going on.
*The Student would be more focused on giving the review and hence judgment made tend to be more objective than subjective.
*This would improve the quality of reviews and will also be helpful for the reviewer.
*By implementing the color-coding feature, the student at any point knows which topic/s are more likely to get and he/she optimizes between "what they want" and "what they can get"
*Also, Bidding in Fun !!!!!!!

From the Instructor's Standpoint:

*Because opinions expressed in the reviews are a result of informed decisions, the inference made from the Machine Learning models trained on these data sets would make more sense.

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

=== Currently Deployed Implementation ===

When an assignment participant wants to review others’ work, they will be asked to choose an assignment topic. The participant will be allowed to choose from among those assignment topics that have not been already assigned to 10 other participants. If they choose the ‘I do not care about the topic I review’ option, a random topic will be chosen from all the available topics and will be assigned to them. That is, the reviews are being allocated on a First Come First Serve basis.

=== Previous Work on this issue ===

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review '''E1856''']

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Spring_2019_-_Project_E1928._Allow_reviewers_to_bid_on_what_to_review '''E1928''']

=== What’s Deeply wrong in those implementations? ===

To understand what's wrong, first consider the differences between Teams Bidding for Assignments and Students Bidding for Reviews. Both of these are Matching or resource Allocation Problems.

The former can be modelled as a one-to-one matching problem. (i.e team and assignment has one-to-one correspondence).

[[File:Teams_Assignments_2.png |center|]]

However, Students and Reviews have many-to-many relationship (a student can choose multiple submissions for review and a submission can be given to multiple students for review)

[[File:Teams_Assignments.png |center|]]

The Mathematical formulation is itself wrong in E1856 and E1928 and they have used the below shown diagram to represent the relationship.

[[File:Teams_Assignments_3.png |center|]]

=== Why does the difference is the representation matter? ===

Since they have modeled the problem on the same lines, they have used the same version of the Gale-Shapley or Top Trading Cycles Algorithm used for one-to-many or one-to-one approach.
Famous Problems Dealt on these lines.

[https://en.wikipedia.org/wiki/Stable_marriage_problem '''Stable Marriage Problem'''] (One-to-One)

[https://www.jstor.org/stable/3132114?seq=1&cid=pdf-reference#references_tab_content '''School Choice Problem'''] (One-to-Many)

=== Other Implementation pitfalls observed ===

'''E1928'''

*Once the bidding for review topics is done, the selections need to be saved to the database which is not happening and when the page is refreshed, the UI does not retain the bids.
*The button responsible for running the algorithm cannot be checked and it is a hunk-like icon.
*The button appears multiple times on the page.
*The algorithm needs to be implemented in the web service which should be ideally be used from the lottery controller which is not they have tried to implement. The entire code is written in Ruby.
*The algorithm needs to skip the review topic that the user has worked on. This Code already exists for topic and again written in the new implementation that needs to be integrated into the existing one and hence needs refactoring.
*Proper tests need to be written.

== '''What needs to be done?''' ==

The participants should be able to bid for topics to review in the same way they bid for topics to work on.

=== Approach for matching students with topics ===

We have a set of students S = {S1, ..., Sn}, and a set of topics T = {T1, ..., Tm}. Each student must get exactly qS topics (The threshold in our case is qS = 4, it’s up to the student to decide how many of these they want to actually review.), while each topic must be assigned to at least qT reviewers. Students will submit a list of preferences over the set of topics in linear order. If they submit the preferences for only a certain topics, the rest of the topics will be appended to their preference list in a random order. The topics shall have (possibly different) linear order preference over the set of students according to the timestamps of their time of bid and the total number of topics they have bid for. Students with lower timestamps and who bid for lower number of topics will be given higher preference.

To begin, we will try to obtain a matching with an approximately equal number of students reviewing each topic, which we call uniform distribution. If no uniform stable matching exists, our mechanism will nonetheless return a stable matching, provided one exists for a given profile of preferences.

The average number of reviewers assigned to each course will be

[[File:Math_eq3.png]]

where n is the number of students, and m is the number of courses.

Clearly, this number is not necessarily an integer. Let us assume that

[[File:Math_eq2]]

where

[[File:eq1.png]]

Matching 𝝁 is a mapping which assigns exactly qS different topics to each student in Sand at least qT different students to each topic in T. We denote the set of topics assigned by matching 𝜇 to the student si and the set of students who were assigned the topic tj as 𝜇(si) and 𝜇(tj),respectively.

Blocking Pair (Defined according to our problem): A pair (s, t) is called a blocking pair, if the student is associated with the topic (If he/she is in the group which works on topic t). A Matching Set M is considered Stable if it does not have any blocking pairs.

The matching is (pairwise) stable if there are no blocking pairs.

Mechanism:

First we calculate k, p|, and p|. The goal is for each topic to be assigned to either p| and p| students, and to assign exactly qS courses.

The algorithm:

Step 1:
Each topic proposes to accept the first pI students in its preference list. Each student accepts no more than qS proposals according to his/her preferences, rejecting the rest.
Step k:
Each course that has z < pI students proposes to accept pI - z students it has not yet proposed to. Each student accepts no more than qS proposals according to his/her preferences, rejecting the others.

The algorithm stops when every topic that has not reached the maximum quota pI has proposed acceptance to every student.

=== Functional Requirements===
*Each participant should be able to submit a list of preferences in decreasing preference order for the topics they would like to review. The list may contain any number of topics.
*During bidding, the topics should be color-coded according to the number of participants who are contending for each topic. The colors should range from green to red with green representing a topic with a low number of bids and red representing a topic with a high number of bids.
*There should be a deadline before which all participants must submit their list of preferences.
*After the deadline, the course instructor should be able to run the bidding algorithm after which each participant will be assigned a list of topics to review. The number of topics assigned to a participant should be minimum(4,k) where ‘k’ is the number of topics in their preference list.
*Once the topics to review have been assigned, the participants should be able to see those topics they have been assigned and give feedback to the submission for each topic.
*If a participant has given feedback to less than 4 topics and chooses to review a submission, the topic with the minimum number of current reviewers will be assigned to that participant.

=== Non-functional Requirements ===

*There must exist a web service that takes the participants and their respective preference lists as input runs a bidding algorithm like top trading cycles and returns a map of the participants and the list of topics assigned to each participant.
*This web service must either be called from the LotteryController or a new controller can be created on the same (if Lottery Controller becomes too complex).

=== Implementing the Web Service ===
*1) Python
*2) Flask (Minimal MVC Framework for Python Backend)
*3) JSON as the Data Serialization Format.
*4) Deployment (AWS or VCL)
*5) Apache Web Server or GUnicorn or Default Flask Server.

=== (Potential) Addition/Modification of files in expertiza ===
*1) Lottery Controller
*2) Review Mapping Controller
*3) A New model to store the bidding information entered by the student.
*4) Unit Test Files Associated
*5) Views/Routes Associated

== '''Test Plan''' ==

*Ensuring that the algorithm results in stable matchings. The concept of stability is defined above.
*Check whether the UI reflects the bids made by the user.
*Testing that the color-coding feature is working and appropriate.
*Check if a user is assigned 4 topics for review, even though he might have bid for any number of topics.
*Regression testing if the code is added in the lottery controller. If new model/controller is added then relevant tests will be included based on the proposal acceptance.
*Tests to ensure that each review topic gets minimum number of reviews.
*Tests to check whether the web service works as expected.

== Project Mentor ==

Ramya Vijayakumar (rvijaya4@ncsu.edu)

== Team Members ==

Yaswanth Soodini (ysoodin@ncsu.edu)

Sai Shruthi Madhuri Kara (skara2@ncsu.edu)

Ayushi Rungta (arungta@ncsu.edu)

Vivek Karri (vkarri@ncsu.edu)

== Important references ==

[https://github.com/peerlogic/IntelligentAssignment/blob/master/app/app.py InteligentAssignment]

[http://ices.gmu.edu/wp-content/uploads/2015/02/Many-to-Many-Matching-Problem-with-Quotas-by-Freer-and-Titova.pdf Many-to-Many-Matching-Problem-with-Quotas-by-Freer-and-Titova]

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Spring_2019_-_Project_E1928._Allow_reviewers_to_bid_on_what_to_review Project_E1928._Allow_reviewers_to_bid_on_what_to_review]

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review E1856_Allow_reviewers_to_bid_on_what_to_review]

CSC/ECE 517 Fall 2019 - E1986. Allow reviewers to bid on what to review

2019-11-16T03:58:47Z

Ysoodin:

== '''Why Should a Student bid for Reviewing Submissions?''' ==

If the choice of which submission to review is left to student itself, he/she would choose to review those topics which are related to their project or whatever they are highly interested in. This would benefit both the reviewer and reviewer because

From a Student's Perspective:

*Because he/she does not have to spend a lot of time understanding what is going on.
*The Student would be more focused on giving the review and hence judgment made tend to be more objective than subjective.
*This would improve the quality of reviews and will also be helpful for the reviewer.
*By implementing the color-coding feature, the student at any point knows which topic/s are more likely to get and he/she optimizes between "what they want" and "what they can get"
*Also, Bidding in Fun !!!!!!!

From the Instructor's Standpoint:

*Because opinions expressed in the reviews are a result of informed decisions, the inference made from the Machine Learning models trained on these data sets would make more sense.

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

=== Currently Deployed Implementation ===

When an assignment participant wants to review others’ work, they will be asked to choose an assignment topic. The participant will be allowed to choose from among those assignment topics that have not been already assigned to 10 other participants. If they choose the ‘I do not care about the topic I review’ option, a random topic will be chosen from all the available topics and will be assigned to them. That is, the reviews are being allocated on a First Come First Serve basis.

=== Previous Work on this issue ===

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review '''E1856''']

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Spring_2019_-_Project_E1928._Allow_reviewers_to_bid_on_what_to_review '''E1928''']

=== What’s Deeply wrong in those implementations? ===

To understand what's wrong, first consider the differences between Teams Bidding for Assignments and Students Bidding for Reviews. Both of these are Matching or resource Allocation Problems.

The former can be modelled as a one-to-one matching problem. (i.e team and assignment has one-to-one correspondence).

[[File:Teams_Assignments_2.png |center|]]

However, Students and Reviews have many-to-many relationship (a student can choose multiple submissions for review and a submission can be given to multiple students for review)

[[File:Teams_Assignments.png |center|]]

The Mathematical formulation is itself wrong in E1856 and E1928 and they have used the below shown diagram to represent the relationship.

[[File:Teams_Assignments_3.png |center|]]

=== Why does the difference is the representation matter? ===

Since they have modeled the problem on the same lines, they have used the same version of the Gale-Shapley or Top Trading Cycles Algorithm used for one-to-many or one-to-one approach.
Famous Problems Dealt on these lines.

[https://en.wikipedia.org/wiki/Stable_marriage_problem '''Stable Marriage Problem'''] (One-to-One)

[https://www.jstor.org/stable/3132114?seq=1&cid=pdf-reference#references_tab_content '''School Choice Problem'''] (One-to-Many)

=== Other Implementation pitfalls observed ===

'''E1928'''

*Once the bidding for review topics is done, the selections need to be saved to the database which is not happening and when the page is refreshed, the UI does not retain the bids.
*The button responsible for running the algorithm cannot be checked and it is a hunk-like icon.
*The button appears multiple times on the page.
*The algorithm needs to be implemented in the web service which should be ideally be used from the lottery controller which is not they have tried to implement. The entire code is written in Ruby.
*The algorithm needs to skip the review topic that the user has worked on. This Code already exists for topic and again written in the new implementation that needs to be integrated into the existing one and hence needs refactoring.
*Proper tests need to be written.

== '''What needs to be done?''' ==

The participants should be able to bid for topics to review in the same way they bid for topics to work on.

=== Approach for matching students with topics ===

We have a set of students S = {S1, ..., Sn}, and a set of topics T = {T1, ..., Tm}. Each student must get exactly qS topics (The threshold in our case is qS = 4, it’s up to the student to decide how many of these they want to actually review.), while each topic must be assigned to at least qT reviewers. Students will submit a list of preferences over the set of topics in linear order. If they submit the preferences for only a certain topics, the rest of the topics will be appended to their preference list in a random order. The topics shall have (possibly different) linear order preference over the set of students according to the timestamps of their time of bid and the total number of topics they have bid for. Students with lower timestamps and who bid for lower number of topics will be given higher preference.

To begin, we will try to obtain a matching with an approximately equal number of students reviewing each topic, which we call uniform distribution. If no uniform stable matching exists, our mechanism will nonetheless return a stable matching, provided one exists for a given profile of preferences.

The average number of reviewers assigned to each course will be

[[File:eq1.png]]

{{math| ''k'' {{=}} {{sfrac|''n''''qS'' |''m'}} }}
where n is the number of students, and m is the number of courses.

Clearly, this number is not necessarily an integer. Let us assume that

p|nqsmp|

where

p| = ⌊nqSm⌋ ℕ

p| = ⌈nqSm⌉ ℕ

Matching 𝝁 is a mapping which assigns exactly qS different topics to each student in Sand at least qT different students to each topic in T. We denote the set of topics assigned by matching 𝜇 to the student si and the set of students who were assigned the topic tj as 𝜇(si) and 𝜇(tj),respectively.

Blocking Pair (Defined according to our problem): A pair (s, t) is called a blocking pair, if the student is associated with the topic (If he/she is in the group which works on topic t). A Matching Set M is considered Stable if it does not have any blocking pairs.

The matching is (pairwise) stable if there are no blocking pairs.

Mechanism:

First we calculate k, p|, and p|. The goal is for each topic to be assigned to either p| and p| students, and to assign exactly qS courses.

The algorithm:

Step 1:
Each topic proposes to accept the first pI students in its preference list. Each student accepts no more than qS proposals according to his/her preferences, rejecting the rest.
Step k:
Each course that has z < pI students proposes to accept pI - z students it has not yet proposed to. Each student accepts no more than qS proposals according to his/her preferences, rejecting the others.

The algorithm stops when every topic that has not reached the maximum quota pI has proposed acceptance to every student.

=== Functional Requirements===
*Each participant should be able to submit a list of preferences in decreasing preference order for the topics they would like to review. The list may contain any number of topics.
*During bidding, the topics should be color-coded according to the number of participants who are contending for each topic. The colors should range from green to red with green representing a topic with a low number of bids and red representing a topic with a high number of bids.
*There should be a deadline before which all participants must submit their list of preferences.
*After the deadline, the course instructor should be able to run the bidding algorithm after which each participant will be assigned a list of topics to review. The number of topics assigned to a participant should be minimum(4,k) where ‘k’ is the number of topics in their preference list.
*Once the topics to review have been assigned, the participants should be able to see those topics they have been assigned and give feedback to the submission for each topic.
*If a participant has given feedback to less than 4 topics and chooses to review a submission, the topic with the minimum number of current reviewers will be assigned to that participant.

=== Non-functional Requirements ===

*There must exist a web service that takes the participants and their respective preference lists as input runs a bidding algorithm like top trading cycles and returns a map of the participants and the list of topics assigned to each participant.
*This web service must either be called from the LotteryController or a new controller can be created on the same (if Lottery Controller becomes too complex).

=== Implementing the Web Service ===
*1) Python
*2) Flask (Minimal MVC Framework for Python Backend)
*3) JSON as the Data Serialization Format.
*4) Deployment (AWS or VCL)
*5) Apache Web Server or GUnicorn or Default Flask Server.

=== (Potential) Addition/Modification of files in expertiza ===
*1) Lottery Controller
*2) Review Mapping Controller
*3) A New model to store the bidding information entered by the student.
*4) Unit Test Files Associated
*5) Views/Routes Associated

== '''Test Plan''' ==

*Ensuring that the algorithm results in stable matchings. The concept of stability is defined above.
*Check whether the UI reflects the bids made by the user.
*Testing that the color-coding feature is working and appropriate.
*Check if a user is assigned 4 topics for review, even though he might have bid for any number of topics.
*Regression testing if the code is added in the lottery controller. If new model/controller is added then relevant tests will be included based on the proposal acceptance.
*Tests to ensure that each review topic gets minimum number of reviews.
*Tests to check whether the web service works as expected.

== Project Mentor ==

Ramya Vijayakumar (rvijaya4@ncsu.edu)

== Team Members ==

Yaswanth Soodini (ysoodin@ncsu.edu)

Sai Shruthi Madhuri Kara (skara2@ncsu.edu)

Ayushi Rungta (arungta@ncsu.edu)

Vivek Karri (vkarri@ncsu.edu)

== Important references ==

[https://github.com/peerlogic/IntelligentAssignment/blob/master/app/app.py InteligentAssignment]

[http://ices.gmu.edu/wp-content/uploads/2015/02/Many-to-Many-Matching-Problem-with-Quotas-by-Freer-and-Titova.pdf Many-to-Many-Matching-Problem-with-Quotas-by-Freer-and-Titova]

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Spring_2019_-_Project_E1928._Allow_reviewers_to_bid_on_what_to_review Project_E1928._Allow_reviewers_to_bid_on_what_to_review]

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review E1856_Allow_reviewers_to_bid_on_what_to_review]

File:Math eq2.png

2019-11-16T03:56:18Z

Ysoodin:

File:Eq2.png

2019-11-16T03:55:01Z

Ysoodin: uploaded a new version of "File:Eq2.png"

File:Eq1.png

2019-11-16T03:54:37Z

Ysoodin:

CSC/ECE 517 Fall 2019 - E1986. Allow reviewers to bid on what to review

2019-11-16T03:49:17Z

Ysoodin: /* Approach for matching students with topics */

== '''Why Should a Student bid for Reviewing Submissions?''' ==

If the choice of which submission to review is left to student itself, he/she would choose to review those topics which are related to their project or whatever they are highly interested in. This would benefit both the reviewer and reviewer because

From a Student's Perspective:

*Because he/she does not have to spend a lot of time understanding what is going on.
*The Student would be more focused on giving the review and hence judgment made tend to be more objective than subjective.
*This would improve the quality of reviews and will also be helpful for the reviewer.
*By implementing the color-coding feature, the student at any point knows which topic/s are more likely to get and he/she optimizes between "what they want" and "what they can get"
*Also, Bidding in Fun !!!!!!!

From the Instructor's Standpoint:

*Because opinions expressed in the reviews are a result of informed decisions, the inference made from the Machine Learning models trained on these data sets would make more sense.

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

=== Currently Deployed Implementation ===

When an assignment participant wants to review others’ work, they will be asked to choose an assignment topic. The participant will be allowed to choose from among those assignment topics that have not been already assigned to 10 other participants. If they choose the ‘I do not care about the topic I review’ option, a random topic will be chosen from all the available topics and will be assigned to them. That is, the reviews are being allocated on a First Come First Serve basis.

=== Previous Work on this issue ===

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review '''E1856''']

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Spring_2019_-_Project_E1928._Allow_reviewers_to_bid_on_what_to_review '''E1928''']

=== What’s Deeply wrong in those implementations? ===

To understand what's wrong, first consider the differences between Teams Bidding for Assignments and Students Bidding for Reviews. Both of these are Matching or resource Allocation Problems.

The former can be modelled as a one-to-one matching problem. (i.e team and assignment has one-to-one correspondence).

[[File:Teams_Assignments_2.png |center|]]

However, Students and Reviews have many-to-many relationship (a student can choose multiple submissions for review and a submission can be given to multiple students for review)

[[File:Teams_Assignments.png |center|]]

The Mathematical formulation is itself wrong in E1856 and E1928 and they have used the below shown diagram to represent the relationship.

[[File:Teams_Assignments_3.png |center|]]

=== Why does the difference is the representation matter? ===

Since they have modeled the problem on the same lines, they have used the same version of the Gale-Shapley or Top Trading Cycles Algorithm used for one-to-many or one-to-one approach.
Famous Problems Dealt on these lines.

[https://en.wikipedia.org/wiki/Stable_marriage_problem '''Stable Marriage Problem'''] (One-to-One)

[https://www.jstor.org/stable/3132114?seq=1&cid=pdf-reference#references_tab_content '''School Choice Problem'''] (One-to-Many)

=== Other Implementation pitfalls observed ===

'''E1928'''

*Once the bidding for review topics is done, the selections need to be saved to the database which is not happening and when the page is refreshed, the UI does not retain the bids.
*The button responsible for running the algorithm cannot be checked and it is a hunk-like icon.
*The button appears multiple times on the page.
*The algorithm needs to be implemented in the web service which should be ideally be used from the lottery controller which is not they have tried to implement. The entire code is written in Ruby.
*The algorithm needs to skip the review topic that the user has worked on. This Code already exists for topic and again written in the new implementation that needs to be integrated into the existing one and hence needs refactoring.
*Proper tests need to be written.

== '''What needs to be done?''' ==

The participants should be able to bid for topics to review in the same way they bid for topics to work on.

=== Approach for matching students with topics ===

We have a set of students S = {S1, ..., Sn}, and a set of topics T = {T1, ..., Tm}. Each student must get exactly qS topics (The threshold in our case is qS = 4, it’s up to the student to decide how many of these they want to actually review.), while each topic must be assigned to at least qT reviewers. Students will submit a list of preferences over the set of topics in linear order. If they submit the preferences for only a certain topics, the rest of the topics will be appended to their preference list in a random order. The topics shall have (possibly different) linear order preference over the set of students according to the timestamps of their time of bid and the total number of topics they have bid for. Students with lower timestamps and who bid for lower number of topics will be given higher preference.

To begin, we will try to obtain a matching with an approximately equal number of students reviewing each topic, which we call uniform distribution. If no uniform stable matching exists, our mechanism will nonetheless return a stable matching, provided one exists for a given profile of preferences.

The average number of reviewers assigned to each course will be

{{math| ''k'' {{=}} {{sfrac|''n''''qS'' |''m'}} }}
where n is the number of students, and m is the number of courses.

Clearly, this number is not necessarily an integer. Let us assume that

p|nqsmp|

where

p| = ⌊nqSm⌋ ℕ

p| = ⌈nqSm⌉ ℕ

Matching 𝝁 is a mapping which assigns exactly qS different topics to each student in Sand at least qT different students to each topic in T. We denote the set of topics assigned by matching 𝜇 to the student si and the set of students who were assigned the topic tj as 𝜇(si) and 𝜇(tj),respectively.

Blocking Pair (Defined according to our problem): A pair (s, t) is called a blocking pair, if the student is associated with the topic (If he/she is in the group which works on topic t). A Matching Set M is considered Stable if it does not have any blocking pairs.

The matching is (pairwise) stable if there are no blocking pairs.

Mechanism:

First we calculate k, p|, and p|. The goal is for each topic to be assigned to either p| and p| students, and to assign exactly qS courses.

The algorithm:

Step 1:
Each topic proposes to accept the first pI students in its preference list. Each student accepts no more than qS proposals according to his/her preferences, rejecting the rest.
Step k:
Each course that has z < pI students proposes to accept pI - z students it has not yet proposed to. Each student accepts no more than qS proposals according to his/her preferences, rejecting the others.

The algorithm stops when every topic that has not reached the maximum quota pI has proposed acceptance to every student.

=== Functional Requirements===
*Each participant should be able to submit a list of preferences in decreasing preference order for the topics they would like to review. The list may contain any number of topics.
*During bidding, the topics should be color-coded according to the number of participants who are contending for each topic. The colors should range from green to red with green representing a topic with a low number of bids and red representing a topic with a high number of bids.
*There should be a deadline before which all participants must submit their list of preferences.
*After the deadline, the course instructor should be able to run the bidding algorithm after which each participant will be assigned a list of topics to review. The number of topics assigned to a participant should be minimum(4,k) where ‘k’ is the number of topics in their preference list.
*Once the topics to review have been assigned, the participants should be able to see those topics they have been assigned and give feedback to the submission for each topic.
*If a participant has given feedback to less than 4 topics and chooses to review a submission, the topic with the minimum number of current reviewers will be assigned to that participant.

=== Non-functional Requirements ===

*There must exist a web service that takes the participants and their respective preference lists as input runs a bidding algorithm like top trading cycles and returns a map of the participants and the list of topics assigned to each participant.
*This web service must either be called from the LotteryController or a new controller can be created on the same (if Lottery Controller becomes too complex).

=== Implementing the Web Service ===
*1) Python
*2) Flask (Minimal MVC Framework for Python Backend)
*3) JSON as the Data Serialization Format.
*4) Deployment (AWS or VCL)
*5) Apache Web Server or GUnicorn or Default Flask Server.

=== (Potential) Addition/Modification of files in expertiza ===
*1) Lottery Controller
*2) Review Mapping Controller
*3) A New model to store the bidding information entered by the student.
*4) Unit Test Files Associated
*5) Views/Routes Associated

== '''Test Plan''' ==

*Ensuring that the algorithm results in stable matchings. The concept of stability is defined above.
*Check whether the UI reflects the bids made by the user.
*Testing that the color-coding feature is working and appropriate.
*Check if a user is assigned 4 topics for review, even though he might have bid for any number of topics.
*Regression testing if the code is added in the lottery controller. If new model/controller is added then relevant tests will be included based on the proposal acceptance.
*Tests to ensure that each review topic gets minimum number of reviews.
*Tests to check whether the web service works as expected.

== Project Mentor ==

Ramya Vijayakumar (rvijaya4@ncsu.edu)

== Team Members ==

Yaswanth Soodini (ysoodin@ncsu.edu)

Sai Shruthi Madhuri Kara (skara2@ncsu.edu)

Ayushi Rungta (arungta@ncsu.edu)

Vivek Karri (vkarri@ncsu.edu)

== Important references ==

[https://github.com/peerlogic/IntelligentAssignment/blob/master/app/app.py InteligentAssignment]

[http://ices.gmu.edu/wp-content/uploads/2015/02/Many-to-Many-Matching-Problem-with-Quotas-by-Freer-and-Titova.pdf Many-to-Many-Matching-Problem-with-Quotas-by-Freer-and-Titova]

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Spring_2019_-_Project_E1928._Allow_reviewers_to_bid_on_what_to_review Project_E1928._Allow_reviewers_to_bid_on_what_to_review]

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review E1856_Allow_reviewers_to_bid_on_what_to_review]

CSC/ECE 517 Fall 2019 - E1986. Allow reviewers to bid on what to review

2019-11-16T03:42:04Z

Ysoodin: /* Approach for matching students with topics */

== '''Why Should a Student bid for Reviewing Submissions?''' ==

If the choice of which submission to review is left to student itself, he/she would choose to review those topics which are related to their project or whatever they are highly interested in. This would benefit both the reviewer and reviewer because

From a Student's Perspective:

*Because he/she does not have to spend a lot of time understanding what is going on.
*The Student would be more focused on giving the review and hence judgment made tend to be more objective than subjective.
*This would improve the quality of reviews and will also be helpful for the reviewer.
*By implementing the color-coding feature, the student at any point knows which topic/s are more likely to get and he/she optimizes between "what they want" and "what they can get"
*Also, Bidding in Fun !!!!!!!

From the Instructor's Standpoint:

*Because opinions expressed in the reviews are a result of informed decisions, the inference made from the Machine Learning models trained on these data sets would make more sense.

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

=== Currently Deployed Implementation ===

When an assignment participant wants to review others’ work, they will be asked to choose an assignment topic. The participant will be allowed to choose from among those assignment topics that have not been already assigned to 10 other participants. If they choose the ‘I do not care about the topic I review’ option, a random topic will be chosen from all the available topics and will be assigned to them. That is, the reviews are being allocated on a First Come First Serve basis.

=== Previous Work on this issue ===

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review '''E1856''']

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Spring_2019_-_Project_E1928._Allow_reviewers_to_bid_on_what_to_review '''E1928''']

=== What’s Deeply wrong in those implementations? ===

To understand what's wrong, first consider the differences between Teams Bidding for Assignments and Students Bidding for Reviews. Both of these are Matching or resource Allocation Problems.

The former can be modelled as a one-to-one matching problem. (i.e team and assignment has one-to-one correspondence).

[[File:Teams_Assignments_2.png |center|]]

However, Students and Reviews have many-to-many relationship (a student can choose multiple submissions for review and a submission can be given to multiple students for review)

[[File:Teams_Assignments.png |center|]]

The Mathematical formulation is itself wrong in E1856 and E1928 and they have used the below shown diagram to represent the relationship.

[[File:Teams_Assignments_3.png |center|]]

=== Why does the difference is the representation matter? ===

Since they have modeled the problem on the same lines, they have used the same version of the Gale-Shapley or Top Trading Cycles Algorithm used for one-to-many or one-to-one approach.
Famous Problems Dealt on these lines.

[https://en.wikipedia.org/wiki/Stable_marriage_problem '''Stable Marriage Problem'''] (One-to-One)

[https://www.jstor.org/stable/3132114?seq=1&cid=pdf-reference#references_tab_content '''School Choice Problem'''] (One-to-Many)

=== Other Implementation pitfalls observed ===

'''E1928'''

*Once the bidding for review topics is done, the selections need to be saved to the database which is not happening and when the page is refreshed, the UI does not retain the bids.
*The button responsible for running the algorithm cannot be checked and it is a hunk-like icon.
*The button appears multiple times on the page.
*The algorithm needs to be implemented in the web service which should be ideally be used from the lottery controller which is not they have tried to implement. The entire code is written in Ruby.
*The algorithm needs to skip the review topic that the user has worked on. This Code already exists for topic and again written in the new implementation that needs to be integrated into the existing one and hence needs refactoring.
*Proper tests need to be written.

== '''What needs to be done?''' ==

The participants should be able to bid for topics to review in the same way they bid for topics to work on.

=== Approach for matching students with topics ===

We have a set of students S = {S1, ..., Sn}, and a set of topics T = {T1, ..., Tm}. Each student must get exactly qS topics (The threshold in our case is qS = 4, it’s up to the student to decide how many of these they want to actually review.), while each topic must be assigned to at least qT reviewers. Students will submit a list of preferences over the set of topics in linear order. If they submit the preferences for only a certain topics, the rest of the topics will be appended to their preference list in a random order. The topics shall have (possibly different) linear order preference over the set of students according to the timestamps of their time of bid and the total number of topics they have bid for. Students with lower timestamps and who bid for lower number of topics will be given higher preference.

To begin, we will try to obtain a matching with an approximately equal number of students reviewing each topic, which we call uniform distribution. If no uniform stable matching exists, our mechanism will nonetheless return a stable matching, provided one exists for a given profile of preferences.

The average number of reviewers assigned to each course will be

<math> k = \frac {n times q_S}{m}</math>
where n is the number of students, and m is the number of courses.

Clearly, this number is not necessarily an integer. Let us assume that

p|nqsmp|

where

p| = ⌊nqSm⌋ ℕ

p| = ⌈nqSm⌉ ℕ

Matching 𝝁 is a mapping which assigns exactly qS different topics to each student in Sand at least qT different students to each topic in T. We denote the set of topics assigned by matching 𝜇 to the student si and the set of students who were assigned the topic tj as 𝜇(si) and 𝜇(tj),respectively.

Blocking Pair (Defined according to our problem): A pair (s, t) is called a blocking pair, if the student is associated with the topic (If he/she is in the group which works on topic t). A Matching Set M is considered Stable if it does not have any blocking pairs.

The matching is (pairwise) stable if there are no blocking pairs.

Mechanism:

First we calculate k, p|, and p|. The goal is for each topic to be assigned to either p| and p| students, and to assign exactly qS courses.

The algorithm:

Step 1:
Each topic proposes to accept the first pI students in its preference list. Each student accepts no more than qS proposals according to his/her preferences, rejecting the rest.
Step k:
Each course that has z < pI students proposes to accept pI - z students it has not yet proposed to. Each student accepts no more than qS proposals according to his/her preferences, rejecting the others.

The algorithm stops when every topic that has not reached the maximum quota pI has proposed acceptance to every student.

=== Functional Requirements===
*Each participant should be able to submit a list of preferences in decreasing preference order for the topics they would like to review. The list may contain any number of topics.
*During bidding, the topics should be color-coded according to the number of participants who are contending for each topic. The colors should range from green to red with green representing a topic with a low number of bids and red representing a topic with a high number of bids.
*There should be a deadline before which all participants must submit their list of preferences.
*After the deadline, the course instructor should be able to run the bidding algorithm after which each participant will be assigned a list of topics to review. The number of topics assigned to a participant should be minimum(4,k) where ‘k’ is the number of topics in their preference list.
*Once the topics to review have been assigned, the participants should be able to see those topics they have been assigned and give feedback to the submission for each topic.
*If a participant has given feedback to less than 4 topics and chooses to review a submission, the topic with the minimum number of current reviewers will be assigned to that participant.

=== Non-functional Requirements ===

*There must exist a web service that takes the participants and their respective preference lists as input runs a bidding algorithm like top trading cycles and returns a map of the participants and the list of topics assigned to each participant.
*This web service must either be called from the LotteryController or a new controller can be created on the same (if Lottery Controller becomes too complex).

=== Implementing the Web Service ===
*1) Python
*2) Flask (Minimal MVC Framework for Python Backend)
*3) JSON as the Data Serialization Format.
*4) Deployment (AWS or VCL)
*5) Apache Web Server or GUnicorn or Default Flask Server.

=== (Potential) Addition/Modification of files in expertiza ===
*1) Lottery Controller
*2) Review Mapping Controller
*3) A New model to store the bidding information entered by the student.
*4) Unit Test Files Associated
*5) Views/Routes Associated

== '''Test Plan''' ==

*Ensuring that the algorithm results in stable matchings. The concept of stability is defined above.
*Check whether the UI reflects the bids made by the user.
*Testing that the color-coding feature is working and appropriate.
*Check if a user is assigned 4 topics for review, even though he might have bid for any number of topics.
*Regression testing if the code is added in the lottery controller. If new model/controller is added then relevant tests will be included based on the proposal acceptance.
*Tests to ensure that each review topic gets minimum number of reviews.
*Tests to check whether the web service works as expected.

== Project Mentor ==

Ramya Vijayakumar (rvijaya4@ncsu.edu)

== Team Members ==

Yaswanth Soodini (ysoodin@ncsu.edu)

Sai Shruthi Madhuri Kara (skara2@ncsu.edu)

Ayushi Rungta (arungta@ncsu.edu)

Vivek Karri (vkarri@ncsu.edu)

== Important references ==

[https://github.com/peerlogic/IntelligentAssignment/blob/master/app/app.py InteligentAssignment]

[http://ices.gmu.edu/wp-content/uploads/2015/02/Many-to-Many-Matching-Problem-with-Quotas-by-Freer-and-Titova.pdf Many-to-Many-Matching-Problem-with-Quotas-by-Freer-and-Titova]

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Spring_2019_-_Project_E1928._Allow_reviewers_to_bid_on_what_to_review Project_E1928._Allow_reviewers_to_bid_on_what_to_review]

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review E1856_Allow_reviewers_to_bid_on_what_to_review]

CSC/ECE 517 Fall 2019 - E1986. Allow reviewers to bid on what to review

2019-11-16T03:41:45Z

Ysoodin:

== '''Why Should a Student bid for Reviewing Submissions?''' ==

If the choice of which submission to review is left to student itself, he/she would choose to review those topics which are related to their project or whatever they are highly interested in. This would benefit both the reviewer and reviewer because

From a Student's Perspective:

*Because he/she does not have to spend a lot of time understanding what is going on.
*The Student would be more focused on giving the review and hence judgment made tend to be more objective than subjective.
*This would improve the quality of reviews and will also be helpful for the reviewer.
*By implementing the color-coding feature, the student at any point knows which topic/s are more likely to get and he/she optimizes between "what they want" and "what they can get"
*Also, Bidding in Fun !!!!!!!

From the Instructor's Standpoint:

*Because opinions expressed in the reviews are a result of informed decisions, the inference made from the Machine Learning models trained on these data sets would make more sense.

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

=== Currently Deployed Implementation ===

When an assignment participant wants to review others’ work, they will be asked to choose an assignment topic. The participant will be allowed to choose from among those assignment topics that have not been already assigned to 10 other participants. If they choose the ‘I do not care about the topic I review’ option, a random topic will be chosen from all the available topics and will be assigned to them. That is, the reviews are being allocated on a First Come First Serve basis.

=== Previous Work on this issue ===

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review '''E1856''']

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Spring_2019_-_Project_E1928._Allow_reviewers_to_bid_on_what_to_review '''E1928''']

=== What’s Deeply wrong in those implementations? ===

To understand what's wrong, first consider the differences between Teams Bidding for Assignments and Students Bidding for Reviews. Both of these are Matching or resource Allocation Problems.

The former can be modelled as a one-to-one matching problem. (i.e team and assignment has one-to-one correspondence).

[[File:Teams_Assignments_2.png |center|]]

However, Students and Reviews have many-to-many relationship (a student can choose multiple submissions for review and a submission can be given to multiple students for review)

[[File:Teams_Assignments.png |center|]]

The Mathematical formulation is itself wrong in E1856 and E1928 and they have used the below shown diagram to represent the relationship.

[[File:Teams_Assignments_3.png |center|]]

=== Why does the difference is the representation matter? ===

Since they have modeled the problem on the same lines, they have used the same version of the Gale-Shapley or Top Trading Cycles Algorithm used for one-to-many or one-to-one approach.
Famous Problems Dealt on these lines.

[https://en.wikipedia.org/wiki/Stable_marriage_problem '''Stable Marriage Problem'''] (One-to-One)

[https://www.jstor.org/stable/3132114?seq=1&cid=pdf-reference#references_tab_content '''School Choice Problem'''] (One-to-Many)

=== Other Implementation pitfalls observed ===

'''E1928'''

*Once the bidding for review topics is done, the selections need to be saved to the database which is not happening and when the page is refreshed, the UI does not retain the bids.
*The button responsible for running the algorithm cannot be checked and it is a hunk-like icon.
*The button appears multiple times on the page.
*The algorithm needs to be implemented in the web service which should be ideally be used from the lottery controller which is not they have tried to implement. The entire code is written in Ruby.
*The algorithm needs to skip the review topic that the user has worked on. This Code already exists for topic and again written in the new implementation that needs to be integrated into the existing one and hence needs refactoring.
*Proper tests need to be written.

== '''What needs to be done?''' ==

The participants should be able to bid for topics to review in the same way they bid for topics to work on.

=== Approach for matching students with topics ===

We have a set of students S = {S1, ..., Sn}, and a set of topics T = {T1, ..., Tm}. Each student must get exactly qS topics (The threshold in our case is qS = 4, it’s up to the student to decide how many of these they want to actually review.), while each topic must be assigned to at least qT reviewers. Students will submit a list of preferences over the set of topics in linear order. If they submit the preferences for only a certain topics, the rest of the topics will be appended to their preference list in a random order. The topics shall have (possibly different) linear order preference over the set of students according to the timestamps of their time of bid and the total number of topics they have bid for. Students with lower timestamps and who bid for lower number of topics will be given higher preference.

To begin, we will try to obtain a matching with an approximately equal number of students reviewing each topic, which we call uniform distribution. If no uniform stable matching exists, our mechanism will nonetheless return a stable matching, provided one exists for a given profile of preferences.

The average number of reviewers assigned to each course will be

<math> k = \frac {n times q_S}{m}
where n is the number of students, and m is the number of courses.

Clearly, this number is not necessarily an integer. Let us assume that

p|nqsmp|

where

p| = ⌊nqSm⌋ ℕ

p| = ⌈nqSm⌉ ℕ

Matching 𝝁 is a mapping which assigns exactly qS different topics to each student in Sand at least qT different students to each topic in T. We denote the set of topics assigned by matching 𝜇 to the student si and the set of students who were assigned the topic tj as 𝜇(si) and 𝜇(tj),respectively.

Blocking Pair (Defined according to our problem): A pair (s, t) is called a blocking pair, if the student is associated with the topic (If he/she is in the group which works on topic t). A Matching Set M is considered Stable if it does not have any blocking pairs.

The matching is (pairwise) stable if there are no blocking pairs.

Mechanism:

First we calculate k, p|, and p|. The goal is for each topic to be assigned to either p| and p| students, and to assign exactly qS courses.

The algorithm:

Step 1:
Each topic proposes to accept the first pI students in its preference list. Each student accepts no more than qS proposals according to his/her preferences, rejecting the rest.
Step k:
Each course that has z < pI students proposes to accept pI - z students it has not yet proposed to. Each student accepts no more than qS proposals according to his/her preferences, rejecting the others.

The algorithm stops when every topic that has not reached the maximum quota pI has proposed acceptance to every student.

=== Functional Requirements===
*Each participant should be able to submit a list of preferences in decreasing preference order for the topics they would like to review. The list may contain any number of topics.
*During bidding, the topics should be color-coded according to the number of participants who are contending for each topic. The colors should range from green to red with green representing a topic with a low number of bids and red representing a topic with a high number of bids.
*There should be a deadline before which all participants must submit their list of preferences.
*After the deadline, the course instructor should be able to run the bidding algorithm after which each participant will be assigned a list of topics to review. The number of topics assigned to a participant should be minimum(4,k) where ‘k’ is the number of topics in their preference list.
*Once the topics to review have been assigned, the participants should be able to see those topics they have been assigned and give feedback to the submission for each topic.
*If a participant has given feedback to less than 4 topics and chooses to review a submission, the topic with the minimum number of current reviewers will be assigned to that participant.

=== Non-functional Requirements ===

*There must exist a web service that takes the participants and their respective preference lists as input runs a bidding algorithm like top trading cycles and returns a map of the participants and the list of topics assigned to each participant.
*This web service must either be called from the LotteryController or a new controller can be created on the same (if Lottery Controller becomes too complex).

=== Implementing the Web Service ===
*1) Python
*2) Flask (Minimal MVC Framework for Python Backend)
*3) JSON as the Data Serialization Format.
*4) Deployment (AWS or VCL)
*5) Apache Web Server or GUnicorn or Default Flask Server.

=== (Potential) Addition/Modification of files in expertiza ===
*1) Lottery Controller
*2) Review Mapping Controller
*3) A New model to store the bidding information entered by the student.
*4) Unit Test Files Associated
*5) Views/Routes Associated

== '''Test Plan''' ==

*Ensuring that the algorithm results in stable matchings. The concept of stability is defined above.
*Check whether the UI reflects the bids made by the user.
*Testing that the color-coding feature is working and appropriate.
*Check if a user is assigned 4 topics for review, even though he might have bid for any number of topics.
*Regression testing if the code is added in the lottery controller. If new model/controller is added then relevant tests will be included based on the proposal acceptance.
*Tests to ensure that each review topic gets minimum number of reviews.
*Tests to check whether the web service works as expected.

== Project Mentor ==

Ramya Vijayakumar (rvijaya4@ncsu.edu)

== Team Members ==

Yaswanth Soodini (ysoodin@ncsu.edu)

Sai Shruthi Madhuri Kara (skara2@ncsu.edu)

Ayushi Rungta (arungta@ncsu.edu)

Vivek Karri (vkarri@ncsu.edu)

== Important references ==

[https://github.com/peerlogic/IntelligentAssignment/blob/master/app/app.py InteligentAssignment]

[http://ices.gmu.edu/wp-content/uploads/2015/02/Many-to-Many-Matching-Problem-with-Quotas-by-Freer-and-Titova.pdf Many-to-Many-Matching-Problem-with-Quotas-by-Freer-and-Titova]

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Spring_2019_-_Project_E1928._Allow_reviewers_to_bid_on_what_to_review Project_E1928._Allow_reviewers_to_bid_on_what_to_review]

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review E1856_Allow_reviewers_to_bid_on_what_to_review]

CSC/ECE 517 Fall 2019 - E1986. Allow reviewers to bid on what to review

2019-11-16T03:33:42Z

Ysoodin:

== '''Why Should a Student bid for Reviewing Submissions?''' ==

If the choice of which submission to review is left to student itself, he/she would choose to review those topics which are related to their project or whatever they are highly interested in. This would benefit both the reviewer and reviewer because

From a Student's Perspective:

*Because he/she does not have to spend a lot of time understanding what is going on.
*The Student would be more focused on giving the review and hence judgment made tend to be more objective than subjective.
*This would improve the quality of reviews and will also be helpful for the reviewer.
*By implementing the color-coding feature, the student at any point knows which topic/s are more likely to get and he/she optimizes between "what they want" and "what they can get"
*Also, Bidding in Fun !!!!!!!

From the Instructor's Standpoint:

*Because opinions expressed in the reviews are a result of informed decisions, the inference made from the Machine Learning models trained on these data sets would make more sense.

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

=== Currently Deployed Implementation ===

When an assignment participant wants to review others’ work, they will be asked to choose an assignment topic. The participant will be allowed to choose from among those assignment topics that have not been already assigned to 10 other participants. If they choose the ‘I do not care about the topic I review’ option, a random topic will be chosen from all the available topics and will be assigned to them. That is, the reviews are being allocated on a First Come First Serve basis.

=== Previous Work on this issue ===

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review '''E1856''']

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Spring_2019_-_Project_E1928._Allow_reviewers_to_bid_on_what_to_review '''E1928''']

=== What’s Deeply wrong in those implementations? ===

To understand what's wrong, first consider the differences between Teams Bidding for Assignments and Students Bidding for Reviews. Both of these are Matching or resource Allocation Problems.

The former can be modelled as a one-to-one matching problem. (i.e team and assignment has one-to-one correspondence).

[[File:Teams_Assignments_2.png |center|]]

However, Students and Reviews have many-to-many relationship (a student can choose multiple submissions for review and a submission can be given to multiple students for review)

[[File:Teams_Assignments.png |center|]]

The Mathematical formulation is itself wrong in E1856 and E1928 and they have used the below shown diagram to represent the relationship.

[[File:Teams_Assignments_3.png |center|]]

=== Why does the difference is the representation matter? ===

Since they have modeled the problem on the same lines, they have used the same version of the Gale-Shapley or Top Trading Cycles Algorithm used for one-to-many or one-to-one approach.
Famous Problems Dealt on these lines.

[https://en.wikipedia.org/wiki/Stable_marriage_problem '''Stable Marriage Problem'''] (One-to-One)

[https://www.jstor.org/stable/3132114?seq=1&cid=pdf-reference#references_tab_content '''School Choice Problem'''] (One-to-Many)

=== Other Implementation pitfalls observed ===

'''E1928'''

*Once the bidding for review topics is done, the selections need to be saved to the database which is not happening and when the page is refreshed, the UI does not retain the bids.
*The button responsible for running the algorithm cannot be checked and it is a hunk-like icon.
*The button appears multiple times on the page.
*The algorithm needs to be implemented in the web service which should be ideally be used from the lottery controller which is not they have tried to implement. The entire code is written in Ruby.
*The algorithm needs to skip the review topic that the user has worked on. This Code already exists for topic and again written in the new implementation that needs to be integrated into the existing one and hence needs refactoring.
*Proper tests need to be written.

== '''What needs to be done?''' ==

The participants should be able to bid for topics to review in the same way they bid for topics to work on.

=== Approach for matching students with topics ===

*:<math>q_S</math>Gale-Shapely is Proven to be a stable solution in one-to-one and one-to-many scenarios. While the literature on using Gale-Shapely for Many-to-Many scenarios is not very well defined as a generalized framework, they are a few resources similar to our case.

* The assignment topics each have a quota 10 of reviewers the topic can be assigned to at maximum. Each of the students have a quota 4 of topics they can be assigned at maximum. Gale and Shapely have proposed a mechanism called the Deferred Acceptance (DA) algorithm for finding a stable solution for one-to-one and many-to-one matching problems. The generalization of the DA algorithm other case of many-to-many matching with different preferences and quotas proposed by Freer et al. can be used as our bidding algorithm. Details can be found in the reference below.

[http://ices.gmu.edu/wp-content/uploads/2015/02/Many-to-Many-Matching-Problem-with-Quotas-by-Freer-and-Titova.pdf '''Use Case on applying Gale-Shapely to a Many-to-Many scenario''']

* Although, we were asked to implement Gale-Shapely as a part of this project, the liberty of using any other alternative and a suitable algorithm is in discussion.

=== Functional Requirements===
*Each participant should be able to submit a list of preferences in decreasing preference order for the topics they would like to review. The list may contain any number of topics.
*During bidding, the topics should be color-coded according to the number of participants who are contending for each topic. The colors should range from green to red with green representing a topic with a low number of bids and red representing a topic with a high number of bids.
*There should be a deadline before which all participants must submit their list of preferences.
*After the deadline, the course instructor should be able to run the bidding algorithm after which each participant will be assigned a list of topics to review. The number of topics assigned to a participant should be minimum(4,k) where ‘k’ is the number of topics in their preference list.
*Once the topics to review have been assigned, the participants should be able to see those topics they have been assigned and give feedback to the submission for each topic.
*If a participant has given feedback to less than 4 topics and chooses to review a submission, the topic with the minimum number of current reviewers will be assigned to that participant.

=== Non-functional Requirements ===

*There must exist a web service that takes the participants and their respective preference lists as input runs a bidding algorithm like top trading cycles and returns a map of the participants and the list of topics assigned to each participant.
*This web service must either be called from the LotteryController or a new controller can be created on the same (if Lottery Controller becomes too complex).

=== Implementing the Web Service ===
*1) Python
*2) Flask (Minimal MVC Framework for Python Backend)
*3) JSON as the Data Serialization Format.
*4) Deployment (AWS or VCL)
*5) Apache Web Server or GUnicorn or Default Flask Server.

=== (Potential) Addition/Modification of files in expertiza ===
*1) Lottery Controller
*2) Review Mapping Controller
*3) A New model to store the bidding information entered by the student.
*4) Unit Test Files Associated
*5) Views/Routes Associated

== '''Test Plan''' ==

*Ensuring that the algorithm results in stable matchings. The concept of stability is defined above.
*Check whether the UI reflects the bids made by the user.
*Testing that the color-coding feature is working and appropriate.
*Check if a user is assigned 4 topics for review, even though he might have bid for any number of topics.
*Regression testing if the code is added in the lottery controller. If new model/controller is added then relevant tests will be included based on the proposal acceptance.
*Tests to ensure that each review topic gets minimum number of reviews.
*Tests to check whether the web service works as expected.

== Project Mentor ==

Ramya Vijayakumar (rvijaya4@ncsu.edu)

== Team Members ==

Yaswanth Soodini (ysoodin@ncsu.edu)

Sai Shruthi Madhuri Kara (skara2@ncsu.edu)

Ayushi Rungta (arungta@ncsu.edu)

Vivek Karri (vkarri@ncsu.edu)

== Important references ==

[https://github.com/peerlogic/IntelligentAssignment/blob/master/app/app.py InteligentAssignment]

[http://ices.gmu.edu/wp-content/uploads/2015/02/Many-to-Many-Matching-Problem-with-Quotas-by-Freer-and-Titova.pdf Many-to-Many-Matching-Problem-with-Quotas-by-Freer-and-Titova]

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Spring_2019_-_Project_E1928._Allow_reviewers_to_bid_on_what_to_review Project_E1928._Allow_reviewers_to_bid_on_what_to_review]

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review E1856_Allow_reviewers_to_bid_on_what_to_review]

CSC/ECE 517 Fall 2019 - E1986. Allow reviewers to bid on what to review

2019-11-16T03:25:11Z

Ysoodin:

== '''Why Should a Student bid for Reviewing Submissions?''' ==

If the choice of which submission to review is left to student itself, he/she would choose to review those topics which are related to their project or whatever they are highly interested in. This would benefit both the reviewer and reviewer because

From a Student's Perspective:

*Because he/she does not have to spend a lot of time understanding what is going on.
*The Student would be more focused on giving the review and hence judgment made tend to be more objective than subjective.
*This would improve the quality of reviews and will also be helpful for the reviewer.
*By implementing the color-coding feature, the student at any point knows which topic/s are more likely to get and he/she optimizes between "what they want" and "what they can get"
*Also, Bidding in Fun !!!!!!!

From the Instructor's Standpoint:

*Because opinions expressed in the reviews are a result of informed decisions, the inference made from the Machine Learning models trained on these data sets would make more sense.

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

=== Currently Deployed Implementation ===

When an assignment participant wants to review others’ work, they will be asked to choose an assignment topic. The participant will be allowed to choose from among those assignment topics that have not been already assigned to 10 other participants. If they choose the ‘I do not care about the topic I review’ option, a random topic will be chosen from all the available topics and will be assigned to them. That is, the reviews are being allocated on a First Come First Serve basis.

=== Previous Work on this issue ===

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review '''E1856''']

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Spring_2019_-_Project_E1928._Allow_reviewers_to_bid_on_what_to_review '''E1928''']

=== What’s Deeply wrong in those implementations? ===

To understand what's wrong, first consider the differences between Teams Bidding for Assignments and Students Bidding for Reviews. Both of these are Matching or resource Allocation Problems.

The former can be modelled as a one-to-one matching problem. (i.e team and assignment has one-to-one correspondence).

[[File:Teams_Assignments_2.png |center|]]

However, Students and Reviews have many-to-many relationship (a student can choose multiple submissions for review and a submission can be given to multiple students for review)

[[File:Teams_Assignments.png |center|]]

The Mathematical formulation is itself wrong in E1856 and E1928 and they have used the below shown diagram to represent the relationship.

[[File:Teams_Assignments_3.png |center|]]

=== Why does the difference is the representation matter? ===

Since they have modeled the problem on the same lines, they have used the same version of the Gale-Shapley or Top Trading Cycles Algorithm used for one-to-many or one-to-one approach.
Famous Problems Dealt on these lines.

[https://en.wikipedia.org/wiki/Stable_marriage_problem '''Stable Marriage Problem'''] (One-to-One)

[https://www.jstor.org/stable/3132114?seq=1&cid=pdf-reference#references_tab_content '''School Choice Problem'''] (One-to-Many)

=== Other Implementation pitfalls observed ===

'''E1928'''

*Once the bidding for review topics is done, the selections need to be saved to the database which is not happening and when the page is refreshed, the UI does not retain the bids.
*The button responsible for running the algorithm cannot be checked and it is a hunk-like icon.
*The button appears multiple times on the page.
*The algorithm needs to be implemented in the web service which should be ideally be used from the lottery controller which is not they have tried to implement. The entire code is written in Ruby.
*The algorithm needs to skip the review topic that the user has worked on. This Code already exists for topic and again written in the new implementation that needs to be integrated into the existing one and hence needs refactoring.
*Proper tests need to be written.

== '''What needs to be done?''' ==

The participants should be able to bid for topics to review in the same way they bid for topics to work on.

=== Approach for matching students with topics ===

*Gale-Shapely is Proven to be a stable solution in one-to-one and one-to-many scenarios. While the literature on using Gale-Shapely for Many-to-Many scenarios is not very well defined as a generalized framework, they are a few resources similar to our case.

* The assignment topics each have a quota 10 of reviewers the topic can be assigned to at maximum. Each of the students have a quota 4 of topics they can be assigned at maximum. Gale and Shapely have proposed a mechanism called the Deferred Acceptance (DA) algorithm for finding a stable solution for one-to-one and many-to-one matching problems. The generalization of the DA algorithm other case of many-to-many matching with different preferences and quotas proposed by Freer et al. can be used as our bidding algorithm. Details can be found in the reference below.

[http://ices.gmu.edu/wp-content/uploads/2015/02/Many-to-Many-Matching-Problem-with-Quotas-by-Freer-and-Titova.pdf '''Use Case on applying Gale-Shapely to a Many-to-Many scenario''']

* Although, we were asked to implement Gale-Shapely as a part of this project, the liberty of using any other alternative and a suitable algorithm is in discussion.

=== Functional Requirements===
*Each participant should be able to submit a list of preferences in decreasing preference order for the topics they would like to review. The list may contain any number of topics.
*During bidding, the topics should be color-coded according to the number of participants who are contending for each topic. The colors should range from green to red with green representing a topic with a low number of bids and red representing a topic with a high number of bids.
*There should be a deadline before which all participants must submit their list of preferences.
*After the deadline, the course instructor should be able to run the bidding algorithm after which each participant will be assigned a list of topics to review. The number of topics assigned to a participant should be minimum(4,k) where ‘k’ is the number of topics in their preference list.
*Once the topics to review have been assigned, the participants should be able to see those topics they have been assigned and give feedback to the submission for each topic.
*If a participant has given feedback to less than 4 topics and chooses to review a submission, the topic with the minimum number of current reviewers will be assigned to that participant.

=== Non-functional Requirements ===

*There must exist a web service that takes the participants and their respective preference lists as input runs a bidding algorithm like top trading cycles and returns a map of the participants and the list of topics assigned to each participant.
*This web service must either be called from the LotteryController or a new controller can be created on the same (if Lottery Controller becomes too complex).

=== Implementing the Web Service ===
*1) Python
*2) Flask (Minimal MVC Framework for Python Backend)
*3) JSON as the Data Serialization Format.
*4) Deployment (AWS or VCL)
*5) Apache Web Server or GUnicorn or Default Flask Server.

=== (Potential) Addition/Modification of files in expertiza ===
*1) Lottery Controller
*2) Review Mapping Controller
*3) A New model to store the bidding information entered by the student.
*4) Unit Test Files Associated
*5) Views/Routes Associated

== '''Test Plan''' ==

*Ensuring that the algorithm results in stable matchings. The concept of stability is defined above.
*Check whether the UI reflects the bids made by the user.
*Testing that the color-coding feature is working and appropriate.
*Check if a user is assigned 4 topics for review, even though he might have bid for any number of topics.
*Regression testing if the code is added in the lottery controller. If new model/controller is added then relevant tests will be included based on the proposal acceptance.
*Tests to ensure that each review topic gets minimum number of reviews.
*Tests to check whether the web service works as expected.

== Project Mentor ==

Ramya Vijayakumar (rvijaya4@ncsu.edu)

== Team Members ==

Yaswanth Soodini (ysoodin@ncsu.edu)

Sai Shruthi Madhuri Kara (skara2@ncsu.edu)

Ayushi Rungta (arungta@ncsu.edu)

Vivek Karri (vkarri@ncsu.edu)

== Important references ==

[https://github.com/peerlogic/IntelligentAssignment/blob/master/app/app.py InteligentAssignment]

[http://ices.gmu.edu/wp-content/uploads/2015/02/Many-to-Many-Matching-Problem-with-Quotas-by-Freer-and-Titova.pdf Many-to-Many-Matching-Problem-with-Quotas-by-Freer-and-Titova]

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Spring_2019_-_Project_E1928._Allow_reviewers_to_bid_on_what_to_review Project_E1928._Allow_reviewers_to_bid_on_what_to_review]

[http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review E1856_Allow_reviewers_to_bid_on_what_to_review]

CSC/ECE 517 Fall 2019 - E1944. Refactor review mapping controller

2019-11-12T20:26:05Z

Ysoodin: /* Links for Demo Videos */

This wiki page is for the description of changes made under E1944 OSS assignment for Fall 2019, CSC/ECE 517.
sd
== About 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.

== Description of the project ==

The focus of the project is on a controller named ReviewMappingController and the primary goal is to make changes to the internal structure of the controller to make it easier to read and cheaper to maintain without changing its observable behavior. This can be achieved through refactoring some of the more complex methods, modifying some of the language to make it more Ruby friendly, removing redundant code, etc.

The number of routes directed to review_mapping_controller is illustrated here and since it is intertwined with a lot of other parts in the code base, it is quite imperative that this code needs to be refactored.

Link to the Pull Request Submitted: [https://github.com/expertiza/expertiza/pull/1538]

[[File:complexity.png]]

== Work-Plan Followed ==
[[File:FlowchartOODD.jpg]]
 
We were tasked to refactor the review_mapping_controller.rb and solve any cascading issues or bugs we could find. We followed the above work plan to complete this task. There were many times when all the Rspec and Cucumber tests passed locally but ran into an issue when we uploaded the changes on GitHub. Prompt feedback from the TRAVIS CI helped us recognize the issue. Then we went on local machine and followed the whole process of refactoring again. In this way, we covered every refactoring we did and ensured that the TRAVIS CI get passed with minimum issues on the code-climate.

== Files modified/created in the current project ==

1. review_mapping_controller.rb 
2. review_mapping_controller_spec.rb 
3. assign_quiz_controller.rb 
4. assign_quiz_controller_spec.rb 
5. review_response_map_controller.rb 
6. review_response_map_controller_spec.rb 
7. routes.rb 
8. Other views & partials associated affected by these changes 

=== ReviewMappingController ===

This controller will map the submissions made by the teams to the students for facilitating peer-reviewing. A couple of '''long''' and '''complex methods''' such as '''peer_review_strategy''' and '''automatic_review_mapping''' were refactored from this controller along with the '''removal''' of some '''non-related methods''' such as '''add_calibration''' and '''assign_quiz_dynamically'''. Variable names have been changed and code has been modularized and helper methods were separated from the important methods into a module and were included in the class.

Test Cases were created for the newly created controllers such as assign_quiz_controller etc.

=== review_mapping_controller_spec.rb ===

After refactoring the '''Review_Mapping_Controller.rb''', there were some tests still present in the spec file of this controller. So, we '''removed such tests''' from the '''review_mapping_controller_spec.rb''' to the appropriate spec file.

=== AssignQuizController ===

'''assign_quiz_dynamically''' (Quizzes are also stored in the Assignment table) is not a seemingly/semantically related task to review mapping. Hence this was moved into a separate controller.

=== assign_quiz_controller_spec.rb ===

Tests related to assign_quiz_controller were moved into this file.

=== ReviewResponseMapController.rb ===

'''Add_Calibration''' is a nuanced method and has seemingly '''different functionality''' than '''Review Mapping''' Controller. Methods having a different purpose than review_mapping or helping review_mapping should not be present in this controller. So we '''moved''' this method '''into''' a '''separate controller'''.

=== review_response_map_controller_spec.rb ===

Tests related to review_response_map were moved into this file.

=== routes.rb ===

New routes were added to newly created controllers.

<pre>
assign_quiz_dynamically_assign_quiz_index POST /assign_quiz/assign_quiz_dynamically(.:format) assign_quiz#assign_quiz_dynamically
</pre>

=== views/partials ===

Routes were changed in the views and partials.

==== Modified View Files: ====
app/views/student_quizzes/_set_dynamic_quiz.html.erb 
app/views/assignments/edit/_calibration.html.erb 

== Details of the changes made==
1. Changed 'instructor = build(:instructor)' to ‘@instructor = build(:instructor, id: 1)’. 
       *Removed code redundancy from review_mapping_controller_spec.rb. Two variables were being initialized containing the same value. One was in the before(:each) loop and other was being called in first three test cases. 
       *Replaced the one in the before(:each) loop by @instructor = build(:instructor, id: 1) and used @instructor class variable, wherever required. 

2. Changed :i_dont_care to :no_particular_topic 
       *:i_dont_care was used in the /app/views/student_review/_set_dynamic_review.html.erb as a flag to store if student is interested in any particular topic or doesn't care which topic to review. 
       *It was also used in review_mapping_controller.rb to check if student has selected any particular topic. 
       *Since, name :i_dont_care was very difficult to understand, we replaced it with something logical such as :no_particular_topic. It gives hint about what the symbol stores. 

3. Removed cascading effects of above change from features spec 
       *Above changes caused ./spec/features/review_assignment_spec.rb this feature test to fail. 
       *This spec has used the above symbol to check if the list of available topics collapse or not after selecting the I don't care option. before: [[File:3bef.jpg]] after: [[File:3aft.jpg]] 

4. Created a variable named ‘allowed_actions’ in method choose_case(action_in_params) 
       *Switch case in above method from review_mapping_controller.rb contained all the actions having the same output for around 70% of cases. 
       *Hence, replaced the switch statements and initialized a list with those switch cases. 
       *Even the space complexity increased, the tradeoff got balanced because if someone has to change some actions, he will have to just add or remove the action name from the allowed_actions list. 
before: [[File:4bef.jpg]] after: [[File:4aft.jpg]] 

5. Refactored Peer_review_strategy by using a helper method gen_random_participant_id 
       *A random participant_id is generated from the possible pool of candidates but the code block for that is kind of a query, i.e. it does not change or set anything. 
       *And it is equally complex enough to confuse the reader. So this has been put into a helper method with an expressive name to increase readability. 
<pre>
## Helper Method for generating a random participant which is to be used in peer_review_strategy method.
def gen_random_participant_id(iterator, participants_hash, num_participants, participants)
if iterator.zero?
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 = []
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
no_particpants = 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
participant_is_owner = (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))
rand_num = if no_particpants or participant_is_owner
# use original method to get random number
rand(0..num_participants - 1)
else
# rand_num should be the position of this participant in original array
participants_with_min_assigned_reviews[rand(0..participants_with_min_assigned_reviews.size - 1)]
end
end
return rand_num
end
</pre>
6. Refactored automatic_review_mapping by using a helper method check_num_reviews_args 
       *Parameters such as num_reviews_per_student, num_calibrated_artifacts etc passed are first verified to check if they are in acceptable range or pattern 
       *To increase readability, the not-so good looking sets of if-else statements have been moved into check_num_reviews_args method. 
<pre>
# Helper Method to check num_reviews_per_student and num_reviews_per_submission arguments passed in by params hash.
def check_num_reviews_args(num_reviews_per_student, num_reviews_per_submission, teams)
has_error_not_raised = true
# check for exit paths first
if num_reviews_per_student == 0 and num_reviews_per_submission == 0
flash[:error] = "Please choose either the number of reviews per student or the number of reviewers per team (student)."
has_error_not_raised = false
elsif num_reviews_per_student != 0 and num_reviews_per_submission != 0
flash[:error] = "Please choose either the number of reviews per student or the number of reviewers per team (student), not both."
has_error_not_raised = false
elsif num_reviews_per_student >= teams.size
# Exception detection: If instructor want to assign too many reviews done
# by each student, there will be an error msg.
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].'
has_error_not_raised = false
end
end
</pre>
7. Modularized helper methods into a module and was mixed in the ReviewMappingController Class. 
       *Only some of the methods written in the class have external usage i.e called by another controllers, views etc. 
       *The other methods are just helpers and as such moved into a Helper method module and mixed in the class 
Before Modularization
<pre>
class ReviewMappingController < ApplicationController
...................
...................
512 Lines & 25 Methods Defined (After moving a few methods into separate controllers)
...................
...................
end
</pre>
After Modularization
<pre>
module Helper_methods
...................
5 Methods and 170 lines
...................
end

class ReviewMappingController < ApplicationController
include Helper_methods
...................
340 Lines & 20 Methods (All of those are used elsewhere directly in the application)
...................
end
</pre>

8. Abided to the principles of Magic Tricks of testing and did not test any internally used methods, The other tests are written were already following this principle. 
       *Internally used methods were not tested 
       *Tests for newly created controllers have been moved into a separate spec files. 

9. Isolated AssignQuizDynamically method into a separate controller as the functionality was not related to ReviewMappingController. 
       *This method assigns a quiz(Stored as an assignment object) to the participant. 
       *This method is not related to ReviewMapping functionality, so it was made into a new controller. 

10. Associated Specs/routes/views/partials have been modified to adapt the change in controllers. 
       *The controller paths present in the views/partials have to be changed to not to break the functionality 
       *The controller paths in views/partials/routes have been changed for the newly created controller of AssignQuizDynamically 

View File Affected by the creation of AssinQuizController
[[File:View.png]]

11. student_review_num sounds like a number or an id associated with a student review, while it actually stores the number of reviews that a student can perform. So it is renamed num_reviews_per_student. 
12. submission_review_num sounds like a number or an id associated with a submission review, while it actually stores the total number of reviews that can be performed on a single submission. So it is renamed num_reviews_per_submission. 
13. calibrated_artifacts_num sounds like a number or an id associated with the calibrated artifacts, while it actually stores the number of calibrated artifacts. So it is renamed num_calibrated_artifacts. Similarly, uncalibrated_artifacts_num is renamed num_uncalibrated_artifacts. 
14. participants_hash is not an appropriate name for a hash whose keys are participant ids and values are number of reviews performed by corresponding participants. So it is renamed num_reviews_by_participant_hash. 
15. Extracted a method make_review_strategy (from automatic_review_mapping_strategy) that returns a review_strategy based on the values of num_reviews_per_submission and num_reviews_per_submission. 
16. add_calibration is a method that changes the attribute of a ReviewResponseMap and has little to do with Review Mapping. So it is now put in a separate controller named ReviewResponseMapController. 
17. The name add_reviewer may lead the reader to think that the method adds a reviewer to a collection of reviewers (e.g. a list of reviewers). Changing the name to assign_reviewer_manually informs the reader that the method assigns a reviewer (to a submission) and hence improves the readability of the code.

== Test Plan ==

=== Rspec Unit Tests ===

Since this is a Refactoring Project, We made sure that the changes made did not break any functionality.

[[File:testplan.png]]

Note: Tests have been run for three controllers (one existing and two new).

=== Capybara Integration and Functional Tests ===

As the controller routes have been modified in the routes.rb and the other view files, there are potential chances of failures in Integration tests.

However, no such failure has been reported by Travis build.

[[File:Travis CI.png]]

[[File:FeaturesCI.png]]

== Code Coverage ==

Code Coverage for Controllers section climbed up.

[https://coveralls.io/builds/26607665] # Link for the COVERALLS stats of our pull request.

[[File:CCC1.png]]

== Project Mentor ==

Ramya Vijayakumar (rvijaya4@ncsu.edu)

== Team Members ==

Yaswanth Soodini (ysoodin@ncsu.edu)

Saurabh Shingte (svshingt@ncsu.edu)

Vivek Karri (vkarri@ncsu.edu)

== Links for Demo Videos ==

Assign Reviewer Demo[https://drive.google.com/file/d/1o9vJQ7fCwk0hwHJdErwpg1z_XjsN_XRT/view?usp=sharing]

CSC/ECE 517 Fall 2019 - E1944. Refactor review mapping controller

2019-11-12T20:25:40Z

Ysoodin: /* Links for Demo Videos */

This wiki page is for the description of changes made under E1944 OSS assignment for Fall 2019, CSC/ECE 517.
sd
== About 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.

== Description of the project ==

The focus of the project is on a controller named ReviewMappingController and the primary goal is to make changes to the internal structure of the controller to make it easier to read and cheaper to maintain without changing its observable behavior. This can be achieved through refactoring some of the more complex methods, modifying some of the language to make it more Ruby friendly, removing redundant code, etc.

The number of routes directed to review_mapping_controller is illustrated here and since it is intertwined with a lot of other parts in the code base, it is quite imperative that this code needs to be refactored.

Link to the Pull Request Submitted: [https://github.com/expertiza/expertiza/pull/1538]

[[File:complexity.png]]

== Work-Plan Followed ==
[[File:FlowchartOODD.jpg]]
 
We were tasked to refactor the review_mapping_controller.rb and solve any cascading issues or bugs we could find. We followed the above work plan to complete this task. There were many times when all the Rspec and Cucumber tests passed locally but ran into an issue when we uploaded the changes on GitHub. Prompt feedback from the TRAVIS CI helped us recognize the issue. Then we went on local machine and followed the whole process of refactoring again. In this way, we covered every refactoring we did and ensured that the TRAVIS CI get passed with minimum issues on the code-climate.

== Files modified/created in the current project ==

1. review_mapping_controller.rb 
2. review_mapping_controller_spec.rb 
3. assign_quiz_controller.rb 
4. assign_quiz_controller_spec.rb 
5. review_response_map_controller.rb 
6. review_response_map_controller_spec.rb 
7. routes.rb 
8. Other views & partials associated affected by these changes 

=== ReviewMappingController ===

This controller will map the submissions made by the teams to the students for facilitating peer-reviewing. A couple of '''long''' and '''complex methods''' such as '''peer_review_strategy''' and '''automatic_review_mapping''' were refactored from this controller along with the '''removal''' of some '''non-related methods''' such as '''add_calibration''' and '''assign_quiz_dynamically'''. Variable names have been changed and code has been modularized and helper methods were separated from the important methods into a module and were included in the class.

Test Cases were created for the newly created controllers such as assign_quiz_controller etc.

=== review_mapping_controller_spec.rb ===

After refactoring the '''Review_Mapping_Controller.rb''', there were some tests still present in the spec file of this controller. So, we '''removed such tests''' from the '''review_mapping_controller_spec.rb''' to the appropriate spec file.

=== AssignQuizController ===

'''assign_quiz_dynamically''' (Quizzes are also stored in the Assignment table) is not a seemingly/semantically related task to review mapping. Hence this was moved into a separate controller.

=== assign_quiz_controller_spec.rb ===

Tests related to assign_quiz_controller were moved into this file.

=== ReviewResponseMapController.rb ===

'''Add_Calibration''' is a nuanced method and has seemingly '''different functionality''' than '''Review Mapping''' Controller. Methods having a different purpose than review_mapping or helping review_mapping should not be present in this controller. So we '''moved''' this method '''into''' a '''separate controller'''.

=== review_response_map_controller_spec.rb ===

Tests related to review_response_map were moved into this file.

=== routes.rb ===

New routes were added to newly created controllers.

<pre>
assign_quiz_dynamically_assign_quiz_index POST /assign_quiz/assign_quiz_dynamically(.:format) assign_quiz#assign_quiz_dynamically
</pre>

=== views/partials ===

Routes were changed in the views and partials.

==== Modified View Files: ====
app/views/student_quizzes/_set_dynamic_quiz.html.erb 
app/views/assignments/edit/_calibration.html.erb 

== Details of the changes made==
1. Changed 'instructor = build(:instructor)' to ‘@instructor = build(:instructor, id: 1)’. 
       *Removed code redundancy from review_mapping_controller_spec.rb. Two variables were being initialized containing the same value. One was in the before(:each) loop and other was being called in first three test cases. 
       *Replaced the one in the before(:each) loop by @instructor = build(:instructor, id: 1) and used @instructor class variable, wherever required. 

2. Changed :i_dont_care to :no_particular_topic 
       *:i_dont_care was used in the /app/views/student_review/_set_dynamic_review.html.erb as a flag to store if student is interested in any particular topic or doesn't care which topic to review. 
       *It was also used in review_mapping_controller.rb to check if student has selected any particular topic. 
       *Since, name :i_dont_care was very difficult to understand, we replaced it with something logical such as :no_particular_topic. It gives hint about what the symbol stores. 

3. Removed cascading effects of above change from features spec 
       *Above changes caused ./spec/features/review_assignment_spec.rb this feature test to fail. 
       *This spec has used the above symbol to check if the list of available topics collapse or not after selecting the I don't care option. before: [[File:3bef.jpg]] after: [[File:3aft.jpg]] 

4. Created a variable named ‘allowed_actions’ in method choose_case(action_in_params) 
       *Switch case in above method from review_mapping_controller.rb contained all the actions having the same output for around 70% of cases. 
       *Hence, replaced the switch statements and initialized a list with those switch cases. 
       *Even the space complexity increased, the tradeoff got balanced because if someone has to change some actions, he will have to just add or remove the action name from the allowed_actions list. 
before: [[File:4bef.jpg]] after: [[File:4aft.jpg]] 

5. Refactored Peer_review_strategy by using a helper method gen_random_participant_id 
       *A random participant_id is generated from the possible pool of candidates but the code block for that is kind of a query, i.e. it does not change or set anything. 
       *And it is equally complex enough to confuse the reader. So this has been put into a helper method with an expressive name to increase readability. 
<pre>
## Helper Method for generating a random participant which is to be used in peer_review_strategy method.
def gen_random_participant_id(iterator, participants_hash, num_participants, participants)
if iterator.zero?
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 = []
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
no_particpants = 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
participant_is_owner = (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))
rand_num = if no_particpants or participant_is_owner
# use original method to get random number
rand(0..num_participants - 1)
else
# rand_num should be the position of this participant in original array
participants_with_min_assigned_reviews[rand(0..participants_with_min_assigned_reviews.size - 1)]
end
end
return rand_num
end
</pre>
6. Refactored automatic_review_mapping by using a helper method check_num_reviews_args 
       *Parameters such as num_reviews_per_student, num_calibrated_artifacts etc passed are first verified to check if they are in acceptable range or pattern 
       *To increase readability, the not-so good looking sets of if-else statements have been moved into check_num_reviews_args method. 
<pre>
# Helper Method to check num_reviews_per_student and num_reviews_per_submission arguments passed in by params hash.
def check_num_reviews_args(num_reviews_per_student, num_reviews_per_submission, teams)
has_error_not_raised = true
# check for exit paths first
if num_reviews_per_student == 0 and num_reviews_per_submission == 0
flash[:error] = "Please choose either the number of reviews per student or the number of reviewers per team (student)."
has_error_not_raised = false
elsif num_reviews_per_student != 0 and num_reviews_per_submission != 0
flash[:error] = "Please choose either the number of reviews per student or the number of reviewers per team (student), not both."
has_error_not_raised = false
elsif num_reviews_per_student >= teams.size
# Exception detection: If instructor want to assign too many reviews done
# by each student, there will be an error msg.
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].'
has_error_not_raised = false
end
end
</pre>
7. Modularized helper methods into a module and was mixed in the ReviewMappingController Class. 
       *Only some of the methods written in the class have external usage i.e called by another controllers, views etc. 
       *The other methods are just helpers and as such moved into a Helper method module and mixed in the class 
Before Modularization
<pre>
class ReviewMappingController < ApplicationController
...................
...................
512 Lines & 25 Methods Defined (After moving a few methods into separate controllers)
...................
...................
end
</pre>
After Modularization
<pre>
module Helper_methods
...................
5 Methods and 170 lines
...................
end

class ReviewMappingController < ApplicationController
include Helper_methods
...................
340 Lines & 20 Methods (All of those are used elsewhere directly in the application)
...................
end
</pre>

8. Abided to the principles of Magic Tricks of testing and did not test any internally used methods, The other tests are written were already following this principle. 
       *Internally used methods were not tested 
       *Tests for newly created controllers have been moved into a separate spec files. 

9. Isolated AssignQuizDynamically method into a separate controller as the functionality was not related to ReviewMappingController. 
       *This method assigns a quiz(Stored as an assignment object) to the participant. 
       *This method is not related to ReviewMapping functionality, so it was made into a new controller. 

10. Associated Specs/routes/views/partials have been modified to adapt the change in controllers. 
       *The controller paths present in the views/partials have to be changed to not to break the functionality 
       *The controller paths in views/partials/routes have been changed for the newly created controller of AssignQuizDynamically 

View File Affected by the creation of AssinQuizController
[[File:View.png]]

11. student_review_num sounds like a number or an id associated with a student review, while it actually stores the number of reviews that a student can perform. So it is renamed num_reviews_per_student. 
12. submission_review_num sounds like a number or an id associated with a submission review, while it actually stores the total number of reviews that can be performed on a single submission. So it is renamed num_reviews_per_submission. 
13. calibrated_artifacts_num sounds like a number or an id associated with the calibrated artifacts, while it actually stores the number of calibrated artifacts. So it is renamed num_calibrated_artifacts. Similarly, uncalibrated_artifacts_num is renamed num_uncalibrated_artifacts. 
14. participants_hash is not an appropriate name for a hash whose keys are participant ids and values are number of reviews performed by corresponding participants. So it is renamed num_reviews_by_participant_hash. 
15. Extracted a method make_review_strategy (from automatic_review_mapping_strategy) that returns a review_strategy based on the values of num_reviews_per_submission and num_reviews_per_submission. 
16. add_calibration is a method that changes the attribute of a ReviewResponseMap and has little to do with Review Mapping. So it is now put in a separate controller named ReviewResponseMapController. 
17. The name add_reviewer may lead the reader to think that the method adds a reviewer to a collection of reviewers (e.g. a list of reviewers). Changing the name to assign_reviewer_manually informs the reader that the method assigns a reviewer (to a submission) and hence improves the readability of the code.

== Test Plan ==

=== Rspec Unit Tests ===

Since this is a Refactoring Project, We made sure that the changes made did not break any functionality.

[[File:testplan.png]]

Note: Tests have been run for three controllers (one existing and two new).

=== Capybara Integration and Functional Tests ===

As the controller routes have been modified in the routes.rb and the other view files, there are potential chances of failures in Integration tests.

However, no such failure has been reported by Travis build.

[[File:Travis CI.png]]

[[File:FeaturesCI.png]]

== Code Coverage ==

Code Coverage for Controllers section climbed up.

[https://coveralls.io/builds/26607665] # Link for the COVERALLS stats of our pull request.

[[File:CCC1.png]]

== Project Mentor ==

Ramya Vijayakumar (rvijaya4@ncsu.edu)

== Team Members ==

Yaswanth Soodini (ysoodin@ncsu.edu)

Saurabh Shingte (svshingt@ncsu.edu)

Vivek Karri (vkarri@ncsu.edu)

== Links for Demo Videos ==

[Assign Reviewer Demo]

CSC/ECE 517 Fall 2019 - E1944. Refactor review mapping controller

2019-11-12T20:24:43Z

Ysoodin:

This wiki page is for the description of changes made under E1944 OSS assignment for Fall 2019, CSC/ECE 517.
sd
== About 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.

== Description of the project ==

The focus of the project is on a controller named ReviewMappingController and the primary goal is to make changes to the internal structure of the controller to make it easier to read and cheaper to maintain without changing its observable behavior. This can be achieved through refactoring some of the more complex methods, modifying some of the language to make it more Ruby friendly, removing redundant code, etc.

The number of routes directed to review_mapping_controller is illustrated here and since it is intertwined with a lot of other parts in the code base, it is quite imperative that this code needs to be refactored.

Link to the Pull Request Submitted: [https://github.com/expertiza/expertiza/pull/1538]

[[File:complexity.png]]

== Work-Plan Followed ==
[[File:FlowchartOODD.jpg]]
 
We were tasked to refactor the review_mapping_controller.rb and solve any cascading issues or bugs we could find. We followed the above work plan to complete this task. There were many times when all the Rspec and Cucumber tests passed locally but ran into an issue when we uploaded the changes on GitHub. Prompt feedback from the TRAVIS CI helped us recognize the issue. Then we went on local machine and followed the whole process of refactoring again. In this way, we covered every refactoring we did and ensured that the TRAVIS CI get passed with minimum issues on the code-climate.

== Files modified/created in the current project ==

1. review_mapping_controller.rb 
2. review_mapping_controller_spec.rb 
3. assign_quiz_controller.rb 
4. assign_quiz_controller_spec.rb 
5. review_response_map_controller.rb 
6. review_response_map_controller_spec.rb 
7. routes.rb 
8. Other views & partials associated affected by these changes 

=== ReviewMappingController ===

This controller will map the submissions made by the teams to the students for facilitating peer-reviewing. A couple of '''long''' and '''complex methods''' such as '''peer_review_strategy''' and '''automatic_review_mapping''' were refactored from this controller along with the '''removal''' of some '''non-related methods''' such as '''add_calibration''' and '''assign_quiz_dynamically'''. Variable names have been changed and code has been modularized and helper methods were separated from the important methods into a module and were included in the class.

Test Cases were created for the newly created controllers such as assign_quiz_controller etc.

=== review_mapping_controller_spec.rb ===

After refactoring the '''Review_Mapping_Controller.rb''', there were some tests still present in the spec file of this controller. So, we '''removed such tests''' from the '''review_mapping_controller_spec.rb''' to the appropriate spec file.

=== AssignQuizController ===

'''assign_quiz_dynamically''' (Quizzes are also stored in the Assignment table) is not a seemingly/semantically related task to review mapping. Hence this was moved into a separate controller.

=== assign_quiz_controller_spec.rb ===

Tests related to assign_quiz_controller were moved into this file.

=== ReviewResponseMapController.rb ===

'''Add_Calibration''' is a nuanced method and has seemingly '''different functionality''' than '''Review Mapping''' Controller. Methods having a different purpose than review_mapping or helping review_mapping should not be present in this controller. So we '''moved''' this method '''into''' a '''separate controller'''.

=== review_response_map_controller_spec.rb ===

Tests related to review_response_map were moved into this file.

=== routes.rb ===

New routes were added to newly created controllers.

<pre>
assign_quiz_dynamically_assign_quiz_index POST /assign_quiz/assign_quiz_dynamically(.:format) assign_quiz#assign_quiz_dynamically
</pre>

=== views/partials ===

Routes were changed in the views and partials.

==== Modified View Files: ====
app/views/student_quizzes/_set_dynamic_quiz.html.erb 
app/views/assignments/edit/_calibration.html.erb 

== Details of the changes made==
1. Changed 'instructor = build(:instructor)' to ‘@instructor = build(:instructor, id: 1)’. 
       *Removed code redundancy from review_mapping_controller_spec.rb. Two variables were being initialized containing the same value. One was in the before(:each) loop and other was being called in first three test cases. 
       *Replaced the one in the before(:each) loop by @instructor = build(:instructor, id: 1) and used @instructor class variable, wherever required. 

2. Changed :i_dont_care to :no_particular_topic 
       *:i_dont_care was used in the /app/views/student_review/_set_dynamic_review.html.erb as a flag to store if student is interested in any particular topic or doesn't care which topic to review. 
       *It was also used in review_mapping_controller.rb to check if student has selected any particular topic. 
       *Since, name :i_dont_care was very difficult to understand, we replaced it with something logical such as :no_particular_topic. It gives hint about what the symbol stores. 

3. Removed cascading effects of above change from features spec 
       *Above changes caused ./spec/features/review_assignment_spec.rb this feature test to fail. 
       *This spec has used the above symbol to check if the list of available topics collapse or not after selecting the I don't care option. before: [[File:3bef.jpg]] after: [[File:3aft.jpg]] 

4. Created a variable named ‘allowed_actions’ in method choose_case(action_in_params) 
       *Switch case in above method from review_mapping_controller.rb contained all the actions having the same output for around 70% of cases. 
       *Hence, replaced the switch statements and initialized a list with those switch cases. 
       *Even the space complexity increased, the tradeoff got balanced because if someone has to change some actions, he will have to just add or remove the action name from the allowed_actions list. 
before: [[File:4bef.jpg]] after: [[File:4aft.jpg]] 

5. Refactored Peer_review_strategy by using a helper method gen_random_participant_id 
       *A random participant_id is generated from the possible pool of candidates but the code block for that is kind of a query, i.e. it does not change or set anything. 
       *And it is equally complex enough to confuse the reader. So this has been put into a helper method with an expressive name to increase readability. 
<pre>
## Helper Method for generating a random participant which is to be used in peer_review_strategy method.
def gen_random_participant_id(iterator, participants_hash, num_participants, participants)
if iterator.zero?
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 = []
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
no_particpants = 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
participant_is_owner = (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))
rand_num = if no_particpants or participant_is_owner
# use original method to get random number
rand(0..num_participants - 1)
else
# rand_num should be the position of this participant in original array
participants_with_min_assigned_reviews[rand(0..participants_with_min_assigned_reviews.size - 1)]
end
end
return rand_num
end
</pre>
6. Refactored automatic_review_mapping by using a helper method check_num_reviews_args 
       *Parameters such as num_reviews_per_student, num_calibrated_artifacts etc passed are first verified to check if they are in acceptable range or pattern 
       *To increase readability, the not-so good looking sets of if-else statements have been moved into check_num_reviews_args method. 
<pre>
# Helper Method to check num_reviews_per_student and num_reviews_per_submission arguments passed in by params hash.
def check_num_reviews_args(num_reviews_per_student, num_reviews_per_submission, teams)
has_error_not_raised = true
# check for exit paths first
if num_reviews_per_student == 0 and num_reviews_per_submission == 0
flash[:error] = "Please choose either the number of reviews per student or the number of reviewers per team (student)."
has_error_not_raised = false
elsif num_reviews_per_student != 0 and num_reviews_per_submission != 0
flash[:error] = "Please choose either the number of reviews per student or the number of reviewers per team (student), not both."
has_error_not_raised = false
elsif num_reviews_per_student >= teams.size
# Exception detection: If instructor want to assign too many reviews done
# by each student, there will be an error msg.
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].'
has_error_not_raised = false
end
end
</pre>
7. Modularized helper methods into a module and was mixed in the ReviewMappingController Class. 
       *Only some of the methods written in the class have external usage i.e called by another controllers, views etc. 
       *The other methods are just helpers and as such moved into a Helper method module and mixed in the class 
Before Modularization
<pre>
class ReviewMappingController < ApplicationController
...................
...................
512 Lines & 25 Methods Defined (After moving a few methods into separate controllers)
...................
...................
end
</pre>
After Modularization
<pre>
module Helper_methods
...................
5 Methods and 170 lines
...................
end

class ReviewMappingController < ApplicationController
include Helper_methods
...................
340 Lines & 20 Methods (All of those are used elsewhere directly in the application)
...................
end
</pre>

8. Abided to the principles of Magic Tricks of testing and did not test any internally used methods, The other tests are written were already following this principle. 
       *Internally used methods were not tested 
       *Tests for newly created controllers have been moved into a separate spec files. 

9. Isolated AssignQuizDynamically method into a separate controller as the functionality was not related to ReviewMappingController. 
       *This method assigns a quiz(Stored as an assignment object) to the participant. 
       *This method is not related to ReviewMapping functionality, so it was made into a new controller. 

10. Associated Specs/routes/views/partials have been modified to adapt the change in controllers. 
       *The controller paths present in the views/partials have to be changed to not to break the functionality 
       *The controller paths in views/partials/routes have been changed for the newly created controller of AssignQuizDynamically 

View File Affected by the creation of AssinQuizController
[[File:View.png]]

11. student_review_num sounds like a number or an id associated with a student review, while it actually stores the number of reviews that a student can perform. So it is renamed num_reviews_per_student. 
12. submission_review_num sounds like a number or an id associated with a submission review, while it actually stores the total number of reviews that can be performed on a single submission. So it is renamed num_reviews_per_submission. 
13. calibrated_artifacts_num sounds like a number or an id associated with the calibrated artifacts, while it actually stores the number of calibrated artifacts. So it is renamed num_calibrated_artifacts. Similarly, uncalibrated_artifacts_num is renamed num_uncalibrated_artifacts. 
14. participants_hash is not an appropriate name for a hash whose keys are participant ids and values are number of reviews performed by corresponding participants. So it is renamed num_reviews_by_participant_hash. 
15. Extracted a method make_review_strategy (from automatic_review_mapping_strategy) that returns a review_strategy based on the values of num_reviews_per_submission and num_reviews_per_submission. 
16. add_calibration is a method that changes the attribute of a ReviewResponseMap and has little to do with Review Mapping. So it is now put in a separate controller named ReviewResponseMapController. 
17. The name add_reviewer may lead the reader to think that the method adds a reviewer to a collection of reviewers (e.g. a list of reviewers). Changing the name to assign_reviewer_manually informs the reader that the method assigns a reviewer (to a submission) and hence improves the readability of the code.

== Test Plan ==

=== Rspec Unit Tests ===

Since this is a Refactoring Project, We made sure that the changes made did not break any functionality.

[[File:testplan.png]]

Note: Tests have been run for three controllers (one existing and two new).

=== Capybara Integration and Functional Tests ===

As the controller routes have been modified in the routes.rb and the other view files, there are potential chances of failures in Integration tests.

However, no such failure has been reported by Travis build.

[[File:Travis CI.png]]

[[File:FeaturesCI.png]]

== Code Coverage ==

Code Coverage for Controllers section climbed up.

[https://coveralls.io/builds/26607665] # Link for the COVERALLS stats of our pull request.

[[File:CCC1.png]]

== Project Mentor ==

Ramya Vijayakumar (rvijaya4@ncsu.edu)

== Team Members ==

Yaswanth Soodini (ysoodin@ncsu.edu)

Saurabh Shingte (svshingt@ncsu.edu)

Vivek Karri (vkarri@ncsu.edu)

== Links for Demo Videos ==

[https://drive.google.com/file/d/1o9vJQ7fCwk0hwHJdErwpg1z_XjsN_XRT/view?usp=sharing]

CSC/ECE 517 Fall 2019 - E1944. Refactor review mapping controller

2019-11-02T19:16:19Z

Ysoodin: /* Description of the project */

CSC/ECE 517 Fall 2019 - E1944. Refactor review mapping controller

2019-11-02T19:16:00Z

Ysoodin: /* Description of the current project */

CSC/ECE 517 Fall 2019 - E1944. Refactor review mapping controller

2019-11-02T19:06:12Z

Ysoodin:

CSC/ECE 517 Fall 2019 - E1944. Refactor review mapping controller

2019-11-02T19:04:59Z

Ysoodin:

This wiki page is for the description of changes made under E1944 OSS assignment for Fall 2019, CSC/ECE 517.
sd
== Expertiza Background==

[http://expertiza.ncsu.edu/ Expertiza] is an open source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.

== Description of the current project ==

The following is an Expertiza based OSS project which deals primarily with a controller namely review_mapping_controller.rb. It focusses on refactoring some of the more complex methods, modifying some of the language to make it more Ruby friendly, removing redundant code etc among the other things. The goal of this project is to attempt to make this part of the application easier to read and maintain.

== Files modified in the current project ==

A controller and a helper file were modified for this project namely: 
1. review_mapping_controller.rb 
2. review_mapping_controller_spec.rb 
3. assign_quiz_controller.rb 
4. assign_quiz_controller_spec.rb 
5. review_response_map_controller.rb 
6. review_response_map_controller_spec.rb 
7. routes.rb 
8. Other views & partials associated affected by these changes 

=== ReviewMappingController ===

This controller will map the submissions made by the teams to the students for facilitating peer-reviewing. A couple of long and complex methods such as peer_review_strategy and automatic_review_mapping were refactored from this controller along with the removal of some non-related methods such as add_calibration and assign_quiz_dynamically. Variable names have been changed and code has been modularized and helper methods were separated from the important methods into a module and were included in the class.

Test Cases were created for the newly created controllers such as assign_quiz_controller etc.

=== review_mapping_controller_spec.rb ===

Tests were removed from this file which belongs to the isolated methods.

=== AssignQuizController ===

Assigning Quizzes (Quizzes are also stored in the Assignment table) is not a seemingly/semantically related task to review mapping. Hence this was moved into a separate controller.

=== assign_quiz_controller_spec.rb ===

Tests related to assign_quiz_controller were moved into this file.

=== ReviewResponseMapController.rb ===

Add Calibration is a nuanced method that has seemingly different functionality than Review Mapping Controller and hence moved into a separate controller.

=== review_response_map_controller_spec.rb ===

Tests related to review_response_map were moved into this file.

=== routes.rb ===

New routes were added to newly created controllers.

=== views/partials ===

Routes were changed in the views and partials.

== Details of the changes made==
1. Changed 'instructor = build(:instructor)' to ‘@instructor = build(:instructor, id: 1)’. 
       *Removed code redundancy from review_mapping_controller_spec.rb. Two variables were being initialized containing the same value. One was in the before(:each) loop and other was being called in first three test cases. 
       *Replaced the one in the before(:each) loop by @instructor = build(:instructor, id: 1) and used @instructor class variable, wherever required. 

2. Changed :i_dont_care to :no_particular_topic 
       *:i_dont_care was used in the /app/views/student_review/_set_dynamic_review.html.erb as a flag to store if student is interested in any particular topic or doesn't care which topic to review. 
       *It was also used in review_mapping_controller.rb to check if student has selected any particular topic. 
       *Since, name :i_dont_care was very difficult to understand, we replaced it with something logical such as :no_particular_topic. It gives hint about what the symbol stores. 

3. Removed cascading effects of above change from features spec 
       *Above changes caused ./spec/features/review_assignment_spec.rb this feature test to fail. 
       *This spec has used the above symbol to check if the list of available topics collapse or not after selecting the I don't care option. 

4. Created a variable named ‘allowed_actions’ in method choose_case(action_in_params) 
       *Switch case in above method from review_mapping_controller.rb contained all the actions having the same output for around 70% of cases. 
       *Hence, replaced the switch statements and initialized a list with those switch cases. 
       *Even the space complexity increased, the tradeoff got balanced because if someone has to change some actions, he will have to just add or remove the action name from the allowed_actions list. 

5. Refactored Peer_review_strategy by using a helper method gen_random_participant_id 
       *A random participant_id is generated from the possible pool of candidates but the code block for that is kind of a query, i.e. it does not change or set anything. 
       *And it is equally complex enough to confuse the reader. So this has been put into a helper method with an expressive name to increase readability. 

6. Refactored automatic_review_mapping by using a helper method check_num_reviews_args 
       *Parameters such as num_reviews_per_student, num_calibrated_artifacts etc passed are first verified to check if they are in acceptable range or pattern 
       *To increase readability, the not-so good looking sets of if-else statements have been moved into check_num_reviews_args method. 

7. Modularized helper methods (which are not accessed by any other functionality outside the review_mapping_controller) into a module and was mixed in the ReviewMappingController Class. 
       *Only some of the methods written in the class have external usage i.e called by another controllers, views etc. 
       *The other methods are just helpers and as such moved into a Helper method module and mixed in the class 

8. Abided to the principles of Magic Tricks of testing and did not test any internally used methods, The other tests are written were already following this principle. 
       *Internally used methods were not tested 
       *Tests for newly created controllers have been moved into a separate spec files. 

9. Isolated AssignQuizDynamically method into a separate controller as the functionality was not related to ReviewMappingController. 
       *This method assigns a quiz(Stored as an assignment object) to the participant. 
       *This method is not related to ReviewMapping functionality, so it was made into a new controller. 

10. Associated Specs/routes/views/partials have been modified to adapt the change in controllers. 
       *The controller paths present in the views/partials have to be changed to not to break the functionality 
       *The controller paths in views/partials/routes have been changed for the newly created controller of AssignQuizDynamically 

11. student_review_num sounds like a number or an id associated with a student review, while it actually stores the number of reviews that a student can perform. So it is renamed num_reviews_per_student. 
12. submission_review_num sounds like a number or an id associated with a submission review, while it actually stores the total number of reviews that can be performed on a single submission. So it is renamed num_reviews_per_submission. 
13. calibrated_artifacts_num sounds like a number or an id associated with the calibrated artifacts, while it actually stores the number of calibrated artifacts. So it is renamed num_calibrated_artifacts. Similarly, uncalibrated_artifacts_num is renamed num_uncalibrated_artifacts. 
14. participants_hash is not an appropriate name for a hash whose keys are participant ids and values are number of reviews performed by corresponding participants. So it is renamed num_reviews_by_participant_hash. 
15. Extracted a method make_review_strategy (from automatic_review_mapping_strategy) that returns a review_strategy based on the values of num_reviews_per_submission and num_reviews_per_submission. 
16. add_calibration is a method that changes the attribute of a ReviewResponseMap and has little to do with Review Mapping. So it is now put in a separate controller named ReviewResponseMapController. 
17. The name add_reviewer may lead the reader to think that the method adds a reviewer to a collection of reviewers (e.g. a list of reviewers). Changing the name to assign_reviewer_manually informs the reader that the method assigns a reviewer (to a submission) and hence improves the readability of the code.

CSC/ECE 517 Fall 2019 - E1944. Refactor review mapping controller

2019-11-02T19:04:05Z

Ysoodin:

This wiki page is for the description of changes made under E1944 OSS assignment for Fall 2019, CSC/ECE 517.
sd
== Expertiza Background==

Expertiza is an educational web application created and maintained by the joint efforts of the students and the faculty at NC State University. It’s an open-source project developed on Ruby on Rails platform and its codebase is available on Github. It facilitates peer review among the students allowing them to improve their work from the feedback. It also assists the faculty in customizing the specifications for the projects and Assignments.

== Description of the current project ==

The following is an Expertiza based OSS project which deals primarily with a controller namely review_mapping_controller.rb. It focusses on refactoring some of the more complex methods, modifying some of the language to make it more Ruby friendly, removing redundant code etc among the other things. The goal of this project is to attempt to make this part of the application easier to read and maintain.

== Files modified in the current project ==

A controller and a helper file were modified for this project namely: 
1. review_mapping_controller.rb 
2. review_mapping_controller_spec.rb 
3. assign_quiz_controller.rb 
4. assign_quiz_controller_spec.rb 
5. review_response_map_controller.rb 
6. review_response_map_controller_spec.rb 
7. routes.rb 
8. Other views & partials associated affected by these changes 

=== ReviewMappingController ===

This controller will map the submissions made by the teams to the students for facilitating peer-reviewing. A couple of long and complex methods such as peer_review_strategy and automatic_review_mapping were refactored from this controller along with the removal of some non-related methods such as add_calibration and assign_quiz_dynamically. Variable names have been changed and code has been modularized and helper methods were separated from the important methods into a module and were included in the class.

Test Cases were created for the newly created controllers such as assign_quiz_controller etc.

=== review_mapping_controller_spec.rb ===

Tests were removed from this file which belongs to the isolated methods.

=== AssignQuizController ===

Assigning Quizzes (Quizzes are also stored in the Assignment table) is not a seemingly/semantically related task to review mapping. Hence this was moved into a separate controller.

=== assign_quiz_controller_spec.rb ===

Tests related to assign_quiz_controller were moved into this file.

=== ReviewResponseMapController.rb ===

Add Calibration is a nuanced method that has seemingly different functionality than Review Mapping Controller and hence moved into a separate controller.

=== review_response_map_controller_spec.rb ===

Tests related to review_response_map were moved into this file.

=== routes.rb ===

New routes were added to newly created controllers.

=== views/partials ===

Routes were changed in the views and partials.

== Details of the changes made==
1. Changed 'instructor = build(:instructor)' to ‘@instructor = build(:instructor, id: 1)’. 
       *Removed code redundancy from review_mapping_controller_spec.rb. Two variables were being initialized containing the same value. One was in the before(:each) loop and other was being called in first three test cases. 
       *Replaced the one in the before(:each) loop by @instructor = build(:instructor, id: 1) and used @instructor class variable, wherever required. 

2. Changed :i_dont_care to :no_particular_topic 
       *:i_dont_care was used in the /app/views/student_review/_set_dynamic_review.html.erb as a flag to store if student is interested in any particular topic or doesn't care which topic to review. 
       *It was also used in review_mapping_controller.rb to check if student has selected any particular topic. 
       *Since, name :i_dont_care was very difficult to understand, we replaced it with something logical such as :no_particular_topic. It gives hint about what the symbol stores. 

3. Removed cascading effects of above change from features spec 
       *Above changes caused ./spec/features/review_assignment_spec.rb this feature test to fail. 
       *This spec has used the above symbol to check if the list of available topics collapse or not after selecting the I don't care option. 

4. Created a variable named ‘allowed_actions’ in method choose_case(action_in_params) 
       *Switch case in above method from review_mapping_controller.rb contained all the actions having the same output for around 70% of cases. 
       *Hence, replaced the switch statements and initialized a list with those switch cases. 
       *Even the space complexity increased, the tradeoff got balanced because if someone has to change some actions, he will have to just add or remove the action name from the allowed_actions list. 

5. Refactored Peer_review_strategy by using a helper method gen_random_participant_id 
       *A random participant_id is generated from the possible pool of candidates but the code block for that is kind of a query, i.e. it does not change or set anything. 
       *And it is equally complex enough to confuse the reader. So this has been put into a helper method with an expressive name to increase readability. 

6. Refactored automatic_review_mapping by using a helper method check_num_reviews_args 
       *Parameters such as num_reviews_per_student, num_calibrated_artifacts etc passed are first verified to check if they are in acceptable range or pattern 
       *To increase readability, the not-so good looking sets of if-else statements have been moved into check_num_reviews_args method. 

7. Modularized helper methods (which are not accessed by any other functionality outside the review_mapping_controller) into a module and was mixed in the ReviewMappingController Class. 
       *Only some of the methods written in the class have external usage i.e called by another controllers, views etc. 
       *The other methods are just helpers and as such moved into a Helper method module and mixed in the class 

8. Abided to the principles of Magic Tricks of testing and did not test any internally used methods, The other tests are written were already following this principle. 
       *Internally used methods were not tested 
       *Tests for newly created controllers have been moved into a separate spec files. 

9. Isolated AssignQuizDynamically method into a separate controller as the functionality was not related to ReviewMappingController. 
       *This method assigns a quiz(Stored as an assignment object) to the participant. 
       *This method is not related to ReviewMapping functionality, so it was made into a new controller. 

10. Associated Specs/routes/views/partials have been modified to adapt the change in controllers. 
       *The controller paths present in the views/partials have to be changed to not to break the functionality 
       *The controller paths in views/partials/routes have been changed for the newly created controller of AssignQuizDynamically 

11. student_review_num sounds like a number or an id associated with a student review, while it actually stores the number of reviews that a student can perform. So it is renamed num_reviews_per_student. 
12. submission_review_num sounds like a number or an id associated with a submission review, while it actually stores the total number of reviews that can be performed on a single submission. So it is renamed num_reviews_per_submission. 
13. calibrated_artifacts_num sounds like a number or an id associated with the calibrated artifacts, while it actually stores the number of calibrated artifacts. So it is renamed num_calibrated_artifacts. Similarly, uncalibrated_artifacts_num is renamed num_uncalibrated_artifacts. 
14. participants_hash is not an appropriate name for a hash whose keys are participant ids and values are number of reviews performed by corresponding participants. So it is renamed num_reviews_by_participant_hash. 
15. Extracted a method make_review_strategy (from automatic_review_mapping_strategy) that returns a review_strategy based on the values of num_reviews_per_submission and num_reviews_per_submission. 
16. add_calibration is a method that changes the attribute of a ReviewResponseMap and has little to do with Review Mapping. So it is now put in a separate controller named ReviewResponseMapController. 
17. The name add_reviewer may lead the reader to think that the method adds a reviewer to a collection of reviewers (e.g. a list of reviewers). Changing the name to assign_reviewer_manually informs the reader that the method assigns a reviewer (to a submission) and hence improves the readability of the code.

CSC/ECE 517 Fall 2019 - E1944. Refactor review mapping controller

2019-11-02T19:03:18Z

Ysoodin:

[[File:wiki.png|alt=Puzzle globe logo]]This wiki page is for the description of changes made under E1944 OSS assignment for Fall 2019, CSC/ECE 517.
sd
== Expertiza Background==

Expertiza is an educational web application created and maintained by the joint efforts of the students and the faculty at NC State University. It’s an open-source project developed on Ruby on Rails platform and its codebase is available on Github. It facilitates peer review among the students allowing them to improve their work from the feedback. It also assists the faculty in customizing the specifications for the projects and Assignments.

== Description of the current project ==

The following is an Expertiza based OSS project which deals primarily with a controller namely review_mapping_controller.rb. It focusses on refactoring some of the more complex methods, modifying some of the language to make it more Ruby friendly, removing redundant code etc among the other things. The goal of this project is to attempt to make this part of the application easier to read and maintain.

== Files modified in the current project ==

A controller and a helper file were modified for this project namely: 
1. review_mapping_controller.rb 
2. review_mapping_controller_spec.rb 
3. assign_quiz_controller.rb 
4. assign_quiz_controller_spec.rb 
5. review_response_map_controller.rb 
6. review_response_map_controller_spec.rb 
7. routes.rb 
8. Other views & partials associated affected by these changes 

=== ReviewMappingController ===

This controller will map the submissions made by the teams to the students for facilitating peer-reviewing. A couple of long and complex methods such as peer_review_strategy and automatic_review_mapping were refactored from this controller along with the removal of some non-related methods such as add_calibration and assign_quiz_dynamically. Variable names have been changed and code has been modularized and helper methods were separated from the important methods into a module and were included in the class.

Test Cases were created for the newly created controllers such as assign_quiz_controller etc.

=== review_mapping_controller_spec.rb ===

Tests were removed from this file which belongs to the isolated methods.

=== AssignQuizController ===

Assigning Quizzes (Quizzes are also stored in the Assignment table) is not a seemingly/semantically related task to review mapping. Hence this was moved into a separate controller.

=== assign_quiz_controller_spec.rb ===

Tests related to assign_quiz_controller were moved into this file.

=== ReviewResponseMapController.rb ===

Add Calibration is a nuanced method that has seemingly different functionality than Review Mapping Controller and hence moved into a separate controller.

=== review_response_map_controller_spec.rb ===

Tests related to review_response_map were moved into this file.

=== routes.rb ===

New routes were added to newly created controllers.

=== views/partials ===

Routes were changed in the views and partials.

== Details of the changes made==
1. Changed 'instructor = build(:instructor)' to ‘@instructor = build(:instructor, id: 1)’. 
       *Removed code redundancy from review_mapping_controller_spec.rb. Two variables were being initialized containing the same value. One was in the before(:each) loop and other was being called in first three test cases. 
       *Replaced the one in the before(:each) loop by @instructor = build(:instructor, id: 1) and used @instructor class variable, wherever required. 

2. Changed :i_dont_care to :no_particular_topic 
       *:i_dont_care was used in the /app/views/student_review/_set_dynamic_review.html.erb as a flag to store if student is interested in any particular topic or doesn't care which topic to review. 
       *It was also used in review_mapping_controller.rb to check if student has selected any particular topic. 
       *Since, name :i_dont_care was very difficult to understand, we replaced it with something logical such as :no_particular_topic. It gives hint about what the symbol stores. 

3. Removed cascading effects of above change from features spec 
       *Above changes caused ./spec/features/review_assignment_spec.rb this feature test to fail. 
       *This spec has used the above symbol to check if the list of available topics collapse or not after selecting the I don't care option. 

4. Created a variable named ‘allowed_actions’ in method choose_case(action_in_params) 
       *Switch case in above method from review_mapping_controller.rb contained all the actions having the same output for around 70% of cases. 
       *Hence, replaced the switch statements and initialized a list with those switch cases. 
       *Even the space complexity increased, the tradeoff got balanced because if someone has to change some actions, he will have to just add or remove the action name from the allowed_actions list. 

5. Refactored Peer_review_strategy by using a helper method gen_random_participant_id 
       *A random participant_id is generated from the possible pool of candidates but the code block for that is kind of a query, i.e. it does not change or set anything. 
       *And it is equally complex enough to confuse the reader. So this has been put into a helper method with an expressive name to increase readability. 

6. Refactored automatic_review_mapping by using a helper method check_num_reviews_args 
       *Parameters such as num_reviews_per_student, num_calibrated_artifacts etc passed are first verified to check if they are in acceptable range or pattern 
       *To increase readability, the not-so good looking sets of if-else statements have been moved into check_num_reviews_args method. 

7. Modularized helper methods (which are not accessed by any other functionality outside the review_mapping_controller) into a module and was mixed in the ReviewMappingController Class. 
       *Only some of the methods written in the class have external usage i.e called by another controllers, views etc. 
       *The other methods are just helpers and as such moved into a Helper method module and mixed in the class 

8. Abided to the principles of Magic Tricks of testing and did not test any internally used methods, The other tests are written were already following this principle. 
       *Internally used methods were not tested 
       *Tests for newly created controllers have been moved into a separate spec files. 

9. Isolated AssignQuizDynamically method into a separate controller as the functionality was not related to ReviewMappingController. 
       *This method assigns a quiz(Stored as an assignment object) to the participant. 
       *This method is not related to ReviewMapping functionality, so it was made into a new controller. 

10. Associated Specs/routes/views/partials have been modified to adapt the change in controllers. 
       *The controller paths present in the views/partials have to be changed to not to break the functionality 
       *The controller paths in views/partials/routes have been changed for the newly created controller of AssignQuizDynamically 

11. student_review_num sounds like a number or an id associated with a student review, while it actually stores the number of reviews that a student can perform. So it is renamed num_reviews_per_student. 
12. submission_review_num sounds like a number or an id associated with a submission review, while it actually stores the total number of reviews that can be performed on a single submission. So it is renamed num_reviews_per_submission. 
13. calibrated_artifacts_num sounds like a number or an id associated with the calibrated artifacts, while it actually stores the number of calibrated artifacts. So it is renamed num_calibrated_artifacts. Similarly, uncalibrated_artifacts_num is renamed num_uncalibrated_artifacts. 
14. participants_hash is not an appropriate name for a hash whose keys are participant ids and values are number of reviews performed by corresponding participants. So it is renamed num_reviews_by_participant_hash. 
15. Extracted a method make_review_strategy (from automatic_review_mapping_strategy) that returns a review_strategy based on the values of num_reviews_per_submission and num_reviews_per_submission. 
16. add_calibration is a method that changes the attribute of a ReviewResponseMap and has little to do with Review Mapping. So it is now put in a separate controller named ReviewResponseMapController. 
17. The name add_reviewer may lead the reader to think that the method adds a reviewer to a collection of reviewers (e.g. a list of reviewers). Changing the name to assign_reviewer_manually informs the reader that the method assigns a reviewer (to a submission) and hence improves the readability of the code.

CSC/ECE 517 Fall 2019 - E1944. Refactor review mapping controller

2019-11-02T19:02:46Z

Ysoodin:

[[File:wiki.jpg]]This wiki page is for the description of changes made under E1944 OSS assignment for Fall 2019, CSC/ECE 517.
sd
== Expertiza Background==

Expertiza is an educational web application created and maintained by the joint efforts of the students and the faculty at NC State University. It’s an open-source project developed on Ruby on Rails platform and its codebase is available on Github. It facilitates peer review among the students allowing them to improve their work from the feedback. It also assists the faculty in customizing the specifications for the projects and Assignments.

== Description of the current project ==

The following is an Expertiza based OSS project which deals primarily with a controller namely review_mapping_controller.rb. It focusses on refactoring some of the more complex methods, modifying some of the language to make it more Ruby friendly, removing redundant code etc among the other things. The goal of this project is to attempt to make this part of the application easier to read and maintain.

== Files modified in the current project ==

A controller and a helper file were modified for this project namely: 
1. review_mapping_controller.rb 
2. review_mapping_controller_spec.rb 
3. assign_quiz_controller.rb 
4. assign_quiz_controller_spec.rb 
5. review_response_map_controller.rb 
6. review_response_map_controller_spec.rb 
7. routes.rb 
8. Other views & partials associated affected by these changes 

=== ReviewMappingController ===

This controller will map the submissions made by the teams to the students for facilitating peer-reviewing. A couple of long and complex methods such as peer_review_strategy and automatic_review_mapping were refactored from this controller along with the removal of some non-related methods such as add_calibration and assign_quiz_dynamically. Variable names have been changed and code has been modularized and helper methods were separated from the important methods into a module and were included in the class.

Test Cases were created for the newly created controllers such as assign_quiz_controller etc.

=== review_mapping_controller_spec.rb ===

Tests were removed from this file which belongs to the isolated methods.

=== AssignQuizController ===

Assigning Quizzes (Quizzes are also stored in the Assignment table) is not a seemingly/semantically related task to review mapping. Hence this was moved into a separate controller.

=== assign_quiz_controller_spec.rb ===

Tests related to assign_quiz_controller were moved into this file.

=== ReviewResponseMapController.rb ===

Add Calibration is a nuanced method that has seemingly different functionality than Review Mapping Controller and hence moved into a separate controller.

=== review_response_map_controller_spec.rb ===

Tests related to review_response_map were moved into this file.

=== routes.rb ===

New routes were added to newly created controllers.

=== views/partials ===

Routes were changed in the views and partials.

== Details of the changes made==
1. Changed 'instructor = build(:instructor)' to ‘@instructor = build(:instructor, id: 1)’. 
       *Removed code redundancy from review_mapping_controller_spec.rb. Two variables were being initialized containing the same value. One was in the before(:each) loop and other was being called in first three test cases. 
       *Replaced the one in the before(:each) loop by @instructor = build(:instructor, id: 1) and used @instructor class variable, wherever required. 

2. Changed :i_dont_care to :no_particular_topic 
       *:i_dont_care was used in the /app/views/student_review/_set_dynamic_review.html.erb as a flag to store if student is interested in any particular topic or doesn't care which topic to review. 
       *It was also used in review_mapping_controller.rb to check if student has selected any particular topic. 
       *Since, name :i_dont_care was very difficult to understand, we replaced it with something logical such as :no_particular_topic. It gives hint about what the symbol stores. 

3. Removed cascading effects of above change from features spec 
       *Above changes caused ./spec/features/review_assignment_spec.rb this feature test to fail. 
       *This spec has used the above symbol to check if the list of available topics collapse or not after selecting the I don't care option. 

4. Created a variable named ‘allowed_actions’ in method choose_case(action_in_params) 
       *Switch case in above method from review_mapping_controller.rb contained all the actions having the same output for around 70% of cases. 
       *Hence, replaced the switch statements and initialized a list with those switch cases. 
       *Even the space complexity increased, the tradeoff got balanced because if someone has to change some actions, he will have to just add or remove the action name from the allowed_actions list. 

5. Refactored Peer_review_strategy by using a helper method gen_random_participant_id 
       *A random participant_id is generated from the possible pool of candidates but the code block for that is kind of a query, i.e. it does not change or set anything. 
       *And it is equally complex enough to confuse the reader. So this has been put into a helper method with an expressive name to increase readability. 

6. Refactored automatic_review_mapping by using a helper method check_num_reviews_args 
       *Parameters such as num_reviews_per_student, num_calibrated_artifacts etc passed are first verified to check if they are in acceptable range or pattern 
       *To increase readability, the not-so good looking sets of if-else statements have been moved into check_num_reviews_args method. 

7. Modularized helper methods (which are not accessed by any other functionality outside the review_mapping_controller) into a module and was mixed in the ReviewMappingController Class. 
       *Only some of the methods written in the class have external usage i.e called by another controllers, views etc. 
       *The other methods are just helpers and as such moved into a Helper method module and mixed in the class 

8. Abided to the principles of Magic Tricks of testing and did not test any internally used methods, The other tests are written were already following this principle. 
       *Internally used methods were not tested 
       *Tests for newly created controllers have been moved into a separate spec files. 

9. Isolated AssignQuizDynamically method into a separate controller as the functionality was not related to ReviewMappingController. 
       *This method assigns a quiz(Stored as an assignment object) to the participant. 
       *This method is not related to ReviewMapping functionality, so it was made into a new controller. 

10. Associated Specs/routes/views/partials have been modified to adapt the change in controllers. 
       *The controller paths present in the views/partials have to be changed to not to break the functionality 
       *The controller paths in views/partials/routes have been changed for the newly created controller of AssignQuizDynamically 

11. student_review_num sounds like a number or an id associated with a student review, while it actually stores the number of reviews that a student can perform. So it is renamed num_reviews_per_student. 
12. submission_review_num sounds like a number or an id associated with a submission review, while it actually stores the total number of reviews that can be performed on a single submission. So it is renamed num_reviews_per_submission. 
13. calibrated_artifacts_num sounds like a number or an id associated with the calibrated artifacts, while it actually stores the number of calibrated artifacts. So it is renamed num_calibrated_artifacts. Similarly, uncalibrated_artifacts_num is renamed num_uncalibrated_artifacts. 
14. participants_hash is not an appropriate name for a hash whose keys are participant ids and values are number of reviews performed by corresponding participants. So it is renamed num_reviews_by_participant_hash. 
15. Extracted a method make_review_strategy (from automatic_review_mapping_strategy) that returns a review_strategy based on the values of num_reviews_per_submission and num_reviews_per_submission. 
16. add_calibration is a method that changes the attribute of a ReviewResponseMap and has little to do with Review Mapping. So it is now put in a separate controller named ReviewResponseMapController. 
17. The name add_reviewer may lead the reader to think that the method adds a reviewer to a collection of reviewers (e.g. a list of reviewers). Changing the name to assign_reviewer_manually informs the reader that the method assigns a reviewer (to a submission) and hence improves the readability of the code.

CSC/ECE 517 Fall 2019 - E1944. Refactor review mapping controller

2019-11-02T18:58:48Z

Ysoodin:

[[File:Example.jpg]]This wiki page is for the description of changes made under E1944 OSS assignment for Fall 2019, CSC/ECE 517.

== Expertiza Background==

Expertiza is an educational web application created and maintained by the joint efforts of the students and the faculty at NC State University. It’s an open-source project developed on Ruby on Rails platform and its codebase is available on Github. It facilitates peer review among the students allowing them to improve their work from the feedback. It also assists the faculty in customizing the specifications for the projects and Assignments.

== Description of the current project ==

The following is an Expertiza based OSS project which deals primarily with a controller namely review_mapping_controller.rb. It focusses on refactoring some of the more complex methods, modifying some of the language to make it more Ruby friendly, removing redundant code etc among the other things. The goal of this project is to attempt to make this part of the application easier to read and maintain.

== Files modified in the current project ==

A controller and a helper file were modified for this project namely: 
1. review_mapping_controller.rb 
2. review_mapping_controller_spec.rb 
3. assign_quiz_controller.rb 
4. assign_quiz_controller_spec.rb 
5. review_response_map_controller.rb 
6. review_response_map_controller_spec.rb 
7. routes.rb 
8. Other views & partials associated affected by these changes 

=== ReviewMappingController ===

This controller will map the submissions made by the teams to the students for facilitating peer-reviewing. A couple of long and complex methods such as peer_review_strategy and automatic_review_mapping were refactored from this controller along with the removal of some non-related methods such as add_calibration and assign_quiz_dynamically. Variable names have been changed and code has been modularized and helper methods were separated from the important methods into a module and were included in the class.

Test Cases were created for the newly created controllers such as assign_quiz_controller etc.

=== review_mapping_controller_spec.rb ===

Tests were removed from this file which belongs to the isolated methods.

=== AssignQuizController ===

Assigning Quizzes (Quizzes are also stored in the Assignment table) is not a seemingly/semantically related task to review mapping. Hence this was moved into a separate controller.

=== assign_quiz_controller_spec.rb ===

Tests related to assign_quiz_controller were moved into this file.

=== ReviewResponseMapController.rb ===

Add Calibration is a nuanced method that has seemingly different functionality than Review Mapping Controller and hence moved into a separate controller.

=== review_response_map_controller_spec.rb ===

Tests related to review_response_map were moved into this file.

=== routes.rb ===

New routes were added to newly created controllers.

=== views/partials ===

Routes were changed in the views and partials.

== Details of the changes made==
1. Changed 'instructor = build(:instructor)' to ‘@instructor = build(:instructor, id: 1)’. 
       *Removed code redundancy from review_mapping_controller_spec.rb. Two variables were being initialized containing the same value. One was in the before(:each) loop and other was being called in first three test cases. 
       *Replaced the one in the before(:each) loop by @instructor = build(:instructor, id: 1) and used @instructor class variable, wherever required. 

2. Changed :i_dont_care to :no_particular_topic 
       *:i_dont_care was used in the /app/views/student_review/_set_dynamic_review.html.erb as a flag to store if student is interested in any particular topic or doesn't care which topic to review. 
       *It was also used in review_mapping_controller.rb to check if student has selected any particular topic. 
       *Since, name :i_dont_care was very difficult to understand, we replaced it with something logical such as :no_particular_topic. It gives hint about what the symbol stores. 

3. Removed cascading effects of above change from features spec 
       *Above changes caused ./spec/features/review_assignment_spec.rb this feature test to fail. 
       *This spec has used the above symbol to check if the list of available topics collapse or not after selecting the I don't care option. 

4. Created a variable named ‘allowed_actions’ in method choose_case(action_in_params) 
       *Switch case in above method from review_mapping_controller.rb contained all the actions having the same output for around 70% of cases. 
       *Hence, replaced the switch statements and initialized a list with those switch cases. 
       *Even the space complexity increased, the tradeoff got balanced because if someone has to change some actions, he will have to just add or remove the action name from the allowed_actions list. 

5. Refactored Peer_review_strategy by using a helper method gen_random_participant_id 
       *A random participant_id is generated from the possible pool of candidates but the code block for that is kind of a query, i.e. it does not change or set anything. 
       *And it is equally complex enough to confuse the reader. So this has been put into a helper method with an expressive name to increase readability. 

6. Refactored automatic_review_mapping by using a helper method check_num_reviews_args 
       *Parameters such as num_reviews_per_student, num_calibrated_artifacts etc passed are first verified to check if they are in acceptable range or pattern 
       *To increase readability, the not-so good looking sets of if-else statements have been moved into check_num_reviews_args method. 

7. Modularized helper methods (which are not accessed by any other functionality outside the review_mapping_controller) into a module and was mixed in the ReviewMappingController Class. 
       *Only some of the methods written in the class have external usage i.e called by another controllers, views etc. 
       *The other methods are just helpers and as such moved into a Helper method module and mixed in the class 

8. Abided to the principles of Magic Tricks of testing and did not test any internally used methods, The other tests are written were already following this principle. 
       *Internally used methods were not tested 
       *Tests for newly created controllers have been moved into a separate spec files. 

9. Isolated AssignQuizDynamically method into a separate controller as the functionality was not related to ReviewMappingController. 
       *This method assigns a quiz(Stored as an assignment object) to the participant. 
       *This method is not related to ReviewMapping functionality, so it was made into a new controller. 

10. Associated Specs/routes/views/partials have been modified to adapt the change in controllers. 
       *The controller paths present in the views/partials have to be changed to not to break the functionality 
       *The controller paths in views/partials/routes have been changed for the newly created controller of AssignQuizDynamically 

11. student_review_num sounds like a number or an id associated with a student review, while it actually stores the number of reviews that a student can perform. So it is renamed num_reviews_per_student. 
12. submission_review_num sounds like a number or an id associated with a submission review, while it actually stores the total number of reviews that can be performed on a single submission. So it is renamed num_reviews_per_submission. 
13. calibrated_artifacts_num sounds like a number or an id associated with the calibrated artifacts, while it actually stores the number of calibrated artifacts. So it is renamed num_calibrated_artifacts. Similarly, uncalibrated_artifacts_num is renamed num_uncalibrated_artifacts. 
14. participants_hash is not an appropriate name for a hash whose keys are participant ids and values are number of reviews performed by corresponding participants. So it is renamed num_reviews_by_participant_hash. 
15. Extracted a method make_review_strategy (from automatic_review_mapping_strategy) that returns a review_strategy based on the values of num_reviews_per_submission and num_reviews_per_submission. 
16. add_calibration is a method that changes the attribute of a ReviewResponseMap and has little to do with Review Mapping. So it is now put in a separate controller named ReviewResponseMapController. 
17. The name add_reviewer may lead the reader to think that the method adds a reviewer to a collection of reviewers (e.g. a list of reviewers). Changing the name to assign_reviewer_manually informs the reader that the method assigns a reviewer (to a submission) and hence improves the readability of the code.

CSC/ECE 517 Fall 2019 - E1944. Refactor review mapping controller

2019-10-29T04:52:39Z

Ysoodin: /* Details of changes */

This wiki page is for the description of changes made under E1944 OSS assignment for Fall 2019, CSC/ECE 517.

== Expertiza Background==

Expertiza is an educational web application created and maintained by the joint efforts of the students and the faculty at NC State University. It’s an open-source project developed on Ruby on Rails platform and its codebase is available on Github. It facilitates peer review among the students allowing them to improve their work from the feedback. It also assists the faculty in customizing the specifications for the projects and Assignments.

== Description of the current project ==

The following is an Expertiza based OSS project which deals primarily with a controller namely review_mapping_controller.rb. It focusses on refactoring some of the more complex methods, modifying some of the language to make it more Ruby friendly, removing redundant code etc among the other things. The goal of this project is to attempt to make this part of the application easier to read and maintain.

== Files modified in the current project ==

A controller and a helper file were modified for this project namely: 
1. review_mapping_controller.rb 
2. review_mapping_controller_spec.rb 
3. assign_quiz_controller.rb 
4. assign_quiz_controller_spec.rb 
5. review_response_map_controller.rb 
6. review_response_map_controller_spec.rb 
7. routes.rb 
8. Other views & partials associated affected by these changes 

=== ReviewMappingController ===

This controller will map the submissions made by the teams to the students for facilitating peer-reviewing. A couple of long and complex methods such as peer_review_strategy and automatic_review_mapping were refactored from this controller along with the removal of some non-related methods such as add_calibration and assign_quiz_dynamically. Variable names have been changed and code has been modularized and helper methods were separated from the important methods into a module and were included in the class.

Test Cases were created for the newly created controllers such as assign_quiz_controller etc.

=== review_mapping_controller_spec.rb ===

Tests were removed from this file which belongs to the isolated methods.

=== AssignQuizController ===

Assigning Quizzes (Quizzes are also stored in the Assignment table) is not a seemingly/semantically related task to review mapping. Hence this was moved into a separate controller.

=== assign_quiz_controller_spec.rb ===

Tests related to assign_quiz_controller were moved into this file.

=== ReviewResponseMapController.rb ===

Add Calibration is a nuanced method that has seemingly different functionality than Review Mapping Controller and hence moved into a separate controller.

=== review_response_map_controller_spec.rb ===

Tests related to review_response_map were moved into this file.

=== routes.rb ===

New routes were added to newly created controllers.

=== views/partials ===

Routes were changed in the views and partials.

== Details of changes ==
1. Changed 'instructor = build(:instructor)' ‘@instructor = build(:instructor, id: 1)’ it was being called in first three test cases and in above loop too. 
2. Changed params[:i_dont_care] to params[:no_particular_topic] 
3. Removed cascading effects of above change from features (CSS) 
4. Created a variable named ‘allowed_actions’ in method choose_case(action_in_params) 
5. Refactored Peer_review_strategy by using a helper method gen_random_participant_id 
6. Refactored automatic_review_mapping by using a helper method check_num_reviews_args 
7. Modularized helper methods (which are not accessed by any other functionality outside the review_mapping_controller) into a module and was mixed in the ReviewMappingController Class. 
8. Abided to the principles of Magic Tricks of testing specified in wand did not test any internally used methods, The other tests are written were already following this principle. 
9. Isolated AssignQuizDynamically method into a separate controller as the functionality was not related to ReviewMappingController. 
10. Associated Specs/routes/views/partials have been modified to adapt the change in controllers. 
11. student_review_num sounds like a number or an id associated with a student review, while it actually stores the number of reviews that a student can perform. So it is renamed num_reviews_per_student. 
12. submission_review_num sounds like a number or an id associated with a submission review, while it actually stores the total number of reviews that can be performed on a single submission. So it is renamed num_reviews_per_submission. 
13. calibrated_artifacts_num sounds like a number or an id associated with the calibrated artifacts, while it actually stores the number of calibrated artifacts. So it is renamed num_calibrated_artifacts. Similarly, uncalibrated_artifacts_num is renamed num_uncalibrated_artifacts. 
14. participants_hash is not an appropriate name for a hash whose keys are participant ids and values are number of reviews performed by corresponding participants. So it is renamed num_reviews_by_participant_hash. 
15. Extracted a method make_review_strategy (from automatic_review_mapping_strategy) that returns a review_strategy based on the values of num_reviews_per_submission and num_reviews_per_submission. 
16. add_calibration is a method that changes the attribute of a ReviewResponseMap and has little to do with Review Mapping. So it is now put in a separate controller named ReviewResponseMapController. 
17. The name add_reviewer may lead the reader to think that the method adds a reviewer to a collection of reviewers (e.g. a list of reviewers). Changing the name to assign_reviewer_manually informs the reader that the method assigns a reviewer (to a submission) and hence improves the readability of the code.

CSC/ECE 517 Fall 2019 - E1944. Refactor review mapping controller

2019-10-29T04:51:05Z

Ysoodin: /* Details of changes */

This wiki page is for the description of changes made under E1944 OSS assignment for Fall 2019, CSC/ECE 517.

== Expertiza Background==

Expertiza is an educational web application created and maintained by the joint efforts of the students and the faculty at NC State University. It’s an open-source project developed on Ruby on Rails platform and its codebase is available on Github. It facilitates peer review among the students allowing them to improve their work from the feedback. It also assists the faculty in customizing the specifications for the projects and Assignments.

== Description of the current project ==

The following is an Expertiza based OSS project which deals primarily with a controller namely review_mapping_controller.rb. It focusses on refactoring some of the more complex methods, modifying some of the language to make it more Ruby friendly, removing redundant code etc among the other things. The goal of this project is to attempt to make this part of the application easier to read and maintain.

== Files modified in the current project ==

A controller and a helper file were modified for this project namely: 
1. review_mapping_controller.rb 
2. review_mapping_controller_spec.rb 
3. assign_quiz_controller.rb 
4. assign_quiz_controller_spec.rb 
5. review_response_map_controller.rb 
6. review_response_map_controller_spec.rb 
7. routes.rb 
8. Other views & partials associated affected by these changes 

=== ReviewMappingController ===

This controller will map the submissions made by the teams to the students for facilitating peer-reviewing. A couple of long and complex methods such as peer_review_strategy and automatic_review_mapping were refactored from this controller along with the removal of some non-related methods such as add_calibration and assign_quiz_dynamically. Variable names have been changed and code has been modularized and helper methods were separated from the important methods into a module and were included in the class.

Test Cases were created for the newly created controllers such as assign_quiz_controller etc.

=== review_mapping_controller_spec.rb ===

Tests were removed from this file which belongs to the isolated methods.

=== AssignQuizController ===

Assigning Quizzes (Quizzes are also stored in the Assignment table) is not a seemingly/semantically related task to review mapping. Hence this was moved into a separate controller.

=== assign_quiz_controller_spec.rb ===

Tests related to assign_quiz_controller were moved into this file.

=== ReviewResponseMapController.rb ===

Add Calibration is a nuanced method that has seemingly different functionality than Review Mapping Controller and hence moved into a separate controller.

=== review_response_map_controller_spec.rb ===

Tests related to review_response_map were moved into this file.

=== routes.rb ===

New routes were added to newly created controllers.

=== views/partials ===

Routes were changed in the views and partials.

== Details of changes ==
1. Changed 'instructor = build(:instructor)' ‘@instructor = build(:instructor, id: 1)’ it was being called in first three test cases and in above loop too. 
2. Changed params[:i_dont_care] to params[:no_particular_topic] 
3. Removed cascading effects of above change from features (CSS) 
4. Created a variable named ‘allowed_actions’ in method choose_case(action_in_params) 
5. Refactored Peer_review_strategy by using a helper method gen_random_participant_id 
6. Refactored automatic_review_mapping by using a helper method check_num_reviews_args 
7. Modularized helper methods (which are not accessed by any other functionality outside the review_mapping_controller) into a module and was mixed in the ReviewMappingController Class. 
8. Abided to the principles of Magic Tricks of testing specified in wand did not test any internally used methods, The other tests are written were already following this principle. 
9. Isolated AssignQuizDynamically method into a separate controller as the functionality was not related to ReviewMappingController. 
10. Associated Specs/routes/views/partials have been modified to adapt the change in controllers. 
11. student_review_num sounds like a number or an id associated with a student review, while it actually stores the number of reviews that a student can perform. So it is renamed num_reviews_per_student.
12. submission_review_num sounds like a number or an id associated with a submission review, while it actually stores the total number of reviews that can be performed on a single submission. So it is renamed num_reviews_per_submission.
13. calibrated_artifacts_num sounds like a number or an id associated with the calibrated artifacts, while it actually stores the number of calibrated artifacts. So it is renamed num_calibrated_artifacts. Similarly, uncalibrated_artifacts_num is renamed num_uncalibrated_artifacts.
14. participants_hash is not an appropriate name for a hash whose keys are participant ids and values are number of reviews performed by corresponding participants. So it is renamed num_reviews_by_participant_hash.
15. Extracted a method make_review_strategy (from automatic_review_mapping_strategy) that returns a review_strategy based on the values of num_reviews_per_submission and num_reviews_per_submission.
16. add_calibration is a method that changes the attribute of a ReviewResponseMap and has little to do with Review Mapping. So it is now put in a separate controller named ReviewResponseMapController.
17. The name add_reviewer may lead the reader to think that the method adds a reviewer to a collection of reviewers (e.g. a list of reviewers). Changing the name to assign_reviewer_manually informs the reader that the method assigns a reviewer (to a submission) and hence improves the readability of the code.

CSC/ECE 517 Fall 2019 - E1944. Refactor review mapping controller

2019-10-29T02:27:09Z

Ysoodin: /* Description of the current project */

This wiki page is for the description of changes made under E1555 OSS assignment for Fall 2015, CSC/ECE 517.

== Expertiza Background==

Expertiza is an educational web application created and maintained by the joint efforts of the students and the faculty at NCSU. It’s an open source project developed on Ruby on Rails platform and it’s code is available on Github. It allows students to review each other’s work and improve their work upon this feedback.

== Description of the current project ==

The following is an Expertiza based OSS project which deals primarily with the ReviewMappingController. It focusses on refactoring some of the more complex methods, modifying some of the language to make it more Ruby friendly, removing some redundant code. The goal of this project is to attempt to make this part of the application easier to read and maintain.

== Files modified in current project ==

A controller and a helper file were modified for this project namely: 
1. GradesController 
2. GradesHelper 

=== GradesController ===

This is a controller that helps students and instructors view grades and reviews, update scores, check for grading conflicts and calculate penalties. A couple of long and complex methods were refactored from this controller along with removal of some non-functional code and a few language changes to make it Ruby style.
Three methods in particular, namely conflict_notification ,calculate_all_penalties and edit were found to be too long and were in need of refactoring into smaller, easier to manage methods. Few more compact methods were created for this purpose.

There were no existing test cases for the controller. We have added a spec file named 'grades_spec.rb' under the spec folder. As no changes were done for the model, no tests for the model were included.

=== GradesHelper ===

This is a helper class which contains methods for constructing a table(construct_table) and to check whether an assignment has a team and metareveiw(has_team_and_metareview)

== List of changes ==
We worked on the following work items(WIs) 
WI1 : Refactor calculate_all_penalties method into smaller methods 
WI2 : Move the repeated code in conflict_notification & edit methods to a separate method list_questions. 
WI3 : Refactor the code as per the Ruby style guidelines and incorporate the good practices 
WI4 : Test the conflict_notification method to test the changes made. 
WI5 : Move the repeated code in view and view_my_scores methods to a separate method retrieve_questions

=== Solutions Implemented and Delivered ===

*Refactoring calculate_all_penalties method

This is used to calculate various penalty values for each assignment if penalty is applicable.

The following changes were made:

1. This method was very complex, performing too many functions within a single method and had to be broken into 3 smaller methods each having a more well defined function.
2. The following 3 methods were created after splitting the first method 
i. calculate_all_penalties 
ii. calculate_penatly_attributes 
iii. assign_all_penalties 
3. Changes were also made to make the code follow ruby style.The language was made more ruby friendly.
4. Finally some redundant code was commented out as it was non-functional.

Refactoring into smaller more specific methods:

[[File:Change6_new.png]]

Removal of non-functional code :

[[File:Change5_new.png]]

Change of language to make it more Ruby friendly:

[[File:Change1_new.png]]

*Move the redundant piece of code from conflict_notification & edit methods to a new method list_questions

The conflict_notification method is used to help the instructors decide if one of the reviews are unfair or inaccurate.
This was again split into 2 methods with some part of the code which is repeated in another method refactored into a new method.

[[File:Change3_new.png]]

Refactored #Created a method which was a duplicate in conflict_notification and edit methods

[[File:Change4_new.png]]

edit method:

This method is used to edit the questionnaires. This method again has code which is repeated in the conflict_notification method and thus the repeated section was split into a new method.

[[File:Change2_new.png]]

New method:
Refactored #Created a method which was a duplicate in conflict_notification and edit methods

[[File:Change4_new.png]]

Similar refactoring was performed to obtain the retrieve_questions method:

[[File:Latest1.png]]

This is the new method created after the above refactoring:

[[File:Latest2.png]]

== Testing Details==

=== RSpec ===
There were no existing test cases for the GradesController. We have added a new spec file 'grades_spec.rb' which covers testing scenario for the newly added method. The specs were run on the previous and current files and they return the same results implying that the refactored code does not break anything.
As the model was not changed, no test cases were added for the model.

=== UI Testing ===

Following steps needs to be performed to test this code from UI: 
1. Login as instructor. Create a course and an assignment under that course. 
2. Keep the has team checkbox checked while creating the assignment. Add a grading rubric to it. Add at least two students as participants to the assignment. 
3. Create topics for the assignment. 
4. Sign in as one of the students who were added to the assignment. 
5. Go to the assignment and sign up for a topic. 
6. Submit student's work by clicking 'Your work' under that assignment. 
7. Sign in as a different student which is participant of the assignment. 
8. Go to Assignments--><assignment name>-->Others' work (If the link is disabled, login as instructor and change the due date of the assignment to current time). 
9. Give reviews on first student's work. 
10. Login as instructor or first student to look at the review grades. 

== Scope for future improvement ==
1. The construct_table method in GradesHelper is not used anywhere. It has no reference in the project. So we feel it can be safely removed. 
2. The has_team_and_metareview? method in GradesHelper can be broken down into separate methods, one each for team and metareview. This will provide improved flexibility. It needs some analysis though, as both the entities(team & metareview) are currently checked in conjuction from all the views they are referenced from.

CSC/ECE 517 Fall 2019 - E1944. Refactor review mapping controller

2019-10-29T02:26:29Z

Ysoodin: /* Peer Review Information */

This wiki page is for the description of changes made under E1555 OSS assignment for Fall 2015, CSC/ECE 517.

== Expertiza Background==

Expertiza is an educational web application created and maintained by the joint efforts of the students and the faculty at NCSU. It’s an open source project developed on Ruby on Rails platform and it’s code is available on Github. It allows students to review each other’s work and improve their work upon this feedback.

== Description of the current project ==

The following is an Expertiza based OSS project which deals primarily with the GradesController and GradesHelper. It focusses on refactoring some of the more complex methods, modifying some of the language to make it more Ruby friendly, removing some redundant code. The goal of this project is to attempt to make this part of the application easier to read and maintain.

== Files modified in current project ==

A controller and a helper file were modified for this project namely: 
1. GradesController 
2. GradesHelper 

=== GradesController ===

This is a controller that helps students and instructors view grades and reviews, update scores, check for grading conflicts and calculate penalties. A couple of long and complex methods were refactored from this controller along with removal of some non-functional code and a few language changes to make it Ruby style.
Three methods in particular, namely conflict_notification ,calculate_all_penalties and edit were found to be too long and were in need of refactoring into smaller, easier to manage methods. Few more compact methods were created for this purpose.

There were no existing test cases for the controller. We have added a spec file named 'grades_spec.rb' under the spec folder. As no changes were done for the model, no tests for the model were included.

=== GradesHelper ===

This is a helper class which contains methods for constructing a table(construct_table) and to check whether an assignment has a team and metareveiw(has_team_and_metareview)

== List of changes ==
We worked on the following work items(WIs) 
WI1 : Refactor calculate_all_penalties method into smaller methods 
WI2 : Move the repeated code in conflict_notification & edit methods to a separate method list_questions. 
WI3 : Refactor the code as per the Ruby style guidelines and incorporate the good practices 
WI4 : Test the conflict_notification method to test the changes made. 
WI5 : Move the repeated code in view and view_my_scores methods to a separate method retrieve_questions

=== Solutions Implemented and Delivered ===

*Refactoring calculate_all_penalties method

This is used to calculate various penalty values for each assignment if penalty is applicable.

The following changes were made:

1. This method was very complex, performing too many functions within a single method and had to be broken into 3 smaller methods each having a more well defined function.
2. The following 3 methods were created after splitting the first method 
i. calculate_all_penalties 
ii. calculate_penatly_attributes 
iii. assign_all_penalties 
3. Changes were also made to make the code follow ruby style.The language was made more ruby friendly.
4. Finally some redundant code was commented out as it was non-functional.

Refactoring into smaller more specific methods:

[[File:Change6_new.png]]

Removal of non-functional code :

[[File:Change5_new.png]]

Change of language to make it more Ruby friendly:

[[File:Change1_new.png]]

*Move the redundant piece of code from conflict_notification & edit methods to a new method list_questions

The conflict_notification method is used to help the instructors decide if one of the reviews are unfair or inaccurate.
This was again split into 2 methods with some part of the code which is repeated in another method refactored into a new method.

[[File:Change3_new.png]]

Refactored #Created a method which was a duplicate in conflict_notification and edit methods

[[File:Change4_new.png]]

edit method:

This method is used to edit the questionnaires. This method again has code which is repeated in the conflict_notification method and thus the repeated section was split into a new method.

[[File:Change2_new.png]]

New method:
Refactored #Created a method which was a duplicate in conflict_notification and edit methods

[[File:Change4_new.png]]

Similar refactoring was performed to obtain the retrieve_questions method:

[[File:Latest1.png]]

This is the new method created after the above refactoring:

[[File:Latest2.png]]

== Testing Details==

=== RSpec ===
There were no existing test cases for the GradesController. We have added a new spec file 'grades_spec.rb' which covers testing scenario for the newly added method. The specs were run on the previous and current files and they return the same results implying that the refactored code does not break anything.
As the model was not changed, no test cases were added for the model.

=== UI Testing ===

Following steps needs to be performed to test this code from UI: 
1. Login as instructor. Create a course and an assignment under that course. 
2. Keep the has team checkbox checked while creating the assignment. Add a grading rubric to it. Add at least two students as participants to the assignment. 
3. Create topics for the assignment. 
4. Sign in as one of the students who were added to the assignment. 
5. Go to the assignment and sign up for a topic. 
6. Submit student's work by clicking 'Your work' under that assignment. 
7. Sign in as a different student which is participant of the assignment. 
8. Go to Assignments--><assignment name>-->Others' work (If the link is disabled, login as instructor and change the due date of the assignment to current time). 
9. Give reviews on first student's work. 
10. Login as instructor or first student to look at the review grades. 

== Scope for future improvement ==
1. The construct_table method in GradesHelper is not used anywhere. It has no reference in the project. So we feel it can be safely removed. 
2. The has_team_and_metareview? method in GradesHelper can be broken down into separate methods, one each for team and metareview. This will provide improved flexibility. It needs some analysis though, as both the entities(team & metareview) are currently checked in conjuction from all the views they are referenced from.

CSC/ECE 517 Fall 2019 - E1944. Refactor review mapping controller

2019-10-29T02:26:09Z

Ysoodin: Created page with "This wiki page is for the description of changes made under E1555 OSS assignment for Fall 2015, CSC/ECE 517. == Peer Review Information == For users intending to view the deplo..."

This wiki page is for the description of changes made under E1555 OSS assignment for Fall 2015, CSC/ECE 517.

== Peer Review Information ==

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:
* Instructor login: username -> instructor6, password -> password
* Student login: username -> student4340, password -> password
* Student login: username -> student4405, password -> password

== Expertiza Background==

Expertiza is an educational web application created and maintained by the joint efforts of the students and the faculty at NCSU. It’s an open source project developed on Ruby on Rails platform and it’s code is available on Github. It allows students to review each other’s work and improve their work upon this feedback.

== Description of the current project ==

The following is an Expertiza based OSS project which deals primarily with the GradesController and GradesHelper. It focusses on refactoring some of the more complex methods, modifying some of the language to make it more Ruby friendly, removing some redundant code. The goal of this project is to attempt to make this part of the application easier to read and maintain.

== Files modified in current project ==

A controller and a helper file were modified for this project namely: 
1. GradesController 
2. GradesHelper 

=== GradesController ===

This is a controller that helps students and instructors view grades and reviews, update scores, check for grading conflicts and calculate penalties. A couple of long and complex methods were refactored from this controller along with removal of some non-functional code and a few language changes to make it Ruby style.
Three methods in particular, namely conflict_notification ,calculate_all_penalties and edit were found to be too long and were in need of refactoring into smaller, easier to manage methods. Few more compact methods were created for this purpose.

There were no existing test cases for the controller. We have added a spec file named 'grades_spec.rb' under the spec folder. As no changes were done for the model, no tests for the model were included.

=== GradesHelper ===

This is a helper class which contains methods for constructing a table(construct_table) and to check whether an assignment has a team and metareveiw(has_team_and_metareview)

== List of changes ==
We worked on the following work items(WIs) 
WI1 : Refactor calculate_all_penalties method into smaller methods 
WI2 : Move the repeated code in conflict_notification & edit methods to a separate method list_questions. 
WI3 : Refactor the code as per the Ruby style guidelines and incorporate the good practices 
WI4 : Test the conflict_notification method to test the changes made. 
WI5 : Move the repeated code in view and view_my_scores methods to a separate method retrieve_questions

=== Solutions Implemented and Delivered ===

*Refactoring calculate_all_penalties method

This is used to calculate various penalty values for each assignment if penalty is applicable.

The following changes were made:

1. This method was very complex, performing too many functions within a single method and had to be broken into 3 smaller methods each having a more well defined function.
2. The following 3 methods were created after splitting the first method 
i. calculate_all_penalties 
ii. calculate_penatly_attributes 
iii. assign_all_penalties 
3. Changes were also made to make the code follow ruby style.The language was made more ruby friendly.
4. Finally some redundant code was commented out as it was non-functional.

Refactoring into smaller more specific methods:

[[File:Change6_new.png]]

Removal of non-functional code :

[[File:Change5_new.png]]

Change of language to make it more Ruby friendly:

[[File:Change1_new.png]]

*Move the redundant piece of code from conflict_notification & edit methods to a new method list_questions

The conflict_notification method is used to help the instructors decide if one of the reviews are unfair or inaccurate.
This was again split into 2 methods with some part of the code which is repeated in another method refactored into a new method.

[[File:Change3_new.png]]

Refactored #Created a method which was a duplicate in conflict_notification and edit methods

[[File:Change4_new.png]]

edit method:

This method is used to edit the questionnaires. This method again has code which is repeated in the conflict_notification method and thus the repeated section was split into a new method.

[[File:Change2_new.png]]

New method:
Refactored #Created a method which was a duplicate in conflict_notification and edit methods

[[File:Change4_new.png]]

Similar refactoring was performed to obtain the retrieve_questions method:

[[File:Latest1.png]]

This is the new method created after the above refactoring:

[[File:Latest2.png]]

== Testing Details==

=== RSpec ===
There were no existing test cases for the GradesController. We have added a new spec file 'grades_spec.rb' which covers testing scenario for the newly added method. The specs were run on the previous and current files and they return the same results implying that the refactored code does not break anything.
As the model was not changed, no test cases were added for the model.

=== UI Testing ===

Following steps needs to be performed to test this code from UI: 
1. Login as instructor. Create a course and an assignment under that course. 
2. Keep the has team checkbox checked while creating the assignment. Add a grading rubric to it. Add at least two students as participants to the assignment. 
3. Create topics for the assignment. 
4. Sign in as one of the students who were added to the assignment. 
5. Go to the assignment and sign up for a topic. 
6. Submit student's work by clicking 'Your work' under that assignment. 
7. Sign in as a different student which is participant of the assignment. 
8. Go to Assignments--><assignment name>-->Others' work (If the link is disabled, login as instructor and change the due date of the assignment to current time). 
9. Give reviews on first student's work. 
10. Login as instructor or first student to look at the review grades. 

== Scope for future improvement ==
1. The construct_table method in GradesHelper is not used anywhere. It has no reference in the project. So we feel it can be safely removed. 
2. The has_team_and_metareview? method in GradesHelper can be broken down into separate methods, one each for team and metareview. This will provide improved flexibility. It needs some analysis though, as both the entities(team & metareview) are currently checked in conjuction from all the views they are referenced from.