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

Ability to use query string params with store.find() #1576

Closed
adamlc opened this issue Dec 13, 2013 · 31 comments
Closed

Ability to use query string params with store.find() #1576

adamlc opened this issue Dec 13, 2013 · 31 comments

Comments

@adamlc
Copy link

adamlc commented Dec 13, 2013

I've come across an issue where I need to append query string params to a find request.

So basically I need to do something like this:

this.store.find('contact', 1, {embed: 'company'});

Which should produce a GET request like:

/contacts/1?embed=company

Going through the code I couldn't see a way of doing this. Has anyone got any thoughts or ideas on how I might achieve this?

@mlb5000
Copy link

mlb5000 commented Dec 13, 2013

It sounds like this warrants a new feature, and this is definitely hokey, but you could do something like this in the meantime:

this.store.find('contact', {id: 1, embed: 'company'});

I'm sure this is precisely the sort of thing you were trying to avoid since it conflates the purpose of your Index endpoint, but you could implement this today.

@mlb5000
Copy link

mlb5000 commented Dec 13, 2013

To be clear, based on the description of find today, there is no built-in way to do this: https://github.com/emberjs/data/blob/master/packages/ember-data/lib/system/store.js#L260

@adamlc
Copy link
Author

adamlc commented Dec 13, 2013

@mlb5000 thanks for the reply. It looks like its going to need a new function in the store and adaptor. Fortunately our system is a few months off any sort of release yet.

I will give it a go next week and submit a PR if everyone thinks its something that should be in the master.

@mlb5000
Copy link

mlb5000 commented Dec 13, 2013

I like the idea myself, but have only just started to get involved. I'm sure some of the veterans will have an opinion.

@mlb5000
Copy link

mlb5000 commented Dec 13, 2013

To play devil's advocate, why not just sideload related entities all the time? Costly query?

@samwgoldman
Copy link

JSON API recently merged a spec[1] describing the ability for a client to request related documents from a compliant server. I think that supplying arguments to store's methods would be a reasonable way to expose this functionality to users of a hypothetical JSON API adapter.

  1. Introduce means to request related documents. json-api/json-api#145

@DougPuchalski
Copy link

I feel like this is really important. For example I'm need to generate additional response data for an edit action, something unnecessary with show.

@oskarols
Copy link

I really think abuiles PR should be reconsidered; this is just one of those things that /should work/.

@morficus
Copy link

I'm using Ember 1.5 for one o my projects and have the same need that is detailed out in this issue.
What I discovered is that by doing this.store.find('post', {'include': 'comments', 'id': 1}) will actually yield the follow URL /posts/1?include=comments, which i what I would expect. But doing so... appears to break the data binding.... (if I dynamically add a new record to the store... the template does not update).

Interestingly enough tho...the way around this was to do the following:

this.store.filter('post', {'include': 'comments', 'id': 1}, function () {
     return true;
})

This would produce the correct URL and also maintain the data binding.

Hopefully this helps anyone else running in to the same or similar issue.

@DougPuchalski
Copy link

Perhaps not clear why passing query params would be supported when fetching a collection but not a single item.

@duizendnegen
Copy link

