Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor on_the_fly_calc #2039

Merged
merged 7 commits into from
Sep 21, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
99 changes: 99 additions & 0 deletions app/helpers/assignment_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,103 @@ def get_team_name_color_in_list_submission(team)
end
end

# Compute total score for this assignment by summing the scores given on all questionnaires.
# Only scores passed in are included in this sum.
def compute_total_score(scores)
total = 0
self.questionnaires.each {|questionnaire| total += questionnaire.get_weighted_score(self, scores) }
total
end

def compute_reviews_hash
@review_scores = {}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are these being passed as instance variables, can you refactor all of this into using parameters

@response_type = 'ReviewResponseMap'
@response_maps = ResponseMap.where(reviewed_object_id: self.id, type: @response_type)
if self.vary_by_round
scores_varying_rubrics
else
scores_non_varying_rubrics
end
@review_scores
end

# calculate the avg score and score range for each reviewee(team), only for peer-review
def compute_avg_and_ranges_hash
scores = {}
contributors = self.contributors # assignment_teams
if self.vary_by_round
rounds = self.rounds_of_reviews
(1..rounds).each do |round|
contributors.each do |contributor|
questions = peer_review_questions_for_team(contributor, round)
assessments = ReviewResponseMap.assessments_for(contributor)
assessments = assessments.select {|assessment| assessment.round == round }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use select!{ |assessment| assessment.round == round}

scores[contributor.id] = {} if round == 1
scores[contributor.id][round] = {}
scores[contributor.id][round] = Response.compute_scores(assessments, questions)
end
end
else
contributors.each do |contributor|
questions = peer_review_questions_for_team(contributor)
assessments = ReviewResponseMap.assessments_for(contributor)
scores[contributor.id] = {}
scores[contributor.id] = Response.compute_scores(assessments, questions)
end
end
scores
end

end

private

# Get all of the questions asked during peer review for the given team's work
def peer_review_questions_for_team(team, round_number = nil)
topic_id = SignedUpTeam.find_by(team_id: team.id).topic_id unless team.nil? or SignedUpTeam.find_by(team_id: team.id).nil?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are literally the same checks so just clean this up

review_questionnaire_id = review_questionnaire_id(round_number, topic_id) unless team.nil? or SignedUpTeam.find_by(team_id: team.id).nil?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use || instead of or

Question.where(questionnaire_id: review_questionnaire_id) unless team.nil? or SignedUpTeam.find_by(team_id: team.id).nil?
end

def calc_review_score
unless @corresponding_response.empty?
@this_review_score_raw = Response.assessment_score(response: @corresponding_response, questions: @questions)
if @this_review_score_raw
@this_review_score = ((@this_review_score_raw * 100) / 100.0).round if @this_review_score_raw >= 0.0
end
else
@this_review_score = -1.0
johnbumgardner marked this conversation as resolved.
Show resolved Hide resolved
end
end

def scores_varying_rubrics
rounds = self.rounds_of_reviews
(1..rounds).each do |round|
@response_maps.each do |response_map|
@questions = peer_review_questions_for_team(response_map.reviewee, round)
reviewer = @review_scores[response_map.reviewer_id]
@corresponding_response = Response.where('map_id = ?', response_map.id)
@corresponding_response = @corresponding_response.select {|response| response.round == round } unless @corresponding_response.empty?
@respective_scores = {}
@respective_scores = reviewer[round] unless reviewer.nil? || reviewer[round].nil?
calc_review_score
@respective_scores[response_map.reviewee_id] = @this_review_score
reviewer = {} if reviewer.nil?
reviewer[round] = {} if reviewer[round].nil?
reviewer[round] = @respective_scores
end
end
end

def scores_non_varying_rubrics
@response_maps.each do |response_map|
@questions = peer_review_questions_for_team(response_map.reviewee)
reviewer = @review_scores[response_map.reviewer_id]
@corresponding_response = Response.where('map_id = ?', response_map.id)
@respective_scores = {}
@respective_scores = reviewer unless reviewer.nil?
calc_review_score
@respective_scores[response_map.reviewee_id] = @this_review_score
@review_scores[response_map.reviewer_id] = @respective_scores
end
end
2 changes: 1 addition & 1 deletion app/models/assignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class Assignment < ActiveRecord::Base
include AssignmentAnalytic
include ReviewAssignment
include QuizAssignment
include OnTheFlyCalc
include AssignmentHelper
has_paper_trail
# When an assignment is created, it needs to
# be created as an instance of a subclass of the Assignment (model) class;
Expand Down
100 changes: 0 additions & 100 deletions app/models/on_the_fly_calc.rb

This file was deleted.

86 changes: 86 additions & 0 deletions spec/helpers/assignment_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,95 @@
# AssignmentForm#assignment_questionnaire
# AssignmentForm#questionnaire

