CSC/ECE 517 Fall 2020/oss E2072 RLM

From Expertiza_Wiki
Jump to navigation Jump to search

This wiki page is for the description of changes made under E2072 OSS assignment for Fall 2020, CSC/ECE 517.

Peer Review Information

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:

  • Instructor login: username -> instructor6, password -> password
  • Student login: username -> student4340, password -> password
  • Student login: username -> student4405, password -> password

Expertiza Background

Expertiza is an educational web application created and maintained by the joint efforts of the students and the faculty at NCSU and supported by MIT and the National Science Foundation.

It’s an open source project developed on Ruby on Rails platform and it’s code is available on Github. It allows students to review each other’s work and improve their work upon this feedback.

Description of the current project

The following is an Expertiza based OSS project which deals primarily with the AssignmentParticipant model and Review Mapping helper. It focuses on refactoring unkept, unused and misleading methods within the aforementioned files. The project’s goal is to enhance some of the methods in use and clean up others that were either outdated or in need of improvement.

Files modified in current project

Our project focused primarily on refactoring the assignment_partipant.rb Model file. The files modified for this project are:
1. assignment_participant.rb
2. review_mapping_helper.rb
3. assignment_participant_spec.rb
4. publishing_controller.rb
5. user.rb
6. quiz_assignment.rb


Assignment Participant Model

This is a controller that helps students and instructors view grades and reviews, update scores, check for grading conflicts and calculate penalties. A couple of long and complex methods were refactored from this controller along with removal of some non-functional code and a few language changes to make it Ruby style. Three methods in particular, namely conflict_notification ,calculate_all_penalties and edit were found to be too long and were in need of refactoring into smaller, easier to manage methods. Few more compact methods were created for this purpose.

There were no existing test cases for the controller. We have added a spec file named 'grades_spec.rb' under the spec folder. As no changes were done for the model, no tests for the model were included.

ReviewMappingHelper

This is a helper class that contained methods to help calculate and display information regarding participant reviews and the volume of each review. One non-functional method was removed from the file and two other methods were updated to support sorting the volume of an arbitrary number of review rounds.

List of changes

We worked on the following work items(WIs)
WI1: Refactor self.grant_publishing_rights to assign_copyright
WI2: Determine whether reviews_by_reviewer method is still needed.
WI3: Refactor copy method name
WI4: Refactor assign_quiz
WI5: Refactor Review Score
WI6: Rewrite Score calculation within the scores method
WI7: Refactor Volume - # Distinct Words in a Review
WI8: Provide documentation for Compute_Assignment_Score


Solutions Implemented and Delivered

Refactoring Review Volume

This involved three methods in review_mapping_helper.rb, sort_reviewer_by_review_volume_desc, initialize_chart_elements and display_volume_metric. The first two methods worked together to sort the reviews by the average volume of reviews in each round of an assignment and then move the data of reviews in each round from a current round to prepare to display the information in chart form. The last method was found to not be used within the entirety of the project.

The following changes were made:

1. The methods were originally implemented in a way that only allowed for the information from three rounds of an assignment to be gathered. The functions had to be re-implemented using a loop that could gather and initialize the data from any given number of rounds.

2. The display_volume_metric was no longer being used within the rest of the project and so was removed to clean up the file.

sort_review_by_review_volume_desc loop re-implementation:

Initialize_chart_elements loop re-implementation:

Display_volume_metric removal:


Rewrite Scores Method The scores method returns the scores that a specific assignment_participant has received Originally, the scores method contained several convoluted method calls with some unused and commented out code. Our team was tasked with rewriting the scores method in assignment_participant.rb to make it more readable and transparent.

The following changes were made to accomplish this task: Removed topic_total_scores (score calculations for a microtask) and moved it into the scores method, since this method was only called once and wasn't used anywhere else. I also removed the associated tests.

Removed calculate_scores method and refactored the code into the scores method, since this method was only called once and wasn't used anywhere else. I also removed the associated tests.


Refactored the merge_scores method which is called by Scores. DRYed out the code by adding a new update_min_or_max method that dynamically sets the min or max

Refactored merge_scores to use update_min_or_max method.

Added 6 RSPEC tests for this new update_min_or_max method

Added comments for the scores method and merge_scores method

