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

Mutable id on first request #2558

Closed
knownasilya opened this issue Dec 4, 2014 · 13 comments
Closed

Mutable id on first request #2558

knownasilya opened this issue Dec 4, 2014 · 13 comments

Comments

@knownasilya
Copy link
Contributor

So I have the following,

this.store.find('my-model', token);

The server returns:

{
  "myModel": {
    "id": 1
  }
}

Where the id !== token, but token will always return the same data. So this doesn't currently work, since ED sets the Model.id to the token, even after the record from the server returns.

I could do

this.store.find('my-model', { token: token })
  .then(function (model) {
    return model.get('firstObject');
  });

but is that the best way? Ultimately I always want to GET the model with the token, but PUT and DELETE by the id.

@knownasilya
Copy link
Contributor Author

The issue is that it's a single record, so a REST call like /api/myModel/?token=2221jkh231 assumes that you could return n records, but really it's /api/myModel/2221jkh231 and there is always 1 or 0 models (404).

@bmac
Copy link
Member

bmac commented Dec 5, 2014

@knownasilya There has been some discussion of implementing a queryOne method #1576 would that solve your issue?

@knownasilya
Copy link
Contributor Author

@bmac not really, since I don't know the final id, so I can't do find('my-model', id, { token: token })

@pangratz
Copy link
Member

@knownasilya have you seen this workaround? #1576 (comment)

@ohcibi
Copy link

ohcibi commented May 1, 2015

I just ran into the same problem. I'd say the main problem here is that the store pushes a record with an ID that doesn't exist on the server. The store should blindly accept whatever the server responded as the records ID.

Plus I think the second parameter for find should not necessarily be considered as the ID but any value, which the backend can use to find the record. The store should not do anything with that parameter, except or passing it to the backend.

@igorT
Copy link
Member

igorT commented May 2, 2015

That fails if your api does sideloading and doesn't tell you what the primary record is. Would findQueryOne work for your usecase?

@ohcibi
Copy link

ohcibi commented May 3, 2015

findQueryOne is a shortcut for find("model", {query: "param"}).then(function(model) { return model.get("firstObject") }, so actually it works already but its a hack and not a solution.

The fact that the store pushes a record with an ID that is not an ID at all is a bug. If the backends response contained model: { id: "1", ...}, ..., the store should simply not push a record with a different ID. In fact the store should not use any parameter of find to replace any data of the backends response.

@igorT
Copy link
Member

igorT commented May 4, 2015

Well, findQueryOne isn't really a hack, because if you do a store.find('post', 1) and there is no record coming back with id:1 that promise should be rejected right?
The eager store pushing is an implementation detail, should be less obvious once the references branch is merged

@ohcibi
Copy link

ohcibi commented May 4, 2015

Well, findQueryOne isn't really a hack

I meant that using findQueryOne to solve this problem is a hack, not the method itself.

because if you do a store.find('post', 1) and there is no record coming back with id:1 that promise should be rejected right?

Either this or it should just return an empty list - the method name is find and not findOrCreate, so actually the ID doesn't matter.. find should simply not push records. Currently the store is even pushing a record that has the exact same properties but a different ID. The situation is like this:

  • this.store.find("model", "ident");
  • Backend responds:
{
  "model": {
    "id": 1,
    "indexedfield": "ident",
    "otherfield": "other value"
  }
}
  • store pushes an object with properties like: {id: "ident", indexedfield: "ident", other field: "other value"}

So ember-data somehow invented a new record out of nothing.

@igorT
Copy link
Member

igorT commented May 4, 2015

findQueryOne is not a hack for this particular use case.
I agree with you that creating a record upfront is a bug, and it should soon be harder to observe.
How did you observe this record in your user code btw?

@ohcibi
Copy link

ohcibi commented May 4, 2015

How did you observe this record in your user code btw?

I'm not exactly sure what you mean. I have no observers for the record, but only computed properties

findQueryOne is not a hack for this particular use case.

agreed, lets focus on the store creating a ghost record

@wecc
Copy link
Contributor

wecc commented Oct 12, 2015

I think store.queryRecord() would solve this?

@knownasilya
Copy link
Contributor Author

Yes sir.

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

7 participants