Skip to content

Commit

Permalink
Search: Fix restrict_sr for domains listings and AllMinus
Browse files Browse the repository at this point in the history
For domains listings, it was possible to have results from the wrong domain be included.  Switching to a phrase search fixes that.  See https://redd.it/5ew4ve for more info about, and caveats of, this method.

For AllMinus, see https://redd.it/5eqiyy for the general context.  This does not need to be explicitly handled for ModMinus, as ModMinus excludes the filtered subreddits from its sr_ids property.  Additionally, ModMinus doesn't need special handling since it inherits the already-handled MultiReddit.
  • Loading branch information
Pokechu22 authored and bsimpson63 committed Dec 1, 2016
1 parent bb1abcc commit e5eb480
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 7 deletions.
10 changes: 8 additions & 2 deletions r2/r2/lib/providers/search/cloudsearch.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import r2.lib.utils as r2utils
from r2.models import (
Account,
AllMinus,
DomainSR,
FakeSubreddit,
FriendsSR,
Expand Down Expand Up @@ -751,7 +752,7 @@ def _restrict_recent(recent):
@staticmethod
def _restrict_sr(sr):
'''Return a cloudsearch appropriate query string that restricts
results to only contain results from self.sr
results to only contain results from sr
'''
if isinstance(sr, MultiReddit):
Expand All @@ -760,7 +761,7 @@ def _restrict_sr(sr):
srs = ["sr_id:%s" % sr_id for sr_id in sr.sr_ids]
return "(or %s)" % ' '.join(srs)
elif isinstance(sr, DomainSR):
return "site:'%s'" % sr.domain
return "site:'\"%s\"'" % sr.domain
elif isinstance(sr, FriendsSR):
if not c.user_is_loggedin or not c.user.friends:
raise InvalidQuery
Expand All @@ -771,6 +772,11 @@ def _restrict_sr(sr):
Account._fullname_from_id36(r2utils.to36(id_))
for id_ in friend_ids]
return "(or %s)" % ' '.join(friends)
elif isinstance(sr, AllMinus):
if not sr.exclude_sr_ids:
raise InvalidQuery
exclude_srs = ["sr_id:%s" % sr_id for sr_id in sr.exclude_sr_ids]
return "(not (or %s))" % ' '.join(exclude_srs)
elif not isinstance(sr, FakeSubreddit):
return "sr_id:%s" % sr._id

Expand Down
10 changes: 5 additions & 5 deletions r2/r2/lib/providers/search/solr.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@
from r2.models import (
Account,
All,
AllMinus,
DefaultSR,
DomainSR,
FakeSubreddit,
Friends,
Link,
ModContribSR,
MultiReddit,
NotFound,
Subreddit,
Expand Down Expand Up @@ -337,7 +337,7 @@ def _restrict_recent(recent):
@staticmethod
def _get_sr_restriction(sr):
'''Return a solr-appropriate query string that restricts
results to only contain results from self.sr
results to only contain results from sr
'''
bq = []
Expand All @@ -356,9 +356,9 @@ def _get_sr_restriction(sr):
Account._fullname_from_id36(r2utils.to36(id_))
for id_ in friend_ids]
bq.extend(friends)
elif isinstance(sr, ModContribSR):
for sr_id in sr.sr_ids:
bq.append("sr_id:%s" % sr_id)
elif isinstance(sr, AllMinus):
for sr_id in sr.exclude_sr_ids:
bq.append("-sr_id:%s" % sr_id)
elif not isinstance(sr, FakeSubreddit):
bq = ["sr_id:%s" % sr._id]
return ' OR '.join(bq)
Expand Down

0 comments on commit e5eb480

Please sign in to comment.