Add Size Query Param for Elastic Search #715
Conversation
| @@ -138,6 +140,11 @@ export default Adapter.extend(PouchAdapterUtils, { | |||
| break; | |||
| } | |||
| } | |||
|
|
|||
| if (!query.options.size) { | |||
There was a problem hiding this comment.
@donaldwasserman I just tested this and I get the following error:
vendor.js:40820 Error while processing route: patients.search Cannot read property 'size' of undefined TypeError: Cannot read property 'size' of undefined
I believe this is because query.options is not passed in.
| @@ -34,4 +34,4 @@ config.couchDbURL = config.getProtocol(config.couchDbUseSsl) + config.couchDbSer | |||
| config.couchAuthDbURL = config.getProtocol(config.couchDbUseSsl) + config.couchCredentials() + config.couchDbServer + ':' + config.couchDbPort; | |||
| // config.searchURL = 'http://localhost:9200'; ELASTIC SEARCH URL (OPTIONAL) | |||
| config.serverInfo = 'Development Ember CLI server'; | |||
| module.exports = config; | |||
| module.exports = config; | |||
There was a problem hiding this comment.
This change shouldn't be here.
There was a problem hiding this comment.
Also, I realized that this size parameter never makes it to the call to elasticsearch.
There was a problem hiding this comment.
After stopping and thinking it through, it seems like _executeContainsSearch is the only method that makes elastic search requests? I know that's a pretty basic question, but this is pretty abstracted out for my simple brain.
There was a problem hiding this comment.
@donaldwasserman correct... _executeContainsSearch is what ends up calling to /search/hrdb/ which is a proxy to ElasticSearch (or a temp view search using couchdb).
This part of the code needs to have the size parameter passed to it:
Ember.$.ajax(searchUrl, {
dataType: 'json',
data: {
q: queryString,
size: variableWithSize //Replace variableWithSize with whatever variable has the size.
},
success: successFn
});Also, even if you don't have ElasticSearch installed you can still test to see if the value is being passed along by running a search from Patient and looking in the developer console to see what url the app is passing along to the search proxy. The url should end with &size=25
There was a problem hiding this comment.
@donaldwasserman can you address these changes? Once they are done, I think this PR is ready to go. Thanks!
| _executeContainsSearch(store, type, query) { | ||
| const self = this; |
There was a problem hiding this comment.
There is no need for this because "this" is properly bound when you need it in the Ember.$.ajax call because
return new Ember.RSVP.Promise((resolve, reject) => {
});is the same as
return new Ember.RSVP.Promise(function(resolve, reject) {
}).bind(this);| @@ -34,4 +34,4 @@ config.couchDbURL = config.getProtocol(config.couchDbUseSsl) + config.couchDbSer | |||
| config.couchAuthDbURL = config.getProtocol(config.couchDbUseSsl) + config.couchCredentials() + config.couchDbServer + ':' + config.couchDbPort; | |||
| // config.searchURL = 'http://localhost:9200'; ELASTIC SEARCH URL (OPTIONAL) | |||
| config.serverInfo = 'Development Ember CLI server'; | |||
| module.exports = config; | |||
| module.exports = config; | |||
There was a problem hiding this comment.
Please remove the change to server/config-example.js (eg put back the newline at the end of the file).
|
should be all set |
| Ember.$.ajax(searchUrl, { | ||
| dataType: 'json', | ||
| data: { | ||
| q: queryString | ||
| q: queryString, | ||
| size: self.get('_esDefaultSize') |
There was a problem hiding this comment.
Didn't try the code, but it appears this should be used instead of self.
|
I forgot to commit the code that changed this to self. should be good now. |
|
Thanks for the PR! Looks good to me, so I'll merge it in. |
Fixes #453.
Changes proposed in this pull request:
Added configurable Elastic Search size option.
I don't have elastic search running locally and it doesn't seem to be an issue with the server routes. Can submit a PR on those if you think it's necessary.
This is an exciting project
cc @HospitalRun/core-maintainers