CSC/ECE 517 Spring 2024 - E2403 Mentor-Meeting Management: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
(Created page with "== Introduction == <nowiki>The User model is a crucial component of the Expertiza application, managing user data and authentication/authorization. It has 9 class methods, 23 instance methods, and is linked with other models for personalized user experience. The user controller is smaller but could benefit from refactoring and improved comments.</nowiki> ==Files Changes== #app/models/user.rb #app/models/instructor.rb #app/models/ta.rb #app/models/superadministrator.rb...")
 
No edit summary
Line 1: Line 1:
== Introduction ==
This wiki page describes changes made under the E2407 OODD assignment for Spring 2024, CSC/ECE 517.
__TOC__
== Expertiza Background==
Ruby on Rails is used in the development of the open-source project Expertiza. The NC State staff and students are in charge of maintaining this online application. With this program, the teacher has total control over the tasks in their class. Expertiza is a feature-rich program that can manage any kind of assignment, with features like peer reviews, group creation, and subject addition. Go to the Expertiza wiki to find out more about all of the features that Expertiza offers.


<nowiki>The User model is a crucial component of the Expertiza application, managing user data and authentication/authorization. It has 9 class methods, 23 instance methods, and is linked with other models for personalized user experience. The user controller is smaller but could benefit from refactoring and improved comments.</nowiki>
== About Controller ==
'''review_mapping_controller''' function is to map the reviewer to an assignment. In essence, the controller manages the distribution of reviews among various groups or individual student users, addressing situations such as peer and self-evaluations. Furthermore, the controller is essential in responding to student users' requests and enabling additional bonus reviews that comply with the assignment criteria.


==Files Changes==
#app/models/user.rb
#app/models/instructor.rb
#app/models/ta.rb
#app/models/superadministrator.rb
#app/controller/user_controller.rb


== Test Plan==
== Functionality of review_mapping_controller ==
<nowiki>Since this is a refactoring project no new features were introduced and any modifications made were focused on refactoring the code and eliminating redundant elements. The main objective of our task was to refactor the user-related methods so that the existing tests pass. New tests have been created, and they along with existing tests run successfully indicating that the code still functions properly.</nowiki>
The review_mapping_controller serves as a critical component within a system designed to manage the assignment mapping and allocation of reviewers for various types of assessments, such as peer reviews and self-assessments. Its primary function revolves around orchestrating the distribution of these reviews, whether they are assigned to multiple teams or to individual student users. This entails a sophisticated algorithmic process that takes into account factors such as fairness, diversity of perspectives, and adherence to assignment guidelines. By controlling the assignment of reviews, the controller ensures that each participant receives a balanced and constructive evaluation of their work, contributing to the overall integrity and effectiveness of the assessment process.


==Modifications made==
Furthermore, the review_mapping_controller plays a pivotal role in handling student requests for additional bonus assessments. These requests may arise due to various reasons such as a desire for more feedback, a pursuit of extra credit, or a need for reassessment. In responding to such requests, the controller must maintain alignment with the established guidelines and constraints of the assignments. This involves evaluating the feasibility of accommodating extra assessments without compromising the integrity or fairness of the evaluation process. Additionally, the controller may need to consider resource constraints, such as the availability of reviewers and the workload distribution among them.
# Refactor can_impersonate? method in user.rb
# Refactor recursively_parent_of? method by shorting
# Refactor teaching_assistant_for? method by shorting
# Refactor validate methods in user.rb
# Removed unused method salt_first?
# Bug fix in teaching_assistant_for? method
# Remove unused method creator_of? in user.rb
# Refactor get_user_list method in instructor.rb
# Removed unused scope in user.rb
# Removed redundant delete calls in user_controller#destroy
# Removed params.permit! by passing required params in user_params
# Refactor user_controller#show_if_authorized
# Refactor user_controller#create method
# Extracted anonymized methods from user.rb to anonymized_helpers.rb
# Renamed get_available_users to get_visible_users_with_lesser_roles to
# Replace delegate methods in user.rb to follows same method style
# Refactor yesorno method in UserHelper method
# Rename method foreign to get_roles


== Description of the Refactored Changes ==
'''Rename method foreign to get_roles: '''


Renamed the method foreign in a precise manner so that it describes the functionality of the method, as the method foreign gets the role of the User.
== Problem Statement ==
<br>
The review_mapping_controller presents a challenge due to its length, complexity, and sparse comments, making it difficult for developers to grasp its functionality efficiently. To address this, the controller should undergo a comprehensive refactoring process aimed at breaking down lengthy methods into smaller, more manageable pieces. This entails decomposing intricate logic into modular components, each responsible for a specific task or subtask within the overall functionality. Moreover, the refactoring effort should target instances of code duplication, consolidating repeated code segments into reusable functions or utility methods to enhance maintainability and reduce the risk of errors. By systematically restructuring the controller codebase and improving its documentation, developers can gain a clearer understanding of its inner workings, facilitating easier maintenance, debugging, and future enhancements.
[[File:pic1_dshah6.png| 1000px]]
<br>


'''Refactor yesorno method in UserHelper method: '''
== Tasks ==
Removed redundant lines of code in the yesorno method and made changes in the code in such a way that the code looks cleaner and describable.
-Refactor the long methods in review_mapping_controller.rb like assign_reviewer_dynamically, add_reviewer, automatic_review_mapping, peer_review_strategy, etc.
<br>
-Rename variable names to convey what they are used for.<br/>
[[File:pic2.png| 1000px]]
-Replace switch statements with subclasses methods.<br/>
<br>
-Create models for the subclasses.<br/>
To make it more concise, we added a return statement for the functionality of the method.  
-Remove hardcoded parameters.<br/>
<br>
-Add meaningful comments and edit/remove/do not unnecessary comments.<br/>
[[File:pic3_dshah6.png| 1000px]]
-Try to increase the test coverage.<br/>
<br>


'''Replace delegate methods in user.rb to follows same method style: '''
=== Phase 1 ===
Replaced all the delegate methods in the user model to make every function of the model to be in the similar fashion. This change increases the code readability.
For Phase 1 of the project we have focused working on the below mentioned issues.<br/>
<br>
-Refactor assign_reviewer_dynamically function<br/>
[[File:pic4_dshah6.png| 1000px]]
-Corresponding changes to the tests for assign_reviewer_dynamically<br/>
<br>
-Refactor add_reviewer function<br/>
-Corresponding changes to the tests for add_reviewer<br/>
-Correct comments and add additional comments<br/>
-Methods are named descriptively to indicate their purpose<br/>
-Fixed Code climate issues<br/>


'''Renamed get_available_users to get_visible_users_with_lesser_roles to: '''
=== Phase 2 ===
Replaced all the occurrences of the ‘get_available_users’ to ‘get_visible_users_with_lesser_roles’  as the method is obtaining results of the users with roles and not the available users. This change makes the function more descriptive on what the method is returning.
For Phase 2 of the project we plan working on the below mentioned issues.<br/>
<br>
-Refactor automatic_review_mapping function<br/>
[[File:pic5_dshah6.png| 1000px]]
-Refactor peer_review_strategy function<br/>
<br>
-Replace switch statements with subclasses methods<br/>
-Increase the test coverage<br/>
-Remove hardcoded parameters<br/>
-Create models for the subclasses<br/>


'''Extracted anonymized methods from user.rb to anonymized_helpers.rb: '''
== Implementation ==
Created a separate module named AnonymizedHelper with the details extracted from the anonymized view in the user model and then included the module in the user model for the extraction of the anonymized view from the model.
<br>
[[File:pic6_dshah6.png| 1000px]]
<br>


'''Refactor user_controller#show_if_authorized:'''
=== Phase 1 ===
Refactored the user controller method by removing redundant statements.
<br>
[[File:pic7_dshah6.png| 1000px]]
<br>


==== add_reviewer ====
The changes made to the `add_reviewer` method can be checked in the commit - https://github.com/NidhayPancholi/expertiza/commit/b325810d67da2a03d3ccb15458926d0049fdb9eb. The changes are described descriptively below.:<br/>


'''Refactor user_controller#create method:'''
- Refactored the `add_reviewer` method to focus on single tasks per method, enhancing code readability and maintainability.<br/>
Refactored the create method in the user_controller by adding appropriate functions separately rather including many lines of code into the single functions and enhancing the readability of the method.
- Extracted the functionality to find a user's ID by name into a separate method named `find_user_id_by_name`.<br/>
<br>
- Separated the logic to check if a user is trying to review their own artifact into its own method named `user_trying_to_review_own_artifact?`.<br/>
[[File:pic8_dshah6.png| 1000px]]
- Abstracted the process of assigning a reviewer to the assignment into a method named `assign_reviewer`.<br/>
<br>
- Created a method named `registration_url` to generate the registration URL for the assignment based on provided parameters.<br/>
- Divided the code to create a review response map into a separate method named `create_review_response_map`.<br/>
- Extracted the logic to redirect to the list mappings page after adding the reviewer into its own method named `redirect_to_list_mappings`.<br/>
- Added descriptive comments to each method to explain its purpose and functionality clearly.<br/>


'''Removed redundant delete calls in user_controller#destroy:'''
==== assign_reviewer_dynamically ====
Removed all the redundant lines of code from the destroy method in the user controller.
The changes made to the `assign_reviewer_dynamically` method can be checked in the commit - https://github.com/NidhayPancholi/expertiza/commit/30a3625d4188e56a58e4b6472c52b60bbfb83df5. The changes are described descriptively below:<br/>
<br>
[[File:pic9_dshah6.png| 1000px]]
<br>


'''Removed params.permit! by passing required params in user_params:'''
- Restructured the `assign_reviewer_dynamically` method to perform single tasks per method, improving code organization and readability.<br/>
Removed the params.permit! statement and included them while passing the user_params.
- Extracted the functionality to find the assignment participant into a separate method called `find_participant_for_assignment`.<br/>
<br>
- Abstracted the logic to handle errors when no topic is selected into a method named `topic_selection_error?`.<br/>
[[File:pic10_dshah6.png| 1000px]]
- Created a method named `dynamically_assign_reviewer` to handle the process of dynamically assigning a reviewer based on the assignment type.<br/>
<br>
- Separated the logic to assign a reviewer when the assignment has topics into a method named `assign_reviewer_with_topic`.<br/>
- Developed a method called `select_topic_to_review` to handle the selection of a topic for review.<br/>
- Extracted the logic to assign a reviewer when the assignment has no topics into a method named `assign_reviewer_without_topic`.<br/>
- Created a method named `select_assignment_team_to_review` to handle the selection of an assignment team for review.<br/>
- Abstracted the process to redirect to the student review list page into a method called `redirect_to_student_review_list`.<br/>
- Added clear comments to each method to explain its purpose and functionality effectively.<br/>


'''Removed unused scope in user.rb:'''
==== Changes to the spec file ====
Removed all the unused redundant scope variables from the user model.  
The changes made to the test files are described below and can be found in the commit - https://github.com/expertiza/expertiza/commit/7c08070f0c2c000e64e55561b882e44fc81bc98f:<br/>
<br>
[[File:pic11_dshah6.png| 1000px]]
<br>


'''Refactor can_impersonate? method in user.rb:'''
- Updated the `ReviewMappingController` spec file.<br/>
Removed all the extra lines of code and refactored it to a single line statement to determine the can_impersonate user function.
- Added a test case in the `ReviewMappingController` spec file for the `add_reviewer` method to ensure correct behavior when a team user exists and `get_reviewer` method returns a reviewer.<br/>
<br>
- Adjusted the expectation in the `assign_reviewer_dynamically` test case to match the corrected error message in the controller. Specifically, removed the extra space from the expected error message to align with the actual error message generated by the controller.<br/>
[[File:pic12_dshah6.png| 1000px]]
- Ensured that all test cases are descriptive and cover the relevant scenarios for each method.<br/>
<br>
- Verified that the test cases accurately reflect the behavior of the controller methods after the code changes.<br/>


'''Refactor recursively_parent_of? method by shorting:'''
== Test Plan ==
Removed and deduced the code present in the recursively_parent function into a single statement.
We plan on adding more tests, increasing the test coverage in Project 4.
<br>
[[File:pic13_dshah6.png| 1000px]]
<br>


