CSC/ECE 517 Fall 2023 - E2375. Refactor classes relating to signing up for topics

Introduction

• When managing sign-ups within expertiza, the evolution of functionality often leads to a need for refactoring existing code. In this case, several classes— sign_up_sheet.rb, sign_up_topic.rb, and signed_up_team.rb —require restructuring to streamline their functionalities and ensure they align with the current project landscape.
• This refactoring endeavor aims to declutter, reorganize, and enhance code readability and maintainability by removing redundant functionalities and repositioning methods in classes where they serve their intended purposes more explicitly. The end goal is a more coherent and efficient codebase that reflects the current needs of the project while laying the groundwork for future enhancements.

Design in sign_up_sheet.rb file

Methods modified

self.signup_team

Existing code:

 def self.signup_team(assignment_id, user_id, topic_id = nil)
    users_team = SignedUpTeam.find_team_users(assignment_id, user_id)
    if users_team.empty?
      # if team is not yet created, create new team.
      # create Team and TeamNode
      team = AssignmentTeam.create_team_with_users(assignment_id, [user_id])
      # create SignedUpTeam
      confirmationStatus = SignUpSheet.confirmTopic(user_id, team.id, topic_id, assignment_id) if topic_id
    else
      confirmationStatus = SignUpSheet.confirmTopic(user_id, users_team[0].t_id, topic_id, assignment_id) if topic_id
    end
    ExpertizaLogger.info "The signup topic save status:#{confirmationStatus} for assignment #{assignment_id} by #{user_id}"
    confirmationStatus
  end

Changed Code

def self.signup_team(assignment_id, user_id, topic_id = nil)
    # Find the team ID for the given assignment and user
    team_id = Team.find_team_users(assignment_id, user_id)&.first&.t_id
    # If the team doesn't exist, create a new team and assign the team ID
    if team_id.nil?
      team = AssignmentTeam.create_team_with_users(assignment_id, [user_id])
      team_id = team.id
    end
    # Confirm the signup topic if a topic ID is provided
    @signup_topic = SignUpTopic.find_by(id: topic_id)
    unless @signup_topic.nil?
      confirmation_status = @signup_topic.signup_team_for_chosen_topic(team_id)
    end
    # Log the signup topic save status
    ExpertizaLogger.info "The signup topic save status:#{confirmation_status} for assignment #{assignment_id} by #{user_id}"
    confirmation_status
  end

Methods completely removed or moved to sign_up_topic.rb

self.confirmTopic

Existing Code:

 def self.confirmTopic(user_id, team_id, topic_id, assignment_id)
    # check whether user has signed up already
    user_signup = SignUpSheet.otherConfirmedTopicforUser(assignment_id, team_id)
    users_team = SignedUpTeam.find_team_users(assignment_id, user_id)
    team = Team.find(users_team.first.t_id)
    if SignedUpTeam.where(team_id: team.id, topic_id: topic_id).any?
      return false
    end

    sign_up = SignedUpTeam.new
    sign_up.topic_id = topic_id
    sign_up.team_id = team_id
    result = false
    if user_signup.empty?

      # Using a DB transaction to ensure atomic inserts
      ApplicationRecord.transaction do
        # check whether slots exist (params[:id] = topic_id) or has the user selected another topic
        team_id, topic_id = create_SignUpTeam(assignment_id, sign_up, topic_id, user_id)
        result = true if sign_up.save
      end
    else
      # This line of code checks if the "user_signup_topic" is on the waitlist. If it is not on the waitlist, then the code returns 
      # false. If it is on the waitlist, the code continues to execute.
      user_signup.each do |user_signup_topic|
        return false unless user_signup_topic.is_waitlisted
      end

      # Using a DB transaction to ensure atomic inserts
      ApplicationRecord.transaction do
        # check whether user is clicking on a topic which is not going to place him in the waitlist
        result = sign_up_wailisted(assignment_id, sign_up, team_id, topic_id)
      end
    end

    result
  end

self.create_SignUpTeam

Existing Code:

 def self.create_SignUpTeam(assignment_id, sign_up, topic_id, user_id)
    if slotAvailable?(topic_id)
      sign_up.is_waitlisted = false
      # Create new record in signed_up_teams table
      team_id = TeamsUser.team_id(assignment_id, user_id)
      topic_id = SignedUpTeam.topic_id(assignment_id, user_id)
      SignedUpTeam.create(topic_id: topic_id, team_id: team_id, is_waitlisted: 0, preference_priority_number: nil)
      ExpertizaLogger.info LoggerMessage.new('SignUpSheet', user_id, "Sign up sheet created with teamId #{team_id}")
    else
      sign_up.is_waitlisted = true
    end
    [team_id, topic_id]
  end

self.slotAvailable?

Existing Code:

def self.slotAvailable?(topic_id)
    SignUpTopic.slotAvailable?(topic_id)
end

Design in sign_up_topic.rb file

Methods moved and modified here from sign_up_sheet

signup_team_for_chosen_topic

