Create feedback #601
Create feedback #601
Conversation
|
What issue does this relate to? |
danhalson
left a comment
danhalson
left a comment
There was a problem hiding this comment.
Looks good, but a couple of suggestions and questions here...happy to discuss in a call if that's easier 😄
lib/concepts/feedback/create.rb
Outdated
| response | ||
| rescue StandardError => e | ||
| Sentry.capture_exception(e) | ||
| if response[:feedback].nil? |
There was a problem hiding this comment.
I'm a bit confused by this, would it maybe be better to be checking is .errors is not empty first and doing the join, and then falling back to this? but maybe I'm misunderstanding something
There was a problem hiding this comment.
Thanks for spotting this - I've simplified it, as I think that check isn't really needed. It was catching the error that was being thrown separately below, but I think we can just allow it to try and save with the school project id being nil and get the error in the normal way through that
Just linked to the ticket in the description |
lib/concepts/feedback/create.rb
Outdated
| response | ||
| rescue StandardError => e | ||
| Sentry.capture_exception(e) | ||
| errors = response[:feedback]&.errors&.full_messages&.join(',') |
There was a problem hiding this comment.
my worry here now is that it could fail to return an error if it's not a validation error, which won't be an array...so I think the fallback was good, it maybe wasn't explicit enough about what it was doing
Status
Related to https://github.com/RaspberryPiFoundation/digital-editor-issues/issues/928
Overall epic is https://github.com/RaspberryPiFoundation/digital-editor-issues/issues/944
What's changed?