CSC/ECE 517 Fall 2023 - E2375. Reimplement Waitlists

Introduction

Expertiza is a Ruby on Rails-based open-source project, collaboratively developed and maintained by both students and faculty at NC State. This web application empowers instructors with comprehensive control over their class assignments. With a rich set of features including the ability to add topics, establish groups, and facilitate peer reviews, Expertiza is a robust and versatile platform capable of handling a wide range of assignment types. To explore the extensive functionality provided by Expertiza, you can delve into its wiki.

Topic Overview & Prior Work

Feature Overview

E2281 contains detailed information on the previous team's work with this feature.

A summary of the desired functionality is presented below:

Any instructor or TA can sign students up for a topic. Students are able to sign themselves up for a topic. If the topic is full then the team will be put on a queue or "waitlist" for the topic. If the current team holding that topic drops the topic, then the team at the top of the waitlist will be assigned the topic.

Issues we found in Current Functionality

• When a team is deleted, and if that team currently holds a particular topic, that team is also being deleted. However, the topic is not being assigned to the next team that is at the top of the waitlist queue.

Approach Followed By Previous Team

• The previous team have created a new class for Waitlists that inherits from Ruby's Queue class. Within that class there is a model, and controller that will house all of the functionality relating to Waitlists.

Comments on why E2240, was not merged

1. The code for the waitlist is distributed all over the application, which is something could have been placed into a single model and called through other files
2. Although manual testing looked solid, there were not enough automated test cases to back their functionality.
3. Too many class methods. A design with multiple class methods is not taking advantage of object orientation, which says that all actions should be structured as operations on a specific receiver.
4. The code does not follow Ruby style design.
5. Lack of descriptive code comments

Files Modified During the implementation

• sign_up_sheet_controller.rb
• teams_controller.rb
• sign_up_topic.rb
• signed_up_team.rb

Changes

sign_up_sheet_controller.rb

delete_signup Method:

In the original code, it directly deletes a signup for a topic. In the refactored code, it finds the user's team using SignedUpTeam.find_team_users and calls delete_signup_for_topic with the topic ID and team ID, enhancing code modularity.

delete_signup_as_instructor:

The refactored code simplifies arguments passed to delete_signup_for_topic, directly using the topic ID and team ID instead of assignment ID and user ID, improving code clarity and reuse.

delete_signup_for_topic Method:

A new method in the refactored code, it reassigns a topic to a different team if a signup needs to be deleted, enhancing code maintainability and separation of responsibilities.

Previous Code

   def delete_signup
    participant = AssignmentParticipant.find(params[:id])
    assignment = participant.assignment
    drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
    # A student who has already submitted work should not be allowed to drop his/her topic!
    # (A student/team has submitted if participant directory_num is non-null or submitted_hyperlinks is non-null.)
    # If there is no drop topic deadline, student can drop topic at any time (if all the submissions are deleted)
    # If there is a drop topic deadline, student cannot drop topic after this deadline.
    if !participant.team.submitted_files.empty? || !participant.team.hyperlinks.empty?
      flash[:error] = 'You have already submitted your work, so you are not allowed to drop your topic.'
      ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping topic for already submitted a work: ' + params[:topic_id].to_s)
    elsif !drop_topic_deadline.nil? && (Time.now > drop_topic_deadline.due_at)
      flash[:error] = 'You cannot drop your topic after the drop topic deadline!'
      ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping topic for ended work: ' + params[:topic_id].to_s)
    else
      delete_signup_for_topic(assignment.id, params[:topic_id], session[:user].id)
      flash[:success] = 'You have successfully dropped your topic!'
      ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].id, 'Student has dropped the topic: ' + params[:topic_id].to_s)
    end
    redirect_to action: 'list', id: params[:id]
  end 

def delete_signup_as_instructor
    # find participant using assignment using team and topic ids
    team = Team.find(params[:id])
    assignment = Assignment.find(team.parent_id)
    user = TeamsUser.find_by(team_id: team.id).user
    participant = AssignmentParticipant.find_by(user_id: user.id, parent_id: assignment.id)
    drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
    if !participant.team.submitted_files.empty? || !participant.team.hyperlinks.empty?
      flash[:error] = 'The student has already submitted their work, so you are not allowed to remove them.'
      ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Drop failed for already submitted work: ' + params[:topic_id].to_s)
    elsif !drop_topic_deadline.nil? && (Time.now > drop_topic_deadline.due_at)
      flash[:error] = 'You cannot drop a student after the drop topic deadline!'
      ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Drop failed for ended work: ' + params[:topic_id].to_s)
    else
      delete_signup_for_topic(assignment.id, params[:topic_id], participant.user_id)
      flash[:success] = 'You have successfully dropped the student from the topic!'
      ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Student has been dropped from the topic: ' + params[:topic_id].to_s)
    end
    redirect_to controller: 'assignments', action: 'edit', id: assignment.id
  end

  def delete_signup_for_topic(assignment_id, topic_id, user_id)
    SignUpTopic.reassign_topic(user_id, assignment_id, topic_id)
  end 

After Refactoring

def delete_signup
    participant = AssignmentParticipant.find(params[:id])
    assignment = participant.assignment
    drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
    # A student who has already submitted work should not be allowed to drop his/her topic!
    # (A student/team has submitted if participant directory_num is non-null or submitted_hyperlinks is non-null.)
    # If there is no drop topic deadline, student can drop topic at any time (if all the submissions are deleted)
    # If there is a drop topic deadline, student cannot drop topic after this deadline.
    if !participant.team.submitted_files.empty? || !participant.team.hyperlinks.empty?
      flash[:error] = 'You have already submitted your work, so you are not allowed to drop your topic.'
      ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping topic for already submitted a work: ' + params[:topic_id].to_s)
    elsif !drop_topic_deadline.nil? && (Time.now > drop_topic_deadline.due_at)
      flash[:error] = 'You cannot drop your topic after the drop topic deadline!'
      ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping topic for ended work: ' + params[:topic_id].to_s)
    else
      users_team = SignedUpTeam.find_team_users(assignment.id, session[:user].id)
      delete_signup_for_topic(params[:topic_id], users_team[0].t_id)
      flash[:success] = 'You have successfully dropped your topic!'
      ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].id, 'Student has dropped the topic: ' + params[:topic_id].to_s)
    end
    redirect_to action: 'list', id: params[:id]
  end 

def delete_signup_as_instructor
    # find participant using assignment using team and topic ids
    team = Team.find(params[:id])
    assignment = Assignment.find(team.parent_id)
    user = TeamsUser.find_by(team_id: team.id).user
    participant = AssignmentParticipant.find_by(user_id: user.id, parent_id: assignment.id)
    drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
    if !participant.team.submitted_files.empty? || !participant.team.hyperlinks.empty?
      flash[:error] = 'The student has already submitted their work, so you are not allowed to remove them.'
      ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Drop failed for already submitted work: ' + params[:topic_id].to_s)
    elsif !drop_topic_deadline.nil? && (Time.now > drop_topic_deadline.due_at)
      flash[:error] = 'You cannot drop a student after the drop topic deadline!'
      ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Drop failed for ended work: ' + params[:topic_id].to_s)
    else
      delete_signup_for_topic(params[:topic_id], team.id)
      flash[:success] = 'You have successfully dropped the student from the topic!'
      ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Student has been dropped from the topic: ' + params[:topic_id].to_s)
    end
    redirect_to controller: 'assignments', action: 'edit', id: assignment.id
  end 

def delete_signup_for_topic(topic_id, team_id)
    # Delete a signup record for a specific topic and team.

    # Find the SignUpTopic record with the specified topic_id using the `find_by` method.
    @sign_up_topic = SignUpTopic.find_by(id: topic_id)
    # Check if the @sign_up_topic record exists (is not nil).
    unless @sign_up_topic.nil?
       # If the @sign_up_topic record exists, reassign the topic for the specified team by calling the `reassign_topic` 
       # instance method of SignUpTopic.
      @sign_up_topic.reassign_topic(team_id)
    end 
  end 

teams_controller.rb

Original Code:

In the original code, it checks if there's exactly one SignedUpTeam record associated with the team, and if it's not waitlisted, it performs a specific topic reassignment logic.

Refactored Code:

In the refactored code, it adds more clarity to this logic by introducing the @signed_topic variable to represent the SignUpTopic associated with the single SignedUpTeam record. If such a record exists, it calls the reassign_topic instance method on @signed_topic to reassign the topic to another team.

Additionally, it checks if there are multiple waitlisted teams for the specified team ID using SignedUpTeam.drop_off_waitlists(params[:id]). This is a new feature in the refactored code, which allows for the removal of all waitlisted teams for the specified team ID, making it easier to manage waitlists.

Previous Code

 def delete
    # delete records in team, teams_users, signed_up_teams table
    @team = Team.find_by(id: params[:id])
    unless @team.nil?
      @signed_up_team = SignedUpTeam.where(team_id: @team.id)
      @teams_users = TeamsUser.where(team_id: @team.id)

      if @signed_up_team == 1 && !@signUps.first.is_waitlisted # if a topic is assigned to this team
        # if there is another team in waitlist, assign this topic to the new team
        topic_id = @signed_up_team.first.topic_id
        next_wait_listed_team = SignedUpTeam.where(topic_id: topic_id, is_waitlisted: true).first
        # Save the topic's new assigned team and delete all waitlists for this team
        SignUpTopic.assign_to_first_waiting_team(next_wait_listed_team) if next_wait_listed_team
      end

      @sign_up_team.destroy_all if @sign_up_team
      @teams_users.destroy_all if @teams_users
      @team.destroy if @team
      undo_link("The team \"#{@team.name}\" has been successfully deleted.")
    end
    redirect_back fallback_location: root_path
  end

After Refactoring

# Deleting a specific team associated with a given parent object
  def delete
    # delete records in team, teams_users, signed_up_teams table
    @team = Team.find_by(id: params[:id])
    unless @team.nil?
      # Find all SignedUpTeam records associated with the found team.
      @signed_up_team = SignedUpTeam.where(team_id: @team.id)
      # Find all TeamsUser records associated with the found team.
      @teams_users = TeamsUser.where(team_id: @team.id)
      # Check if there are SignedUpTeam records associated with the found team.
      unless @signed_up_team.nil?
        # If a topic is assigned to this team and there is only one signed up team record, and it's not waitlisted.
        if @signed_up_team.count == 1 && !@signed_up_team.first.is_waitlisted  # if a topic is assigned to this team
            # Fetch the SignUpTopic object associated with the single signed up team.
            @signed_topic = SignUpTopic.find_by(id: @signed_up_team.first.topic_id)
            unless @signed_topic.nil?
              # Call the instance method `reassign_topic` of SignUpTopic to reassign the topic.
              @signed_topic.reassign_topic(@signed_up_team.first.team_id)
            end
        else
          # Drop all waitlists in SignedUpTeam for the specified team ID.
          SignedUpTeam.drop_off_waitlists(params[:id])
        end
      end
     # @sign_up_team.destroy_all if @sign_up_team
      @teams_users.destroy_all if @teams_users
      @team.destroy if @team
      undo_link("The team \"#{@team.name}\" has been successfully deleted.")
    end
    redirect_back fallback_location: root_path
  end

sign_up_topic.rb

In the reimplementation, two significant changes have been made:

Introduction of the longest_waiting_team method:

This new method abstracts the logic for finding the team that has been waiting the longest for a specific topic by querying the SignedUpTeam table. It enhances code modularity and readability.

Refactoring of the reassign_topic method:

This method now accepts a team_id as an argument, focusing on reassigning topics for a specific team. It starts by fetching the topic ID and signup record. It checks if the signup record is not marked as waitlisted before proceeding. If the condition is met, it determines the longest waiting team for the same topic using the longest_waiting_team method. If found, the method assigns the topic to this team and clears the waitlist. Finally, it removes the entry for the team that was previously assigned or waiting. These changes improve code clarity, modularity, and maintainability.

Previous code

 def self.reassign_topic(session_user_id, assignment_id, topic_id)
    # find whether assignment is team assignment
    assignment = Assignment.find(assignment_id)

    # making sure that the drop date deadline hasn't passed
    dropDate = AssignmentDueDate.where(parent_id: assignment.id, deadline_type_id: '6').first
    if dropDate.nil? || dropDate.due_at >= Time.now
      # if team assignment find the creator id from teamusers table and teams
      # ACS Removed the if condition (and corresponding else) which differentiate assignments as team and individual assignments
      # to treat all assignments as team assignments
      # users_team will contain the team id of the team to which the user belongs
      users_team = SignedUpTeam.find_team_users(assignment_id, session_user_id)
      signup_record = SignedUpTeam.where(topic_id: topic_id, team_id:  users_team[0].t_id).first
      assignment = Assignment.find(assignment_id)
      # if a confirmed slot is deleted then push the first waiting list member to confirmed slot if someone is on the waitlist
      unless assignment.is_intelligent?
        unless signup_record.try(:is_waitlisted)
          # find the first wait listed user if exists
          first_waitlisted_user = SignedUpTeam.where(topic_id: topic_id, is_waitlisted: true).first

          unless first_waitlisted_user.nil?
            # As this user is going to be allocated a confirmed topic, all of his waitlisted topic signups should be purged
            ### Bad policy!  Should be changed! (once users are allowed to specify waitlist priorities) -efg
            first_waitlisted_user.is_waitlisted = false
            first_waitlisted_user.save

            # ACS Removed the if condition (and corresponding else) which differentiate assignments as team and individual assignments
            # to treat all assignments as team assignments
            Waitlist.cancel_all_waitlists(first_waitlisted_user.team_id, assignment_id)
          end
        end
      end
      signup_record.destroy unless signup_record.nil?
      ExpertizaLogger.info LoggerMessage.new('SignUpTopic', session_user_id, "Topic dropped: #{topic_id}")
    else
      # flash[:error] = "You cannot drop this topic because the drop deadline has passed."
    end # end condition for 'drop deadline' check
  end

After Reimplementation

 def longest_waiting_team(topic_id)
    # Find and return the team that has been waiting the longest for the specified topic.

    # Find the first waitlisted user (team) for the given topic by querying the SignedUpTeam table.
    # Check for records where the topic_id matches the specified topic_id and is_waitlisted is true.
    first_waitlisted_user = SignedUpTeam.where(topic_id: topic_id, is_waitlisted: true).first   
    # If a waitlisted user (team) is found, return it as the team that has been waiting the longest.
    unless first_waitlisted_user.nil?
      return first_waitlisted_user
    end 
    # If no waitlisted user is found, return nil to indicate that there are no teams waiting.
    return nil
  end

  def reassign_topic(team_id)
    # Reassigns topic when a team is dropped from a topic.
    # Get the topic ID for reassignment.
    topic_id = self.id
    # Fetch the record in SignedUpTeam where topic_id matches the topic that needs reassignment
    # and team_id matches the provided team_id. Retrieve the first matching record.
    signup_record = SignedUpTeam.where(topic_id: topic_id, team_id:  team_id).first
    # If the signup record is not marked as waitlisted (is_waitlisted is either false or nil),
    # proceed with reassignment.
    unless signup_record.try(:is_waitlisted)
      # Find the longest waiting team for the same topic.
      longest_waiting_team  = longest_waiting_team(topic_id)
      # If a longest waiting team is found, proceed with reassignment.
      unless longest_waiting_team.nil?
        # Assign the topic to the longest waiting team.
        # Update the is_waitlisted boolean to false for the longest waiting team.
        longest_waiting_team.is_waitlisted = false
        longest_waiting_team.save
        # Drop all waitlisted records for that team.
        SignedUpTeam.drop_off_waitlists(longest_waiting_team.team_id)
      end
    end
    # Delete the entry for the team that was either previously assigned or waiting.
    SignedUpTeam.drop_off_signup_record(topic_id, team_id)
  end

signed_up_team.rb

The provided code includes two class methods for managing records in the SignedUpTeam model:

drop_off_signup_record(topic_id, team_id):

This method removes a specific record in the signed_up_teams table associated with a given topic and team. It locates the record by matching the provided topic_id and team_id, and if found, it is deleted.

drop_off_waitlists(team_id):

This method is designed to remove all waitlisted records in the signed_up_teams table linked to a specific team. It queries the database for records where the team_id matches the provided value and is_waitlisted is set to true, and then proceeds to delete all these records.

# Remove a specific signed_up_teams record for a given topic and team.
  def self.drop_off_signup_record(topic_id,team_id)
    # Fetching record for a given topic and team.
    signup_record = SignedUpTeam.find_by(topic_id: topic_id, team_id: team_id)
    # If the signup_record in not nil destroy it.
    signup_record.destroy unless signup_record.nil?
  end

  # Remove all waitlisted records in signed_up_teams associated with a specific team.
  def self.drop_off_waitlists(team_id)
    # Fetch all records that matches the given team_id with is_waitlisted as true
    # and destroy all the records.
    SignedUpTeam.where(team_id: team_id, is_waitlisted: true).destroy_all
  end

end