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

Search: Only perform exact handle match if term is a valid handle #1455

Merged
merged 2 commits into from
Apr 19, 2021
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: 7 additions & 3 deletions services/brig/src/Brig/User/API/Search.hs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import Brig.User.Search.TeamUserSearch (RoleFilter (..), TeamUserSearchSortBy (.
import qualified Brig.User.Search.TeamUserSearch as Q
import Control.Lens (view)
import Data.Domain (Domain)
import Data.Handle (Handle (Handle))
import Data.Handle (parseHandle)
import Data.Id
import Data.Predicate
import Data.Range
Expand Down Expand Up @@ -199,9 +199,13 @@ searchLocally searcherId searchTerm maybeMaxResults = do

exactHandleSearch :: TeamSearchInfo -> Handler (Maybe Contact)
exactHandleSearch teamSearchInfo = do
let searchedHandleMaybe = parseHandle searchTerm
exactHandleResult <-
contactFromProfile
<$$> HandleAPI.getLocalHandleInfo searcherId (Handle searchTerm)
case searchedHandleMaybe of
Nothing -> pure Nothing
Just searchedHandle ->
contactFromProfile
<$$> HandleAPI.getLocalHandleInfo searcherId searchedHandle
pure $ case teamSearchInfo of
Search.TeamOnly t ->
if Just t == (contactTeam =<< exactHandleResult)
Expand Down
6 changes: 6 additions & 0 deletions services/brig/src/Brig/User/Handle.hs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@ glimpseHandle :: Handle -> AppIO (Maybe UserId)
glimpseHandle = lookupHandleWithPolicy One

{-# INLINE lookupHandleWithPolicy #-}

-- | Sending an empty 'Handle' here causes C* to throw "Key may not be empty"
-- error.
--
-- FUTUREWORK: This should ideally be tackled by hiding constructor for 'Handle'
-- and only allowing it to be parsed.
lookupHandleWithPolicy :: Consistency -> Handle -> AppIO (Maybe UserId)
lookupHandleWithPolicy policy h = do
join . fmap runIdentity
Expand Down
10 changes: 10 additions & 0 deletions services/brig/test/integration/API/Search.hs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ tests opts mgr galley brig = do
testWithBothIndices opts mgr "by-handle" $ testSearchByHandle brig,
testWithBothIndices opts mgr "size - when exact handle matches a team user" $ testSearchSize brig True,
testWithBothIndices opts mgr "size - when exact handle matches a non team user" $ testSearchSize brig False,
test mgr "empty query" $ testSearchEmpty brig,
test mgr "reindex" $ testReindex brig,
testWithBothIndices opts mgr "no match" $ testSearchNoMatch brig,
testWithBothIndices opts mgr "no extra results" $ testSearchNoExtraResults brig,
Expand Down Expand Up @@ -172,6 +173,15 @@ testSearchByHandle brig = do
Just h = fromHandle <$> userHandle u1
assertCanFind brig uid2 quid1 h

testSearchEmpty :: TestConstraints m => Brig -> m ()
testSearchEmpty brig = do
-- This user exists just in case empty string starts matching everything
_someUser <- randomUserWithHandle brig
searcher <- randomUser brig
refreshIndex brig
res <- searchResults <$> executeSearch brig (userId searcher) ""
liftIO $ assertEqual "nothing should be returned" [] res

testSearchSize :: TestConstraints m => Brig -> Bool -> m ()
testSearchSize brig exactHandleInTeam = do
(handleMatch, searchTerm) <-
Expand Down