def signup_team_for_chosen_topic(team_id)
    topic_id = self.id
    team = Team.find(team_id)
    # Fetch all topics for the user within the team for the assignment
    user_signup = SignedUpTeam.find_user_signup_topics(team.parent_id, team_id)
    # Check if the user is already signed up and waitlisted for the topic
    if !user_signup.empty?
      return false unless user_signup.first&.is_waitlisted == true
    end
    # Create a new SignedUpTeam instance with the provided topic and team details
    sign_up = SignedUpTeam.new(topic_id: topic_id, team_id: team_id)
    if SignUpTopic.slot_available?(topic_id)
      # Assign the topic to the team if a slot is available and drop off the team from all waitlists
      SignUpTopic.assign_topic_to_team(sign_up, topic_id)
      # Once assigned, drop all the waitlisted topics for this team
      result = SignedUpTeam.drop_off_waitlists(team_id)
    else
      # Save the team as waitlisted if no slots are available
      result = SignUpTopic.save_waitlist_entry(sign_up, team_id)
    end
    result
end

self.assign_topic_to_team

def self.assign_topic_to_team(sign_up, topic_id)
    # Set the team's waitlist status to false as they are assigned a topic
    sign_up.update(is_waitlisted: false)
    # Update the topic_id in the signed_up_teams table for the user
    signed_up_team = SignedUpTeam.find_by(topic_id: topic_id)
    signed_up_team.update(topic_id: topic_id) if signed_up_team
end

self.save_waitlist_entry

def self.save_waitlist_entry(sign_up, team_id)
    sign_up.is_waitlisted = true
    # Save the user's waitlist status
    result = sign_up.save
    # Log the creation of the sign-up sheet for the waitlisted user
    ExpertizaLogger.info(LoggerMessage.new('SignUpSheet', '', "Sign up sheet created for waitlisted with teamId #{team_id}"))
    result
end

Design in signed_up_team.rb file

Methods Removed

self.find_team_participants

Existing Code:

def self.find_team_participants(assignment_id, ip_address = nil)
    @participants = SignedUpTeam.joins('INNER JOIN sign_up_topics ON signed_up_teams.topic_id = sign_up_topics.id')
                                .select('signed_up_teams.id as id, sign_up_topics.id as topic_id, sign_up_topics.topic_name as name,
                                  sign_up_topics.topic_name as team_name_placeholder, sign_up_topics.topic_name as user_name_placeholder,
                                  signed_up_teams.is_waitlisted as is_waitlisted, signed_up_teams.team_id as team_id')
                                .where('sign_up_topics.assignment_id = ?', assignment_id)
    @participants.each_with_index do |participant, i|
      participant_names = User.joins('INNER JOIN teams_users ON users.id = teams_users.user_id')
                              .joins('INNER JOIN teams ON teams.id = teams_users.team_id')
                              .select('users.name as u_name, teams.name as team_name')
                              .where('teams.id = ?', participant.team_id)

      team_name_added = false
      names = '(missing team)'

      participant_names.each do |participant_name|
        if team_name_added
          names += User.find_by(name: participant_name.u_name).name(ip_address) + ' '
          participant.user_name_placeholder += User.find_by(name: participant_name.u_name).name(ip_address) + ' '
        else
          names = '[' + participant_name.team_name + '] ' + User.find_by(name: participant_name.u_name).name(ip_address) + ' '
          participant.team_name_placeholder = participant_name.team_name
          participant.user_name_placeholder = User.find_by(name: participant_name.u_name).name(ip_address) + ' '
          team_name_added = true
        end
      end
      @participants[i].name = names
    end

Testing Plan

Automated Testing (RSpec):

Unit Tests:

• Write RSpec tests for each refactored method in the respective classes.
• Test various scenarios, edge cases, and error handling using mocks and stubs where necessary.
• Test cases for different paths through conditional statements.
• Check database interactions using test databases or mock database interactions to ensure data consistency.

Integration Tests:

• Test the interaction between classes or modules after refactoring.
• Ensure that refactored methods work together as expected.
• Test scenarios involving multiple methods or classes collaborating to complete a task.

Code Coverage:

• Utilize RSpec's code coverage tools to ensure that the tests cover a significant portion of the refactored codebase.
• Aim for a high code coverage percentage to validate the effectiveness of the tests.

Regression Testing:

• Run existing test suites against the refactored code to ensure no unintended changes have occurred.
• Verify that the refactored code doesn't introduce new bugs or regressions in the system's functionalities

Conclusion

In conclusion, the outlined refactoring process presents a clear path towards substantial enhancements within the sign-up management classes of Expertiza. By addressing the identified issues—confusing method names, redundant functionalities, and violations of coding principles—we're poised to elevate the functionality and maintainability of these crucial components.

The proposed improvements, encompassing method renaming, separation of concerns, and adherence to coding conventions, signify a concerted effort to refine the codebase. These changes are not merely about tidying up the existing code; they are a strategic investment in the project's future.

Moving forward, we'll embark on a systematic approach to refactor these classes. Each step will involve precise modifications aimed at streamlining functionalities, reducing complexities, and aligning methods with their intended purposes. This iterative process will be driven by a commitment to best practices, readability, and scalability, ensuring that the codebase evolves in sync with the evolving needs of Expertiza.

Team

Mentor

• Ed Gehringer

Members

• Dinesh Pasupuleti <dpasupu@ncsu.edu>
• Prateek Singamsetty <pksingam@ncsu.edu>
• Rushabh Shah <rshah32@ncsu.edu>