Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compatability with Lotus::Model/Entity #52

Closed
plukevdh opened this issue Mar 4, 2015 · 5 comments · Fixed by #55
Closed

Compatability with Lotus::Model/Entity #52

plukevdh opened this issue Mar 4, 2015 · 5 comments · Fixed by #55
Assignees
Milestone

Comments

@plukevdh
Copy link

plukevdh commented Mar 4, 2015

Seeing consistency break down when including/using validations with Lotus::Model's Entity mixin. The interface to a defined entity changes subtly so as to break expectations. See the following example:

class DummyModel
  include Lotus::Entity
  attributes :name, :email
end
# => [:name, :email]

DummyModel.attributes
# => #<Set: {:id, :name, :email}>

m = DummyModel.new name: "Luke", email: "test@me.com"
# => #<DummyModel:0x007fdc8d135b48 @email="test@me.com", @name="Luke">

m.to_h
# => {:id=>nil, :name=>"Luke", :email=>"test@me.com"}

Vs the output of:

class OtherModel
  include Lotus::Entity
  include Lotus::Validations
  attribute :email, presence: true
  attribute :name, presence: true
end
#=> [:name]

OtherModel.attributes
#=> #<Set: {:id, :email, :name}>
# Note, attributes includes the id here...

o = OtherModel.new name: "Luke", email: "test@me.com"
#=> #<OtherModel:0x007fc03ba9c1f0 @attributes=#<Lotus::Utils::Attributes:0x007fc03ba9c1c8 @attributes={"name"=>"Luke", "email"=>"test@me.com"}>>
# The internal structure (or at least the output of the inspect method) has changed

o.to_h
#=> {:email=>"test@me.com", :name=>"Luke"}
# What happened to the id field? `.attributes` tells me I should be able to expect it returned...

I can note that if I specify attribute :id at the same time as the validators, I can get it to return the id as well, but this is not consistent with how Lotus::Model acts.

@plukevdh
Copy link
Author

plukevdh commented Mar 4, 2015

One more note, that is more confusing as the docs indicate this ought to work:

class OneMore
  include Lotus::Entity
  include Lotus::Validations
  attributes :name, :email
  validates :email, presence: true
end
# => nil

OneMore.attributes
# => #<Set: {:id, :name, :email}>

m = OneMore.new name: "Luke", email: "test@me.com"
# => #<OneMore:0x007fc03bba6ff0 @attributes=#<Lotus::Utils::Attributes:0x007fc03bba6fc8 @attributes={"email"=>"test@me.com"}>>

m.to_h
# => {:email=>"test@me.com"}

I assume, therefore that attributes are only returned if they're validated, which seems like an undesirable effect.

@jodosha jodosha modified the milestone: v0.3.0 Mar 16, 2015
@jodosha jodosha assigned jodosha and unassigned AlfonsoUceda Mar 21, 2015
@jodosha jodosha removed this from the v0.3.0 milestone Mar 22, 2015
@jodosha
Copy link
Member

jodosha commented Mar 22, 2015

@plukevdh Hello and thanks for sharing this problem. It absolutely deserves a fix. My plans were to include it with 0.3.0, but the release date is tomorrow, and we don't have enough time to do it now.

I tried to write a patch on the fly, but it deserves a better investigation. I hope it won't be a problem if we postpone the fix a bit. Thank you 😄

@jodosha
Copy link
Member

jodosha commented Apr 7, 2015

@plukevdh Would you mind to check if the PR above fixes your issue? Thank you!

@plukevdh
Copy link
Author

plukevdh commented Apr 7, 2015

Can do. Sorry for the delay. I'll update here as soon as I have a chance to run.

@jodosha
Copy link
Member

jodosha commented Apr 18, 2015

@plukevdh We've merged that fixing PR because it looks good. Feel free to reopen this ticket if still doesn't work for you. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants