Skip to content

Commit

Permalink
Search cleanup (mastodon#1333)
Browse files Browse the repository at this point in the history
* Clean up SQL output in Tag and Account search methods

* Add basic coverage for Tag.search_for

* Add coverage for Account.search_for

* Add coverage for Account.advanced_search_for
  • Loading branch information
mjankowski authored and Gargron committed Apr 9, 2017
1 parent 71706f2 commit 388ec0d
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 8 deletions.
12 changes: 6 additions & 6 deletions app/models/account.rb
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ def find_remote(username, domain)
end

def triadic_closures(account, limit = 5)
sql = <<SQL
sql = <<-SQL.squish
WITH first_degree AS (
SELECT target_account_id
FROM follows
Expand All @@ -216,7 +216,7 @@ def triadic_closures(account, limit = 5)
GROUP BY target_account_id, accounts.id
ORDER BY count(account_id) DESC
LIMIT ?
SQL
SQL

Account.find_by_sql([sql, account.id, account.id, limit])
end
Expand All @@ -226,15 +226,15 @@ def search_for(terms, limit = 10)
textsearch = '(setweight(to_tsvector(\'simple\', accounts.display_name), \'A\') || setweight(to_tsvector(\'simple\', accounts.username), \'B\') || setweight(to_tsvector(\'simple\', coalesce(accounts.domain, \'\')), \'C\'))'
query = 'to_tsquery(\'simple\', \'\'\' \' || ' + terms + ' || \' \'\'\' || \':*\')'

sql = <<SQL
sql = <<-SQL.squish
SELECT
accounts.*,
ts_rank_cd(#{textsearch}, #{query}, 32) AS rank
FROM accounts
WHERE #{query} @@ #{textsearch}
ORDER BY rank DESC
LIMIT ?
SQL
SQL

Account.find_by_sql([sql, limit])
end
Expand All @@ -244,7 +244,7 @@ def advanced_search_for(terms, account, limit = 10)
textsearch = '(setweight(to_tsvector(\'simple\', accounts.display_name), \'A\') || setweight(to_tsvector(\'simple\', accounts.username), \'B\') || setweight(to_tsvector(\'simple\', coalesce(accounts.domain, \'\')), \'C\'))'
query = 'to_tsquery(\'simple\', \'\'\' \' || ' + terms + ' || \' \'\'\' || \':*\')'

sql = <<SQL
sql = <<-SQL.squish
SELECT
accounts.*,
(count(f.id) + 1) * ts_rank_cd(#{textsearch}, #{query}, 32) AS rank
Expand All @@ -254,7 +254,7 @@ def advanced_search_for(terms, account, limit = 10)
GROUP BY accounts.id
ORDER BY rank DESC
LIMIT ?
SQL
SQL

Account.find_by_sql([sql, account.id, account.id, limit])
end
Expand Down
4 changes: 2 additions & 2 deletions app/models/tag.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ def search_for(terms, limit = 5)
textsearch = 'to_tsvector(\'simple\', tags.name)'
query = 'to_tsquery(\'simple\', \'\'\' \' || ' + terms + ' || \' \'\'\' || \':*\')'

sql = <<SQL
sql = <<-SQL.squish
SELECT
tags.*,
ts_rank_cd(#{textsearch}, #{query}) AS rank
FROM tags
WHERE #{query} @@ #{textsearch}
ORDER BY rank DESC
LIMIT ?
SQL
SQL

Tag.find_by_sql([sql, limit])
end
Expand Down
55 changes: 55 additions & 0 deletions spec/models/account_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,61 @@
end
end

describe '.search_for' do
before do
@match = Fabricate(
:account,
display_name: "Display Name",
username: "username",
domain: "example.com"
)
_missing = Fabricate(
:account,
display_name: "Missing",
username: "missing",
domain: "missing.com"
)
end

it 'finds accounts with matching display_name' do
results = Account.search_for("display")
expect(results).to eq [@match]
end

it 'finds accounts with matching username' do
results = Account.search_for("username")
expect(results).to eq [@match]
end

it 'finds accounts with matching domain' do
results = Account.search_for("example")
expect(results).to eq [@match]
end

it 'ranks multiple matches higher' do
account = Fabricate(
:account,
username: "username",
display_name: "username"
)
results = Account.search_for("username")
expect(results).to eq [account, @match]
end
end

describe '.advanced_search_for' do
it 'ranks followed accounts higher' do
account = Fabricate(:account)
match = Fabricate(:account, username: "Matching")
followed_match = Fabricate(:account, username: "Matcher")
Fabricate(:follow, account: account, target_account: followed_match)

results = Account.advanced_search_for("match", account)
expect(results).to eq [followed_match, match]
expect(results.first.rank).to be > results.last.rank
end
end

describe '.find_local' do
before do
Fabricate(:account, username: 'Alice')
Expand Down
11 changes: 11 additions & 0 deletions spec/models/tag_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,15 @@
expect(subject.match('https://en.wikipedia.org/wiki/Ghostbusters_(song)#Lawsuit')).to be_nil
end
end

describe '.search_for' do
it 'finds tag records with matching names' do
tag = Fabricate(:tag, name: "match")
_miss_tag = Fabricate(:tag, name: "miss")

results = Tag.search_for("match")

expect(results).to eq [tag]
end
end
end

0 comments on commit 388ec0d

Please sign in to comment.