|
|
| Line 10: |
Line 10: |
|
| |
|
| == Tasks == | | == 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.<br/>
| |
| -Replace switch statements with subclasses methods.<br/>
| |
| -Create models for the subclasses.<br/>
| |
| -Remove hardcoded parameters.<br/>
| |
| -Add meaningful comments and edit/remove/do not unnecessary comments.<br/>
| |
| -Try to increase the test coverage.<br/>
| |
|
| |
|
| === Phase 1 === | | === Email Notifications === |
| For Phase 1 of the project we have focused working on the below mentioned issues.<br/> | | For Phase 1 of the project we have focused working on the below mentioned issues.<br/> |
| -Refactor assign_reviewer_dynamically function<br/> | | -Refactor assign_reviewer_dynamically function<br/> |
| Line 28: |
Line 21: |
| -Fixed Code climate issues<br/> | | -Fixed Code climate issues<br/> |
|
| |
|
| === Phase 2 === | | === Mentor Meeting Management === |
| For Phase 2 of the project we plan working on the below mentioned issues.<br/> | | For Phase 2 of the project we plan working on the below mentioned issues.<br/> |
| -Refactor automatic_review_mapping function<br/> | | -Refactor automatic_review_mapping function<br/> |
| Line 39: |
Line 32: |
| == Implementation == | | == Implementation == |
|
| |
|
| === Phase 1 === | | === Email Notifications === |
|
| |
|
| ==== add_reviewer ==== | | ==== Students ==== |
| 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/>
| |
|
| |
|
| - Refactored the `add_reviewer` method to focus on single tasks per method, enhancing code readability and maintainability.<br/>
| | ==== Mentors ==== |
| - Extracted the functionality to find a user's ID by name into a separate method named `find_user_id_by_name`.<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/>
| |
| - Abstracted the process of assigning a reviewer to the assignment into a method named `assign_reviewer`.<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/>
| |
| | |
| ==== 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:<br/>
| |
| | |
| - Restructured the `assign_reviewer_dynamically` method to perform single tasks per method, improving code organization and readability.<br/>
| |
| - Extracted the functionality to find the assignment participant into a separate method called `find_participant_for_assignment`.<br/>
| |
| - Abstracted the logic to handle errors when no topic is selected into a method named `topic_selection_error?`.<br/>
| |
| - Created a method named `dynamically_assign_reviewer` to handle the process of dynamically assigning a reviewer based on the assignment type.<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/>
| |
| | |
| ==== 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:<br/>
| |
| | |
| - Updated the `ReviewMappingController` spec file.<br/>
| |
| - 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/>
| |
| - 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/>
| |
| - Ensured that all test cases are descriptive and cover the relevant scenarios for each method.<br/>
| |
| - Verified that the test cases accurately reflect the behavior of the controller methods after the code changes.<br/>
| |
| | |
| == 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 <code>review_mapping_controller.rb</code> file for the following functions:<br/>
| |
| | |
| 1. Test <code>`action_allowed?`</code> method:<br/>
| |
| - Test when the action is <code>'add_dynamic_reviewer'</code>.<br/>
| |
| - Test when the action is <code>'show_available_submissions'</code>.<br/>
| |
| - Test when the action is <code>'assign_reviewer_dynamically'</code>.<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/>
| |
| | |
| 3. Test <code>`select_reviewer`</code> method:<br/>
| |
| - Test when called with a valid contributor_id.<br/>
| |
| - Test when called with an invalid contributor_id.<br/>
| |
| | |
| 4. Test <code>`select_metareviewer`</code> method:<br/>
| |
| - Test when given a valid response map id.<br/>
| |
| - Test when given an invalid response map id.<br/>
| |
| | |
| 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/>
| |
| | |
| 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/>
| |
| | |
| 8. Test <code>`check_outstanding_reviews?`</code> method:<br/>
| |
| - Test when there are no review mappings for the assignment and reviewer.<br/>
| |
| - Test when there are review mappings for the assignment and reviewer, and all reviews are completed.<br/>
| |
| - 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/>
| |
| | |
| 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/>
| |
| | |
| 11. Test <code>`assign_metareviewer_dynamically`</code> method:<br/>
| |
| - Test when there are reviews to Meta review.<br/>
| |
| - Test when there are no reviews to Meta review.<br/>
| |
| - Test when an error occurs during assignment of metareviewer.<br/>
| |
| | |
| 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/>
| |
| | |
| 13. Test <code>`delete_outstanding_reviewers`</code> method:<br/>
| |
| - Test when there are outstanding reviewers.<br/>
| |
| - Test when there are no outstanding reviewers.<br/>
| |
| | |
| 14. Test <code>`delete_all_metareviewers`</code> method:<br/>
| |
| - 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/>
| |
| | |
| 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/>
| |
| | |
| 16. Test <code>`delete_reviewer`</code> method:<br/>
| |
| - Test when the review response map exists and there are no associated responses.<br/>
| |
| - Test when the review response map exists but there are associated responses.<br/>
| |
| - Test when the review response map does not exist.<br/>
| |
| | |
| 17. Test <code>`delete_metareviewer`</code> method:<br/>
| |
| - 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 == | | == 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 == | | == Relevant Links == |
| * '''Github Repository:''' https://github.com/NidhayPancholi/expertiza | | * '''Github Repository:''' https://github.com/ |
| * '''Pull Request:''' https://github.com/expertiza/expertiza/pull/2767 | | * '''Pull Request:''' https://github.com/expertiza/expertiza/pull/ |
| | * '''Pull Request:''' https://github.com/expertiza/expertiza/pull/ |
|
| |
|
| == Team == | | == Team == |
| === Mentor === | | === Mentor === |
| * Ananya Mantravadi (amantra) | | * Ed Gehringer (efg) |
|
| |
|
| === Team Members === | | === Team Members === |
| * Lagani Patel (lpatel) | | * Samuel Kwiatkowski-Martin (slkwiatk) |
| * Nidhay Pancholi (nrpancho) | | * Tanmay Pardeshi (tpardes) |
| * Saloni Shah (sshah39) | | * Bala Logesh Sudalaimuthu Pandian (bsudala) |