let(:assignment_helper) { Class.new { extend AssignmentHelper } }
let(:questionnaire) { create(:questionnaire, id: 1) }
let(:question1) { create(:question, questionnaire: questionnaire, weight: 1, id: 1) }
let(:response) { build(:response, id: 1, map_id: 1, scores: [answer]) }
let(:answer) { Answer.new(answer: 1, comments: 'Answer text', question_id: 1) }
let(:team) { build(:assignment_team) }
let(:assignment) { build(:assignment, id: 1, name: 'Test Assgt') }
let(:questionnaire1) { build(:questionnaire, name: "abc", private: 0, min_question_score: 0, max_question_score: 10, instructor_id: 1234) }
let(:contributor) { build(:assignment_team, id: 1) }
let(:signed_up_team) { build(:signed_up_team, team_id: contributor.id) }

describe '#questionnaire_options' do
it 'throws exception if type argument nil' do
expect { questionnaire_options(nil) }.to raise_exception(NoMethodError)
end
end

describe '#compute_total_score' do
context 'when avg score is nil' do
it 'computes total score for this assignment by summing the score given on all questionnaires' do
assignment_helper = Assignment.new(id: 1, name: 'Test Assgt')
scores = {review1: {scores: {max: 80, min: 0, avg: nil}, assessments: [response]}}
allow(assignment_helper).to receive(:questionnaires).and_return([questionnaire1])
allow(ReviewQuestionnaire).to receive_message_chain(:assignment_questionnaires, :find_by)
.with(no_args).with(assignment_id: 1).and_return(double('AssignmentQuestionnaire', id: 1))
allow(AssignmentQuestionnaire).to receive(:find_by).with(assignment_id: 1, questionnaire_id: nil)
.and_return(double('AssignmentQuestionnaire', used_in_round: 1))
expect(assignment_helper.compute_total_score(scores)).to eq(0)
end
end
end

describe '#compute_review_hash' do
let(:response_map) { create(:review_response_map, id: 1, reviewer_id: 1) }
let(:response_map2) { create(:review_response_map, id: 2, reviewer_id: 2) }
let!(:response_record) { create(:response, id: 1, response_map: response_map) }
let!(:response_record2) { create(:response, id: 2, response_map: response_map2) }
before(:each) do
allow(Response).to receive(:assessment_score).and_return(50, 30)
allow(ResponseMap).to receive(:where).and_return([response_map, response_map2])
allow(SignedUpTeam).to receive(:find_by).with(team_id: contributor.id).and_return(signed_up_team)
end
context 'when current assignment varies rubrics by round' do
it 'scores varying rubrics and returns review scores' do
allow(assignment).to receive(:vary_by_round).and_return(true)
allow(assignment).to receive(:rounds_of_reviews).and_return(1)
expect(assignment.compute_reviews_hash).to eq({})
end
end
context 'when current assignment does not vary rubrics by round' do
it 'scores rubrics and returns review scores' do
allow(assignment).to receive(:vary_by_round).and_return(false)
allow(DueDate).to receive(:get_next_due_date).with(assignment.id).and_return(double(:DueDate, round: 1))
expect(assignment.compute_reviews_hash).to eq(1 => {1 => 50}, 2 => {1 => 30})
end
end
end

describe '#compute_avg_and_ranges_hash' do
before(:each) do
score = {min: 50.0, max: 50.0, avg: 50.0}
allow(assignment_helper).to receive(:contributors).and_return([contributor])
allow(Response).to receive(:compute_scores).with([], [question1]).and_return(score)
allow(ReviewResponseMap).to receive(:assessments_for).with(contributor).and_return([])
allow(SignedUpTeam).to receive(:find_by).with(team_id: contributor.id).and_return(signed_up_team)
allow(assignment_helper).to receive(:review_questionnaire_id).and_return(1)
end
context 'when current assignment varies rubrics by round' do
it 'computes avg score and score range for each team in each round and return scores' do
allow(assignment_helper).to receive(:vary_by_round).and_return(true)
allow(assignment_helper).to receive(:rounds_of_reviews).and_return(1)
expect(assignment_helper.compute_avg_and_ranges_hash).to eq(1 => {1 => {min: 50.0, max: 50.0, avg: 50.0}})
end
end
context 'when current assignment does not vary rubrics by round' do
it 'computes avg score and score range for each team and return scores' do
allow(assignment_helper).to receive(:vary_by_round).and_return(false)
expect(assignment_helper.compute_avg_and_ranges_hash).to eq(1 => {min: 50.0, max: 50.0, avg: 50.0})
end
end
end
describe '#peer_review_questions_for_team' do
context 'when there is no signed up team' do
it 'peer review questions should return nil' do
val = assignment_helper.send(:peer_review_questions_for_team, nil)
expect(val).to be_nil
end
end
end


end