CSC/ECE 517 Fall 2021 - E2125. Refactor review mapping helper.rb

E2125. Refactor review mapping helper.rb

This page describes the Expertiza based OSS project.

Expertiza

Expertiza is an open-source project contributed by both the students and the faculty at NCSU. It is developed on Ruby-on-Rails and software to create reusable learning objects through peer review. Expertiza allows the instructor to create new assignments, customize new or existing assignments, and create a topic list for students to sign up. It also supports students to form teams for team projects and allows submission with almost any document type, including URLs and wiki pages.

Background

E2125. Refactor review_mapping_helper.rb

• Files involved: review_mapping_helper.rb
• What is wrong:
  • The review_mapping_helper.rb has methods that exceed the limit on lines of code Also, it is missing proper comments for each functionality. Cyclomatic complexity of most of the methods is way too high as per the standard defined in the Code Climate.
• What needs to be done:

1. Use Code Climate to do a diagnosis for Expertiza. Search for the issues involving the files mentioned above.
2. Read about cyclomatic complexity here.
3. Fix all the methods having the issue with cyclomatic complexity, assignment branch condition size, extra commas and space or lack of it and assigning branching condition.

Write test cases of any new code written. Test the existing functionality after refactoring.

However, since the code-quality-analysis result from Code Climate shows that all cyclomatic complexity was resolved, the main work was to fix the logic of the method "get_team_color".

Modifications in review_mapping_helper.rb

get_team_color

This method is to determine the state of reviewers review on each team. Related pages can be found in "manages... -> assignments -> view reports -> view"

There are five different states:‎

Example

Under the column "Team Reviewed", color brown indicates that a score is given to the reviewer (including zero), and color blue indicates that a score havnen't given to the reviewer.

---

The original code will not return the correct color, because when it is checking the grade for the reviewer, it is not checking the correct variable.

 def get_team_color(response_map)
    # Storing redundantly computed value in a variable
    assignment_created = @assignment.created_at

    # Storing redundantly computed value in a variable
    assignment_due_dates = DueDate.where(parent_id: response_map.reviewed_object_id)

    # Returning colour based on conditions
    if Response.exists?(map_id: response_map.id)
      if !response_map.try(:reviewer).try(:review_grade).grade_for_reviewer.nil?
        'brown'
      elsif response_for_each_round?(response_map)
        'blue'
      else
        check_submission_state(response_map, assignment_created, assignment_due_dates, @assignment.num_review_rounds)
      end
    else
      'red'
    end
  end

check_submission_state

This method checks whether the team submit the project on time or not. (If there are no submission or the submission is not on time, it is reasonable that the reviewer cannot review the project). In addition, the submission time of a link should be checked by the last modified time of the webpage, not the moment the link itself is submitted.

---

 def check_submission_state(response_map, assignment_created, assignment_due_dates, round)
    if submitted_within_round?(round, response_map, assignment_created, assignment_due_dates)
      'purple'
    else
      link = submitted_hyperlink(round, response_map, assignment_created, assignment_due_dates)
      if link.nil? or (link !~ %r{https*:\/\/wiki(.*)}) # can be extended for github links in future
        'green'
      else
        link_updated_at = get_link_updated_at(link)
        link_updated_since_last?(round, assignment_due_dates, link_updated_at) ? 'purple' : 'green'
      end
    end
  end

obtain_team_color

---

This method has been removed because it is running unnecessary check on old submissions.

def obtain_team_color(response_map, assignment_created, assignment_due_dates)
  color = []
  (1..@assignment.num_review_rounds).each do |round|
    check_submission_state(response_map, assignment_created, assignment_due_dates, round, color)
  end
  color[-1]
end 

Test Plan

RSpec

---

All 32 test cases are currently passing.

1. In method get_team_color(response_map) in review mapping helper.rb: Only checking if the record exist is not enough, because all mapping records pre-exist with empty scores in it.
2. The for loop for checking multiple submission stage is unnecesasry, so we removed obtain_team_color and call check_submission_state directly.
3. Stubbing NET::HTTP to fix test cases that originally failed if calling the method get_link_updated_at.

 allow(Net::HTTP).to receive(:get_response).and_return(obj)

Scenarios for get_team_color:
1. color should be red if response_map does not exist
2. color should be blue if the a review was submitted for each round
3. color should NOT be blue if the a review was NOT submitted for each round
4. color should be brown if the review and review_grade exist
5. color should be green if the submission link is non-existent
6. color should be green if the submission link is NOT a link to a Expertiza wiki
7. color should be purple if review were submitted for each round (on time)
8. color should be purple if submission link has been updated since due date for a specified round
9. should return green color if the submission link is not present
10. should return green color if the assignment was not submitted within the round
11. should return purple color if the assignment was submitted within the round

Scenarios for check_submission_state:
1. should return green color if the submitted link is not a wiki link
2. should return green color if the submission link is not present
3. should return green color if the assignment was not submitted within the round
4. should return purple color if the assignment was submitted within the round

Example test case:

 it 'color should be purple if submission link has been updated since due date for a specified round' do
      # deadline_right inspired from bookmark_review_spec
      allow(Net::HTTP).to receive(:get_response).and_return({'last-modified' => DateTime.now.in_time_zone})
      create(:deadline_right, name: 'No')
      create(:deadline_right, name: 'Late')
      create(:deadline_right, name: 'OK')

      response_map_with_reviewee = create(:review_response_map, reviewer: @reviewer, reviewee: @reviewee_with_assignment)

      create(:submission_record, assignment_id: @assignment.id, team_id: @reviewee_with_assignment.id, operation: 'Submit Hyperlink',
        content: 'https://wiki.archlinux.org/', created_at: DateTime.now.in_time_zone - 7.day)

      create(:assignment_due_date, assignment: @assignment, parent_id: @assignment.id, round: 1, due_at: DateTime.now.in_time_zone - 5.day)
      create(:assignment_due_date, assignment: @assignment, parent_id: @assignment.id, round: 2, due_at: DateTime.now.in_time_zone + 6.day)

      create(:response, response_map: response_map_with_reviewee)

      color = get_team_color(response_map_with_reviewee)
      expect(color).to eq('purple')
    end
  end

Manual Testing

1. Click "manage..." from the tabs above the webpage
2. Select assignments from the dropdown list
3. select the view reports icon from each created assignments
4. click on view button to view different status of different conditions (can create additional assignments with shorter due date intervals, submit new works, and reviews to it)

Peer Review Credentials

Username: instructor6
Password: password