E1839 Review Requirements and Thresholds: Difference between revisions
m (→Test Plans) |
|||
Line 319: | Line 319: | ||
== Team Information == | == Team Information == | ||
Rayan Dasoriya (rdasori@ncsu.edu) | #Rayan Dasoriya (rdasori@ncsu.edu) | ||
#Prakshatkumar Shah (pmshah2@ncsu.edu) | |||
#Ravindersingh Rajpal (rkrajpal@ncsu.edu) | |||
#'''Mentor:''' Ed Gehringer (efg@ncsu.edu) | |||
==References== | |||
#[https://github.com/expertiza/expertiza Expertiza on GitHub] | |||
#[https://github.com/rayandasoriya/expertiza/tree/e1839 Github repository] | |||
#[http://expertiza.ncsu.edu/ The live Expertiza website] | |||
#[http://wikis.lib.ncsu.edu/index.php/Expertiza Expertiza project documentation wiki] | |||
#Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin |
Revision as of 23:58, 29 October 2018
Description
This wiki page is the description of the E1839 OSS Writeup project - Review Requirements and Thresholds - for Fall 2018, CSC/ECE 517.
Introduction
Expertiza is an open source project based on Ruby on Rails framework. The Expertiza project is a software that creates reusable learning objects through peer review. It is a web application where students can submit and peer-review learning objects (articles, code, websites, etc). It is used in some courses at NC State University and by professors at several other universities. It supports team projects, and the submission of almost any document type, including URLs and wiki pages. Expertiza enables the instructor to create new and customize existing assignments. It also enables the instructor to create a list of topics the students can sign up for as part of a project. Students can form teams in Expertiza to work on various projects and assignments. Expertiza supports submission across various document types, including the URLs and wiki pages.
Problem Description
In Expertiza, there are two ways of assigning reviews to reviewers:
1. The instructor decides who reviews whom (“instructor-selected”)
2. “Auto-selected,” in which case reviews are not assigned until a student seeks to choose something to review.
This section is mostly about auto-selected reviewing. A reviewer is ordinarily allowed to choose only among work that has received the minimum number of reviews so far. For example, if all submissions have received at least one review, the next person who tries to review is allowed to choose only a submission that has only one review so far, unless there are no more such submissions, in which case the reviewer chooses from among submissions that have 2 reviews (if there are any), or the minimum number of reviews, whatever that is.
For assignments that have topics, this severely constrains a reviewer’s choice of what topic to review on. (For assignments that don’t have topics, a reviewer doesn’t have anything to choose; (s)he just gets one of the submissions that have the fewest reviews so far.) To allow reviewers a larger set of topics to choose from, the instructor can set the threshold (on the Review Strategy tab of assignment creation/editing) to some integer k > 0. Then any submission that has within k reviews of the fewest number can be chosen by a new reviewer. Let’s say that all submissions have at least 1 review. If k = 3, then the reviewer can choose any topic where there is a submission that has 4 or fewer reviews so far.
There's another issue. Suppose that the minimum number of reviews for any submission is 2, but that the person has reviewed all the submissions that have only 2 reviews. Then they are not allowed to review at all (unless k > 0). That’s wrong; they should always be allowed to review the work with the least reviews that I have not already reviewed. And that should generalize to situations in which k > 0.
Another issue is that there is no way for Expertiza to tell a reviewer how many reviews are required. In the case of instructor-selected reviewing, that’s not a problem, but for auto-selected reviewing, there is no way to specify how many reviews are required (or even how many are allowed, in case students are allowed to do extra reviews).
Implementation
Issue #402 : To allow reviewers a larger set of topics to choose from, the instructor can set the threshold (on the Review Strategy tab of assignment creation/editing) to some integer k > 0. Then any submission that has within k reviews of the fewest number can be chosen by a new reviewer. Let’s say that all submissions have at least 1 review. If k = 3, then the reviewer can choose any topic where there is a submission that has 4 or fewer reviews so far. Suppose that the minimum number of reviews for any submission is 2, but that I have reviewed all the submissions that have only 2 reviews. Then I’m not allowed to review at all (unless k > 0).
- Solution: This issue has been fixed previously. The reviewer will get assigned a submission even if it has fulfilled the required number of reviews, to ensure that the reviewer always receives a new submission to review as long as it is not his own.
Issue #228 : To Allow a reviewer who has already reviewed all submissions that have the minimum number of reviews m to review any submission that has ≤ m+k reviews.
- Solution: This issue is similar to issue 402 and has been already fixed. If the student has already reviewed the least number of reviewed assignment, then he/she will get the next least reviewed assignment. Refer to expertiza/app/models/review_assignment.rb file
def candidate_topics_to_review(reviewer) return nil if sign_up_topics.empty? # This is not a topic assignment # Initialize contributor set with all teams participating in this assignment contributor_set = Array.new(contributors) # Reject contributors that have not selected a topic, or have no submissions contributor_set = reject_by_no_topic_selection_or_no_submission(contributor_set) # Reject contributions of topics whose deadline has passed, or which are not reviewable in the current stage contributor_set = reject_by_deadline(contributor_set) # Filter submissions already reviewed by reviewer contributor_set = reject_previously_reviewed_submissions(contributor_set, reviewer) # Filter submission by reviewer him/her self contributor_set = reject_own_submission(contributor_set, reviewer) # Filter the contributors with the least number of reviews # (using the fact that each contributor is associated with a topic) contributor_set = reject_by_least_reviewed(contributor_set) contributor_set = reject_by_max_reviews_per_submission(contributor_set) # if this assignment does not allow reviewer to review other artifacts on the same topic, # remove those teams from candidate list. contributor_set = reject_by_same_topic(contributor_set, reviewer) unless self.can_review_same_topic? # Add topics for all remaining submissions to a list of available topics for review candidate_topics = Set.new contributor_set.each do |contributor| candidate_topics.add(signed_up_topic(contributor)) end candidate_topics end
This snippet of code will do the following features:
1. Return nil if the array of choices of review is empty
2. Reject contributors that have not selected a topic, or have no submissions
3. Reject contributions of topics whose deadline has passed, or which are not reviewable in the current stage
4. Filter submissions already reviewed by a reviewer
5. Filter the contributors with the least number of reviews
6. If this assignment does not allow the reviewer to review other artifacts on the same topic, remove those teams
7. Add topics for all remaining submissions to a list of available topics for review
Issue #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 and viewable when a student clicks on “Others’ work”. Do the same for metareviews (“reviews of reviews”), which students can also be assigned to do for an Expertiza assignment.
- Solution: This issue has been resolved by adding the following piece of code in the file /expertiza/app/views/student_review/list.html.erb
<h4> Number of reviews left: <%= @assignment.num_reviews_allowed - @num_reviews_completed %></h4> <h4> Number of Meta-Reviews left: <%= @assignment.num_metareviews_allowed - @num_metareviews_completed %></h4>
Test Plans
Issue #402 and Issue #228 : This is shown graphically with dummy topics and assignment. We create a dummy assignment with two different topics oss1 and oss2. oss1 has 2 slots and oss2 has 1 slot. So when a student who has submitted oss2 tries to review others work he will not be able to select oos2 at all because he cannot review his own submission. When a person who has oss1 as the topic goes to this page and he asks to get a new submission for review it will give another slot’s oss1. If he does it again it says there are no more submissions available to review for this topic. So if he has already reviewed all the other submissions of his topic oss1 and he requests for one more submission of oss1, he will not get any more submission to review because the only one left is his own submission.
Issue #417 : We have added the reviews and meta-reviews left option. It can be demonstrated from the image below.
Improved UI and Ruby Code practices
- a. The two statements "Set Allowed Number of Reviews per reviewer" and "Set Required Number of Reviews per reviewer." are not well differentiated. It can be very confusing for an instructor to give values to them.
- Solution: Information buttons have been provided beside these statements in the review strategy tab to make sure that the instructor knows the difference between the two.
- b) When the number of allowed or required reviews are not set on the Review Strategy tab, the system does not have a message to display to a reviewer about how many submissions of work they are required to and allowed to review depending on the values set by the instructor in the "allowed number of reviewers per reviewer" field and the "Set Required Number of Reviews per reviewer." field.
- Solution: Added a toggle hidden option to show the options only when we are setting the max reviews and meta-reviews limit.
- c)The capitalization and punctuation of statements such as "Set Allowed Number of Reviews per reviewer" and "Set Required Number of Reviews per reviewer." in review strategy tab are incorrect.
- Solution: This view has been fixed by making changes in the respective files.
All these modifications are made in the app/views/assignments/edit/_review_strategy.html.erb
<!--The reason why add an empty form_tag here. (https://github.com/expertiza/expertiza/commit/4d1a9d50833aca4e8771afd851daa40455d4fc37)--> <%= form_tag controller: 'assignment', action: 'edit', assignment_id: @assignment_form.assignment.id do %> <% end %> <table border=0 class="table-hover table-borderless" style="width: 100%"> <col width="150"/> <col width="70"/> <col width="70"/> <tr id='assignment_review_assignment_strategy_row' class="form-inline"> <td> <%= label_tag('assignment_form[assignment][review_assignment_strategy]', 'Review Strategy:') %> <img src="/assets/info.png" title='- Instructor-Selected: Instructor decides who reviews whom- Auto-Selected: When a student is about to begin a review, Expertiza assigns that student a particular review.'> <%= select('assignment_form[assignment]', 'review_assignment_strategy', review_strategy_options, {:selected => @assignment_form.assignment.review_assignment_strategy, :style => 'margin-left :250px', :class => "form-control width-200"}) %> <script> jQuery(document).ready(function() {reviewStrategyChanged();}); jQuery('#assignment_form_assignment_review_assignment_strategy').on('change', function() {reviewStrategyChanged();}); </script> </td> </tr> <tr id="assignment_review_topic_threshold_row"> <tr class="form-inline"> <td class="form-inline"> <%= label_tag('assignment_form[assignment][review_topic_threshold]', 'Review Topic Threshold (k):') %> <img src="/assets/info.png" title='A topic is reviewable if the minimum number of reviews already done for the submissions on that topic is within k of the minimum number of reviews done on the least-reviewed submission on any topic.'> <%= text_field_tag('assignment_form[assignment][review_topic_threshold]', @assignment_form.assignment.review_topic_threshold, size: 1, class: "form-control width-70", :style => 'margin-left :220px') %> </td> </tr> <tr id="assignment_review_topic_threshold_row" class="form-inline"> <td> <%= label_tag('assignment_form[assignment][max_reviews_per_submission]', 'Maximum Number of Reviews Per Submission:') %> <img src="/assets/info.png" title='You can set the maximum number of reviews can be done for a particular submissions.'> <%= text_field_tag('assignment_form[assignment][max_reviews_per_submission]', @assignment_form.assignment.max_reviews_per_submission ||= 10, size: 2, class: "form-control width-70", :style => 'margin-left : 100px') %> </td> </tr> <tr> <td> <%= check_box_tag('review', 'true', true, {:onChange => 'hasReviewChanged()'}) %> <%= label_tag('review', 'Has Max Review Limit?') %> <img src="/assets/info.png" title='You can select whether the particular submission needs to have maximum limit for review or not.'> </td> <td> </td> </tr> <tr id="assignment_review_topic_threshold_row"> <td> <span id="reviews_allowed" <%= 'hidden' if @assignment_form.assignment.num_reviews_allowed.nil?%> class = "form-inline"> <%= label_tag('assignment_form[assignment][num_reviews_allowed]', ' Set Allowed Number of Reviews per Reviewer') %> <%if @assignment_form.assignment.num_reviews_allowed && @assignment_form.assignment.num_reviews_allowed >= 0 %> <%= text_field_tag('assignment_form[assignment][num_reviews_allowed]', @assignment_form.assignment.num_reviews_allowed, size: 1, class: "form-control width-70", :style => 'margin-left :130px') %> <%else%> <%= text_field_tag('assignment_form[assignment][num_reviews_allowed]', 3, size: 1, class: "form-control width-70", :style => 'margin-left :130px') %> <%end%> <br/> <%= label_tag('assignment_form[assignment][num_reviews_required]', 'Set Required Number of Reviews per Reviewer') %> <%= text_field_tag('assignment_form[assignment][num_reviews_required]', @assignment_form.assignment.num_reviews_required ||= @assignment_form.assignment.num_reviews, size: 1, class: "form-control width-70", :style => 'margin-left :120px') %> </span> </td> </tr> <tr> <td> <%= check_box_tag('meta_review', 'true', true, {:onChange => 'hasMetaReviewChanged()'}) %> <%= label_tag('meta_review', 'Has Meta-review Limit?') %> <img src="/assets/info.png" title='You can set if a particular submission can have a Meta-review or not.'> </td> <td> </td> </tr> <tr id="assignment_review_topic_threshold_row"> <td> <span id="meta_reviews_allowed" <%= 'hidden' if @assignment_form.assignment.num_metareviews_allowed.nil?%> class="form-inline"> <%= label_tag('assignment_form[assignment][num_metareviews_allowed]', ' Set Allowed Number of Meta-reviews per Reviewer') %> <%if @assignment_form.assignment.num_metareviews_allowed && @assignment_form.assignment.num_metareviews_allowed >= 0 %> <%= text_field_tag('assignment_form[assignment][num_metareviews_allowed]', @assignment_form.assignment.num_metareviews_allowed ||= 3, size: 1, class: "form-control width-70", :style => 'margin-left :100px') %> <%else%> <%= text_field_tag('assignment_form[assignment][num_metareviews_allowed]', 3, size: 1, class: "form-control width-70", :style => 'margin-left :100px') %> <%end%> </br> <%= label_tag('assignment_form[assignment][num_metareviews_allowed]', 'Set Required Number of Meta-reviews per Reviewer') %> <%if @assignment_form.assignment.num_metareviews_allowed && @assignment_form.assignment.num_metareviews_allowed >= 0 %> <%= text_field_tag('assignment_form[assignment][num_metareviews_allowed]', @assignment_form.assignment.num_metareviews_allowed ||= 3, size: 1, class: "form-control width-70", :style => 'margin-left :90px') %> <%else%> <%= text_field_tag('assignment_form[assignment][num_metareviews_allowed]', 3, size: 1, class: "form-control width-70", :style => 'margin-left :90px') %> <%end%> </span> </td> </tr> <tr id="instructor_selected_review_mapping_mechanism" hidden='hidden'> <td> <%= form_tag :controller => 'review_mapping', :action => 'automatic_review_mapping', :id => @assignment_form.assignment.id do %> <input name="max_team_size" type="hidden" value="<%= @assignment_form.assignment.max_team_size %>"/> <%= radio_button_tag 'num_reviews', 'student', true %> <%= label_tag('num_reviews_per_student', 'Set number of reviews done by each student') %> <span id='num_reviews_per_student_threshold' class="form-inline"> <%= text_field_tag('num_reviews_per_student', 0, {:class => 'form-control width-50'}) %> <%= submit_tag 'Assign reviewers', :id => 'first_submit_tag', :class => "btn btn-default" %> </span><br/> <%= radio_button_tag 'num_reviews', 'submission', false %> <%= label_tag('num_reviews_per_submission', 'Set minimum number of reviews done for each submission') %> <span id='num_reviews_per_submission_threshold' hidden='hidden'> <%= text_field_tag('num_reviews_per_submission', 0, {:style => 'width:20px'}) %> <%= submit_tag 'Assign reviewers', :id => 'second_submit_tag' %> </span><br/> <%= radio_button_tag 'num_reviews', 'calibrated_and_uncalibrated', false %> <%= label_tag('assign_both_calibrated_and_uncalibrated_artifacts', 'Set both calibrated and uncalibrated artifacts') %> <span id='calibrated_and_uncalibrated_artifacts_threshold' hidden='hidden'> <%= text_field_tag('num_calibrated_artifacts', 0, {:style => 'width:20px'}) %> <%= label_tag('num_calibrated_artifacts', 'of calibrated artifacts and ') %> <%= text_field_tag('num_uncalibrated_artifacts', 0, {:style => 'width:20px'}) %> <%= label_tag('num_uncalibrated_artifacts', 'of uncalibrated artifacts done by each student') %> <%= submit_tag 'Assign both calibrated and uncalibrated artifacts', :id => 'third_submit_tag' %> </span><br/> <% end %> </td> </tr> <tr> <td id='is_anonymous'> <input name="assignment_form[assignment][is_anonymous]" type="hidden" value="false"/> <%= check_box_tag('assignment_form[assignment][is_anonymous]', 'true', @assignment_form.assignment.is_anonymous) %> <%= label_tag('assignment_form[assignment][is_anonymous]', 'Is Review Anonymous?') %> <img src="/assets/info.png" title='You can select whether the review name should be visible or not'> </td> <td></td> </tr> <tr> <td id='assignment_is_selfreview_enabled'> <input name="assignment_form[assignment][is_selfreview_enabled]" type="hidden" value="false"/> <%= check_box_tag('assignment_form[assignment][is_selfreview_enabled]', 'true', @assignment_form.assignment.is_selfreview_enabled) %> <%= label_tag('assignment_form[assignment][is_selfreview_enabled]', 'Allow Self Reviews?') %> <img src="/assets/info.png" title='Select if you want a person to allow review their own submission'> </td> <td></td> </tr> </table> <script> function hasReviewChanged() { var checkbox = jQuery('#review'); var reviews_allowed_field = jQuery('#reviews_allowed'); if (checkbox.is(':checked')) { reviews_allowed_field.removeAttr('hidden'); jQuery('#assignment_form_assignment_num_reviews_allowed').val('<%=@assignment_form.assignment.num_reviews_allowed ||= 3%>'); } else { reviews_allowed_field.attr('hidden', true); jQuery('#assignment_form_assignment_num_reviews_allowed').val('-1'); } } function hasMetaReviewChanged() { var checkbox = jQuery('#meta_review'); var meta_reviews_allowed_field = jQuery('#meta_reviews_allowed'); if (checkbox.is(':checked')) { meta_reviews_allowed_field.removeAttr('hidden'); jQuery('#assignment_form_assignment_num_metareviews_allowed').val(<%=@assignment_form.assignment.num_metareviews_allowed ||= 3%>); } else { meta_reviews_allowed_field.attr('hidden', true); jQuery('#assignment_form_assignment_num_metareviews_allowed').val('-1'); } } </script>
Design Pattern
1. MVC
The project is implemented in Ruby on Rails that uses MVC architecture. It separates an application’s data model, user interface, and control logic into three distinct components (model, view and controller, respectively).
2. Dry Principle
We are trying to reuse the existing functionalities in Expertiza, thus avoiding code duplication. Whenever possible, code modification based on the existing classes, controllers, or tables will be done instead of creating the new one.
Team Information
- Rayan Dasoriya (rdasori@ncsu.edu)
- Prakshatkumar Shah (pmshah2@ncsu.edu)
- Ravindersingh Rajpal (rkrajpal@ncsu.edu)
- Mentor: Ed Gehringer (efg@ncsu.edu)
References
- Expertiza on GitHub
- Github repository
- The live Expertiza website
- Expertiza project documentation wiki
- Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin