Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API Search profiles: adding parameter to search (only) by username #3235

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions app/api/srch/search.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ class Search < Grape::API
Search.execute(:all, params)
end

# Request URL should be /api/srch/profiles?srchString=QRY[&sort_by=recent&order_direction=desc&seq=KEYCOUNT&showCount=NUM_ROWS&pageNum=PAGE_NUM]
# Request URL should be /api/srch/profiles?srchString=QRY[&sort_by=recent&order_direction=desc&field=username&seq=KEYCOUNT&showCount=NUM_ROWS&pageNum=PAGE_NUM]
# Basic implementation from classic plots2 SearchController
desc 'Perform a search of profiles', hidden: false,
is_array: false,
nickname: 'srchGetProfiles'

params do
use :common, :sorting, :ordering
use :common, :sorting, :ordering, :field
end
get :profiles do
Search.execute(:profiles, params)
Expand Down Expand Up @@ -103,12 +103,8 @@ class Search < Grape::API

def self.execute(endpoint, params)
sresult = DocList.new
search_query = params[:srchString]
tag_query = params[:tagName]
order_query = params[:order_direction]
sort_query = params[:sort_by]
search_type = endpoint
search_criteria = SearchCriteria.new(search_query, tag: tag_query, sort_by: sort_query, order_direction: order_query)
search_criteria = SearchCriteria.new(params)

if search_criteria.valid?
sresult = ExecuteSearch.new.by(search_type, search_criteria)
Expand Down
5 changes: 5 additions & 0 deletions app/api/srch/shared_params.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ module SharedParams
optional :sort_by, type: String, documentation: { example: 'recent' }
end

params :field do
optional :field, type: String, documentation: { example: 'username' }
optional :limit, type: Integer, documentation: { example: 0 }
end

params :commontypeahead do
requires :srchString, type: String, documentation: { example: 'Spec' }
optional :seq, type: Integer, documentation: { example: 995 }
Expand Down
3 changes: 1 addition & 2 deletions app/api/srch/typeahead.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,8 @@ class Typeahead < Grape::API

def self.execute(endpoint, params)
sresult = TagList.new
search_query = params[:srchString]
search_type = endpoint
search_criteria = SearchCriteria.new(search_query)
search_criteria = SearchCriteria.new(params)

