samvera / active_fedora

A Rails interface to the Fedora repository, akin to ActiveModel
Other
54 stars 63 forks source link

Change tracking not working when using shift operator on properties? #540

Open jcoyne opened 9 years ago

jcoyne commented 9 years ago

8) GenericFile delegations that have been saved should be able to be added to w/o unexpected graph behavior Failure/Error: expect(f.title).to include("Newer work") expected ["New work"] to include "Newer work"

./spec/models/generic_file_spec.rb:304:in `block (4 levels) in <top (required)>'

awead commented 9 years ago

@jcoyne there's not enough information here for us to proceed. Where is the relevant place in the AF stack? Can you explain the issue more?

jcoyne commented 9 years ago

When you shift (<<) onto an AT property, it isn't registered in the changes hash, so a sparql insert isn't created. This is just my suspicion, needs further confirmation.

awead commented 9 years ago

@jcoyne I've confirmed it and have a failing test in ActiveFedora

awead commented 9 years ago

@jcoyne I've added a failing test cfc013d9f1469b1c149033cefd2f5d32997756db and tracked the problem down to @changed_attributes. When we use << to add a new term, @changed_attributes is still empty, so the resulting sparql update or ds.update doesn't occur.

afred commented 9 years ago

so.. afaik, the << operator operates on an array or string "in place", instead of making a copy and assigning it, the way += does. Using @awead's test, i found that changing << to += got the test to pass, by triggering the key piece of code.. which is the call to attribute_will_change! in ActiveFedora::AttributeMethods::Dirty#set_value. The call to attribute_will_change! does not get called when appending using << operator.

So, if we really want to get this to work, i think we need some way for ActiveTriples::Term to signal a "changed" state.

Or, we can just tell people to use += instead.

jcoyne commented 9 years ago

At the developer meeting @no-reply suggested that ActiveModel::Dirty could move into ActiveTriples. That would be a way to fix this.

afred commented 9 years ago

i took a minute to look at how that might be implemented and got hung up on the fact that all (except a few) instance methods of Array are being delegated to the return value of #results. I was unsure how you might call [attribute]_has_changed! because it's unclear what [attribute] would be.