I went out to try to at least try and write a new function that wrapped findQuery and extracted a single element. I did not succeed (payload gives me content, but there's problems extracting it). This was I guess because as an end user I know what to do with Promises (Ember is quite transparant with them), but I don't have any traction yet with implementing them from my side.

I ended up just writing {#each} in my .hbs, knowing that it will only get executed once.
Leaving my WIP here though, in case anyone wants to imbed it in their solution.

(in Coffeescript, sorry for that)

Promise = Ember.RSVP.Promise;

PromiseObject = Ember.ObjectProxy.extend(Ember.PromiseProxyMixin);

module.exports.ApplicationStore = DS.Store.extend
  findSingleQuery: (typeName, query) ->
    _this = this

    promiseObject = PromiseObject.create({
      promise: Promise.cast(_this.findQuery(typeName, query).then( (payload) ->
        return payload.content[0];
      ), "DS: Store#findSingleQuery")
    });

    return promiseObject;

to be used as

this.store.findSingleQuery('book', {id: params.book_id, library: params.library_id});

@kodayashi
Copy link

Thanks for the comment @morficus, however, the following attempts at passing query parameters when fetching a single item failed:

this.store.find('post', {'include': 'comments', 'id': 1}); this.store.filter('post', {'include': 'comments', 'id': 1});

Both resulted in:

http:////posts?include=comments&id=1

As opposed to:

http:////posts/1?include=comments

Any suggestions?

@mitchlloyd
Copy link
Contributor

As some have noticed, there is no way to customize Serializers and Adapters to accomplish a query for a single record because the contract between the Store, Serializer, and Adapter won't allow it. Because of this, the store has to get involved too, and the store is not supposed to be customized. :(

Here is a hack that I just put into place to accomplish this. Of course this uses non-public APIs so good luck during upgrades.

First I have a file that exports a function that I call findOneQuery

export default function(store, type, id, query) {
  var adapter = store.adapterFor(type);
  var serializer = store.serializerFor(type);
  var typeClass = store.modelFor(type);
  var url = adapter.buildURL(type, id);
  var ajaxPromise = adapter.ajax(url, 'GET', { data: query });

  return ajaxPromise.then(function(rawPayload) {
    var extractedPayload = serializer.extract(store, typeClass, rawPayload, id, 'find');
    return store.push(typeClass, extractedPayload);
  });
}

This hits an API like http://posts/1?include=comments

You could use that as a stand-alone function, but I went one step further and added it to my store in an initializer:

import DS from 'ember-data';
import findOneQuery from 'rqst/utils/find-one-query';

export function initialize() {
  DS.Store.reopen({
    findOneQuery: function(type, id, query) {
      return findOneQuery(this, type, id, query);
    }
  });
}

export default {
  name: 'AddFindOneQueryToStore',
  initialize: initialize
};

I'm a little worried about the find api on store. It seems quite overloaded at the moment, and it's impossible to express common queries that people want.

@dennismende
Copy link

Hello,

@wycats are there any news about the progress. We would like to use this feature to change the language of an items properties. It would be really great if we could find a possible solution.

@shawndeprey
Copy link

👍 @mitchlloyd very nice work on that work around. It works fantastic. I do believe this is a very important case to support in ember data. I am currently following the JSON standards proposed at http://jsonapi.org/ and this query case is too common to not be supported.

@shawndeprey
Copy link

I took the work around of @mitchlloyd above and did it in straight JS.

Config:

DS.Store.reopen({
  findOneQuery: function(type, id, query) {
    var store = this;
    var adapter = store.adapterFor(type);
    var serializer = store.serializerFor(type);
    var typeClass = store.modelFor(type);
    var url = adapter.buildURL(type, id);
    var ajaxPromise = adapter.ajax(url, 'GET', { data: query });

    return ajaxPromise.then(function(rawPayload) {
      var extractedPayload = serializer.extract(store, typeClass, rawPayload, id, 'find');
      return store.push(typeClass, extractedPayload);
    });
  }
});

Usage:

this.store.findOneQuery('model', params.id, {'include': 'other_models'})

Worked like a charm for me. I would really like to see something like this included with Ember Data.

@brendensoares
Copy link

@shawndeprey I fixed your code so it works for model specific adapters:

DS.Store.reopen({
    findOneQuery: function(type, id, query) {
        var store = this;
        var typeClass = store.modelFor(type);
        var adapter = store.adapterFor(typeClass);
        var serializer = store.serializerFor(typeClass);
        var url = adapter.buildURL(type, id);
        var ajaxPromise = adapter.ajax(url, 'GET', { data: query });

        return ajaxPromise.then(function(rawPayload) {
            var extractedPayload = serializer.extract(store, typeClass, rawPayload, id, 'find');
            return store.push(typeClass, extractedPayload);
        });
    }
});

Thanks for pointing me in the right direction :)

@seocahill
Copy link

Thanks for the patches hope the pull request is accepted

@mgharbik
Copy link

mgharbik commented Mar 4, 2015

Awesome, It worked perfect!!!!

@igorT
Copy link
Member

igorT commented Jun 11, 2015

You will now be able to use adapterOptions to pass options to your adapter, using findRecord. We will soon have support for include/embed like things, but passing query params should also be easy.
Tracking this under #3281

@igorT igorT closed this as completed Jun 11, 2015
@jpoiri
Copy link

jpoiri commented Jul 21, 2015

When are you targeting this feature to be release?

@abuiles
Copy link
Member

abuiles commented Jul 21, 2015

queryRecord is now available in the store.

@jpoiri
Copy link

jpoiri commented Jul 21, 2015

Is this the right way to use it?

this.store.queryRecord('team', 77, {include_members: true}); Doesn't seem to work for me

@wecc
Copy link
Contributor

wecc commented Jul 22, 2015

@jpoiri try this.store.queryRecord('team', 77, { query: { include_members: true } });

@jpoiri
Copy link

jpoiri commented Jul 22, 2015

@wecc I'm getting this now using the RestAdapter
:
Error: Assertion Failed: You tried to make a query but your adapter does not implement queryRecord

@wecc
Copy link
Contributor

wecc commented Jul 22, 2015

@jpoiri #3514

@jpoiri
Copy link

jpoiri commented Jul 22, 2015

thanks, I guess I'll have to wait

@tschoartschi
Copy link

@jpoiri did you solve the issue? For me the provided solution does not work as I would expect it.
@wecc I also need this behaviour but I'm not sure how it works, because the following code snipped does not work for me.

this.store.queryRecord('team', 77, { query: { include_members: true } });

I use ember-data 1.13.7 with the RESTAdapter and ember 1.13.6

What I did to get it working as expected (or as I would expect it):

// SOMEWHERE IN A CONTROLLER OR ELSEWHERE 
    this.get('store').queryRecord(this.get('modelname'), {
        __id__: id,
        contentLanguage: newLang
    }).then(function (response) {
        // HANDLE SUCCESS 
    }, function (error) {
        // HANDLE ERROR
    });

And then I need to adapt buildURL in the RESTAdapter:

import Ember from 'ember';
import DS from 'ember-data';

export default DS.RESTAdapter.extend({
    //...
    buildURL: function (type, id, snapshot, queryType, query) {
        if (queryType === 'queryRecord' && query.__id__) {
            url += '/' + query.__id__;
            query.__id__ = null;
            delete query.__id__;
        }
        return url;
    }
});

I think this is a hack and not a nice software design. Is queryRecord expected to query for one record AND add query params?

Could someone help me figure out what would be a nice solution? Thanks a lot :-)

If someone is interested; our use case is the following:
In the admin interface the user can enter informations to some product. After the user added all informations in one language he has the possibility to switch the content language, which means he can enter the same informations in another language. Because record.reload does some merging-magic it is not suitable for us. The form should always be populated with the fresh data from the server. The data is always depending on the content language. I implemented the behaviour as follows:
When the user switches the content language I unload the record from the store and reload the record with queryRecord from the server. With the outlined solution it works as I need it but I think this is a hacky solution

@ZackMattor
Copy link

@tschoartschi check out @wecc's example here. #3596 (comment) it worked for me :)

@bkCDL
Copy link

bkCDL commented Aug 20, 2015

@wecc 's example is excellent. Thanks!

Incidentally, I was able to call the querystring object in BuildURL itself also:

buildURL: function buildURL(modelName, id, snapshot, requestType, query) {
  var e, sURL;
  try {
    sURL = [this.get("host"), this.get("namespace"), "reports", id].join("/");
    query = Ember['default'].get(snapshot, "adapterOptions.query");
    if (!Em.isBlank(query)) {
      sURL += "?" + Ember['default'].$.param(query);
    }
    return sURL;
  } catch (_error) {
    e = _error;
    return Em.Logger.error("report.buildURL ERROR", e);
  }
}

I called the report model from the controller like this:

 this.store.findRecord('report', id, { adapterOptions: { query: qsObj }}).then(...

where qsObj is a json object I use filled with various query string parameters my API expects for reports I call.

Bryan

@allthesignals
Copy link
Contributor

Struggling a bit with queryRecord() as well. The following code builds the url, "profiles?1." I am using the JSONAPI adapter.

export default Ember.Route.extend({
  model: function(params) {
    return this.store.queryRecord("profile", params.profile_id, { 'query': { 'include': 'place' } });
  }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests