CSC/ECE 517 Fall 2024 - E2456. Refactor teams user.rb (Phase 2 - Design Document)

Expertiza

Expertiza is a web application through which students can submit and peer-review learning objects (articles, code, web sites, etc). The National Science Foundation supports the Expertiza project. It is used in select courses at NC State and by professors at several other colleges and universities.

Expertiza is a Ruby on Rails based open source project.

Problem Statement

The TeamsUser class in the Expertiza repository serves as a join model between the Team and Participant classes, establishing a many-to-many relationship. Its primary purpose is to manage team memberships within the context of assignments or courses, facilitating functionalities such as retrieving a user's name, deleting users or teams, and managing participant associations.

The primary issue with the TeamsUser class is that it associates teams directly with users rather than with participants. In the context of the application, participants are users who are specifically associated with an assignment or a course. Since teams are formed within the scope of assignments or courses, handling team memberships through participants is more appropriate. Almost everywhere else, an assignment uses Participant objects rather than User objects. The exception is when it comes to Teams. It would simplify the design to use TeamsParticipants instead of TeamsUsers.

Purpose

The TeamsUser class currently serves as a join model between Team and Participant classes, managing team memberships for assignments and courses. However, it creates an architectural inconsistency by linking teams directly to users rather than participants, even though participants are the primary entities associated with assignments and courses throughout the rest of the application. The refactoring initiative will replace the TeamsUser class with a new TeamsParticipant class, bringing several key improvements:

1. Align team management with the application's existing participant-based pattern.

2. Creates proper associations between teams and participants within assignment contexts.

Design Goal

The primary design goal is to improve the data model and overall architecture of the Expertiza application by replacing the TeamsUser class with a more appropriate TeamsParticipant class. This change aims to achieve the following objectives:

1. Enhance Data Model Consistency: Modify the association between teams and users to reflect a more accurate relationship between teams and participants throughout the app.

2. Simplify Application Logic: Align the team related management with the existing pattern of using Participant objects rather than User objects throughout the application, reducing exceptions and inconsistencies.

3. Improve Code Maintainability: Refactor the codebase to use TeamsParticipant consistently, updating all relevant models, views, and controllers to reflect this change.

4. Enhance Database Structure: Update the database schema to include appropriate foreign keys and indexes for the TeamsParticipant table, improving data integrity and query performance.

5. Ensure Backward Compatibility: Implement the changes in phases, allowing for a gradual transition from TeamsUser to TeamsParticipant while maintaining functionality throughout the refactoring process.

6. Improve Testability: Develop comprehensive test cases for the new TeamsParticipant model and update existing tests to reflect the changes in associations and logic.

Design Pattern

To manage the complexity and responsibilities of the TeamsParticipant class, the following design patterns can be applied here. Since TeamsParticipant is handling both join table duties and additional business logic, a combination of patterns could improve maintainability and flexibility.

1. Facade Pattern for Simplifying Interface

The TeamsParticipant class handles multiple responsibilities, from managing teams and participants to removing members and adding members who accepted invites. Applying the Facade Pattern can simplify its interface by offloading complex processes to service classes. For example, a TeamsParticipantService could manage team membership rules, while TeamsParticipantManager could handle table-related operations.

class TeamParticipantService
  def self.add_accepted_invitee_to_team(inviter_participant_id, invited_participant_id, assignment_id)
    participants_teams = TeamsParticipant.where(participant_id: inviter_participant_id)
    participants_teams.any? do |team|
      assigned_team = AssignmentTeam.find_by(id: team.team_id, parent_id: assignment_id)
      assigned_team&.add_member(Participant.find(invited_participant_id), assignment_id) if assigned_team
    end
  end

  def self.handle_pair_programming(team_id, user_id)
    user = TeamsParticipant.find_by(team_id: team_id, user_id: user_id)
    user.update_attributes(pair_programming_status: "A")
  end

  def self.team_empty?(team_id)
    TeamsParticipant.where(team_id: team_id).empty?
  end
end

Then, within TeamsParticipant, these actions could be delegated to the facade for simplicity and modularity:

class TeamsParticipant < ApplicationRecord
  # Other associations and methods
  
  def self.add_accepted_invitee_to_team(*args)
    TeamParticipantService.add_accepted_invitee_to_team(*args)
  end
end

2. Repository Pattern for Database Queries

The Repository Pattern would be useful for separating data access logic since this class is heavily querying the database for create, update, find operations. This involves creating a TeamsParticipantRepository that encapsulates all finders and queries, improving modularity and making it easier to test and reuse.

class TeamsParticipantRepository
  def self.find_team_participant(participant_id, team_id)
    TeamsParticipant.find_by(participant_id: participant_id, team_id: team_id)
  end

  def self.first_participant_for_team(team_id)
    TeamsParticipant.find_by(team_id: team_id)
  end

  def self.team_empty?(team_id)
    TeamsParticipant.where(team_id: team_id).empty?
  end

  def self.find_team_id(assignment_id, user_id)
    TeamsParticipant.find_team_id(assignment_id, user_id)
  end

  def self.get_team_participants(team_id)
    TeamsParticipant.where(team_id: team_id)
  end

  def self.get_participants_for_review(team_id)
    TeamsParticipant.where(team_id: team_id).each do |team_user|
      yield team_user
  end
end

Final design

The initial design proposed using the Facade and Repository patterns to improve the architecture of the TeamsParticipant refactoring. However, during implementation, we took a more direct approach for several reasons:

1. Existing Code Structure: The codebase already had established patterns for controller-model interactions. The changes were made to align with these existing patterns, as seen in the invitation.rb and review_mapping_controller.rb modifications

2. Minimal Benefit: The main goal was to fix the architectural inconsistency of teams being linked to users instead of participants. The proposed patterns would have added an extra layer of abstraction without providing significant immediate benefits to this specific problem

3. Future Considerations: While the Facade and Repository patterns could improve code organization and maintainability, they would be better implemented as part of a larger architectural refactoring effort.

Migration Strategy

1. Create new TeamsParticipant model and table

2. Implement parallel methods in TeamsParticipant

3. Update controllers to use TeamsParticipant

4. Migrate existing data

5. Remove TeamsUser dependencies

Class Diagram

Sequence Diagram

Solutions/Details of Changes Made

Phase 1 Improvements

Renamed TeamsUser to TeamsParticipant to modify application logic to join Teams with Participants instead of Users
Updated associations in immediate neighbor classes i.e Teams, Users and Participants to ensure correctness and consistency in the data model
Renamed method names for increased readability and removed redundant methods from the TeamsParticipant class
Refactored methods: add_accepted_invitee_to_team, and find_team_id for efficiency and reducing complexity, improving overall maintainability of the codebase
Renamed TeamUserNode to TeamParticipantNode for correctness, to align with the new design of Participation association in TeamsParticipant
Added migrations to add foreign key for Participant in TeamsParticipant table and updated indexes for the same

Phase 2 Plan

Update associations in all the models that were previously using TeamsUser to implement them with TeamsParticipant
Modify implementations of all models, views, and controllers to use Participant with TeamsParticipant class instead of User
This future involves modifying queries to retrieve participant and user objects that are passed to the TeamsParticipant class as commands to their methods
Deprecate user_id foreign key from TeamsParticipant to completely remove the association of User from TeamsParticant, only linking Participants to Teams
Update all the test cases to work with this implementation, including updating associations and mocking DB calls

Files Added/Modified

As part of the refactoring process, we need to update all references and method calls related to the TeamsParticipant table and the teams_participant class throughout the repository. This refactor will involve over 200 instances across various files.

Affected Components

  Models: 
  1. course_team.rb
  2. mentor_management.rb
  3. mentored_team.rb
  4. review_response_map.rb
  5. signed_up_team.rb
  6. tag_prompt_deployment.rb
  7. invitation.rb

  Controllers: 
  1. assessment360_controller.rb
  2. grades_controller.rb
  3. join_team_requests_controller.rb
  4. lottery_controller.rb
  5. popup_controller.rb
  6. sign_up_sheet_controller.rb
  7. student_teams_controller.rb
  8. suggestion_controller.rb
  9. review_mapping_controller.rb
  10. pair_programming_controller.rb

  Helpers/Workers:
  1. assignment_helper.rb
  2. manage_team_helper.rb
  3. review_mapping_helper.rb
  4. sign_up_sheet_helper.rb
  5. teams_users_helper.rb
  6. mail_worker.rb

  Views:
  1. assignments/edit/_calibration.html.erb
  2. bookmarks/list.html.erb
  3. popup/participants_popup.html.erb
  4. reports/_teammate_review_report.html.erb
  5. response/response.html.erb
  6. review_mapping/_list_review_mappings.html.erb
  7. sign_up_sheet/show_team.html.erb
  8. student_teams/_received_invitations.html.erb
  9. student_teams/view.html.erb
  10. submitted_content/_hyperlink.html.erb

The required changes will follow the pattern shown in the attached modifications for models and controllers.

invitation.rb

review_mapping_controller.rb

pair_programming_controller.rb

Test Plan

We added a new spec file for the teams_participant model, which was previously missing, and fixed issues in the team_participant_controller_spec. Additionally, we resolved model and controller tests for direct associations with Team, User, and Participant.

Detailed Test Plan for `TeamsParticipant` Model

