Security: Another scrub pass on importer logging

app/importers/file_importers/behavior_importer.rb

@@ -111,7 +111,6 @@ def import_row(row)
     student = Student.find_by_local_id(row[:local_id])
     if student.nil?
       @skipped_from_invalid_student_id += 1
-      log("skipping, StudentLocalID not found: #{row[:local_id]}")
       return
     end
 

app/importers/file_importers/students_importer.rb

@@ -112,7 +112,7 @@ def import_row(row)
 
     # Match student records
     maybe_homeroom_id = match_homeroom_id(row)
-    maybe_student = StudentRow.new(row, maybe_homeroom_id, school_ids_dictionary, @log).build
+    maybe_student = StudentRow.new(row, maybe_homeroom_id, school_ids_dictionary).build
 
     # Sync records
     @syncer.validate_mark_and_sync!(maybe_student)

app/importers/helpers/fuzzy_student_matcher.rb

@@ -21,11 +21,11 @@ def match_from_last_first(text)
   def match_from_full_name(full_name)
     student_id = guess_from_name(full_name)
     if student_id.nil?
-      @invalid_student_name += 1
+      @invalid_student_name_count += 1
       return nil
     end
 
-    @valid_student_name += 1
+    @valid_student_name_count += 1
     {
       full_name: full_name,
       student_id: student_id
@@ -47,14 +47,14 @@ def match_file(file_text, &block)
 
   private
   def reset_counters!
-    @invalid_student_name = 0
-    @valid_student_name = 0
+    @invalid_student_name_count = 0
+    @valid_student_name_count = 0
   end
 
   def stats
     {
-      invalid_student_name: @invalid_student_name,
-      valid_data_points: @valid_data_points
+      invalid_student_name_count: @invalid_student_name_count,
+      valid_student_name_count: @valid_student_name_count
     }
   end
 

app/importers/helpers/import_matcher.rb

@@ -142,7 +142,7 @@ def stats
     {
       valid_rows_count: @valid_rows_count,
       invalid_rows_count: @invalid_rows_count,
-      invalid_student_local_ids: @invalid_student_local_ids,
+      invalid_student_local_ids_size: @invalid_student_local_ids.size,
       invalid_educator_emails_size: @invalid_educator_emails.size,
       invalid_educator_last_names_size: @invalid_educator_last_names.size,
       invalid_educator_logins_size: @invalid_educator_logins.size,

app/importers/iep_import/iep_storer.rb

@@ -38,7 +38,7 @@ def store_only_new
     # Match to student
     student = Student.find_by_local_id(student_local_id)
     if student.nil?
-      log("student local_id: #{student_local_id} not found, dropping the IEP PDF file...")
+      log("student local_id not found, dropping the IEP PDF file...")
       return nil
     end
 
@@ -68,7 +68,7 @@ def iep_pdf_already_exists?(student)
 
   # Returns filename on success, nil on error
   def store_object_in_s3(student_local_id)
-    log("storing iep pdf for student_local_id:#{student_local_id} in s3...")
+    log("storing iep pdf in s3...")
 
     # s3 filenames are sorted by student / upload date / iep
     # Filename shape: {64-character hash} / {YYYY}-{MM}-{DD} / {64-character hash}.

app/importers/rows/dibels_row.rb

@@ -40,7 +40,7 @@ def extract_benchmark_from_string(raw_string)
     return 'INTENSIVE' if raw_string.upcase == 'INTENSIVE'
     return 'INTENSIVE' if raw_string.upcase.include?('INT')
 
-    log.puts("DibelsRow: couldn't parse DIBELS benchmark: #{raw_string}")
+    log.puts("DibelsRow: couldn't parse DIBELS benchmark")
 
     return nil
   end

app/importers/rows/student_row.rb

@@ -1,4 +1,4 @@
-class StudentRow < Struct.new(:row, :homeroom_id, :school_ids_dictionary, :log)
+class StudentRow < Struct.new(:row, :homeroom_id, :school_ids_dictionary)
   # Represents a row in a CSV export from Somerville's Aspen X2 student information system.
   # Some of those rows will enter Student Insights, and the data in the CSV will be written into the database.
   #

app/importers/star/star_importer.rb

@@ -94,7 +94,7 @@ def import_row(raw_row)
 
     if test_result.invalid?
       @invalid_rows_count += 1
-      log("error: #{test_result.errors.full_messages}")
+      log("errors.keys: #{test_result.errors.keys}")
       return nil
     end
 

spec/importers/iep_import/iep_pdf_import_job_spec.rb

@@ -29,11 +29,14 @@ def create_import_job_with_mock_files(attrs = {})
       expect(log.output).to include 'unzipped 2 files from iep-pdfs-for-test-2.zip...'
       expect(log.output).to include 'unzipping iep-pdfs-for-test-1.zip...'
       expect(log.output).to include 'unzipped 1 files from iep-pdfs-for-test-1.zip...'
-      expect(log.output).to include 'storing iep pdf for student_local_id:2222222211 in s3...'
-      expect(log.output).to include 'storing iep pdf for student_local_id:333333333 in s3...'
-      expect(log.output).to include 'storing iep pdf for student_local_id:111111111 in s3...'
+      expect(log.output).to include 'storing iep pdf in s3...' # 3 times
       expect(log.output).to include 'found 3 PDF files within downloaded zips.'
       expect(log.output).to include 'created 3 IepDocument records.'
+
+      # avoid logging local student ids
+      expect(log.output).not_to include '111111111'
+      expect(log.output).not_to include '2222222211'
+      expect(log.output).not_to include '333333333'
     end
   end
 end

spec/importers/reading/mega_reading_processor_spec.rb

@@ -76,7 +76,7 @@ def create_students!
         :matcher => {
           :valid_rows_count=>28,
           :invalid_rows_count=>0,
-          :invalid_student_local_ids=>[],
+          :invalid_student_local_ids_size=>0,
           :invalid_educator_emails_size=>0,
           :invalid_educator_last_names_size=>0,
           :invalid_educator_logins_size=>0,

spec/importers/star/star_math_importer_spec.rb

@@ -71,7 +71,7 @@ def mock_for_fixture!(district_key, local_fixture_filename)
     it 'handles bad data (v2 as example)' do
       importer, log = create_mocked_importer(PerDistrict::SOMERVILLE, "#{Rails.root}/spec/importers/star/star_math_v2_invalid.csv")
       importer.import
-      expect(log.output).to include('error: ["Percentile rank can\'t be blank"]')
+      expect(log.output).to include('errors.keys: [:percentile_rank]')
       expect(log.output).to include('skipped 1 invalid rows')
       expect(StarMathResult.all.size).to eq(0)
     end

spec/importers/star/star_reading_importer_spec.rb

@@ -71,7 +71,7 @@ def mock_for_fixture!(district_key, local_fixture_filename)
     it 'skips and logs bad data (v2 as example)' do
       importer, log = create_mocked_importer(PerDistrict::SOMERVILLE, "#{Rails.root}/spec/importers/star/star_reading_v2_invalid.csv")
       importer.import
-      expect(log.output).to include('error: ["Percentile rank too high"]')
+      expect(log.output).to include('errors.keys: [:percentile_rank]')
       expect(log.output).to include('skipped 1 invalid rows')
       expect(StarReadingResult.all.size).to eq(0)
     end

spec/importers/transitions/bedford_davis_transition_notes_importer_spec.rb

@@ -54,7 +54,7 @@ def create_importer(test_folder_id, options = {})
           :invalid_educator_last_names_size => 0,
           :invalid_educator_logins_size => 0,
           :invalid_sep_oids => [],
-          :invalid_student_local_ids => []
+          :invalid_student_local_ids_size => 0
         },
         :syncer=>{
           :total_sync_calls_count=>1,