Add presence validation for object in SnapshotItem model #56
Conversation
dmrAnderson
changed the title
object in SnapshotItem model
|
Can we reduce the scope of this PR to adding the validation only? The other change regarding changes to the object method to add the |
test/models/snapshot_item_test.rb
Outdated
|
|
||
| [:item_id, :item_type, :snapshot_id].each do |attr| | ||
| %i[item_id item_type snapshot_id object].each do |attr| |
There was a problem hiding this comment.
Can we just keep the original form
| %i[item_id item_type snapshot_id object].each do |attr| | |
| [:item_id, :item_type, :snapshot_id, :object].each do |attr| |
| end | ||
|
|
||
| @object = YAML.send(yaml_method, self[:object]) | ||
| @object = super() ? YAML.public_send(yaml_method, super()) : {} |
There was a problem hiding this comment.
Looks like the rest of the changes in this method are no longer relevant
There was a problem hiding this comment.
Do I have to remove the rest or change the error class?
| @object = self[:object] | ||
| @object = super() | ||
| else | ||
| raise ArgumentError, "Unsupported storage_method: `#{ActiveSnapshot.config.storage_method}`" |
There was a problem hiding this comment.
Im not sure that ArgumentError is appropriate here as storage_method wasnt a method argument. Maybe just StandardError is fine.
| raise ArgumentError, "Unsupported storage_method: `#{ActiveSnapshot.config.storage_method}`" | |
| raise StandardError, "Unhandled ActiveSnapshot.config.storage_method" |
| elsif ActiveSnapshot.config.storage_method_native_json? | ||
| @object = self[:object] | ||
| @object = super |
There was a problem hiding this comment.
All of the mentions of super do not seem relevant in this PR. Should they not remain as self[:object]
|
|
||
| def object | ||
| return @object if @object | ||
|
|
||
| if ActiveSnapshot.config.storage_method_json? | ||
| @object = JSON.parse(self[:object]) | ||
| @object = self[:object] ? JSON.parse(self[:object]) : {} |
There was a problem hiding this comment.
Instead of doing this empty object fallback on every line Maybe we can do @object ||= {} after the if else storage method statements
There was a problem hiding this comment.
Even better, but the new instance with fresh @object will go to JSON.parse(self[:object]) # => Error
#51
Improved Data Integrity
By adding
validates :object, presence: true, you ensure that the object attribute cannot be null or empty when aSnapshotItemis created or updated. This helps maintain data integrity and prevents situations where object might be inadvertently left empty.Error Handling for Unsupported Storage Methods
An explicit error is raised if an unsupported
storage_methodis used, making it clear when configuration issues occur and preventing unexpected behavior.