CSC/ECE 517 Fall 2021 - E2151. Allow reviewers to bid on what to review

Topic Overview

Statement of Problem

Assigning reviews to users is a complicated process. Currently, reviews are still assigned using a first-come-first-served basis. However, reviews could be bid on like how topics are bid on. This would involve matching multiple students to review a submission up to the maximum reviewers for a submission. A similar feature is present in bidding for topics.

The goal of this topic is to allow both reviews and topics to be bid on by students following a bidding algorithm, and for both of these features to come from the same root functionality. This will involve refactoring the current bidding functionality of expertiza, as well as incorporating new code to allow this functionality to also be utilized for reviews.

Prior Work

Before starting, it is vital to note the substantial amount of prior work done on this task. The prior work done in Fall of 2020 (linked here [[1]]) achieved much of the desired functionality for this task, and contains a comprehensive explanation of the underlying design principles behind the algorithm implemented. However, the implementation was rejected due to DRY violations. Thus, the primary goal of this project topic was to rebuild and refactor this prior work to make it consistent with DRY principles and able to be merged into the expertiza environment.

A brief review will be given of the desired functionality and the underlying design for the bidding algorithm. For a more exhaustive explanation, please see the aforementioned link to the prior year's work.

Feature

In the current state of implementation the reviews are still assigned using a first-come-first-served basis. Current state of the feature implementation contains bad code smell and needs improvement to resolve merge conflict and follow DRY principle in order to finalize the implementation. With the current state in mind, the new implementation will enable the capability that reviews could be bid on like how topics are bid on. Behind the scene, the process would involve matching multiple students to review a submission up to the maximum reviewers for a submission.

Use Case Diagram

Edge Case Consideration

For the review workflow that consists of multiple review, each of the review could be bidded. Since we have had considered the case based on initial feedback, our current solution for such case is that the user shall indicate the number of reviews that a user want to review and algorithm will take the inputs into account. As for our implementation, we will focus more on the integration of the review bidding feature with the current system.

Explanation of Feature

Existing Bidding Functionality for Topics

Currently in expertiza, students may bid on which topics they would like to work on. An image showcasing this functionality is given below:

Students may choose a number of topics to bid on, in order of priority, and once the deadline is reached, all students are given a topic.

Behind the scenes, this works by utilizing a modified version of the Gale-Shapely Algorithm. This algorithm seeks to find a 'stable' matching of each student to each topic, such that in the end, all students have a topic they wanted (at least to some degree), and that the preferences of students is respected as well as possible. To help break situations where there are ties in preferences, each topic assigns a Priority to each student. This Priority is determined by A. whether the student is part of a group bidding for this topic (groups getting higher priority), and B. the time at which the bid for the topic was made (earlier bidders are given Priority). Once this has been determined and the deadline for bidding has ended, the following steps occur:

1. Each student is matched with each topic they most prefer.
2. If the number of students assigned to a topic is above the maximum allowed for that topic, the students with the highest Priority are assigned the topic
3. All maximally filled topics are removed from the pool of available topics
4. All unmatched students begin this cycle again, now choosing the most preferred topic remaining
5. Repeat until all students are assigned a topic
6. If any matches are 'unstable' (two students would have their preferences better met if they were swapped), perform any swaps until matches are as stable as possible.

This algorithm has a time complexity of O(n^2). For more details on this algorithm and its implementation, refer to the previous year's work, where this algorithm was implemented.

Desired Functionality for Reviews

The desired feature for reviews should have an almost identical structure to this, with reviews instead of topics. Students should be shown a list of the topics that were worked on, and may bid on which of those topics they would like to review the most. The Priority system could be handled in the same way, with the only caveat being that a student should not be able to review their own work.

Because of this similarity in functionality, our goal is to make both reviews and topics use the same underlying feature mechanisms.

Planned Changes

General Design Goals

• Update the prior work for this topic to be in line with the current state of the expertiza beta branch.
• DRY out prior implementation's controller methods by creating a single controller which handles bidding for both topics and reviews.
• DRY out prior implementation's views by implementing a view from scratch (rather than a code copy of topic bidding view).
• Create exhaustive tests for newly DRY-ed methods, including edge cases.
• Retain all functionality from prior work.

Specific Design Goals (may be expanded as project continues)

• Inspect implementation structure to refactor out duplicated code
• Review def self.reviewer_self_topic in review_bid for refacotring to address DRY violation
• Modify app/views/review_bids/_all_actions.html.erb as fitted to integrate reafactored controller modules
• Update tests to test the refactored DRYed-out methods

Initial Implementation

A considerable amount of time and effort was necessary to review, understand, and re-implement the previous implementation of the Review Bidding feature. While the feature was only created a year ago, several underlying functions and methods have since been refactored, and so needed to be changed in the implementation. Additionally, certain parts of how the implementation was designed did not seem to be properly implemented, and so had to be revised.

Code Transfer

First, all files that were added in the original pull request were copied and moved to the new version of the expertiza beta branch. This includes the following files:

• controllers/review_bids_controller.rb: the primary controller that handles calls to review bids functionality from views, managing them itself or directing them to the appropriate model
• helpers/review_bids_helper.rb: contains helper methods for rendering the rows of topics available to choose a review for
• models/review_bid.rb: contains the methods that each individual review bid knows and manages, including assigning a review to a student, getting info on the topic it applies to, etc.
• views/review_bids/_all_actions.html.erb: a view partial used to render the actions available to a student on the review bidding view. Nearly identical to the _all_actions file for topic bidding.
• views/review_bids/_table_header.html.erb: a view partial used to render the head of the table for review bids. Though structurally similar to that of topics reviews, contains significantly less information, as less needs to be known by a student for particular topics to review.
• views/review_bids/_table_line.html.erb: a view partial used to render individual rows of the review bids table. Like the table header, is structurally similar to that of topic bidding, but functions differently.
• views/review_bids/show.html.erb: the view which displays all the available topics to bid on reviewing. Uses the aforementioned partials to render this bidding table.
• views/review_bids/others_work.html.erb: displays the available review work which has the user has bid for.
• db/migrate/20201107203204_create_review_bids.rb: the migration which creates the review bid objects within the expertiza database.
• spec/controllers/review_bids_controller_spec.rb: the unit tests for the review bids controller
• spec/models/review_bid_spec.rb: the unit tests for the review bids model
• test/controllers/review_bids_controller_test.rb: a more elaborate feature test for the review bids functionality
• test/fixtures/review_bids.yml: fixtures that are utilized for the review_bids_controller_test
• test/models/review_bid_test.rb: A more elaborate feature test of the review bid model.

In addition to adding the previous files, the following files were also added to existing pages as modifications:

• controllers/student_review_controller.rb: the student review controller's "list" method now redirects to the review_bid_controller's "index" method if the assignment in question has a review_choosing_algorithm set to "Bidding" (the option an instructor sets when they wish to allow students to bid on reviews).
• student_task/view.html.erb: Instead of simply linking directly to the student_review controller's "list" method, the view instead checks if the assignment this view pertains to has their review_choosing_algorithm set to Bidding. If it does, and the assignment allows students to choose the topic to review (meaning the algorithm to assign bids has not yet been run), they are redirected to the 'show' method of the review_bids_controller. Otherwise, they are redirected to the 'index' method (showing them the info on their currently available topics to review).
• config/webservices.yml: added a new url for the website that is called to run the review bidding algorithm
• config/routes.rb: routes added for review_bids.
• views/assignments/edit/_topics.html.erb: Added a new option box which allows an instructor to choose to allow bidding to occur using the review bidding algorithm, rather than the default 'Simple Choose.' Once this option is selected, it also offers a button that, when clicked, runs the review bidding algorithm on all currently bid upon reviews.

Website Reimplementation

In the previous implementation, the algorithm which handles the assigning of reviews was hosted on an external website. This was done for the sake of improved performance. Thus, to get the feature in working condition, we pulled the code necessary to run the algorithm and hosted it on our own website (linked at the end of this documentation). We also made a note in the pull request that the place where this website is referenced (in the "assign_reviews_algorithm" method of the review_bids_controller) should be replaced by a proper external web tool, as is done for similar web tools in other parts of expertiza.

Modifications to Methods

Certain methods utilized in the original version of the webpages were changed or renamed in later versions, and so, needed to be modified. The primary example of this is the method that assignments use to determine what stage of assignment completion they are in. The previous year's implementation called this method as "assignment.find_current_stage(topic_id)," but this failed when trying to execute. After comparing how this page worked to how normal review pages work, we determined the new call to this method is "assignment.current_stage(topic_id)." All calls to this method were replaced in all files.

Errors Corrected

