[Improvement?] find should take both id and query parameters

[abobwhite]

I have a situation where I need to find a model via /models/{id} but to also provide query parameters: /models/{id}?key=value. The reason is that this particular model is under version control on the server. It can get "approved" and become publicly visible but another version can still be edited by the user in a "draft" state. When the user is logged in, they should be able to access the model in that "draft" state. There are two obvious solutions:

1. Enhance find({id}) to optionally take query params: find({id}, {/* draft : true */})
2. Extend the model to be something like a "DraftModel" which would only serve as a wrapper to hit a different server endpoint: /draftModels/{id}

I prefer option 1 but I can see how opinions would be split between the options. Option 1 can be used to provide different information on the same model depending on a specific use-case and the state of the data.

Thoughts?

[abobwhite]

#391 and #551 both are related to this.

As @wycats mentioned here: #551 (comment), it is certainly an option to just use find('model', {id: params.model_id}), however, this makes the endpoint (GET /models?id=12345) cluttered because that same endpoint will also take filter, sort, and pagination parameters for retrieving a list of these models. Since we are only needing 1 model, /models/{id} makes sense but is just not enough.

[wycats]

An easier solution might be to implement your own findQuery that uses the /models/id endpoint with query params.

App.ApplicationAdapter = DS.RESTAdapter.extend({
  findQuery: function(store, type, query) {
    if (query.id) {
      url = this.buildURL(type.typeKey, query.id);
      delete query.id;
    } else {
      url = this.buildURL(type.typeKey);
    }

    return this.ajax(url, 'GET', { data: query });
  }
});

[abobwhite]

@wycats Makes sense. Works beautifully. That is definitely easier. I suppose the use case is more of the 20% rather than the 80% so it makes sense to be customization rather than a default.

[abobwhite]

@wycats The problem with this approach is that the extract method in the serializer still expects the server to return an array. Your solution to #551 makes sense, however, I need "find" functionality (no array) but with "findQuery" functionality (query params). I've looked at writing my own find in addition to findMany, however, this adapter method isn't called until after the recordArray has been constructed. It seems that I don't have any power to override the findQuery in DS.Store to prevent this. The only solution I can see is to have the server respond with the object inside an array but would only work for requests using the findQuery and not the normal use-case of using find. I suppose I could override extractArray for the types that need query params:

App.SupplierSerializer = DS.RESTSerializer.extend({
  extractArray: function(store, primaryType, payload) {
    payload = this.normalizePayload(primaryType, payload);

    var primaryTypeName = primaryType.typeKey,
      primaryArray;

    // To allow findQuery('supplier', {id: _id, latest: true}) to work, we need to make the returned object in an array
    var firstKeyInHash = this._getFirstKey(payload);
    if(Ember.isEqual(firstKeyInHash,'supplier') && !$.isArray(payload + '.' + firstKeyInHash)){
      var newPayload = {};
      newPayload.suppliers = [payload.supplier];
      payload = newPayload;
    }

    for (var prop in payload) {
      if (payload.hasOwnProperty(prop)){
        var typeName = this.typeForRoot(prop),
          type = store.modelFor(typeName),
          isPrimary = typeName === primaryTypeName;

        /*jshint loopfunc:true*/
        var normalizedArray = payload[prop].map(function(hash) {
          return this.normalize(type, hash, prop);
        }, this);

        if (isPrimary) {
          primaryArray = normalizedArray;
        } else {
          store.pushMany(typeName, normalizedArray);
        }
      }
    }

    return primaryArray;
  },

  _getFirstKey: function(obj) {
    for (var key in obj){
      if (obj.hasOwnProperty(key)){
        return key;
      }
    }
  }
});

And in my route:

model: function(){
    return this.store.find('supplier', {id: App.AuthManager.get('authenticatedUser.partyId'), latest: true}).then(function(array){
      return array[0];
    });
  },

However, I am now getting this error: Cannot delegate set('myProperty', (null)) to the 'content' property of object proxy <App.DashboardSupplierProfileEditController:ember735>: its 'content' is undefined. because the content hasn't yet been loaded...I can't find a way around this last little bit...

[abobwhite]

UPDATE: I have found a workaround to the Route problem via:

model: function(){
  return this.store.find('supplier', {id: App.AuthManager.get('authenticatedUser.partyId'), latest: true});
},
setupController: function(controller, model) {
  controller.set('model', model.get('content')[0]);
},

Which I suppose will work...do you have an opinion?

[ryanjm]

@abobwhite Did you find a clean solution to this? When I try your last update model comes back as an empty array (since the API is returning an object). Or did you have to do it in conjunction to your extractArray work?

[abobwhite]

@ryanjm I haven't worked toward a cleaner solution yet. Indeed, I had to do a small manipulation in my extractArray to get it to work correctly. Let me know if you think of something else more clever.

Calling find with parameters causes the expected response to be an array (an assumption ED makes, for right or wrong). So in extractArray, we must take the single object and make it an array of 1.

[nosachamos]

@abobwhite @ryanjm I'm facing the same problem. in my case I must find by id, and pass an additional parameter to request all fields to be included in the response (some are omitted by default), like so:

GET /posts/1?include=all

I tried to do it this way:

this.get('store').find('post', params.post_id, { include : 'all' });

But my params were ignored due to this limitation. I could go about implementing as suggested above, but seems like a hack for such a common use case...

It would be awesome if Ember would allow for a syntax like find('model', id, paramsObj);

This way we keep the same API signature for find, allowing for an optional data object that would be added to the query as query params.

What you guys think?

[abobwhite]

@nosachamos I definitely agree with you. There are plenty of valid use cases like yours that it would be worth looking into a better solution. I do not have enough time to work on a PR myself but your find('model', id, paramsObj); seems plausible.

Another option that @wycats suggested to me back in the fall would be to implement some sort of model.configure({params}) which could alter the way certain attributes and relationships get fetched. This could also be used for preventing a model from caching among other things.

[nosachamos]

Cool, I'll try to get a PR in later tonight, after work. Cross your fingers, this would be great to have but since I'll have to make changes in a few places in both the store and adapter I'm not too hopeful it will be accepted.

While this does not happen a workaround would be to pass the info using request headers instead. Not an option if you are not the owner of the API, but if you are, it's something to consider.

[abobwhite]

@nosachamos Good luck. I think there are deep implications in ED with doing something like this. There may be a way to do this in a way that it's the first step of many but doesn't feel like it's half-baked. Something like I mentioned above could be far-reaching. It would be nice to pick some brains on the Ember team to hear some thoughts on this - particularly @wycats, who originally proposed it.

[nosachamos]

@abobwhite you're right. Mon and Tue I'll be in a conference with a few members of the core team, so I'll pick their brains on this before submitting anything. Thanks, I'll keep this thread updated with any developments.

[thomasvanlankveld]

+1 for find('model', id, paramsObj);. Feels only natural with find('model', id); and find('model', paramsObj); available. Is this really an edge case?

[abobwhite]

@thomasvanlankveld I agree with you. There should be some standard way to do this. I do not believe it to be an edge case. It's fairly common practice even if it's not the most common scenario. It seems this and other related issues have been dismissed (closed) by core team members because they probably had bigger fish to fry. But currently, all solutions suggested making this work are a hack. I think the ultimate issue at hand here is that if we try this.store.find('myModel', {id : myId, otherParam: true}) which used id as a param, the serializer expects the result to be an array because findQuery is invoked. If findQuery were to not assume array, then this might be a bit simpler and could conceivably be handled on the app-level rather than at an ember level.

That being said, I'm in favor of simply permitting query params on the find for a single model.

[jurgenwerk]

+1 for find('model', id, paramsObj). It's really frustrating using find('myModel', {id : myId, otherParam: true}), because the result is passed as an array. I don't believe this is really such an edge case, and would gladly see it getting implemented in the near future.

