CSC/ECE 517 Spring 2024/ OSS E2400 Allow Reviewers to Bid on What to Review

This wiki page provides information regarding the refactoring of E2400 review_mapping_controller.rb OSS project for Spring 2024, CSC/ECE 517

Introduction

The Expertiza project makes use of peer review to allow students to learn from one another. It's a Ruby on Rails-based open-source application. It is used by faculty and students for the management of courses and assignments for specific courses. Different screens in the manage content area of the application offer information about users, courses, assignments, questionnaires, and reviews.

A review bid is a system used in Expertiza, where students indicate their preferences for reviewing certain assignments or projects. Students bid on the tasks they prefer to review through ranking their interests. The allocation of review assignments is determined by a bidding algorithm, balancing student preferences with fair and efficient distribution.

Test Login Credentials

UserId: instructor6

Password: password

Problem Statement

1. The process of reviewing plays a crucial role in the learning and improvement cycle, benefiting both the reviewer and the reviewee. Allowing reviewers to choose their topics freely can lead to more thorough reviews and increased engagement. However, the current system of assigning review topics on a first-come-first-served basis disadvantages students who are slower to sign up, preventing them from reviewing topics they are most interested in.
2. An improved approach would be to introduce a bidding system for review topics, similar to the one used for topic selection. This system would enable students to express their preferences for topics they are keen to review. An effective bidding algorithm could ensure that all interested students receive a topic they are eager to review.
3. While the prior work met the functional requirements, it could not be integrated due to numerous violations of the DRY (Don't Repeat Yourself) principle.

Background

When topics are opened up for bidding, students can see how “hot” each topic is by the color it has on their topic list. However, instructors have no way to view the bidding process except by impersonating students. Furthermore, when the bidding assignment algorithm is run, there is no way to verify that it did in fact assign teams to topics they had chosen.

Feature Overview

Current Topic Bidding Feature

Students may bid on topics they would like to work on. The following image shows the functionality of this feature.

Students are allowed to bid on several topics, ranking them by priority. When the bidding deadline passes, every student is allocated a topic based on their preferences.

This allocation process utilizes a modified Gale-Shapely Algorithm to ensure a stable match between students and topics, aiming to satisfy student preferences as much as possible. To resolve preference ties, topics prioritize students based on whether they are bidding as part of a group (with group members receiving higher priority) and the timing of their bid (earlier bids receive higher priority). After the bidding deadline:

1. Students are initially paired with their top-choice topic.
2. If a topic exceeds its capacity, it retains only the students with the highest priority.
3. Topics that have reached capacity are then removed from the selection pool.
4. Unmatched students repeat the process, selecting from the remaining topics.
5. This cycle continues until every student has been assigned a topic.
6. The algorithm checks for and corrects any 'unstable' matches, where swapping students between topics would more accurately fulfill their preferences.

This process, with a time complexity of O(n^2), ensures that all students are matched with a topic they prefer, respecting their choices as closely as possible. For further information on this algorithm and its application, one can refer to documentation from the previous year when this method was first implemented.

Preferred Features for Reviews

The following Use Case Diagram demonstrates the primary functionality explained in the previous section for different types of users.

The desired feature for reviews should have a nearly same layout as this, but with reviews rather than themes. Students should be presented a list of topics that have been worked on, and they should be able to bid on the ones they want to review the most. The Priority system might be implemented in the same way, with the exception that a student should not be able to review their own work.

This feature appears as an option when editing/creating a new assignment under the "Topics" area, as shown in the below.

When this option is enabled, students will be able to access a new view that lists of the topics from submitted assignments (except their own). The user can drag the topics from topics table from the left into the selection table. The user can rearrange the selected topics in the selection table based on the priority. The list of prioritized topics are used to run the bidding algorithm.

Proposed Solutions

Review_bids_others_work is a DRY violation

The previously implemented code had DRY violation found here The below code is implemented to over come the DRY violation.

<% def display_topic_and_participant(title, review_no, topic_id, assignment, participant) %> 
  <td>
  <b><%= "#{title} #{review_no}." %></b>
  # --In ‘review_bids/index’ page, if the topic_identifier is empty, ‘:’ sign should not be displayed.-->
  <% topic = SignUpTopic.find(topic_id) %>
  <% if !topic_id.nil? %>
    <%= "#{topic.topic_identifier.present? ? "#{topic.topic_identifier}: " : ""}#{topic.topic_name}" %>
  <%end%>
  <%if !@assignment.is_anonymous%>
      (author: <%= participant.fullname%>)
  <%end%>
  </td>
<% end %>


<% def display_assignment_reviews_info(assignment, num_reviews_completed) %>
  <h2>Reviews for "<%= assignment.name %>"</h2>
  <!-- add headers that provide information regarding review numbers -->
  <% if assignment.num_reviews_allowed.nil? || assignment.num_reviews_allowed == -1 %>
    <h4>Your instructor expects you to do <%= assignment.num_reviews_required %> reviews. You are not allowed to do any extra reviews. </h4>
  <% elsif assignment.num_reviews_allowed == assignment.num_reviews_required %>
    <h4>You should perform exactly <%= assignment.num_reviews_allowed %> reviews </h4>
  <% else %>
    <h4>You may perform between <%= assignment.num_reviews_required %> and <%= assignment.num_reviews_allowed %> reviews</h4>
  <% end %>
  <h4>You are required to do <%= assignment.num_reviews_required %> reviews</h4>
  <h4>Number of reviews left: <%= assignment.num_reviews_allowed - num_reviews_completed %></h4>
<% end %>

#This methods displays flash
<% = flash_message %>
  
# This method displays the number of reviews to be done defined in review_bids_helper
<%= display_assignment_reviews_info(@assignment, @num_reviews_completed) %>

# -- table with reviews available to complete -->
<table>
  <% review_no = 1 %>
  <% title = 'Review' %>

  <% @review_mappings.each do |map| %>
     # Create sorted_response array to store information from particular map id 
    <% @sorted_responses = [] %>
    <% team = AssignmentTeam.find(map.reviewee_id) %>
    <% participant = team.participants.first %>

    <% if participant %>
      <% topic_id = SignedUpTeam.topic_id(participant.parent_id, participant.user_id) %>
      <tr>
        <%= display_topic_and_participant(title, review_no, topic_id, assignment, participant) %>
        
        <% latest_submission = team.most_recent_submission %>
        <td> </td>

        <% if !map.response.empty? && latest_submission %>
          <% # Populate sorted response array from corresponding id and set array_not_empty flag %>
          <% @sorted_responses = Response.where(map_id: map.id).to_a %>
          <% array_not_empty = @sorted_responses.present? ? 1 : 0 %>

          <% if array_not_empty == 1 %>
            <% # the latest should be at the last %>
            <% @sorted_responses = @sorted_responses.sort_by {|obj| obj.updated_at} %>
            <% @latest_response = @sorted_responses.last %>

            # Determine the round of the latest response: retrieve it from the response if available, else use AssignmentDueDate. %>
            <% last_response_round = @latest_response.round.nil? ? AssignmentDueDate.done_in_assignment_round(@assignment.id, @latest_response) : @latest_response.round %>
            <% current_round = @assignment.number_of_current_round(topic_id) %>

            <td>
              <%= link_to "View", {:controller => 'response', :action => 'view', :id => @latest_response.id} %>
            </td>

            <% if @assignment.current_stage(topic_id) != "Finished" %>
              <td><%= render_not_finished_response_links_and_info(@assignment, topic_id, last_response_round, current_round, @latest_response, latest_submission, map) %></td>
            <% elsif @assignment.current_stage(topic_id) != "Complete" && @assignment.can_review(topic_id) && latest_submission %>
              <td><%= link_to "Begin", {:controller => 'response', :action => 'new', :id => map.id} %></td>
              <td>  </td>
            <% else %>
              <td> Work has not yet been submitted for Review <%= review_no %></td>
            <% end %>

            <% review_no += 1 %>
          <% end %> <!-- This end closes the if block for array_not_empty == 1 -->
        <% end %> <!-- This end closes the if block for !map.response.empty? && latest_submission -->
      </tr>
    <% end %> <!-- This end closes the if block for participant -->
  <% end %> <!-- This end closes the loop for @review_mappings.each -->
</table>
<br>
<BR/>
<%= link_to "Back", :controller=>'student_task', :action => 'view', :id => @participant.id %>

Action_allowed needs to use Authorization Utilities

The problems listed below are examples of the four main classes of problems that were encountered in previously implemented code and the fixes made while reimplementing the same:

• Problem 1: Much of the authorization logic is repeated (un-DRY). For example, multiple controllers contain the following exact code.

  ['Super-Administrator',
  'Administrator',
  'Instructor',
  'Teaching Assistant'].include? current_role_name

• Solution 1: Use one of the helper methods from the new authorization_helper.rb (link) to allow TAs *and above* (instructors, admins, super-admins) to perform this work.

  current_user_has_ta_privileges?

• Problem 2: Some logic is slightly incorrect. For example, some places call for a specific user type, when users "above" this type should also be allowed to perform the work. In the following example (advertise_for_partner_controller.rb), only Students may advertise for partners. However per Dr. Gehringer, "There are no cases I am aware of where a particular type of user can do something that more-privileged users cannot do".

  current_user.role.name.eql?("Student")

• Solution 2: Use one of the helper methods from the new authorization_helper.rb (link) to allow Students *and above* (TAs, instructors, admins, super-admins) to perform this work.

  current_user_has_student_privileges?

• However, in case there IS a need to know if the current user has one specific role, this is still supported by the helper method current_user_is_a?

• Problem 3: Too much authorization logic is present in the controllers. This makes the controllers more difficult to read, and scatters authorization logic, when it would be easier to understand if it were all in one place.

  def action_allowed?
    if %w[edit update list_submissions].include? params[:action]
      assignment = Assignment.find(params[:id])
      (%w[Super-Administrator Administrator].include? current_role_name) ||
      (assignment.instructor_id == current_user.try(:id)) ||
      TaMapping.exists?(ta_id: current_user.try(:id), course_id: assignment.course_id) ||
      (assignment.course_id && Course.find(assignment.course_id).instructor_id == current_user.try(:id))
    else
      ['Super-Administrator',
       'Administrator',
       'Instructor',
       'Teaching Assistant'].include? current_role_name
    end
  end

• Solution 3: Establish helper methods in the new authorization_helper.rb (link) to centralize as much authorization logic as possible. In this way, a developer with questions about authorization knows just where to look to find answers - authorization_helper.rb (link).

  def action_allowed?
    if %w[edit update list_submissions].include? params[:action]
      current_user_has_admin_privileges? || current_user_teaching_staff_of_assignment?(params[:id])
    else
      current_user_has_ta_privileges?
    end
  end

• Problem 4: Some action_allowed? methods are difficult to follow, and/or knowledge about how the action parameter should affect authorization is buried in another method.

  def action_allowed?
    current_user_has_student_privileges?and
    ((%w[edit].include? action_name) ? are_needed_authorizations_present?(params[:id], "reader", "reviewer") : true) and
    one_team_can_submit_work?
  end

• Solution 4: Clean up action_allowed? methods and make the influence of the action parameter visible at this level.

  def action_allowed?
    case params[:action]
    when 'edit'
      current_user_has_student_privileges? &&
      are_needed_authorizations_present?(params[:id], "reader", "reviewer")
    when 'submit_file', 'submit_hyperlink'
      current_user_has_student_privileges? &&
      one_team_can_submit_work?
    else
      current_user_has_student_privileges?
    end
  end

The Run_bid algorithm should be assign_reviewers

A bidding algorithms implemented to assign the groups their first choice of bidding. As this is an NP-complete problem, the algorithm has to use heuristics to guess efficiently. There has to be a way for instructors to easily view and compare the results of the bidding algorithms to get a quantitative sense of their effectiveness.

Refactored class methods to instance methods in Review_bid.rb

The previously implemented project has class methods. We have refactored this to instance method.

class ReviewBid < ApplicationRecord
  belongs_to :topic, class_name: 'SignUpTopic'
  belongs_to :participant, class_name: 'Participant'
  belongs_to :assignment, class_name: 'Assignment'

  # method to get bidding data
  # returns the bidding data needed for the assigning algorithm
  # student_ids, topic_ids, student_preferences, topic_preferences, max reviews allowed

  def bidding_data(assignment_id, reviewer_ids)
    # create basic hash and set basic hash data
    bidding_data = { 'tid' => [], 'users' => {}, 'max_accepted_proposals' => [] }
    bidding_data['tid'] = SignUpTopic.where(assignment_id: assignment_id).ids
    bidding_data['max_accepted_proposals'] = Assignment.where(id: assignment_id).pluck(:num_reviews_allowed).first

    # loop through reviewer_ids to get reviewer specific bidding data
    reviewer_ids.each do |reviewer_id|
      bidding_data['users'][reviewer_id] = reviewer_bidding_data(reviewer_id, assignment_id)
    end
    bidding_data
  end

  # assigns topics to reviews as matched by the webservice algorithm
  def assign_review_topics(assignment_id, reviewer_ids, matched_topics, _min_num_reviews = 2)
    # if review response map already created, delete it
    if ReviewResponseMap.where(reviewed_object_id: assignment_id)
      ReviewResponseMap.where(reviewed_object_id: assignment_id).destroy_all
    end
    # loop through reviewer_ids to assign reviews to each reviewer
    reviewer_ids.each do |reviewer_id|
      topics_to_assign = matched_topics[reviewer_id.to_s]
      topics_to_assign.each do |topic|
        assign_topic_to_reviewer(assignment_id, reviewer_id, topic)
      end
    end
  end

  # method to assign a single topic to a reviewer
  def assign_topic_to_reviewer(assignment_id, reviewer_id, topic)
    team_to_review = SignedUpTeam.where(topic_id: topic).pluck(:team_id).first
    team_to_review.nil? ? [] : ReviewResponseMap.create(reviewed_object_id: assignment_id, reviewer_id: reviewer_id, reviewee_id: team_to_review, type: 'ReviewResponseMap')
  end

  # method for getting individual reviewer_ids bidding data
  # returns user's bidding data hash
  def reviewer_bidding_data(reviewer_id, assignment_id)
    reviewer_user_id = AssignmentParticipant.find(reviewer_id).user_id
    self_topic = SignedUpTeam.topic_id(assignment_id, reviewer_user_id)
    bidding_data = { 'tid' => [], 'otid' => self_topic, 'priority' => [], 'time' => [] }
    bids = ReviewBid.where(participant_id: reviewer_id)

    # loop through each bid for a topic to get specific data
    bids.each do |bid|
      bidding_data['tid'] << bid.signuptopic_id
      bidding_data['priority'] << bid.priority
      bidding_data['time'] << bid.updated_at
    end
    bidding_data
  end
end

Future Plan of Action

Test Plan

• 1. Manual Testing===
  • UI testing of the implemented functionality to be done.
  • No UI has been created been created for the conference paper review bidding. Hence assignment bidding controller is used to manually test the code on a local host to determine the feasibility of code.

• 1. Log in as an instructor
  • • • Go to an Assignment
    • Go to topics section, add few topics for the assignment. Later check the enable bidding checkbox button.
    • Go to add participants page, add few student to the assignment.
    • Now save the assignment.
    • Login as student (who is a participant of the assignment)
    • Go to the assignment page, then go to sign up sheet and bid for topics.
    • Do the above the step for the participants.
  • Login back to an instructor, go to manage assignments page, click on the run intelligent assignment button.
  • Go to the topics tab in the assignment, here you will be able to view all the team's assignments to their topics.

Automated Test Cases

• TDD and Feature Test cases to be written. For instance, we will do Rspec test in cases below:
  • When all the reviewers submit their preferences.
  • When there are no preferences from any user.
  • When few have their preferences and few don't.

Edge cases

• Case 1: No reviewer submits a list of preferred topics to review : In this case all topics are assigned same bid score. Topics are assigned to users on the basis of "balanced assignment", first come first serve.

• Case 2: All reviewers submit exactly the same list of topics to review: It will be first come, first serve.

• Case 3: Number of topics exceed the number of teams: Some topics may not be reviewed,depending on the limits put by the conference. It is responsibility of administrator to add more teams in such a situation