Opened 10 years ago
Closed 9 years ago
#12835 closed defect (fixed)
dojo.store.Observable does not observe correctly an added Object.
Reported by: | Simon Speich | Owned by: | Kris Zyp |
---|---|---|---|
Priority: | high | Milestone: | 1.8 |
Component: | Data | Version: | 1.6.0 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description
Objects added to the store, where the store creates the id, are not observed correctly:
- store.remove() is not observed at all
- store.put() is observed as -1 >-1 instead of >-1 >-1
var store, res, id; store = new dojo.store.Memory({ data: [ {id: 1, parId: 2} ] }); store = dojo.store.Observable(store); res = store.query({ parId: 2 }); res.observe(function(item, removedFrom, insertedInto) { console.log(removedFrom, insertedInto); }, true); id = store.add({ // id: 2, parId: 2 }); var obj = store.get(id); store.put(obj); store.remove(id);
incorrectly logs as:
-1 1 -1 1
removing the comment id: 2
correctly logs as:
-1 1 1 1 1 -1
Attachments (3)
Change History (18)
comment:1 Changed 10 years ago by
Owner: | set to Kris Zyp |
---|
comment:2 Changed 10 years ago by
comment:3 Changed 10 years ago by
When adding an object without an id to the store with store.add(), the store creates and returns the id, but does not add it to the object. Is that the intentional behavior? If so I think it's confusing and should be mentioned in the docs.
If I add the returned id to the object before calling store.put() in the above code example
obj.id = id; store.put(obj);
then observe() works correctly. I guess this means that, this defect occurs, because the object does not have an identity property (and depending on the correct behavior of add() this defect could be closed).
comment:4 Changed 9 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:5 Changed 9 years ago by
Component: | Core → Data |
---|
This needs an automated test case. Also need to set the milestone on the ticket.
comment:6 Changed 9 years ago by
Milestone: | tbd → 1.8 |
---|
Oh, looks like it was fixed in [27072], for 1.8, and test case was added there.
comment:7 Changed 9 years ago by
If I use the JsonRest? store instead of the Memory store, I get the same problem as explained above. Remove is not observed at all and put is observed as add, e.g.: -1 1 -1 1
comment:8 Changed 9 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Reopening this, since it sounds like there may still be issues in use with other stores?
comment:9 Changed 9 years ago by
Simon, can you provide more specifics on what the REST responses look like? From what I can tell in my tests, remove() is working and if the response properly includes an id, put() is working properly too.
comment:10 Changed 9 years ago by
I also uploaded the testcase to http://www.speich.net/projects/programming/dojo-jsonrest-test.htm I hope I'm not doing something stupid...
comment:11 Changed 9 years ago by
Simon, I think the core issue with this testcase is that your initial query is not returning an array as the JsonRest? store expects. The initial query (store.query({parId: 2})) is returning an object. The subsequent notifications are all acting on that object, and trying to determine index values based on that object. Since indices into object (instead of an array) don't make sense, the from/to index values are incorrect. I would recommend having the initial query return [{id:1,parId:2}].
comment:12 Changed 9 years ago by
I did find an issue with index values being incorrect with paged queries, that I will check in a fix for, but I am not sure if it will fix this test case.
Changed 9 years ago by
Attachment: | dojo-jsonrest-controller.php added |
---|
generates the REST responses for the test
comment:14 Changed 9 years ago by
I changed the initial response to an array. Without the fix I still get the same -1 0, -1 0, with the fix its even worse now, I only get -1, 0 and it doesn't make a difference if the initial response is an object or an array.
I updated the test case on speich.net, but since that is pulling in nightly it doesn't contain your fix yet.
In [26097]: