Add query support to structured #763

Merged
merged 5 commits into from Jan 12, 2017

Conversation

Projects
None yet
2 participants
@trescube
Contributor

trescube commented Dec 9, 2016

Adds query parameter support to /v1/search/structured. Most everything was in place before, this just adds query to analysis synthesis and adjusts tests to query module changes.

Connected to pelias/pelias#483

@trescube trescube added the in progress label Dec 9, 2016

@trescube trescube self-assigned this Dec 14, 2016

@trescube trescube added in review and removed in progress labels Dec 14, 2016

@trescube trescube added this to the Five-Box Address Search milestone Dec 14, 2016

sanitizer/_synthesize_analysis.js
@@ -2,6 +2,7 @@ const _ = require('lodash');
const text_analyzer = require('pelias-text-analyzer');
const fields = {
+ 'query': 'query',

This comment has been minimized.

@dianashk

dianashk Dec 16, 2016

Contributor

Can we call the param something other than query? Maybe poi_name or venue? Query is confusing I'm my opinion.

@dianashk

dianashk Dec 16, 2016

Contributor

Can we call the param something other than query? Maybe poi_name or venue? Query is confusing I'm my opinion.

This comment has been minimized.

@trescube

trescube Dec 17, 2016

Contributor

It would have to be something generic since it doesn't have to be a venue or POI name, it could be a category like "restaurants" or "bars".

@trescube

trescube Dec 17, 2016

Contributor

It would have to be something generic since it doesn't have to be a venue or POI name, it could be a category like "restaurants" or "bars".

@dianashk

This comment has been minimized.

Show comment
Hide comment
@dianashk

dianashk Jan 10, 2017

Contributor

After discussing in our team meeting, we think changing this to name makes sense here. We will continue to also search the categories for results, but the user doesn't need to be directly aware of that. When we officially release the categories parameter for search it will be supported in this endpoint as well, so it doesn't seem necessary to include a dedicated category parameter for the query as well.

Contributor

dianashk commented Jan 10, 2017

After discussing in our team meeting, we think changing this to name makes sense here. We will continue to also search the categories for results, but the user doesn't need to be directly aware of that. When we officially release the categories parameter for search it will be supported in this endpoint as well, so it doesn't seem necessary to include a dedicated category parameter for the query as well.

@trescube

This comment has been minimized.

Show comment
Hide comment
@trescube

trescube Jan 10, 2017

Contributor

Oh, sorry I missed this discussion since I was ooto.

Contributor

trescube commented Jan 10, 2017

Oh, sorry I missed this discussion since I was ooto.

@trescube

This comment has been minimized.

Show comment
Hide comment
@trescube

trescube Jan 12, 2017

Contributor

After a team meeting on 1/11/2017, it was decided that the parameter name would be venue.

Contributor

trescube commented Jan 12, 2017

After a team meeting on 1/11/2017, it was decided that the parameter name would be venue.

@trescube trescube referenced this pull request in pelias/acceptance-tests Jan 12, 2017

Merged

switched structured tests from `query` to `venue` #337

@dianashk

Thank you, @trescube!

@trescube trescube merged commit 1f49886 into master Jan 12, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@trescube trescube deleted the add-query-support-to-structured branch Jan 12, 2017

@trescube trescube removed the in review label Jan 12, 2017

je-l pushed a commit to nlsfi/pelias-api that referenced this pull request Aug 31, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment