From bb20ff506b2b5b1287d5081dbf210ca05c18d49f Mon Sep 17 00:00:00 2001 From: Nelson Jovel Date: Tue, 11 Jan 2022 21:53:04 +0100 Subject: [PATCH] Correct error affecting subcategory scores where teacher or student survey items below the threshold were still being included in the score for the subcategory. Ensure queries for survey item responses take into account the school and academic year. --- Gemfile | 2 +- app/models/survey_item_response.rb | 62 ++++++++++++------- spec/presenters/subcategory_presenter_spec.rb | 6 +- 3 files changed, 43 insertions(+), 27 deletions(-) diff --git a/Gemfile b/Gemfile index 1b394bde..5405e92f 100644 --- a/Gemfile +++ b/Gemfile @@ -66,11 +66,11 @@ group :development do gem 'listen', '~> 3.0.5' gem 'web-console' # Spring speeds up development by keeping your application running in the background. Read more: https://github.com/rails/spring + gem 'bullet' gem 'nested_scaffold' gem 'seed_dump' gem 'spring' gem 'spring-watcher-listen', '~> 2.0.0' - gem 'bullet' end group 'test' do diff --git a/app/models/survey_item_response.rb b/app/models/survey_item_response.rb index de62a336..323e4ba2 100644 --- a/app/models/survey_item_response.rb +++ b/app/models/survey_item_response.rb @@ -11,23 +11,32 @@ class SurveyItemResponse < ActiveRecord::Base sufficient_data?(measure: measure, school: school, academic_year: academic_year) end - SurveyItemResponse.for_measures(measures) - .where(academic_year: academic_year, school: school) - .average(:likert_score) + return nil unless measures.size.positive? + + measures.map do |measure| + responses_for_measure(measure: measure, school: school, academic_year: academic_year).average(:likert_score) + end.average end - def self.score_for_measure(measure:, school:, academic_year:) + def self.responses_for_measure(measure:, school:, academic_year:) meets_teacher_threshold = teacher_sufficient_data? measure: measure, school: school, academic_year: academic_year meets_student_threshold = student_sufficient_data? measure: measure, school: school, academic_year: academic_year meets_all_thresholds = meets_teacher_threshold && meets_student_threshold - survey_item_responses = if meets_all_thresholds - SurveyItemResponse.for_measure(measure) - elsif meets_teacher_threshold - SurveyItemResponse.teacher_responses_for_measure(measure) - elsif meets_student_threshold - SurveyItemResponse.student_responses_for_measure(measure) - end + if meets_all_thresholds + SurveyItemResponse.for_measure(measure, school, academic_year) + elsif meets_teacher_threshold + SurveyItemResponse.teacher_responses_for_measure(measure, school, academic_year) + elsif meets_student_threshold + SurveyItemResponse.student_responses_for_measure(measure, school, academic_year) + end + end + + def self.score_for_measure(measure:, school:, academic_year:) + meets_teacher_threshold = teacher_sufficient_data? measure: measure, school: school, academic_year: academic_year + meets_student_threshold = student_sufficient_data? measure: measure, school: school, academic_year: academic_year + + survey_item_responses = responses_for_measure(measure: measure, school: school, academic_year: academic_year) score_for_measure = survey_item_responses.average(:likert_score) unless survey_item_responses.nil? @@ -40,20 +49,29 @@ class SurveyItemResponse < ActiveRecord::Base meets_teacher_threshold || meets_student_threshold end - scope :for_measure, ->(measure) { joins(:survey_item).where('survey_items.measure_id': measure.id) } - scope :for_measures, ->(measures) { joins(:survey_item).where('survey_items.measure_id': measures.map(&:id)) } - scope :teacher_responses_for_measure, lambda { |measure| - for_measure(measure).where("survey_items.survey_item_id LIKE 't-%'") + scope :for_measure, lambda { |measure, school, academic_year| + joins(:survey_item) + .where('survey_items.measure': measure) + .where(school: school, academic_year: academic_year) + } + scope :for_measures, lambda { |measures, school, academic_year| + joins(:survey_item) + .where('survey_items.measure': measures) + .where(school: school, academic_year: academic_year) + } + + scope :teacher_responses_for_measure, lambda { |measure, school, academic_year| + for_measure(measure, school, academic_year) + .where("survey_items.survey_item_id LIKE 't-%'") } - scope :student_responses_for_measure, lambda { |measure| - for_measure(measure).where("survey_items.survey_item_id LIKE 's-%'") + scope :student_responses_for_measure, lambda { |measure, school, academic_year| + for_measure(measure, school, academic_year) + .where("survey_items.survey_item_id LIKE 's-%'") } def self.student_sufficient_data?(measure:, school:, academic_year:) if measure.includes_student_survey_items? - student_survey_item_responses = SurveyItemResponse.student_responses_for_measure(measure).where( - academic_year: academic_year, school: school - ) + student_survey_item_responses = SurveyItemResponse.student_responses_for_measure(measure, school, academic_year) average_number_of_survey_item_responses = student_survey_item_responses.count / measure.student_survey_items.count meets_student_threshold = average_number_of_survey_item_responses >= STUDENT_RESPONSE_THRESHOLD @@ -63,9 +81,7 @@ class SurveyItemResponse < ActiveRecord::Base def self.teacher_sufficient_data?(measure:, school:, academic_year:) if measure.includes_teacher_survey_items? - teacher_survey_item_responses = SurveyItemResponse.teacher_responses_for_measure(measure).where( - academic_year: academic_year, school: school - ) + teacher_survey_item_responses = SurveyItemResponse.teacher_responses_for_measure(measure, school, academic_year) average_number_of_survey_item_responses = teacher_survey_item_responses.count / measure.teacher_survey_items.count meets_teacher_threshold = average_number_of_survey_item_responses >= TEACHER_RESPONSE_THRESHOLD diff --git a/spec/presenters/subcategory_presenter_spec.rb b/spec/presenters/subcategory_presenter_spec.rb index 13245e12..d5205995 100644 --- a/spec/presenters/subcategory_presenter_spec.rb +++ b/spec/presenters/subcategory_presenter_spec.rb @@ -27,6 +27,7 @@ describe SubcategoryPresenter do create(:admin_data_item, measure: measure_of_only_admin_data, watch_low_benchmark: 2, growth_low_benchmark: 3, approval_low_benchmark: 3.5, ideal_low_benchmark: 4) + # Adding responses corresponding to different years and schools should not pollute the score calculations create_survey_item_responses_for_different_years_and_schools(survey_item1) return SubcategoryPresenter.new(subcategory: subcategory, academic_year: academic_year, school: school) @@ -45,7 +46,6 @@ describe SubcategoryPresenter do end it 'returns a gauge presenter responsible for the aggregate admin data and survey item response likert scores' do - # average scores will be 3 and growth low benchmark is 2.916 : (4.25 + 1.5 + 3)/3 expect(subcategory_presenter.gauge_presenter.title).to eq 'Growth' end @@ -65,8 +65,8 @@ describe SubcategoryPresenter do def create_survey_item_responses_for_different_years_and_schools(survey_item) create_list(:survey_item_response, SurveyItemResponse::TEACHER_RESPONSE_THRESHOLD, survey_item: survey_item, - school: school, likert_score: 1) + school: School.new(name: 'Worst School' , dese_id: 2), likert_score: 1) create_list(:survey_item_response, SurveyItemResponse::TEACHER_RESPONSE_THRESHOLD, survey_item: survey_item, - academic_year: academic_year, likert_score: 1) + academic_year: AcademicYear.create(range: '2000-01'), likert_score: 1) end end