CSC/ECE 517 Spring 2025 - E2503. Refactor the Team hierarchy

From Expertiza_Wiki
Jump to navigation Jump to search

Team Information

Mentor:

  • Ed Gehringer

Team Members:

  • Ahmed Hassan (aohassan)
  • Rameez Malik (remalik)
  • Moaad Benkaraache (mbenkar)

Relevant Links:

Expertiza Background

Expertiza is an open-source project built using the Ruby on Rails MVC framework, designed to facilitate peer review and collaborative learning. The project has undergone multiple iterations, with ongoing efforts focused on refactoring the existing codebase to improve maintainability, efficiency, and scalability. Additionally, there are initiatives aimed at reimplementing a new version of Expertiza to enhance its architecture and functionality.

About Team/CourseTeam/AssignmentTeam/MentoredTeam

The Team hierarchy contains the classes that hold teams of students enrolled in Expertiza. Teams can reserve topics and submit work. There are currently four classes in the team hierarchy: Team, CourseTeam, AssignmentTeam, and MentoredTeam. Instances of the Team superclass should never be created since any actual team will be a member of one of the three other classes. CourseTeams stay together for the entire course duration and work on all assignments together. Although some instructors choose not to use CourseTeams, others employ Team-Based Learning and do benefit from using CourseTeams. It is possible to both copy CourseTeams into AssignmentTeams, and vice-versa. Any team formed to take part in an assignment will be an AssignmentTeam, unless the assignment is set up to automatically assign mentors to teams. In this case, the assignment teams will be created as MentoredTeams (a subclass of AssignmentTeam) which has a designated member known as the mentor.

Problem Statement

The Team hierarchy in Expertiza is responsible for managing student teams, allowing them to reserve topics, submit work, and, in some cases, review other teams' submissions. However, the current implementation is overly complex, with redundant methods and unnecessary dependencies. The Team class has 27 methods, many of which should be refactored or removed, while AssignmentTeam contains 36 methods, several of which duplicate functionality from the superclass. Additionally, the system currently models team members as Users, but the new design will transition to Participants, requiring updates to team membership logic.

Tasks

Refactoring the Team Hierarchy

  • Prevent direct instantiation of Team, ensuring all actual teams belong to CourseTeam, AssignmentTeam, or MentoredTeam.
  • Move only the core shared functionalities to the Team and reduce the method count.
  • Eliminate Redundant Methods in AssignmentTeam and CourseTeam.
  • Identify duplicate methods across Team, AssignmentTeam, and CourseTeam.
  • Ensure CourseTeam and AssignmentTeam override import/export functionality from Team instead of defining separate versions.

Transition from Users to Participants

  • Modify add_member and related methods to work with CourseParticipants and AssignmentParticipants instead of Users.
  • Ensure teams enforce correct membership rules (e.g., CourseTeam only accepts CourseParticipants).

Optimizing Team Copying

  • Ensure clear and efficient handling of copying between AssignmentTeam and CourseTeam.
  • Consolidate logic for copying members across different team types.

Improving Method Clarity and Encapsulation

  • Convert class methods like size into instance methods.
  • Standardize method naming conventions for clarity (e.g., avoid inconsistencies like copy_members vs. copy_content).

Removing Unused and Inefficient Code

  • Identify and remove any unused methods in Team, AssignmentTeam, and CourseTeam.
  • Remove commented-out or deprecated code that no longer serves a purpose.
  • Simplify methods that are overly complex by leveraging helper functions where necessary.

Testing and Code Validation

  • Update tests to reflect the refactored team hierarchy and ensure correctness.
  • Remove or rewrite tests that validate methods no longer in use.

Plan of Work

We began by listing all of the methods belonging to each class in the Team hierarchy. Next, we met with our mentor (Dr. Gehringer) and reviewed every single method’s readability, maintainability, and adherence to best practices (SOLID principles, DRY, etc.). Once we organized the methods by class, we assigned groups of methods as tasks amongst team members and began refactoring existing methods while making sure existing test cases still passed and new tests were added as needed.

Implementation

Refactor Team Model

  • Methods involved:
    • self.allowed_types
    • self.team_operation
    • self.responses
    • author_names
    • user?(user)
    • add_member(user, _assignment_id = nil)
    • self.size(team_id)
    • copy_members(new_team)
    • self.check_for_existing(parent, name, team_type)
    • self.randomize_all_by_parent(parent, team_type, min_team_size)
    • self.create_team_from_single_users(min_team_size, parent, team_type, users)
    • self.assign_single_users_to_teams(min_team_size, parent, teams, users)
    • self.generate_team_name(_team_name_prefix = “”)
    • self.create_team_and_node(id)
    • self.create_team_with_users(parent_id, user_ids)
    • self.remove_user_from_previous_team(parent_id, user_id)
    • self.find_team_users(assignment_id, user_id)
  • Changes made:
    • Moved parent_entity_type and self.find_parent_entity(id) from CourseTeam to Team.

    • Renamed author_names to member_names.

    • Removed unnecessary parameter from add_member; replaced size with instance method; CourseTeam’s add_participant method was removed, and CourseTeam inherits Team.add_member instead.

    • Renamed randomize_all_by_parent to create_random_teams.

    • Renamed create_team_from_single_users to team_from_users.

    • No changes to self.handle_duplicate.

    • Added optional user_ids parameter to facilitate removal of create_team_with_users method to avoid redundancy.

Refactor CourseTeam Model

  • Methods involved:
    • parent_model
    • self.parent_model(id)
    • assignment_id
    • self.prototype
    • copy(assignment_id)
    • add_participant(course_id, user)
    • self.import(row, course_id, options)
    • self.export(csv, parent_id, options)
    • self.export_fields(options)
    • add_member(user, _id, = nil)
  • Changes made:
    • parent_model - Replaced with parent_entity_type() in the Team class for clarity, since it duplicated functionality.
    • self.parent_model(id) - Logic was moved to Team.find_parent_entity(id) to consolidate redundant behavior.
    • assignment_id - No changes made; it correctly returns nil to indicate it's not an AssignmentTeam.
    • self.prototype - Removed because it's unnecessary—creating a new instance with CourseTeam.new is sufficient.
    • copy(assignment_id) - Refactored and renamed to copy_to_assignment() for clarity.

    • add_participant(course_id, user) - Removed, as this responsibility already exists in the Course class.
    • self.import(row, course_id, options) - Left unchanged since it appropriately overrides Team.import().
    • self.export(csv, parent_id, options) - Left unchanged as it overrides Team.export() correctly.
    • self.export_fields(options) - Left unchanged because it’s used in export_file_controller.rb for file exports.
    • add_member(user, _id = nil) - Removed due to duplication; the Team class already implements add_member().

Refactor AssignmentTeam Model

  • Methods involved:
    • user_id
    • set_current_user(current_user)
    • includes?(participant)
    • parent_model
    • self.parent_model(id)
    • fullname
    • review_map_type
    • self.prototype
    • assign_reviewer(reviewer)
    • get_reviewer
    • reviewed_by?(reviewer)
    • topic
    • has_submissions?
    • participants
    • delete
    • destroy
    • self.first_member(team_id)
    • submitted_files(path = self.path)
    • self.import(row, assignment_id, options)
    • self.export(csv, parent_id, options)
    • copy(course_id)
    • add_participant(assignment_id, user)
    • hyperlinks
    • files(directory)
    • submit_hyperlink(hyperlink)
    • remove_hyperlink(hyperlink_to_delete)
    • self.team(participant)
    • self.export_fields(options)
    • self.remove_team_by_id(id)
    • path
    • set_student_directory_num
    • received_any_peer_review
    • most_recent_submission
    • get_logged_in_reviewer_id(current_user_id)
    • current_user_is_reviewer?(current_user_id)
    • create_new_team(user_id, signuptopic)
  • Changes made:
    • user_id was split into current_user_id and first_user_id for clarity
    • set_current_user(current_user) was renamed to store_current_user(current_user) for clarity
    • includes?(participant) was moved to Team for clarity, and now called has_participant?(participant)
    • parent_model was replaced with parent_entity_type() in the Team class for clarity, since it duplicated functionality
    • self.parent_model(id) logic was moved to Team.find_parent_entity(id) to consolidate redundant behavior
    • review_map_type left as is [no change]
    • self.prototype removed because it's unnecessary—creating a new instance with AssignmentTeam.new is sufficient
    • assign_reviewer left as is [no change]
    • get_reviewer() was renamed to reviewer()
    • reviewed_by left as is [no change]
    • topic() renamed to topic_id()

    • has_submissions? left as is [no change]
    • participants left as is [no change]
    • delete left as is [no change]
    • destroy left as is [no change]
    • self.first_member(team_id) removed since the system now reviews teams instead of review Individuals
    • submitted_files(path = self.path) left as is [no change]
    • self.import(row, assignment_id, options) was refactored to delegate to Team.import()
    • self.export(csv, parent_id, options) was refactored to delegate to Team.export()
    • add_participant(assignment_id, user) now is called add_participant(user) since the assignment_id parameter was redundant assuming that a user is already associated with an Assignment Team
    • hyperlinks left as is [no change]
    • files() refactored to use an iterator-based approach

    • submit_hyperlink(hyperlink) left as is [no change]
    • remove_hyperlink(hyperlink_to_delete) left as is [no change]
    • self.team(participant) left as is [no change]
    • self.export_fields(options) enabled fields to be an explicit return value to improve readability
    • self.remove_team_by_id(id) left as is, very concise; “unless nil” check ensures safety
    • path() was refactored to avoid string concatenation
    • set_student_directory_num was renamed and refactored to set_team_directory_num

    • received_any_peer_review has been renamed to has_been_reviewed?
    • most_recent_submission left as is [no change]
    • get_logged_in_reviewer_id(current_user_id) left as is [no change]
    • current_user_is_reviewer?(current_user_id) left as is [no change]
    • create_new_team(user_id, signuptopic) was renamed to link_user_and_topic(user_id, signuptopic)

Refactor MentoredTeam Model

  • Methods involved:
    • add_member(user, assignment_id = nil)
    • import_team_members(row_hash)
  • Changes made:
    • Refactored add_member since most of the functionality is already in Team.add_member(), and it just needs to overwrite it to add the assigning a mentor functionality to MentoredTeam

    • import_team_members isn't changed since it calls add_member(user, parent_id) which is not similar to what is in the Team.import_team_members()

Test Plan

This project involves refactoring the Team hierarchy, ensuring that existing functionality remains unchanged while improving structure and maintainability. Since the refactor involves modifying class structures, consolidating redundant methods, and introducing better encapsulation, the following steps were taken to validate correctness:

  • Refactored RSpec test files to reflect changes in class structure, method names, and logic updates.
  • Ran the test suite after each major change to verify that functionality remained consistent across Team, CourseTeam, AssignmentTeam, and MentoredTeam.
  • Ensured that core team operations (adding/removing members, copying teams, importing/exporting teams) worked as expected by validating test outputs against pre-refactor results.
  • Ensured all refactored classes maintained high test coverage, targeting at least 90% coverage in Team, AssignmentTeam, and CourseTeam.
  • Ensured that removed methods were either unnecessary or replaced with equivalent logic in the refactored structure.
  • Validated UI functionality by testing that teams were correctly displayed, managed, and interacted with in the system.

Retrieved from "https://wiki.expertiza.ncsu.edu/index.php?title=CSC/ECE_517_Spring_2025_-_E2503._Refactor_the_Team_hierarchy&oldid=162909"