Skip to content
This repository has been archived by the owner on Apr 17, 2018. It is now read-only.

dm-types Flag fails to persist values assigned with the left shift operator. #26

Open
solnic opened this issue May 17, 2011 · 11 comments
Open

Comments

@solnic
Copy link
Contributor

solnic commented May 17, 2011

DataMapper::Types::Flag caches values assigned/appended with the left shift << operator, but fails to persist them on save().

I've created a gist to illustrate the problem.

I might also add that finders like Model.all(:flagprop => [:one, :two]) do not behave as (I) expected, but I really have no idea how they are supposed to work.

See also:
#1128


Created by Austin Bales (at 417east) - 2009-12-21 05:26:02 UTC

Original Lighthouse ticket: http://datamapper.lighthouseapp.com/projects/20609/tickets/1161

@solnic
Copy link
Contributor Author

solnic commented May 17, 2011

<< fails to invoke dirty tracking so calling save fails to see any changes.

This raises a larger issue about how to handle methods on attributes which modify the receiver therefore bypassing dirty tracking.

3 approaches to handle this:

  1. Document this issue as an implementation detail for the user
  2. Override receiver modifying methods to invoke dirty tracking
  3. Freeze the attribute so it cannot be modified; the user has to reassign changes

I like approach 3. A simple fix that works across all kinds of properties and works with dirty tracking. It also makes it hard to accidentally change an attribute without reassigning it.

by Carl Porth

@solnic
Copy link
Contributor Author

solnic commented May 17, 2011

For example, you could do:

by Carl Porth

@solnic
Copy link
Contributor Author

solnic commented May 17, 2011

I see your reason for supporting approach 3, but it seems an unusual restriction to set (perhaps I’m uninformed). However, if the behavior is atypical, then it sort of presents a usability issue.
Is it that approach two is too complex or costly? I’m by no means a Ruby expert, but bear with me...

Because the Flag type contains multiple values, and is represented as an array, it seems logical that I can push things into it. One of the reasons I appreciate DataMapper is that it does feel like "All Ruby All The Time", and things work naturally. I think this is especially important if #1128’s conclusion was that Flag should yield arrays.

by Austin Bales (at 417east)

@solnic
Copy link
Contributor Author

solnic commented May 17, 2011

I don’t think your example is atypical at all; consider the following case with calling capitalize! on a string attribute: http://gist.github.com/276969

Receiver modifying methods seem to be a weakness with the current implementation of dirty tracking. ActiveRecord addresses this by requiring you to call #{attribute}_will_change! before modifying the receiver.

Maybe there is a better pattern or ruby idiom to solve this?

by Carl Porth

@solnic
Copy link
Contributor Author

solnic commented May 17, 2011

I don’t love #attribute_will_change!, but what if you could set values to explicitly save.

p = Person.get(3)
p.first_name.capitalize!
p.visits << Visit.new()
p.save :first_name, :visits

by Austin Bales (at 417east)

@solnic
Copy link
Contributor Author

solnic commented May 17, 2011

This is going to be resolved in 1.x.x series as it requires changes in dm-core and we won’t make it for 1.0.0

by Piotr Solnica (solnic)

@solnic
Copy link
Contributor Author

solnic commented May 17, 2011

[bulk edit]

by Dan Kubb (dkubb)

@jpr5
Copy link
Member

jpr5 commented Jun 9, 2011

Note that pull request 33 (#33) + adding the DirtyMinder concept to Flag would conceptually fix this.

@dkubb
Copy link
Member

dkubb commented Jun 9, 2011

@jpr5's pull request was merged which solves this problem. Closing this ticket.

If the problem still persists please add a comment to this ticket, otherwise enjoy!

@dkubb dkubb closed this as completed Jun 9, 2011
@dkubb dkubb reopened this Jun 9, 2011
@dkubb
Copy link
Member

dkubb commented Jun 9, 2011

Ok, my bad. That commit does not solve this issue yet. However the same lib can be used with this. @jpr5 has said he would look into it.

@lsiden
Copy link

lsiden commented Dec 28, 2011

Aaron Pfeiffer offered this temporary sollution in his post https://groups.google.com/d/msg/datamapper/JBkeBWxZUmI/H6nPD0kCWwcJ

I had to update it just a little but it's working for me now. For example:

  account.status << :email_confirmed
  account.taint! :status
  account.save
STDERR.puts account.saved? ? 'account is saved' : 'account was not saved'
account.reload
p account.status # ==> [ :email_confirmed ]

Maybe it will help someone else until they find a better way. I'm just getting my sea-legs with Ruby. This looks like a pretty challenging problem.

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

No branches or pull requests

4 participants