API for fetching nearby coordinates information #1978
Conversation
|
When correct format is followed in URL When correct format not followed |
app/services/search_service.rb
Outdated
| @@ -182,4 +182,29 @@ def textSearch_questions(srchString) | |||
| end | |||
| sresult | |||
| end | |||
|
|
|||
| def search_nearbyLocations(srchString) | |||
There was a problem hiding this comment.
This is looking great! Could we call this nearbyNodes?
| @@ -115,6 +115,30 @@ class Search < Grape::API | |||
| sresult | |||
| end | |||
|
|
|||
| # Request URL should be /api/srch/locations?srchString=QRY[&seq=KEYCOUNT&showCount=NUM_ROWS&pageNum=PAGE_NUM] | |||
There was a problem hiding this comment.
Super! Perhaps we can add this also to /doc/API.md?
There was a problem hiding this comment.
Curious to know. With those changes coming in, can't be have their unit tests/specs written along with it ? Just to ensure this code changes won't break anything if we have the right amount of unit tests in place. @jywarren any thoughts?
| coordinates = srchString.split(",") | ||
| lat = coordinates[0] | ||
| lon = coordinates[1] | ||
|
|
There was a problem hiding this comment.
latitude, longitude = coordinates
directly we can assign, ruby will take care of it.
There was a problem hiding this comment.
You are absolutely right but I think for sake of clarity of all writing one extra line is good
There was a problem hiding this comment.
Good suggestion! I always forget to do this myself.
| .collect(&:nid) | ||
|
|
||
| nids = nids || [] | ||
|
|
| get :locations do | ||
| sresult = DocList.new | ||
| unless params[:srchString].nil? || params[:srchString] == 0 || !(params[:srchString].include? ",") | ||
| sservice = SearchService.new |
There was a problem hiding this comment.
params[:srchString].nil? || params[:srchString] == 0
this particular code is kind of used in multiple places in this file. can't be move it to a private method and re-use it relevant place ?
There was a problem hiding this comment.
I think so -- but perhaps independently from this PR so we address code repetition as its own issues. Would you mind opening a new issue to address this? I agree that the services and api code in particular can be enormously compacted without loss of organization. Thank you!
|
jeff could you plz help with the build logs |
|
hmm, this is odd, no? let me take a look. |
|
|
|
oh, do you have two branches of almost the same name? |
|
Tough one -- it says the name collision may originate in a different PR -- scary! I'll watch if other PR's travis tests are failing... |
|
same logs again |
|
:-/ i'm not sure about this one! @publiclab/reviewers can anyone help debug this? |
|
There is warning about missing secret_key_base too.Do you think it has any role in this ? |
|
Hmm, that may be a question for @icarito ... It's a rails 4.1 thing!
…On Jan 12, 2018 5:27 PM, "Gaurav Sachdeva" ***@***.***> wrote:
There is warning about missing secret_key_base too.Do you think it has any
role to play?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1978 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJyhEf5dsI6xlbkI_3OfrA2MHCz0-ks5tJ9wRgaJpZM4RcxWP>
.
|
|
Jeff error was generated due to the wrong merging of code on my master branch.Its resolved now.Thanks |
Fixes #1934
API working.Suggestions regarding naming and comments are welcome.