'''Refactor teaching_assistant_for? method by shorting:'''
In our Test-Driven Development (TDD) endeavors, our team would strategically adopt Mustafa Olmez's meticulously crafted test skeletons. These meticulously designed skeletons will serve as invaluable blueprints, delineating the essential tests necessary for our unit. By integrating these meticulously designed test skeletons into our workflow, we conduct comprehensive testing of our controller module. This method would enable us to thoroughly explore the functionality of our controller, ensuring meticulous examination and validation of its behavior. Incorporating these test skeletons will not only establish a sturdy framework for our unit tests but also elevate the overall quality and dependability of our codebase.
Reduce the code size of teaching_assistant_for? Function by using an or operator to combine two separate return statements.
<br>
[[File:pic14_dshah6.png| 1000px]]
<br>


'''Refactor validate methods in user.rb'''
'''Test Plan'''
Reduced the code size by combining multiple validation attributes written separately into one line validation containing all attributes.
<br>
[[File:pic15_dshah6.png| 1000px]]
<br>


'''removed unused method salt_first? In user.rb and user_spec.rb'''
Our Test Plan includes test for <code>review_mapping_controller.rb</code> file for the following functions:<br/>
Removed salt_first? Function in user.rb and its tests in user_spec.rb  as it doesn’t have any it is not used for any functionality of the app
<br>
[[File:pic16_dshah6.png| 1000px]]
<br>
<br>
[[File:pic17_dshah6.png| 1000px]]
<br>


'''bug fix in teaching_assistant_for? Method in user.rb'''
1. Test <code>`action_allowed?`</code> method:<br/>
Bug cause by the code written by us was fixed in this commit
  - Test when the action is <code>'add_dynamic_reviewer'</code>.<br/>
<br>
  - Test when the action is <code>'show_available_submissions'</code>.<br/>
[[File:pic18_dshah6.png| 1000px]]
  - Test when the action is <code>'assign_reviewer_dynamically'</code>.<br/>
<br>
  - Test when the action is <code>'assign_metareviewer_dynamically'</code>.<br/>
  - Test when the action is <code>'assign_quiz_dynamically'</code>.<br/>
  - Test when the action is <code>'start_self_review'</code>.<br/>
  - Test when the action is not any of the allowed actions for different roles.<br/>


2. Test <code>`add_calibration`</code> method:<br/>
  - Test when the participant is already assigned.<br/>
  - Test when the participant is not assigned.<br/>
  - Test when a calibration map already exists.<br/>
  - Test when a calibration map does not exist.<br/>
  - Test redirection to the response creation page.<br/>


'''remove unused method creator_of? in user.rb'''
3. Test <code>`select_reviewer`</code> method:<br/>
creator_of function is removed from user.rb as it doesn’t contribute to the functionality of app and its test in user_spec.rb are also deleted.
  - Test when called with a valid contributor_id.<br/>
<br>
  - Test when called with an invalid contributor_id.<br/>
[[File:pic19_dshah6.png| 1000px]]
<br>
<br>
[[File:pic20_dshah6.png| 1000px]]
<br>


'''refactor get_user_list method in instructor.rb '''
4. Test <code>`select_metareviewer`</code> method:<br/>
<br>
  - Test when given a valid response map id.<br/>
[[File:pic21_dshah6.png| 1000px]]
  - Test when given an invalid response map id.<br/>
<be>


== Additional Tests Added ==
5. Test <code>`add_reviewer`</code> method:<br/>
  - Test when the reviewer is not assigned to review their own artifact.<br/>
  - Test when the reviewer is assigned to review their own artifact.<br/>
  - Test when the reviewer is already assigned to the contributor.<br/>


6. Test <code>`assign_reviewer_dynamically`</code> method:<br/>
  - Test when a topic is selected and review is allowed.<br/>
  - Test when no topic is selected and review is allowed.<br/>
  - Test when no topic is selected and review is not allowed.<br/>
  - Test when a topic is selected and review is not allowed.<br/>
  - Test when there are no available topics to review.<br/>
  - Test when there are no available artifacts to review.<br/>
  - Test when the reviewer has reached the maximum number of outstanding reviews.<br/>


'''Tests added for yesorno method: '''
7. Test <code>`review_allowed?`</code> method:<br/>
  - Test when the reviewer has not reached the maximum number of reviews allowed for the assignment.<br/>
  - Test when the reviewer has reached the maximum number of reviews allowed for the assignment.<br/>