A notable period of time was also spent on determining the purpose of a particular local area that caused issues when trying to view the bids, that being a variable named "@number_of_reviews_to_show". This variable apparently kept track of how many reviews a student needed to be shown, so that they could request another. The issue was that this was created as a local variable within the review_bids_controller, and therefore would never work outside of a manual testing environment, as the variable would need to be different from every student and every assignment. After some investigation, we realized that all this variable did was limit the number of already-assigned reviews the student was able to see (as the review bidding algorithm already assigned all possible reviews). Thus, rather than try to recreate this functionality, we simply removed it.

This means that, when the review bidding algorithm runs, a student is able to see all of the reviews assigned to them, up to the maximum number of reviews for the assignment, and therefore have no reason to ask for any more. It may be the case that this feature could be improved by adding a possibility for students to request any reviews not assigned to any other student, or that still have available slots, using the "normal" review assignment scheme. For the time being, we did not consider this part of the baseline necessary functionality, and so did not implement it.

Expected File Change

Since the task is based on previous attempted implementational changes, lottery_controller.rb, review_bids_controller.rb, student_review_controller.rb, review_bids_helper.rb, review_bid.rb, show.html.erb, _topics.html.erb will be changed and used or added for implementation.

Refinement Implementation

DRY Principle and Code Duplication Refactoring

Per comments and feedbacks on the previous implementation E2085, there were 3 sections of the code that potentially were DRY violations. For each of those violations, we have had made appropriate and provided the justification on the results.

review_bids_controller.rb

For the refactoring of review bids controller.eb, the original issue is the DRY principle violation on the internal logic of finding the phase of assignments. The original implementation to be refactored and optimized are shown as follow.

    # Finding the current phase that we are in
    due_dates = AssignmentDueDate.where(parent_id: @assignment.id)
    @very_last_due_date = AssignmentDueDate.where(parent_id: @assignment.id).order("due_at DESC").limit(1)
    next_due_date = @very_last_due_date[0]
    for due_date in due_dates
      if due_date.due_at > Time.now
        next_due_date = due_date if due_date.due_at < next_due_date.due_at
      end
    end
    @review_phase = next_due_date.deadline_type_id

Through some discovery process, we have found some related logics that is already implemented from the system which enable the refactoring and reuse of the method. Some code snippets are demonstrated as follow.

  def self.current_due_date(due_dates)
    #Get the current due date from list of due dates
    due_dates.each do |due_date|
      if due_date.due_at > Time.now
        current_due_date = due_date
        return current_due_date
      end
    end
    #in case current due date not found
    return nil 
  end

  def self.teammate_review_allowed(student)
    # time when teammate review is allowed
    due_date = current_due_date(student.assignment.due_dates)
    student.assignment.find_current_stage == 'Finished' ||
    due_date &&
    (due_date.teammate_review_allowed_id == 3 ||
    due_date.teammate_review_allowed_id == 2) # late(2) or yes(3)
  end

As we can observed above, both methods of current_due_date and teammate_review_allowed provides the capability of getting the review_phase without the repetition. Additionally, we can see that those existing implementation still have some difference in implementation and purpose to be fully reusable by the logic that find current phase of a topic because the methods provided are helper methods for broader abstraction.

Furthermore, the section that we attempted to refactoring out have a similar logic that is used to get the current stage as shown below.

  def find_current_stage(topic_id = nil)
    next_due_date = DueDate.get_next_due_date(self.id, topic_id)
    return 'Finished' if next_due_date.nil?
    next_due_date
  end

However, it is worth pointing out that the method is not fully substitutable, particularly the internal functionality that finds the @very_last_due_date. A similar logic does exist that is meant to use to acquire current due date and assist with finding current stage.

  def current_stage_name(topic_id = nil)
    if self.staggered_deadline?
      return (topic_id.nil? ? 'Unknown' : current_stage(topic_id))
    end

    due_date = find_current_stage(topic_id)
    unless due_date == 'Finished' || due_date.nil? || due_date.deadline_name.nil?
      return due_date.deadline_name
    end
    current_stage(topic_id)
  end

With all the findings, we have come to conclusion that the portion of the code to be dry is in similar, but required full restructure, form of existing methods. The internal logic flow itself is not fully matching the reusable methods consists across the controllers. Because of the clarity of limited reusability of existing methods, we have come to conclusion that the portions to be refactored are not duplication of any existing methods. Additionally, after a more detailed discovery, it has come to realization that the code portion of getting the current phase of the next due date was, in fact, meant to find the last due date. After further analysis of the purpose of the code, the portion has been deleted from review_bids_controller.rb because there was no usage observed with the result of the execution.

Tests Case Update

Throughout the refactoring and merging of E2085 to E2151, there were some updates that has been made to the test suits to maintain corrosponding testing automation.

Test Plan

