CSC/ECE 517 Fall 2023 - E2356. Refactor review mapping controller.rb (Phase 2)

From Expertiza_Wiki
Jump to navigation Jump to search

This wiki page is for information regarding the changes made for the E2356 Refactor review_mapping_controller.rb OSS assignment for Fall 2023, CSC/ECE 517.

Introduction

Expertiza is an open-source project developed on Ruby on Rails. This web application is maintained by the students 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.

About Controller

The functionality of review_mapping_controller is to map the reviewer to an assignment. Basically, the controller handles the assignment of reviews to different teams or individual student users, covering scenarios like peer reviews and self-reviews. Additionally, the controller plays a crucial role in handling requests from student users, facilitating extra bonus reviews in alignment with the assignment policy.

Problem Statement

  • Refactor the long methods in review_mapping_controller.rb like assign_reviewer_dynamically, add_reviewer, automatic_review_mapping,peer_review_strategy etc.
  • Fix two identified major bugs.
  • Add meaningful comments and edit/remove unnecessary comments.
  • Ensure that code changes do not break any functionality. Refactoring method names might cause cascaded updates in other files.
  • Add Tests as provided in test skeletons.

We have worked rigorously in Phase 1 on refactoring the same controller. Documentation for the same can be found here.

Design Pattern

During the code refactoring process, we conscientiously followed various design patterns to enhance the overall structure. One of the most common ones is the “Extract Method.” There were several instances where certain methods became overly lengthy and intricate. By extracting specific functionalities into separate methods, we significantly improved the code's readability and comprehension. This way, it became easier and clearer to understand what the method achieved. Furthermore, we tackled the issue of excessive conditional statements by employing the "Refactoring Conditionals" design pattern. This approach involves isolating the block of code within a conditional statement into a distinct method and subsequently invoking that method. This strategic refactoring not only streamlined our code but also contributed to a more readable and maintainable codebase.

File(s) Modified

1. review_mapping_controller.rb
2. review_mapping_controller_spec.rb
Changes can be seen in the pull request created here.

Solutions/Details of Changes Made

1. Refactor peer_review_strategy method in review_mapping_controller.rb file

The method was split into smaller functions specifically isolating the code for handling strategy for the last team in the list.


2. Bug fix regarding in-progress reviews for a participant

Changes made to check_outstanding_reviews method to return the correct results. Previously if all the reviews were completed by a participant, the method returned true (num_reviews_in_progress = 0), which would not be the right output. This is fixed by ensuring a return value of true only if num_reviews_in_progress > 0.


3. Bug fix regarding Assign Reviewer Button

Bug Description: When a user tries to click on the 'Assign Reviewers' button without having completed the creation of assignment the following error occurs -

This error indicates that an assignment needs to be instantiated and only then can reviewers be added to this assignment. The list_mapping method displays all participants eligible as reviewers for a particular assignment. To this method we added a check to confirm if an assignment existed.

If not, the user is redirected to the assignment creation page with the following error message -

4. Test Cases Addition

The major focus during this phase was on adding tests and making the review_mapping_controller.rb controller fail-proof. By the end of this phase we were able to incorporate the majority of tests provided in the Test Skeleton.

Test Plan

Test Plan, Testing Strategy and Results

In the realm of Test-Driven Development (TDD) research, our team made the strategic decision to utilize Mustafa Olmez's meticulously crafted test skeletons. These skeletons provided us with invaluable guidance, outlining the essential tests required for our unit. By incorporating 50 of these meticulously designed test skeletons into our work, we were able to conduct rigorous testing of our controller module. This approach allowed us to delve deeply into the functionality of our controller, ensuring thorough examination and validation of its behavior. The integration of these test skeletons not only provided a robust framework for our unit tests but also enhanced the overall quality and reliability of our codebase.

Test Plan

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

1. action_allowed?

2. select_reviewer

3. select_metareviewer

4. review_allowed?

5. check_outstanding_reviews?

6. assign_quiz_dynamically

7. assign_metareviewer_dynamically

8. delete_outstanding_reviewers

9. delete_metareview

10. list_mappings

11. save_grade_and_comment_for_reviewer


Testing Strategy

For testing of various functions in review_mapping_controller.rb we followed the following strategy:

1. One expectation/assertion per unit test

2. Unit testing includes checking for:

  • Logical flow of the code
  • Flash error messages
  • Redirection to particular URLs/path
  • DB updation


For a detailed view of the changes made, please refer to the complete file at the following link:

https://github.com/expertiza/expertiza/pull/2657/files#diff-bccc23eb7ba2a55b74837eed3f4c85aa1d8c55d356ce0e0bc7de7afd50121b13

Results

The additional tests ensure every method of review_mapping_controller is tested. By adopting this approach we have increased the overall coverage from 47.65% to 47.86%. The changes made passed all the necessary Travis CI/CD Pipeline tests.

SimpleCov Coverage Detail Report of review_mapping_controller.rb

Tests prior to the changes covered 69.65% of review_mapping_controller.rb


After adding 50 tests, the tests covered 71.35% of refactored review_mapping_controller.rb. It improved slightly to be an even better coverage.

Relevant Links

Team

Mentor

  • Ameya Vaichalikar (agvaicha)

Team Members

  • Chirag Bheemaiah Palanganda Karumbaiah (cpalang)
  • Shanmukh Pawan Moparthi (smopart2)
  • Amit Bhujbal (abhujba)
Retrieved from "https://wiki.expertiza.ncsu.edu/index.php?title=CSC/ECE_517_Fall_2023_-_E2356._Refactor_review_mapping_controller.rb_(Phase_2)&oldid=152639"