From 9a32276b3a6cf81796e9648fd619e077216464c0 Mon Sep 17 00:00:00 2001 From: Andrew Williamson Date: Thu, 12 Sep 2024 08:03:27 +0100 Subject: [PATCH 1/4] rm sponsored, verified --- src/olympia/addons/tests/test_models.py | 14 +++++----- src/olympia/addons/tests/test_views.py | 19 +++++-------- src/olympia/constants/promoted.py | 27 +------------------ src/olympia/hero/tests/test_models.py | 10 +++---- src/olympia/promoted/tests/test_admin.py | 13 +++------ src/olympia/promoted/tests/test_models.py | 8 +++--- src/olympia/reviewers/tests/test_utils.py | 3 +-- src/olympia/search/tests/test_filters.py | 4 --- .../search/tests/test_search_ranking.py | 4 +-- 9 files changed, 30 insertions(+), 72 deletions(-) diff --git a/src/olympia/addons/tests/test_models.py b/src/olympia/addons/tests/test_models.py index 3fad538fc34..92a3c40d4d2 100644 --- a/src/olympia/addons/tests/test_models.py +++ b/src/olympia/addons/tests/test_models.py @@ -43,7 +43,7 @@ from olympia.bandwagon.models import Collection from olympia.blocklist.models import Block, BlocklistSubmission from olympia.constants.categories import CATEGORIES -from olympia.constants.promoted import LINE, NOT_PROMOTED, RECOMMENDED, SPONSORED +from olympia.constants.promoted import LINE, NOT_PROMOTED, RECOMMENDED, SPOTLIGHT from olympia.devhub.models import RssKey from olympia.files.models import File from olympia.files.tests.test_models import UploadMixin @@ -1765,17 +1765,17 @@ def test_promoted_group(self): # if the group has changes the approval for the current version isn't # valid - promoted.update(group_id=SPONSORED.id) + promoted.update(group_id=SPOTLIGHT.id) assert not addon.promoted_group() assert addon.promoted_group(currently_approved=False) - assert addon.promoted_group(currently_approved=False) == SPONSORED + assert addon.promoted_group(currently_approved=False) == SPOTLIGHT promoted.approve_for_version(version=addon.current_version) - assert addon.promoted_group() == SPONSORED + assert addon.promoted_group() == SPOTLIGHT # Application specific group membership should work too # if no app is specifed in the PromotedAddon everything should match - assert addon.promoted_group() == SPONSORED + assert addon.promoted_group() == SPOTLIGHT # update to mobile app promoted.update(application_id=amo.ANDROID.id) assert addon.promoted_group() @@ -1786,7 +1786,7 @@ def test_promoted_group(self): del addon.current_version.approved_for_groups assert not addon.promoted_group() promoted.update(application_id=amo.FIREFOX.id) - assert addon.promoted_group() == SPONSORED + assert addon.promoted_group() == SPOTLIGHT # check it doesn't error if there's no current_version addon.current_version.file.update(status=amo.STATUS_DISABLED) @@ -1812,7 +1812,7 @@ def test_promoted(self): # If the group changes the approval for the current version isn't # valid. - promoted.update(group_id=SPONSORED.id) + promoted.update(group_id=SPOTLIGHT.id) del addon.promoted assert addon.promoted is None diff --git a/src/olympia/addons/tests/test_views.py b/src/olympia/addons/tests/test_views.py index 9d926a010d7..64d6869da0f 100644 --- a/src/olympia/addons/tests/test_views.py +++ b/src/olympia/addons/tests/test_views.py @@ -48,14 +48,7 @@ from olympia.constants.browsers import CHROME from olympia.constants.categories import CATEGORIES, CATEGORIES_BY_ID from olympia.constants.licenses import LICENSE_GPL3 -from olympia.constants.promoted import ( - LINE, - RECOMMENDED, - SPONSORED, - SPOTLIGHT, - STRATEGIC, - VERIFIED, -) +from olympia.constants.promoted import LINE, RECOMMENDED, SPOTLIGHT, STRATEGIC from olympia.files.tests.test_models import UploadMixin from olympia.files.utils import parse_addon, parse_xpi from olympia.ratings.models import Rating @@ -5705,7 +5698,7 @@ def test_filter_by_promoted(self): assert {res['id'] for res in data['results']} == {addon.pk, addon5.pk} # test with other other promotions - for promo in (SPONSORED, VERIFIED, LINE, SPOTLIGHT, STRATEGIC): + for promo in (LINE, SPOTLIGHT, STRATEGIC): self.make_addon_promoted(addon, promo, approve_version=True) self.reindex(Addon) data = self.perform_search( @@ -6495,8 +6488,8 @@ def test_sort_ignored(self): def test_promoted(self): not_promoted = addon_factory(name='not promoted') - sponsored = addon_factory(name='is promoted') - self.make_addon_promoted(sponsored, SPONSORED, approve_version=True) + spotlight = addon_factory(name='is promoted') + self.make_addon_promoted(spotlight, SPOTLIGHT, approve_version=True) addon_factory(name='something') self.refresh() @@ -6507,11 +6500,11 @@ def test_promoted(self): assert 'prev' not in data assert len(data['results']) == 2 - assert {itm['id'] for itm in data['results']} == {not_promoted.pk, sponsored.pk} + assert {itm['id'] for itm in data['results']} == {not_promoted.pk, spotlight.pk} sponsored_result, not_result = ( (data['results'][0], data['results'][1]) - if data['results'][0]['id'] == sponsored.id + if data['results'][0]['id'] == spotlight.id else (data['results'][1], data['results'][0]) ) assert sponsored_result['promoted']['category'] == 'sponsored' diff --git a/src/olympia/constants/promoted.py b/src/olympia/constants/promoted.py index e16e505868e..4f4a3e78cf5 100644 --- a/src/olympia/constants/promoted.py +++ b/src/olympia/constants/promoted.py @@ -69,30 +69,7 @@ def __bool__(self): can_be_compatible_with_all_fenix_versions=True, ) -SPONSORED = PromotedClass( - id=2, - name=_('Sponsored'), - api_name='sponsored', - listed_pre_review=True, - badged=True, - autograph_signing_states={ - applications.FIREFOX.short: 'verified', - applications.ANDROID.short: 'verified', - }, - can_primary_hero=True, -) - -VERIFIED = PromotedClass( - id=3, - name=_('Verified'), - api_name='verified', - listed_pre_review=True, - badged=True, - autograph_signing_states={ - applications.FIREFOX.short: 'verified', - applications.ANDROID.short: 'verified', - }, -) +# id 3 & 4 were previously used for Sponsored and Verified LINE = PromotedClass( id=4, @@ -139,8 +116,6 @@ def __bool__(self): PROMOTED_GROUPS = [ NOT_PROMOTED, RECOMMENDED, - SPONSORED, - VERIFIED, LINE, SPOTLIGHT, STRATEGIC, diff --git a/src/olympia/hero/tests/test_models.py b/src/olympia/hero/tests/test_models.py index ad7f0a859c8..5bf4ce411a1 100644 --- a/src/olympia/hero/tests/test_models.py +++ b/src/olympia/hero/tests/test_models.py @@ -5,7 +5,7 @@ from olympia.amo.tests import TestCase, addon_factory from olympia.amo.tests.test_helpers import get_uploaded_file -from olympia.constants.promoted import RECOMMENDED, SPOTLIGHT, VERIFIED +from olympia.constants.promoted import RECOMMENDED, SPOTLIGHT, STRATEGIC from olympia.hero.models import ( PrimaryHero, PrimaryHeroImage, @@ -62,17 +62,17 @@ def test_clean_requires_approved_can_primary_hero_group(self): ph.clean() # it raises if there's an error # change to a different group - ph.promoted_addon.update(group_id=VERIFIED.id) + ph.promoted_addon.update(group_id=STRATEGIC.id) ph.promoted_addon.approve_for_version(ph.promoted_addon.addon.current_version) ph.reload() ph.enabled = True - assert ph.promoted_addon.addon.promoted_group() == VERIFIED + assert ph.promoted_addon.addon.promoted_group() == STRATEGIC with self.assertRaises(ValidationError) as context: - # VERIFIED isn't a group that can be added as a primary hero + # STRATEGIC isn't a group that can be added as a primary hero ph.clean() assert context.exception.messages == [ 'Only add-ons that are Recommended, Sponsored, By Firefox, ' - 'Spotlight can be enabled for non-external primary shelves.' + 'Strategic can be enabled for non-external primary shelves.' ] # change to a different group that *can* be added as a primary hero diff --git a/src/olympia/promoted/tests/test_admin.py b/src/olympia/promoted/tests/test_admin.py index d7110ab0118..b31cf057d15 100644 --- a/src/olympia/promoted/tests/test_admin.py +++ b/src/olympia/promoted/tests/test_admin.py @@ -4,12 +4,7 @@ from olympia.amo.reverse import django_reverse from olympia.amo.tests import TestCase, addon_factory, user_factory, version_factory from olympia.amo.tests.test_helpers import get_uploaded_file -from olympia.constants.promoted import ( - LINE, - NOT_PROMOTED, - RECOMMENDED, - VERIFIED, -) +from olympia.constants.promoted import LINE, NOT_PROMOTED, RECOMMENDED from olympia.hero.models import PrimaryHero, PrimaryHeroImage from olympia.promoted.models import PromotedAddon, PromotedApproval @@ -523,7 +518,7 @@ def test_can_delete_when_primary_hero_too(self): # The approval *won't* have been deleted though assert PromotedApproval.objects.filter().exists() - def test_updates_not_promoted_to_verified(self): + def test_updates_not_promoted_to_line(self): item = PromotedAddon.objects.create( addon=addon_factory(), group_id=NOT_PROMOTED.id ) @@ -537,11 +532,11 @@ def test_updates_not_promoted_to_verified(self): dict( self._get_approval_form(item, []), **self._get_heroform(item.id), - **{'group_id': VERIFIED.id}, # change group + **{'group_id': LINE.id}, # change group ), follow=True, ) item.reload() assert response.status_code == 200 - assert item.group_id == VERIFIED.id + assert item.group_id == LINE.id diff --git a/src/olympia/promoted/tests/test_models.py b/src/olympia/promoted/tests/test_models.py index 17e63dba57a..5242f573ff0 100644 --- a/src/olympia/promoted/tests/test_models.py +++ b/src/olympia/promoted/tests/test_models.py @@ -19,9 +19,9 @@ def setUp(self): def test_basic(self): promoted_addon = PromotedAddon.objects.create( - addon=addon_factory(), group_id=promoted.SPONSORED.id + addon=addon_factory(), group_id=promoted.LINE.id ) - assert promoted_addon.group == promoted.SPONSORED + assert promoted_addon.group == promoted.LINE assert promoted_addon.application_id is None assert promoted_addon.all_applications == [ applications.FIREFOX, @@ -49,12 +49,12 @@ def test_is_approved_applications(self): ] # but not if it's for a different type of promotion - promoted_addon.update(group_id=promoted.SPONSORED.id) + promoted_addon.update(group_id=promoted.SPOTLIGHT.id) assert addon.promotedaddon.approved_applications == [] # unless that group has an approval too PromotedApproval.objects.create( version=addon.current_version, - group_id=promoted.SPONSORED.id, + group_id=promoted.SPOTLIGHT.id, application_id=applications.FIREFOX.id, ) addon.reload() diff --git a/src/olympia/reviewers/tests/test_utils.py b/src/olympia/reviewers/tests/test_utils.py index 9ee7910dad3..a7be11f789c 100644 --- a/src/olympia/reviewers/tests/test_utils.py +++ b/src/olympia/reviewers/tests/test_utils.py @@ -38,7 +38,6 @@ LINE, NOTABLE, RECOMMENDED, - SPONSORED, SPOTLIGHT, STRATEGIC, ) @@ -551,7 +550,7 @@ def test_actions_promoted_admin_review_needs_admin_permission(self): ) # only for groups that are admin_review though - self.make_addon_promoted(self.addon, SPONSORED, approve_version=True) + self.make_addon_promoted(self.addon, NOTABLE, approve_version=True) expected = [ 'public', 'reject', diff --git a/src/olympia/search/tests/test_filters.py b/src/olympia/search/tests/test_filters.py index d674f3e5e94..d19e6f2839f 100644 --- a/src/olympia/search/tests/test_filters.py +++ b/src/olympia/search/tests/test_filters.py @@ -18,9 +18,7 @@ LINE, PROMOTED_API_NAME_TO_IDS, RECOMMENDED, - SPONSORED, STRATEGIC, - VERIFIED, ) from olympia.search.filters import ( AddonRatingQueryParam, @@ -959,8 +957,6 @@ def test_search_by_promoted(self): 'promoted.group_id': [ # recommended shouldn't be there twice RECOMMENDED.id, - SPONSORED.id, - VERIFIED.id, LINE.id, STRATEGIC.id, ] diff --git a/src/olympia/search/tests/test_search_ranking.py b/src/olympia/search/tests/test_search_ranking.py index f66af2c6216..b7201beb37f 100644 --- a/src/olympia/search/tests/test_search_ranking.py +++ b/src/olympia/search/tests/test_search_ranking.py @@ -5,7 +5,7 @@ from olympia import amo from olympia.amo.tests import APITestClientSessionID, ESTestCase, reverse_ns -from olympia.constants.promoted import LINE, RECOMMENDED, VERIFIED +from olympia.constants.promoted import LINE, RECOMMENDED, SPOTLIGHT from olympia.constants.search import SEARCH_LANGUAGE_TO_ANALYZER @@ -643,7 +643,7 @@ def setUpTestData(cls): slug='stripy-dog-3', summary='A new friend in every new window.', weekly_downloads=350, - promoted=VERIFIED, + promoted=SPOTLIGHT, ) amo.tests.addon_factory( average_daily_users=4089, From 423e812f56489fd8607de3cdc49bf91f447ff689 Mon Sep 17 00:00:00 2001 From: Andrew Williamson Date: Tue, 17 Sep 2024 18:05:01 +0100 Subject: [PATCH 2/4] more clean-up --- docs/topics/api/addons.rst | 4 +--- docs/topics/api/v4_frozen/addons.rst | 4 +--- scripts/autograph_localdev_config.yaml | 1 - src/olympia/addons/tests/test_views.py | 4 ++-- src/olympia/hero/tests/test_models.py | 4 ++-- src/olympia/search/tests/test_filters.py | 2 +- src/olympia/search/tests/test_search_ranking.py | 2 +- static/css/zamboni/reviewers.less | 2 -- 8 files changed, 8 insertions(+), 15 deletions(-) diff --git a/docs/topics/api/addons.rst b/docs/topics/api/addons.rst index 0c388de27ce..7fa0527f62e 100644 --- a/docs/topics/api/addons.rst +++ b/docs/topics/api/addons.rst @@ -278,14 +278,12 @@ This endpoint allows you to fetch a specific add-on by id, slug or guid. line "By Firefox" category notable Notable category recommended Recommended category - sponsored Sponsored category spotlight Spotlight category strategic Strategic category - verified Verified category badged A meta category that's available for the ``promoted`` search filter that is all the categories we expect an API client to expose as "reviewed" by Mozilla. - Currently equal to ``line&recommended&sponsored&verified``. + Currently equal to ``line&recommended``. ============== ========================================================== diff --git a/docs/topics/api/v4_frozen/addons.rst b/docs/topics/api/v4_frozen/addons.rst index 0cd6702b7ee..4b8703d1a87 100644 --- a/docs/topics/api/v4_frozen/addons.rst +++ b/docs/topics/api/v4_frozen/addons.rst @@ -270,14 +270,12 @@ This endpoint allows you to fetch a specific add-on by id, slug or guid. ============== ========================================================== line "By Firefox" category recommended Recommended category - sponsored Sponsored category spotlight Spotlight category strategic Strategic category - verified Verified category badged A meta category that's available for the ``promoted`` search filter that is all the categories we expect an API client to expose as "reviewed" by Mozilla. - Currently equal to ``line&recommended&sponsored&verified``. + Currently equal to ``line&recommended``. ============== ========================================================== ----------------------------- diff --git a/scripts/autograph_localdev_config.yaml b/scripts/autograph_localdev_config.yaml index 4786a86edb2..2ad22c64143 100644 --- a/scripts/autograph_localdev_config.yaml +++ b/scripts/autograph_localdev_config.yaml @@ -132,7 +132,6 @@ signers: states: recommended: true recommended-android: true - verified: true line: true relative_start: 0h duration: 26298h diff --git a/src/olympia/addons/tests/test_views.py b/src/olympia/addons/tests/test_views.py index 64d6869da0f..39b26b620af 100644 --- a/src/olympia/addons/tests/test_views.py +++ b/src/olympia/addons/tests/test_views.py @@ -6502,12 +6502,12 @@ def test_promoted(self): assert {itm['id'] for itm in data['results']} == {not_promoted.pk, spotlight.pk} - sponsored_result, not_result = ( + spotlight_result, not_result = ( (data['results'][0], data['results'][1]) if data['results'][0]['id'] == spotlight.id else (data['results'][1], data['results'][0]) ) - assert sponsored_result['promoted']['category'] == 'sponsored' + assert spotlight_result['promoted']['category'] == 'spotlight' assert not_result['promoted'] is None diff --git a/src/olympia/hero/tests/test_models.py b/src/olympia/hero/tests/test_models.py index 5bf4ce411a1..73f2cf8fe88 100644 --- a/src/olympia/hero/tests/test_models.py +++ b/src/olympia/hero/tests/test_models.py @@ -71,8 +71,8 @@ def test_clean_requires_approved_can_primary_hero_group(self): # STRATEGIC isn't a group that can be added as a primary hero ph.clean() assert context.exception.messages == [ - 'Only add-ons that are Recommended, Sponsored, By Firefox, ' - 'Strategic can be enabled for non-external primary shelves.' + 'Only add-ons that are Recommended, By Firefox, Spotlight can be enabled ' + 'for non-external primary shelves.' ] # change to a different group that *can* be added as a primary hero diff --git a/src/olympia/search/tests/test_filters.py b/src/olympia/search/tests/test_filters.py index d19e6f2839f..ecc2a41b5b9 100644 --- a/src/olympia/search/tests/test_filters.py +++ b/src/olympia/search/tests/test_filters.py @@ -1181,7 +1181,7 @@ def test_filter_featured_sort_random(self): @freeze_time('2023-12-24') def test_filter_promoted_sort_random(self): - qs = self._filter(data={'promoted': 'verified', 'sort': 'random'}) + qs = self._filter(data={'promoted': 'spotlight', 'sort': 'random'}) bool_ = qs['query']['bool'] assert 'must_not' not in bool_ diff --git a/src/olympia/search/tests/test_search_ranking.py b/src/olympia/search/tests/test_search_ranking.py index b7201beb37f..8c3549b9477 100644 --- a/src/olympia/search/tests/test_search_ranking.py +++ b/src/olympia/search/tests/test_search_ranking.py @@ -1055,7 +1055,7 @@ def test_scenario_promoted(self): ( ['Stripy Dog 1', 2921], # recommended ['Stripy Dog 2', 2921], # line - ['Stripy Dog 3', 584], # verified (no boost) + ['Stripy Dog 3', 584], # spotlight (no boost) ['Stripy Dog 4', 584], # not promoted ), ) diff --git a/static/css/zamboni/reviewers.less b/static/css/zamboni/reviewers.less index 2b29c567820..48dada0d7f8 100644 --- a/static/css/zamboni/reviewers.less +++ b/static/css/zamboni/reviewers.less @@ -45,8 +45,6 @@ .ed-sprite-auto-approval-delayed-temporarily { background-position: 0 -288px; } .ed-sprite-auto-approval-delayed-indefinitely { background-position: 0 -304px; } .ed-sprite-promoted-recommended { background-position: 0 -336px; } -.ed-sprite-promoted-sponsored { background-position: 0 -352px; } -.ed-sprite-promoted-verified { background-position: 0 -352px; } .ed-sprite-promoted-line { background-position: 0 -368px; } .ed-sprite-promoted-strategic { background-position: 0 -320px; } .ed-sprite-promoted-spotlight { background-position: 0 -320px; } From b8edcc8823429974db93a1c8276c8aa3f6eae37e Mon Sep 17 00:00:00 2001 From: Andrew Williamson Date: Wed, 18 Sep 2024 11:44:14 +0100 Subject: [PATCH 3/4] still accept obsolete groups as search params, behind a gate --- src/olympia/addons/serializers.py | 2 +- src/olympia/addons/views.py | 8 ++--- src/olympia/lib/settings_base.py | 3 ++ src/olympia/search/filters.py | 40 +++++++++++++++++------- src/olympia/search/tests/test_filters.py | 27 ++++++++++++++-- 5 files changed, 61 insertions(+), 19 deletions(-) diff --git a/src/olympia/addons/serializers.py b/src/olympia/addons/serializers.py index 72ce87a1ddc..312c8523299 100644 --- a/src/olympia/addons/serializers.py +++ b/src/olympia/addons/serializers.py @@ -789,7 +789,7 @@ def get_current_compatible_version(self, addon): try: # AddonAppVersionQueryParam.get_values() returns (app_id, min, max) # but we want {'min': min, 'max': max}. - value = AddonAppVersionQueryParam(request.GET).get_values() + value = AddonAppVersionQueryParam(request).get_values() application = value[0] appversions = dict(zip(('min', 'max'), value[1:], strict=True)) except ValueError as exc: diff --git a/src/olympia/addons/views.py b/src/olympia/addons/views.py index 7f4128bae79..11aafecd0dd 100644 --- a/src/olympia/addons/views.py +++ b/src/olympia/addons/views.py @@ -1060,14 +1060,14 @@ def get_query_params(self): if AddonAppVersionQueryParam.query_param in self.request.GET: # app parameter is mandatory with appversion try: - application = AddonAppQueryParam(self.request.GET).get_value() + application = AddonAppQueryParam(self.request).get_value() except ValueError as exc: raise exceptions.ParseError( 'Invalid or missing app parameter while appversion parameter is ' 'set.' ) from exc try: - value = AddonAppVersionQueryParam(self.request.GET).get_values() + value = AddonAppVersionQueryParam(self.request).get_values() appversions = {'min': value[1], 'max': value[2]} except ValueError as exc: raise exceptions.ParseError('Invalid appversion parameter.') from exc @@ -1081,7 +1081,7 @@ def get_query_params(self): # to filter by type if they want appversion filtering. if AddonTypeQueryParam.query_param in self.request.GET or appversions: try: - addon_types = tuple(AddonTypeQueryParam(self.request.GET).get_values()) + addon_types = tuple(AddonTypeQueryParam(self.request).get_values()) except ValueError as exc: raise exceptions.ParseError( 'Invalid or missing type parameter while appversion ' @@ -1093,7 +1093,7 @@ def get_query_params(self): # author is optional. It's a string representing the username(s) we're # filtering on. if AddonAuthorQueryParam.query_param in self.request.GET: - authors = AddonAuthorQueryParam(self.request.GET).get_values() + authors = AddonAuthorQueryParam(self.request).get_values() else: authors = None diff --git a/src/olympia/lib/settings_base.py b/src/olympia/lib/settings_base.py index 005f861be05..f99c4fdaba7 100644 --- a/src/olympia/lib/settings_base.py +++ b/src/olympia/lib/settings_base.py @@ -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', @@ -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', ), } diff --git a/src/olympia/search/filters.py b/src/olympia/search/filters.py index 32b78fab2a6..78fa308b70e 100644 --- a/src/olympia/search/filters.py +++ b/src/olympia/search/filters.py @@ -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, '') @@ -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: @@ -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 @@ -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 @@ -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( @@ -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 ) @@ -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 diff --git a/src/olympia/search/tests/test_filters.py b/src/olympia/search/tests/test_filters.py index ecc2a41b5b9..15ebad5ff2b 100644 --- a/src/olympia/search/tests/test_filters.py +++ b/src/olympia/search/tests/test_filters.py @@ -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 @@ -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) @@ -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'] @@ -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'] From 35a4b178256b5f0a3dfa9fe608b9e244c2fa8b6b Mon Sep 17 00:00:00 2001 From: Andrew Williamson Date: Thu, 19 Sep 2024 10:58:59 +0100 Subject: [PATCH 4/4] add clean up migration --- .../migrations/0021_auto_20240919_0952.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 src/olympia/promoted/migrations/0021_auto_20240919_0952.py diff --git a/src/olympia/promoted/migrations/0021_auto_20240919_0952.py b/src/olympia/promoted/migrations/0021_auto_20240919_0952.py new file mode 100644 index 00000000000..cd655aa04c1 --- /dev/null +++ b/src/olympia/promoted/migrations/0021_auto_20240919_0952.py @@ -0,0 +1,21 @@ +# Generated by Django 4.2.16 on 2024-09-19 09:52 + +from django.db import migrations + + +def delete_verified_sponsored_promoted_usage(apps, schema_editor): + PromotedAddon = apps.get_model('promoted', 'PromotedAddon') + PromotedApproval = apps.get_model('promoted', 'PromotedApproval') + PromotedAddon.objects.filter(group_id__in=(3, 4)).update(group_id=0) + PromotedApproval.objects.filter(group_id__in=(3, 4)).delete() + + +class Migration(migrations.Migration): + + dependencies = [ + ('promoted', '0020_auto_20221214_1331'), + ] + + operations = [ + migrations.RunPython(delete_verified_sponsored_promoted_usage, migrations.RunPython.noop) + ]