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.
4 years ago · bb20ff506b
parent 5a8d032dd0
commit bb20ff506b
3 changed files with 43 additions and 27 deletions
--- a/2
+++ b/2
@ -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
--- 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
--- 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