Short sources gid #574

Merged
merged 2 commits into from Jul 6, 2016

Conversation

Projects
None yet
3 participants
@avulfson17
Contributor

avulfson17 commented Jun 30, 2016

This (hopefully) will allows someone to search using a gid typing in the short name of a source, e.g oa:venue:123456 as opposed to openaddresses:venue:123456

Fixes #441

sanitiser/_ids.js
return;
}
if (!_.includes(type_mapping.layers, layer)) {
messages.errors.push( targetError(layer, type_mapping.layers) );
return;
}
+ //converts the shortened source names to the full name

This comment has been minimized.

@orangejulius

orangejulius Jun 30, 2016

Member

You could remove the if statement here, since the source_mapping supports either the short or long name

@orangejulius

orangejulius Jun 30, 2016

Member

You could remove the if statement here, since the source_mapping supports either the short or long name

This comment has been minimized.

@orangejulius

orangejulius Jun 30, 2016

Member

A nice reason to do that is it keeps more knowledge of what the "real" source name is in the type_mapping file, rather than here

@orangejulius

orangejulius Jun 30, 2016

Member

A nice reason to do that is it keeps more knowledge of what the "real" source name is in the type_mapping file, rather than here

@orangejulius

This comment has been minimized.

Show comment
Hide comment
@orangejulius

orangejulius Jul 1, 2016

Member

As mentioned by @dianashk it would be great to have acceptance tests for this. They can go in the place.json test case file

Member

orangejulius commented Jul 1, 2016

As mentioned by @dianashk it would be great to have acceptance tests for this. They can go in the place.json test case file

Cleaned up code for accepting short sources in gid
Removed an extraneous if statement
@trescube

This comment has been minimized.

Show comment
Hide comment
@trescube

trescube Jul 6, 2016

Contributor

:shipit:

Contributor

trescube commented Jul 6, 2016

:shipit:

@orangejulius

This comment has been minimized.

Show comment
Hide comment
Member

orangejulius commented Jul 6, 2016

LGTM

@orangejulius orangejulius merged commit 1b175fe into master Jul 6, 2016

3 checks passed

approvals/lgtm this commit looks good
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@orangejulius orangejulius removed the in review label Jul 6, 2016

@orangejulius orangejulius deleted the short-sources-gid branch Jul 7, 2016

@avulfson17 avulfson17 referenced this pull request in pelias/acceptance-tests Jul 22, 2016

Merged

Added description for failing test #265

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