* rm sponsored, verified

* more clean-up

* still accept obsolete groups as search params, behind a gate

* add clean up migration
This commit is contained in:
Andrew Williamson 2024-09-20 10:04:17 +01:00 коммит произвёл GitHub
Родитель b16c61601f
Коммит ddc10704fb
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: B5690EEEBB952194
18 изменённых файлов: 119 добавлений и 105 удалений

Просмотреть файл

@ -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``.
============== ==========================================================

Просмотреть файл

@ -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``.
============== ==========================================================
-----------------------------

Просмотреть файл

@ -132,7 +132,6 @@ signers:
states:
recommended: true
recommended-android: true
verified: true
line: true
relative_start: 0h
duration: 26298h

Просмотреть файл

@ -789,7 +789,7 @@ class CurrentVersionSerializer(SimpleVersionSerializer):
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:

Просмотреть файл

@ -43,7 +43,7 @@ from olympia.applications.models import AppVersion
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 @@ class TestAddonModels(TestCase):
# 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 @@ class TestAddonModels(TestCase):
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 @@ class TestAddonModels(TestCase):
# 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

Просмотреть файл

@ -48,14 +48,7 @@ from olympia.bandwagon.models import CollectionAddon
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 @@ class TestAddonSearchView(ESTestCase):
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 @@ class TestAddonAutoCompleteSearchView(ESTestCase):
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,14 +6500,14 @@ class TestAddonAutoCompleteSearchView(ESTestCase):
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 = (
spotlight_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'
assert spotlight_result['promoted']['category'] == 'spotlight'
assert not_result['promoted'] is None

Просмотреть файл

@ -1060,14 +1060,14 @@ class LanguageToolsView(ListAPIView):
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 @@ class LanguageToolsView(ListAPIView):
# 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 @@ class LanguageToolsView(ListAPIView):
# 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

Просмотреть файл

@ -69,30 +69,7 @@ RECOMMENDED = PromotedClass(
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 @@ NOTABLE = PromotedClass(
PROMOTED_GROUPS = [
NOT_PROMOTED,
RECOMMENDED,
SPONSORED,
VERIFIED,
LINE,
SPOTLIGHT,
STRATEGIC,

Просмотреть файл

@ -5,7 +5,7 @@ from django.test.utils import override_settings
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 @@ class TestPrimaryHero(TestCase):
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.'
'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

Просмотреть файл

@ -1383,6 +1383,7 @@ DRF_API_GATES = {
'del-version-license-slug',
'del-preview-position',
'categories-application',
'promoted-verified-sponsored',
),
'v4': (
'l10n_flat_input_output',
@ -1398,12 +1399,14 @@ DRF_API_GATES = {
'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',
),
}

Просмотреть файл

@ -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)
]

Просмотреть файл

@ -4,12 +4,7 @@ from olympia import amo
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 @@ class TestPromotedAddonAdmin(TestCase):
# 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 @@ class TestPromotedAddonAdmin(TestCase):
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

Просмотреть файл

@ -19,9 +19,9 @@ class TestPromotedAddon(TestCase):
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 @@ class TestPromotedAddon(TestCase):
]
# 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()

Просмотреть файл

@ -38,7 +38,6 @@ from olympia.constants.promoted import (
LINE,
NOTABLE,
RECOMMENDED,
SPONSORED,
SPOTLIGHT,
STRATEGIC,
)
@ -551,7 +550,7 @@ class TestReviewHelper(TestReviewHelperBase):
)
# 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',

Просмотреть файл

@ -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 @@ class AddonGuidQueryParam(AddonQueryMultiParam):
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 @@ class AddonCategoryQueryParam(AddonQueryParam):
# 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 @@ class SearchParameterFilter(BaseFilterBackend):
# 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

Просмотреть файл

@ -3,6 +3,7 @@ from datetime import timedelta
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
@ -18,9 +19,7 @@ from olympia.constants.promoted import (
LINE,
PROMOTED_API_NAME_TO_IDS,
RECOMMENDED,
SPONSORED,
STRATEGIC,
VERIFIED,
)
from olympia.search.filters import (
AddonRatingQueryParam,
@ -38,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)
@ -419,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']
@ -959,8 +959,6 @@ class TestSearchParameterFilter(FilterTestsBase):
'promoted.group_id': [
# recommended shouldn't be there twice
RECOMMENDED.id,
SPONSORED.id,
VERIFIED.id,
LINE.id,
STRATEGIC.id,
]
@ -968,6 +966,25 @@ class TestSearchParameterFilter(FilterTestsBase):
}
] == 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']
@ -1185,7 +1202,7 @@ class TestCombinedFilter(FilterTestsBase):
@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_

Просмотреть файл

@ -5,7 +5,7 @@ from django.utils.encoding import force_str
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 @@ class TestRankingScenarios(ESTestCase):
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,
@ -1055,7 +1055,7 @@ class TestRankingScenarios(ESTestCase):
(
['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
),
)

Просмотреть файл

@ -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; }