Pass class name to Sequel::MassAssignmentRestriction error #2079
Conversation
If the instance has a class name, pass this name to the error message to better understand the error in monitoring systems such as Sentry.
artofhuman
force-pushed
the
class-for-mass-assigment-error
branch
from
33e2558
to
51de3cf
Compare
|
Instead of including the model class name in the message, we should add a method to set the model instance as an instance variable in the exception. That takes less work, and still allows the rescuer to get the class name. This should be implemented using a class method on |
artofhuman
force-pushed
the
class-for-mass-assigment-error
branch
from
fe16f0b
to
5ab8426
Compare
|
Added |
lib/sequel/model/exceptions.rb
Outdated
| # The Sequel::Model object related to this exception. | ||
| attr_reader :model | ||
|
|
||
| def initialize(msg, model) |
There was a problem hiding this comment.
This is not backwards compatible, and will break code that uses raise MassAssignmentRestriction. I recommend using a different class method, such as create.
There was a problem hiding this comment.
Maybe better to make model arg optional?
There was a problem hiding this comment.
I would prefer a separate method.
lib/sequel/model/exceptions.rb
Outdated
| attr_reader :model | ||
|
|
||
| def self.create(msg, model) | ||
| @model = model |
There was a problem hiding this comment.
This sets the instance variable on the class, not on the created instance. You don't catch this because you only test for the message and not for the model value. The tests should be updated to test that the model is set correctly.
artofhuman
force-pushed
the
class-for-mass-assigment-error
branch
from
840e72c
to
fbad090
Compare
|
Thanks for continuing to work on this. I'll try to merge and clean this up on Monday. |
|
Squash merged at 1e00c84 |
If the instance has a class name, pass this name to the error message to better understand the error in monitoring systems such as Sentry.