Added additional tests for yesorno function in User Helper file
8. Test <code>`check_outstanding_reviews?`</code> method:<br/>
<br>
  - Test when there are no review mappings for the assignment and reviewer.<br/>
[[File:tests_pic_1.png| 1000px]]
  - Test when there are review mappings for the assignment and reviewer, and all reviews are completed.<br/>
<be>
  - Test when there are review mappings for the assignment and reviewer, and some reviews are in progress.<br/>


9. Test <code>`assign_quiz_dynamically`</code> method:<br/>
  - Test when the reviewer has already taken the quiz.<br/>
  - Test when the reviewer has not taken the quiz yet.<br/>
  - Test when an error occurs during the assignment process.<br/>


'''Tests for show_if_authorized: '''
10. Test <code>`add_metareviewer`</code> method:<br/>
    - Test when a metareviewer is successfully added.<br/>
    - Test when the metareviewer is already assigned to the reviewer.<br/>
    - Test when an error occurs during the process.<br/>


Added additional tests for show_if_authorized function in user controller file. The test redirects to lists and sets a flash message if user doesn't exit
11. Test <code>`assign_metareviewer_dynamically`</code> method:<br/>
<br>
    - Test when there are reviews to Meta review.<br/>
[[File:tests_pic_2.png| 1000px]]
    - Test when there are no reviews to Meta review.<br/>
<be>
    - Test when an error occurs during assignment of metareviewer.<br/>


==Test Coverage==
12. Test <code>`get_reviewer`</code> method:<br/>
    - Test when the user is a participant in the assignment.<br/>
    - Test when the user is not a participant in the assignment.<br/>


Test coverage has been increased by 39.5% as it can be seen in the PR.
13. Test <code>`delete_outstanding_reviewers`</code> method:<br/>
    - Test when there are outstanding reviewers.<br/>
    - Test when there are no outstanding reviewers.<br/>


==Project Mentor==
14. Test <code>`delete_all_metareviewers`</code> method:<br/>
Ankur Mundra[[(amundra@ncsu.edu)]]
    - Test when there are metareview mappings to delete.<br/>
    - Test when there are unsuccessful deletes.<br/>
    - Test when there are no metareview mappings to delete.<br/>


==Contributors to this project==
15. Test <code>`unsubmit_review`</code> method:<br/>
    - Test when the response is successfully unsubmitted.<br/>
    - Test when the response fails to be unsubmitted.<br/>


Dhrumil Jignesh Shah [[(dshah6@ncsu.edu)]]
16. Test <code>`delete_reviewer`</code> method:<br/>
Keerthana Telaprolu [[(ktelapr@ncsu.edu)]]
    - Test when the review response map exists and there are no associated responses.<br/>
Saigirishwar Rohit Geddam [[(sgeddam2@ncsu.edu)]]
    - Test when the review response map exists but there are associated responses.<br/>
    - Test when the review response map does not exist.<br/>


