CSC/ECE 517 Fall 2022 - E2260. Refactor review mapping controller
This wiki page is for the information regarding the changes made for the E2260 OSS assignment for Fall 2022, CSC/ECE 517.
Team
Mentor
- Vishal Veera Reddy (vveerar2)
Team Members
- Abhimanyu Bellam (abellam)
- Pradeep Balaji Muthukumar (pmuthuk)
- Vishnu Vinod Erapalli (verapal)
Introduction
Expertiza is an open-source project developed on Ruby on Rails. This web application is maintained by the student and faculty at NC State. This application gives complete control to the instructor to maintain the assignments in their class. With multiple functionalities such as adding topics, creating groups, and peer reviews, Expertiza is a well-developed application that can handle all types of assignments. To learn more about the full functionality Expertiza has to offer, visit the Expertiza wiki.
Project Description
The project is aimed at refactoring the review_mapping_controller as it is a long and complex file with methods that do not include comments. This project restructures the file without modifying the functionality and improves readability by using illustrious comments and indicative variable names.
The older working repository used for review 1 where all step-by-step commits are made:
Link to previous working Respository
This previous repository and PR were discontinued after review 1 since too many files were changed.
Manual UI testing is not applicable to this project. Refer to the section on Testing.
About Controller and Functionality
The review mapping controller
- Map reviewer and assignment
- Allows assigning topics/assignments to different teams or single student teams.
- Respond to a student user request for an extra bonus based on assignment policy.
Functionality:
The functionality of review_mapping_controller is to provide mapping for the reviewer and assignment. The controller handles the assignment of reviews to different teams or single student users, such as events of peer review and self-review. This controller is responsible to respond to student user requests for extra bonus reviews, based on assignment policy.
The review mapping controller had to be refactored and hence the code was pulled from [Expertiza Repository] onto our working [repository]. During the course of refactoring, a lot of files changed other than the 4 refactored files and hence the changes were cherry-picked and continued at this [repository].
In order to review the code, one can:
1. Go to [main repo] and check for the refactored changes.
2. Go to [Pull Request] and check the PR.
3. To understand changes, commit-by-commit, one can visit our old working repository [commit history]
4. To view our project board, visit this [link].
Issues
- Long methods are present in the file of review_mapping_controller.rb and to name a few, we have: assign_reviewer_dynamically, add_reviewer and automatic_review_mapping.
- Variables have been named in a manner that is not indicative of what they are meant to do.
- There are repetitive if-else constructs that affect the single responsibility of a particular function.These computations must be done in other methods.
- There are hard-coded parameters.
- Some methods have no comments and are not indicative of their functionalities. Some comments have not been written accurately.
Design Patterns
1. Refactoring conditionals: Aims to improve the readability of the code. Placed the block of code present in a conditional statement in a different method and called that method.
2. Extract Method: Functionalities of Long and Complex methods were moved to different methods to enhance readability.
Files modified
1. review_mapping_controller.rb
2. review_mapping_controller_spec.rb
3. _calibration.html.erb
4. config/routes.rb
Solutions
1. Changed add_calibration function name to add_calibration_for_instructor and added necessary comments. The add_calibration function was called only on an instructor and hence the name for the function 'add_calibration_for_instructor' is more meaningful. Changes can be found at: https://github.com/AbhimanyuBellam/expertiza/commit/40179533beeb258fc43a8d7b1bb00c6f71118fac
2. Refactored add_reviewer function- changed function name, split function to 2 parts. The initial function did two things: I) it fetched the details of the user and a topic and checked if the topic belongs to the same user's team, in which case he/she cannot review it. II) It assigned the user to a team to peer review. These two functionalities were split logically into two functions: add_reviewer_to team and add_unseen_reviewer_to_team which means that the user has not performed a review yet and he/she will be assigned a topic that is not of their own to review.
Changes can be found at:
https://github.com/AbhimanyuBellam/expertiza/commit/c3df6eabb47bc92f52452d4bbd31f4f3de1078cb
3. Refactored function assign_reviewer_dynamically, split function into parts: This method was used to assign submissions to students for peer review and was also used for an instructor to assign reviewers to an instructor-selected assignment. This method was broken down into assign_reviewer_with topic and assign_reviewer_without_topic because both these functionalities were implemented in the same function and the checking was also done there. Changes can be found at: https://github.com/AbhimanyuBellam/expertiza/commit/cc2fc9038c3dddca24df1b36759c113d94e31f45
4. Added illustrious comments to signify the functionality of the meta-reviewer methods https://github.com/AbhimanyuBellam/expertiza/commit/3a1bf601fad19f130f46dbfbadedf640e486f419
5. Added explanative comments for delete_reviewer and delete_metareviewer https://github.com/AbhimanyuBellam/expertiza/commit/01e6b55f4ea41673cfe5944435ecc94dc40ac141
6. Refactored automatic_review_mapping, split functions, and added explanative comments: The automatic_review_mapping created a team and it also computes the number of reviews performed by a student and if admissible, it goes on to call a review mapping strategy function. These functionalities can be split into two methods and each method will handle one functionality each. Changes can be found at: https://github.com/AbhimanyuBellam/expertiza/commit/61ed8664f526a9e087fe82bab08cde4ceaed0764
7. Refactored automatic_review_mapping- changed variable names, added comments: Added explanatory comments to the existing function and added conditions to check if the variable is not null. Also renamed variables to indicate their significance. Changes can be found at: https://github.com/AbhimanyuBellam/expertiza/commit/0107572655798b199d95ca216d548cbb67711bd6
8. Added comments to save_grade_and_comment_for_reviewer: Added comments to the function to explain what functionality it holds. Changes can be found at: https://github.com/AbhimanyuBellam/expertiza/commit/e20fed656d40bbf1cc92cf331abeb4b575223840
9. Added explanatory comments for start_self_review Changes can be found at: https://github.com/AbhimanyuBellam/expertiza/commit/141227a5aa7a94f9d35287a60a5ed28bab4bdac4
10. Refactored peer_review_strategy- split into functions, wrote explanatory comments: This method performed three functions I) TO allocate reviews to participants II)To distribute reviews among teams using a strategy, and III) Generate random participant index. Hence, three separate functions were made and each function handled one functionality. Changes can be found at: https://github.com/AbhimanyuBellam/expertiza/commit/7779c8a3530f4f0e6bcfd2ff76b82a11d7e2b2df
Testing
- Testing Plan
- Manual UI Test
- The deployed project database is incomplete and the accounts for students cannot be created. A complete UI test plan cannot be hence proposed. Also, the internal functionalities of the controllers cannot be tested using UI tests. In the demo setup of our VCL, we cannot simulate the real working of the website highlighting the role of the controller since we cannot make the same kind of submissions and review it. The logic of the controller is not related to the UI and hence we cannot perform the UI test plan on the deployed project.
- Link to the original workflow for the controller can be found here :
https://expertiza.csc.ncsu.edu/index.php/Review_mappings
- RSpec
- No functionality has been changed and only refactoring has been done. Rspec tests were carefully run and checked. To run the spec file, run:
- rspec spec/controllers/review_mapping_controller_spec.rb
- Increase in Code Coverage
- Two test cases were added and the code coverage increased.
- Travis CI Build Passed