The test plan for the TeamsParticipant refactoring will verify both the model's core functionality and its integration with the invitation and review mapping systems.

Test 1. Associations

Verify correct associations:
 * Belongs to participant
 * Belongs to team
 * Has one team_participant_node (dependent: destroy)

Test 2. Method: #name

 Returns participant’s name.
 Appends "(Mentor)" if participant is a mentor.

Test 3. Method: #delete

Calls methods for cleanup:
 * remove_team_participant_node
 * remove_team_if_participants_empty
 * remove_teams_participant_instance
Destroys itself and associated TeamParticipantNode.
Deletes the team if it’s the last participant.

Test 4. Method: .remove_participant

Destroys TeamsParticipant if it exists.
Does nothing if it doesn’t exist.

Test 5. Method: .first_participant_for_team

Retrieves the first participant for a given team ID.

Test 6. Method: .team_empty?

Returns true if team has no participants; false otherwise.

Test 7. Method: .add_accepted_invitee_to_team

Adds a participant to the team, returning true if successful.

Test 8. Method: .team_id

Returns team ID if participant is assigned to a team.
Returns nil if not assigned.

Model Tests

 describe '.remove_participant' do
   before do
     allow(TeamsParticipant).to receive(:find_by).with(user_id: user.id, team_id: team.id).and_return(team_participant)
   end
   context 'when the team participant exists' do
     it 'destroys the team participant' do
       expect(team_participant).to receive(:destroy)
       TeamsParticipant.remove_team_participant(user.id, team.id)
     end
   end
   context 'when the team participant does not exist' do
     before do
       allow(TeamsParticipant).to receive(:find_by).with(user_id: user.id, team_id: team.id).and_return(nil)
     end
 
     it 'does not call destroy' do
       expect(team_participant).not_to receive(:destroy)
       TeamsParticipant.remove_team_participant(user.id, team.id)
     end
   end
 end

Integration Tests

 describe 'edit method' do
   context 'when team with given id that exists' do
     it 'successfully returns the team with the given team id' do
       allow(Team).to receive(:find).and_return(team1)
       request_params = { id: team1.id }
       user_session = { user: ta }
       result = get :edit, params: request_params, session: user_session
       expect(result.status).to eq 200
       expect(controller.instance_variable_get(:@team)).to eq team1
     end
   end
   context 'when team with given id does not exist' do
     it 'raises an ActiveRecord::RecordNotFound error' do
       expect {
         get :edit, params: { id: 999 }, session: { user: ta }
       }.to raise_error(ActiveRecord::RecordNotFound)
     end
   end
 end

UI Testing

1. Created a new team for an assignment, sent an invite to another user and successfully accepted the invite, and added another user to the team, demonstrating the working of team-participants

2. Added a participant to an existing course

Sample Test Changes

We went ahead with updating all the table references as needed, adjusting or adding mocks for methods, and revising database queries involving the TeamsParticipant table.

Below are examples of changes that were made to the 200 refrences across all models, controllers and views throughout the repository.

invitation_spec.rb

review_mapping_controller_spec.rb

pair_programming_controller_spec.rb

Test success

Final Pull Request

The PR to Expertiza repo can be found here:

E2456. Refactor teams user.rb #2887

Project Demo

Click here to view demo

Team

Mentor

Jay, Patel

Members

Agarwal, Arjit
Manbhekar, Pranav
Singhania, Chinmay

References

CSC/ECE 517 Fall 2024 - E2456. Refactor teams user.rb (Phase 2 - Design Document)

Contents

Expertiza

Problem Statement

Purpose

Design Goal

Design Pattern

Final design

Migration Strategy

Class Diagram

Sequence Diagram

Solutions/Details of Changes Made

Phase 1 Improvements

Phase 2 Plan

Files Added/Modified

Affected Components

Test Plan

Detailed Test Plan for `TeamsParticipant` Model

Model Tests

Integration Tests

UI Testing

Sample Test Changes

Test success

Final Pull Request

Project Demo

Team

Mentor

Members

References

Navigation menu

CSC/ECE 517 Fall 2024 - E2456. Refactor teams user.rb (Phase 2 - Design Document)

Expertiza

Problem Statement

Purpose

Design Goal

Design Pattern

Final design

Migration Strategy

Class Diagram

Sequence Diagram

Solutions/Details of Changes Made

Phase 1 Improvements

Phase 2 Plan

Files Added/Modified

Affected Components

Test Plan

Detailed Test Plan for TeamsParticipant Model

Model Tests

Integration Tests

UI Testing

Sample Test Changes

Test success

Final Pull Request

Project Demo

Team

Mentor

Members

References

Navigation menu

Search

Detailed Test Plan for `TeamsParticipant` Model