Skip to content
This repository has been archived by the owner. It is now read-only.

Implement MessageSchema.{byBbox,byLocation,byEdges} resolver #31

Merged
merged 16 commits into from Jul 5, 2017

Conversation

@c-w
Copy link
Contributor

@c-w c-w commented Jul 3, 2017

project-fortis#48

@c-w c-w force-pushed the messages-resolver branch from a7a65c0 to 972af01 Jul 3, 2017
@c-w c-w changed the title Implement MessageSchema.byBbox resolver Implement MessageSchema.{byBbox,byLocation,byEdges} resolver Jul 3, 2017
@c-w c-w force-pushed the messages-resolver branch from c018d31 to 05424c9 Jul 3, 2017
@c-w c-w requested a review from erikschlegel Jul 3, 2017
@c-w c-w force-pushed the messages-resolver branch from e1efad4 to 3baf5ce Jul 3, 2017
Copy link
Collaborator

@erikschlegel erikschlegel left a comment

LGTM

const clauses = defaults.clauses;
let params = defaults.params;

clauses.push(`(${placeIds.map(_ => '(detectedplaceids CONTAINS ?)').join(' OR ')})`); // eslint-disable-line no-unused-vars
Copy link
Collaborator

@erikschlegel erikschlegel Jul 5, 2017

I'd add some validation to ensure that detectedplaceids are all numbers

@c-w c-w force-pushed the messages-resolver branch from d8e5740 to 8890391 Jul 5, 2017
@c-w c-w merged commit 919a4ae into master Jul 5, 2017
2 checks passed
@c-w c-w deleted the messages-resolver branch Jul 5, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants