View source for CSC/CSC 517 Spring 2017/oss E1718

E1718 was the project code for an Expertiza OSS assignment. It largely dealt with the handling of how peer reviews are assigned to students.

'''
== About Expertiza ==
'''

The Expertiza project is a collaborative open source project that allows courses to create assignments for students. One of the main functions of Expertiza is to allow students to peer review the submitted assignments by their fellow classmates. 


'''
== E1718 ==
'''

This page covers the E1718 project. E1718 covers the system behind assigning peer reviews to students. The main focus of this topic is the thresholds that guide which submissions a student can review as well as how many they need to review, and can review. A review can either be assigned to a student by the instructor, or it can be requested directly by the student. When the student handles it they can choose to be assigned a random submission, or they can choose the topic which they wish to review. Assignments can be set up so that each student must review a minimum number of submissions, with the choice of extra credit for completing up to a maximum number of reviews. When review topics are chosen, Expertiza sets it up so that the student can only choose a topic that has a low enough number of other reviews. At times this can constrain the choices a student has, so to fix this the instructor can set the threshold to allow for more choices. The threshold itself is the acceptable range for the number of reviews a topic has compared to the least reviewed topic. For instance, lets say there are three topics: Blue, Red and Green. Blue has had 1 review, Red has had 2 reviews, and Green has had 3 reviews. If the threshold is set to 0, then the only topic that can be chosen for review is Blue. If the threshold is 1, then both Blue and Red can be chosen. If it is 3, then all 3 topics can be chosen.

By solving the issues raised by E1718 this system will be improved in several ways. As currently set up, if a student has already reviewed all assignments within the minimum threshold, they will be unable to request another review. To solve this, when a student has reviewed all submissions within threshold k, the system should increase the threshold by m so that the student can review all submissions within threshold k+m. The current version of Expertiza also fails to show a student what the minimum number of reviews they must complete is, as well as the upper limit of reviews allowed. This will be fixed in this project so that an instructor can set the minimum and maximum number of reviews from the Review Strategy tab, and those limits show up on the student's end when viewing Others' Work. A previous issue that has since been solved was that a student was allowed to review their own submission. While this did not need further work, it did need automated testing to be written for it. Those tests were completed, as well as tests for the issues described above.

'''
== Issues ==
'''

'''228:''' Increase the threshold for a student who has already completed reviews for all submissions with the minimum number.

'''402:''' Write automated tests to check if a student can review a topic only they have submitted on.

'''417:''' Implement a num_reviews_required (and num_reviews_allowed) field in the assignments table to say how many reviews per reviewer are required, and how many are allowed (default should be # allowed = # req’d.).  Make it settable from the Review Strategy tab (see Issue 417) and viewable when a student clicks on “Others’ work”.


'''
== Solutions ==
'''

'''Project Files (new or changed)'''

*expertiza/spec/features/review_assignment.rb
*expertiza/db/migrate/20170322074046_add_num_metareviews_required_to_assignments.rb
*expertiza/db/migrate/20170322074238_add_num_metareviews_allowed_to_assignments.rb
*expertiza/db/migrate/schema.rb
*expertiza/app/models/assignment_form.rb
*expertiza/app/models/review_assignment.rb
*expertiza/app/views/assignments/edit/_review_strategy.html.erb
*expertiza/app/views/student_review/list.html.erb
*expertiza/spec/features/review_assignement_spec.rb

'''Issue 228'''

According to this issue, the reviewers, having reviewed the submission with the fewest outstanding reviews, can't review any other submission until the submission they've just reviewed catches up to other submissions in number of reviews. Reviewers who have already reviewed the submission with the fewest reviews should be able to review the submission with the next fewest reviews, regardless of what the threshold is. Solution to this would be, every time a student requests for review, setup a proper filtering process with appropriate thresholds. 

If topic x and topic y has two reviews each and topic z has no reviews, the reviewer is allowed to review topic z, as it has the least number of reviews. He is not allowed anymore reviews until topic z catches up with x and y. To rectify this, changes were made to review_assignment.rb file where when review is requested, initially the responses with no entries, self submitted topic are filtered out from the review pool. Then, if the reviewer has done any other reviews before, such reviews are filtered out. Then the review pool is combed for submissions with least reviews and the minimum reviews count is obtained. This, added to review_topic_threshold becomes the benchmarch against which number of reviews for a topic is measured, and if it’s less than the benchmark, the review is assigned to the requestee. This way, the reviewer is provided with more flexible review policy than before.

<pre>
    def filter_least_reviewed(contributor_set)
      contributor = contributor_set.min_by {|contributor| contributor.review_mappings.reject {|review_mapping| review_mapping.response.nil? }.count }
      minimum_reviews = contributor.review_mappings.reject {|review_mapping| review_mapping.response.nil? }.count rescue 0
      allowed_reviews = minimum_reviews + review_topic_threshold
      contributor_set.reject! {|contributor| contributor.review_mappings.reject {|review_mapping| review_mapping.response.nil? }.count > allowed_reviews }

      contributor_set
    end
</pre>

'''Issue 402'''

This solution required the creation of the review_assignment_spec.rb file. It conducts tests to ensure that a student may not review a topic in which their submission is the only one. This tests the review_assignment.rb module, which is heavily used by assignment.rb and review_mapping_controller.rb.

The specific tests that examined this potential error are as follows:
<pre>
  it "is not able to be assigned to review a topic only they have submitted on" do
    submit_to_topic
    visit '/student_task/list'
    click_link "TestAssignment"
    click_link "Others' work"
    find(:css, "#i_dont_care").set(true)
    click_button "Request a new submission to review"
    expect(page).to have_content "No topics are available to review at this time. Please try later."
  end

  it "is not able to select topic for review only they have submitted to" do
    submit_to_topic
    visit '/student_task/list'
    click_link "TestAssignment"
    click_link "Others' work"
    expect(page).to have_content 'Reviews for "TestAssignment"'
    expect(page).not_to have_button("topic_id_#{SignUpTopic.find_by_topic_name("TestTopic").id}")
  end
</pre>

The first test checks to see if a student can be assigned their own submission if they request a review via the "I don't care" checkbox, which allows them to be assigned a submission from any topic. Doing so correctly triggered the warning message "No topics are available to review at this time. Please try later." This was further backed up by a test in which another student submitted an assignment and the same path was followed. This case did not trigger a warning and the student was correctly assigned the submission. This shows the check is working as intended.

The second test checks to see if a student can choose a topic in which their assignment is the only submission. This was the main focus of issue 402. When choosing a topic the student must select a radio button for that topic and then request a review. This was no longer possible after the 402 fix as the student was unable to select the radio button for the topic if they were the only one who had submitted to it. Consequentially, this prevented the student from reaching the error message described in issue 402 "There are no more submissions to review on that topic." This test passed as expected showing the issue had been resolved. It was also tested whether they could select a topic in which they weren't the only submission, and this test passed as expected as well.

'''Issue 417'''

The 4 variables - num_reviews, num_reviews_required, num_metareviews_required and num_metareviews_allowed - are added to the assignment table using rails migration. The default of these variables are set to 3. 

<pre>
  class AddNumReviewsRequiredToAssignments < ActiveRecord::Migration
    def change
      add_column :assignments, :num_reviews_required, :integer, :default => 3
    end
  end

  class AddNumReviewsToAssignments < ActiveRecord::Migration
    def change
      change_column :assignments, :num_reviews, :integer, :default => 3
    end
  end

  class AddNumMetareviewsRequiredToAssignments < ActiveRecord::Migration
    def change
      add_column :assignments, :num_metareviews_required, :integer, :default => 3
    end
  end

  class AddNumMetareviewsAllowedToAssignments < ActiveRecord::Migration
    def change
      add_column :assignments, :num_metareviews_allowed, :integer, :default => 3
    end
  end

</pre>
These variables are viewable at "others" page for each assignment in students account, and settable at "edit assignment" page, "review_strategy" tab in instructor's account. The instructor are allowed to choose whether they want upper bound on number of reviews (num_reviews && num_metareviews_allowed) by clicking check box. By this way, the instructor can allow students to review as many submission as possible but grading only required number of reviews.


Validations are done by checking whether the number of reviews required is less than number of reviews allowed. i.e.) num_reviews_required <= num_reviews_allowed. Similarly for condition follows for meta reviews. These validation are done in assignment.rb file and are executed before an update is saved into database. 
<pre>
   class Assignment < ActiveRecord::Base
      validate :valid_num_review
      def valid_num_review
        if(self.num_reviews && self.num_reviews != -1 && self.num_reviews < self.num_reviews_required)
           self.errors.add(:message, "Num of reviews required cannot be greater than number of reviews allowed")
         elsif(self.num_metareviews_allowed && self.num_metareviews_allowed != -1 && self.num_metareviews_allowed < self.num_metareviews_required)
           self.errors.add(:message, "Number of Meta-Reviews required cannot be greater than number of meta-reviews allowed")
        end
      end
   end