Refactor assign_quiz Method The assign_quiz method was originally located in the assignment_participant.rb file and was requested to be moved into quiz_questionnaire.rb. However, upon further investigation, it was discovered that there existed two assign_quiz_dynamically methods, one which called the aforementioned assign_quiz method. Our investigation concluded that a newly constructed assign_quiz_dynamically method, located in review_mapping_controller.rb, had become the primary method of performing the tasks associated with assign_quiz and assign_quiz dynamically. Hence, following the consultation of our mentor, we elected to remove the definition and all instances of assign_quiz (assignment_participant.rb) and assign_quiz_dynamically (quiz_assignment.rb).

The following changes were made:

Removed all instances of assign_quiz method (defined in assignment_participant.rb)

Removed all unused instances assign_quiz_dynamically (defined in quiz_assignment.rb) Alternative assign_quiz_dynamically method located in review_mapping_controller.rb became the integral method used in recent years

Ensured all instances of assign_quiz_dynamically (review_mapping_controller.rb) were untouched and operational as expected This version does not invoke any assign_quiz method Removed #assign_quiz spec test from assignment_participant_spec.rb


Investigate reviews_by_reviewer Method The reviews_by_reviewer method returns the reviews of a certain assignment_participant. However, for the past 5 years or so all reviews have been of assignment_teams, not assignment_participant. Our team was tasked with investigating the reviews_by_reviewer method in assignment_participant.rb to determine whether it is still used, or would be useful to continue maintaining.

The following determinations were made: Determined that all reviews are of assignment_teams and not of assignment_participant The only calls to reviews_by_reviewer method happen in CollusionCycle. After verifying with Yang and Dr. Gehringer, we determined that Collusion Cycle is no longer used in the application. CollusionCycle was originally a piece of code that helped detect whether bias existed when teammates gave reviews of other teammates' work. Because CollusionCycle is unused, we deemed it appropriate to make the following changes: Removed the method definition from assignment_participant.rb


Removed the rspec test for this method


Refactor copy Method Name The copy method was determined to have a misleading name for a method that copies a participant to a course. As a result, the copy method name was refactored into 'copy_participant'.

The resulting changes were made to correspond to the name change:

1. Refactored copy method in assignment_participant.rb into 'copy_participant'

2. Refactored all instances of copy_participant associated with a assignment_participant

  • Spec test in assignment_participant_spec.rb was only instance found


Refactor self.grant_publishing_rights to assign_copyright Aside from simply refactoring the name of self.grant_publishing_rights into assign_copyright, upon further investigation, we noted that the definition of the method in assignment_participant.rb, did not require the participant parameter and participant loop, but was simplified by moving this functionality to wherever the method is called in the application.

The following changes were made:

Refactored self.grant_publishing_rights into 'assign_copyright' Refactored definition of the method in assignment_participant.rb, removing the participant parameter and participant loop; moving this functionality to wherever the method is called in the application

Altered functionality of the method call in def generate_keys (user.rb) to handle the 'participants.each do' loop

Altered functionality of the method call in grant_with_private_key (publishing_controller.rb) to handle the 'participants.each do' loop

Built spec test for modified assign_copyright method in assignment_participant_spec.rb Creates new RSA key-pair for a participant Tests if key matches using assign_copyright

Refactor review_score method The review_score method returns the review score for a particular questionnaire. The issue originally stated that the method should be able to determine the review score for any round of review. The original method returned the first round (index position 0) review score.

We made the following determinations: The review_score method only returns the first round review score which is from index position 0:

We determined that review_score method is only being called in collusion_cycle.rb, which is no longer used in the Expertiza

Testing Details

RSpec

There were no existing test cases for the GradesController. We have added a new spec file 'grades_spec.rb' which covers testing scenario for the newly added method. The specs were run on the previous and current files and they return the same results implying that the refactored code does not break anything. As the model was not changed, no test cases were added for the model.

Refactor assign_quiz Method Removed #assign_quiz spec test from assignment_participant_spec.rb to correlate with the deletion of the assign_quiz method itself.


Refactor copy Method Name Refactored all instances of copy_participant, including spec test in assignment_participant_spec.rb.


Refactor self.grant_publishing_rights to assign_copyright Following refactor, a new spec test needed to be built for the modified assign_copyright method in assignment_participant_spec.rb. Creates new RSA key-pair for a participant Tests if key matches using assign_copyright


UI Testing

Following steps needs to be performed to test the volume refactoring in the UI:
1. Login as instructor and enter anonymized view using the manage tag.
2. Go to the manage tab and enter the courses page
3. Choose a course and in any of the listed assignments, under the action section, click “view reports”
4. Select the view button
5. You should see the volume of reviews by round for each reviewer.


Scope for future improvement