From d418ffb258365d417b51c5ce97b5ba4e96291a12 Mon Sep 17 00:00:00 2001 From: Dan Halson Date: Mon, 27 Oct 2025 14:41:27 +0000 Subject: [PATCH 1/3] 977: Ignore class member validation errors in sentry --- .../class_member/operations/create.rb | 4 ++-- spec/concepts/class_member/create_spec.rb | 22 ++++++++++++++----- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/lib/concepts/class_member/operations/create.rb b/lib/concepts/class_member/operations/create.rb index b156dd3ae..d720121e6 100644 --- a/lib/concepts/class_member/operations/create.rb +++ b/lib/concepts/class_member/operations/create.rb @@ -45,14 +45,14 @@ def create_class_students(school_class:, students:, response:) end def handle_class_teacher_error(exception, class_teacher, teacher, response) - Sentry.capture_exception(exception) + Sentry.capture_exception(exception) unless exception.is_a?(ActiveRecord::RecordInvalid) errors = class_teacher.errors.full_messages.join(',') response[:error] ||= "Error creating one or more class members - see 'errors' key for details" response[:errors][teacher.id] = "Error creating class member for teacher_id #{teacher.id}: #{errors}" end def handle_class_student_error(exception, class_student, student, response) - Sentry.capture_exception(exception) + Sentry.capture_exception(exception) unless exception.is_a?(ActiveRecord::RecordInvalid) errors = class_student.errors.full_messages.join(',') response[:error] ||= "Error creating one or more class members - see 'errors' key for details" response[:errors][student.id] = "Error creating class member for student_id #{student.id}: #{errors}" diff --git a/spec/concepts/class_member/create_spec.rb b/spec/concepts/class_member/create_spec.rb index ef761eb59..6b69afbab 100644 --- a/spec/concepts/class_member/create_spec.rb +++ b/spec/concepts/class_member/create_spec.rb @@ -86,7 +86,7 @@ expect(response[:error]).to match(/No valid school members provided/) end - it 'sent the exception to Sentry' do + it 'sends the exception to Sentry' do described_class.call(school_class:, students:, teachers:) expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError)) end @@ -122,9 +122,9 @@ expect(response[:errors][different_school_student.id]).to include("Error creating class member for student_id #{different_school_student.id}: Student '#{different_school_student.id}' does not have the 'school-student' role for organisation '#{school.id}'") end - it 'sent the exception to Sentry' do + it 'does not send the exception to Sentry' do described_class.call(school_class:, students:) - expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError)) + expect(Sentry).not_to have_received(:capture_exception) end end @@ -181,11 +181,23 @@ expect(response[:errors][different_school_student.id]).to eq("Error creating class member for student_id #{different_school_student.id}: Student '#{different_school_student.id}' does not have the 'school-student' role for organisation '#{school.id}'") end - it 'sent the exception to Sentry' do + it 'does not send the exception to Sentry' do described_class.call(school_class:, students: new_students, teachers:) - expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError)) + expect(Sentry).not_to have_received(:capture_exception) end end end + + context 'when a non-validation error occurs' do + it 'sends the exception to Sentry' do + # rubocop:disable RSpec/AnyInstance + allow_any_instance_of(ClassStudent).to receive(:save!).and_raise(RuntimeError) + # rubocop:enable RSpec/AnyInstance + + described_class.call(school_class:, students:) + + expect(Sentry).to have_received(:capture_exception).with(kind_of(RuntimeError)).at_least(:once) + end + end end end From ca4d170791aa04f07bed71a07d7ca2d7256a616d Mon Sep 17 00:00:00 2001 From: Dan Halson Date: Mon, 27 Oct 2025 16:28:03 +0000 Subject: [PATCH 2/3] 977: Only skip sentry duplicates --- .../class_member/operations/create.rb | 4 ++-- spec/concepts/class_member/create_spec.rb | 22 ++++++++----------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/lib/concepts/class_member/operations/create.rb b/lib/concepts/class_member/operations/create.rb index d720121e6..ec0477cf5 100644 --- a/lib/concepts/class_member/operations/create.rb +++ b/lib/concepts/class_member/operations/create.rb @@ -45,14 +45,14 @@ def create_class_students(school_class:, students:, response:) end def handle_class_teacher_error(exception, class_teacher, teacher, response) - Sentry.capture_exception(exception) unless exception.is_a?(ActiveRecord::RecordInvalid) + Sentry.capture_exception(exception) unless exception.message.include?('has already been taken') errors = class_teacher.errors.full_messages.join(',') response[:error] ||= "Error creating one or more class members - see 'errors' key for details" response[:errors][teacher.id] = "Error creating class member for teacher_id #{teacher.id}: #{errors}" end def handle_class_student_error(exception, class_student, student, response) - Sentry.capture_exception(exception) unless exception.is_a?(ActiveRecord::RecordInvalid) + Sentry.capture_exception(exception) unless exception.message.include?('has already been taken') errors = class_student.errors.full_messages.join(',') response[:error] ||= "Error creating one or more class members - see 'errors' key for details" response[:errors][student.id] = "Error creating class member for student_id #{student.id}: #{errors}" diff --git a/spec/concepts/class_member/create_spec.rb b/spec/concepts/class_member/create_spec.rb index 6b69afbab..a32480886 100644 --- a/spec/concepts/class_member/create_spec.rb +++ b/spec/concepts/class_member/create_spec.rb @@ -122,9 +122,9 @@ expect(response[:errors][different_school_student.id]).to include("Error creating class member for student_id #{different_school_student.id}: Student '#{different_school_student.id}' does not have the 'school-student' role for organisation '#{school.id}'") end - it 'does not send the exception to Sentry' do + it 'sends the exception to Sentry' do described_class.call(school_class:, students:) - expect(Sentry).not_to have_received(:capture_exception) + expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError)) end end @@ -181,22 +181,18 @@ expect(response[:errors][different_school_student.id]).to eq("Error creating class member for student_id #{different_school_student.id}: Student '#{different_school_student.id}' does not have the 'school-student' role for organisation '#{school.id}'") end - it 'does not send the exception to Sentry' do + it 'sends the exception to Sentry' do described_class.call(school_class:, students: new_students, teachers:) - expect(Sentry).not_to have_received(:capture_exception) + expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError)) end end end - context 'when a non-validation error occurs' do - it 'sends the exception to Sentry' do - # rubocop:disable RSpec/AnyInstance - allow_any_instance_of(ClassStudent).to receive(:save!).and_raise(RuntimeError) - # rubocop:enable RSpec/AnyInstance - - described_class.call(school_class:, students:) - - expect(Sentry).to have_received(:capture_exception).with(kind_of(RuntimeError)).at_least(:once) + context 'when duplicate validation errors occur' do + it 'does not send the exception to Sentry' do + duplicate_student = students.first + described_class.call(school_class:, students: [duplicate_student, duplicate_student]) + expect(Sentry).not_to have_received(:capture_exception) end end end From 18323a4d6e1c2735508d5d12134bc9226741088f Mon Sep 17 00:00:00 2001 From: Dan Halson Date: Mon, 27 Oct 2025 16:29:56 +0000 Subject: [PATCH 3/3] Revert description --- spec/concepts/class_member/create_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/concepts/class_member/create_spec.rb b/spec/concepts/class_member/create_spec.rb index a32480886..e6b67b185 100644 --- a/spec/concepts/class_member/create_spec.rb +++ b/spec/concepts/class_member/create_spec.rb @@ -86,7 +86,7 @@ expect(response[:error]).to match(/No valid school members provided/) end - it 'sends the exception to Sentry' do + it 'sent the exception to Sentry' do described_class.call(school_class:, students:, teachers:) expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError)) end @@ -122,7 +122,7 @@ expect(response[:errors][different_school_student.id]).to include("Error creating class member for student_id #{different_school_student.id}: Student '#{different_school_student.id}' does not have the 'school-student' role for organisation '#{school.id}'") end - it 'sends the exception to Sentry' do + it 'sent the exception to Sentry' do described_class.call(school_class:, students:) expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError)) end @@ -181,7 +181,7 @@ expect(response[:errors][different_school_student.id]).to eq("Error creating class member for student_id #{different_school_student.id}: Student '#{different_school_student.id}' does not have the 'school-student' role for organisation '#{school.id}'") end - it 'sends the exception to Sentry' do + it 'sent the exception to Sentry' do described_class.call(school_class:, students: new_students, teachers:) expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError)) end