API Search profiles: adding parameter to search (only) by username #3235
Conversation
ghost
added
the
Generated by 🚫 Danger |
app/api/srch/search.rb
Outdated
| search_type = endpoint | ||
| search_criteria = SearchCriteria.new(search_query, tag: tag_query, sort_by: sort_query, order_direction: order_query) | ||
| search_criteria = SearchCriteria.new(search_query, tag: tag_query, sort_by: sort_query, order_direction: order_query, field: field_query) |
There was a problem hiding this comment.
Do you think we could break this up over multiple lines? It could be indented so they line up. And is there an advantage to making variables to store these values in the above lines versus directly using params[] in the .new call?
app/services/search_service.rb
Outdated
| user_scope = SrchScope.find_users(search_criteria.query, limit = 10) | ||
| user_scope = | ||
| if search_criteria.field == "username" | ||
| SrchScope.find_by_username(search_criteria.query, limit = 10) |
There was a problem hiding this comment.
Here we might think of setting an optional limit parameter in profiles(search_criteria, limit = 10) and then we could override it if needed?
app/services/srch_scope.rb
Outdated
| end | ||
| end | ||
|
|
||
| def self.find_by_username(query, limit) |
| @@ -148,6 +148,34 @@ def app | |||
| assert matcher =~ json | |||
| end | |||
|
|
|||
| # search by username and bio, returns users by id when order_by is not provided and sorted direction default DESC | |||
| test 'search profiles by username and bio without order_by and default sort_direction' do | |||
| get '/api/srch/profiles?srchString=steff' | |||
There was a problem hiding this comment.
Sorry, looking on my phone... is there also a text for only bio content? Something that has no username matches?
There was a problem hiding this comment.
we did like this: the previous tests (that we first had in mind to search only by username) have the field=username and therefore, returns only the users searched by username. So we added another one that would search only by username, but yeah, good point, we added a new one to search only by bio!
There was a problem hiding this comment.
@jywarren we pushed some changes, thanks for the feedback!
|
Hey @jywarren, We've just realized that code in the master branch contains an error. In line 9 of plots2/app/services/srch_scope.rb we forgot to replace input for query. |
|
@publiclab/reviewers anyone to review this PR? :) |
| @@ -129,7 +129,7 @@ test_user: | |||
| password_salt: <%= salt = Authlogic::Random.hex_token %> | |||
| crypted_password: <%= Authlogic::CryptoProviders::Sha512.encrypt("secretive" + salt) %> | |||
| persistence_token: <%= Authlogic::Random.hex_token %> | |||
| bio: '' | |||
| bio: 'I love ruby' | |||
| crypted_password: <%= Authlogic::CryptoProviders::Sha512.encrypt("secretive" + salt) %> | ||
| persistence_token: <%= Authlogic::Random.hex_token %> | ||
| last_request_at: <%= Time.now %> | ||
| bio: 'data is a nice cat and he loves steff!' |
|
Hi, this looks great, and I'm going to merge it. Just for future reference, if you can also say something like "ready to merge if you approve" then I can do so straight away... here it looks fine but sometimes folks want feedback but not actually to have it merged. Maybe that'll help me speed things up on my end a bit -- many thanks! |
|
thank you, @jywarren ! Ok, from now on we are gonna add this "ready to merge if you approve" tag xD |
…ubliclab#3235) * Add field username to profiles endpoint to be used on the autocomplete call * Version 1 * Added missing json * Change the params to create a new SearchCriteria instance * Adding limit parameter * Refactor find_users and add bio search test * fix codeclimate
Hey everybody!
We are opening this request to fix the issue #3228.
We added a new parameter to the profile endpoint so now we can search only by username. We will use this new functionality on the autocomplete feature.
We would like some feedback, thanks! :)
fixes #3228
@publiclab/reviewers @publiclab/soc @jywarren