From 2a00ddaf40e403b68f6f9e7d2b178973cc308054 Mon Sep 17 00:00:00 2001 From: Ali Qureshi <61347679+qureshi-ali@users.noreply.github.com> Date: Thu, 8 Feb 2024 12:31:08 -0500 Subject: [PATCH] Revert "E2353. Further refactoring and improvement of review_mapping_helper" --- PR.md | 1 - app/helpers/data_mapping_helper.rb | 48 --- app/helpers/review_mapping_charts_helper.rb | 88 ------ app/helpers/review_mapping_helper.rb | 296 +++++++++++++----- .../reports/_calibration_report.html.erb | 2 +- app/views/reports/_feedback_report.html.erb | 8 +- app/views/reports/_review_report.html.erb | 11 +- app/views/reports/_team_score.html.erb | 2 +- .../_team_score_score_awarded.html.erb | 2 +- spec/controllers/badges_controller_spec.rb | 24 +- .../controllers/publishing_controller_spec.rb | 8 +- spec/features/review_mapping_helper_spec.rb | 12 +- spec/helpers/participants_helper_spec.rb | 2 +- spec/helpers/review_mapping_helper_spec.rb | 112 +++---- spec/models/collusion_cycle_spec.rb | 5 +- 15 files changed, 293 insertions(+), 328 deletions(-) delete mode 100644 PR.md delete mode 100644 app/helpers/data_mapping_helper.rb delete mode 100644 app/helpers/review_mapping_charts_helper.rb diff --git a/PR.md b/PR.md deleted file mode 100644 index 4e793a883e3..00000000000 --- a/PR.md +++ /dev/null @@ -1 +0,0 @@ -+PR File diff --git a/app/helpers/data_mapping_helper.rb b/app/helpers/data_mapping_helper.rb deleted file mode 100644 index f771e7281a2..00000000000 --- a/app/helpers/data_mapping_helper.rb +++ /dev/null @@ -1,48 +0,0 @@ -# maps data and options in review_mapping_charts_helper for relevant methods -module DataMappingHelper - # provide options data for tagging interval chart - def provide_tagging_options - { - width: '200', - height: '125', - scales: { - yAxes: [{ stacked: false, ticks: { beginAtZero: true } }], - xAxes: [{ stacked: false }] - } - } - end - - # provide volume metric options - def provide_volume_metric_options - { - legend: { position: 'top', labels: { usePointStyle: true } }, - width: '200', - height: '125', - scales: { - yAxes: [{ stacked: true, id: 'bar-y-axis1', barThickness: 10 }, { display: false, stacked: true, id: 'bar-y-axis2', barThickness: 15, type: 'category', categoryPercentage: 0.8, barPercentage: 0.9, gridLines: { offsetGridLines: true } }], - xAxes: [{ stacked: false, ticks: { beginAtZero: true, stepSize: 50, max: 400 } }] - } - } - end - - # provides data for tagging interval chart - def display_tagging_interval_chart_data(intervals, interval_mean) - { - labels: [*1..intervals.length], - datasets: [{ backgroundColor: 'rgba(255,99,132,0.8)', data: intervals, label: 'time intervals' }, intervals_check(intervals, interval_mean)] - } - end - - # mapping interval length and mean for display_tagging_interval_chart_data method - def intervals_check(intervals, interval_mean) - return { data: Array.new(intervals.length, interval_mean), label: 'Mean time spent' } if intervals.empty? - end - - # provide data for volume metric chart using reviewer data - def volume_metric_chart_data(labels, reviewer_data, all_reviewers_data) - { - labels: labels, - datasets: [{ label: 'vol.', backgroundColor: 'rgba(255,99,132,0.8)', borderWidth: 1, data: reviewer_data, yAxisID: 'bar-y-axis1' }, { label: 'avg. vol.', backgroundColor: 'rgba(255,206,86,0.8)', borderWidth: 1, data: all_reviewers_data, yAxisID: 'bar-y-axis2' }] - } - end -end diff --git a/app/helpers/review_mapping_charts_helper.rb b/app/helpers/review_mapping_charts_helper.rb deleted file mode 100644 index 2b127dd7ad7..00000000000 --- a/app/helpers/review_mapping_charts_helper.rb +++ /dev/null @@ -1,88 +0,0 @@ -# moves data of reviews in each round from a current round -module ReviewMappingChartsHelper - # Provide data for chart elements - def initialize_chart_elements(reviewer) - avg_vol_per_round(reviewer) - labels.push 'Total' - reviewer_data.push reviewer.overall_avg_vol - all_reviewers_data.push @all_reviewers_overall_avg_vol - [labels, reviewer_data, all_reviewers_data] - end - - # display avg volume for all reviewers per round - def avg_vol_per_round(reviewer) - labels, round = 0, [] - reviewer_data = [] - all_reviewers_data = [] - @num_rounds.times do |rnd| - next unless @all_reviewers_avg_vol_per_round[rnd] > 0 - round += 1 - labels.push round - reviewer_data.push reviewer.avg_vol_per_round[rnd] - all_reviewers_data.push @all_reviewers_avg_vol_per_round[rnd] - end - end - - # The data of all the reviews is displayed in the form of a bar chart - def display_volume_metric_chart(reviewer) - labels, reviewer_data, all_reviewers_data = initialize_chart_elements(reviewer) - data = volume_metric_chart_data(labels, reviewer_data, all_reviewers_data) - options = provide_volume_metric_options - horizontal_bar_chart data, options - end - - # E2082 Generate chart for review tagging time intervals - def display_tagging_interval_chart(intervals) - # if someone did not do any tagging in 30 seconds, then ignore this interval - threshold = 30 - intervals = intervals.select { |v| v < threshold } - unless intervals.empty? - interval_mean = intervals.reduce(:+) / intervals.size.to_f - end - # build the parameters for the chart - data = display_tagging_interval_chart_data(intervals, interval_mean) - options = provide_tagging_options - line_chart data, options - end - - # Calculate mean, min, max, variance, and stand deviation for tagging intervals - def calculate_key_chart_information(intervals) - # if someone did not do any tagging in 30 seconds, then ignore this interval - threshold = 30 - interval_precision = 2 # Round to 2 Decimal Places - intervals = intervals.select { |v| v < threshold } - metric_information(intervals, interval_precision) - # if no Hash object is returned, the UI handles it accordingly - end - - # Get Metrics once tagging intervals are available - def metric_information(intervals, interval_precision) - # if intervals are empty return empty - return {} if intervals.empty? - - metrics = {} - # calculate various metric values - metrics[:mean] = calculate_mean(intervals, interval_precision) - metrics[:min] = intervals.min - metrics[:max] = intervals.max - metrics[:variance] = calculate_variance(intervals, metrics[:mean], interval_precision) - metrics[:stand_dev] = calculate_standard_deviation(metrics[:variance], interval_precision) - metrics - end - - # calculate means from the input intervals - def calculate_mean(intervals, interval_precision) - (intervals.reduce(:+) / intervals.size.to_f).round(interval_precision) - end - - # calculate variance from intervals and mean - def calculate_variance(intervals, mean, interval_precision) - sum = intervals.inject(0) { |accum, i| accum + (i - mean)**2 } - (sum / intervals.size.to_f).round(interval_precision) - end - - # calculate standard deviation - def calculate_standard_deviation(variance, interval_precision) - Math.sqrt(variance).round(interval_precision) - end -end diff --git a/app/helpers/review_mapping_helper.rb b/app/helpers/review_mapping_helper.rb index e66191d9ddd..e249a054d1e 100644 --- a/app/helpers/review_mapping_helper.rb +++ b/app/helpers/review_mapping_helper.rb @@ -1,5 +1,4 @@ module ReviewMappingHelper - include ReviewMappingChartsHelper def create_report_table_header(headers = {}) render partial: 'report_table_header', locals: { headers: headers } end @@ -7,11 +6,11 @@ def create_report_table_header(headers = {}) # # gets the response map data such as reviewer id, reviewed object id and type for the review report # - def rspan_data(reviewed_object_id, reviewer_id, type) - # rspan = 0 + def get_data_for_review_report(reviewed_object_id, reviewer_id, type) + rspan = 0 (1..@assignment.num_review_rounds).each { |round| instance_variable_set('@review_in_round_' + round.to_s, 0) } - response_maps = response_maps_data(reviewed_object_id, reviewer_id, type) + response_maps = ResponseMap.where(['reviewed_object_id = ? AND reviewer_id = ? AND type = ?', reviewed_object_id, reviewer_id, type]) response_maps.each do |ri| rspan += 1 if Team.exists?(id: ri.reviewee_id) responses = ri.response @@ -19,52 +18,50 @@ def rspan_data(reviewed_object_id, reviewer_id, type) instance_variable_set('@review_in_round_' + round.to_s, instance_variable_get('@review_in_round_' + round.to_s) + 1) if responses.exists?(round: round) end end - - rspan - end - - def response_maps_data(reviewed_object_id, reviewer_id, type) - # rspan = 0 - (1..@assignment.num_review_rounds).each { |round| instance_variable_set('@review_in_round_' + round.to_s, 0) } - - response_maps = ResponseMap.where(['reviewed_object_id = ? AND reviewer_id = ? AND type = ?', reviewed_object_id, reviewer_id, type]) - - response_maps + [response_maps, rspan] end + # # gets the team name's color according to review and assignment submission status - def team_color(response_map) - # Red means review is not yet completed - return 'red' unless Response.exists?(map_id: response_map.id) - # if no reviewer assigned - return 'brown' unless response_map.reviewer.nil? || response_map.reviewer.review_grade.nil? - # Blue if review is submitted in each round but grade is not assigned yet - return 'blue' if response_for_each_round?(response_map) - obtain_team_color(response_map, @assignment.created_at, DueDate.where(parent_id: response_map.reviewed_object_id)) + # + 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).nil? + 'brown' + elsif response_for_each_round?(response_map) + 'blue' + else + obtain_team_color(response_map, assignment_created, assignment_due_dates) + end + else + 'red' + end end - # loops through the number of assignment review rounds and obtains the team color + # loops through the number of assignment review rounds and obtains the team colour def obtain_team_color(response_map, assignment_created, assignment_due_dates) - # color array is kept empty initially color = [] (1..@assignment.num_review_rounds).each do |round| check_submission_state(response_map, assignment_created, assignment_due_dates, round, color) end - # the last pused color is the required color code color[-1] end - # checks the submission state within each round and assigns team color + # checks the submission state within each round and assigns team colour def check_submission_state(response_map, assignment_created, assignment_due_dates, round, color) if submitted_within_round?(round, response_map, assignment_created, assignment_due_dates) color.push 'purple' else link = submitted_hyperlink(round, response_map, assignment_created, assignment_due_dates) - # Green if author didn't update the code, the reviewer doesn't need to re-review the work. if link.nil? || (link !~ %r{https*:\/\/wiki(.*)}) # can be extended for github links in future color.push 'green' else - link_updated_at = link_update_time(link) + link_updated_at = get_link_updated_at(link) color.push link_updated_since_last?(round, assignment_due_dates, link_updated_at) ? 'purple' : 'green' end end @@ -84,25 +81,25 @@ def response_for_each_round?(response_map) def submitted_within_round?(round, response_map, assignment_created, assignment_due_dates) submission_due_date = assignment_due_dates.where(round: round, deadline_type_id: 1).try(:first).try(:due_at) submission = SubmissionRecord.where(team_id: response_map.reviewee_id, operation: ['Submit File', 'Submit Hyperlink']) - submission_created_at = submission.where(created_at: assignment_created..submission_due_date) + subm_created_at = submission.where(created_at: assignment_created..submission_due_date) if round > 1 submission_due_last_round = assignment_due_dates.where(round: round - 1, deadline_type_id: 1).try(:first).try(:due_at) - submission_created_at = submission.where(created_at: submission_due_last_round..submission_due_date) + subm_created_at = submission.where(created_at: submission_due_last_round..submission_due_date) end - !submission_created_at.try(:first).try(:created_at).nil? + !subm_created_at.try(:first).try(:created_at).nil? end # returns hyperlink of the assignment that has been submitted on the due date def submitted_hyperlink(round, response_map, assignment_created, assignment_due_dates) submission_due_date = assignment_due_dates.where(round: round, deadline_type_id: 1).try(:first).try(:due_at) - submission_hyperlink = SubmissionRecord.where(team_id: response_map.reviewee_id, operation: 'Submit Hyperlink') - submitted_h = submission_hyperlink.where(created_at: assignment_created..submission_due_date) + subm_hyperlink = SubmissionRecord.where(team_id: response_map.reviewee_id, operation: 'Submit Hyperlink') + submitted_h = subm_hyperlink.where(created_at: assignment_created..submission_due_date) submitted_h.try(:last).try(:content) end # returns last modified header date # only checks certain links (wiki) - def link_update_time(link) + def get_link_updated_at(link) uri = URI(link) res = Net::HTTP.get_response(uri)['last-modified'] res.to_time @@ -116,14 +113,16 @@ def link_updated_since_last?(round, due_dates, link_updated_at) end # For assignments with 1 team member, the following method returns user's fullname else it returns "team name" that a particular reviewee belongs to. - def team_display_name(max_team_size, _response, reviewee_id, ip_address) + def get_team_reviewed_link_name(max_team_size, _response, reviewee_id, ip_address) team_reviewed_link_name = if max_team_size == 1 TeamsUser.where(team_id: reviewee_id).first.user.fullname(ip_address) else # E1991 : check anonymized view here Team.find(reviewee_id).name end - '(' + team_reviewed_link_name + ')' + team_reviewed_link_name = '(' + team_reviewed_link_name + ')' + # if !response.empty? and !response.last.is_submitted? + team_reviewed_link_name end # if the current stage is "submission" or "review", function returns the current round number otherwise, @@ -137,12 +136,17 @@ def team_display_name(max_team_size, _response, reviewee_id, ip_address) # gets the review score awarded based on each round of the review - def compute_awarded_review_score(reviewer_id, team_id) + def get_awarded_review_score(reviewer_id, team_id) + # Storing redundantly computed value in num_rounds variable num_rounds = @assignment.num_review_rounds - + # Setting values of instance variables + (1..num_rounds).each { |round| instance_variable_set('@score_awarded_round_' + round.to_s, '-----') } + # Iterating through list (1..num_rounds).each do |round| - score = @review_scores && @review_scores[reviewer_id] && @review_scores[reviewer_id][round] && @review_scores[reviewer_id][round][team_id] - instance_variable_set("@score_awarded_round_#{round}", "#{score}%") unless score.nil? || score == -1.0 + # Changing values of instance variable based on below condition + if @review_scores[reviewer_id] && @review_scores[reviewer_id][round] && @review_scores[reviewer_id][round][team_id] && @review_scores[reviewer_id][round][team_id] != -1.0 + instance_variable_set('@score_awarded_round_' + round.to_s, @review_scores[reviewer_id][round][team_id].to_s + '%') + end end end @@ -158,43 +162,182 @@ def review_metrics(round, team_id) end # sorts the reviewers by the average volume of reviews in each round, in descending order - # generalized method that takes in metric parameter which is a string to determine how to sort reviewers - def sort_reviewer_desc(metric) - case metric - when 'review_volume' - @reviewers.each do |r| - # get the volume of review comments - review_volumes = Response.volume_of_review_comments(@assignment.id, r.id) - r.avg_vol_per_round = [] - review_volumes.each_index do |i| - if i.zero? - r.overall_avg_vol = review_volumes[0] - else - r.avg_vol_per_round.push(review_volumes[i]) - end + def sort_reviewer_by_review_volume_desc + @reviewers.each do |r| + # get the volume of review comments + review_volumes = Response.volume_of_review_comments(@assignment.id, r.id) + r.avg_vol_per_round = [] + review_volumes.each_index do |i| + if i.zero? + r.overall_avg_vol = review_volumes[0] + else + r.avg_vol_per_round.push(review_volumes[i]) end end - # get the number of review rounds for the assignment - @num_rounds = @assignment.num_review_rounds.to_f.to_i - @all_reviewers_avg_vol_per_round = [] - @all_reviewers_overall_avg_vol = @reviewers.inject(0) { |sum, r| sum + r.overall_avg_vol } / (@reviewers.blank? ? 1 : @reviewers.length) - @num_rounds.times do |round| - @all_reviewers_avg_vol_per_round.push(@reviewers.inject(0) { |sum, r| sum + r.avg_vol_per_round[round] } / (@reviewers.blank? ? 1 : @reviewers.length)) - end - @reviewers.sort! { |r1, r2| r2.overall_avg_vol <=> r1.overall_avg_vol } - # other metric cases can be added easily by using "when" - else - puts 'metric not available' end + # get the number of review rounds for the assignment + @num_rounds = @assignment.num_review_rounds.to_f.to_i + @all_reviewers_avg_vol_per_round = [] + @all_reviewers_overall_avg_vol = @reviewers.inject(0) { |sum, r| sum + r.overall_avg_vol } / (@reviewers.blank? ? 1 : @reviewers.length) + @num_rounds.times do |round| + @all_reviewers_avg_vol_per_round.push(@reviewers.inject(0) { |sum, r| sum + r.avg_vol_per_round[round] } / (@reviewers.blank? ? 1 : @reviewers.length)) + end + @reviewers.sort! { |r1, r2| r2.overall_avg_vol <=> r1.overall_avg_vol } + end + + # moves data of reviews in each round from a current round + def initialize_chart_elements(reviewer) + round = 0 + labels = [] + reviewer_data = [] + all_reviewers_data = [] + + # display avg volume for all reviewers per round + @num_rounds.times do |rnd| + next unless @all_reviewers_avg_vol_per_round[rnd] > 0 + + round += 1 + labels.push round + reviewer_data.push reviewer.avg_vol_per_round[rnd] + all_reviewers_data.push @all_reviewers_avg_vol_per_round[rnd] + end + + labels.push 'Total' + reviewer_data.push reviewer.overall_avg_vol + all_reviewers_data.push @all_reviewers_overall_avg_vol + [labels, reviewer_data, all_reviewers_data] + end + + # The data of all the reviews is displayed in the form of a bar chart + def display_volume_metric_chart(reviewer) + labels, reviewer_data, all_reviewers_data = initialize_chart_elements(reviewer) + data = { + labels: labels, + datasets: [ + { + label: 'vol.', + backgroundColor: 'rgba(255,99,132,0.8)', + borderWidth: 1, + data: reviewer_data, + yAxisID: 'bar-y-axis1' + }, + { + label: 'avg. vol.', + backgroundColor: 'rgba(255,206,86,0.8)', + borderWidth: 1, + data: all_reviewers_data, + yAxisID: 'bar-y-axis2' + } + ] + } + options = { + legend: { + position: 'top', + labels: { + usePointStyle: true + } + }, + width: '200', + height: '225', + scales: { + yAxes: [{ + stacked: true, + id: 'bar-y-axis1', + barThickness: 10 + }, { + display: false, + stacked: true, + id: 'bar-y-axis2', + barThickness: 15, + type: 'category', + categoryPercentage: 0.8, + barPercentage: 0.9, + gridLines: { + offsetGridLines: true + } + }], + xAxes: [{ + stacked: false, + ticks: { + beginAtZero: true, + stepSize: 50, + max: 400 + } + }] + } + } + bar_chart data, options + end + + # E2082 Generate chart for review tagging time intervals + def display_tagging_interval_chart(intervals) + # if someone did not do any tagging in 30 seconds, then ignore this interval + threshold = 30 + intervals = intervals.select { |v| v < threshold } + unless intervals.empty? + interval_mean = intervals.reduce(:+) / intervals.size.to_f + end + # build the parameters for the chart + data = { + labels: [*1..intervals.length], + datasets: [ + { + backgroundColor: 'rgba(255,99,132,0.8)', + data: intervals, + label: 'time intervals' + }, + unless intervals.empty? + { + data: Array.new(intervals.length, interval_mean), + label: 'Mean time spent' + } + end + ] + } + options = { + width: '200', + height: '125', + scales: { + yAxes: [{ + stacked: false, + ticks: { + beginAtZero: true + } + }], + xAxes: [{ + stacked: false + }] + } + } + line_chart data, options + end + + # Calculate mean, min, max, variance, and stand deviation for tagging intervals + def calculate_key_chart_information(intervals) + # if someone did not do any tagging in 30 seconds, then ignore this interval + threshold = 30 + interval_precision = 2 # Round to 2 Decimal Places + intervals = intervals.select { |v| v < threshold } + + # Get Metrics once tagging intervals are available + unless intervals.empty? + metrics = {} + metrics[:mean] = (intervals.reduce(:+) / intervals.size.to_f).round(interval_precision) + metrics[:min] = intervals.min + metrics[:max] = intervals.max + sum = intervals.inject(0) { |accum, i| accum + (i - metrics[:mean])**2 } + metrics[:variance] = (sum / intervals.size.to_f).round(interval_precision) + metrics[:stand_dev] = Math.sqrt(metrics[:variance]).round(interval_precision) + metrics + end + # if no Hash object is returned, the UI handles it accordingly end - # Extracts the files submitted for a particular pair of participant and reviewee def list_review_submissions(participant_id, reviewee_team_id, response_map_id) participant = Participant.find(participant_id) team = AssignmentTeam.find(reviewee_team_id) html = '' unless team.nil? || participant.nil? - # Build a path to the review submissions using the team's path and the response map ID review_submissions_path = team.path + '_review' + '/' + response_map_id.to_s files = team.submitted_files(review_submissions_path) html += display_review_files_directory_tree(participant, files) if files.present? @@ -219,7 +362,7 @@ def list_hyperlink_submission(response_map_id, question_id) end # gets review and feedback responses for all rounds for the feedback report - def author_reviews_and_feedback_response(author) + def get_each_review_and_feedback_response_map(author) @team_id = TeamsUser.team_id(@id.to_i, author.user_id) # Calculate how many responses one team received from each round # It is the feedback number each team member should make @@ -244,11 +387,9 @@ def feedback_response_map_record(author) end # gets review and feedback responses for a certain round for the feedback report - def feedback_response_for_author(author) - # This is a lot of logic in the view (gross) and this method is called if the review rounds don't have varying rubrics - - # this variable is used to populate all the feedback response data if the assignments don't vary by round. - @feedback_response_data = FeedbackResponseMap.where(['reviewed_object_id IN (?) and reviewer_id = ?', @all_review_response_ids, author.id]) + def get_certain_review_and_feedback_response_map(author) + # Setting values of instance variables + @feedback_response_maps = FeedbackResponseMap.where(['reviewed_object_id IN (?) and reviewer_id = ?', @all_review_response_ids, author.id]) @team_id = TeamsUser.team_id(@id.to_i, author.user_id) @review_response_map_ids = ReviewResponseMap.where(['reviewed_object_id = ? and reviewee_id = ?', @id, @team_id]).pluck('id') @review_responses = Response.where(['map_id IN (?)', @review_response_map_ids]) @@ -258,7 +399,7 @@ def feedback_response_for_author(author) # # for calibration report # - def calibration_report_css_class(diff) + def get_css_style_for_calibration_report(diff) # diff - difference between stu's answer and instructor's answer dict = { 0 => 'c5', 1 => 'c4', 2 => 'c3', 3 => 'c2' } css_class = if dict.key?(diff.abs) @@ -269,7 +410,6 @@ def calibration_report_css_class(diff) css_class end - # These classes encapsulate the logic for different review strategies in the system class ReviewStrategy attr_accessor :participants, :teams @@ -280,8 +420,6 @@ def initialize(participants, teams, review_num) end end - # Used when student_review_num is not zero and submission_review_num is zero in the ReviewMappingController - # Provides specific review strategy calculations for individual students class StudentReviewStrategy < ReviewStrategy def reviews_per_team (@participants.size * @review_num * 1.0 / @teams.size).round @@ -296,8 +434,6 @@ def reviews_per_student end end - # Used when student_review_num is zero and submission_review_num is not zero in the ReviewMappingController - # Provides specific review strategy calculations for teams class TeamReviewStrategy < ReviewStrategy def reviews_per_team @review_num diff --git a/app/views/reports/_calibration_report.html.erb b/app/views/reports/_calibration_report.html.erb index 43ec6d719aa..aa2fc1e6072 100644 --- a/app/views/reports/_calibration_report.html.erb +++ b/app/views/reports/_calibration_report.html.erb @@ -34,7 +34,7 @@ th, td { <% end %> <% count_hash.each do |answer, count| %> <% answer = 0 if answer.nil? %> - <% css_class = calibration_report_css_class(answer - instructor_answer) %> + <% css_class = get_css_style_for_calibration_report(answer - instructor_answer) %>