CSC/ECE 517 Fall 2024 - E2452. Refactor review mapping controller.rb

Expertiza Background

The open-source project Expertiza is built using Ruby on Rails, maintained by both the staff and students at NC State. This platform grants instructors complete authority over managing class assignments. Expertiza is packed with versatile tools to support various types of assignments, including options for peer evaluations, forming groups, and adding topics. For a comprehensive overview of all that Expertiza provides, visit the Expertiza wiki.

About Controller

The `review_mapping_controller` function is responsible for assigning reviewers to specific assignments. This controller plays a key role in allocating reviews across different student groups or individuals, handling both peer and self-assessment scenarios. Additionally, it manages student requests for reviews and facilitates extra bonus reviews that align with the assignment's requirements.

Functionality of review_mapping_controller

The `review_mapping_controller` is a vital part of a system dedicated to managing the assignment and allocation of reviewers for different types of evaluations, such as peer reviews and self-assessments. Its main role is to coordinate the assignment of these reviews across individual students or various groups, using an intricate process that factors in elements like fairness, diverse perspectives, and alignment with assignment criteria. Through this coordination, the controller ensures that every participant receives a thorough and balanced assessment, supporting the overall quality and reliability of the evaluation process.

In addition, the `review_mapping_controller` is essential for handling student requests for additional assessments, often driven by needs for extra feedback, additional credit opportunities, or reevaluation. When processing these requests, the controller must adhere to assignment guidelines and carefully evaluate the feasibility of granting extra assessments without affecting the integrity or fairness of the review process. This may involve accounting for practical constraints, such as reviewer availability and workload distribution, to ensure an equitable experience for all participants.

Problem Statement

The `review_mapping_controller` poses challenges due to its extensive size, complexity, and lack of comments, which make it hard for developers to fully understand its functions. To improve this, a thorough refactoring is recommended to divide lengthy methods into smaller, more understandable sections. This approach would break down complex logic into modular parts, each handling a particular task within the controller’s scope. Additionally, refactoring should focus on eliminating redundant code, consolidating repeated segments into reusable functions or utilities to improve code quality and minimize errors. By reorganizing the controller code and enhancing its documentation, developers can gain a clearer insight into its functionality, simplifying maintenance, troubleshooting, and potential updates.

Tasks

1. Refine Extensive Methods: Methods like `assign_reviewer_dynamically`, `add_reviewer`, `automatic_review_mapping`, and `peer_review_strategy` will be further broken down for greater clarity and efficiency.

2. Clarify Variable Names: Variables will be renamed to make their roles more transparent, enhancing readability and comprehension.

3. Use Subclass Methods Instead of Switch Statements: Existing switch statements will be replaced with subclass methods, contributing to a more modular and scalable code structure.

4. Introduce Subclass Models: New models for subclasses will be implemented to streamline organization and simplify management.

5. Eliminate Hardcoded Parameters: Any hardcoded parameters will be removed to increase adaptability across different use cases.

6. Enhance Comments: Additional comments will be included where helpful, and any vague or redundant comments will be clarified or removed.

7. Expand Test Coverage: The test suite will be extended to ensure thorough testing, particularly following the refactoring process.

Phase 1

1. Restructure the `assign_reviewer_dynamically` function to enhance its clarity and modular design.
2. Revise and improve the tests related to the `assign_reviewer_dynamically` function.
3. Simplify the `add_reviewer` function by breaking down its complex logic into smaller, more manageable components.
4. Update and broaden the tests associated with the `add_reviewer` function to ensure proper functionality.
5. Refine existing comments and introduce new ones to clarify intricate parts of the code.
6. Change variable names to make their purposes clear, allowing for immediate understanding.
7. Tackle and fix Code Climate issues to produce cleaner and more efficient code.
8. Obtained assignment details using the assignment ID, ensuring that an error message is displayed if a user is invalid (not found in the database), and that reviewer assignments are only made if the reviewer is confirmed as part of the assignment.
9. Collected participant details when a user was added to an assignment.

Test Cases

1. Verify that `assign_reviewer_dynamically()` correctly assigns a reviewer based on defined criteria.
2. Check that `add_reviewer()` successfully adds a reviewer to the system.
3. Ensure `automatic_review_mapping()` maps submissions to reviewers automatically.
4. Validate the logic of `peer_review_strategy()` for peer reviewer assignments.
5. Ensure all variables have been renamed according to the new naming conventions.
6. Confirm that all switch statements have been replaced with subclass methods.
7. Verify that hardcoded parameters have been removed and replaced with configurable options.
8. Check for meaningful comments and the removal of redundant or unclear comments.
9. Assess whether the test suite covers all critical functions and edge cases.
10. Test dynamic reviewer assignment for edge cases, such as no available reviewers.

Implementation

Restructuring assign_reviewer_dynamically

Original Code

The assign_reviewer_dynamically function contained multiple responsibilities, making it lengthy and challenging to follow.

Updated Code

The function was refactored to focus on specific tasks, such as:

• Validating user eligibility
• Fetching assignment details
• Assigning a reviewer only if they are confirmed as part of the assignment

Revising Tests for assign_reviewer_dynamically

Original Code

• The tests related to the assign_reviewer_dynamically function were limited and did not cover all scenarios.

Updated Code

• The tests have been improved and broadened to cover:
• Valid user scenarios
• Invalid user scenarios (e.g., user not found in the database)

Simplifying add_reviewer

Original Code

• The add_reviewer function had complex logic that hindered readability and maintainability.

Updated Code

• This function was simplified by breaking it down into smaller, manageable components that handle Parameter Validation, Sending notifications

Updating Tests for add_reviewer

Original Code

• The existing tests for add_reviewer were insufficient to ensure the function worked correctly in all situations.

Updated Code

Tests were expanded to include:

• Successful addition of reviewers
• Handling of invalid inputs

Refining Comments and Documentation

Original Code

• Comments in the original code were sparse and often unclear.

Updated Code

Descriptive comments were added throughout the code to explain:

• The purpose of each method
• The logic behind complex code sections

Changing Variable Names

Original Code

• Variable names were generic and did not convey their purposes.

Updated Code

• Variable names have been updated to be more descriptive, making their purposes immediately clear.

Design Document

Current Status

The project has been refactored to improve code readability, modularity, and maintainability. This refactoring has focused on breaking down complex methods into smaller, single-purpose functions, enhancing error handling, and reducing redundant code through reusable helper methods. These changes align with Ruby best practices and the SOLID principles, making the code easier to understand, extend, and maintain.

Highlights of the Refactored Controller:

• Improved modularity with clearly defined helper methods.
• Consistent error handling and validation checks for invalid user assignments and outstanding reviews.
• Reduction in redundant code by centralizing frequently used logic, enhancing the controller's readability and scalability.

Key Changes

1. Modularization of Functions:

• Decomposition of Methods: Long methods such as add_reviewer, assign_reviewer_dynamically, and automatic_review_mapping were broken down into smaller helper methods, e.g., get_participant_or_reviewer, get_review_response_mapping, assign_reviewer_to_contributor, check_for_self_review, and others.
• Guard Clauses: Used guard clauses in methods like assign_reviewer_dynamically to reduce nesting and exit early when conditions are not met, improving clarity.

2. Extraction of Reusable Components:

• Topic and Team Assignment: The logic for assigning reviewers based on topics and for handling teams with or without topics was extracted into assignment_with_topics and assignment_with_no_topics methods, following SRP.
• Dynamic Review Assignment: Introduced modular helper functions (assign_reviewer_based_on_topics, assign_topics_to_reviewer, and assign_team_to_review) to assign topics and teams dynamically, making the main controller logic clearer and more concise.

3. Error Handling and Validation Improvements:

• Error Feedback and Edge Case Handling: Added validations to handle cases where a user or reviewer does not exist or has outstanding reviews, providing informative error messages to users.
• Refined Reviewer Selection Logic: Prevented users from assigning themselves to review their own work (check_for_self_review) and introduced conditions to verify reviewer assignments before proceeding.

4. Automated Review Mapping:

• Helper Methods: Introduced handle_standard_review_mapping and handle_calibrated_and_uncalibrated_artifacts for handling scenarios with or without calibrated artifacts, streamlining the process for assigning review tasks based on specific review needs.
• Dynamic Strategy Application: Created a strategy-based approach (automatic_review_mapping_strategy) to handle review mapping dynamically based on student and submission count, enhancing the flexibility of review assignment.

Principles Applied

• Single Responsibility Principle (SRP): Each method now has a single responsibility, with helper methods extracted for distinct actions.
• Open/Closed Principle: The refactored code can be extended with additional helper functions or strategies without modifying existing methods.
• DRY (Don't Repeat Yourself): Repeated code segments, especially for participant/reviewer/team assignment, were extracted into reusable methods, reducing redundancy.
• Error Handling: Improved resilience by checking for conditions like invalid users, duplicate reviewer assignments, and outstanding reviews.

Code References

1. Single Responsibility Principle (SRP):

• Decomposition of Long Methods: Large methods were divided to ensure each method has a single responsibility. For example:

assign_reviewer_dynamically was split into assign_reviewer_based_on_topics, assignment_with_topics, and assignment_with_no_topics, isolating the logic for dynamic assignment with or without topics.

• add_reviewer was decomposed to include helper methods like assign_reviewer_to_contributor, get_participant_or_reviewer, and check_for_self_review, separating concerns within the reviewer addition process.
• Error Checks: Methods like check_invalid_topic and check_for_self_review handle specific validation tasks, isolating logic to keep main functions focused on their core

operations.

2. Open/Closed Principle:

• Extensible Review Strategies: The assign_reviewer_based_on_topics method allows for adding new assignment strategies without modifying the core method, promoting extensibility.

Dynamic Review Assignment Strategy: Helper methods like assignment_with_topics and assignment_with_no_topics make it easy to introduce additional criteria or rules for assigning topics or teams to reviewers without altering existing logic.

• Automatic Review Mapping Strategy: The automatic_review_mapping_strategy allows flexibility to handle multiple review scenarios (student reviews, submission reviews) by passing parameters, making it easy to adjust the review strategies as needed without modifying the core controller code.

3. DRY (Don't Repeat Yourself):

• Extraction of Common Code: Common logic for fetching assignment, participant, and review mappings was consolidated:
• Methods like get_participant_or_reviewer, get_assignment, and get_review_response_mapping centralize database queries, reducing redundancy in places like add_calibration, assign_reviewer_dynamically, and automatic_review_mapping.
• Review Mapping Logic: Shared review assignment logic across multiple methods, such as assign_topics_to_reviewer and assign_team_to_review, was centralized to avoid repeating topic and team checks for different review paths.
• Error Messages and Flash Notices: Error handling messages, such as those for insufficient or outstanding reviews, were standardized across methods to avoid redundant inline error logic.

4. Enhanced Error Handling:

• Guard Clauses: Guard clauses were added to methods such as assign_reviewer_dynamically and add_reviewer to handle errors early and avoid nested conditional logic. For instance, assign_reviewer_dynamically checks for valid topics and outstanding reviews at the beginning, setting flash messages if conditions are not met.
• Invalid User/Reviewer Checks: In add_reviewer, the method now verifies user existence, preventing invalid users from being assigned. This is handled by checking user_id.nil? and setting flash[:error] if the user is not found.
• Duplicate Assignments: In assign_reviewer_to_contributor, a check for existing reviewer assignments was added to prevent duplicate entries, raising an error if a reviewer is already assigned to the contributor.

Future Scope

A comprehensive testing strategy is being implemented for the `ReviewMappingController` to ensure functionality, reliability, and scalability. This approach includes well-defined test categories and a systematic execution process to rigorously validate both new and existing features. This enhances the controller’s performance, adaptability, and overall quality, providing a solid foundation for current operations and future growth.

General Enhancements and Best Practices

• FactoryBot for Model Setup: Necessary models, including ReviewGrade, Instructor, AssignmentTeam, and others, will be defined in FactoryBot factories. This approach will reduce redundant setup code, improve test readability, and facilitate maintenance and extension.
• Mocking and Spying: RSpec’s allow and expect methods will be utilized to mock dependencies, isolating the logic under test, enhancing reliability, and expediting execution.
• Comprehensive Edge Case Testing: Each method will be tested for edge cases such as empty collections, nil values, and invalid parameters to ensure the controller’s resilience against unexpected inputs.
• Error Handling: Error scenarios will be specifically addressed, particularly cases involving database errors or missing records, to validate the controller’s resilience and stability.
• Flash Message Validation: Tests will be designed to confirm that all flash messages, for both success and failure scenarios, provide accurate and informative feedback to the user.

Test Case Categories and Example Scenarios

Test cases will be organized into the following categories to ensure comprehensive coverage of all functionality in the ReviewMappingController:

1. Validation and Error Handling

• automatic_review_mapping with Invalid Parameters: Test cases will cover scenarios involving missing parameters, negative values (e.g., num_reviews_per_student set to -1), and conflicting options (e.g., both num_reviews_per_student and num_reviews_per_submission provided). Invalid parameters should result in appropriate flash messages, preventing further execution.
• Edge Cases for Flash Messages: Flash messages will be tested to ensure they accurately reflect outcomes, such as when attempting to delete an already-deleted review or submitting an invalid review assignment.

2. State Transitions

• Duplicate Self-Review Prevention in start_self_review: Tests will confirm that the controller prevents duplicate self-review mappings, redirecting the user with a message indicating an existing self-review assignment if applicable.
• New Review Mappings via automatic_review_mapping: Tests will verify that new review mappings are correctly created and that the system state is updated to include these mappings. Expected outcomes include proper redirection and flash messages upon successful execution.

3. Flash Messages & Redirects

• Successful Deletion in delete_metareview: Tests will confirm that the correct success flash message is displayed when a metareview is deleted and that the user is redirected appropriately.
• Forced Deletion After Failure in delete_metareview: Forced deletion cases will be tested to ensure that relevant flash messages are displayed, providing error details and notifying the user of the force option.

4. Boundary and Edge Conditions

• High Volume Handling in automatic_review_mapping: Test cases simulating large numbers of teams and review requests (e.g., hundreds) will validate that the method can handle scalability demands. These cases will focus on performance and stability under load.
• Empty Team List in automatic_review_mapping: Tests will cover scenarios where no teams are available but review parameters are valid. The system should handle these cases gracefully, displaying relevant flash messages regarding the lack of available teams.

Test Execution Process

The following structured process will guide the execution of test cases to ensure comprehensive and consistent coverage:

1. Mock Dependencies: RSpec’s allow and expect methods will be used to mock dependencies, such as external services and database queries, allowing each test to focus solely on the controller’s logic.

2. Analyze Test Coverage: Each test case will be assessed to ensure it covers critical functionality, focusing on edge cases, error handling, and user interactions. Validation, state transitions, flash messages, redirects, and boundary conditions will all be reviewed for completeness.

3. Run Tests Using RSpec: The test suite will be executed to verify expected outcomes, including flash messages, redirects, and state transitions.

4. Evaluate Outcomes:

• Success: Confirm that flash messages, redirects, and state transitions function as expected.
• Failure: Verify that errors and edge cases are handled gracefully, providing relevant feedback to the user.

Implemented Screenshots

Test Case and code coverage

Refactored assign_reviewer_dynamically to reduce nesting, improve readability, and streamline error handling with guard clauses and modularized logic.

Refactored `add_reviewer` to reduce complexity by extracting reviewer assignment logic into `assign_reviewer_to_contributor`, simplifying the method and improving readability through guard clauses for error handling.

Refactored `automatic_review_mapping` by extracting complex logic for standard, calibrated, and uncalibrated artifact mapping into helper methods (`handle_standard_review_mapping` and `handle_calibrated_and_uncalibrated_artifacts`), reducing method length and improving readability. Guard clauses were introduced for early exit conditions, and variable assignments were streamlined to ensure single responsibility and modularity, making the method more maintainable and readable.

Team

Mentor

• Nainisha Bhallamudi

Members

• Gauri Verma
• Vaibhav Hawaldar
• Raj Kalantri

References

1. Expertiza
2. OSS Projects on Expertiza
3. Github
4. Pull Request
5. VCL Server