fix failing serialization with sequel & custom serializer #84
Conversation
lib/surrealist/helper.rb
Outdated
| @@ -11,5 +11,9 @@ module Helper | |||
| def self.surrealist?(klass) | |||
| klass < Surrealist || klass < Surrealist::Serializer | |||
| end | |||
|
|
|||
| def self.collection?(object) | |||
| object.kind_of?(Enumerable) | |||
There was a problem hiding this comment.
Prefer Object#is_a? over Object#kind_of?.
azhi
force-pushed
the
fix-sequel-custom-serializer
branch
from
e4754b4 to
f6a7f78
Compare
azhi
changed the title
azhi
force-pushed
the
fix-sequel-custom-serializer
branch
from
f6a7f78 to
62e9e95
Compare
lib/surrealist/helper.rb
Outdated
|
|
||
| private | ||
|
|
||
| def self.ar_relation?(object) |
There was a problem hiding this comment.
Inconsistent indentation detected.
private (on line 22) does not make singleton methods private. Use private_class_method or private inside a class << self block instead.
lib/surrealist/helper.rb
Outdated
| object.is_a?(Enumerable) || self.ar_relation?(object) | ||
| end | ||
|
|
||
| private |
lib/surrealist/helper.rb
Outdated
| # 4.2 AR relation object did not include Enumerable (it defined | ||
| # all necessary method through ActiveRecord::Delegation module), | ||
| # so we need to explicitly check for this | ||
| object.is_a?(Enumerable) || self.ar_relation?(object) |
azhi
force-pushed
the
fix-sequel-custom-serializer
branch
from
62e9e95 to
696427b
Compare
Tests on travis succeeded on latest Sequel, ROM, AR, as well as on ROM 3.x and AR 4.2. I didn't check on older sequel or ROM, so it can be broken there :). |
azhi
changed the title
|
@azhi this looks great, only one thing I have to mention: I think we should add a spec for collection serialization through custom serializers (like the one you have added for instance) |
refactor checking whether object is a collection: move the check to helper use more strict version of a check: responding to `each` does not always mean that object is a collection, but most of collections include Enumerable add explicit check for ActiveRecord::Relation - though it does not include Enumerable in version 4.2, we still need to consider it a collection improve error message when collection does not fit these rules
azhi
force-pushed
the
fix-sequel-custom-serializer
branch
from
696427b to
a0746a8
Compare
|
@nesaulov updated. |
refactor checking whether object is a collection: move the check to
helper
use more strict version of a check: responding to
eachdoes not alwaysmean that object is a collection, but most of collections include
Enumerable
add explicit check for ActiveRecord::Relation - though it does not
include Enumerable in version 4.2, we still need to consider it a
collection
improve error message when collection does not fit these rules
Fixes #83