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

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

Open
jcoyne opened this issue Nov 2, 2014 · 7 comments
Open
Assignees

Comments

@jcoyne
Copy link
Member

jcoyne commented Nov 2, 2014

  1. 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)>'

@jcoyne jcoyne added this to the Fedora 4 milestone Nov 2, 2014
@awead
Copy link
Contributor

awead commented Nov 5, 2014

@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
Copy link
Member Author

jcoyne commented Nov 5, 2014

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
Copy link
Contributor

awead commented Nov 5, 2014

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

@awead
Copy link
Contributor

awead commented Nov 6, 2014

@jcoyne I've added a failing test cfc013d 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 afred self-assigned this Nov 7, 2014
awead added a commit to samvera-deprecated/sufia that referenced this issue Nov 7, 2014
An issue with ActiveModel::Dirty is affecting the shift operator in ActiveFedora. Until samvera/active_fedora#540 is resolved, we
should avoid using it.
@afred
Copy link
Contributor

afred commented Nov 7, 2014

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
Copy link
Member Author

jcoyne commented Nov 11, 2014

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

@afred
Copy link
Contributor

afred commented Nov 11, 2014

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.

@awead awead removed this from the 9.0.0 milestone Nov 12, 2014
@jcoyne jcoyne modified the milestone: 9.1.0 Nov 12, 2014
awead added a commit to samvera-deprecated/sufia that referenced this issue Nov 25, 2014
An issue with ActiveModel::Dirty is affecting the shift operator in ActiveFedora. Until samvera/active_fedora#540 is resolved, we
should avoid using it.
awead added a commit to samvera-deprecated/sufia that referenced this issue Dec 10, 2014
An issue with ActiveModel::Dirty is affecting the shift operator in ActiveFedora. Until samvera/active_fedora#540 is resolved, we
should avoid using it.
awead added a commit to samvera-deprecated/sufia that referenced this issue Dec 10, 2014
An issue with ActiveModel::Dirty is affecting the shift operator in ActiveFedora. Until samvera/active_fedora#540 is resolved, we
should avoid using it.
@awead awead modified the milestone: 9.1.0 Jul 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants