Skip to content

Commit

Permalink
still accept obsolete groups as search params, behind a gate
Browse files Browse the repository at this point in the history
  • Loading branch information
eviljeff committed Sep 18, 2024
1 parent 423e812 commit 212cc66
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 14 deletions.
3 changes: 3 additions & 0 deletions src/olympia/lib/settings_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -1383,6 +1383,7 @@ def read_only_mode(env):
'del-version-license-slug',
'del-preview-position',
'categories-application',
'promoted-verified-sponsored',
),
'v4': (
'l10n_flat_input_output',
Expand All @@ -1398,12 +1399,14 @@ def read_only_mode(env):
'del-version-license-slug',
'del-preview-position',
'categories-application',
'promoted-verified-sponsored',
),
'v5': (
'addons-search-_score-field',
'ratings-can_reply',
'ratings-score-filter',
'addon-submission-api',
'promoted-verified-sponsored',
),
}

Expand Down
40 changes: 29 additions & 11 deletions src/olympia/search/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ class AddonQueryParam:
valid_values = None
es_field = None

def __init__(self, query_data):
self.query_data = query_data
def __init__(self, request):
self.request = request
self.query_data = request.GET

def get_value(self):
value = self.query_data.get(self.query_param, '')
Expand Down Expand Up @@ -77,8 +78,9 @@ class AddonQueryMultiParam:
valid_values = None # if None then all values are valid
es_field = None

def __init__(self, query_data):
self.query_data = query_data
def __init__(self, request):
self.query_data = request.GET
self.request = request

def process_value(self, value):
try:
Expand Down Expand Up @@ -185,7 +187,7 @@ class AddonAppVersionQueryParam(AddonQueryParam):

def get_values(self):
appversion = self.query_data.get(self.query_param)
app = AddonAppQueryParam(self.query_data).get_value()
app = AddonAppQueryParam(self.request).get_value()

if appversion and app:
# Get a min version less than X.0, and a max greater than X.0a
Expand Down Expand Up @@ -277,7 +279,10 @@ def get_es_query(self):
if not switch_is_active('return-to-amo-for-all-listed'):
filters.extend(
AddonPromotedQueryParam(
{AddonPromotedQueryParam.query_param: BADGED_API_NAME}
self.request,
query_data={
AddonPromotedQueryParam.query_param: BADGED_API_NAME
},
).get_es_query()
)
return filters
Expand Down Expand Up @@ -308,7 +313,7 @@ def __init__(self, request):
# dict in the categories constants and use that as the reverse dict,
# and make sure to use get_value_from_object_from_reverse_dict().
try:
types = AddonTypeQueryParam(self.query_data).get_values()
types = AddonTypeQueryParam(request).get_values()
self.reverse_dict = [CATEGORIES[type_] for type_ in types]
except KeyError as exc:
raise ValueError(
Expand Down Expand Up @@ -391,14 +396,27 @@ class AddonPromotedQueryParam(AddonQueryMultiParam):
reverse_dict = PROMOTED_API_NAME_TO_IDS
valid_values = PROMOTED_API_NAME_TO_IDS.values()

def __init__(self, request, query_data=None):
super().__init__(request)
if query_data is not None:
self.query_data = query_data

def get_values(self):
values = super().get_values()
obsolete = (
('verified', 'sponsored')
if is_gate_active(self.request, 'promoted-verified-sponsored')
else ()
)
values = str(self.query_data.get(self.query_param, '')).split(',')
processed_values = [
self.process_value(value) for value in values if value not in obsolete
]
# The values are lists of ids so flatten into a single list
return list({y for x in values for y in x})
return list({y for x in processed_values for y in x})

def get_app(self):
return (
AddonAppQueryParam(self.query_data).get_value()
AddonAppQueryParam(self.request).get_value()
if AddonAppQueryParam.query_param in self.query_data
else None
)
Expand Down Expand Up @@ -936,7 +954,7 @@ def get_applicable_clauses(self, request):
# present in the request, otherwise don't, to avoid raising
# exceptions because of missing params in complex filters.
if param_class.query_param in request.GET:
clauses.extend(param_class(request.GET).get_es_query())
clauses.extend(param_class(request).get_es_query())
except ValueError as exc:
raise serializers.ValidationError(*exc.args) from exc
return clauses
Expand Down
27 changes: 24 additions & 3 deletions src/olympia/search/tests/test_filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from unittest.mock import Mock, patch

from django.test.client import RequestFactory
from django.test.utils import override_settings
from django.utils import translation
from django.utils.http import urlsafe_base64_encode

Expand Down Expand Up @@ -36,11 +37,12 @@ class FilterTestsBase(TestCase):

def setUp(self):
super().setUp()
self.req = RequestFactory().get('/')
self.view_class = Mock()

def _filter(self, req=None, data=None):
req = req or RequestFactory().get('/', data=data or {})
if not req:
req = RequestFactory().get('/api/v5/', data=data or {})
req.version = 'v5'
queryset = Search()
for filter_class in self.filter_classes:
queryset = filter_class().filter_queryset(req, queryset, self.view_class)
Expand Down Expand Up @@ -417,7 +419,7 @@ class TestReviewedContentFilter(FilterTestsBase):
filter_classes = [ReviewedContentFilter]

def test_status(self):
qs = self._filter(self.req)
qs = self._filter()
assert 'must' not in qs['query']['bool']
filter_ = qs['query']['bool']['filter']

Expand Down Expand Up @@ -964,6 +966,25 @@ def test_search_by_promoted(self):
}
] == filter_

def test_search_by_promoted_obsolete_groups(self):
with self.assertRaises(serializers.ValidationError) as context:
with override_settings(DRF_API_GATES={}):
self._filter(data={'promoted': 'sponsored,line'})
assert context.exception.detail == ['Invalid "promoted" parameter.']

# test that now obsolete groups are silently filtered out
overridden_api_gates = {'v5': ('promoted-verified-sponsored',)}
with override_settings(DRF_API_GATES=overridden_api_gates):
qs = self._filter(data={'promoted': 'sponsored,line'})
filter_ = qs['query']['bool']['filter']
assert [{'terms': {'promoted.group_id': [LINE.id]}}] == filter_

# and repeat to check when there are no groups remaining
with override_settings(DRF_API_GATES=overridden_api_gates):
qs = self._filter(data={'promoted': 'verified'})
filter_ = qs['query']['bool']['filter']
assert [{'terms': {'promoted.group_id': []}}] == filter_

def test_search_by_color(self):
qs = self._filter(data={'color': 'ff0000'})
filter_ = qs['query']['bool']['filter']
Expand Down

0 comments on commit 212cc66

Please sign in to comment.