</pre>
Here num_reviews and num_metareviews_allowed are negative when the upper bound for reivews are infinity.

The corresponding tests are written in assignment_spec.rb
<pre>
  it "check whether assignment doesnot accept required value greater than allowed value - Review" do
    @assignment.num_reviews = 1
    @assignment.num_reviews_required = 3
    expect(@assignment).not_to be_valid
  end

  it "check whether assignment doesnot accept required value greater than allowed value - Meta-Review" do
    @assignment.num_metareviews_allowed = 1
    @assignment.num_metareviews_required = 3
    expect(@assignment).not_to be_valid
  end
</pre>
The first test checks whether num_reviews accepts values less than num_reviews_required for which the validation should fail.
The Second test checks whether num_metareviews_allowed accepts values less than num_metareviews_required for which the validation should fail too.

'''

== Manual Testing ==
'''
*Please follow these instructions for testing these solutions manually.

'''228'''
* Log in as instructor(user:instructor6) and create a new assignment and add four students to the assignment(user:student360,student361,student362,student364). 
* Log in as student, submit an artifact. (e.g:a test link)
* Log in as second student and submit an artifact.
* Log in as first student and review an artifact, since there is only one other submission at this point, second student's submission is up for review.
* Log in as second student and review an artifact.
* Now, log in as third student, submit an artifact and also do two reviews, naturally you would be reviewing first and second student's submissions.
* At this point, first and second students' submission has two reviews and third student's submission has zero review. 
* Now, log in as the fourth student and review, you should be allowed to review all three artifacts instead of just the one with least reviews.

'''402'''
*Log in with an instructor.
*Create an assignment with topics to choose from.
*Log in with a student.
*Sign up for one of the topics from the previously created assignment.
*Submit an assignment for that topic.
*Go to the Others' Work tab.
*Attempt to select that topic for review -OR- Check the box for "I Don't Care"
*Attempt to request review


'''417'''
*Log in as an instructor
*go to edit page of an assignment and navigate to Review Strategy tab.
*Change the allowed and required field of reviews and meta-reviews.
*Try giving greater value for required field than allowed field. It should throw an error stating the relevant message.
*Give the correct value and save it.
*Sign in as a student who has enrolled for that particular assignment.
*go to "others" page.
*The allowed and required field are displayed saying that the assignment requires atleast k reviews.
*If meta-review is enabled for that topic then the meta-review information is also displayed. 
'''

== References ==
'''
#[https://github.com/expertiza/expertiza Expertiza GitHub]
#[http://expertiza.ncsu.edu/ Expertiza Website]
#[https://github.com/crajase/expertiza Expertiza GitHub Fork]
#[https://github.com/expertiza/expertiza/issues/228 Issue 228]
#[https://github.com/expertiza/expertiza/issues/402 Issue 402]
#[https://github.com/expertiza/expertiza/issues/417 Issue 417]