From 8516111a15763531fc1a2a26c351405f2d723735 Mon Sep 17 00:00:00 2001 From: Nelson Jovel Date: Wed, 12 Jun 2024 10:35:03 -0700 Subject: [PATCH] chore: refactor analyze page --- app/presenters/analyze/graph/all_data.rb | 2 +- .../analyze/graph/column/all_admin.rb | 4 +- .../analyze/graph/column/all_data.rb | 37 ++++++++++++ .../analyze/graph/column/all_student.rb | 6 +- .../analyze/graph/column/all_survey_data.rb | 4 +- .../analyze/graph/column/all_teacher.rb | 4 +- .../graph/column/ell_column/score_for_ell.rb | 3 +- .../column/gender_column/score_for_gender.rb | 3 +- .../graph/column/grade/score_for_grade.rb | 5 +- .../column/grouped_bar_column_presenter.rb | 56 ++++++------------- .../column/income_column/score_for_income.rb | 3 +- .../analyze/graph/column/score_for_race.rb | 3 +- .../column/sped_column/score_for_sped.rb | 3 +- .../column/gender_column/unknown_spec.rb | 42 +++++++------- .../grouped_bar_column_presenter_spec.rb | 18 +++--- 15 files changed, 100 insertions(+), 93 deletions(-) create mode 100644 app/presenters/analyze/graph/column/all_data.rb diff --git a/app/presenters/analyze/graph/all_data.rb b/app/presenters/analyze/graph/all_data.rb index 93a9bd50..d360e66b 100644 --- a/app/presenters/analyze/graph/all_data.rb +++ b/app/presenters/analyze/graph/all_data.rb @@ -14,7 +14,7 @@ module Analyze end def columns - [AllStudent, AllTeacher, AllAdmin, GroupedBarColumnPresenter] + [AllStudent, AllTeacher, AllAdmin, Analyze::Graph::Column::AllData] end def source diff --git a/app/presenters/analyze/graph/column/all_admin.rb b/app/presenters/analyze/graph/column/all_admin.rb index 74af6ed0..159e37df 100644 --- a/app/presenters/analyze/graph/column/all_admin.rb +++ b/app/presenters/analyze/graph/column/all_admin.rb @@ -26,8 +26,8 @@ module Analyze ["data not", "available"] end - def score(year_index) - measure.admin_score(school:, academic_year: academic_years[year_index]) + def score(academic_year) + measure.admin_score(school:, academic_year:) end def type diff --git a/app/presenters/analyze/graph/column/all_data.rb b/app/presenters/analyze/graph/column/all_data.rb new file mode 100644 index 00000000..3bd7fe00 --- /dev/null +++ b/app/presenters/analyze/graph/column/all_data.rb @@ -0,0 +1,37 @@ +module Analyze + module Graph + module Column + class AllData < GroupedBarColumnPresenter + def label + %w[All Data] + end + + def show_irrelevancy_message? + false + end + + def show_insufficient_data_message? + scores = academic_years.map do |year| + measure.score(school:, academic_year: year) + end + + scores.none? do |score| + score.meets_teacher_threshold? || score.meets_student_threshold? || score.meets_admin_data_threshold? + end + end + + def score(academic_year) + measure.score(school:, academic_year:) || 0 + end + + def type + :all_data + end + + def basis + "student surveys" + end + end + end + end +end diff --git a/app/presenters/analyze/graph/column/all_student.rb b/app/presenters/analyze/graph/column/all_student.rb index 788dca38..e51f201c 100644 --- a/app/presenters/analyze/graph/column/all_student.rb +++ b/app/presenters/analyze/graph/column/all_student.rb @@ -17,11 +17,11 @@ module Analyze measure.score(school:, academic_year: year) end - scores.all? { |score| !score.meets_student_threshold? } + scores.none? { |score| score.meets_student_threshold? } end - def score(year_index) - measure.student_score(school:, academic_year: academic_years[year_index]) + def score(academic_year) + measure.student_score(school:, academic_year:) end def type diff --git a/app/presenters/analyze/graph/column/all_survey_data.rb b/app/presenters/analyze/graph/column/all_survey_data.rb index c3e42c94..004bd79b 100644 --- a/app/presenters/analyze/graph/column/all_survey_data.rb +++ b/app/presenters/analyze/graph/column/all_survey_data.rb @@ -20,8 +20,8 @@ module Analyze scores.all? { |score| !score.meets_student_threshold? && !score.meets_teacher_threshold? } end - def score(year_index) - combined_score(school:, academic_year: academic_years[year_index]) + def score(academic_year) + combined_score(school:, academic_year:) end def type diff --git a/app/presenters/analyze/graph/column/all_teacher.rb b/app/presenters/analyze/graph/column/all_teacher.rb index 15f665f3..4495bfe5 100644 --- a/app/presenters/analyze/graph/column/all_teacher.rb +++ b/app/presenters/analyze/graph/column/all_teacher.rb @@ -24,8 +24,8 @@ module Analyze scores.all? { |score| !score.meets_teacher_threshold? } end - def score(year_index) - measure.teacher_score(school:, academic_year: academic_years[year_index]) + def score(academic_year) + measure.teacher_score(school:, academic_year:) end def type diff --git a/app/presenters/analyze/graph/column/ell_column/score_for_ell.rb b/app/presenters/analyze/graph/column/ell_column/score_for_ell.rb index cd39cd0f..97589b90 100644 --- a/app/presenters/analyze/graph/column/ell_column/score_for_ell.rb +++ b/app/presenters/analyze/graph/column/ell_column/score_for_ell.rb @@ -3,8 +3,7 @@ module Analyze module Column module EllColumn module ScoreForEll - def score(year_index) - academic_year = academic_years[year_index] + def score(academic_year) meets_student_threshold = sufficient_student_responses?(academic_year:) return Score::NIL_SCORE unless meets_student_threshold diff --git a/app/presenters/analyze/graph/column/gender_column/score_for_gender.rb b/app/presenters/analyze/graph/column/gender_column/score_for_gender.rb index 2b5e22c4..b7f8c77d 100644 --- a/app/presenters/analyze/graph/column/gender_column/score_for_gender.rb +++ b/app/presenters/analyze/graph/column/gender_column/score_for_gender.rb @@ -3,8 +3,7 @@ module Analyze module Column module GenderColumn module ScoreForGender - def score(year_index) - academic_year = academic_years[year_index] + def score(academic_year) meets_student_threshold = sufficient_student_responses?(academic_year:) return Score::NIL_SCORE unless meets_student_threshold diff --git a/app/presenters/analyze/graph/column/grade/score_for_grade.rb b/app/presenters/analyze/graph/column/grade/score_for_grade.rb index 8b2e5d15..38548c47 100644 --- a/app/presenters/analyze/graph/column/grade/score_for_grade.rb +++ b/app/presenters/analyze/graph/column/grade/score_for_grade.rb @@ -3,13 +3,12 @@ module Analyze module Column module Grade module ScoreForGrade - def score(year_index) - academic_year = academic_years[year_index] + def score(academic_year) meets_student_threshold = sufficient_student_responses?(academic_year:) return Score::NIL_SCORE unless meets_student_threshold averages = SurveyItemResponse.averages_for_grade(measure.student_survey_items, school, - academic_years[year_index], grade) + academic_year, grade) average = bubble_up_averages(averages:).round(2) Score.new(average:, diff --git a/app/presenters/analyze/graph/column/grouped_bar_column_presenter.rb b/app/presenters/analyze/graph/column/grouped_bar_column_presenter.rb index 450ad4b9..c08da3c2 100644 --- a/app/presenters/analyze/graph/column/grouped_bar_column_presenter.rb +++ b/app/presenters/analyze/graph/column/grouped_bar_column_presenter.rb @@ -20,10 +20,6 @@ module Analyze @number_of_columns = number_of_columns end - def score(year_index) - measure.score(school:, academic_year: academic_years[year_index]) || 0 - end - def bars @bars ||= yearly_scores.map.each_with_index do |yearly_score, index| year = yearly_score.year @@ -31,25 +27,31 @@ module Analyze score: yearly_score.score, x_position: bar_x(index), color: bar_color(year)) - end + end.reject(&:blank?).select { |bar| bar.score.average&.positive? } end def label - %w[All Data] + raise NotImplementedError end def show_irrelevancy_message? - false + raise NotImplementedError end def show_insufficient_data_message? - scores = academic_years.map do |year| - measure.score(school:, academic_year: year) - end + raise NotImplementedError + end + + def score(academic_year) + raise NotImplementedError + end - scores.all? do |score| - !score.meets_teacher_threshold? && !score.meets_student_threshold? && !score.meets_admin_data_threshold? - end + def basis + "student surveys" + end + + def type + raise NotImplementedError end def column_midpoint @@ -101,14 +103,6 @@ module Analyze number_of_columns end - def basis - "student surveys" - end - - def type - :all_data - end - def show_popover? %i[student teacher].include? type end @@ -126,17 +120,6 @@ module Analyze ["survey response", "rate below 25%"] end - def sufficient_data?(year_index) - case basis - when "student" - score(year_index).meets_student_threshold - when "teacher" - score(year_index).meets_teacher_threshold - else - true - end - end - def grades Respondent.by_school_and_year(school:, academic_year: academic_years)&.enrollment_by_grade&.keys end @@ -145,12 +128,9 @@ module Analyze YearlyScore = Struct.new(:year, :score) def yearly_scores - yearly_scores = academic_years.each_with_index.map do |year, index| - YearlyScore.new(year, score(index)) - end - yearly_scores.reject do |yearly_score| - yearly_score.score.blank? - end + yearly_scores = academic_years.each.map do |year| + YearlyScore.new(year, score(year)) + end.reject { |year| year.score.nil? || year.score.blank? } end def bar_color(year) diff --git a/app/presenters/analyze/graph/column/income_column/score_for_income.rb b/app/presenters/analyze/graph/column/income_column/score_for_income.rb index 52a23244..d1580f99 100644 --- a/app/presenters/analyze/graph/column/income_column/score_for_income.rb +++ b/app/presenters/analyze/graph/column/income_column/score_for_income.rb @@ -3,8 +3,7 @@ module Analyze module Column module IncomeColumn module ScoreForIncome - def score(year_index) - academic_year = academic_years[year_index] + def score(academic_year) meets_student_threshold = sufficient_student_responses?(academic_year:) return Score::NIL_SCORE unless meets_student_threshold diff --git a/app/presenters/analyze/graph/column/score_for_race.rb b/app/presenters/analyze/graph/column/score_for_race.rb index 5fdc2e90..978d511c 100644 --- a/app/presenters/analyze/graph/column/score_for_race.rb +++ b/app/presenters/analyze/graph/column/score_for_race.rb @@ -2,8 +2,7 @@ module Analyze module Graph module Column module ScoreForRace - def score(year_index) - academic_year = academic_years[year_index] + def score(academic_year) meets_student_threshold = sufficient_student_responses?(academic_year:) return Score::NIL_SCORE unless meets_student_threshold diff --git a/app/presenters/analyze/graph/column/sped_column/score_for_sped.rb b/app/presenters/analyze/graph/column/sped_column/score_for_sped.rb index 4727e374..25831802 100644 --- a/app/presenters/analyze/graph/column/sped_column/score_for_sped.rb +++ b/app/presenters/analyze/graph/column/sped_column/score_for_sped.rb @@ -3,8 +3,7 @@ module Analyze module Column module SpedColumn module ScoreForSped - def score(year_index) - academic_year = academic_years[year_index] + def score(academic_year) meets_student_threshold = sufficient_student_responses?(academic_year:) return Score::NIL_SCORE unless meets_student_threshold diff --git a/spec/models/analyze/graph/column/gender_column/unknown_spec.rb b/spec/models/analyze/graph/column/gender_column/unknown_spec.rb index 82a268a0..defb9d01 100644 --- a/spec/models/analyze/graph/column/gender_column/unknown_spec.rb +++ b/spec/models/analyze/graph/column/gender_column/unknown_spec.rb @@ -1,4 +1,4 @@ -require 'rails_helper' +require "rails_helper" RSpec.describe Analyze::Graph::Column::GenderColumn::Unknown, type: :model do let(:school) { create(:school) } @@ -9,65 +9,65 @@ RSpec.describe Analyze::Graph::Column::GenderColumn::Unknown, type: :model do let(:number_of_columns) { 1 } let(:gender) { create(:gender, qualtrics_code: 99) } - let(:subcategory) { create(:subcategory, subcategory_id: '1A') } - let(:measure) { create(:measure, measure_id: '1A-iii', subcategory:) } + let(:subcategory) { create(:subcategory, subcategory_id: "1A") } + let(:measure) { create(:measure, measure_id: "1A-iii", subcategory:) } let(:scale) { create(:student_scale, measure:) } let(:survey_item) { create(:student_survey_item, scale:) } let(:teacher_scale) { create(:teacher_scale, measure:) } let(:teacher_survey_item) { create(:teacher_survey_item, scale:) } - context 'when no teacher responses exist' do - context 'when there are insufficient unknown students' do - it 'reports a score of 3 when the average is 3' do + context "when no teacher responses exist" do + context "when there are insufficient unknown students" do + it "reports a score of 3 when the average is 3" do expect(Analyze::Graph::Column::GenderColumn::Unknown.new(school:, academic_years:, position:, measure:, - number_of_columns:).score(0).average).to eq(nil) + number_of_columns:).score(academic_year).average).to eq(nil) end - it 'reports insufficient data' do + it "reports insufficient data" do expect(Analyze::Graph::Column::GenderColumn::Unknown.new(school:, academic_years:, position:, measure:, number_of_columns:).sufficient_student_responses?(academic_year:)).to eq(false) end end - context 'when there are sufficient unknown students' do + context "when there are sufficient unknown students" do before :each do create_list(:survey_item_response, 10, school:, academic_year:, gender:, survey_item:) end - it 'reports a score of 3 when the average is 3' do + it "reports a score of 3 when the average is 3" do expect(Analyze::Graph::Column::GenderColumn::Unknown.new(school:, academic_years:, position:, measure:, - number_of_columns:).score(0).average).to eq(3) + number_of_columns:).score(academic_year).average).to eq(3) end - it 'reports sufficient data' do + it "reports sufficient data" do expect(Analyze::Graph::Column::GenderColumn::Unknown.new(school:, academic_years:, position:, measure:, number_of_columns:).sufficient_student_responses?(academic_year:)).to eq(true) end end end - context 'when teacher responses exist' do + context "when teacher responses exist" do before :each do create_list(:survey_item_response, 10, school:, academic_year:, survey_item: teacher_survey_item, likert_score: 5) end - context 'when there are insufficient unknown students' do - it 'reports a score of 3 when the average is 3' do + context "when there are insufficient unknown students" do + it "reports a score of 3 when the average is 3" do expect(Analyze::Graph::Column::GenderColumn::Unknown.new(school:, academic_years:, position:, measure:, - number_of_columns:).score(0).average).to eq(nil) + number_of_columns:).score(academic_year).average).to eq(nil) end - it 'reports insufficient data' do + it "reports insufficient data" do expect(Analyze::Graph::Column::GenderColumn::Unknown.new(school:, academic_years:, position:, measure:, number_of_columns:).sufficient_student_responses?(academic_year:)).to eq(false) end end - context 'when there are sufficient unknown students' do + context "when there are sufficient unknown students" do before :each do create_list(:survey_item_response, 10, school:, academic_year:, gender:, survey_item:) end - it 'reports a score of 3 when the average is 3' do + it "reports a score of 3 when the average is 3" do expect(Analyze::Graph::Column::GenderColumn::Unknown.new(school:, academic_years:, position:, measure:, - number_of_columns:).score(0).average).to eq(3) + number_of_columns:).score(academic_year).average).to eq(3) end - it 'reports sufficient data' do + it "reports sufficient data" do expect(Analyze::Graph::Column::GenderColumn::Unknown.new(school:, academic_years:, position:, measure:, number_of_columns:).sufficient_student_responses?(academic_year:)).to eq(true) end diff --git a/spec/presenters/grouped_bar_column_presenter_spec.rb b/spec/presenters/grouped_bar_column_presenter_spec.rb index fb60b5c6..194e1218 100644 --- a/spec/presenters/grouped_bar_column_presenter_spec.rb +++ b/spec/presenters/grouped_bar_column_presenter_spec.rb @@ -73,8 +73,8 @@ describe GroupedBarColumnPresenter do end let(:all_data_presenter) do - GroupedBarColumnPresenter.new measure: measure_composed_of_student_and_teacher_items, school:, academic_years:, - position: 0, number_of_columns: 3 + Analyze::Graph::Column::AllData.new measure: measure_composed_of_student_and_teacher_items, school:, academic_years:, + position: 0, number_of_columns: 3 end before do @@ -120,8 +120,8 @@ describe GroupedBarColumnPresenter do end it "returns a score that is an average of the likert scores " do - expect(all_data_presenter.score(0).average).to eq 4.5 - expect(all_data_presenter.score(1).average).to eq nil + expect(all_data_presenter.score(academic_year).average).to eq 4.5 + expect(all_data_presenter.score(another_academic_year).average).to eq nil expect(all_data_presenter.academic_years[0].range).to be academic_year.range expect(all_data_presenter.academic_years[1].range).to be another_academic_year.range end @@ -133,8 +133,8 @@ describe GroupedBarColumnPresenter do academic_year: another_academic_year, likert_score: 3) end it "returns independent scores for each year of data" do - expect(all_data_presenter.score(0).average).to eq 4.5 - expect(all_data_presenter.score(1).average).to eq 4 + expect(all_data_presenter.score(academic_year).average).to eq 4.5 + expect(all_data_presenter.score(another_academic_year).average).to eq 4 end end end @@ -144,12 +144,8 @@ describe GroupedBarColumnPresenter do it_behaves_like "measure_name" it_behaves_like "column_midpoint" - it "returns an emtpy set of bars" do - expect(student_presenter.bars).to eq [] - end - it "returns an emty score" do - expect(student_presenter.score(year_index).average).to eq nil + expect(student_presenter.score(academic_year).average).to eq nil end it "shows the irrelevancy message " do