CSC/ECE 517 Fall 2023 - E2375. Refactor classes relating to signing up for topics: Difference between revisions
No edit summary |
|||
(29 intermediate revisions by 2 users not shown) | |||
Line 3: | Line 3: | ||
* 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. | * 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. | ||
== | == Flowchart Illustrating Our Approach == | ||
* Below is the | * Below is the flow of our approach to signing up a user for a specific topic. | ||
[[File:E2375 FlowChart.drawio.png|400px|center]] | [[File:E2375 FlowChart.drawio.png|400px|center]] | ||
== UML Diagram == | == UML Diagram == | ||
* Below is the UML diagram of our proposed approach | * Below is the UML diagram of our proposed approach | ||
[[File: | [[File:E2375_UML_OODD_Final.png|600px|center]] | ||
'''SignUpSheet Class:''' | |||
The `SignUpSheet` class has a single method: | |||
* '''`signup_team(assignment_id, user_id, topic_id = nil)`:''' Used to sign up a team for a chosen topic. It internally communicates with the `SignUpTopic` class. | |||
'''SignUpTopic Class:''' | |||
The `SignUpTopic` class has three methods: | |||
* '''`signup_team_for_chosen_topic(team_id)`:''' Handles the process of signing up a team for a chosen topic. | |||
* '''`assign_topic_to_team(sign_up, topic_id)`:''' Assigns a topic to a team and updates the waitlist status. | |||
* '''`save_waitlist_entry(sign_up, team_id)`:''' Saves a team to the waitlist if no slots are available. It also logs the creation of the sign-up sheet for waitlisted users. This class communicates with the `SignedUpTeam` class. | |||
'''SignedUpTeam Class:''' | |||
The `SignedUpTeam` class has two methods: | |||
* '''`drop_off_waitlists(team_id)`:''' Drops a team from all waitlists. | |||
* '''`find_user_signup_topics(assignment_id, team_id)`:''' Finds all topics for a user within a team for a specific assignment. | |||
'''Associations:''' | |||
* The associations between classes represent communication or dependency. For example, `SignUpSheet` communicates with `SignUpTopic`, and `SignUpTopic` communicates with `SignedUpTeam`. | |||
Each class is represented with its methods, and the associations show the flow of communication between these classes. The "1" near the associations signifies the multiplicity or the number of instances participating in the relationship. | |||
== Design in sign_up_sheet.rb file== | == Design in sign_up_sheet.rb file== | ||
Line 41: | Line 62: | ||
</pre> | </pre> | ||
Eliminated Methods in sign_up_sheet.rb | |||
We removed the methods '''self.confirmTopic''','''self.create_SignUpTeam''' and '''self.slotAvailable?''' in '''signup_sheet.rb''' due to redundancy, lengthiness, and unnecessary repetitive checks. We reimplemented new methods for this functionality in '''sign_up_topic.rb''' focusing on eliminating redundant checks and optimizing the flow for better efficiency and readability. | We removed the methods '''self.confirmTopic''','''self.create_SignUpTeam''' and '''self.slotAvailable?''' in '''signup_sheet.rb''' due to redundancy, lengthiness, and unnecessary repetitive checks. We reimplemented new methods for this functionality in '''sign_up_topic.rb''' focusing on eliminating redundant checks and optimizing the flow for better efficiency and readability. | ||
=== self.confirmTopic === | === self.confirmTopic === | ||
<pre> def self.confirmTopic(user_id, team_id, topic_id, assignment_id) | <pre> def self.confirmTopic(user_id, team_id, topic_id, assignment_id) | ||
# check whether user has signed up already | # check whether user has signed up already | ||
Line 88: | Line 108: | ||
=== self.create_SignUpTeam === | === self.create_SignUpTeam === | ||
<pre> def self.create_SignUpTeam(assignment_id, sign_up, topic_id, user_id) | <pre> def self.create_SignUpTeam(assignment_id, sign_up, topic_id, user_id) | ||
if slotAvailable?(topic_id) | if slotAvailable?(topic_id) | ||
Line 106: | Line 125: | ||
=== self.slotAvailable? === | === self.slotAvailable? === | ||
<pre>def self.slotAvailable?(topic_id) | <pre>def self.slotAvailable?(topic_id) | ||
SignUpTopic.slotAvailable?(topic_id) | SignUpTopic.slotAvailable?(topic_id) | ||
Line 196: | Line 214: | ||
This method handles the process of marking users as waitlisted, recording this status in the database, and logging this action for reference purposes. | This method handles the process of marking users as waitlisted, recording this status in the database, and logging this action for reference purposes. | ||
== Design in signed_up_team.rb file == | == Design in signed_up_team.rb file== | ||
''' | We relocated the '''self.find_team_users''' method to '''Team.rb''' to align its functionality with the appropriate file, as it pertains to the functionalities defined in '''Team.rb'''. | ||
<pre> | |||
def self.find_team_users(assignment_id, user_id) | |||
TeamsUser.joins('INNER JOIN teams ON teams_users.team_id = teams.id') | |||
<pre>def self. | .select('teams.id as t_id') | ||
.where('teams.parent_id = ? and teams_users.user_id = ?', assignment_id, user_id) | |||
end | |||
</pre> | |||
== Testing Plan == | == Testing Plan == | ||
=== RSpec=== | === RSpec=== | ||
The test scenarios cover various functionalities of the system under test across different modules. Each test scenario includes a set of detailed test cases designed to validate specific behaviors and functionalities. These test cases encompass positive and negative test scenarios, checking inputs, expected outputs, and system responses under varying conditions. The scenarios focus on functionalities such as slot availability checks, team signup operations, waitlist handling, and retrieval of topic IDs by team IDs. These tests aim to ensure the system operates as intended, detecting and reporting any anomalies or deviations from expected behavior | |||
'''spec/models/sign_up_topic_spec.rb''' | '''spec/models/sign_up_topic_spec.rb''' | ||
Line 346: | Line 338: | ||
'''spec/models/sign_up_team_spec.rb''' | '''spec/models/sign_up_team_spec.rb''' | ||
1 : topic_id_by_team_id | '''1 : topic_id_by_team_id''' | ||
<pre> | <pre> | ||
describe '.topic_id_by_team_id' do | describe '.topic_id_by_team_id' do | ||
Line 370: | Line 362: | ||
</pre> | </pre> | ||
''' Screenshot of RSpec Test Cases Running Successfully ''' | |||
[[File:E2375 testcases.jpeg|1000px]] | |||
==Demo Video Link== | |||
*[https://www.youtube.com/watch?v=1SFfc1I1A_0 https://www.youtube.com/watch?v=1SFfc1I1A_0] | |||
==Team== | ==Team== | ||
Line 379: | Line 377: | ||
* Prateek Singamsetty <pksingam@ncsu.edu> | * Prateek Singamsetty <pksingam@ncsu.edu> | ||
* Rushabh Shah <rshah32@ncsu.edu> | * Rushabh Shah <rshah32@ncsu.edu> | ||
==Pull Request== | |||
*[https://github.com/expertiza/expertiza/pull/2708 https://github.com/expertiza/expertiza/pull/2708] |
Latest revision as of 05:40, 5 December 2023
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.
Flowchart Illustrating Our Approach
- Below is the flow of our approach to signing up a user for a specific topic.
UML Diagram
- Below is the UML diagram of our proposed approach
SignUpSheet Class:
The `SignUpSheet` class has a single method:
- `signup_team(assignment_id, user_id, topic_id = nil)`: Used to sign up a team for a chosen topic. It internally communicates with the `SignUpTopic` class.
SignUpTopic Class: The `SignUpTopic` class has three methods:
- `signup_team_for_chosen_topic(team_id)`: Handles the process of signing up a team for a chosen topic.
- `assign_topic_to_team(sign_up, topic_id)`: Assigns a topic to a team and updates the waitlist status.
- `save_waitlist_entry(sign_up, team_id)`: Saves a team to the waitlist if no slots are available. It also logs the creation of the sign-up sheet for waitlisted users. This class communicates with the `SignedUpTeam` class.
SignedUpTeam Class: The `SignedUpTeam` class has two methods:
- `drop_off_waitlists(team_id)`: Drops a team from all waitlists.
- `find_user_signup_topics(assignment_id, team_id)`: Finds all topics for a user within a team for a specific assignment.
Associations:
- The associations between classes represent communication or dependency. For example, `SignUpSheet` communicates with `SignUpTopic`, and `SignUpTopic` communicates with `SignedUpTeam`.
Each class is represented with its methods, and the associations show the flow of communication between these classes. The "1" near the associations signifies the multiplicity or the number of instances participating in the relationship.
Design in sign_up_sheet.rb file
Methods modified
self.signup_team
Below are the changes made to the existing signup_team method
Modified 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
Eliminated Methods in sign_up_sheet.rb
We removed the methods self.confirmTopic,self.create_SignUpTeam and self.slotAvailable? in signup_sheet.rb due to redundancy, lengthiness, and unnecessary repetitive checks. We reimplemented new methods for this functionality in sign_up_topic.rb focusing on eliminating redundant checks and optimizing the flow for better efficiency and readability.
self.confirmTopic
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
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?
def self.slotAvailable?(topic_id) SignUpTopic.slotAvailable?(topic_id) end
Design in sign_up_topic.rb file
Newly implemented methods for topic sign-up functionality.
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
This method, signup_team_for_chosen_topic, facilitates the signup process for a chosen topic by a team. Here's a breakdown of its functionalities:
- Checking User's Signup Status: It verifies if the user is already signed up and waitlisted for the chosen topic within their team for a specific assignment.
- Creating a New Signup Entry: If the user is not already signed up, it creates a new SignedUpTeam entry for the provided topic and team details.
- Slot Availability Check: It verifies if a slot is available for the topic. If a slot is open: Assigns the topic to the team and then removes the team from any existing waitlists.
- Handling Waitlist: If no slots are available, the team is saved to the waitlist for the topic.
This method effectively manages the signup process by ensuring slot availability and handling waitlist scenarios for teams signing up for specific topics.
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
This method, assign_topic_to_team, is responsible for assigning a topic to a team within the signup process. Here's a summary of its functionalities:
- Waitlist Status Update: It updates the team's waitlist status to false as they are assigned a topic, indicating that they are no longer on the waitlist.
- Updating Topic ID: It updates the topic_id in the SignedUpTeam table for the user who has been assigned the topic. It finds the relevant entry based on the topic_id and updates it if the entry exists.
This method manages the assignment process by updating the team's status and ensuring the appropriate update of the topic ID in the SignedUpTeam table.
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
The save_waitlist_entry method is responsible for saving the signup entry for users who have been waitlisted. Here's a brief overview of its functionality:
- Updating Waitlist Status: It sets the is_waitlisted attribute of the sign_up object to true, indicating that the user is being waitlisted for a particular topic.
- Saving Waitlist Status: It saves this updated status in the database, ensuring that the user's waitlist status is correctly recorded.
- Logging Creation: Additionally, it logs the creation of the signup sheet for the waitlisted user, providing a record of the waitlist action for the specified team ID.
This method handles the process of marking users as waitlisted, recording this status in the database, and logging this action for reference purposes.
Design in signed_up_team.rb file
We relocated the self.find_team_users method to Team.rb to align its functionality with the appropriate file, as it pertains to the functionalities defined in Team.rb.
def self.find_team_users(assignment_id, user_id) TeamsUser.joins('INNER JOIN teams ON teams_users.team_id = teams.id') .select('teams.id as t_id') .where('teams.parent_id = ? and teams_users.user_id = ?', assignment_id, user_id) end
Testing Plan
RSpec
The test scenarios cover various functionalities of the system under test across different modules. Each test scenario includes a set of detailed test cases designed to validate specific behaviors and functionalities. These test cases encompass positive and negative test scenarios, checking inputs, expected outputs, and system responses under varying conditions. The scenarios focus on functionalities such as slot availability checks, team signup operations, waitlist handling, and retrieval of topic IDs by team IDs. These tests aim to ensure the system operates as intended, detecting and reporting any anomalies or deviations from expected behavior
spec/models/sign_up_topic_spec.rb
1 : slot_available?
describe 'slot_available?' do let(:topic) { instance_double(SignUpTopic, id: 1, max_choosers: 3) } context 'when no students have selected the topic yet' do it 'returns true' do allow(SignedUpTeam).to receive(:where).with(topic_id: 1, is_waitlisted: false).and_return([]) allow(SignUpTopic).to receive(:find).with(1).and_return(topic) expect(SignUpTopic.slot_available?(1)).to eq(true) end end context 'when students have selected the topic but slots are available' do it 'returns true' do allow(SignedUpTeam).to receive(:where).with(topic_id: 1, is_waitlisted: false).and_return([double, double]) allow(SignUpTopic).to receive(:find).with(1).and_return(topic) expect(SignUpTopic.slot_available?(1)).to eq(true) end end context 'when all slots for the topic are filled' do it 'returns false' do allow(SignedUpTeam).to receive(:where).with(topic_id: 1, is_waitlisted: false).and_return([double, double, double]) allow(SignUpTopic).to receive(:find).with(1).and_return(topic) expect(SignUpTopic.slot_available?(1)).to eq(false) end end end
2 : save_waitlist_entry
describe 'save_waitlist_entry' do let(:sign_up) { instance_double(SignedUpTeam, is_waitlisted: false, save: true) } let(:logger_message) { instance_double(LoggerMessage) } before do allow(LoggerMessage).to receive(:new).and_return(logger_message) allow(sign_up).to receive(:is_waitlisted=) allow(sign_up).to receive(:save).and_return(true) allow(ExpertizaLogger).to receive(:info) end context 'when saving the user as waitlisted' do it 'updates the user\'s waitlist status and logs the creation of sign-up sheet' do result = SignUpTopic.save_waitlist_entry(sign_up, 123) expect(sign_up).to have_received(:is_waitlisted=).with(true) expect(sign_up).to have_received(:save) expect(LoggerMessage).to have_received(:new).with('SignUpSheet', '', "Sign up sheet created for waitlisted with teamId 123") expect(ExpertizaLogger).to have_received(:info).with(logger_message) expect(result).to eq(true) end end end
3 : signup_team_for_chosen_topic
describe '#signup_team_for_chosen_topic' do let(:team) { create(:team) } let(:topic) { SignUpTopic.new(id: 1, max_choosers: 3) } context 'when the team is not already signed up or waitlisted for the topic' do before do allow(SignedUpTeam).to receive(:find_user_signup_topics).and_return([]) end it 'signs up the team for the chosen topic' do allow(topic).to receive(:signup_team_for_chosen_topic).and_return(true) allow(SignUpTopic).to receive(:slot_available?).and_return(true) expect(topic.signup_team_for_chosen_topic(team.id)).to eq(true) end end context 'when the team is already signed up and waitlisted for the topic' do before do allow(SignedUpTeam).to receive(:find_user_signup_topics).and_return([double(is_waitlisted: true)]) end it 'does not create a new signup entry' do allow(topic).to receive(:signup_team_for_chosen_topic).and_return(false) expect(topic.signup_team_for_chosen_topic(team.id)).to eq(false) end end context 'when there are no available slots for the topic' do before do allow(SignUpTopic).to receive(:slot_available?).and_return(false) end it 'creates a new waitlisted signup entry' do allow(topic).to receive(:signup_team_for_chosen_topic).and_return(true) expect(topic.signup_team_for_chosen_topic(team.id)).to eq(true) end end end
spec/models/sign_up_team_spec.rb
1 : topic_id_by_team_id
describe '.topic_id_by_team_id' do let(:team_id_existing) { 1 } let(:team_id_non_existing) { 2 } it 'returns topic_id when team has signed up and not on waitlist' do # Create a SignedUpTeam record with team_id_existing and is_waitlisted: false signed_up_team = create(:signed_up_team, team_id: team_id_existing, is_waitlisted: false) expect(described_class.topic_id_by_team_id(team_id_existing)).to eq(signed_up_team.topic_id) end it 'returns nil when team has signed up but is on waitlist' do # Create a SignedUpTeam record with team_id_existing and is_waitlisted: true create(:signed_up_team, team_id: team_id_existing, is_waitlisted: true) expect(described_class.topic_id_by_team_id(team_id_existing)).to be_nil end it 'returns nil when team has not signed up' do expect(described_class.topic_id_by_team_id(team_id_non_existing)).to be_nil end end
Screenshot of RSpec Test Cases Running Successfully
Demo Video Link
Team
Mentor
- Ed Gehringer
Members
- Dinesh Pasupuleti <dpasupu@ncsu.edu>
- Prateek Singamsetty <pksingam@ncsu.edu>
- Rushabh Shah <rshah32@ncsu.edu>