if search_criteria.valid?
sresult = ExecuteTypeahead.new.by(search_type, search_criteria, TYPEAHEAD_LIMIT)
Expand Down
2 changes: 1 addition & 1 deletion app/assets/javascripts/atwho_autocomplete.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
at: "@",
callbacks: {
remoteFilter: function(query, callback) {
$.getJSON("/api/srch/profiles?srchString=" + query, {}, function(data) {
$.getJSON("/api/srch/profiles?srchString=" + query + "&sort_by=recent&field=username", {}, function(data) {
if (data.hasOwnProperty('items') && data.items.length > 0) {
callback(data.items.map(function(i) { return i.docTitle }));
}
Expand Down
4 changes: 4 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ def self.search(query)
User.where('MATCH(bio, username) AGAINST(? IN BOOLEAN MODE)', query + '*')
end

def self.search_by_username(query)
User.where('MATCH(username) AGAINST(? IN BOOLEAN MODE)', query + '*')
end

def new_contributor
@uid = id
return "<span class = 'label label-success'><i>New Contributor</i></span>".html_safe if Node.where(:uid => @uid).length === 1
Expand Down
14 changes: 8 additions & 6 deletions app/services/search_criteria.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
class SearchCriteria
attr_reader :query, :tag, :sort_by
attr_reader :query, :tag, :sort_by, :field, :limit

def initialize(query, tag: nil, sort_by: nil, order_direction: "DESC")
@query = query
@tag = tag
@sort_by = sort_by
@order_direction = order_direction
def initialize(params)
@query = params[:srchString]
@tag = params[:tagName]
@sort_by = params[:sort_by]
@order_direction = params[:order_direction]
@field = params[:field]
@limit = params[:limit]
end

def valid?
Expand Down
7 changes: 4 additions & 3 deletions app/services/search_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,19 +50,20 @@ def textSearch_all(search_criteria)
# If no sort_by value present, then it returns a list of profiles ordered by id DESC
# a recent activity may be a node creation or a node revision
def profiles(search_criteria)
user_scope = SrchScope.find_users(search_criteria.query, limit = 10)
limit = search_criteria.limit ? search_criteria.limit : 10

user_scope = SrchScope.find_users(search_criteria.query, search_criteria.field, limit = 10)

user_scope =
if search_criteria.sort_by == "recent"
user_scope.joins(:revisions)
.order("node_revisions.timestamp #{search_criteria.order_direction}")
.distinct

else
user_scope.order(id: :desc)
end

users = user_scope.limit(10)
users = user_scope.limit(limit)

sresult = DocList.new
users.each do |match|
Expand Down
17 changes: 8 additions & 9 deletions app/services/srch_scope.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
# This class provides common methods that Typehead and Search services use
class SrchScope
def self.find_users(query, limit)
if ActiveRecord::Base.connection.adapter_name == 'Mysql2'
User.search(query)
.where('rusers.status = ?', 1)
.limit(limit)
else
User.where('username LIKE ? AND rusers.status = 1', '%' + input + '%')
.limit(limit)
end
def self.find_users(query, type = nil, limit)
users =
if ActiveRecord::Base.connection.adapter_name == 'Mysql2'
type == "username" ? User.search_by_username(query).where('rusers.status = ?', 1) : User.search(query).where('rusers.status = ?', 1)
else
User.where('username LIKE ? AND rusers.status = 1', '%' + query + '%')
end
users = users.limit(limit)
end

def self.find_tags(input, limit)
Expand Down
11 changes: 11 additions & 0 deletions doc/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,17 @@ https://publiclab.org/api/swagger_doc.json
Per-model API endpoints are:

* Profiles: https://publiclab.org/api/srch/profiles?srchString=foo

This also accepts the following additional parameters:

- `sort_by`: if 'recent' is passed, it returns the profiles from users with the most recent activity, otherwise, the results are sorted by user id (desc);
- `order_direction`: `ASC` or `DESC` (the latter is the default);
- `field`: if 'username' is passed, it returns the profiles from users searched by username only. Otherwise it returns the profiles by username and bio for a broader search.

Example of full URL with the optional params (using the order_by DESC default value):

https://publiclab.org/api/srch/profiles?srchString=foo&sort_by=recent&field=username

* Questions: https://publiclab.org/api/srch/questions?srchString=foo
* Tags: https://publiclab.org/api/srch/tags?srchString=foo
* Notes: https://publiclab.org/api/srch/notes?srchString=foo
Expand Down
7 changes: 7 additions & 0 deletions test/fixtures/drupal_users.yml
Original file line number Diff line number Diff line change
Expand Up @@ -116,3 +116,10 @@ steff3:
mail: steff3@pxlshp.com
uid: 17
created: <%= Time.now.to_i %>

data:
name: data
status: 1
mail: data@pxlshp.com
uid: 18
created: <%= Time.now.to_i %>
12 changes: 12 additions & 0 deletions test/fixtures/nodes.yml
Original file line number Diff line number Diff line change
Expand Up @@ -294,3 +294,15 @@ post_test3:
type: "note"
cached_likes: 0
slug: user3-<%= DateTime.new(2018,8,15).strftime("%m-%d-%Y") %>-post-test3

post_test4:
nid: 26
uid: 18
title: "Post Test 4"
path: "/notes/user4/<%= DateTime.new(2018,8,20).strftime("%m-%d-%Y") %>/post-test4"
created: <%= DateTime.new(2018,8,20).to_i %>
changed: <%= DateTime.new(2018,8,20).to_i %>
status: 1
type: "note"
cached_likes: 0
slug: user3-<%= DateTime.new(2018,8,20).strftime("%m-%d-%Y") %>-post-test4
11 changes: 9 additions & 2 deletions test/fixtures/revisions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,13 @@ post_test2:
post_test3:
nid: 25
uid: 16
title: Post test review 2
body: Post test review 2
title: Post test review 3
body: Post test review 3
timestamp: <%= DateTime.new(2018,8,15).to_i %>

post_test4:
nid: 26
uid: 18
title: Post test review 4
body: Post test review 4
timestamp: <%= DateTime.new(2018,8,20).to_i %>
15 changes: 14 additions & 1 deletion test/fixtures/users.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😄 💎

created_at: <%= Time.now %>
updated_at: <%= Time.now %>

Expand Down Expand Up @@ -210,3 +210,16 @@ steff3:
bio: ''
created_at: <%= Time.now %>
updated_at: <%= Time.now %>

data:
username: data
status: 1
email: data@pxlshp.com
id: 18
password_salt: <%= salt = Authlogic::Random.hex_token %>
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!'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😹

created_at: <%= Time.now %>
updated_at: <%= Time.now %>
4 changes: 2 additions & 2 deletions test/functional/notes_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ def teardown

assert_response :success
selector = css_select 'div.note'
assert_equal selector.size, 20
assert_equal selector.size, 21
assert_select "div p", 'Pending approval by community moderators. Please be patient!'
end

Expand Down Expand Up @@ -342,7 +342,7 @@ def teardown

assert_response :success
selector = css_select 'div.note'
assert_equal selector.size, 20
assert_equal selector.size, 21
assert_select "p", "Moderate first-time post: \n Approve\n Spam"
end

Expand Down
71 changes: 62 additions & 9 deletions test/functional/search_api_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ def app

end

# returns users by id when order_by is not provided and sorted direction default DESC
test 'search profiles without order_by and default sort_direction' do
get '/api/srch/profiles?srchString=steff'
# search by username and returns users by id when order_by is not provided and sorted direction default DESC
test 'search profiles by username without order_by and default sort_direction' do
get '/api/srch/profiles?srchString=steff&field=username'
assert last_response.ok?

# Expected search pattern
Expand All @@ -100,9 +100,9 @@ def app

end

# returns users sorteded by recent activity and order direction default DESC
test 'search recent profiles with sort_by=recent present' do
get '/api/srch/profiles?srchString=steff&sort_by=recent'
# search by username and returns users sorteded by recent activity and order direction default DESC
test 'search recent profiles by username with sort_by=recent present' do
get '/api/srch/profiles?srchString=steff&field=username&sort_by=recent'
assert last_response.ok?

# Expected search pattern
Expand All @@ -124,9 +124,9 @@ def app
assert matcher =~ json
end

# returns users ordered by recent activity and sorted by ASC direction
test 'search recent profiles with sort_by=recent present and order_direction ASC' do
get '/api/srch/profiles?srchString=steff&sort_by=recent&order_direction=ASC'
# search by username and returns users ordered by recent activity and sorted by ASC direction
test 'search recent profiles by username with sort_by=recent present and order_direction ASC' do
get '/api/srch/profiles?srchString=steff&field=username&sort_by=recent&order_direction=ASC'
assert last_response.ok?

# Expected search pattern
Expand All @@ -148,6 +148,59 @@ 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'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, looking on my phone... is there also a text for only bio content? Something that has no username matches?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jywarren we pushed some changes, thanks for the feedback!

assert last_response.ok?

# User.search() only works for mysql/mariadb
if ActiveRecord::Base.connection.adapter_name == 'Mysql2'
# Expected search pattern
pattern = {
srchParams: {
srchString: 'steff',
seq: nil,
}.ignore_extra_keys!
}.ignore_extra_keys!

matcher = JsonExpressions::Matcher.new(pattern)

json = JSON.parse(last_response.body)

assert_equal "/profile/data", json['items'][0]['docUrl']
assert_equal "/profile/steff3", json['items'][1]['docUrl']
assert_equal "/profile/steff2", json['items'][2]['docUrl']
assert_equal "/profile/steff1", json['items'][3]['docUrl']

assert matcher =~ json
end
end

# search by bio only
test 'search profiles by bio without order_by and default sort_direction' do
get '/api/srch/profiles?srchString=ruby'
assert last_response.ok?

# User.search() only works for mysql/mariadb
if ActiveRecord::Base.connection.adapter_name == 'Mysql2'
# Expected search pattern
pattern = {
srchParams: {
srchString: 'ruby',
seq: nil,
}.ignore_extra_keys!
}.ignore_extra_keys!

matcher = JsonExpressions::Matcher.new(pattern)

json = JSON.parse(last_response.body)

assert_equal "/profile/testuser", json['items'][0]['docUrl']

assert matcher =~ json
end
end

test 'search notes functionality' do
get '/api/srch/notes?srchString=Blog'
assert last_response.ok?
Expand Down
3 changes: 2 additions & 1 deletion test/unit/node_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,8 @@ class NodeTest < ActiveSupport::TestCase
notes = Node.research_notes
expected = [nodes(:one), nodes(:spam), nodes(:first_timer_note), nodes(:blog),
nodes(:moderated_user_note), nodes(:activity), nodes(:upgrade),
nodes(:draft), nodes(:post_test1), nodes(:post_test2), nodes(:post_test3)]
nodes(:draft), nodes(:post_test1), nodes(:post_test2),
nodes(:post_test3), nodes(:post_test4)]
assert_equal expected, notes
end

Expand Down