CSC/ECE 517 Spring 2014/oss E1402 mmb: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
No edit summary
 
(53 intermediate revisions by 3 users not shown)
Line 1: Line 1:
== Background ==
== Background ==
There are three different review strategies in Expertiza.
There are three review strategies in Expertiza:
<br>Auto Selected
#Auto Selected
<br>Student Selected
#Student Selected
<br>Instructor Selected
#Instructor Selected
<br>In auto-selected reviewing, a reviewer picks the topic to review on, but not the individual submission.
In auto-selected reviewing, a reviewer picks the topic to review on, but not the individual submission. Student-selected allows the reviewer to pick from a list of submissions, not topics. In instructor-selected, instructor/TA assigns submissions to reviewers. In auto-selected and student-selected reviewing, the student is not allowed to pick from any topic or submission, but only to choose among those that have received the fewest reviews so far.  This strategy may be modified by selecting a non-zero threshold k as follows:
Student-selected allows the reviewer to pick from a list of submissions, not topics.
Instructor-selected is where the instructor/TA assigns submissions to reviewers.
In the case of auto-selected and student-selected reviewing, the student is not allowed to pick from any topic or submission, but only to choose among those that have received the fewest reviews so far.  But this strategy may be modified by selecting a non-zero threshold k as follows
A topic is review able 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.
A topic is review able 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.
==Motivation==
==Motivation==
The goal of Expertiza is to support student-generated content through peer review and teamwork. It is a peer-review system that allows students to select a topic to work on, submit their work and review their peers' submissions. As mentioned above there are different strategies available for reviewing other student's work. However, these strategies do not work as expected. It allows students to choose many topics without submitting any review for them. In some cases, this causes total number of reviews for a submission to exceed the maximum limit set for assignment. Allowing only the topic/submission with the minimum reviews to be allowed for review, did not allow users to select a topic which has few reviews more than the minimum but has not yet crossed the maximum threshold. Certain changes in the algorithm for making a topic/submission allowable for review helps us overcome these problems.


==Design==
==Design==
Few faulty designs in the project which we have improvised are:
#The view '_set_self_review' was related to Student-selected review strategy. This name was misleading. We have refactored and renamed it as '_set_student_review'.
#Ability to set maximum threshold of reviews for an assignment should be available only for Auto-selected and Student-selected but not for Instructor-Selected review strategies. The existing design hid this option to set max threshold for Auto-selected only. We have fixed this faulty behavior by fixing the jQuery code to hide this field for Instuctor-selected reviewing only. <br>Another design alternative was to display this field only for student and auto selected. After discussing this with instructor, we concluded that it will be better to hide this field for Instructor-selected so that code update is not required if new review strategies are added latter on.
#If the assignment had no topics, the user was directed to a page with a single button which further directed him to the page which listed available submissions for review. Although there is nothing wrong in this approach, the intermediate page was redundant and not performing any task. So we have placed a redirect using JavaScript button click to redirect user to next page directly.
#The existing design had no restriction on the number of reviews the student is allowed to begin without actually submitting any of them. This design had some problems. As mentioned before, this could evade the maximum number of reviews for a submission. Another problem was some students were unable to perform any reviews as others performed too many. We have devised an improved strategy which overcomes these drawbacks. If we restrict the outstanding number of reviews per student to three, number of selections per student will get restricted if they have more than 3 outstanding (i.e. topic selected, but review not started) reviews.
#A topic with only reviewer's own submission or with already reviewed submissions was also available for selection. Since reviewer can't review own submission or any submission already reviewed, he/she was provided a message that no submission is available to review after selecting such a topic. The user then had to go back and select some other topic if available. This process was time-consuming. We have improved this design by displaying only topics with valid submissions to review.
==Implementation==
==Implementation==
===Objective===
 
#Objective 1
===Objective 1===
Update code for Student-selected reviewing to cycle through submissions and not topics.<br/>
> We observed that student-selected reviewing cycles through submission after topic-selection. The topic selection makes sense instead of listing out all submissions in a topic-based assignment since reviewer can make a better choice of shortlisting submissions to review after choosing a topic of interest.
 
===Objective 2===
Make the ‘maximum threshold’ input field appear on Create/Edit assignment>Review Strategy tab only when Auto-selected or Student-selected reviewing is chosen and not when Instructor-selected is chosen.
 
> The Maximum Threshold Box was getting displayed or hidden using JavaScript code. However the logic used to display was faulty. We modified the code according to requirements to hide this box when Instructor-selected reviewing was chosen.
<br>
> We also removed JavaScript functions repeated twice in this view.
<br><br>
views>assignments>edit>general.html.erb>reviewStrategyChanged() (called from _review_strategy.html.erb)
<br>
<pre>
function reviewStrategyChanged() {
    var val = jQuery('#assignment_review_assignment_strategy').val();
        if (val == 'Student-Selected' || val == 'Auto-Selected') {
          jQuery('#assignment_review_topic_threshold_row').removeAttr('hidden');
        } else {
          jQuery('#assignment_review_topic_threshold_row').attr('hidden', true);
        }
  }
</pre>
 
===Objective 3===
Allowing a student to choose any number of reviews to do, without submitting any of them, allows a student to work on multiple reviews at the same time, but it also allows students to evade the limit on number of reviews that can be done. Devise a strategy to introduce certain threshold so that the student does not evades this limit.
 
> We implemented the following strategy. Introduction of a fixed threshold value - '3' which limits the number of reviews the user can begin without submitting previously selected reviews. Although this threshold can be chosen dynamically by the instructor or for an assignment, we have kept it static because it is not dependent on any assignment. It would be a good practice for restricting the user at a static value for every assignment.
We have calculated this condition in the list action of student_review_controller.rb and checked it on page student_review/list.html.erb. This will help in limiting the evasion of maximum threshold.
<pre>
  <% if @num_reviews_in_progress < 3 %>
    <% if @assignment.review_assignment_strategy == Assignment::RS_AUTO_SELECTED %>
      <%= render :partial => 'set_dynamic_review', :locals => {:assignment => @assignment} %>
    <% elsif @assignment.review_assignment_strategy == Assignment::RS_STUDENT_SELECTED %>
      <%= render :partial => 'set_student_review', :locals => {:assignment => @assignment} %>
    <% end %>
  <% else%>
    <br><br>
    <span>Note: You can't have more than 3 outstanding review. Complete existing reviews.</span>
  <% end %>
</pre>
Also we have introduced code to reject user's own submission and submissions already reviewed by user so that user has right choice of submission to review every time mentioned in Objective 5.
 
===Objective 4===  
If there are no topics, Auto-selected reviewing should still work, but it should select one of the submissions that has the fewest reviews so far.  Test this.
 
> Successfully tested with threshold set zero and it works as expected.
 
===Objective 5===
Fix Issue 402 on the Github issue tracker.
 
> If the reviewer selects his own submission for review, he is not allowed to perform the review and presented with message saying “ There are no more submissions to review on that topic.” Instead it would be better not to allow him to select his own submission for review. Also, if the user has reviewed the submission/topic with the least reviews, he should be allowed to perform review on other topics which have not reached their maximum threshold.
<br>
> We have also refactored the candidate_topics_to_review() method into multiple methods.
> models>assignment.rb>candidate_topics_to_review
<pre>
# Filter submission by reviewer him/her self
  contributor_set=reject_own_submission(contributor_set, reviewer)
 
# Filter submissions already reviewed by reviewer
  contributor_set=reject_previously_reviewed_submissions(contributor_set, reviewer)
</pre>
<pre>
  def reject_previously_reviewed_submissions(contributor_set, reviewer)
    contributor_set.reject! { |contributor| contributor.reviewed_by?(Participant.where(user_id: reviewer.id, parent_id: contributor.assignment.id).first) }
    return contributor_set
  end
 
  def reject_own_submission(contributor_set, reviewer)
    contributor_set.reject! { |contributor| contributor.teams_users.find_by_user_id(reviewer.id) }
    return contributor_set
  end
</pre>
 
===Objective 6===
Provide a comprehensive suite of tests so that we can be sure that all review strategies are working.
 
> Discussed in Section 6. Testing
 
==Issues==
==Issues==
===Initial Setup===
There were many issues faced in the process of fixing the review strategies. The issues are as follos:
*Unsuccessful bundler install
We faced the issue of raspell.c, which is the dictionary for auto-completing the English words. This was resolved by commenting out the following gems from the gem file
gem 'automated_metareview', github: 'expertiza/automated_metareview'
gem 'raspell'
*Databases for Expertiza project
We initially had to set up several databases like student, instructor, assignments, review rubrics. This was resolved with the help of 'expertiza_scrubbed' versions provided by the instructor and TA.
*Troubleshooting the Expertiza project.
We had troubles to run the database dump. Constantly rake db:migrate was failing, tree display was not working and we were always prompted with "No_method" error. This was again resolved by completely replacing the existing database-dump
===Core tasks===
*Permission setting for review strategy
Fistly, we tried to understand the working of the present code. We created a new assignment with 5 topics. We logged in as a student to select the topics, but we were prompted with the 'Permission Denied to sign_up_sheet#add_signup_topics'. This was resolved by changing the action_allowed method of sign_up_sheet_controller
*Missing partials
Some of the partials were missing in the given project code like submitted_content/_self_review.html.
*Missing table attributes
There were some missing table attributes in the given database which resulted in "No_Method" error. We had to create a few attributes like created_at and updated_at in the ResponseMap table.
*Tweaking the code
Some of the non-existing/irrelevant methods and attributes were accessed. These issues were resolved by modifying the code by comparing with previous versions and using Rails console. The corresponding actions were tested for correctness.
*Incomplete views
The student_review/_response.html view had incomplete conditional code. We fixed the code manually by understanding and testing the code flow.
*Duplicate methods
There were present multiple definitions for the same function (e.g., new,edit) in response controller. We had to discard the method that was not referenced.
==Testing==
Testing was done with the Unit tests. Test cases were written for the corresponding models. The following commands were performed before writing the test cases: <br/>
rake db:test:clone<br/>
rake db:test:clone_structure<br/>
rake db:test:load<br/>
rake db:test:prepare<br/>
rake db:test:purge<br/>
Generally one can write test cases to check the following:
*Web request successful
*User redirected to the right page
*Successfully authenticated the user/object
*Correct object stored in response
*Appropriate message displayed for the user. <br/>
The following test cases were written for the review_strategies:
;test_candidate_topics_to_review
:A test case was written to test if candidate_topics_to_review returns nil if there are no topics to the assignment submission.
;test_signup_topics
:A test case was written to test if the existing signup_topic method in the expertiza code. The functionality of signup_topic method was tested: <br/>
:signup_topics return non-empty set of values if it has topics for the assignment.
:signup_topics returns a empty set of values if it has no topics for the assignment. <br/>
;tests for candidate_topics_to_review
:We re-factored the candiate_topics_to_review method in the assignment.rb model. contributor set is build according to a set of conditions. The modified candiate_topics_to_review is more modular and clean. <br/>
Test cases are written for each of the method that describes each condition.
==Future Work==
* Fix control flow of student-selected reviewing similar/through auto-selected reviewing. Currently, student-selected reviewing calls review_assignment() method in DynamicReviewAssignmentHelper which filters topics/submissions similar to auto-selected but not as efficiently. As per Professor's comments, student-selected reviewing process is the same as auto-selected except steps 3 and 5 are not needed, and in step 2, the check is only for whether they have submitted. Steps are as follows:
** A contributor set is initialized with all of the teams participating in the assignment.
** Contributors are removed if they have not selected a topic or not submitted.
** Contributors are removed if their topic is not reviewable at the current time.
** Contributors are removed if they have “too many” reviews (as determined by the review threshold).
** Then we cycle through the remaining contributors, adding their topics to candidate_topics set.
* Logic to filter candidates is repeated in candidate_topics_to_review() and contributor_to_review() methods. We could skip filtering in the contributor_to_review() method.
* For student-selected assignments with no topics, a single button is displayed to proceed to submission-selection page. We have currently implemented it using JavaScript to call the click() function of this button. This could be implemented in a better way by calling the next controller action if no topics are present.
* The view views/student_review/_response.html.erb contains a lot of procedural code. This should ideally be moved from view to controller.

Latest revision as of 23:18, 7 April 2014

Background

There are three review strategies in Expertiza:

  1. Auto Selected
  2. Student Selected
  3. Instructor Selected

In auto-selected reviewing, a reviewer picks the topic to review on, but not the individual submission. Student-selected allows the reviewer to pick from a list of submissions, not topics. In instructor-selected, instructor/TA assigns submissions to reviewers. In auto-selected and student-selected reviewing, the student is not allowed to pick from any topic or submission, but only to choose among those that have received the fewest reviews so far. This strategy may be modified by selecting a non-zero threshold k as follows: A topic is review able 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.

Motivation

The goal of Expertiza is to support student-generated content through peer review and teamwork. It is a peer-review system that allows students to select a topic to work on, submit their work and review their peers' submissions. As mentioned above there are different strategies available for reviewing other student's work. However, these strategies do not work as expected. It allows students to choose many topics without submitting any review for them. In some cases, this causes total number of reviews for a submission to exceed the maximum limit set for assignment. Allowing only the topic/submission with the minimum reviews to be allowed for review, did not allow users to select a topic which has few reviews more than the minimum but has not yet crossed the maximum threshold. Certain changes in the algorithm for making a topic/submission allowable for review helps us overcome these problems.

Design

Few faulty designs in the project which we have improvised are:

  1. The view '_set_self_review' was related to Student-selected review strategy. This name was misleading. We have refactored and renamed it as '_set_student_review'.
  2. Ability to set maximum threshold of reviews for an assignment should be available only for Auto-selected and Student-selected but not for Instructor-Selected review strategies. The existing design hid this option to set max threshold for Auto-selected only. We have fixed this faulty behavior by fixing the jQuery code to hide this field for Instuctor-selected reviewing only.
    Another design alternative was to display this field only for student and auto selected. After discussing this with instructor, we concluded that it will be better to hide this field for Instructor-selected so that code update is not required if new review strategies are added latter on.
  3. If the assignment had no topics, the user was directed to a page with a single button which further directed him to the page which listed available submissions for review. Although there is nothing wrong in this approach, the intermediate page was redundant and not performing any task. So we have placed a redirect using JavaScript button click to redirect user to next page directly.
  4. The existing design had no restriction on the number of reviews the student is allowed to begin without actually submitting any of them. This design had some problems. As mentioned before, this could evade the maximum number of reviews for a submission. Another problem was some students were unable to perform any reviews as others performed too many. We have devised an improved strategy which overcomes these drawbacks. If we restrict the outstanding number of reviews per student to three, number of selections per student will get restricted if they have more than 3 outstanding (i.e. topic selected, but review not started) reviews.
  5. A topic with only reviewer's own submission or with already reviewed submissions was also available for selection. Since reviewer can't review own submission or any submission already reviewed, he/she was provided a message that no submission is available to review after selecting such a topic. The user then had to go back and select some other topic if available. This process was time-consuming. We have improved this design by displaying only topics with valid submissions to review.

Implementation

Objective 1

Update code for Student-selected reviewing to cycle through submissions and not topics.
> We observed that student-selected reviewing cycles through submission after topic-selection. The topic selection makes sense instead of listing out all submissions in a topic-based assignment since reviewer can make a better choice of shortlisting submissions to review after choosing a topic of interest.

Objective 2

Make the ‘maximum threshold’ input field appear on Create/Edit assignment>Review Strategy tab only when Auto-selected or Student-selected reviewing is chosen and not when Instructor-selected is chosen.

> The Maximum Threshold Box was getting displayed or hidden using JavaScript code. However the logic used to display was faulty. We modified the code according to requirements to hide this box when Instructor-selected reviewing was chosen.
> We also removed JavaScript functions repeated twice in this view.

views>assignments>edit>general.html.erb>reviewStrategyChanged() (called from _review_strategy.html.erb)

function reviewStrategyChanged() {
     var val = jQuery('#assignment_review_assignment_strategy').val();
        if (val == 'Student-Selected' || val == 'Auto-Selected') {
          jQuery('#assignment_review_topic_threshold_row').removeAttr('hidden');
        } else {
          jQuery('#assignment_review_topic_threshold_row').attr('hidden', true);
        }
   }

Objective 3

Allowing a student to choose any number of reviews to do, without submitting any of them, allows a student to work on multiple reviews at the same time, but it also allows students to evade the limit on number of reviews that can be done. Devise a strategy to introduce certain threshold so that the student does not evades this limit.

> We implemented the following strategy. Introduction of a fixed threshold value - '3' which limits the number of reviews the user can begin without submitting previously selected reviews. Although this threshold can be chosen dynamically by the instructor or for an assignment, we have kept it static because it is not dependent on any assignment. It would be a good practice for restricting the user at a static value for every assignment. We have calculated this condition in the list action of student_review_controller.rb and checked it on page student_review/list.html.erb. This will help in limiting the evasion of maximum threshold.

  <% if @num_reviews_in_progress < 3 %>
    <% if @assignment.review_assignment_strategy == Assignment::RS_AUTO_SELECTED %>
      <%= render :partial => 'set_dynamic_review', :locals => {:assignment => @assignment} %>
    <% elsif @assignment.review_assignment_strategy == Assignment::RS_STUDENT_SELECTED %>
      <%= render :partial => 'set_student_review', :locals => {:assignment => @assignment} %>
    <% end %>
  <% else%>
    <br><br>
    <span>Note: You can't have more than 3 outstanding review. Complete existing reviews.</span>
  <% end %>

Also we have introduced code to reject user's own submission and submissions already reviewed by user so that user has right choice of submission to review every time mentioned in Objective 5.

Objective 4

If there are no topics, Auto-selected reviewing should still work, but it should select one of the submissions that has the fewest reviews so far. Test this.

> Successfully tested with threshold set zero and it works as expected.

Objective 5

Fix Issue 402 on the Github issue tracker.

> If the reviewer selects his own submission for review, he is not allowed to perform the review and presented with message saying “ There are no more submissions to review on that topic.” Instead it would be better not to allow him to select his own submission for review. Also, if the user has reviewed the submission/topic with the least reviews, he should be allowed to perform review on other topics which have not reached their maximum threshold.
> We have also refactored the candidate_topics_to_review() method into multiple methods. > models>assignment.rb>candidate_topics_to_review

# Filter submission by reviewer him/her self
  contributor_set=reject_own_submission(contributor_set, reviewer)

# Filter submissions already reviewed by reviewer
  contributor_set=reject_previously_reviewed_submissions(contributor_set, reviewer)
  def reject_previously_reviewed_submissions(contributor_set, reviewer)
    contributor_set.reject! { |contributor| contributor.reviewed_by?(Participant.where(user_id: reviewer.id, parent_id: contributor.assignment.id).first) }
    return contributor_set
  end

  def reject_own_submission(contributor_set, reviewer)
    contributor_set.reject! { |contributor| contributor.teams_users.find_by_user_id(reviewer.id) }
    return contributor_set
  end

Objective 6

Provide a comprehensive suite of tests so that we can be sure that all review strategies are working.

> Discussed in Section 6. Testing

Issues

Initial Setup

There were many issues faced in the process of fixing the review strategies. The issues are as follos:

  • Unsuccessful bundler install

We faced the issue of raspell.c, which is the dictionary for auto-completing the English words. This was resolved by commenting out the following gems from the gem file gem 'automated_metareview', github: 'expertiza/automated_metareview' gem 'raspell'

  • Databases for Expertiza project

We initially had to set up several databases like student, instructor, assignments, review rubrics. This was resolved with the help of 'expertiza_scrubbed' versions provided by the instructor and TA.

  • Troubleshooting the Expertiza project.

We had troubles to run the database dump. Constantly rake db:migrate was failing, tree display was not working and we were always prompted with "No_method" error. This was again resolved by completely replacing the existing database-dump

Core tasks

  • Permission setting for review strategy

Fistly, we tried to understand the working of the present code. We created a new assignment with 5 topics. We logged in as a student to select the topics, but we were prompted with the 'Permission Denied to sign_up_sheet#add_signup_topics'. This was resolved by changing the action_allowed method of sign_up_sheet_controller

  • Missing partials

Some of the partials were missing in the given project code like submitted_content/_self_review.html.

  • Missing table attributes

There were some missing table attributes in the given database which resulted in "No_Method" error. We had to create a few attributes like created_at and updated_at in the ResponseMap table.

  • Tweaking the code

Some of the non-existing/irrelevant methods and attributes were accessed. These issues were resolved by modifying the code by comparing with previous versions and using Rails console. The corresponding actions were tested for correctness.

  • Incomplete views

The student_review/_response.html view had incomplete conditional code. We fixed the code manually by understanding and testing the code flow.

  • Duplicate methods

There were present multiple definitions for the same function (e.g., new,edit) in response controller. We had to discard the method that was not referenced.

Testing

Testing was done with the Unit tests. Test cases were written for the corresponding models. The following commands were performed before writing the test cases:
rake db:test:clone
rake db:test:clone_structure
rake db:test:load
rake db:test:prepare
rake db:test:purge
Generally one can write test cases to check the following:

  • Web request successful
  • User redirected to the right page
  • Successfully authenticated the user/object
  • Correct object stored in response
  • Appropriate message displayed for the user.

The following test cases were written for the review_strategies:

test_candidate_topics_to_review
A test case was written to test if candidate_topics_to_review returns nil if there are no topics to the assignment submission.
test_signup_topics
A test case was written to test if the existing signup_topic method in the expertiza code. The functionality of signup_topic method was tested:
signup_topics return non-empty set of values if it has topics for the assignment.
signup_topics returns a empty set of values if it has no topics for the assignment.
tests for candidate_topics_to_review
We re-factored the candiate_topics_to_review method in the assignment.rb model. contributor set is build according to a set of conditions. The modified candiate_topics_to_review is more modular and clean.

Test cases are written for each of the method that describes each condition.

Future Work

  • Fix control flow of student-selected reviewing similar/through auto-selected reviewing. Currently, student-selected reviewing calls review_assignment() method in DynamicReviewAssignmentHelper which filters topics/submissions similar to auto-selected but not as efficiently. As per Professor's comments, student-selected reviewing process is the same as auto-selected except steps 3 and 5 are not needed, and in step 2, the check is only for whether they have submitted. Steps are as follows:
    • A contributor set is initialized with all of the teams participating in the assignment.
    • Contributors are removed if they have not selected a topic or not submitted.
    • Contributors are removed if their topic is not reviewable at the current time.
    • Contributors are removed if they have “too many” reviews (as determined by the review threshold).
    • Then we cycle through the remaining contributors, adding their topics to candidate_topics set.
  • Logic to filter candidates is repeated in candidate_topics_to_review() and contributor_to_review() methods. We could skip filtering in the contributor_to_review() method.
  • For student-selected assignments with no topics, a single button is displayed to proceed to submission-selection page. We have currently implemented it using JavaScript to call the click() function of this button. This could be implemented in a better way by calling the next controller action if no topics are present.
  • The view views/student_review/_response.html.erb contains a lot of procedural code. This should ideally be moved from view to controller.