In the design planning phase of the project, most of the test plan will be focusing on manual testing that is highly concise for lightweight validation.

Once the design feature is further into develop, this page will be updated with specific code regarding the RSpec test cases desired for this feature. Until then, the following are outlined below:

Bidding Controller/View Tests

All previously created tests for the bidding functionality of topics must still pass.

Review Controller/View Tests

Unit Test Cases:

• A review bidding instance can be created by an instructor
• A review bidding instance cannot be created by a student
• A student can assign their preferences to a review bidding instance
• When the deadline for bidding passes, students are given their reviews
• (Edge Case) A Student cannot be choose to, nor be assigned to, review their own project.
• (Edge Case) When a student does not bid on a review when the deadline passes, they are assigned one randomly.
• (Edge Case) In a sufficiently large class, if no students bid on a review, that project can still be reviewed.

Feature Test Cases:

• A professor can see the option to make an assignment's reviews have the "Bidding" style and select it
• A student can see the project names of available reviews
• A student can drag multiple reviews into their review preferences
• When multiple students wish to be given the same review, the number of students assigned to a review does not exceed the maximum allowed
• Under a specific combination of choices of preference, the algorithm assigns students to the reviews they should theoretically receive

Conclusions and Future Work

Across the process of refining the refactored front end page view along with more robust edge cases testing towards the algorithm service, we have discovered an underlying issue that could impact the availability of the review bidding algorithm service. Through cause isolation, we have observed a high correlation to flask application crash with the edge case scenario input where the algorithm service is trying to match a student who was not signed up with any topic with other students. With our scope limitation of refactoring, we will provide the following details for the reference of future team who will work on the stability improvement of the algorithm service. For further details, please refer to the information below.

"POST /match_topics HTTP/1.1" 500 -
{'tid': [5133, 5134, 5135, None], 'users': {'45514': {'otid': 5135, 'tid': [5133, 5134], 'time': ['2021-11-30T17:57:20.000-05:00', '2021-11-30T17:57:24.000-05:00'], 'priority': [2, 1]}, '45513': {'otid': 5134, 'tid': [5133, 5135], 'time': ['2021-11-30T17:57:41.000-05:00', '2021-11-30T17:57:43.000-05:00'], 'priority': [2, 1]}, '45512': {'otid': 5133, 'tid': [5134, 5135], 'time': ['2021-11-30T17:57:58.000-05:00', '2021-11-30T17:58:00.000-05:00'], 'priority': [2, 1]}, '45515': {'otid': None, 'tid': [], 'time': [0], 'priority': [3, 1, 4, 2]}}, 'max_accepted_proposals': 3}
[2021-11-28 18:10:40,576] ERROR in app: Exception on /match_topics [POST]
Traceback (most recent call last):
  File "/home/usr/.local/lib/python3.5/site-packages/flask/app.py", line 2447, in wsgi_app
    response = self.full_dispatch_request()
  File "/home/usr/.local/lib/python3.5/site-packages/flask/app.py", line 1952, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/home/usr/.local/lib/python3.5/site-packages/flask/app.py", line 1821, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/home/usr/.local/lib/python3.5/site-packages/flask/_compat.py", line 39, in reraise
    raise value
  File "/home/usr/.local/lib/python3.5/site-packages/flask/app.py", line 1950, in full_dispatch_request
    rv = self.dispatch_request()
  File "/home/usr/.local/lib/python3.5/site-packages/flask/app.py", line 1936, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/home/usr/IntelligentAssignment/app/app.py", line 19, in topic_matching
    return jsonify(assigned_topics_model.get_student_topic_matches())
  File "/home/usr/IntelligentAssignment/app/topics_matcher.py", line 57, in get_student_topic_matches
    student.accept_proposal()
  File "/home/usr/IntelligentAssignment/app/student.py", line 24, in accept_proposal
    self.proposals.sort(key=lambda proposal: self.choices.index(proposal))
  File "/home/usr/IntelligentAssignment/app/student.py", line 24, in <lambda>
    self.proposals.sort(key=lambda proposal: self.choices.index(proposal))
ValueError: None is not in list

Github

The Github corresponding to this task is publicly available here: https://github.com/WeiRui-Wang/expertiza

Contributors

This feature was created as part of Dr. Edward Gehringer's "CSC/ECE 517: Object-Oriented Design and Development" class, Fall 2021. The contributors were: Geoff Garrido, Ayush Luthra, and WeiRui Wang. Our project mentor was Luis Delossantos (lgdeloss@ncsu.edu)