CSC/ECE 517 Spring 2024 - E2404 Refactor student teams functionality
This page provides details about Expertiza Issue E2404. "Refactor student_teams functionality" which is an OSS project in CSC517 Spring 2024.
Background
The Expertiza project is software to create reusable learning objects through peer review. It also supports team projects, and the submission of almost any document type, including URLs and wiki pages. Users & Participants: Everyone who took the OODD class is a user in expertiza, and instructors can add users as participants in an assignment so that they can access it. Almost everything within Assignments is done by participants, not users. That is, everything except teams_users. In the documentation, it can be noted that the team_users table references users instead of participants. This anomaly causes problems with how student teams are rendered in the UI, and it doesn’t go well with the new functionality that was recently introduced.
Requirements
Changing the schema
- Rename the table table teams_users to teams_participants
- Create a column that would be called “participant_id”, which will be a foreign key that references participants table
- Create a migration that fetches the assignment_id for each tuple from the teams table.
- Find out all the places where user_id is being used, refactor those so that participant_id is used.
Refactoring the teams rendering (/views/student_teams/view.html.erb)
- The logic in the team's view that iterates and fetches information of all the team members and displays them would be refactored so that it matches our new design.
- The new changes should not break the view that works for data where things are populated with user_id. Hence best approach is separate the older functionality into a separate partial and code the new functionality into a separate partial.
- All the teams are being formed with participant_id instead of user_id
Other Changes
- Update logic for all the crud functionalities in teams_users controller
- Rename the teams_users_controller to teams_participants_controller
- Some code where sql queries are written to fetch data, should be replaced with fetching data from rails ActiveRecord methods.
- The html.erb files have extensive ruby code written for fetching data from the database. This code should be removed from the view files and placed into helper classes.
- Extensive test cases should be present for all the code that is written.
Changes suggested for Phase 2
- Since the team introduced a new file teams_participants_controller.rb, it might be beneficial to add more comments.
- The create function is too long which could be split into a few more.
- There are still some other places violating DRY (In the send_invitations, accept, and decline methods, there is repeated code for updating the pair_programming_status attribute of users based on whether they are TeamsUser or TeamsParticipant.
- Similarly, there is duplication in the flash message logic across these methods).
- Furthermore, the existing test cases were modified so that they would pass for the new code which includes participant_id.
- A few tests are currently failing or canceled on GitHub actions obscuring the coverage count and they should be modified so that they are passing. Many other files need to be modified to factor in the renaming done in the project.
Schema Changes
Here's a brief overview of the key changes we implemented:
- Table Renaming: Our schema initially included a teams_users table, which, upon review, seemed misaligned with our domain model. To better represent the relationship between teams and participants, we decided to rename this table to teams_participants. GitHub Commit
- Introduction of participant_id: To establish a more direct linkage between team participations and individual participants, we added a participant_id column to the newly named teams_participants table. This column serves as a foreign key that references the participants table, clearly defining the relationship. GitHub Commit
- Populating participant_id column: The centerpiece of our refactor was the migration designed to populate the new participant_id column with accurate data. Leveraging the existing assignment_id from the teams table and user_id from the teams_participants table, our migration carefully matched each team participation to its corresponding participant and updated the participant_id accordingly.GitHub Commit
ER diagram for updated schema
Refactoring teams rendering
The objective of this refactoring task is to improve the readability, maintainability, and scalability of the code responsible for rendering team information in the view.html.erb file of the student_teams view. This refactoring involves separating existing functionality into specific partials and ensuring compatibility with the new data structure using participant_id instead of user_id. It's crucial to ensure that these changes do not break existing functionality that relies on user_id data. By separating old and new functionalities into distinct partials and conducting a data migration, the codebase will become more organized and adaptable, facilitating future development efforts. GitHub Commit For phase 2 of Final Project, files committed can be found here. GitHub Commit
Refactor Implementation
We have successfully refactored the 'view.html.erb' file in the student_teams view to enhance readability and maintainability. The original code was cluttered, making it challenging to understand and extend. Through modularization, we have simplified the code by introducing partials, resulting in a more organized structure.
The following partials were created as part of the refactoring process:
1. '_invited_people.html.erb' : This partial is responsible for displaying invited participants to the assignment team.
2. '_display_advertisements.html.erb' : This partial handles the display of advertisements within the view.
3. '_send_invitations.html.erb': This partial implements the functionality for sending invitations to team members.
4. '_received_invitations.html.erb': This partial is used to display received invitations for the current user.
These partials contribute to a cleaner and more manageable codebase, improving the overall developer experience and facilitating future updates to the Expertiza application.
Refactor Implementation - Phase 2 changes
'app/controllers/pair_programming_controller.rb' : This controller methods were refactored to avoid violating DRY principle.
Refactoring CRUD Functionalities in Teams Participants Controller
Firstly,we renamed teams_users_controller to teams_participants_controller and also all its reference files like teams_users_helper.rb and teams_users_controller_spec.rb.
In create method,
- Refactored to fetch the AssignmentParticipant only once instead of twice by removing a redundant find query. This change reduces the load on the database, which can be crucial for performance when dealing with many users or heavy server load.
- Adjusted to check if the user is already part of the team by using assignment.participant_on_team? instead of finding the AssignmentParticipant first and then checking the team. This process of using a method called `participant_on_team?`, hides the complexity of the lookup. It's easier to read and can be reused wherever this check is needed. If the way to determine team membership ever changes, we only need to change the logic in one place — the `participant_on_team?` method — instead of everywhere the check is performed.
- Similarly checked for CourseTeam as well.The refactoring eliminated the need for separate searches for a `CourseParticipant` in the database. By combining them into one query, the refactored code reduces database access and, therefore, improves efficiency.
- Improved the process of fetching CourseParticipant by consolidating two separate find queries into one and using if course.participant_on_team? for a cleaner check on team membership. For both assignment teams and course teams, the refactored code uses a consistent approach to check for team membership (`participant_on_team?`). This consistency makes the code easier to understand and maintain. If there's ever a bug or a need to update how team membership is determined, developers will have an easier time addressing it because they only need to focus on the logic inside the `participant_on_team?` method.
In delete method,
- The delete method now uses Participant.find_by instead of User.find to obtain the participant object directly using the participant ID. This reduces the number of database queries.
- Removed a redundant User.find query since the user can be fetched through the participant object.
Refactoring CRUD Functionalities in Teams Participants Controller- Phase 2
The create method is split into few more methods.
create
- This method adapts the process of adding a user to a team. It starts by validating the user's existence through the `validate_user` method, using the user's name passed via parameters. If the user does not exist or validation fails, the method exits early. Assuming validation succeeds, the method then retrieves a specific team based on the provided team ID (`params[:id]`). Following the team retrieval, `process_team_addition` is called to manage the addition of the user to the team. The method concludes by redirecting to a team list page, indicating where the user should be navigated next, effectively managing user flow and maintaining a clean user experience.
validate_user
- The `validate_user` method focuses on ensuring that a user exists in the system before any team assignment actions are taken. It searches for a user by name, stripping any extra spaces to avoid common errors in name entry. If the user isn't found, it sets an error message advising that the user needs to be created first, and redirects back to a fallback location (typically the home page). If the user is found, it returns the user object, making it available for further processing. This step is crucial for maintaining data integrity and providing clear user feedback.
process_team_addition
- This method manages adding the user to a team once the user has been validated. It involves determining if the user can be a participant in the team's activities by calling `find_or_create_participant`. Depending on the validation of the participant, the method might terminate early, or it might proceed to add the user to the team through `add_member_to_team`. This step ensures that team additions are handled appropriately based on the team's requirements and participant eligibility.
find_or_create_participant
- `find_or_create_participant` is critical for managing team composition. It determines whether the user should be an `AssignmentParticipant` or a `CourseParticipant`, based on the team's classification. The method checks if a participant record already exists for the user in relation to the team's parent entity. If the participant already exists and is part of the team, or if no such participant can be added, it generates an error and redirects back to the previous page. This safeguard prevents duplication and ensures that only eligible users are added to teams.
add_member_to_team
- The final step in the user addition process is handled by `add_member_to_team`. This method attempts to add the participant to the team. It accounts for potential errors, such as the team already having the maximum number of members, and captures any exceptions that occur during the addition process. By managing these exceptions and potential errors, the method ensures that the user is either successfully added to the team or provided with appropriate feedback about why the addition could not be completed. This level of error handling is essential for robust application performance and user satisfaction.
Refactoring Signedup Team
- In SignedUpTeam, the superclass is corrected from ApplicationRecord to ActiveRecord::Base, ensuring compatibility with the expected Rails inheritance structure.
- In queries,team_users has been replaced with team_participants in finding team participants and team users.
Implementation
Main files which were changed are
Controllers:
- app/controllers/assessment360_controller.rb
- app/controllers/grades_controller.rb
- app/controllers/invitations_controller.rb
- app/controllers/join_team_requests_controller.rb
- app/controllers/lottery_controller.rb
- app/controllers/pair_programming_controller.rb
- app/controllers/popup_controller.rb
- app/controllers/review_mapping_controller.rb
- app/controllers/sign_up_sheet_controller.rb
- app/controllers/teams_participants_controller.rb
- app/controllers/teams_users_controller.rb
- app/controllers/student_teams_controller.rb
- app/controllers/suggestion_controller.rb
Models:
- app/models/signed_up_team.rb
- app/models/team.rb
- app/models/teams_participant.rb
- app/models/teams_user.rb
- app/models/user.rb
- app/models/assignment.rb
- app/models/assignment_team.rb
- app/models/assignment_participant.rb
- app/models/cake.rb
- app/models/course_team.rb
- app/models/invitation.rb
- app/models/mentor_management.rb
- app/models/participant.rb
- app/models/review_response_map.rb
- app/models/sign_up_sheet.rb
- app/models/tag_prompt_deployment.rb
- app/models/team_user_node.rb
Helpers:
- app/helpers/teams_participants_helper.rb
- app/helpers/teams_users_helper.rb
- app/helpers/review_mapping_helper.rb
- app/helpers/manage_team_helper.rb
- app/helpers/assignment_helper.rb
Views:
- app/views/student_teams/_display_advertisements.html.erb
- app/views/student_teams/_invited_people.html.erb
- app/views/student_teams/_received_invitations.html.erb
- app/views/student_teams/_select_duty_form.html.erb
- app/views/student_teams/_send_invitations.html.erb
- app/views/student_teams/_team_display.html.erb
- app/views/student_teams/edit.html.erb
- app/views/student_teams/view.html.erb
- app/views/assignments/edit/_calibration.html.erb
- app/views/bookmarks/list.html.erb
- app/views/popup/participants_popup.html.erb
- app/views/reports/_teammate_review_report.html.erb
- app/views/response/response.html.erb
- app/views/review_mapping/_list_review_mappings.html.erb
- app/views/sign_up_sheet/show_team.html.erb
- app/views/submitted_content/_hyperlink.html.erb
- app/views/submitted_content/_self_review.html.erb
- app/views/teams_users/list.html.erb
- app/views/tree_display/actions/_teams_actions.html.erb
- app/views/tree_display/actions/_teams_users_actions.html.erb
Config:
- config/locales/en_US.yml
- config/routes.rb
db:
- db/migrate/20240324042615_rename_teams_users_to_teams_participants.rb
- db/migrate/20240324043736_add_participant_id_to_teams_participants.rb
- db/migrate/20240324044135_populate_participant_id_in_teams_participants.rb
- db/migrate/20230415194444_add_participant_id_and_populate.rb
- db/migrate/20220405141744_add_pair_programming_status_to_teams_users.rb
- db/migrate/20211110161054_add_duty_id_to_teams_users.rb
- db/migrate/20141111010259_add_index_to_response_maps.rb
- db/migrate/20150527105416_convert_all_individual_assignments_to_one_member_team_assignments.rb
- db/migrate/20141204022200_add_team_index_to_teams_users.rb
- db/migrate/20131103014327_create_participant_score_views.rb
- db/migrate/102_standardize_review_mappings.rb
- db/migrate/066_create_team_user_nodes.rb
- db/migrate/019_create_teams_users.rb
- db/migrate/002_initialize_custom.rb
- db/migrate_save/020_teams_users.rb
- db/schema.rb
spec:
- spec/controllers/student_teams_controller_spec.rb
- spec/controllers/teams_participants_controller_spec.rb
- spec/controllers/teams_users_controller_spec.rb
- spec/controllers/users_controller_spec.rb
- spec/controllers/suggestion_controller_spec.rb
- spec/controllers/sign_up_sheet_controller_spec.rb
- spec/controllers/review_mapping_controller_spec.rb
- spec/controllers/pair_programming_controller_spec.rb
- spec/controllers/lottery_controller_spec.rb
- spec/controllers/invitations_controller_spec.rb
- spec/controllers/grades_controller_spec.rb
- spec/controllers/assessment360_controller_spec.rb
- spec/controllers/airbrake_exception_errors_controller_tests_spec.rb
- spec/factories/factories.rb
- spec/features/assignment_team_member_spec.rb
- spec/helpers/grades_helper_spec.rb
- spec/models/assignment_spec.rb
- spec/models/assignment_team_spec.rb
- spec/models/course_spec.rb
- spec/models/course_team_spec.rb
- spec/models/course_team_spec.rb
- spec/models/team_spec.rb
- spec/models/participant_spec.rb
- spec/models/tag_prompt_deployment_spec.rb
- spec/models/sign_up_sheet_spec.rb
- spec/models/review_response_map_spec.rb
- spec/models/invitation_spec.rb
- spec/models/cake_spec.rb
- spec/models/assignment_participant_spec.rb
- spec/models/airbrake_expection_errors_unit_tests_spec.rb
Test Plan
Manual Testing
After implementing changes in 'teams_users', it's essential to reassess the flows and user stories involving its usage. This entails manually examining various pathways through which a team for an assignment can be created in Expertiza, along with associated operations like removing a user from a team, adding a new user, deleting a team, and so forth. This testing phase is crucial to verify that Expertiza's functionality remains intact and operates as expected post-refactoring.
The primary user stories we examined include: Student Creates a Team:
- First, the student navigates to the assignments section in their view.
- Then, they select a specific assignment and access the "Your Team" page.
- On this page, they input a team name and confirm by clicking the "Name Team" button.
Student Sends Invitations to Other Students:
- After successfully creating a team, the student proceeds to send invitations to other students.
- They input the usernames of the desired recipients and send invitations by clicking on the "Invite" button.
Student Joins a Team:
- In the recipient student's Expertiza view, they will see a notification or invitation indicating that they have been invited to join a team.
- They navigate to the relevant assignment and locate the invitation.
- Finally, they accept the invitation to join the team.
Automated RSpec Testing
As this is a refactoring project, the testing scope is confined to validating the changes made during the refactoring process. Additionally, it involves ensuring that the existing test cases remain intact and are adjusted accordingly to accommodate any modifications.GitHub Commit
The `Team` class encapsulates functionalities related to managing teams in the context of assignments or courses. It defines methods for handling team participants, checking for existing teams, generating unique team names, and importing/exporting team data. The provided code includes extensive test cases to ensure the reliability and correctness of these methods. Test blocks such as `describe '#participant'` validate the retrieval of participants for a team, while `describe '.check_for_existing'` verifies the behavior of checking for existing teams within a specific assignment context. Additionally, the class includes methods for importing teams from external sources, handling duplicates, and managing team names. Overall, the `Team` class provides a robust framework for managing teams within the application, with thorough testing to ensure its functionality in various scenarios.
The `Assignment` class is thoroughly tested through a series of test cases, validating its functionalities in various scenarios. Tests include checking default values, verifying team assignments, examining topic presence, and ensuring proper course assignment. Methods for managing teams, handling rubrics, and assigning reviewers dynamically are also rigorously tested. Error handling for review configurations and metareviewer assignments is validated, ensuring robustness. Overall, these tests offer comprehensive coverage of the `Assignment` class, guaranteeing its reliability and correctness in different contexts and configurations.
The test suite for the `AssignmentTeam` model in the provided code is extensive, covering a wide range of functionalities and scenarios. It validates key aspects such as hyperlinks submission, participant inclusion, deletion of teams, import/export operations, and more. Each test case meticulously examines different conditions to ensure the model's correctness and robustness. From verifying the presence of hyperlinks to assigning reviewers and checking submissions, the tests provide comprehensive coverage. Additionally, the test suite demonstrates adherence to best practices in software testing, including clear setup, execution, and verification steps. Overall, these tests play a crucial role in maintaining the reliability and stability of the `AssignmentTeam` model within the Expertiza application.
The verification process extended to testing scenarios such as adding a new team member when the team was already full, removing and subsequently re-adding the same user, and sending invitations to users already part of the team. This ensured that the team management feature in Expertiza functioned reliably and consistently across diverse user interactions and situations.
Bonus Task
Issue:
The Team information page allows the same user to be added multiple times, likely due to instructors adding a member already invited by a student.
Recreation of the issue:
- Instructor adds a member already invited by a student.
- Student invites a member who is subsequently added by the instructor.
Primary methods to create a team within the Expertiza platform
BY INSTRUCTOR:
- Instructors have the authority to create teams through the "Manage" tab.
- They can access this option by navigating to the specific course and clicking on the "Add Teams" feature.
- This allows instructors to directly form teams and assign students to them.
BY STUDENT:
- Students can initiate the team creation process through the "Assignments" tab.
- Within this section, students can select any assignment and proceed to the "Your Team" subsection.
- Here, they have the option to create a new team, providing a team name and possibly inviting other students to join.
Resolution
- Identification of Issue Source:
First, we need to identify the root cause of the problem. This may involve tracing the code paths where team creation occurs and pinpointing where duplicate entries are being allowed.
- Validation Checks:
Implement validation checks to prevent the addition of duplicate users to a team. This can be done by: Checking if the user is already a member of the team before adding them again. Verifying if the user has already received an invitation to join the team before sending another invitation. Ensuring that each user can only be added to the team once.
- Error Handling:
Provide appropriate error messages or notifications to users when they attempt to add a user who is already part of the team. This will inform them of the issue and prevent accidental duplication.
- Refinement of Team Creation Workflow:
Review the team creation workflow to streamline the process and minimize the likelihood of duplicate entries. This may involve simplifying the interface, providing clearer instructions, or implementing additional safeguards.
- Testing and Validation:
Thoroughly test the updated team creation functionality to ensure that duplicate entries are properly prevented. Test various scenarios, including adding existing team members, inviting users who have already been invited, and attempting to add users who are already part of the team.
While some solutions, such as displaying a message when a user is already a team member, have already been implemented, there are still further changes required to fully address the issue.
Team Members
- Meghana Chowdary Ainampudi (mainamp@ncsu.edu)
- Sai Abhigna Tummala (stummal5@ncsu.edu)
- Parshav Gandhi (pjgandh3@ncsu.edu)
Mentor: Ananya Mantravadi
Code Details
Github Pull Request - https://github.com/expertiza/expertiza/pull/2770
Repo - https://github.com/abhigna98/expertiza
Demo Link - https://youtu.be/s_v-sAGq_l4