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

Already on GitHub? Sign in to your account

deleteRecord example from Readme doesn't seem to work #168

Closed
scottmessinger opened this Issue Mar 19, 2012 · 14 comments

Comments

Projects
None yet
7 participants

Problem: calling deleteRecord() on an instance of a model doesn't seem to delete it.

Example from the readme:

var person = App.store.find(App.Person, 1);
person.deleteRecord();

Here's the fiddle:
http://jsfiddle.net/scottmessinger/mWmNy/5/

Here's the relevant code from the fiddle:

App.person1 = App.store.createRecord(App.Person, {id: 1, name: 'p1'})
App.person2 = App.store.createRecord(App.Person, {id: 2, name: 'p2'})
App.person3 = App.store.createRecord(App.Person, {id: 3, name: 'p3'})

App.people = App.store.findAll(App.Person)

App.person1.deleteRecord()

Is this a bug or am I doing something wrong?

Btw, I pulled the ember-data repo from github and built it, so the ember-data should reflect the latest updates to master.

sohara commented Mar 27, 2012

I seem to be having the same issue on the most recent master (with the previous 'wip' branch merged in). I'm using the REST API adapter. The deleted item seems to remain in the store but the delete request is sent (successfully) to the API server.

So I'm doing the following:

item.deleteRecord();
App.store.commit();```

dziamid commented Mar 30, 2012

The key thing here is that only newly created models won't get deleted. Have a look at my fiddle: http://jsfiddle.net/Adw4F/2/
(only records created by App.store.createRecord cannot be deleted).

dziamid commented Apr 2, 2012

This can be solved by explicitly removing model from model arrays before deleting it., e.g.:
this.content.removeObject(item);
item.deleteRecord();

@dziamid Where would this.content.removeObject(item) go? I took a stab at it but couldn't figure it out. Could you create a fork of the fiddle to show what you mean?

dziamid commented Apr 3, 2012

Sorry, you are right, this is not a solution at all. It works only for relations. In my case it worked out because i had a one to many relation, and manually deleting a model from related models' array does the trick. Seems like functionality of ember data is limited to only displaying the model arrays.

sirvine commented Apr 5, 2012

@dziamid I had the same problem as you did (newly-added association are removed from the store but not the ManyArray).

Unfortunately, your solution also did not work, as the call to item.deleteRecord() results in:

'undefined' is not an object (evaluating 'records.remove')

Also, reversing the order so that item.deleteRecord precedes the attempt to remove it from the ManyArray results in an error from DS.StateManager:

could not respond to event setAssociation in state rootState.deleted.start.

The thing that I am unable to determine is what is unique about newly-created records that causes this variance. Older records are removed from both the server and locally just fine.

Contributor

thomasboyt commented Apr 18, 2012

Just ran into this issue. Been tracing it back through ember-data, and it seems that when recordArraysForClientId() is called to find the RecordArray that goes with the deleted record, it returns an empty array, as recordArraysByClientId does not contain the RecordArrays for newly-created records.

Owner

wagenet commented Apr 18, 2012

See #200

Owner

wagenet commented Apr 18, 2012

This looks like a duplicate of #148.

@wagenet wagenet closed this Apr 18, 2012

Contributor

thomasboyt commented Apr 18, 2012

While #148 is talking about uncommited records, new records that are added to a hasMany association's RecordArray can't be removed from the array, regardless of whether they've been committed or not. I think these may be separate issues?

Owner

tomdale commented Apr 18, 2012

Fixed in 46f5a9e

Thanks! The fix looks good.

Owner

tomdale commented Apr 18, 2012

@scottmessinger I tried the fix with the jsfiddle you posted. It turns out you sussed out a race condition that was causing a bug, despite my passing unit test. I've since updated the unit tests and implementations in 49be811, and the jsfiddle you posted now works correctly.

Please let me know if you guys find any more related bugs!

@pangratz pangratz pushed a commit to pangratz/data that referenced this issue Apr 18, 2012

tomhuda Remove new deleted records from record arrays
Fixes #168. Previously, only records that were
sent to the adapter were being removed from
record arrays. This is because the logic for
removing them was in the deleted.start enter
transition event, but new records that are deleted
transition directly to the deleted.saved state
(because deleting a new record is effectively a
no-op; the adapter should never be notified about
the record).

As part of this change, we now have records
that transition from created.uncommitted directly
move themselves to their transaction's clean
bucket (as they do not need to be sent to the
adapter).

This also fixes a bug where updated and
uncommitted records were moving themselves to the
clean bucket. Only newly created records should
do this; updated records should move to the
transaction's deleted bucket when deleted.
46f5a9e

@tomdale Awesome. Thanks!

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