amco / dolly

Not an ORM for CouchDB in rails.
8 stars 3 forks source link

Handle empty values with default not being setup #188

Open ErickFabian opened 2 years ago

ErickFabian commented 2 years ago

Properties set with a non nil value would not be overwritten using the default value

class Test < Couch::Base
  property :roles, class_name: Hash, default: {teacher: {}, student: {}}
end
 t3 = Test.find 'test/558d163d26141e5b770516292aa5afec'
=> #<Test:0x00007f9c59b8fc98 @doc={:_id=>"test/558d163d26141e5b770516292aa5afec", :_rev=>"2-8166172731f239617071bf3fe0ef7046", :roles=>{}}, @roles={}>
[27] pry(main)> t3.roles
=> {}
[28] pry(main)> t3.roles[:teacher]
=> nil
alejandrodevs commented 2 years ago

@ErickFabian This way we are using defaults kind of weird, Ex.

test = Test.new
test.roles #=> { teacher: {}, student: {}}
test.roles = {}
test.roles #=> { teacher: {}, student: {}}
test.roles = nil
test.roles #=> { teacher: {}, student: {}}

Default values should be only returned when the attributes has never been set. Ex.

test = Test.new
test.roles #=> { teacher: {}, student: {}}
test.roles = {}
test.roles #=> {}
test.roles = nil
test.roles #=> nil

We need some consistency and not unexpected behaviors. We should see how other libraries for class attributes work, like ActiveModel::Attributes, dry-types, etc

ErickFabian commented 2 years ago

@alejandrodevs That was sort of the error, the school_contract property was set to {} but it should always at least have { teacher: {}, students: {} } the behaviour you are proposing is what is happening now

ErickFabian commented 2 years ago

I'm not exactly sure if this bevahiour change is the right thing to do, i wanted to get feedback on it too, i'm not super set on if this should happen or not