==Relevant Links==
17. Test <code>`delete_metareviewer`</code> method:<br/>
GitHub PR [[https://github.com/expertiza/expertiza/pull/2525]]
    - Test when the metareview mapping exists.<br/>
    - Test when the metareview mapping does not exist.<br/>
 
19. Test <code>`list_mappings`</code> method.<br/>
 
20. Test <code>`automatic_review_mapping`</code> method.<br/>
 
21. Test <code>`automatic_review_mapping_strategy`</code> method.<br/>
 
22. Test <code>`automatic_review_mapping_staggered`</code> method.<br/>
 
23. Test <code>`save_grade_and_comment_for_reviewer`</code> method.<br/>
 
24. Test <code>`start_self_review`</code> method.<br/>
 
25. Test <code>`assign_reviewers_for_team`</code> method.<br/>
 
26. Test <code>`peer_review_strategy`</code> method.<br/>
 
27. Test <code>`review_mapping_params`</code> method.<br/>
 
== Design Pattern ==
During our code refactoring process, we leveraged various design patterns to enhance readability and maintainability. One commonly applied pattern was "Extract Method," where we identified lengthy and intricate methods and extracted segments of functionality into separate methods. This restructuring made the code more comprehensible and easier to grasp by isolating specific tasks within dedicated methods.
 
Additionally, we addressed the issue of excessive conditional statements by employing the "Refactoring Conditionals" design pattern. Instead of cluttering the code with numerous conditionals, we refactored by encapsulating the logic within these conditionals into distinct methods. By doing so, we streamlined the code's flow and improved its readability, making it clearer to understand the purpose and execution of each segment.
 
 
== Relevant Links ==
* '''Github Repository:''' https://github.com/NidhayPancholi/expertiza
* '''Pull Request:''' https://github.com/expertiza/expertiza/pull/2767
 
== Team ==
=== Mentor ===
* Ananya Mantravadi  (amantra)
 
=== Team Members ===
* Lagani Patel (lpatel)
* Nidhay Pancholi (nrpancho)
* Saloni Shah (sshah39)

Revision as of 15:56, 24 March 2024

This wiki page describes changes made under the E2407 OODD assignment for Spring 2024, CSC/ECE 517.

Expertiza Background

Ruby on Rails is used in the development of the open-source project Expertiza. The NC State staff and students are in charge of maintaining this online application. With this program, the teacher has total control over the tasks in their class. Expertiza is a feature-rich program that can manage any kind of assignment, with features like peer reviews, group creation, and subject addition. Go to the Expertiza wiki to find out more about all of the features that Expertiza offers.

About Controller

review_mapping_controller function is to map the reviewer to an assignment. In essence, the controller manages the distribution of reviews among various groups or individual student users, addressing situations such as peer and self-evaluations. Furthermore, the controller is essential in responding to student users' requests and enabling additional bonus reviews that comply with the assignment criteria.


Functionality of review_mapping_controller

The review_mapping_controller serves as a critical component within a system designed to manage the assignment mapping and allocation of reviewers for various types of assessments, such as peer reviews and self-assessments. Its primary function revolves around orchestrating the distribution of these reviews, whether they are assigned to multiple teams or to individual student users. This entails a sophisticated algorithmic process that takes into account factors such as fairness, diversity of perspectives, and adherence to assignment guidelines. By controlling the assignment of reviews, the controller ensures that each participant receives a balanced and constructive evaluation of their work, contributing to the overall integrity and effectiveness of the assessment process.

Furthermore, the review_mapping_controller plays a pivotal role in handling student requests for additional bonus assessments. These requests may arise due to various reasons such as a desire for more feedback, a pursuit of extra credit, or a need for reassessment. In responding to such requests, the controller must maintain alignment with the established guidelines and constraints of the assignments. This involves evaluating the feasibility of accommodating extra assessments without compromising the integrity or fairness of the evaluation process. Additionally, the controller may need to consider resource constraints, such as the availability of reviewers and the workload distribution among them.


Problem Statement

The review_mapping_controller presents a challenge due to its length, complexity, and sparse comments, making it difficult for developers to grasp its functionality efficiently. To address this, the controller should undergo a comprehensive refactoring process aimed at breaking down lengthy methods into smaller, more manageable pieces. This entails decomposing intricate logic into modular components, each responsible for a specific task or subtask within the overall functionality. Moreover, the refactoring effort should target instances of code duplication, consolidating repeated code segments into reusable functions or utility methods to enhance maintainability and reduce the risk of errors. By systematically restructuring the controller codebase and improving its documentation, developers can gain a clearer understanding of its inner workings, facilitating easier maintenance, debugging, and future enhancements.

Tasks

-Refactor the long methods in review_mapping_controller.rb like assign_reviewer_dynamically, add_reviewer, automatic_review_mapping, peer_review_strategy, etc. -Rename variable names to convey what they are used for.
-Replace switch statements with subclasses methods.
-Create models for the subclasses.
-Remove hardcoded parameters.
-Add meaningful comments and edit/remove/do not unnecessary comments.
-Try to increase the test coverage.

Phase 1

For Phase 1 of the project we have focused working on the below mentioned issues.
-Refactor assign_reviewer_dynamically function
-Corresponding changes to the tests for assign_reviewer_dynamically
-Refactor add_reviewer function
-Corresponding changes to the tests for add_reviewer
-Correct comments and add additional comments
-Methods are named descriptively to indicate their purpose
-Fixed Code climate issues

Phase 2

For Phase 2 of the project we plan working on the below mentioned issues.
-Refactor automatic_review_mapping function
-Refactor peer_review_strategy function
-Replace switch statements with subclasses methods
-Increase the test coverage
-Remove hardcoded parameters
-Create models for the subclasses

Implementation

Phase 1

add_reviewer

The changes made to the `add_reviewer` method can be checked in the commit - https://github.com/NidhayPancholi/expertiza/commit/b325810d67da2a03d3ccb15458926d0049fdb9eb. The changes are described descriptively below.:

- Refactored the `add_reviewer` method to focus on single tasks per method, enhancing code readability and maintainability.
- Extracted the functionality to find a user's ID by name into a separate method named `find_user_id_by_name`.
- Separated the logic to check if a user is trying to review their own artifact into its own method named `user_trying_to_review_own_artifact?`.
- Abstracted the process of assigning a reviewer to the assignment into a method named `assign_reviewer`.
- Created a method named `registration_url` to generate the registration URL for the assignment based on provided parameters.
- Divided the code to create a review response map into a separate method named `create_review_response_map`.
- Extracted the logic to redirect to the list mappings page after adding the reviewer into its own method named `redirect_to_list_mappings`.
- Added descriptive comments to each method to explain its purpose and functionality clearly.

assign_reviewer_dynamically

The changes made to the `assign_reviewer_dynamically` method can be checked in the commit - https://github.com/NidhayPancholi/expertiza/commit/30a3625d4188e56a58e4b6472c52b60bbfb83df5. The changes are described descriptively below:

- Restructured the `assign_reviewer_dynamically` method to perform single tasks per method, improving code organization and readability.
- Extracted the functionality to find the assignment participant into a separate method called `find_participant_for_assignment`.
- Abstracted the logic to handle errors when no topic is selected into a method named `topic_selection_error?`.
- Created a method named `dynamically_assign_reviewer` to handle the process of dynamically assigning a reviewer based on the assignment type.
- Separated the logic to assign a reviewer when the assignment has topics into a method named `assign_reviewer_with_topic`.
- Developed a method called `select_topic_to_review` to handle the selection of a topic for review.
- Extracted the logic to assign a reviewer when the assignment has no topics into a method named `assign_reviewer_without_topic`.
- Created a method named `select_assignment_team_to_review` to handle the selection of an assignment team for review.
- Abstracted the process to redirect to the student review list page into a method called `redirect_to_student_review_list`.
- Added clear comments to each method to explain its purpose and functionality effectively.

Changes to the spec file

The changes made to the test files are described below and can be found in the commit - https://github.com/expertiza/expertiza/commit/7c08070f0c2c000e64e55561b882e44fc81bc98f:

- Updated the `ReviewMappingController` spec file.
- Added a test case in the `ReviewMappingController` spec file for the `add_reviewer` method to ensure correct behavior when a team user exists and `get_reviewer` method returns a reviewer.
- Adjusted the expectation in the `assign_reviewer_dynamically` test case to match the corrected error message in the controller. Specifically, removed the extra space from the expected error message to align with the actual error message generated by the controller.
- Ensured that all test cases are descriptive and cover the relevant scenarios for each method.
- Verified that the test cases accurately reflect the behavior of the controller methods after the code changes.

Test Plan

We plan on adding more tests, increasing the test coverage in Project 4.

In our Test-Driven Development (TDD) endeavors, our team would strategically adopt Mustafa Olmez's meticulously crafted test skeletons. These meticulously designed skeletons will serve as invaluable blueprints, delineating the essential tests necessary for our unit. By integrating these meticulously designed test skeletons into our workflow, we conduct comprehensive testing of our controller module. This method would enable us to thoroughly explore the functionality of our controller, ensuring meticulous examination and validation of its behavior. Incorporating these test skeletons will not only establish a sturdy framework for our unit tests but also elevate the overall quality and dependability of our codebase.

Test Plan

Our Test Plan includes test for review_mapping_controller.rb file for the following functions:

1. Test `action_allowed?` method:

  - Test when the action is 'add_dynamic_reviewer'.
- Test when the action is 'show_available_submissions'.
- Test when the action is 'assign_reviewer_dynamically'.
- Test when the action is 'assign_metareviewer_dynamically'.
- Test when the action is 'assign_quiz_dynamically'.
- Test when the action is 'start_self_review'.
- Test when the action is not any of the allowed actions for different roles.

2. Test `add_calibration` method:

  - Test when the participant is already assigned.
- Test when the participant is not assigned.
- Test when a calibration map already exists.
- Test when a calibration map does not exist.
- Test redirection to the response creation page.

3. Test `select_reviewer` method:

  - Test when called with a valid contributor_id.
- Test when called with an invalid contributor_id.

4. Test `select_metareviewer` method:

  - Test when given a valid response map id.
- Test when given an invalid response map id.

5. Test `add_reviewer` method:

  - Test when the reviewer is not assigned to review their own artifact.
- Test when the reviewer is assigned to review their own artifact.
- Test when the reviewer is already assigned to the contributor.

6. Test `assign_reviewer_dynamically` method:

  - Test when a topic is selected and review is allowed.
- Test when no topic is selected and review is allowed.
- Test when no topic is selected and review is not allowed.
- Test when a topic is selected and review is not allowed.
- Test when there are no available topics to review.
- Test when there are no available artifacts to review.
- Test when the reviewer has reached the maximum number of outstanding reviews.

7. Test `review_allowed?` method:

  - Test when the reviewer has not reached the maximum number of reviews allowed for the assignment.
- Test when the reviewer has reached the maximum number of reviews allowed for the assignment.

8. Test `check_outstanding_reviews?` method:

  - Test when there are no review mappings for the assignment and reviewer.
- Test when there are review mappings for the assignment and reviewer, and all reviews are completed.
- Test when there are review mappings for the assignment and reviewer, and some reviews are in progress.

9. Test `assign_quiz_dynamically` method:

  - Test when the reviewer has already taken the quiz.
- Test when the reviewer has not taken the quiz yet.
- Test when an error occurs during the assignment process.

10. Test `add_metareviewer` method:

   - Test when a metareviewer is successfully added.
- Test when the metareviewer is already assigned to the reviewer.
- Test when an error occurs during the process.

11. Test `assign_metareviewer_dynamically` method:

   - Test when there are reviews to Meta review.
- Test when there are no reviews to Meta review.
- Test when an error occurs during assignment of metareviewer.

12. Test `get_reviewer` method:

   - Test when the user is a participant in the assignment.
- Test when the user is not a participant in the assignment.

13. Test `delete_outstanding_reviewers` method:

   - Test when there are outstanding reviewers.
- Test when there are no outstanding reviewers.

14. Test `delete_all_metareviewers` method:

   - Test when there are metareview mappings to delete.
- Test when there are unsuccessful deletes.
- Test when there are no metareview mappings to delete.

15. Test `unsubmit_review` method:

   - Test when the response is successfully unsubmitted.
- Test when the response fails to be unsubmitted.

16. Test `delete_reviewer` method:

   - Test when the review response map exists and there are no associated responses.
- Test when the review response map exists but there are associated responses.
- Test when the review response map does not exist.

17. Test `delete_metareviewer` method:

   - Test when the metareview mapping exists.
- Test when the metareview mapping does not exist.

19. Test `list_mappings` method.

20. Test `automatic_review_mapping` method.

21. Test `automatic_review_mapping_strategy` method.

22. Test `automatic_review_mapping_staggered` method.

23. Test `save_grade_and_comment_for_reviewer` method.

24. Test `start_self_review` method.

25. Test `assign_reviewers_for_team` method.

26. Test `peer_review_strategy` method.

27. Test `review_mapping_params` method.

Design Pattern

During our code refactoring process, we leveraged various design patterns to enhance readability and maintainability. One commonly applied pattern was "Extract Method," where we identified lengthy and intricate methods and extracted segments of functionality into separate methods. This restructuring made the code more comprehensible and easier to grasp by isolating specific tasks within dedicated methods.

Additionally, we addressed the issue of excessive conditional statements by employing the "Refactoring Conditionals" design pattern. Instead of cluttering the code with numerous conditionals, we refactored by encapsulating the logic within these conditionals into distinct methods. By doing so, we streamlined the code's flow and improved its readability, making it clearer to understand the purpose and execution of each segment.


Relevant Links

Team

Mentor

  • Ananya Mantravadi (amantra)

Team Members

  • Lagani Patel (lpatel)
  • Nidhay Pancholi (nrpancho)
  • Saloni Shah (sshah39)