[jeffjarchow]

+1 for find('model', id, paramsObj). This is something I find myself needing on many of my endpoints of my API. It just doesn't feel right having to fetch a single record as an array and extract the index I need every time.

There is possibly another option, although I don't know how much it might slow things down (and doesn't quite feel right either). A possible solution is to see if any one of the keys in the params object matches the models primaryKey. If so, return a single object instead of an array. But this might be difficult to pull off considering the id parameter can be an array of ids (if I'm not mistaken).

[thaume]

This will be possible once findQueryOne will be merged (which should be
quite soon).
#2584
You'll be able to fetch with an id + parameters and get an object in
return, not an array.
Le 26 janv. 2015 23:41, "Jeff Jarchow" [email protected] a écrit :

+1 for find('model', id, paramsObj). This is something I find myself
needing on many of my endpoints of my API. It just doesn't feel right
having to fetch a single record as an array and extract the index I need
every time.

There is possibly another option, although I don't know how much it might
slow things down (and doesn't quite feel right either). A possible solution
is to see if any one of the keys in the params object matches the models
primaryKey. If so, return a single object instead of an array. But this
might be difficult to pull off considering the id parameter can be an array
of ids (if I'm not mistaken).

—
Reply to this email directly or view it on GitHub
#1326 (comment).

[abobwhite]

@thaume Great! Glad to see something came out of this. Thanks for putting the time to develop the feature. I hope it gets merged soon.

This issue has only been here for 1.5 years! One of the key features needed to make ED really robust.

[jeffjarchow]

Great news @thaume. Thank you for the update.

[iamjstates]

@thaume - I was looking for this today. I hope this get in the next beta release.

[tschoartschi]

I would also need this functionality. Our use case is as follows:

When I query an object just by it's ID I get all labels of this object in the default language (which is english). Now when I specify a contentLanguage as query param I get the labels in the corresponding language. But when I want to query a object by it's ID and get it in a special contentLanguage I can't do it. My first thought was also to query like the others mentioned:

this.get('store').find('item',id,{contentLanguage: 'de'}).then(function(response) {
        // PROCESS SUCCESS
}, function(error) {
        // PROCESS ERROR
});

But it does not work! Is there any way to accomplish this behavior without some hack?

[abobwhite]

@tschoartschi While this use-case definitely needs to happen (I've been waiting for over 1.5 years for it), I think in your case, you could probably use the HTTP Header: 'Accept-Language' which can be defined in your adapter. On your server-side, you would just read this header and apply logic accordingly, thus removing the need for a query parameter here.

[friksa]

+1 for fitting into the real world

[wecc]

Please note that the third parameter to store.findRecord(modelName, id, options) is options, not query params. You can pass { adapterOptions: { /*custom things*/ } } and they will be available to your adapter methods.

The reason for this not being a plain object for query params is that we want to be able to extend the behavior of Ember Data in the future. First-class support for pagination, include and sorting are a couple of things being discussed.

We want to reserve the possibility of having something like filter or include as an option and make ED keep track of the value for refresh calls and so on.

[timmyomahony]

Another request for this feature. I assumed that I could pass parameters when using find or findRecord as you are able to do so with query so it's quite confusing from a new user's perspective.

A common example is when you need to pass extra data to the server to annotate the result coming back. In my case, I want to annotate the distance to the returned object from the user's position (passed by query params)

[buschtoens]

With the advent of JSONAPI and the include request parameter, this is once again a much needed feature.

[buschtoens]

Could it be that this is actually already possible through the adapterOptions? It seems like they are already passed through to the buildQuery method.

[bmac]

While arbitrary query parameters are not supported include it is possible to specify the include parameter by doing store.findRecord('post', 1, { include: 'comments' });

[buschtoens]

Yup. Relevant RFC for anyone wondering. emberjs/rfcs#99
And the blog post.

I should be more observant...