From e741b60bec8810ada2116ec99d4436553fdecc53 Mon Sep 17 00:00:00 2001 From: Nelson Jovel Date: Fri, 8 Nov 2024 15:36:34 -0800 Subject: [PATCH] fix: make sure all likert scores get counted even when the survey item id has different capitalization. Add tests for uploading parent data. Change the parent response rate calcuation to count all students in the school instead of just for the grades that were given the student survey --- app/lib/seeder.rb | 3 + app/presenters/overview/overview_presenter.rb | 4 - .../parent_response_rate_presenter.rb | 1 + .../student_response_rate_presenter.rb | 1 + .../teacher_response_rate_presenter.rb | 1 + app/services/survey_item_values.rb | 15 +++- app/services/survey_responses_data_loader.rb | 1 + .../test_2020-21_parent_survey_responses.csv | 8 ++ spec/services/survey_item_values_spec.rb | 8 ++ .../survey_responses_data_loader_spec.rb | 84 +++++++++++++++++++ 10 files changed, 118 insertions(+), 8 deletions(-) create mode 100644 spec/fixtures/test_2020-21_parent_survey_responses.csv diff --git a/app/lib/seeder.rb b/app/lib/seeder.rb index ff0e6536..f781c366 100644 --- a/app/lib/seeder.rb +++ b/app/lib/seeder.rb @@ -51,6 +51,7 @@ class Seeder def seed_sqm_framework(csv_file) admin_data_item_ids = [] + survey_item_ids = [] CSV.parse(File.read(csv_file), headers: true) do |row| next if row["Source"] == "No source" @@ -105,6 +106,7 @@ class Seeder survey_item.ideal_low_benchmark = ideal_low if ideal_low survey_item.on_short_form = marked? on_short_form survey_item.update! prompt: row["Question/item (23-24)"].strip + survey_item_ids << survey_item.id end active_admin = row["Active admin & survey items"] @@ -122,6 +124,7 @@ class Seeder end AdminDataValue.where.not(admin_data_item_id: admin_data_item_ids).delete_all AdminDataItem.where.not(id: admin_data_item_ids).delete_all + SurveyItem.where.not(id: survey_item_ids).delete_all end def seed_demographics(csv_file) diff --git a/app/presenters/overview/overview_presenter.rb b/app/presenters/overview/overview_presenter.rb index 1b13f2b5..6c20e5dc 100644 --- a/app/presenters/overview/overview_presenter.rb +++ b/app/presenters/overview/overview_presenter.rb @@ -47,10 +47,6 @@ class Overview::OverviewPresenter ParentResponseRatePresenter.new(school: @school, academic_year: @academic_year) end - def parent_response_rate_presenter - ResponseRatePresenter.new(focus: :parent, school: @school, academic_year: @academic_year) - end - def presenter_for_measure(measure) score = measure.score(school: @school, academic_year: @academic_year) diff --git a/app/presenters/parent_response_rate_presenter.rb b/app/presenters/parent_response_rate_presenter.rb index b221bfe1..1574ee92 100644 --- a/app/presenters/parent_response_rate_presenter.rb +++ b/app/presenters/parent_response_rate_presenter.rb @@ -22,3 +22,4 @@ class ParentResponseRatePresenter < ResponseRatePresenter "parent" end end + diff --git a/app/presenters/student_response_rate_presenter.rb b/app/presenters/student_response_rate_presenter.rb index 530528c6..12c930ab 100644 --- a/app/presenters/student_response_rate_presenter.rb +++ b/app/presenters/student_response_rate_presenter.rb @@ -40,3 +40,4 @@ class StudentResponseRatePresenter < ResponseRatePresenter "student" end end + diff --git a/app/presenters/teacher_response_rate_presenter.rb b/app/presenters/teacher_response_rate_presenter.rb index 2144cba8..94eb7e84 100644 --- a/app/presenters/teacher_response_rate_presenter.rb +++ b/app/presenters/teacher_response_rate_presenter.rb @@ -18,3 +18,4 @@ class TeacherResponseRatePresenter < ResponseRatePresenter "teacher" end end + diff --git a/app/services/survey_item_values.rb b/app/services/survey_item_values.rb index 36521830..eec27f59 100644 --- a/app/services/survey_item_values.rb +++ b/app/services/survey_item_values.rb @@ -3,8 +3,8 @@ class SurveyItemValues def initialize(row:, headers:, survey_items:, schools:, academic_years: AcademicYear.all) @row = row - # Remove any newlines in headers - headers = headers.map { |item| item.delete("\n") if item.present? } + # Remove any newlines in headers and + @headers = normalize_headers(headers:) @headers = include_all_headers(headers:) @survey_items = survey_items @schools = schools @@ -25,6 +25,13 @@ class SurveyItemValues copy_data_to_main_column(main: /Gender/i, secondary: /Gender Secondary|Gender-1/i) end + def normalize_headers(headers:) + headers + .select(&:present?) + .map { |item| item.strip } + .map { |item| item.downcase if item.match(/[stp]-/i) } + end + def copy_data_to_main_column(main:, secondary:) main_column = headers.find { |header| main.match(header) } row[main_column] = value_from(pattern: secondary) if row[main_column].nil? @@ -97,7 +104,7 @@ class SurveyItemValues end def likert_score(survey_item_id:) - row[survey_item_id] || row["#{survey_item_id}-1"] + row[survey_item_id] || row["#{survey_item_id}-1"] || value_from(pattern: /#{survey_item_id}/i) end def school @@ -196,7 +203,7 @@ class SurveyItemValues output = nil matches = headers.select do |header| pattern.match(header) - end.map { |item| item.delete("\n") } + end matches.each do |match| output ||= row[match]&.strip diff --git a/app/services/survey_responses_data_loader.rb b/app/services/survey_responses_data_loader.rb index a80154f8..6da9ef71 100644 --- a/app/services/survey_responses_data_loader.rb +++ b/app/services/survey_responses_data_loader.rb @@ -165,6 +165,7 @@ class SurveyResponsesDataLoader .parse(headers) .first .filter(&:present?) + .map(&:downcase) .filter { |header| header.start_with?("t-", "s-", "p-") } end end diff --git a/spec/fixtures/test_2020-21_parent_survey_responses.csv b/spec/fixtures/test_2020-21_parent_survey_responses.csv new file mode 100644 index 00000000..0e1091c8 --- /dev/null +++ b/spec/fixtures/test_2020-21_parent_survey_responses.csv @@ -0,0 +1,8 @@ +StartDate,EndDate,Status,IPAddress,Progress,Duration (in seconds),Finished,RecordedDate,ResponseId,DistributionChannel,UserLanguage,DESE ID,Number of Children,Gender-1,Gender-1_7_TEXT,Race-1,Race-1_7_TEXT,Gender-2,Gender-2_7_TEXT,Race-2,Race-2_7_TEXT,Gender-3,Gender-3_7_TEXT,Race-3,Race-3_7_TEXT,Gender-4,Gender-4_7_TEXT,Race-4,Race-4_7_TEXT,Gender-5,Gender-5_7_TEXT,Race-5,Race-5_7_TEXT,Gender,Gender_7_TEXT,p-scrp-q3,p-scrp-q2,p-valm-q1,p-valm-q2,p-valm-q3,p-valm-q4,p-comm-q1,p-comm-q2,p-comm-q3,p-tcom-q1,P-tcom-q2,p-tcom-q3,p-evnt-q4,p-comm-q4,p-evnt-q3,p-evnt-q1,p-evnt-q2,p-socx-q3,p-socx-q4,p-scrp-q1,p-socx-q1,p-sosu-q1,p-sosu-q2,p-sosu-q3,p-socx-q2,p-sosu-q4,p-phys-q3,p-acpr-q1,p-acpr-q2,p-acpr-q3,p-acpr-q4,p-cure-q1,p-cure-q2,p-cure-q3,p-cure-q4,Housing,Housing_100_TEXT,Employment,Employment_100_TEXT,Caregivers,Caregivers_100_TEXT,Education,Education_100_TEXT,Benefits,Benefits_100_TEXT,Language,Language_100_TEXT,Raw Income,Income,Raw ELL,ELL,Raw SpEd,SpEd,Progress Count,Race +5/1/2024 10:04:34,5/1/2024 10:10:49,0,72.93.86.98,100,374,1,2021-03-31T10:01:36,parent_survey_response_1,email,EN,1500025,1,,,,,,,,,,,,,,,,,,,,,2,,4,5,5,4,5,5,5,5,5,4,4,5,4,5,3,4,5,4,4,5,5,5,5,5,5,5,1,2,2,2,1,4,5,5,5,1,,1,,2,,5,,2,,1,,,Economically Disadvantaged - N,,Not ELL,,Not Special Education,34,99 +5/1/2024 10:03:52,5/1/2024 10:14:42,0,73.69.182.58,100,649,1,2021-04-01T10:01:36,parent_survey_response_2,email,EN,1500025,1,,,,,,,,,,,,,,,,,,,,,1,,4,4,5,5,5,5,5,5,5,5,5,5,3,5,4,5,5,5,5,5,4,4,4,4,5,5,1,5,4,5,5,5,5,5,5,1,,99,,2,,3,,1,,1,,,Economically Disadvantaged - N,,Not ELL,,Not Special Education,34,99 +5/1/2024 10:06:44,5/1/2024 10:15:41,0,50.235.109.170,100,537,1,2021-04-02T10:01:36,parent_survey_response_3,email,EN,1500025,2,2,,5,,2,,5,,,,,,,,,,,,,,2,,5,5,5,4,5,5,5,5,5,4,4,5,4,4,3,4,4,4,4,5,4,4,5,5,2,5,3,4,4,4,4,5,5,5,5,1,,1,,3,,6,,2,,1,,,Economically Disadvantaged - N,,Not ELL,,Not Special Education,34,5 +5/1/2024 10:14:23,5/1/2024 10:22:22,0,73.38.238.192,100,478,1,2021-04-03T10:01:36,parent_survey_response_4,email,EN,1500025,1,,,,,,,,,,,,,,,,,,,,,1,,5,5,5,5,5,5,5,4,5,4,4,4,2,5,4,5,4,5,5,5,3,5,5,5,2,5,1,5,5,5,5,5,5,5,5,1,,1,,2,,5,,2,,1,,,Economically Disadvantaged - N,,Not ELL,,Not Special Education,34,99 +5/1/2024 10:18:39,5/1/2024 10:23:49,0,73.69.158.255,100,310,1,2021-04-04T10:01:36,parent_survey_response_5,email,EN,1500025,2,2,,5,,1,,5,,,,,,,,,,,,,,2,,5,4,5,5,5,5,1,1,1,1,1,1,3,1,4,4,5,1,1,1,4,1,1,1,4,5,1,5,5,5,5,1,5,1,1,1,,"2,4",,3,,5,,2,,1,,,Economically Disadvantaged - N,,Not ELL,,Not Special Education,34,5 +5/1/2024 10:20:30,5/1/2024 10:25:16,0,73.182.146.201,100,285,1,2021-04-05T10:01:36,parent_survey_response_6,email,EN,1500025,1,,,,,,,,,,,,,,,,,,,,,1,,3,3,3,1,3,2,4,2,4,1,1,3,3,4,3,4,1,5,5,4,3,5,4,3,3,1,3,5,5,4,5,4,4,5,4,2,,2,,2,,5,,2,,1,,,Economically Disadvantaged - N,,Not ELL,,Not Special Education,34,99 +5/1/2024 10:14:01,5/1/2024 10:27:19,0,209.107.182.203,100,798,1,2021-04-06T10:01:36,parent_survey_response_7,email,EN,1500025,2,1,,5,,1,,5,,,,,,,,,,,,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,,1,,2,,5,,2,,1,,,Economically Disadvantaged - N,,Not ELL,,Not Special Education,34,5 diff --git a/spec/services/survey_item_values_spec.rb b/spec/services/survey_item_values_spec.rb index 7e70d462..5aaa5a56 100644 --- a/spec/services/survey_item_values_spec.rb +++ b/spec/services/survey_item_values_spec.rb @@ -107,6 +107,14 @@ RSpec.describe SurveyItemValues, type: :model do (survey_item_ids << common_headers).flatten end + context ".normalize_headers" do + it "normalizes the headers to remove invisible newlines and lowercase survey item ids" do + headers = [ " p-tcom-q1\n", " P-tcom-q2\r\n ", " P-tcom-q3 " ] + normalized_headers = SurveyItemValues.new(row: {}, headers:, survey_items:, schools:, academic_years:).normalize_headers(headers:) + expect(normalized_headers).to eq ["p-tcom-q1", "p-tcom-q2", "p-tcom-q3"] + end + end + context ".recorded_date" do it "returns the recorded date" do row = { "RecordedDate" => "2017-01-01T12:12:121" } diff --git a/spec/services/survey_responses_data_loader_spec.rb b/spec/services/survey_responses_data_loader_spec.rb index 199f7cc1..2f2cf747 100644 --- a/spec/services/survey_responses_data_loader_spec.rb +++ b/spec/services/survey_responses_data_loader_spec.rb @@ -3,6 +3,7 @@ require "rails_helper" describe SurveyResponsesDataLoader do let(:path_to_teacher_responses) { Rails.root.join("spec", "fixtures", "test_2020-21_teacher_survey_responses.csv") } let(:path_to_student_responses) { Rails.root.join("spec", "fixtures", "test_2020-21_student_survey_responses.csv") } + let(:path_to_parent_responses) { Rails.root.join("spec", "fixtures", "test_2020-21_parent_survey_responses.csv") } let(:path_to_butler_student_responses) do Rails.root.join("spec", "fixtures", "test_2022-23_butler_student_survey_responses.csv") end @@ -41,6 +42,54 @@ describe SurveyResponsesDataLoader do create(:survey_item, survey_item_id: id) end end + let(:parent_survey_items) do + ids = %w[ + p-socx-q1 + p-socx-q2 + p-socx-q3 + p-socx-q4 + p-sosu-q1 + p-sosu-q2 + p-sosu-q3 + p-sosu-q4 + p-tcom-q1 + p-tcom-q2 + p-tcom-q3 + p-comm-q1 + p-comm-q2 + p-comm-q3 + p-comm-q4 + p-valm-q1 + p-valm-q2 + p-valm-q3 + p-valm-q4 + p-acpr-q1 + p-acpr-q2 + p-acpr-q3 + p-acpr-q4 + ] + + ids.each do |id| + create(:survey_item, survey_item_id: id) + end + end + + let(:vestigial_parent_ids) do + %w[ + p-scrp-q3 + p-cure-q1 + p-cure-q2 + p-cure-q3 + p-cure-q4 + p-evnt-q1 + p-evnt-q2 + p-evnt-q3 + p-evnt-q4 + p-phys-q3 + p-scrp-q1 + p-scrp-q2 + ] + end let(:t_pcom_q3) { create(:survey_item, survey_item_id: "t-pcom-q3") } let(:t_pcom_q2) { create(:survey_item, survey_item_id: "t-pcom-q2") } @@ -174,6 +223,41 @@ describe SurveyResponsesDataLoader do end end end + + describe "parent survey responses" do + before do + school + ay_2020_21 + parent_survey_items + SurveyResponsesDataLoader.new.load_data filepath: path_to_parent_responses + end + + it "adds only the surveyitems that exist in source of truth" do + si = (SurveyItemResponse.where(school:, response_id: "parent_survey_response_1").map do |response| + response.survey_item.survey_item_id + end) + response_ids = %w[ + parent_survey_response_1 + parent_survey_response_2 + parent_survey_response_3 + parent_survey_response_4 + parent_survey_response_5 + parent_survey_response_6 + ] + response_ids.each do |id| + expect(SurveyItemResponse.where(response_id: id, + survey_item: SurveyItem.parent_survey_items).count).to eq 23 + end + + expect(SurveyItemResponse.where(response_id: "parent_survey_response_7").count).to eq 0 + end + + it "does not add surveyitems from questions that have been disabled" do + vestigial_parent_ids.each do |id| + expect(SurveyItemResponse.where(school:, survey_item: id).count).to eq 0 + end + end + end end def assigns_academic_year_to_survey_item_responses