CSC/ECE 517 Fall 2021 - E2125. Refactor review mapping helper.rb: Difference between revisions
Line 24: | Line 24: | ||
===== get_team_color ===== | ===== get_team_color ===== | ||
This method is to determine the state of reviewers review on each team. There are five different states: | |||
---- | ---- | ||
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. | 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. |
Revision as of 23:45, 26 October 2021
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:
- Use Code Climate to do a diagnosis for Expertiza. Search for the issues involving the files mentioned above.
- Read about cyclomatic complexity here.
- 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 in code climate it shows that all cyclomatic complexity was resolved, our main work was fixing 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. There are five different states:
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 loop.
Testing Details in review_mapping_helper.rb
RSpec
All 32 test cases are currently passing.
- 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.
- The for loop for checking multiple submission stage is unnecesasry, so we removed obtain_team_color and call check_submission_state directly.
- Stubbing NET::HTTP to fix test cases that originally failed if calling the method get_link_updated_at.
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
Peer Review Credentials
Username: instructor6
Password: password