Skip to content

Conversation

@danhalson
Copy link
Contributor

Status

https://github.com/RaspberryPiFoundation/digital-editor-issues/issues/977

What's changed?

Ensures validation errors aren't raised for ClassMembers in Sentry, this is especially important for the importer where a re-import would trigger potentially hundreds of these

Steps to perform after deploying to production

N/A

@cla-bot cla-bot bot added the cla-signed label Oct 27, 2025
@danhalson danhalson self-assigned this Oct 27, 2025

def handle_class_teacher_error(exception, class_teacher, teacher, response)
Sentry.capture_exception(exception)
Sentry.capture_exception(exception) unless exception.is_a?(ActiveRecord::RecordInvalid)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could there be any other reason for the record to be invalid apart from the cases we want to exclude?

Copy link
Contributor Author

@danhalson danhalson Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've gone back and forth a bit with this one, since I think it's probably the case that none of the validations should be logged to sentry and RecordInvalid is indicative of validations...but if we were to add any validations further down the line that are exceptions to this rule (and we need them to be logged) we'd be suppressing them...so I think safest is to just to check explicitly for this error, and add a test giving us a bit of regression coverage

@danhalson danhalson temporarily deployed to editor-api-p-977-ignore-hkeuuk October 27, 2025 16:30 Inactive
Copy link
Contributor

@loiswells97 loiswells97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

@danhalson danhalson merged commit ec87a3a into main Oct 27, 2025
3 checks passed
@danhalson danhalson deleted the 977-ignore-validation-errors-in-sentry branch October 27, 2025 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants