From c8ccf36fe51a7d9e0196bf4eb8b819d674265144 Mon Sep 17 00:00:00 2001 From: Andrew Williamson Date: Fri, 19 Jun 2026 18:33:56 +0100 Subject: [PATCH 01/14] rm ADDONS_RECOMMENDED_REVIEW --- src/olympia/access/acl.py | 6 +----- src/olympia/constants/permissions.py | 2 -- src/olympia/reviewers/models.py | 1 - src/olympia/reviewers/templates/reviewers/review.html | 6 ++---- src/olympia/reviewers/utils.py | 6 +----- 5 files changed, 4 insertions(+), 17 deletions(-) diff --git a/src/olympia/access/acl.py b/src/olympia/access/acl.py index abcefab352e3..0fc3611ba45b 100644 --- a/src/olympia/access/acl.py +++ b/src/olympia/access/acl.py @@ -117,10 +117,7 @@ def check_addon_ownership( def is_listed_addons_reviewer(user, allow_content_reviewers=True): - permissions = [ - amo.permissions.ADDONS_REVIEW, - amo.permissions.ADDONS_RECOMMENDED_REVIEW, - ] + permissions = [amo.permissions.ADDONS_REVIEW] if allow_content_reviewers: permissions.append(amo.permissions.ADDONS_CONTENT_REVIEW) allow_access = any(action_allowed_for(user, perm) for perm in permissions) @@ -178,7 +175,6 @@ def is_user_any_kind_of_reviewer(user, allow_viewers=False): amo.permissions.ADDONS_REVIEW, amo.permissions.ADDONS_REVIEW_UNLISTED, amo.permissions.ADDONS_CONTENT_REVIEW, - amo.permissions.ADDONS_RECOMMENDED_REVIEW, amo.permissions.STATIC_THEMES_REVIEW, ] if allow_viewers: diff --git a/src/olympia/constants/permissions.py b/src/olympia/constants/permissions.py index a685a87aa8e1..506be9741a3e 100644 --- a/src/olympia/constants/permissions.py +++ b/src/olympia/constants/permissions.py @@ -48,8 +48,6 @@ ADDON_REVIEWER_MOTD_EDIT = AclPermission('AddonReviewerMOTD', 'Edit') # Can review a static theme. STATIC_THEMES_REVIEW = AclPermission('Addons', 'ThemeReview') -# Can review recommend(ed|able) add-ons -ADDONS_RECOMMENDED_REVIEW = AclPermission('Addons', 'RecommendedReview') # Can triage (and therefore see in the queues) add-ons with a temporary delay ADDONS_TRIAGE_DELAYED = AclPermission('Addons', 'TriageDelayed') # Can see add-ons with all due dates in the queue, rather than just upcoming ones diff --git a/src/olympia/reviewers/models.py b/src/olympia/reviewers/models.py index f40521cb963d..92d32fd44807 100644 --- a/src/olympia/reviewers/models.py +++ b/src/olympia/reviewers/models.py @@ -192,7 +192,6 @@ def send_notifications(sender=None, instance=None, signal=None, **kw): listed_perms = [ amo.permissions.ADDONS_REVIEW, amo.permissions.ADDONS_CONTENT_REVIEW, - amo.permissions.ADDONS_RECOMMENDED_REVIEW, amo.permissions.STATIC_THEMES_REVIEW, amo.permissions.REVIEWER_TOOLS_VIEW, ] diff --git a/src/olympia/reviewers/templates/reviewers/review.html b/src/olympia/reviewers/templates/reviewers/review.html index fc82342097c1..753bfa8bd859 100644 --- a/src/olympia/reviewers/templates/reviewers/review.html +++ b/src/olympia/reviewers/templates/reviewers/review.html @@ -36,8 +36,7 @@

by {# Technically could be just pending approval, but that doesn't matter for this message. #}

This is a {{ promoted_groups.name }} add-on. - {% if (amo.promoted.PROMOTED_GROUP_CHOICES.RECOMMENDED in promoted_groups.group_id and not action_allowed_for(request.user, amo.permissions.ADDONS_RECOMMENDED_REVIEW)) or - (promoted_groups.admin_review|python_any and not action_allowed_for(request.user, amo.permissions.REVIEWS_ADMIN)) %} + {% if (promoted_groups.admin_review|python_any and not action_allowed_for(request.user, amo.permissions.REVIEWS_ADMIN)) %} You don't have permission to review it. {% endif %}

@@ -132,8 +131,7 @@

{# Technically could be just pending approval, but that doesn't matter for this message. #}

This is a {{ promoted_groups.name }} add-on. - {% if (amo.promoted.PROMOTED_GROUP_CHOICES.RECOMMENDED in promoted_groups.group_id and not action_allowed_for(request.user, amo.permissions.ADDONS_RECOMMENDED_REVIEW)) or - (promoted_groups.admin_review|python_any and not action_allowed_for(request.user, amo.permissions.REVIEWS_ADMIN)) %} + {% if (promoted_groups.admin_review|python_any and not action_allowed_for(request.user, amo.permissions.REVIEWS_ADMIN)) %} You don't have permission to review it. {% endif %}

diff --git a/src/olympia/reviewers/utils.py b/src/olympia/reviewers/utils.py index e13f8f862928..6ff13e387f38 100644 --- a/src/olympia/reviewers/utils.py +++ b/src/olympia/reviewers/utils.py @@ -32,7 +32,6 @@ AddonUser, ) from olympia.constants.abuse import DECISION_ACTIONS -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES from olympia.files.models import File from olympia.lib.crypto.signing import sign_file from olympia.ratings.models import Rating @@ -430,10 +429,7 @@ def get_actions(self): is_admin_needed = is_admin_needed_post_review = False # More complex/specific cases. - if PROMOTED_GROUP_CHOICES.RECOMMENDED in promoted_group.group_id: - permission = amo.permissions.ADDONS_RECOMMENDED_REVIEW - permission_post_review = permission - elif version_is_unlisted: + if version_is_unlisted: permission = amo.permissions.ADDONS_REVIEW_UNLISTED permission_post_review = permission elif any(promoted_group.admin_review): From 10b91c3b9901742bbacb2c0e3a1f435ce35c939f Mon Sep 17 00:00:00 2001 From: Andrew Williamson Date: Fri, 19 Jun 2026 19:28:50 +0100 Subject: [PATCH 02/14] rm PROMOTED_GROUP_CHOICES --- src/olympia/addons/indexers.py | 8 +--- src/olympia/addons/models.py | 10 +---- src/olympia/addons/serializers.py | 15 +++---- src/olympia/amo/sitemap.py | 6 +-- src/olympia/constants/promoted.py | 39 +------------------ src/olympia/discovery/views.py | 8 ++-- .../migrations/0014_auto_20200717_1120.py | 2 +- .../commands/promote_by_firefox_themes.py | 3 +- .../migrations/0006_auto_20200819_0951.py | 2 +- .../migrations/0021_auto_20240919_0952.py | 2 +- ...p_alter_promotedaddon_group_id_and_more.py | 2 +- .../migrations/0026_auto_20250401_1656.py | 2 +- .../migrations/0028_auto_20250425_1948.py | 4 +- .../migrations/0029_auto_20250429_1315.py | 4 +- .../migrations/0031_auto_20250526_1200.py | 2 +- src/olympia/promoted/migrations/__init__.py | 12 ++++++ src/olympia/promoted/models.py | 22 +---------- src/olympia/promoted/tasks.py | 24 +++++------- src/olympia/reviewers/cron.py | 8 ++-- src/olympia/search/filters.py | 34 ++-------------- src/olympia/versions/models.py | 2 +- 21 files changed, 59 insertions(+), 152 deletions(-) diff --git a/src/olympia/addons/indexers.py b/src/olympia/addons/indexers.py index 0e702a6263c0..4edbeedcb37d 100644 --- a/src/olympia/addons/indexers.py +++ b/src/olympia/addons/indexers.py @@ -4,7 +4,7 @@ from olympia import amo from olympia.amo.celery import create_chunked_tasks_signatures from olympia.amo.utils import attach_trans_dict, to_language -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.constants.search import SEARCH_LANGUAGE_TO_ANALYZER from olympia.search.utils import create_index from olympia.versions.compare import version_int @@ -487,7 +487,6 @@ def get_mapping(cls): 'promoted': { 'type': 'object', 'properties': { - 'group_id': {'type': 'byte'}, 'category': {'type': 'keyword'}, 'approved_for_apps': {'type': 'byte'}, }, @@ -662,7 +661,7 @@ def extract_document(cls, obj): data['has_privacy_policy'] = bool(obj.privacy_policy) data['is_recommended'] = any( - PROMOTED_GROUP_CHOICES.RECOMMENDED == promotion.group_id + promotion.api_name == RECOMMENDED_API_NAME for promotion in obj.publicly_promoted_groups ) @@ -678,7 +677,6 @@ def extract_document(cls, obj): data['promoted'] = [ { - 'group_id': promotion.group_id, 'category': promotion.api_name, # store the app approvals because .approved_applications needs it. 'approved_for_apps': [ @@ -688,8 +686,6 @@ def extract_document(cls, obj): for promotion in obj.publicly_promoted_groups ] # Add an additional bump for certain promoted groups. - # TODO: actually use this value in the search ranking algorithm, and drop - # the existing function that uses PROMOTED_GROUPS_CHOICES max_promoted_ranking_bump = max( ( promotion.search_ranking_bump diff --git a/src/olympia/addons/models.py b/src/olympia/addons/models.py index 55d49f92ec34..bacd836dbcc9 100644 --- a/src/olympia/addons/models.py +++ b/src/olympia/addons/models.py @@ -59,9 +59,7 @@ from olympia.constants.blocklist import BlockReason from olympia.constants.browsers import BROWSERS from olympia.constants.categories import CATEGORIES_BY_ID -from olympia.constants.promoted import ( - PROMOTED_GROUP_CHOICES, -) +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.constants.reviewers import REPUTATION_CHOICES from olympia.files.models import File from olympia.files.utils import extract_translations, resolve_i18n_message @@ -1631,11 +1629,7 @@ def publicly_promoted_groups(self): if self._is_recommended_theme(): from olympia.promoted.models import PromotedGroup - return [ - PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED - ) - ] + return [PromotedGroup.objects.get(api_name=RECOMMENDED_API_NAME)] return [] @cached_property diff --git a/src/olympia/addons/serializers.py b/src/olympia/addons/serializers.py index 939578575c44..01997f28e3df 100644 --- a/src/olympia/addons/serializers.py +++ b/src/olympia/addons/serializers.py @@ -41,7 +41,7 @@ from olympia.constants.applications import APP_IDS, APPS_ALL from olympia.constants.base import ADDON_TYPE_CHOICES_API from olympia.constants.categories import CATEGORIES_BY_ID -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.files.models import File, FileUpload from olympia.files.utils import DuplicateAddonID, parse_addon from olympia.promoted.models import PromotedGroup @@ -1000,10 +1000,7 @@ def validate_user_id(self, value): class PromotedGroupSerializer(AMOModelSerializer): apps = serializers.SerializerMethodField() - category = ReverseChoiceField( - choices=PROMOTED_GROUP_CHOICES.api_choices, - source='group_id', - ) + category = serializers.CharField(source='api_name') class Meta: model = PromotedGroup @@ -1204,7 +1201,7 @@ def get_is_featured(self, obj): # fake it with promoted status instead. def is_recommended(obj): return any( - PROMOTED_GROUP_CHOICES.RECOMMENDED == promotion.group_id + promotion.api_name == RECOMMENDED_API_NAME for promotion in obj.publicly_promoted_groups ) @@ -1652,11 +1649,9 @@ def fake_object(self, data): obj.promoted = [] for promotion in promoted_list: approved_for_apps = promotion.get('approved_for_apps') - group = PROMOTED_GROUP_CHOICES(promotion['group_id']) obj.promoted.append( { - 'group_id': promotion['group_id'], - 'category': promotion.get('category', group.api_value), + 'category': promotion.get('category'), 'apps': [ APP_IDS.get(app_id).short for app_id in approved_for_apps ], @@ -1702,7 +1697,7 @@ def get_promoted(self, obj): def get_is_featured(self, obj): def is_recommended(obj): return any( - PROMOTED_GROUP_CHOICES.RECOMMENDED == promotion['group_id'] + promotion['category'] == RECOMMENDED_API_NAME for promotion in obj.promoted ) diff --git a/src/olympia/amo/sitemap.py b/src/olympia/amo/sitemap.py index 59cc421b715f..b3b5bd4724e5 100644 --- a/src/olympia/amo/sitemap.py +++ b/src/olympia/amo/sitemap.py @@ -20,7 +20,7 @@ from olympia.amo.utils import id_to_path from olympia.bandwagon.models import Collection from olympia.constants.categories import CATEGORIES -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.promoted.models import PromotedAddon from olympia.tags.models import AddonTag, Tag from olympia.users.models import UserProfile @@ -135,9 +135,9 @@ def _current_app(self): def get_android_promoted_addons(): return PromotedAddon.objects.filter( Q(application_id=amo.ANDROID.id) | Q(application_id__isnull=True), - promoted_group__group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_group__api_name=RECOMMENDED_API_NAME, addon___current_version__promoted_versions__application_id=(amo.ANDROID.id), - addon___current_version__promoted_versions__promoted_group__group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + addon___current_version__promoted_versions__promoted_group__api_name=RECOMMENDED_API_NAME, ).values_list('addon_id', flat=True) diff --git a/src/olympia/constants/promoted.py b/src/olympia/constants/promoted.py index dbb5b0654b73..c788b09ee6c8 100644 --- a/src/olympia/constants/promoted.py +++ b/src/olympia/constants/promoted.py @@ -1,38 +1,3 @@ -from types import DynamicClassAttribute - -from olympia.amo.enum import EnumChoices - - -class _EnumChoicesWithSearchRankingBump(EnumChoices): - @DynamicClassAttribute - def search_ranking_bump(self): - return { - 'RECOMMENDED': 5.0, - 'LINE': 5.0, - }.get(self.name, 0.0) - - -class PROMOTED_GROUP_CHOICES(_EnumChoicesWithSearchRankingBump): - RECOMMENDED = 1, 'Recommended' - LINE = 4, 'By Firefox' - SPOTLIGHT = 5, 'Spotlight' - STRATEGIC = 6, 'Strategic' - NOTABLE = 7, 'Notable' - SPONSORED = 8, 'Sponsored' - VERIFIED = 9, 'Verified' - PARTNER = 10, 'Partner' - - -PROMOTED_GROUP_CHOICES.add_subset('BADGED', ('RECOMMENDED', 'LINE')) -PROMOTED_GROUP_CHOICES.add_subset( - 'ACTIVE', ('RECOMMENDED', 'LINE', 'SPOTLIGHT', 'STRATEGIC', 'NOTABLE', 'PARTNER') -) -# Note: SPONSORED & VERIFIED should not be included, they are no longer valid promoted -# groups - BADGED_API_NAME = 'badged' # Special alias for all badged groups - -PROMOTED_API_NAME_TO_IDS = { - **{p.api_value: [p.value] for p in PROMOTED_GROUP_CHOICES.ACTIVE}, - BADGED_API_NAME: [value for value in PROMOTED_GROUP_CHOICES.BADGED.values], -} +RECOMMENDED_API_NAME = 'recommended' # Name of the "original" promoted group +NOTABLE_API_NAME = 'notable' # Name of the group we automatically add add-ons to diff --git a/src/olympia/discovery/views.py b/src/olympia/discovery/views.py index 08100f236de9..9e83f7068936 100644 --- a/src/olympia/discovery/views.py +++ b/src/olympia/discovery/views.py @@ -5,7 +5,7 @@ from rest_framework.response import Response from rest_framework.viewsets import GenericViewSet -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.discovery.models import DiscoveryItem from olympia.discovery.serializers import ( DiscoveryEditorialContentSerializer, @@ -71,10 +71,8 @@ def filter_queryset(self, qs): if self.request.query_params.get('recommended', False) == 'true': qs = qs.filter( **{ - 'addon__promotedaddon__promoted_group__group_id': ( - PROMOTED_GROUP_CHOICES.RECOMMENDED - ), - 'addon___current_version__promoted_versions__promoted_group__group_id': PROMOTED_GROUP_CHOICES.RECOMMENDED, # noqa + 'addon__promotedaddon__promoted_group__api_name': RECOMMENDED_API_NAME, # noqa + 'addon___current_version__promoted_versions__promoted_group__api_name': RECOMMENDED_API_NAME, # noqa } ).distinct() return qs diff --git a/src/olympia/hero/migrations/0014_auto_20200717_1120.py b/src/olympia/hero/migrations/0014_auto_20200717_1120.py index a61628742c45..34621c96b3d8 100644 --- a/src/olympia/hero/migrations/0014_auto_20200717_1120.py +++ b/src/olympia/hero/migrations/0014_auto_20200717_1120.py @@ -4,7 +4,7 @@ import django.db.models.deletion from olympia.amo.decorators import use_primary_db -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.promoted.migrations import PROMOTED_GROUP_CHOICES @use_primary_db def add_promoted_for_each_recommended(apps, schema_editor): diff --git a/src/olympia/promoted/management/commands/promote_by_firefox_themes.py b/src/olympia/promoted/management/commands/promote_by_firefox_themes.py index 686c208dd03b..1b0f7fa28339 100644 --- a/src/olympia/promoted/management/commands/promote_by_firefox_themes.py +++ b/src/olympia/promoted/management/commands/promote_by_firefox_themes.py @@ -4,7 +4,6 @@ from olympia import amo from olympia.addons.models import Addon from olympia.amo.decorators import use_primary_db -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES from olympia.promoted.models import PromotedAddon, PromotedGroup from olympia.users.models import UserProfile @@ -22,7 +21,7 @@ def handle(self, *args, **options): ) for addon in addons: self.stdout.write(f'Promoting {addon.slug}') - group = PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.LINE) + group = PromotedGroup.objects.get(name='By Firefox') PromotedAddon.objects.get_or_create( addon=addon, application_id=amo.FIREFOX.id, diff --git a/src/olympia/promoted/migrations/0006_auto_20200819_0951.py b/src/olympia/promoted/migrations/0006_auto_20200819_0951.py index 2593ba9fb9e5..835809cdc954 100644 --- a/src/olympia/promoted/migrations/0006_auto_20200819_0951.py +++ b/src/olympia/promoted/migrations/0006_auto_20200819_0951.py @@ -4,7 +4,7 @@ from olympia import amo from olympia.addons.tasks import index_addons -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.promoted.migrations import PROMOTED_GROUP_CHOICES @amo.decorators.use_primary_db diff --git a/src/olympia/promoted/migrations/0021_auto_20240919_0952.py b/src/olympia/promoted/migrations/0021_auto_20240919_0952.py index 4f2feef66e2a..fe44b1529a5e 100644 --- a/src/olympia/promoted/migrations/0021_auto_20240919_0952.py +++ b/src/olympia/promoted/migrations/0021_auto_20240919_0952.py @@ -1,6 +1,6 @@ # Generated by Django 4.2.16 on 2024-09-19 09:52 -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.promoted.migrations import PROMOTED_GROUP_CHOICES from django.db import migrations diff --git a/src/olympia/promoted/migrations/0023_promotedgroup_alter_promotedaddon_group_id_and_more.py b/src/olympia/promoted/migrations/0023_promotedgroup_alter_promotedaddon_group_id_and_more.py index 0f9c60127869..5fb9a3f49fd0 100644 --- a/src/olympia/promoted/migrations/0023_promotedgroup_alter_promotedaddon_group_id_and_more.py +++ b/src/olympia/promoted/migrations/0023_promotedgroup_alter_promotedaddon_group_id_and_more.py @@ -6,7 +6,7 @@ import django.utils.timezone import olympia.amo.models from olympia.constants import applications -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.promoted.migrations import PROMOTED_GROUP_CHOICES _PromotedSuperClass = namedtuple( '_PromotedSuperClass', diff --git a/src/olympia/promoted/migrations/0026_auto_20250401_1656.py b/src/olympia/promoted/migrations/0026_auto_20250401_1656.py index 040142c2650a..b9eacebb23b8 100644 --- a/src/olympia/promoted/migrations/0026_auto_20250401_1656.py +++ b/src/olympia/promoted/migrations/0026_auto_20250401_1656.py @@ -2,7 +2,7 @@ from django.db import migrations, models -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.promoted.migrations import PROMOTED_GROUP_CHOICES def create_partner_promoted_group(apps, schema_editor): PromotedGroup = apps.get_model('promoted', 'PromotedGroup') diff --git a/src/olympia/promoted/migrations/0028_auto_20250425_1948.py b/src/olympia/promoted/migrations/0028_auto_20250425_1948.py index 208d36699600..4ed3613b015f 100644 --- a/src/olympia/promoted/migrations/0028_auto_20250425_1948.py +++ b/src/olympia/promoted/migrations/0028_auto_20250425_1948.py @@ -3,9 +3,9 @@ from django.db import migrations from olympia.addons.tasks import index_addons -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.promoted.migrations import PROMOTED_GROUP_CHOICES -PROMOTED_GROUPS_TO_UPDATE = [PROMOTED_GROUP_CHOICES.STRATEGIC, +PROMOTED_GROUPS_TO_UPDATE = [PROMOTED_GROUP_CHOICES.STRATEGIC, PROMOTED_GROUP_CHOICES.NOTABLE] def update_public_promoted_groups(apps, schema_editor): diff --git a/src/olympia/promoted/migrations/0029_auto_20250429_1315.py b/src/olympia/promoted/migrations/0029_auto_20250429_1315.py index 5f483a6e3c1f..080baae476e3 100644 --- a/src/olympia/promoted/migrations/0029_auto_20250429_1315.py +++ b/src/olympia/promoted/migrations/0029_auto_20250429_1315.py @@ -2,11 +2,11 @@ from django.db import migrations -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.promoted.migrations import PROMOTED_GROUP_CHOICES def set_partner_group_high_profile_rating(apps, schema_editor): PromotedGroup = apps.get_model('promoted', 'PromotedGroup') - try: + try: group = PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.PARTNER) group.high_profile_rating = True group.save() diff --git a/src/olympia/promoted/migrations/0031_auto_20250526_1200.py b/src/olympia/promoted/migrations/0031_auto_20250526_1200.py index 63c763c9a977..7f8815da59c0 100644 --- a/src/olympia/promoted/migrations/0031_auto_20250526_1200.py +++ b/src/olympia/promoted/migrations/0031_auto_20250526_1200.py @@ -2,7 +2,7 @@ from django.db import migrations -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.promoted.migrations import PROMOTED_GROUP_CHOICES def set_partner_group_pre_review(apps, schema_editor): PromotedGroup = apps.get_model('promoted', 'PromotedGroup') diff --git a/src/olympia/promoted/migrations/__init__.py b/src/olympia/promoted/migrations/__init__.py index e69de29bb2d1..68028dd82109 100644 --- a/src/olympia/promoted/migrations/__init__.py +++ b/src/olympia/promoted/migrations/__init__.py @@ -0,0 +1,12 @@ +from olympia.amo.enum import EnumChoices + + +class PROMOTED_GROUP_CHOICES(EnumChoices): + RECOMMENDED = 1, 'Recommended' + LINE = 4, 'By Firefox' + SPOTLIGHT = 5, 'Spotlight' + STRATEGIC = 6, 'Strategic' + NOTABLE = 7, 'Notable' + SPONSORED = 8, 'Sponsored' + VERIFIED = 9, 'Verified' + PARTNER = 10, 'Partner' diff --git a/src/olympia/promoted/models.py b/src/olympia/promoted/models.py index 2110be42ca77..696429db836f 100644 --- a/src/olympia/promoted/models.py +++ b/src/olympia/promoted/models.py @@ -1,4 +1,3 @@ -from django.core.exceptions import ValidationError from django.db import models from django.dispatch import receiver @@ -6,7 +5,6 @@ from olympia.addons.models import Addon from olympia.amo.models import BaseQuerySet, ModelBase from olympia.constants.applications import APP_IDS, APPS_CHOICES -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES from olympia.reviewers.models import NeedsHumanReview from olympia.versions.models import Version @@ -41,9 +39,9 @@ def approved_for(self, addon): if not addon.current_version: return self.none() approved_promotions = addon.approved_promotions().values_list( - 'promoted_group__group_id', flat=True + 'promoted_group_pk', flat=True ) - return self.all_for(addon=addon).filter(group_id__in=approved_promotions) + return self.all_for(addon=addon).filter(id__in=approved_promotions) def active(self): return self.get_queryset().active() @@ -54,10 +52,6 @@ class PromotedGroup(models.Model): NOTE: This model replaces the legacy PromotedClass and its constants """ - group_id = models.SmallIntegerField( - help_text='The legacy ID from back when promoted groups were static classes', - choices=PROMOTED_GROUP_CHOICES.choices, - ) name = models.CharField( max_length=255, help_text='Human-readable name for the promotion group.' ) @@ -125,18 +119,6 @@ class PromotedGroup(models.Model): ) objects = PromotedGroupManager() - def save(self, *args, **kwargs): - # Obsolete, never used in production, only there to prevent us from re-using - # the ids. Both these classes used to have specific properties set that were - # removed since they are not supposed to be used anyway. - if ( - self.group_id in PROMOTED_GROUP_CHOICES.values - and self.group_id not in PROMOTED_GROUP_CHOICES.ACTIVE.values - and not self.pk - ): - raise ValidationError(f'Legacy ID {self.group_id} is not allowed') - super().save(*args, **kwargs) - def __str__(self): return self.name diff --git a/src/olympia/promoted/tasks.py b/src/olympia/promoted/tasks.py index ff2115fde718..6e27550a784c 100644 --- a/src/olympia/promoted/tasks.py +++ b/src/olympia/promoted/tasks.py @@ -6,9 +6,7 @@ from olympia.addons.serializers import PromotedGroup from olympia.amo.celery import task from olympia.amo.decorators import use_primary_db -from olympia.constants.promoted import ( - PROMOTED_GROUP_CHOICES, -) +from olympia.constants.promoted import NOTABLE_API_NAME from olympia.reviewers.models import UsageTier from olympia.versions.utils import get_staggered_review_due_date_generator @@ -17,8 +15,6 @@ log = olympia.core.logger.getLogger('z.promoted.tasks') -NOTABLE_TIER_SLUG = 'notable' - @task @use_primary_db @@ -26,13 +22,14 @@ def add_high_adu_extensions_to_notable(): """Add add-ons with high ADU to Notable promoted group.""" try: lower_adu_threshold = UsageTier.objects.get( - slug=NOTABLE_TIER_SLUG + slug=NOTABLE_API_NAME ).lower_adu_threshold except UsageTier.DoesNotExist: lower_adu_threshold = None if not lower_adu_threshold: return + notable_group = PromotedGroup.objects.get(api_name=NOTABLE_API_NAME) due_date_generator = get_staggered_review_due_date_generator() addons_ids_and_slugs = Addon.unfiltered.filter( ~Q(status=amo.STATUS_DISABLED), @@ -44,7 +41,7 @@ def add_high_adu_extensions_to_notable(): log.info( 'Starting adding %s addons to %s', count, - PROMOTED_GROUP_CHOICES.NOTABLE.api_value, + notable_group.api_value, ) for addon_id, addon_slug, adu in addons_ids_and_slugs: due_date = next(due_date_generator) @@ -58,17 +55,18 @@ def add_high_adu_extensions_to_notable(): 'With addon id[%s], attempt to overwrite %s with %s. Skipping', addon_id, [promo.group.name for promo in promotions], - PROMOTED_GROUP_CHOICES.NOTABLE.api_value, + notable_group.api_value, ) else: raise PromotedAddon.DoesNotExist except PromotedAddon.DoesNotExist: # Can reconstruct addon.all_applications() directly from APP_USAGE, # since there's no existing promotions. - notable = PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.NOTABLE) for app in amo.APP_USAGE: promo = PromotedAddon( - addon_id=addon_id, promoted_group=notable, application_id=app.id + addon_id=addon_id, + promoted_group=notable_group, + application_id=app.id, ) promo.save(_due_date=due_date) @@ -78,8 +76,6 @@ def add_high_adu_extensions_to_notable(): addon_id, addon_slug, adu, - PROMOTED_GROUP_CHOICES.NOTABLE.api_value, + notable_group.api_value, ) - log.info( - 'Done adding %s addons to %s', count, PROMOTED_GROUP_CHOICES.NOTABLE.api_value - ) + log.info('Done adding %s addons to %s', count, notable_group.api_value) diff --git a/src/olympia/reviewers/cron.py b/src/olympia/reviewers/cron.py index 6aaed450ab50..b5a01881a8a5 100644 --- a/src/olympia/reviewers/cron.py +++ b/src/olympia/reviewers/cron.py @@ -1,8 +1,6 @@ from datetime import date -from olympia.constants.promoted import ( - PROMOTED_GROUP_CHOICES, -) +from olympia.promoted.models import PromotedGroup from olympia.reviewers.models import QueueCount from olympia.reviewers.utils import PendingManualApprovalQueueTable from olympia.reviewers.views import reviewer_tables_registry @@ -17,10 +15,10 @@ def record_reviewer_queues_counts(): } # Also drill down manual review queue by promoted class (there is no real # queue for each, but we still want that data). - for group in PROMOTED_GROUP_CHOICES.ACTIVE: + for group in PromotedGroup.objects.active(): querysets[f'{PendingManualApprovalQueueTable.name}/{group.api_value}'] = ( PendingManualApprovalQueueTable.get_queryset(None).filter( - promotedaddon__promoted_group__group_id=group.value + promotedaddon__promoted_group=group ) ) diff --git a/src/olympia/search/filters.py b/src/olympia/search/filters.py index bae73bd46db8..51c28182a1a6 100644 --- a/src/olympia/search/filters.py +++ b/src/olympia/search/filters.py @@ -15,11 +15,7 @@ from olympia import amo from olympia.api.utils import is_gate_active from olympia.constants.categories import CATEGORIES, CATEGORIES_BY_ID -from olympia.constants.promoted import ( - BADGED_API_NAME, - PROMOTED_API_NAME_TO_IDS, - PROMOTED_GROUP_CHOICES, -) +from olympia.constants.promoted import BADGED_API_NAME from olympia.versions.compare import version_int @@ -406,8 +402,6 @@ class AddonFeaturedQueryParam(AddonQueryParam): class AddonPromotedQueryParam(AddonQueryMultiParam): query_param = 'promoted' - 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) @@ -435,7 +429,7 @@ def get_app(self): ) def get_es_query(self): - query = [Q(self.operator, **{'promoted.group_id': self.get_values()})] + query = [Q(self.operator, **{'promoted.category': self.get_values()})] if app := self.get_app(): query.append(Q('term', **{'promoted.approved_for_apps': app})) @@ -911,30 +905,8 @@ def apply_search_query(self, search_query, qs, sort=None): ), } ), + query.SF('field_value_factor', field='ranking_bump'), ] - ranking_bump_groups = amo.utils.sorted_groupby( - PROMOTED_GROUP_CHOICES.ACTIVE, - lambda g: getattr(g, 'search_ranking_bump', 0.0), - reverse=True, - ) - for bump, promo_groups in ranking_bump_groups: - if not bump: - continue - functions.append( - query.SF( - { - 'weight': bump, - 'filter': ( - Q( - 'terms', - **{ - 'promoted.group_id': [p.value for p in promo_groups] - }, - ) - ), - } - ) - ) # Assemble everything together qs = qs.query( diff --git a/src/olympia/versions/models.py b/src/olympia/versions/models.py index 7ff54cdf8ee6..9567f0113f6d 100644 --- a/src/olympia/versions/models.py +++ b/src/olympia/versions/models.py @@ -1082,7 +1082,7 @@ def can_be_disabled_and_deleted(self): .distinct()[:1] ) previous_approval = PromotedApproval.objects.filter( - promoted_group__group_id__in=promotions.group_id, + promoted_group__in=promotions, version__in=previous_version, ) return previous_approval.exists() From da36a5f7d762d3a37fdd6293f6362dfb167030d2 Mon Sep 17 00:00:00 2001 From: Andrew Williamson Date: Mon, 22 Jun 2026 12:11:37 +0100 Subject: [PATCH 03/14] replace use of PROMOTED_CHOICES in tests WIP TODO: filters.py changes are lacking - need way to replicate badged --- src/olympia/abuse/tests/test_actions.py | 35 +++++++++++------------ src/olympia/abuse/tests/test_cinder.py | 7 ++--- src/olympia/abuse/tests/test_models.py | 12 ++++---- src/olympia/abuse/tests/test_utils.py | 7 +---- src/olympia/addons/tests/test_indexers.py | 21 ++++++-------- src/olympia/amo/tests/__init__.py | 24 +++++++--------- src/olympia/landfill/serializers.py | 35 ++++++++++++++++------- 7 files changed, 72 insertions(+), 69 deletions(-) diff --git a/src/olympia/abuse/tests/test_actions.py b/src/olympia/abuse/tests/test_actions.py index d7676502a948..511113bf3f37 100644 --- a/src/olympia/abuse/tests/test_actions.py +++ b/src/olympia/abuse/tests/test_actions.py @@ -38,11 +38,9 @@ from olympia.constants.abuse import DECISION_ACTIONS from olympia.constants.blocklist import BlockReason, BlockType from olympia.constants.permissions import ADDONS_HIGH_IMPACT_APPROVE -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES from olympia.constants.reviewers import REVIEWER_DELAYED_REJECTION_PERIOD_DAYS_DEFAULT from olympia.core import set_user from olympia.files.models import File -from olympia.promoted.models import PromotedGroup from olympia.ratings.models import Rating from olympia.reviewers.models import AutoApprovalSummary, NeedsHumanReview from olympia.versions.models import VersionReviewerFlags @@ -585,7 +583,7 @@ def test_should_hold_action(self): assert action_helper.should_hold_action() is False addon = addon_factory(users=[self.user]) assert action_helper.should_hold_action() is False - self.make_addon_promoted(addon, PROMOTED_GROUP_CHOICES.RECOMMENDED) + self.make_addon_promoted(addon, api_name='recommended', high_profile=True) assert action_helper.should_hold_action() is True self.user.banned = datetime.now() @@ -867,7 +865,7 @@ def test_should_hold_action(self): action_helper = self.ActionClass(self.decision) assert action_helper.should_hold_action() is False - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED) + self.make_addon_promoted(self.addon, api_name='recommended', high_profile=True) assert action_helper.should_hold_action() is True self.addon.status = amo.STATUS_DISABLED @@ -1473,7 +1471,7 @@ def test_execute_action_with_stakeholder_email(self): ) self.version.file.update(is_signed=True) self.another_version.file.update(approval_date=datetime(2025, 2, 3)) - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED) + self.make_addon_promoted(self.addon, api_name='recommended', high_profile=True) self._test_reject_version(content_review=False, expected_emails_from_action=1) assert len(mail.outbox) == 4 assert mail.outbox[0].recipients() == [stakeholder.email] @@ -1629,7 +1627,7 @@ def test_execute_action_delayed_with_stakeholder_email(self): ) self.version.file.update(is_signed=True) self.another_version.file.update(approval_date=datetime(2025, 2, 3)) - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED) + self.make_addon_promoted(self.addon, api_name='recommended', high_profile=True) self._test_reject_version_delayed( content_review=False, expected_emails_from_action=1 ) @@ -1743,7 +1741,7 @@ def test_should_hold_action(self): action_helper = self.ActionClass(self.decision) assert action_helper.should_hold_action() is False - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED) + self.make_addon_promoted(self.addon, api_name='recommended', high_profile=True) self.decision.target_versions.add(self.another_version) assert self.decision.target_versions.filter(file__is_signed=True).exists() assert action_helper.should_hold_action() is True @@ -1755,7 +1753,7 @@ def test_should_hold_action(self): def test_should_hold_action_some_versions_remain(self): self.decision.update(action=DECISION_ACTIONS.AMO_REJECT_VERSION_ADDON) - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED) + self.make_addon_promoted(self.addon, api_name='recommended', high_profile=True) self.version.file.update(is_signed=True) # While there are more public listed versions that wouldn't be affected @@ -1830,7 +1828,7 @@ def test_notify_stakeholders(self): assert len(mail.outbox) == 0 # make the addon promoted - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED) + self.make_addon_promoted(self.addon, api_name='recommended', high_profile=True) action_helper.notify_stakeholders('teh reason') assert len(mail.outbox) == 1 body = mail.outbox[0].body @@ -1912,7 +1910,7 @@ def test_notify_stakeholders_with_private_notes(self): self.decision.update(private_notes='These are the private notes.') # make the addon promoted - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED) + self.make_addon_promoted(self.addon, api_name='recommended', high_profile=True) action_helper.notify_stakeholders('teh reason') assert len(mail.outbox) == 1 assert mail.outbox[0].recipients() == [stakeholder.email] @@ -1943,7 +1941,7 @@ def test_notify_stakeholders_with_policy_texts(self): ) # make the addon promoted - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED) + self.make_addon_promoted(self.addon, api_name='recommended', high_profile=True) action_helper.notify_stakeholders('teh reason') assert len(mail.outbox) == 1 assert mail.outbox[0].recipients() == [stakeholder.email] @@ -2080,14 +2078,11 @@ def test_already_blocked(self): assert ActivityLog.objects.count() == 0 def test_should_hold_action(self): - PromotedGroup.objects.get_or_create( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, high_profile=True - ) self.decision.update(action=self.default_decision_action) action_helper = self.ActionClass(self.decision) assert action_helper.should_hold_action() is False - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED) + self.make_addon_promoted(self.addon, api_name='recommended', high_profile=True) assert action_helper.should_hold_action() is True # if one version is not blocked we still hold the action @@ -2949,15 +2944,15 @@ def test_execute_action_unlisted(self): assert 'has been approved' in mail.outbox[0].body def test_execute_action_promoted(self): - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED) + self.make_addon_promoted(self.addon, api_name='recommended', high_profile=True) assert not self.addon.promoted_groups() self.test_execute_action() assert self.addon.promoted_groups() assert self.version.promoted_versions.filter( - promoted_group__group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + promoted_group__api_name='recommended' ).exists() assert self.old_version.promoted_versions.filter( - promoted_group__group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + promoted_group__api_name='recommended' ).exists() def test_execute_action_not_human(self): @@ -3567,7 +3562,9 @@ def test_should_hold_action(self): AddonUser.objects.create(addon=self.rating.addon, user=self.rating.user) assert action_helper.should_hold_action() is False - self.make_addon_promoted(self.rating.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED) + self.make_addon_promoted( + self.rating.addon, api_name='recommended', high_profile=True + ) assert action_helper.should_hold_action() is False self.rating.update( reply_to=Rating.objects.create( diff --git a/src/olympia/abuse/tests/test_cinder.py b/src/olympia/abuse/tests/test_cinder.py index d15a1e0700e3..ef145f4e1fbf 100644 --- a/src/olympia/abuse/tests/test_cinder.py +++ b/src/olympia/abuse/tests/test_cinder.py @@ -30,7 +30,6 @@ ILLEGAL_CATEGORIES, ILLEGAL_SUBCATEGORIES, ) -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES from olympia.promoted.models import PromotedAddon from olympia.ratings.models import Rating from olympia.reviewers.models import NeedsHumanReview @@ -389,7 +388,7 @@ def test_build_report_payload_promoted_recommended(self): privacy_policy='Söme privacy policy', version_kw={'release_notes': 'Søme release notes'}, ) - self.make_addon_promoted(addon, group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED) + self.make_addon_promoted(addon, api_name='recommended', high_profile=True) message = ' bad addon!' cinder_addon = self.CinderClass(addon) encoded_message = cinder_addon.get_str(message) @@ -457,7 +456,7 @@ def test_build_report_payload_promoted_notable(self): privacy_policy='Söme privacy policy', version_kw={'release_notes': 'Søme release notes'}, ) - self.make_addon_promoted(addon, group_id=PROMOTED_GROUP_CHOICES.NOTABLE) + self.make_addon_promoted(addon, api_name='notable', high_profile=True) message = ' bad addon!' cinder_addon = self.CinderClass(addon) encoded_message = cinder_addon.get_str(message) @@ -1802,7 +1801,7 @@ def test_build_event_payload_promoted_notable(self): version_kw={'release_notes': 'Søme release notes'}, requires_payment=True, ) - self.make_addon_promoted(addon, group_id=PROMOTED_GROUP_CHOICES.NOTABLE) + self.make_addon_promoted(addon, api_name='notable', high_profile=True) cinder_addon = self.CinderClass(addon) data = cinder_addon.build_event_payload() diff --git a/src/olympia/abuse/tests/test_models.py b/src/olympia/abuse/tests/test_models.py index 4f74c1e4058b..99f5d58897f0 100644 --- a/src/olympia/abuse/tests/test_models.py +++ b/src/olympia/abuse/tests/test_models.py @@ -35,7 +35,6 @@ ILLEGAL_SUBCATEGORIES, ) from olympia.constants.permissions import ADDONS_HIGH_IMPACT_APPROVE -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES from olympia.core import set_user from olympia.files.models import File from olympia.ratings.models import Rating @@ -3415,7 +3414,7 @@ def test_execute_action_disable_addon_held(self): self.grant_permission(user_factory(), ':'.join(ADDONS_HIGH_IMPACT_APPROVE)) addon = addon_factory(users=[user_factory()]) self.make_addon_promoted( - addon, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + addon, api_name='recommended', high_profile=True, approve_version=True ) decision = ContentDecision.objects.create( addon=addon, @@ -3547,7 +3546,7 @@ def test_execute_action_reject_version_held(self): addon = addon_factory(users=[user_factory()], file_kw={'is_signed': True}) version = addon.current_version self.make_addon_promoted( - addon, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + addon, api_name='recommended', high_profile=True, approve_version=True ) version.needshumanreview_set.create( reason=NeedsHumanReview.REASONS.ABUSE_ADDON_VIOLATION @@ -3738,7 +3737,7 @@ def test_execute_action_reject_version_delayed_held(self): addon = addon_factory(users=[user_factory()], file_kw={'is_signed': True}) version = addon.current_version self.make_addon_promoted( - addon, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + addon, api_name='recommended', high_profile=True, approve_version=True ) some_time_ago = self.days_ago(13) little_over_fourteen_days = timedelta(days=14, minutes=1) @@ -4020,7 +4019,10 @@ def test_execute_action_delete_rating_held(self): reviewer_user=self.reviewer_user, ) self.make_addon_promoted( - rating.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + rating.addon, + api_name='recommended', + high_profile=True, + approve_version=True, ) assert decision.action_date is None mail.outbox.clear() diff --git a/src/olympia/abuse/tests/test_utils.py b/src/olympia/abuse/tests/test_utils.py index 761a247f5aff..39461687ea5e 100644 --- a/src/olympia/abuse/tests/test_utils.py +++ b/src/olympia/abuse/tests/test_utils.py @@ -16,8 +16,6 @@ from olympia.blocklist.models import Block, BlockVersion from olympia.constants.abuse import DECISION_ACTIONS from olympia.constants.blocklist import BlockReason, BlockType -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES -from olympia.promoted.models import PromotedGroup from olympia.ratings.models import Rating from olympia.users.models import UserProfile @@ -39,11 +37,8 @@ def test_reject_and_block_addons(): user_factory(id=settings.TASK_USER_ID) normal_addon = addon_factory(users=[user_factory()]) normal_addon_version = normal_addon.current_version - PromotedGroup.objects.get_or_create( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, high_profile=True - ) recommended_addon = addon_factory( - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={'api_name': 'recommended', 'high_profile': True}, users=[user_factory()], version_kw={'promotion_approved': False}, ) diff --git a/src/olympia/addons/tests/test_indexers.py b/src/olympia/addons/tests/test_indexers.py index 48e476f7f3f2..b252da37250d 100644 --- a/src/olympia/addons/tests/test_indexers.py +++ b/src/olympia/addons/tests/test_indexers.py @@ -14,7 +14,7 @@ from olympia.bandwagon.models import Collection from olympia.constants.applications import FIREFOX from olympia.constants.licenses import LICENSES_BY_BUILTIN -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.constants.search import SEARCH_LANGUAGE_TO_ANALYZER from olympia.files.models import WebextPermission from olympia.promoted.models import PromotedApproval @@ -528,14 +528,15 @@ def test_extract_promoted(self): assert extracted['is_recommended'] is False # Promoted extension. - self.addon = addon_factory(promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED) + self.addon = addon_factory( + promoted_kwargs={'api_name': 'public-group', 'public': True} + ) promotion_groups = self.addon.publicly_promoted_groups assert len(promotion_groups) == 1 - assert promotion_groups[0].group_id == PROMOTED_GROUP_CHOICES.RECOMMENDED + assert promotion_groups[0].category == 'public-group' extracted = self._extract() assert extracted['promoted'][0] - assert extracted['promoted'][0]['group_id'] == promotion_groups[0].group_id assert extracted['promoted'][0]['category'] == promotion_groups[0].api_name assert extracted['promoted'][0]['approved_for_apps'] == [ amo.FIREFOX.id, @@ -557,19 +558,17 @@ def test_extract_promoted(self): # With multiple promotions self.make_addon_promoted( addon=self.addon, - group_id=PROMOTED_GROUP_CHOICES.LINE, + promoted_kwargs={'api_name': 'other-public', 'public': True}, apps=[amo.FIREFOX], ) self.addon.approve_for_version() del self.addon.publicly_promoted_groups promotion_groups = self.addon.publicly_promoted_groups assert len(promotion_groups) == 2 - assert promotion_groups[0].group_id == PROMOTED_GROUP_CHOICES.RECOMMENDED - assert promotion_groups[1].group_id == PROMOTED_GROUP_CHOICES.LINE + assert promotion_groups[0].category == 'public-group' + assert promotion_groups[1].category == 'other-public' extracted = self._extract() assert extracted['promoted'] - assert extracted['promoted'][0]['group_id'] == promotion_groups[0].group_id - assert extracted['promoted'][1]['group_id'] == promotion_groups[1].group_id assert extracted['promoted'][0]['category'] == promotion_groups[0].api_name assert extracted['promoted'][1]['category'] == promotion_groups[1].api_name @@ -590,9 +589,7 @@ def test_extract_promoted(self): featured_collection.add_addon(self.addon) extracted = self._extract() assert extracted['promoted'][0] - assert ( - extracted['promoted'][0]['group_id'] == PROMOTED_GROUP_CHOICES.RECOMMENDED - ) + assert extracted['promoted'][0]['category'] == RECOMMENDED_API_NAME assert extracted['promoted'][0]['approved_for_apps'] == [ amo.FIREFOX.id, amo.ANDROID.id, diff --git a/src/olympia/amo/tests/__init__.py b/src/olympia/amo/tests/__init__.py index 5b292cfe36d9..dbcec45c00fb 100644 --- a/src/olympia/amo/tests/__init__.py +++ b/src/olympia/amo/tests/__init__.py @@ -601,12 +601,14 @@ def change_channel_for_addon(self, addon, listed): version.update(channel=channel) @classmethod - def make_addon_promoted(cls, addon, group_id, approve_version=False, apps=None): - """ - Promotes the addon for the group in the given apps, or all if none are given. - """ - - promoted_group = PromotedGroup.objects.get(group_id=group_id) + def make_addon_promoted( + cls, addon, *, approve_version=False, apps=None, **promoted_kw + ): + if 'api_name' in promoted_kw and 'name' not in promoted_kw: + promoted_kw['name'] = promoted_kw['api_name'] + elif 'name' in promoted_kw and 'api_name' not in promoted_kw: + promoted_kw['api_name'] = promoted_kw['name'].lower().replace(' ', '_') + promoted_group, _ = PromotedGroup.objects.get_or_create(**promoted_kw) apps_to_create = apps if apps else amo.APP_USAGE promotions = [] @@ -724,7 +726,7 @@ def addon_factory(status=amo.STATUS_APPROVED, version_kw=None, file_kw=None, **k if slug is None: slug = name.replace(' ', '-').lower()[:30] - promoted_group_id = kw.pop('promoted_id', None) + promoted_kwargs = kw.pop('promoted_kwargs', None) reviewer_flags = kw.pop('reviewer_flags', None) kwargs = { @@ -752,12 +754,8 @@ def addon_factory(status=amo.STATUS_APPROVED, version_kw=None, file_kw=None, **k addon = Addon.objects.create(type=type_, **kwargs) # Save 2. - if promoted_group_id: - group = PromotedGroup.objects.get(group_id=promoted_group_id) - for app in amo.APP_USAGE: - PromotedAddon.objects.create( - addon=addon, promoted_group=group, application_id=app.id - ) + if promoted_kwargs: + TestCase.make_addon_promoted(addon, **promoted_kwargs) if 'promotion_approved' not in version_kw: version_kw['promotion_approved'] = True diff --git a/src/olympia/landfill/serializers.py b/src/olympia/landfill/serializers.py index 3060a1034bf9..554ff164363f 100644 --- a/src/olympia/landfill/serializers.py +++ b/src/olympia/landfill/serializers.py @@ -17,7 +17,7 @@ from olympia.amo.tests import addon_factory, copy_file_to_temp, user_factory from olympia.constants.applications import FIREFOX from olympia.constants.base import ADDON_EXTENSION, ADDON_STATICTHEME -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.devhub.utils import create_version_for_upload from olympia.hero.models import PrimaryHero, SecondaryHero from olympia.landfill.collection import generate_collection @@ -38,6 +38,21 @@ def __init__(self): self.fxa_id = None self.fxa_email = 'uitest-%s@restmail.net' % uuid.uuid4() self.user = self._create_addon_user() + self.recommended = PromotedGroup.objects.create( + name='Recommended', + api_name=RECOMMENDED_API_NAME, + search_ranking_bump=5.0, + listed_pre_review=True, + badged=True, + autograph_signing_states={ + 'firefox': 'recommended', + 'android': 'recommended-android', + }, + can_primary_hero=True, + can_be_compatible_with_all_fenix_versions=True, + high_profile=True, + high_profile_rating=True, + ) def _create_addon_user(self): """Create addon user with fxa information assigned.""" @@ -66,7 +81,7 @@ def create_generic_featured_addons(self): for _ in range(5): addon = addon_factory( status=amo.STATUS_APPROVED, - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={'id': self.recommended.id} ) AddonUser.objects.create(user=user_factory(), addon=addon) promoted_group = PromotedGroup.objects.all_for(addon=addon).first() @@ -87,7 +102,7 @@ def create_generic_featured_themes(self): addon = addon_factory( status=amo.STATUS_APPROVED, type=ADDON_STATICTHEME, - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={'id': self.recommended.id} ) generate_version(addon=addon) addon.update_version() @@ -114,7 +129,7 @@ def create_named_addon_with_author(self, name, author=None): users=[UserProfile.objects.get(username=author)], name=f'{name}', slug=f'{name}', - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={'id': self.recommended.id} ) addon.save() else: @@ -124,7 +139,7 @@ def create_named_addon_with_author(self, name, author=None): users=[UserProfile.objects.get(username=author.username)], name=f'{name}', slug=f'{name}', - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={'id': self.recommended.id} ) addon.save() return addon @@ -154,7 +169,7 @@ def create_featured_addon_with_version(self): total_ratings=500, weekly_downloads=9999999, developer_comments='This is a testing addon.', - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={'id': self.recommended.id} ) Preview.objects.create(addon=addon, position=1) Rating.objects.create(addon=addon, rating=5, user=user_factory()) @@ -205,7 +220,7 @@ def create_featured_android_addon(self): total_ratings=500, weekly_downloads=9999999, developer_comments='This is a testing addon for Android.', - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={'id': self.recommended.id} ) Preview.objects.create(addon=addon, position=1) Rating.objects.create(addon=addon, rating=5, user=user_factory()) @@ -243,7 +258,7 @@ def create_featured_addon_with_version_for_install(self): tags=['some_tag', 'another_tag', 'ui-testing', 'selenium', 'python'], weekly_downloads=9999999, developer_comments='This is a testing addon.', - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={'id': self.recommended.id} ) addon.save() generate_collection(addon, app=FIREFOX) @@ -276,7 +291,7 @@ def create_featured_theme(self): total_ratings=777, weekly_downloads=123456, developer_comments='This is a testing theme, used within pytest.', - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={'id': self.recommended.id} ) addon.save() generate_collection(addon, app=FIREFOX) @@ -294,7 +309,7 @@ def create_featured_themes(self): addon = addon_factory( status=amo.STATUS_APPROVED, type=ADDON_STATICTHEME, - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={'id': self.recommended.id} ) generate_collection(addon) From f3814d652470e12a676c0562e9a72213dc055706 Mon Sep 17 00:00:00 2001 From: Andrew Williamson Date: Fri, 3 Jul 2026 11:02:04 +0100 Subject: [PATCH 04/14] Fix badged --- src/olympia/constants/promoted.py | 1 + src/olympia/lib/settings_base.py | 3 -- src/olympia/search/filters.py | 20 +++---- src/olympia/search/tests/test_filters.py | 67 +++++------------------- 4 files changed, 23 insertions(+), 68 deletions(-) diff --git a/src/olympia/constants/promoted.py b/src/olympia/constants/promoted.py index c788b09ee6c8..b9d73361b9ca 100644 --- a/src/olympia/constants/promoted.py +++ b/src/olympia/constants/promoted.py @@ -1,3 +1,4 @@ BADGED_API_NAME = 'badged' # Special alias for all badged groups +BADGED_GROUPS = ['recommended', 'line'] # special badged groups RECOMMENDED_API_NAME = 'recommended' # Name of the "original" promoted group NOTABLE_API_NAME = 'notable' # Name of the group we automatically add add-ons to diff --git a/src/olympia/lib/settings_base.py b/src/olympia/lib/settings_base.py index 891833b02813..29dea6e406e6 100644 --- a/src/olympia/lib/settings_base.py +++ b/src/olympia/lib/settings_base.py @@ -1319,7 +1319,6 @@ def read_only_mode(env): 'del-version-license-slug', 'del-preview-position', 'categories-application', - 'promoted-verified-sponsored', 'minimal-profile-has-all-fields-shim', 'promoted-groups-shim', ), @@ -1337,7 +1336,6 @@ def read_only_mode(env): 'del-version-license-slug', 'del-preview-position', 'categories-application', - 'promoted-verified-sponsored', 'block-min-max-versions-shim', 'block-versions-list-shim', 'promoted-groups-shim', @@ -1348,7 +1346,6 @@ def read_only_mode(env): 'ratings-can_reply', 'ratings-score-filter', 'addon-submission-api', - 'promoted-verified-sponsored', 'block-versions-list-shim', ), } diff --git a/src/olympia/search/filters.py b/src/olympia/search/filters.py index 51c28182a1a6..bf6e8d3a5e87 100644 --- a/src/olympia/search/filters.py +++ b/src/olympia/search/filters.py @@ -15,7 +15,7 @@ from olympia import amo from olympia.api.utils import is_gate_active from olympia.constants.categories import CATEGORIES, CATEGORIES_BY_ID -from olympia.constants.promoted import BADGED_API_NAME +from olympia.constants.promoted import BADGED_API_NAME, BADGED_GROUPS from olympia.versions.compare import version_int @@ -409,17 +409,13 @@ def __init__(self, request, query_data=None): self.query_data = query_data def get_values(self): - 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 processed_values for y in x}) + values = super().get_values() + if BADGED_API_NAME in values: + values = [ + *(value for value in values if value != BADGED_API_NAME), + *BADGED_GROUPS + ] + return values def get_app(self): return ( diff --git a/src/olympia/search/tests/test_filters.py b/src/olympia/search/tests/test_filters.py index 716ba541337c..952467e6bf91 100644 --- a/src/olympia/search/tests/test_filters.py +++ b/src/olympia/search/tests/test_filters.py @@ -4,7 +4,6 @@ from django.test import SimpleTestCase 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 @@ -14,7 +13,7 @@ from olympia import amo from olympia.constants.categories import CATEGORIES -from olympia.constants.promoted import PROMOTED_API_NAME_TO_IDS, PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import BADGED_GROUPS from olympia.search.filters import ( AddonCreatedQueryParam, AddonRatingQueryParam, @@ -190,10 +189,7 @@ def _test_q(self, qs, query): assert functions[2] == { 'filter': { 'terms': { - 'promoted.group_id': [ - PROMOTED_GROUP_CHOICES.RECOMMENDED, - PROMOTED_GROUP_CHOICES.LINE, - ] + 'promoted.category': BADGED_GROUPS } }, 'weight': 5.0, @@ -704,9 +700,7 @@ def test_return_to_amo(self): assert {'terms': {'guid': ['@foobar']}} in filter_ assert { 'terms': { - 'promoted.group_id': [ - group_id for group_id in PROMOTED_GROUP_CHOICES.BADGED.values - ] + 'promoted.category': BADGED_GROUPS } } in filter_ @@ -723,9 +717,7 @@ def test_return_to_amo_for_all_listed(self): assert {'terms': {'guid': ['@foobar']}} in filter_ assert { 'terms': { - 'promoted.group_id': [ - group_id for group_id in PROMOTED_GROUP_CHOICES.BADGED.values - ] + 'promoted.category': BADGED_GROUPS } } not in filter_ @@ -942,19 +934,14 @@ def test_search_by_featured_no_app_no_locale(self): assert context.exception.detail == ['Invalid "featured" parameter.'] def test_search_by_promoted(self): - with self.assertRaises(serializers.ValidationError) as context: - self._filter(data={'promoted': 'foo'}) - assert context.exception.detail == ['Invalid "promoted" parameter.'] - - for api_name, ids in PROMOTED_API_NAME_TO_IDS.items(): - qs = self._filter(data={'promoted': api_name}) - filter_ = qs['query']['bool']['filter'] - assert [{'terms': {'promoted.group_id': ids}}] == filter_ + qs = self._filter(data={'promoted': 'foo'}) + filter_ = qs['query']['bool']['filter'] + assert [{'terms': {'promoted.category': ['foo']}}] == filter_ - qs = self._filter(data={'promoted': api_name, 'app': 'firefox'}) - filter_ = qs['query']['bool']['filter'] - assert {'terms': {'promoted.group_id': ids}} in filter_ - assert {'term': {'promoted.approved_for_apps': amo.FIREFOX.id}} in filter_ + qs = self._filter(data={'promoted': 'foo', 'app': 'firefox'}) + filter_ = qs['query']['bool']['filter'] + assert {'terms': {'promoted.category': ['foo']}} in filter_ + assert {'term': {'promoted.approved_for_apps': amo.FIREFOX.id}} in filter_ # test multiple param values qs = self._filter(data={'promoted': 'recommended,line'}) @@ -962,10 +949,7 @@ def test_search_by_promoted(self): assert [ { 'terms': { - 'promoted.group_id': [ - PROMOTED_GROUP_CHOICES.RECOMMENDED, - PROMOTED_GROUP_CHOICES.LINE, - ] + 'promoted.category': ['recommended', 'line'] } } ] == filter_ @@ -976,37 +960,14 @@ def test_search_by_promoted(self): assert [ { 'terms': { - 'promoted.group_id': [ + 'promoted.category': [ # recommended shouldn't be there twice - PROMOTED_GROUP_CHOICES.RECOMMENDED, - PROMOTED_GROUP_CHOICES.LINE, - PROMOTED_GROUP_CHOICES.STRATEGIC, + 'recommended', 'line', 'strategic' ] } } ] == 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': [PROMOTED_GROUP_CHOICES.LINE]}} - ] == 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 b2c9427617ed930a0ed59cebcb39ffb3b07782c2 Mon Sep 17 00:00:00 2001 From: Andrew Williamson Date: Fri, 3 Jul 2026 11:31:19 +0100 Subject: [PATCH 05/14] more test fixes --- src/olympia/reviewers/tests/test_commands.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/olympia/reviewers/tests/test_commands.py b/src/olympia/reviewers/tests/test_commands.py index 0238fffeafc7..bba1d0176503 100644 --- a/src/olympia/reviewers/tests/test_commands.py +++ b/src/olympia/reviewers/tests/test_commands.py @@ -24,7 +24,6 @@ ) from olympia.amo.utils import days_ago from olympia.constants.abuse import DECISION_ACTIONS -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES from olympia.constants.scanners import DELAY_AUTO_APPROVAL, NARC, YARA from olympia.files.models import FileManifest, FileValidation from olympia.files.utils import lock @@ -129,7 +128,7 @@ def create_candidates(self): recommendable_addon_nominated = addon_factory( name='Recommendable Addon', status=amo.STATUS_NOMINATED, - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={'name': 'some group', 'listed_pre_review': True}, version_kw={ 'due_date': self.days_ago(3), 'created': self.days_ago(6), @@ -139,7 +138,7 @@ def create_candidates(self): recommended_addon = addon_factory( name='Recommended Addon', - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={'name': 'some other group', 'listed_pre_review': True}, version_kw={'promotion_approved': False}, ) recommended_addon_version = version_factory( @@ -914,7 +913,9 @@ def check_assertions(): assert nhr.reason == NeedsHumanReview.REASONS.BELONGS_TO_PROMOTED_GROUP assert nhr.is_active - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.NOTABLE) + self.make_addon_promoted( + self.addon, name='pre review group', listed_pre_review=True + ) call_command('auto_approve') check_assertions() From c869c4a0ad6174c52da708d940b6325b29479b74 Mon Sep 17 00:00:00 2001 From: Andrew Williamson Date: Fri, 3 Jul 2026 11:38:08 +0100 Subject: [PATCH 06/14] make PromotedGroup.api_name unique --- .../0036_alter_promotedgroup_api_name.py | 18 ++++++++++++++++++ src/olympia/promoted/models.py | 4 +++- 2 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 src/olympia/promoted/migrations/0036_alter_promotedgroup_api_name.py diff --git a/src/olympia/promoted/migrations/0036_alter_promotedgroup_api_name.py b/src/olympia/promoted/migrations/0036_alter_promotedgroup_api_name.py new file mode 100644 index 000000000000..956bc172079e --- /dev/null +++ b/src/olympia/promoted/migrations/0036_alter_promotedgroup_api_name.py @@ -0,0 +1,18 @@ +# Generated by Django 5.2.15 on 2026-07-03 10:36 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('promoted', '0035_alter_promotedaddon_promoted_group'), + ] + + operations = [ + migrations.AlterField( + model_name='promotedgroup', + name='api_name', + field=models.CharField(help_text='Programmatic API name for the promotion group.', max_length=100, unique=True), + ), + ] diff --git a/src/olympia/promoted/models.py b/src/olympia/promoted/models.py index 696429db836f..8e02e7c091e1 100644 --- a/src/olympia/promoted/models.py +++ b/src/olympia/promoted/models.py @@ -56,7 +56,9 @@ class PromotedGroup(models.Model): max_length=255, help_text='Human-readable name for the promotion group.' ) api_name = models.CharField( - max_length=100, help_text='Programmatic API name for the promotion group.' + max_length=100, + help_text='Programmatic API name for the promotion group.', + unique=True, ) search_ranking_bump = models.FloatField( help_text=( From 42d0137a4cc471bf4a47a670a580e5ff7ff0975b Mon Sep 17 00:00:00 2001 From: Andrew Williamson Date: Fri, 3 Jul 2026 11:39:28 +0100 Subject: [PATCH 07/14] landfill ruff fix --- src/olympia/landfill/serializers.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/olympia/landfill/serializers.py b/src/olympia/landfill/serializers.py index 554ff164363f..6a1be9acfa09 100644 --- a/src/olympia/landfill/serializers.py +++ b/src/olympia/landfill/serializers.py @@ -80,8 +80,7 @@ def create_generic_featured_addons(self): """ for _ in range(5): addon = addon_factory( - status=amo.STATUS_APPROVED, - promoted_kwargs={'id': self.recommended.id} + status=amo.STATUS_APPROVED, promoted_kwargs={'id': self.recommended.id} ) AddonUser.objects.create(user=user_factory(), addon=addon) promoted_group = PromotedGroup.objects.all_for(addon=addon).first() @@ -102,7 +101,7 @@ def create_generic_featured_themes(self): addon = addon_factory( status=amo.STATUS_APPROVED, type=ADDON_STATICTHEME, - promoted_kwargs={'id': self.recommended.id} + promoted_kwargs={'id': self.recommended.id}, ) generate_version(addon=addon) addon.update_version() @@ -129,7 +128,7 @@ def create_named_addon_with_author(self, name, author=None): users=[UserProfile.objects.get(username=author)], name=f'{name}', slug=f'{name}', - promoted_kwargs={'id': self.recommended.id} + promoted_kwargs={'id': self.recommended.id}, ) addon.save() else: @@ -139,7 +138,7 @@ def create_named_addon_with_author(self, name, author=None): users=[UserProfile.objects.get(username=author.username)], name=f'{name}', slug=f'{name}', - promoted_kwargs={'id': self.recommended.id} + promoted_kwargs={'id': self.recommended.id}, ) addon.save() return addon @@ -169,7 +168,7 @@ def create_featured_addon_with_version(self): total_ratings=500, weekly_downloads=9999999, developer_comments='This is a testing addon.', - promoted_kwargs={'id': self.recommended.id} + promoted_kwargs={'id': self.recommended.id}, ) Preview.objects.create(addon=addon, position=1) Rating.objects.create(addon=addon, rating=5, user=user_factory()) @@ -220,7 +219,7 @@ def create_featured_android_addon(self): total_ratings=500, weekly_downloads=9999999, developer_comments='This is a testing addon for Android.', - promoted_kwargs={'id': self.recommended.id} + promoted_kwargs={'id': self.recommended.id}, ) Preview.objects.create(addon=addon, position=1) Rating.objects.create(addon=addon, rating=5, user=user_factory()) @@ -258,7 +257,7 @@ def create_featured_addon_with_version_for_install(self): tags=['some_tag', 'another_tag', 'ui-testing', 'selenium', 'python'], weekly_downloads=9999999, developer_comments='This is a testing addon.', - promoted_kwargs={'id': self.recommended.id} + promoted_kwargs={'id': self.recommended.id}, ) addon.save() generate_collection(addon, app=FIREFOX) @@ -291,7 +290,7 @@ def create_featured_theme(self): total_ratings=777, weekly_downloads=123456, developer_comments='This is a testing theme, used within pytest.', - promoted_kwargs={'id': self.recommended.id} + promoted_kwargs={'id': self.recommended.id}, ) addon.save() generate_collection(addon, app=FIREFOX) @@ -309,7 +308,7 @@ def create_featured_themes(self): addon = addon_factory( status=amo.STATUS_APPROVED, type=ADDON_STATICTHEME, - promoted_kwargs={'id': self.recommended.id} + promoted_kwargs={'id': self.recommended.id}, ) generate_collection(addon) From 5fa63f02e8b1797f084edd9cb1c9c2c277711d0d Mon Sep 17 00:00:00 2001 From: Andrew Williamson Date: Fri, 3 Jul 2026 11:39:49 +0100 Subject: [PATCH 08/14] filters changes ruff fix --- src/olympia/search/filters.py | 2 +- src/olympia/search/tests/test_filters.py | 30 ++++++------------------ 2 files changed, 8 insertions(+), 24 deletions(-) diff --git a/src/olympia/search/filters.py b/src/olympia/search/filters.py index bf6e8d3a5e87..6a31089f9c4b 100644 --- a/src/olympia/search/filters.py +++ b/src/olympia/search/filters.py @@ -413,7 +413,7 @@ def get_values(self): if BADGED_API_NAME in values: values = [ *(value for value in values if value != BADGED_API_NAME), - *BADGED_GROUPS + *BADGED_GROUPS, ] return values diff --git a/src/olympia/search/tests/test_filters.py b/src/olympia/search/tests/test_filters.py index 952467e6bf91..0eeff6ee585e 100644 --- a/src/olympia/search/tests/test_filters.py +++ b/src/olympia/search/tests/test_filters.py @@ -187,11 +187,7 @@ def _test_q(self, qs, query): 'weight': 4.0, } assert functions[2] == { - 'filter': { - 'terms': { - 'promoted.category': BADGED_GROUPS - } - }, + 'filter': {'terms': {'promoted.category': BADGED_GROUPS}}, 'weight': 5.0, } return qs @@ -698,11 +694,7 @@ def test_return_to_amo(self): assert 'must_not' not in qs['query']['bool'] filter_ = qs['query']['bool']['filter'] assert {'terms': {'guid': ['@foobar']}} in filter_ - assert { - 'terms': { - 'promoted.category': BADGED_GROUPS - } - } in filter_ + assert {'terms': {'promoted.category': BADGED_GROUPS}} in filter_ @patch( 'olympia.search.filters.switch_is_active', @@ -715,11 +707,7 @@ def test_return_to_amo_for_all_listed(self): assert 'must_not' not in qs['query']['bool'] filter_ = qs['query']['bool']['filter'] assert {'terms': {'guid': ['@foobar']}} in filter_ - assert { - 'terms': { - 'promoted.category': BADGED_GROUPS - } - } not in filter_ + assert {'terms': {'promoted.category': BADGED_GROUPS}} not in filter_ def test_search_by_app_invalid(self): with self.assertRaises(serializers.ValidationError) as context: @@ -946,13 +934,7 @@ def test_search_by_promoted(self): # test multiple param values qs = self._filter(data={'promoted': 'recommended,line'}) filter_ = qs['query']['bool']['filter'] - assert [ - { - 'terms': { - 'promoted.category': ['recommended', 'line'] - } - } - ] == filter_ + assert [{'terms': {'promoted.category': ['recommended', 'line']}}] == filter_ # test combining multiple values with the meta "badged" group qs = self._filter(data={'promoted': 'badged,recommended,strategic'}) @@ -962,7 +944,9 @@ def test_search_by_promoted(self): 'terms': { 'promoted.category': [ # recommended shouldn't be there twice - 'recommended', 'line', 'strategic' + 'recommended', + 'line', + 'strategic', ] } } From 8c4396657b31af02958598c95f884c7460d68196 Mon Sep 17 00:00:00 2001 From: Andrew Williamson Date: Fri, 3 Jul 2026 16:01:48 +0100 Subject: [PATCH 09/14] finish fixing tests --- src/olympia/addons/indexers.py | 7 +- src/olympia/addons/tests/test_indexers.py | 15 +- src/olympia/addons/tests/test_models.py | 212 +++++++------- src/olympia/addons/tests/test_serializers.py | 67 +++-- src/olympia/addons/tests/test_views.py | 135 ++++++--- src/olympia/amo/tests/__init__.py | 8 +- src/olympia/amo/tests/test_cron.py | 5 +- src/olympia/amo/tests/test_sitemap.py | 32 ++- src/olympia/devhub/tests/test_forms.py | 8 +- src/olympia/devhub/tests/test_views.py | 27 +- src/olympia/devhub/tests/test_views_submit.py | 24 +- .../devhub/tests/test_views_versions.py | 32 ++- src/olympia/discovery/tests/test_views.py | 8 +- src/olympia/hero/tests/test_models.py | 50 ++-- src/olympia/hero/tests/test_serializers.py | 12 +- src/olympia/landfill/serializers.py | 45 +-- src/olympia/lib/crypto/tests/test_signing.py | 51 +++- .../0037_remove_promotedgroup_group_id.py | 17 ++ src/olympia/promoted/models.py | 2 +- src/olympia/promoted/tasks.py | 8 +- src/olympia/promoted/tests/test_admin.py | 103 ++----- src/olympia/promoted/tests/test_commands.py | 9 +- src/olympia/promoted/tests/test_models.py | 37 +-- src/olympia/promoted/tests/test_tasks.py | 44 ++- src/olympia/reviewers/cron.py | 2 +- src/olympia/reviewers/tests/test_cron.py | 24 +- src/olympia/reviewers/tests/test_models.py | 18 +- src/olympia/reviewers/tests/test_utils.py | 268 ++++-------------- src/olympia/reviewers/tests/test_views.py | 101 ++----- src/olympia/scanners/tests/test_actions.py | 18 +- src/olympia/scanners/tests/test_tasks.py | 6 +- src/olympia/search/filters.py | 13 +- src/olympia/search/tests/test_filters.py | 5 +- .../search/tests/test_search_ranking.py | 25 +- src/olympia/shelves/tests/test_serializers.py | 12 +- src/olympia/shelves/tests/test_views.py | 8 +- src/olympia/versions/tests/test_commands.py | 16 +- src/olympia/versions/tests/test_models.py | 97 ++++--- 38 files changed, 785 insertions(+), 786 deletions(-) create mode 100644 src/olympia/promoted/migrations/0037_remove_promotedgroup_group_id.py diff --git a/src/olympia/addons/indexers.py b/src/olympia/addons/indexers.py index 4edbeedcb37d..3f24c6f9222a 100644 --- a/src/olympia/addons/indexers.py +++ b/src/olympia/addons/indexers.py @@ -693,8 +693,11 @@ def extract_document(cls, obj): ), default=1.0, ) - if max_promoted_ranking_bump: - data['ranking_bump'] = max_promoted_ranking_bump + # ranking_bump is a multiplicative factor in the search ranking, so a + # group without a bump (or no promotion at all) must map to a neutral + # 1.0 rather than 0.0/absent, which would zero the score or make + # field_value_factor fail on the missing field. + data['ranking_bump'] = max_promoted_ranking_bump or 1.0 data['ratings'] = { 'average': obj.average_rating, diff --git a/src/olympia/addons/tests/test_indexers.py b/src/olympia/addons/tests/test_indexers.py index b252da37250d..53d84c878b4d 100644 --- a/src/olympia/addons/tests/test_indexers.py +++ b/src/olympia/addons/tests/test_indexers.py @@ -529,11 +529,15 @@ def test_extract_promoted(self): # Promoted extension. self.addon = addon_factory( - promoted_kwargs={'api_name': 'public-group', 'public': True} + promoted_kwargs={ + 'api_name': RECOMMENDED_API_NAME, + 'is_public': True, + 'search_ranking_bump': 5.0, + } ) promotion_groups = self.addon.publicly_promoted_groups assert len(promotion_groups) == 1 - assert promotion_groups[0].category == 'public-group' + assert promotion_groups[0].api_name == RECOMMENDED_API_NAME extracted = self._extract() assert extracted['promoted'][0] @@ -558,15 +562,16 @@ def test_extract_promoted(self): # With multiple promotions self.make_addon_promoted( addon=self.addon, - promoted_kwargs={'api_name': 'other-public', 'public': True}, + api_name='other-public', + is_public=True, apps=[amo.FIREFOX], ) self.addon.approve_for_version() del self.addon.publicly_promoted_groups promotion_groups = self.addon.publicly_promoted_groups assert len(promotion_groups) == 2 - assert promotion_groups[0].category == 'public-group' - assert promotion_groups[1].category == 'other-public' + assert promotion_groups[0].api_name == RECOMMENDED_API_NAME + assert promotion_groups[1].api_name == 'other-public' extracted = self._extract() assert extracted['promoted'] assert extracted['promoted'][0]['category'] == promotion_groups[0].api_name diff --git a/src/olympia/addons/tests/test_models.py b/src/olympia/addons/tests/test_models.py index 26118f2af3ec..0be7ec01c026 100644 --- a/src/olympia/addons/tests/test_models.py +++ b/src/olympia/addons/tests/test_models.py @@ -45,7 +45,7 @@ from olympia.blocklist.models import Block, BlocklistSubmission, BlockVersion from olympia.constants.blocklist import BlockReason, BlockType from olympia.constants.categories import CATEGORIES -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.devhub.models import RssKey from olympia.files.models import File from olympia.files.tests.test_models import UploadMixin @@ -1763,93 +1763,90 @@ def test_no_promoted_groups(self): def test_unapproved_promoted_groups(self): addon = addon_factory() self.make_addon_promoted( - addon=addon, group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + addon=addon, api_name='recommended', listed_pre_review=True ) - self.make_addon_promoted(addon=addon, group_id=PROMOTED_GROUP_CHOICES.LINE) + self.make_addon_promoted(addon=addon, api_name='line', listed_pre_review=True) assert addon.promoted_groups(currently_approved=False) assert not addon.promoted_groups() def test_approved_promoted_groups(self): addon = addon_factory() self.make_addon_promoted( - addon=addon, group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + addon=addon, api_name='recommended', listed_pre_review=True + ) + self.make_addon_promoted(addon=addon, api_name='line', listed_pre_review=True) + # spotlight group used later to swap a promotion to an unapproved group + spotlight_group, _ = PromotedGroup.objects.get_or_create( + api_name='spotlight', name='spotlight', listed_pre_review=True ) - self.make_addon_promoted(addon=addon, group_id=PROMOTED_GROUP_CHOICES.LINE) addon.approve_for_version() assert addon.promoted_groups() - assert PROMOTED_GROUP_CHOICES.RECOMMENDED in addon.promoted_groups().group_id - assert PROMOTED_GROUP_CHOICES.LINE in addon.promoted_groups().group_id + assert 'recommended' in addon.promoted_groups().api_name + assert 'line' in addon.promoted_groups().api_name # if the group for one approved group changes, its # approval for the current version isn't valid, # but other groups remain valid PromotedAddon.objects.filter( - addon=addon, promoted_group__group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED - ).update( - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.SPOTLIGHT - ) - ) - assert ( - PROMOTED_GROUP_CHOICES.SPOTLIGHT - in addon.promoted_groups(currently_approved=False).group_id - ) - assert ( - PROMOTED_GROUP_CHOICES.LINE - in addon.promoted_groups(currently_approved=False).group_id - ) - assert PROMOTED_GROUP_CHOICES.SPOTLIGHT not in addon.promoted_groups().group_id - assert PROMOTED_GROUP_CHOICES.LINE in addon.promoted_groups().group_id + addon=addon, promoted_group__api_name='recommended' + ).update(promoted_group=spotlight_group) + assert 'spotlight' in addon.promoted_groups(currently_approved=False).api_name + assert 'line' in addon.promoted_groups(currently_approved=False).api_name + assert 'spotlight' not in addon.promoted_groups().api_name + assert 'line' in addon.promoted_groups().api_name def test_unapproved_group_after_approval_promoted_groups(self): addon = addon_factory() self.make_addon_promoted( - addon=addon, group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + addon=addon, api_name='recommended', listed_pre_review=True ) - self.make_addon_promoted(addon=addon, group_id=PROMOTED_GROUP_CHOICES.LINE) + self.make_addon_promoted(addon=addon, api_name='line', listed_pre_review=True) addon.approve_for_version() - self.make_addon_promoted(addon=addon, group_id=PROMOTED_GROUP_CHOICES.NOTABLE) - assert addon.promoted_groups() - assert PROMOTED_GROUP_CHOICES.RECOMMENDED in addon.promoted_groups().group_id - assert PROMOTED_GROUP_CHOICES.LINE in addon.promoted_groups().group_id - assert PROMOTED_GROUP_CHOICES.NOTABLE not in addon.promoted_groups().group_id - assert ( - PROMOTED_GROUP_CHOICES.NOTABLE - in addon.promoted_groups(currently_approved=False).group_id + self.make_addon_promoted( + addon=addon, api_name='notable', listed_pre_review=True ) + assert addon.promoted_groups() + assert 'recommended' in addon.promoted_groups().api_name + assert 'line' in addon.promoted_groups().api_name + assert 'notable' not in addon.promoted_groups().api_name + assert 'notable' in addon.promoted_groups(currently_approved=False).api_name # Unless approved. addon.approve_for_version() - assert PROMOTED_GROUP_CHOICES.RECOMMENDED in addon.promoted_groups().group_id - assert PROMOTED_GROUP_CHOICES.LINE in addon.promoted_groups().group_id - assert PROMOTED_GROUP_CHOICES.NOTABLE in addon.promoted_groups().group_id + assert 'recommended' in addon.promoted_groups().api_name + assert 'line' in addon.promoted_groups().api_name + assert 'notable' in addon.promoted_groups().api_name def test_application_specific_multiple_promoted_groups(self): addon = addon_factory() self.make_addon_promoted( addon=addon, - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + api_name='recommended', + listed_pre_review=True, approve_version=True, ) self.make_addon_promoted( - addon=addon, group_id=PROMOTED_GROUP_CHOICES.LINE, approve_version=True + addon=addon, + api_name='line', + listed_pre_review=True, + approve_version=True, ) # Application specific group membership should still work - assert PROMOTED_GROUP_CHOICES.RECOMMENDED in addon.promoted_groups().group_id - assert PROMOTED_GROUP_CHOICES.LINE in addon.promoted_groups().group_id + assert 'recommended' in addon.promoted_groups().api_name + assert 'line' in addon.promoted_groups().api_name # update to android only PromotedAddon.objects.filter( addon=addon, - promoted_group__group_id=PROMOTED_GROUP_CHOICES.LINE, + promoted_group__api_name='line', application_id=amo.FIREFOX.id, ).delete() - assert PROMOTED_GROUP_CHOICES.RECOMMENDED in addon.promoted_groups().group_id - assert PROMOTED_GROUP_CHOICES.LINE in addon.promoted_groups().group_id + assert 'recommended' in addon.promoted_groups().api_name + assert 'line' in addon.promoted_groups().api_name # but if there's no approval for Android it's not promoted addon.current_version.promoted_versions.filter( application_id=amo.ANDROID.id ).delete() - assert PROMOTED_GROUP_CHOICES.RECOMMENDED in addon.promoted_groups().group_id - assert PROMOTED_GROUP_CHOICES.LINE not in addon.promoted_groups().group_id + assert 'recommended' in addon.promoted_groups().api_name + assert 'line' not in addon.promoted_groups().api_name def test_no_promoted(self): addon = addon_factory() @@ -1859,7 +1856,7 @@ def test_no_promoted(self): def test_promoted_groups_doesnt_error_with_no_version(self): addon = addon_factory() self.make_addon_promoted( - addon=addon, group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + addon=addon, api_name='recommended', listed_pre_review=True ) addon.current_version.file.update(status=amo.STATUS_DISABLED) addon.update_version() @@ -1870,84 +1867,79 @@ def test_promoted_groups_doesnt_error_with_no_version(self): def test_unapproved_promoted(self): addon = addon_factory() self.make_addon_promoted( - addon=addon, group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + addon=addon, api_name='recommended', listed_pre_review=True ) - self.make_addon_promoted(addon=addon, group_id=PROMOTED_GROUP_CHOICES.LINE) + self.make_addon_promoted(addon=addon, api_name='line', listed_pre_review=True) assert addon.publicly_promoted_groups == [] def test_approved_promoted(self): addon = addon_factory() self.make_addon_promoted( - addon=addon, group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + addon=addon, api_name='recommended', listed_pre_review=True + ) + self.make_addon_promoted(addon=addon, api_name='line', listed_pre_review=True) + # spotlight group used later to swap a promotion to an unapproved group + spotlight_group, _ = PromotedGroup.objects.get_or_create( + api_name='spotlight', name='spotlight', listed_pre_review=True ) - self.make_addon_promoted(addon=addon, group_id=PROMOTED_GROUP_CHOICES.LINE) addon.approve_for_version(addon.current_version) assert ( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED) + PromotedGroup.objects.get(api_name='recommended') in addon.publicly_promoted_groups ) assert ( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.LINE) - in addon.publicly_promoted_groups + PromotedGroup.objects.get(api_name='line') in addon.publicly_promoted_groups ) # If the group changes the approval for that group # in the current version isn't valid. PromotedAddon.objects.filter( - addon=addon, promoted_group__group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED - ).update( - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.SPOTLIGHT - ) - ) + addon=addon, promoted_group__api_name='recommended' + ).update(promoted_group=spotlight_group) del addon.publicly_promoted_groups + assert spotlight_group not in addon.publicly_promoted_groups assert ( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.SPOTLIGHT) - not in addon.publicly_promoted_groups - ) - assert ( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.LINE) - in addon.publicly_promoted_groups + PromotedGroup.objects.get(api_name='line') in addon.publicly_promoted_groups ) def test_unapproved_group_after_approval_promoted(self): addon = addon_factory() self.make_addon_promoted( - addon=addon, group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + addon=addon, api_name='recommended', listed_pre_review=True ) - self.make_addon_promoted(addon=addon, group_id=PROMOTED_GROUP_CHOICES.LINE) + self.make_addon_promoted(addon=addon, api_name='line', listed_pre_review=True) addon.approve_for_version(addon.current_version) - self.make_addon_promoted(addon=addon, group_id=PROMOTED_GROUP_CHOICES.SPOTLIGHT) + self.make_addon_promoted( + addon=addon, api_name='spotlight', listed_pre_review=True + ) assert ( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.LINE) - in addon.publicly_promoted_groups + PromotedGroup.objects.get(api_name='line') in addon.publicly_promoted_groups ) assert ( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED) + PromotedGroup.objects.get(api_name='recommended') in addon.publicly_promoted_groups ) assert ( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.SPOTLIGHT) + PromotedGroup.objects.get(api_name='spotlight') not in addon.publicly_promoted_groups ) # Approving approves them all addon.approve_for_version(addon.current_version) del addon.publicly_promoted_groups assert ( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.LINE) - in addon.publicly_promoted_groups + PromotedGroup.objects.get(api_name='line') in addon.publicly_promoted_groups ) assert ( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED) + PromotedGroup.objects.get(api_name='recommended') in addon.publicly_promoted_groups ) assert ( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.SPOTLIGHT) + PromotedGroup.objects.get(api_name='spotlight') in addon.publicly_promoted_groups ) def test_promoted_theme(self): - recommended = PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + recommended, _ = PromotedGroup.objects.get_or_create( + api_name=RECOMMENDED_API_NAME, name='Recommended' ) addon = addon_factory(type=amo.ADDON_STATICTHEME) # default case - no group so return None. @@ -2009,7 +2001,12 @@ def test_can_be_compatible_with_all_fenix_versions_property(self): assert not addon.can_be_compatible_with_all_fenix_versions # It's promoted but nothing has been approved. - self.make_addon_promoted(addon=addon, group_id=PROMOTED_GROUP_CHOICES.LINE) + self.make_addon_promoted( + addon=addon, + api_name='line', + listed_pre_review=True, + can_be_compatible_with_all_fenix_versions=True, + ) assert not addon.can_be_compatible_with_all_fenix_versions # The latest version is approved. @@ -2019,81 +2016,89 @@ def test_can_be_compatible_with_all_fenix_versions_property(self): addon.promotedaddon.all().delete() self.make_addon_promoted( - addon=addon, group_id=PROMOTED_GROUP_CHOICES.LINE, apps=[amo.FIREFOX] + addon=addon, + api_name='line', + listed_pre_review=True, + can_be_compatible_with_all_fenix_versions=True, + apps=[amo.FIREFOX], ) assert not addon.can_be_compatible_with_all_fenix_versions self.make_addon_promoted( - addon=addon, group_id=PROMOTED_GROUP_CHOICES.LINE, apps=[amo.ANDROID] + addon=addon, + api_name='line', + listed_pre_review=True, + can_be_compatible_with_all_fenix_versions=True, + apps=[amo.ANDROID], ) assert addon.can_be_compatible_with_all_fenix_versions def test_all_approved_applications_for_group_removal_after_approval(self): addon = addon_factory() - self.make_addon_promoted(addon=addon, group_id=PROMOTED_GROUP_CHOICES.SPOTLIGHT) - self.make_addon_promoted(addon=addon, group_id=PROMOTED_GROUP_CHOICES.LINE) + self.make_addon_promoted( + addon=addon, api_name='spotlight', listed_pre_review=True + ) + self.make_addon_promoted(addon=addon, api_name='line', listed_pre_review=True) assert addon.all_applications_for( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.SPOTLIGHT) + PromotedGroup.objects.get(api_name='spotlight') ) == [amo.FIREFOX, amo.ANDROID] assert ( addon.approved_applications_for( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.SPOTLIGHT) + PromotedGroup.objects.get(api_name='spotlight') ) == [] ) addon.approve_for_version() assert addon.all_applications_for( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.SPOTLIGHT) + PromotedGroup.objects.get(api_name='spotlight') ) == [amo.FIREFOX, amo.ANDROID] assert addon.approved_applications_for( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.SPOTLIGHT) + PromotedGroup.objects.get(api_name='spotlight') ) == [amo.FIREFOX, amo.ANDROID] # If an app is removed, it should no longer be in all_apps nor approved PromotedAddon.objects.filter( - promoted_group__group_id=PROMOTED_GROUP_CHOICES.LINE, + promoted_group__api_name='line', application_id=amo.FIREFOX.id, ).delete() assert addon.all_applications_for( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.LINE) + PromotedGroup.objects.get(api_name='line') ) == [amo.ANDROID] assert addon.approved_applications_for( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.LINE) + PromotedGroup.objects.get(api_name='line') ) == [amo.ANDROID] # Shouldn't affect any other promotion assert addon.all_applications_for( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.SPOTLIGHT) + PromotedGroup.objects.get(api_name='spotlight') ) == [amo.FIREFOX, amo.ANDROID] assert addon.approved_applications_for( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.SPOTLIGHT) + PromotedGroup.objects.get(api_name='spotlight') ) == [amo.FIREFOX, amo.ANDROID] # But the approval still exists assert PromotedApproval.objects.filter( - promoted_group__group_id=PROMOTED_GROUP_CHOICES.LINE, + promoted_group__api_name='line', application_id=amo.FIREFOX.id, ).exists() # And if we add the PromotedAddon back, it should be approved again - self.make_addon_promoted(addon=addon, group_id=PROMOTED_GROUP_CHOICES.LINE) + self.make_addon_promoted(addon=addon, api_name='line', listed_pre_review=True) assert addon.approved_applications_for( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.LINE) + PromotedGroup.objects.get(api_name='line') ) == [amo.FIREFOX, amo.ANDROID] def test_all_approved_applications_for_group_addition_after_approval(self): addon = addon_factory() self.make_addon_promoted( - addon=addon, group_id=PROMOTED_GROUP_CHOICES.LINE, apps=[amo.FIREFOX] + addon=addon, api_name='line', listed_pre_review=True, apps=[amo.FIREFOX] ) assert addon.all_applications_for( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.LINE) + PromotedGroup.objects.get(api_name='line') ) == [amo.FIREFOX] assert ( - addon.approved_applications_for( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.LINE) - ) + addon.approved_applications_for(PromotedGroup.objects.get(api_name='line')) == [] ) @@ -2101,13 +2106,14 @@ def test_all_approved_applications_for_group_addition_after_approval(self): addon.approve_for_version() self.make_addon_promoted( addon=addon, - group_id=PROMOTED_GROUP_CHOICES.LINE, + api_name='line', + listed_pre_review=True, ) assert addon.all_applications_for( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.LINE) + PromotedGroup.objects.get(api_name='line') ) == [amo.FIREFOX, amo.ANDROID] assert addon.approved_applications_for( - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.LINE) + PromotedGroup.objects.get(api_name='line') ) == [amo.FIREFOX] def test_rollbackable_versions_qs_unavailable(self): diff --git a/src/olympia/addons/tests/test_serializers.py b/src/olympia/addons/tests/test_serializers.py index fc8df1947b85..17dbd47cb61d 100644 --- a/src/olympia/addons/tests/test_serializers.py +++ b/src/olympia/addons/tests/test_serializers.py @@ -63,7 +63,7 @@ LICENSES_BY_BUILTIN, ) from olympia.constants.promoted import ( - PROMOTED_GROUP_CHOICES, + RECOMMENDED_API_NAME, ) from olympia.files.models import WebextPermission from olympia.promoted.models import ( @@ -538,7 +538,12 @@ def test_has_policies(self): def test_is_featured(self): # As we've dropped featuring, we're faking it with recommended status - self.addon = addon_factory(promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED) + self.addon = addon_factory( + promoted_kwargs={ + 'api_name': RECOMMENDED_API_NAME, + 'listed_pre_review': True, + } + ) result = self.serialize() assert 'is_featured' not in result @@ -554,11 +559,16 @@ def test_is_featured(self): def test_promoted(self): # With a promoted extension. - self.addon = addon_factory(promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED) + self.addon = addon_factory( + promoted_kwargs={ + 'api_name': RECOMMENDED_API_NAME, + 'listed_pre_review': True, + } + ) result = self.serialize() promoted = result['promoted'][0] - assert promoted['category'] == PROMOTED_GROUP_CHOICES.RECOMMENDED.api_value + assert promoted['category'] == RECOMMENDED_API_NAME assert promoted['apps'] == [app.short for app in amo.APP_USAGE] # With a specific application approved. @@ -569,32 +579,25 @@ def test_promoted(self): assert result['promoted'][0]['apps'] == [amo.FIREFOX.short] # Test multiple promotions. + line_group, _ = PromotedGroup.objects.get_or_create( + api_name='line', name='By Firefox', listed_pre_review=True + ) PromotedAddon.objects.create( addon=self.addon, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.LINE - ), + promoted_group=line_group, application_id=amo.FIREFOX.id, ) self.addon.approve_for_version(self.addon.current_version) result = self.serialize() assert len(result['promoted']) == 2 - assert ( - result['promoted'][0]['category'] - == PROMOTED_GROUP_CHOICES.RECOMMENDED.api_value - ) - assert ( - result['promoted'][1]['category'] == PROMOTED_GROUP_CHOICES.LINE.api_value - ) + assert result['promoted'][0]['category'] == RECOMMENDED_API_NAME + assert result['promoted'][1]['category'] == 'line' # Directly returns first promotion in v3, v4 gates = {self.request.version: ('promoted-groups-shim',)} with override_settings(DRF_API_GATES=gates): result = self.serialize() - assert ( - result['promoted']['category'] - == PROMOTED_GROUP_CHOICES.RECOMMENDED.api_value - ) + assert result['promoted']['category'] == RECOMMENDED_API_NAME # With a recommended theme. PromotedAddon.objects.filter(addon=self.addon).delete() @@ -606,11 +609,15 @@ def test_promoted(self): result = self.serialize() promoted = result['promoted'][0] - assert promoted['category'] == PROMOTED_GROUP_CHOICES.RECOMMENDED.api_value + assert promoted['category'] == RECOMMENDED_API_NAME assert promoted['apps'] == [app.short for app in amo.APP_USAGE] def test_promoted_shim(self): - group = PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED) + group, _ = PromotedGroup.objects.get_or_create( + api_name=RECOMMENDED_API_NAME, + name='Recommended', + listed_pre_review=True, + ) self.addon = addon_factory() PromotedAddon.objects.create( @@ -631,16 +638,22 @@ def test_promoted_shim(self): with override_settings(DRF_API_GATES=gates): result = self.serialize() promoted = result['promoted'] - assert promoted['category'] == PROMOTED_GROUP_CHOICES.RECOMMENDED.api_value + assert promoted['category'] == RECOMMENDED_API_NAME assert promoted['apps'] == [amo.FIREFOX.short] def test_private_promoted_api_hidden(self): - for group in PromotedGroup.objects.filter(is_public=False): - self.addon = addon_factory(promoted_id=group.group_id) - result = self.serialize() - assert not result['promoted'] - # But still promoted - assert group.group_id in self.addon.promoted_groups().group_id + self.addon = addon_factory( + promoted_kwargs={ + 'api_name': 'private-group', + 'name': 'Private group', + 'listed_pre_review': True, + 'is_public': False, + } + ) + result = self.serialize() + assert not result['promoted'] + # But still promoted + assert 'private-group' in self.addon.promoted_groups().api_name def test_translations(self): translated_descriptions = { diff --git a/src/olympia/addons/tests/test_views.py b/src/olympia/addons/tests/test_views.py index fae091b03c21..49cb521d618e 100644 --- a/src/olympia/addons/tests/test_views.py +++ b/src/olympia/addons/tests/test_views.py @@ -48,7 +48,7 @@ from olympia.constants.categories import CATEGORIES, CATEGORIES_BY_ID from olympia.constants.licenses import LICENSE_GPL3 from olympia.constants.promoted import ( - PROMOTED_GROUP_CHOICES, + RECOMMENDED_API_NAME, ) from olympia.files.tests.test_models import UploadMixin from olympia.files.utils import parse_addon, parse_xpi @@ -3530,7 +3530,11 @@ def test_compatibility_forbidden_range_android(self): # Recommended add-ons for Android don't have that restriction. self.make_addon_promoted( - self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + self.addon, + api_name='recommended', + listed_pre_review=True, + can_be_compatible_with_all_fenix_versions=True, + approve_version=True, ) response = self.request( compatibility={ @@ -3554,7 +3558,11 @@ def test_compatibility_forbidden_range_android_only_min_specified(self): # Recommended add-ons for Android don't have that restriction. self.make_addon_promoted( - self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + self.addon, + api_name='recommended', + listed_pre_review=True, + can_be_compatible_with_all_fenix_versions=True, + approve_version=True, ) response = self.request( compatibility={'android': {'min': amo.DEFAULT_WEBEXT_MIN_VERSION_ANDROID}} @@ -4670,7 +4678,12 @@ def test_disable_enabled_version(self): def test_cannot_disable_if_promoted(self): self.make_addon_promoted( - self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + self.addon, + api_name='recommended', + name='Recommended', + listed_pre_review=True, + badged=True, + approve_version=True, ) response = self.client.patch(self.url, data={'is_disabled': True}) @@ -4828,7 +4841,12 @@ def test_author_developer_can_delete(self): def test_cannot_delete_if_promoted(self): self.make_addon_promoted( - self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + self.addon, + api_name='recommended', + name='Recommended', + listed_pre_review=True, + badged=True, + approve_version=True, ) self.client.login_api(self.user) @@ -6156,10 +6174,13 @@ def test_filter_by_featured_no_app_no_lang(self): addon = addon_factory( slug='my-addon', name='Featured Addôn', - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={ + 'api_name': RECOMMENDED_API_NAME, + 'listed_pre_review': True, + }, ) addon_factory(slug='other-addon', name='Other Addôn') - assert PROMOTED_GROUP_CHOICES.RECOMMENDED in addon.promoted_groups().group_id + assert RECOMMENDED_API_NAME in addon.promoted_groups().api_name self.reindex(Addon) data = self.perform_search(self.url, {'featured': 'true'}) @@ -6176,7 +6197,11 @@ def test_filter_by_promoted(self): ) addon = addon_factory( - name='Recomménded Addôn', promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + name='Recomménded Addôn', + promoted_kwargs={ + 'api_name': RECOMMENDED_API_NAME, + 'listed_pre_review': True, + }, ) ApplicationsVersions.objects.get_or_create( application=amo.ANDROID.id, @@ -6184,7 +6209,7 @@ def test_filter_by_promoted(self): min=av_min, max=av_max, ) - assert PROMOTED_GROUP_CHOICES.RECOMMENDED in addon.promoted_groups().group_id + assert RECOMMENDED_API_NAME in addon.promoted_groups().api_name assert list( PromotedAddon.objects.filter(addon=addon).values_list( 'application_id', flat=True @@ -6199,7 +6224,11 @@ def test_filter_by_promoted(self): ] addon2 = addon_factory( - name='Fírefox Addôn', promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + name='Fírefox Addôn', + promoted_kwargs={ + 'api_name': RECOMMENDED_API_NAME, + 'listed_pre_review': True, + }, ) ApplicationsVersions.objects.get_or_create( application=amo.ANDROID.id, @@ -6211,10 +6240,11 @@ def test_filter_by_promoted(self): addon2.promotedaddon.all().delete() self.make_addon_promoted( addon=addon2, - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + api_name=RECOMMENDED_API_NAME, + listed_pre_review=True, apps=[amo.FIREFOX], ) - assert PROMOTED_GROUP_CHOICES.RECOMMENDED in addon2.promoted_groups().group_id + assert RECOMMENDED_API_NAME in addon2.promoted_groups().api_name assert list( PromotedAddon.objects.filter(addon=addon2).values_list( 'application_id', flat=True @@ -6243,18 +6273,17 @@ def test_filter_by_promoted(self): ) self.make_addon_promoted( addon4, - PROMOTED_GROUP_CHOICES.RECOMMENDED, + api_name=RECOMMENDED_API_NAME, + listed_pre_review=True, apps=[amo.FIREFOX], approve_version=True, ) PromotedAddon.objects.create( addon=addon4, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED - ), + promoted_group=PromotedGroup.objects.get(api_name=RECOMMENDED_API_NAME), application_id=amo.ANDROID.id, ) - assert PROMOTED_GROUP_CHOICES.RECOMMENDED in addon4.promoted_groups().group_id + assert RECOMMENDED_API_NAME in addon4.promoted_groups().api_name assert list( PromotedAddon.objects.filter(addon=addon4).values_list( 'application_id', flat=True @@ -6275,18 +6304,17 @@ def test_filter_by_promoted(self): ) self.make_addon_promoted( addon5, - PROMOTED_GROUP_CHOICES.RECOMMENDED, + api_name=RECOMMENDED_API_NAME, + listed_pre_review=True, apps=[amo.ANDROID], approve_version=True, ) PromotedAddon.objects.create( addon=addon5, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED - ), + promoted_group=PromotedGroup.objects.get(api_name=RECOMMENDED_API_NAME), application_id=amo.FIREFOX.id, ) - assert PROMOTED_GROUP_CHOICES.RECOMMENDED in addon5.promoted_groups().group_id + assert RECOMMENDED_API_NAME in addon5.promoted_groups().api_name assert list( PromotedAddon.objects.filter(addon=addon5).values_list( 'application_id', flat=True @@ -6330,14 +6358,13 @@ def test_filter_by_promoted(self): assert {res['id'] for res in data['results']} == {addon.pk, addon5.pk} # test with other other promotions - for group_id, group_name in ( - (PROMOTED_GROUP_CHOICES.LINE, PROMOTED_GROUP_CHOICES.LINE.api_value), - ( - PROMOTED_GROUP_CHOICES.SPOTLIGHT, - PROMOTED_GROUP_CHOICES.SPOTLIGHT.api_value, - ), - ): - self.make_addon_promoted(addon, group_id, approve_version=True) + for group_name in ('line', 'spotlight'): + self.make_addon_promoted( + addon, + api_name=group_name, + listed_pre_review=True, + approve_version=True, + ) self.reindex(Addon) data = self.perform_search( self.url, {'promoted': group_name, 'app': 'firefox'} @@ -6724,7 +6751,11 @@ def test_filter_by_guid_return_to_amo(self): name='My Addôn', guid='random@guid', popularity=999, - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={ + 'api_name': RECOMMENDED_API_NAME, + 'listed_pre_review': True, + 'badged': True, + }, ) addon_factory() self.reindex(Addon) @@ -6976,10 +7007,18 @@ def test_with_recommended_addons(self): # Now made some of the add-ons recommended self.make_addon_promoted( - addon2, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + addon2, + api_name=RECOMMENDED_API_NAME, + listed_pre_review=True, + search_ranking_bump=5.0, + approve_version=True, ) self.make_addon_promoted( - addon4, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + addon4, + api_name=RECOMMENDED_API_NAME, + listed_pre_review=True, + search_ranking_bump=5.0, + approve_version=True, ) self.refresh() @@ -7211,7 +7250,10 @@ def test_promoted(self): not_promoted = addon_factory(name='not promoted') spotlight = addon_factory(name='is promoted') self.make_addon_promoted( - spotlight, PROMOTED_GROUP_CHOICES.SPOTLIGHT, approve_version=True + spotlight, + api_name='spotlight', + listed_pre_review=True, + approve_version=True, ) addon_factory(name='something') @@ -7250,10 +7292,20 @@ def tearDown(self): self.refresh() def test_basic(self): - addon1 = addon_factory(promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED) - addon2 = addon_factory(promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED) - assert PROMOTED_GROUP_CHOICES.RECOMMENDED in addon1.promoted_groups().group_id - assert PROMOTED_GROUP_CHOICES.RECOMMENDED in addon2.promoted_groups().group_id + addon1 = addon_factory( + promoted_kwargs={ + 'api_name': RECOMMENDED_API_NAME, + 'listed_pre_review': True, + } + ) + addon2 = addon_factory( + promoted_kwargs={ + 'api_name': RECOMMENDED_API_NAME, + 'listed_pre_review': True, + } + ) + assert RECOMMENDED_API_NAME in addon1.promoted_groups().api_name + assert RECOMMENDED_API_NAME in addon2.promoted_groups().api_name addon_factory() # not recommended so shouldn't show up self.refresh() @@ -7268,7 +7320,12 @@ def test_basic(self): def test_page_size(self): for _ in range(0, 15): - addon_factory(promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED) + addon_factory( + promoted_kwargs={ + 'api_name': RECOMMENDED_API_NAME, + 'listed_pre_review': True, + } + ) self.refresh() diff --git a/src/olympia/amo/tests/__init__.py b/src/olympia/amo/tests/__init__.py index dbcec45c00fb..37fc6c886a9d 100644 --- a/src/olympia/amo/tests/__init__.py +++ b/src/olympia/amo/tests/__init__.py @@ -608,7 +608,13 @@ def make_addon_promoted( promoted_kw['name'] = promoted_kw['api_name'] elif 'name' in promoted_kw and 'api_name' not in promoted_kw: promoted_kw['api_name'] = promoted_kw['name'].lower().replace(' ', '_') - promoted_group, _ = PromotedGroup.objects.get_or_create(**promoted_kw) + # api_name is unique, so key the lookup on it and only apply the + # remaining fields when actually creating the group. This lets tests + # reuse a well-known group (e.g. the seeded 'recommended'/'line') by + # api_name without colliding on the unique constraint. + promoted_group, _ = PromotedGroup.objects.get_or_create( + api_name=promoted_kw.pop('api_name'), defaults=promoted_kw + ) apps_to_create = apps if apps else amo.APP_USAGE promotions = [] diff --git a/src/olympia/amo/tests/test_cron.py b/src/olympia/amo/tests/test_cron.py index f6cedd544817..c882e9062980 100644 --- a/src/olympia/amo/tests/test_cron.py +++ b/src/olympia/amo/tests/test_cron.py @@ -12,7 +12,7 @@ from olympia.amo.sitemap import get_sitemaps from olympia.amo.tests import TestCase, addon_factory, user_factory, version_factory from olympia.constants.activity import RETENTION_DAYS -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.constants.scanners import YARA from olympia.files.models import FileUpload from olympia.scanners.models import ScannerResult @@ -229,7 +229,8 @@ def setUp(self): addon_factory() TestCase.make_addon_promoted( addon_factory(version_kw={'application': amo.ANDROID.id}), - PROMOTED_GROUP_CHOICES.RECOMMENDED, + api_name=RECOMMENDED_API_NAME, + listed_pre_review=True, approve_version=True, ) assert len(os.listdir(settings.SITEMAP_STORAGE_PATH)) == 0 diff --git a/src/olympia/amo/tests/test_sitemap.py b/src/olympia/amo/tests/test_sitemap.py index 44cf6318176c..cced2c5590a0 100644 --- a/src/olympia/amo/tests/test_sitemap.py +++ b/src/olympia/amo/tests/test_sitemap.py @@ -33,7 +33,7 @@ version_factory, ) from olympia.constants.categories import CATEGORIES -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.ratings.models import Rating from olympia.tags.models import Tag @@ -64,7 +64,10 @@ def setUp(self): version_kw={'application': amo.ANDROID.id} ) # shouldn't show up in expected self.make_addon_promoted( - self.android_addon, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + self.android_addon, + api_name=RECOMMENDED_API_NAME, + listed_pre_review=True, + approve_version=True, ) # addon_c is noindexed currently and shouldn't show up. addon_d = addon_factory(slug='addon-d', created=self.days_ago(1)) @@ -143,7 +146,10 @@ def test_android(self): # make some of the Firefox add-ons be Android compatible version_factory(addon=self.addon_a, application=amo.ANDROID.id) self.make_addon_promoted( - self.addon_a, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + self.addon_a, + api_name=RECOMMENDED_API_NAME, + listed_pre_review=True, + approve_version=True, ) self.addon_a.reload() version_factory(addon=self.addon_b, application=amo.ANDROID.id) @@ -493,14 +499,20 @@ def test_android(self): version_kw={'application': amo.ANDROID.id}, ) self.make_addon_promoted( - extension, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + extension, + api_name=RECOMMENDED_API_NAME, + listed_pre_review=True, + approve_version=True, ) extra_extension_a = addon_factory( users=(user_with_extensions, user_with_both), version_kw={'application': amo.ANDROID.id}, ) self.make_addon_promoted( - extra_extension_a, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + extra_extension_a, + api_name=RECOMMENDED_API_NAME, + listed_pre_review=True, + approve_version=True, ) extra_extension_b = addon_factory( users=(user_with_extensions, user_with_both), @@ -524,7 +536,10 @@ def test_android(self): version_kw={'application': amo.FIREFOX.id}, ) self.make_addon_promoted( - firefox_addon, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + firefox_addon, + api_name=RECOMMENDED_API_NAME, + listed_pre_review=True, + approve_version=True, ) # there would be 3 addons but one of them isn't promoted @@ -545,7 +560,10 @@ def test_android(self): ] self.make_addon_promoted( - extra_extension_b, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + extra_extension_b, + api_name=RECOMMENDED_API_NAME, + listed_pre_review=True, + approve_version=True, ) with override_url_prefix(app_name='android'): assert list(AccountSitemap().items()) == [ diff --git a/src/olympia/devhub/tests/test_forms.py b/src/olympia/devhub/tests/test_forms.py index 1e225b30cfb1..9096fdd99e66 100644 --- a/src/olympia/devhub/tests/test_forms.py +++ b/src/olympia/devhub/tests/test_forms.py @@ -29,7 +29,7 @@ from olympia.amo.utils import rm_local_tmp_dir from olympia.api.models import APIKey, APIKeyConfirmation from olympia.applications.models import AppVersion -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.devhub import forms from olympia.files.models import FileUpload from olympia.tags.models import AddonTag, Tag @@ -481,7 +481,11 @@ def test_fenix_range_not_disabled_for_recommended_android_extensions(self): max=AppVersion.objects.get(application=amo.ANDROID.id, version='*'), ) self.make_addon_promoted( - self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + addon=self.addon, + api_name=RECOMMENDED_API_NAME, + name='Recommended', + can_be_compatible_with_all_fenix_versions=True, + approve_version=True, ) del self.addon.publicly_promoted_groups # Reset property data = None diff --git a/src/olympia/devhub/tests/test_views.py b/src/olympia/devhub/tests/test_views.py index 1b4f22a04803..776a055837da 100644 --- a/src/olympia/devhub/tests/test_views.py +++ b/src/olympia/devhub/tests/test_views.py @@ -41,7 +41,7 @@ from olympia.amo.tests.test_helpers import get_image_path from olympia.api.models import SYMMETRIC_JWT_TYPE, APIKey, APIKeyConfirmation from olympia.applications.models import AppVersion -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.devhub.decorators import dev_required from olympia.devhub.forms import APIKeyForm, SupportForm from olympia.devhub.models import BlogPost, SurveyResponse @@ -336,18 +336,21 @@ def test_disabled_post_admin(self): def test_dev_promoted_status(self): self.make_addon_promoted( - addon=self.addon, group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + addon=self.addon, + api_name=RECOMMENDED_API_NAME, + name='Recommended', + badged=True, ) - self.make_addon_promoted(addon=self.addon, group_id=PROMOTED_GROUP_CHOICES.LINE) self.make_addon_promoted( - addon=self.addon, group_id=PROMOTED_GROUP_CHOICES.SPOTLIGHT + addon=self.addon, api_name='line', name='By Firefox', badged=True ) - self.addon.approve_for_version() - assert ( - PROMOTED_GROUP_CHOICES.RECOMMENDED in self.addon.promoted_groups().group_id + self.make_addon_promoted( + addon=self.addon, api_name='spotlight', name='Spotlight', badged=False ) - assert PROMOTED_GROUP_CHOICES.LINE in self.addon.promoted_groups().group_id - assert PROMOTED_GROUP_CHOICES.SPOTLIGHT in self.addon.promoted_groups().group_id + self.addon.approve_for_version() + assert 'recommended' in self.addon.promoted_groups().api_name + assert 'line' in self.addon.promoted_groups().api_name + assert 'spotlight' in self.addon.promoted_groups().api_name response = self.client.get(self.get_url) assert response.status_code == 200 @@ -570,7 +573,11 @@ def test_my_addons(self): def test_my_addons_recommended(self): self.make_addon_promoted( - self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + addon=self.addon, + api_name=RECOMMENDED_API_NAME, + name='Recommended', + badged=True, + approve_version=True, ) latest_version = self.addon.find_latest_version(amo.CHANNEL_LISTED) latest_file = latest_version.file diff --git a/src/olympia/devhub/tests/test_views_submit.py b/src/olympia/devhub/tests/test_views_submit.py index 3ff6cf201e65..227203389d87 100644 --- a/src/olympia/devhub/tests/test_views_submit.py +++ b/src/olympia/devhub/tests/test_views_submit.py @@ -32,7 +32,7 @@ ) from olympia.constants.categories import CATEGORIES from olympia.constants.licenses import LICENSE_CC_COPYRIGHT -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import NOTABLE_API_NAME, RECOMMENDED_API_NAME from olympia.devhub import views from olympia.files.tests.test_models import UploadMixin from olympia.files.utils import parse_addon @@ -2507,7 +2507,13 @@ def test_submit_notification_warning_pre_review_ignore_if_not_promoted_groups(se assert not doc('.notification-box.warning') def test_submit_notification_warning_pre_review(self): - self.make_addon_promoted(self.addon, group_id=PROMOTED_GROUP_CHOICES.NOTABLE) + self.make_addon_promoted( + addon=self.addon, + api_name=NOTABLE_API_NAME, + name='Notable', + listed_pre_review=True, + unlisted_pre_review=True, + ) config = Config.objects.create( key='submit_notification_warning_pre_review', value='Warning for pre_review and a link.', @@ -2519,7 +2525,13 @@ def test_submit_notification_warning_pre_review(self): assert doc('.notification-box.warning').html().strip() == config.value def test_submit_notification_warning_pre_review_generic_test_already_present(self): - self.make_addon_promoted(self.addon, group_id=PROMOTED_GROUP_CHOICES.NOTABLE) + self.make_addon_promoted( + addon=self.addon, + api_name=NOTABLE_API_NAME, + name='Notable', + listed_pre_review=True, + unlisted_pre_review=True, + ) config = Config.objects.create( key='submit_notification_warning', value='Warning with a link.', @@ -2760,7 +2772,11 @@ def test_android_compatibility_modal(self): assert doc(modal_selector) self.make_addon_promoted( - self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + addon=self.addon, + api_name=RECOMMENDED_API_NAME, + name='Recommended', + can_be_compatible_with_all_fenix_versions=True, + approve_version=True, ) response = self.client.get(url) doc = pq(response.content) diff --git a/src/olympia/devhub/tests/test_views_versions.py b/src/olympia/devhub/tests/test_views_versions.py index 6de9116623c9..e8fa91a1451b 100644 --- a/src/olympia/devhub/tests/test_views_versions.py +++ b/src/olympia/devhub/tests/test_views_versions.py @@ -29,7 +29,7 @@ ) from olympia.applications.models import AppVersion from olympia.blocklist.models import BlockType -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.files.models import File from olympia.reviewers.models import AutoApprovalSummary from olympia.users.models import Group, UserProfile @@ -220,7 +220,12 @@ def test_cant_disable_or_delete_current_version_recommended(self): # If the add-on is recommended you can't disable or delete the current # version. self.make_addon_promoted( - self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + addon=self.addon, + api_name=RECOMMENDED_API_NAME, + name='Recommended', + listed_pre_review=True, + badged=True, + approve_version=True, ) assert self.version == self.addon.current_version self.client.post(self.delete_url, self.delete_data) @@ -244,7 +249,11 @@ def test_can_disable_or_delete_current_ver_if_previous_recommended(self): # If the add-on is recommended you *can* disable or delete the current # version if the previous version is approved for recommendation too. self.make_addon_promoted( - self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + addon=self.addon, + api_name=RECOMMENDED_API_NAME, + name='Recommended', + listed_pre_review=True, + approve_version=True, ) previous_version = self.version self.version = version_factory(addon=self.addon, promotion_approved=True) @@ -273,15 +282,17 @@ def test_can_disable_or_delete_current_ver_if_previous_recommended(self): self.addon.reload() assert self.addon.current_version == previous_version # It's still recommended. - assert ( - PROMOTED_GROUP_CHOICES.RECOMMENDED in self.addon.promoted_groups().group_id - ) + assert 'recommended' in self.addon.promoted_groups().api_name def test_can_still_disable_or_delete_old_version_recommended(self): # If the add-on is recommended, you can still disable or delete older # versions than the current one. self.make_addon_promoted( - self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + addon=self.addon, + api_name=RECOMMENDED_API_NAME, + name='Recommended', + listed_pre_review=True, + approve_version=True, ) version_factory(addon=self.addon, promotion_approved=True) self.addon.reload() @@ -307,7 +318,12 @@ def test_can_still_disable_or_delete_old_version_recommended(self): def test_can_still_disable_or_delete_current_version_unapproved(self): # If the add-on is in recommended group but hasn't got approval yet, # then deleting the current version is fine. - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED) + self.make_addon_promoted( + addon=self.addon, + api_name=RECOMMENDED_API_NAME, + name='Recommended', + listed_pre_review=True, + ) assert self.version == self.addon.current_version self.delete_data['disable_version'] = '' diff --git a/src/olympia/discovery/tests/test_views.py b/src/olympia/discovery/tests/test_views.py index 851bee3ad400..ffe6041dd835 100644 --- a/src/olympia/discovery/tests/test_views.py +++ b/src/olympia/discovery/tests/test_views.py @@ -3,7 +3,7 @@ from olympia import amo from olympia.amo.templatetags.jinja_helpers import absolutify from olympia.amo.tests import TestCase, addon_factory, reverse_ns, user_factory -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.discovery.models import DiscoveryItem @@ -281,12 +281,14 @@ def test_recommended(self): self.make_addon_promoted( addon=self.items[0].addon, - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + api_name=RECOMMENDED_API_NAME, + listed_pre_review=True, approve_version=True, ) self.make_addon_promoted( addon=self.items[2].addon, - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + api_name=RECOMMENDED_API_NAME, + listed_pre_review=True, approve_version=True, ) with self.assertNumQueries(1): diff --git a/src/olympia/hero/tests/test_models.py b/src/olympia/hero/tests/test_models.py index d22ce4f31c77..6e3e88873e80 100644 --- a/src/olympia/hero/tests/test_models.py +++ b/src/olympia/hero/tests/test_models.py @@ -6,7 +6,7 @@ from olympia import amo from olympia.amo.tests import TestCase, addon_factory from olympia.amo.tests.test_helpers import get_uploaded_file -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.hero.models import ( PrimaryHero, PrimaryHeroImage, @@ -43,9 +43,31 @@ def test_gradiant(self): def test_clean_requires_approved_can_primary_hero_group(self): addon = addon_factory() - promoted_group = PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + # The error message below lists every active can_primary_hero group by + # name, so create the three that can be added to a primary shelf. + recommended_group, _ = PromotedGroup.objects.get_or_create( + api_name='recommended', + name='Recommended', + can_primary_hero=True, + active=True, + listed_pre_review=True, ) + PromotedGroup.objects.get_or_create( + api_name='line', + name='By Firefox', + can_primary_hero=True, + active=True, + ) + spotlight_group, _ = PromotedGroup.objects.get_or_create( + api_name='spotlight', + name='Spotlight', + can_primary_hero=True, + active=True, + ) + strategic_group, _ = PromotedGroup.objects.get_or_create( + api_name='strategic', name='Strategic', can_primary_hero=False + ) + promoted_group = recommended_group promotion = PromotedAddon.objects.create( addon=addon, promoted_group=promoted_group, application_id=amo.FIREFOX.id ) @@ -64,19 +86,17 @@ def test_clean_requires_approved_can_primary_hero_group(self): ph.addon.approve_for_version(ph.addon.current_version) ph.reload() ph.enabled = True - assert PROMOTED_GROUP_CHOICES.RECOMMENDED in ph.addon.promoted_groups().group_id + assert 'recommended' in ph.addon.promoted_groups().api_name ph.clean() # it raises if there's an error # change to a different group - promoted_group = PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.STRATEGIC - ) + promoted_group = strategic_group promotion.update(promoted_group=promoted_group) promotion.reload() ph.addon.approve_for_version(ph.addon.current_version) ph.reload() ph.enabled = True - assert PROMOTED_GROUP_CHOICES.STRATEGIC in ph.addon.promoted_groups().group_id + assert 'strategic' in ph.addon.promoted_groups().api_name with self.assertRaises(ValidationError) as context: # STRATEGIC isn't a group that can be added as a primary hero ph.clean() @@ -86,15 +106,13 @@ def test_clean_requires_approved_can_primary_hero_group(self): ] # change to a different group that *can* be added as a primary hero - promoted_group = PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.SPOTLIGHT - ) + promoted_group = spotlight_group promotion.update(promoted_group=promoted_group) promotion.reload() ph.addon.approve_for_version(ph.addon.current_version) ph.reload() ph.enabled = True - assert PROMOTED_GROUP_CHOICES.SPOTLIGHT in ph.addon.promoted_groups().group_id + assert 'spotlight' in ph.addon.promoted_groups().api_name ph.clean() # it raises if there's an error def test_clean_external_requires_homepage(self): @@ -117,8 +135,8 @@ def test_clean_external_requires_homepage(self): def test_clean_gradient_and_image(self): # Currently, gradient is required and image isn't. addon = addon_factory() - recommended_group = PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + recommended_group, _ = PromotedGroup.objects.get_or_create( + api_name=RECOMMENDED_API_NAME, name='Recommended', can_primary_hero=True ) PromotedAddon.objects.create( addon=addon, promoted_group=recommended_group, application_id=amo.FIREFOX.id @@ -145,8 +163,8 @@ def test_clean_gradient_and_image(self): def test_clean_only_enabled(self): addon = addon_factory() - promoted_group = PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + promoted_group, _ = PromotedGroup.objects.get_or_create( + api_name=RECOMMENDED_API_NAME, name='Recommended', can_primary_hero=True ) PromotedAddon.objects.create( addon=addon, promoted_group=promoted_group, application_id=amo.FIREFOX.id diff --git a/src/olympia/hero/tests/test_serializers.py b/src/olympia/hero/tests/test_serializers.py index caa86db1321a..f44cb638354a 100644 --- a/src/olympia/hero/tests/test_serializers.py +++ b/src/olympia/hero/tests/test_serializers.py @@ -5,7 +5,7 @@ from olympia.amo.tests.test_helpers import get_uploaded_file from olympia.amo.urlresolvers import get_outgoing_url from olympia.constants.promoted import ( - PROMOTED_GROUP_CHOICES, + RECOMMENDED_API_NAME, ) from ..models import ( @@ -34,7 +34,13 @@ def setUp(self): ) def test_basic(self): - addon = addon_factory(promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED) + addon = addon_factory( + promoted_kwargs={ + 'api_name': RECOMMENDED_API_NAME, + 'name': 'Recommended', + 'can_primary_hero': True, + } + ) hero = PrimaryHero.objects.create( addon=addon, description='Déscription', @@ -51,7 +57,7 @@ def test_basic(self): assert data['addon']['promoted'] == [ { 'apps': [amo.FIREFOX.short, amo.ANDROID.short], - 'category': PROMOTED_GROUP_CHOICES.RECOMMENDED.api_value, + 'category': RECOMMENDED_API_NAME, } ] diff --git a/src/olympia/landfill/serializers.py b/src/olympia/landfill/serializers.py index 6a1be9acfa09..97be5f08fa42 100644 --- a/src/olympia/landfill/serializers.py +++ b/src/olympia/landfill/serializers.py @@ -38,20 +38,22 @@ def __init__(self): self.fxa_id = None self.fxa_email = 'uitest-%s@restmail.net' % uuid.uuid4() self.user = self._create_addon_user() - self.recommended = PromotedGroup.objects.create( - name='Recommended', + self.recommended, _ = PromotedGroup.objects.get_or_create( api_name=RECOMMENDED_API_NAME, - search_ranking_bump=5.0, - listed_pre_review=True, - badged=True, - autograph_signing_states={ - 'firefox': 'recommended', - 'android': 'recommended-android', + defaults={ + 'name': 'Recommended', + 'search_ranking_bump': 5.0, + 'listed_pre_review': True, + 'badged': True, + 'autograph_signing_states': { + 'firefox': 'recommended', + 'android': 'recommended-android', + }, + 'can_primary_hero': True, + 'can_be_compatible_with_all_fenix_versions': True, + 'high_profile': True, + 'high_profile_rating': True, }, - can_primary_hero=True, - can_be_compatible_with_all_fenix_versions=True, - high_profile=True, - high_profile_rating=True, ) def _create_addon_user(self): @@ -80,7 +82,8 @@ def create_generic_featured_addons(self): """ for _ in range(5): addon = addon_factory( - status=amo.STATUS_APPROVED, promoted_kwargs={'id': self.recommended.id} + status=amo.STATUS_APPROVED, + promoted_kwargs={'api_name': self.recommended.api_name}, ) AddonUser.objects.create(user=user_factory(), addon=addon) promoted_group = PromotedGroup.objects.all_for(addon=addon).first() @@ -101,7 +104,7 @@ def create_generic_featured_themes(self): addon = addon_factory( status=amo.STATUS_APPROVED, type=ADDON_STATICTHEME, - promoted_kwargs={'id': self.recommended.id}, + promoted_kwargs={'api_name': self.recommended.api_name}, ) generate_version(addon=addon) addon.update_version() @@ -128,7 +131,7 @@ def create_named_addon_with_author(self, name, author=None): users=[UserProfile.objects.get(username=author)], name=f'{name}', slug=f'{name}', - promoted_kwargs={'id': self.recommended.id}, + promoted_kwargs={'api_name': self.recommended.api_name}, ) addon.save() else: @@ -138,7 +141,7 @@ def create_named_addon_with_author(self, name, author=None): users=[UserProfile.objects.get(username=author.username)], name=f'{name}', slug=f'{name}', - promoted_kwargs={'id': self.recommended.id}, + promoted_kwargs={'api_name': self.recommended.api_name}, ) addon.save() return addon @@ -168,7 +171,7 @@ def create_featured_addon_with_version(self): total_ratings=500, weekly_downloads=9999999, developer_comments='This is a testing addon.', - promoted_kwargs={'id': self.recommended.id}, + promoted_kwargs={'api_name': self.recommended.api_name}, ) Preview.objects.create(addon=addon, position=1) Rating.objects.create(addon=addon, rating=5, user=user_factory()) @@ -219,7 +222,7 @@ def create_featured_android_addon(self): total_ratings=500, weekly_downloads=9999999, developer_comments='This is a testing addon for Android.', - promoted_kwargs={'id': self.recommended.id}, + promoted_kwargs={'api_name': self.recommended.api_name}, ) Preview.objects.create(addon=addon, position=1) Rating.objects.create(addon=addon, rating=5, user=user_factory()) @@ -257,7 +260,7 @@ def create_featured_addon_with_version_for_install(self): tags=['some_tag', 'another_tag', 'ui-testing', 'selenium', 'python'], weekly_downloads=9999999, developer_comments='This is a testing addon.', - promoted_kwargs={'id': self.recommended.id}, + promoted_kwargs={'api_name': self.recommended.api_name}, ) addon.save() generate_collection(addon, app=FIREFOX) @@ -290,7 +293,7 @@ def create_featured_theme(self): total_ratings=777, weekly_downloads=123456, developer_comments='This is a testing theme, used within pytest.', - promoted_kwargs={'id': self.recommended.id}, + promoted_kwargs={'api_name': self.recommended.api_name}, ) addon.save() generate_collection(addon, app=FIREFOX) @@ -308,7 +311,7 @@ def create_featured_themes(self): addon = addon_factory( status=amo.STATUS_APPROVED, type=ADDON_STATICTHEME, - promoted_kwargs={'id': self.recommended.id}, + promoted_kwargs={'api_name': self.recommended.api_name}, ) generate_collection(addon) diff --git a/src/olympia/lib/crypto/tests/test_signing.py b/src/olympia/lib/crypto/tests/test_signing.py index 14bc41e6d0c0..9c61686559a0 100644 --- a/src/olympia/lib/crypto/tests/test_signing.py +++ b/src/olympia/lib/crypto/tests/test_signing.py @@ -28,7 +28,7 @@ version_factory, ) from olympia.amo.tests.test_helpers import get_addon_file -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.lib.crypto import signing, tasks from olympia.versions.compare import VersionString, version_int @@ -427,7 +427,15 @@ def test_call_signing_promoted(self): # in "pending" and only *after* we approve and sign them they will # become "promoted" for that group. If their promoted group changes # we won't sign further versions as promoted. - self.make_addon_promoted(self.file_.version.addon, PROMOTED_GROUP_CHOICES.LINE) + self.make_addon_promoted( + self.file_.version.addon, + api_name='line', + listed_pre_review=True, + autograph_signing_states={ + amo.FIREFOX.short: 'line', + amo.ANDROID.short: 'line', + }, + ) # it's promoted for all applications, but it's the same state for both # desktop and android so don't include twice. @@ -435,7 +443,13 @@ def test_call_signing_promoted(self): def test_call_signing_promoted_recommended(self): self.make_addon_promoted( - self.file_.version.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED + self.file_.version.addon, + api_name=RECOMMENDED_API_NAME, + listed_pre_review=True, + autograph_signing_states={ + amo.FIREFOX.short: 'recommended', + amo.ANDROID.short: 'recommended-android', + }, ) # Recommended has different states for desktop and android @@ -444,7 +458,12 @@ def test_call_signing_promoted_recommended(self): def test_call_signing_promoted_recommended_android_only(self): self.make_addon_promoted( self.file_.version.addon, - PROMOTED_GROUP_CHOICES.RECOMMENDED, + api_name=RECOMMENDED_API_NAME, + listed_pre_review=True, + autograph_signing_states={ + amo.FIREFOX.short: 'recommended', + amo.ANDROID.short: 'recommended-android', + }, apps=[amo.ANDROID], ) @@ -455,7 +474,13 @@ def test_call_signing_promoted_unlisted(self): # Unlisted versions, even when the add-on is in promoted group, should # never be signed as promoted. self.make_addon_promoted( - self.file_.version.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED + self.file_.version.addon, + api_name=RECOMMENDED_API_NAME, + listed_pre_review=True, + autograph_signing_states={ + amo.FIREFOX.short: 'recommended', + amo.ANDROID.short: 'recommended-android', + }, ) self.version.update(channel=amo.CHANNEL_UNLISTED) @@ -470,9 +495,11 @@ def test_call_signing_promoted_unlisted(self): assert 'Name: mozilla-recommendation.json' not in manifest def test_call_signing_promoted_no_special_autograph_group(self): - # SPOTLIGHT addons aren't signed differently. + # Groups without autograph_signing_states aren't signed differently. self.make_addon_promoted( - self.file_.version.addon, PROMOTED_GROUP_CHOICES.SPOTLIGHT + self.file_.version.addon, + api_name='spotlight', + listed_pre_review=True, ) assert signing.sign_file(self.file_) @@ -659,7 +686,10 @@ def test_resign_bypass_name_checks(self, mock_resolve_message, mock_sign_file): def test_resign_carry_over_promotion(self, mock_sign_file): self.make_addon_promoted( - self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + self.addon, + api_name=RECOMMENDED_API_NAME, + listed_pre_review=True, + approve_version=True, ) assert self.addon.publicly_promoted_groups # Should have an approval for Firefox and one for Android. @@ -674,7 +704,10 @@ def test_resign_carry_over_promotion(self, mock_sign_file): def test_resign_doesnt_carry_over_unapproved_promotion(self, mock_sign_file): self.make_addon_promoted( - self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=False + self.addon, + api_name=RECOMMENDED_API_NAME, + listed_pre_review=True, + approve_version=False, ) assert not self.addon.publicly_promoted_groups assert self.addon.current_version.promoted_versions.count() == 0 diff --git a/src/olympia/promoted/migrations/0037_remove_promotedgroup_group_id.py b/src/olympia/promoted/migrations/0037_remove_promotedgroup_group_id.py new file mode 100644 index 000000000000..17d0249f1e74 --- /dev/null +++ b/src/olympia/promoted/migrations/0037_remove_promotedgroup_group_id.py @@ -0,0 +1,17 @@ +# Generated by Django 5.2.15 on 2026-07-03 11:00 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('promoted', '0036_alter_promotedgroup_api_name'), + ] + + operations = [ + migrations.RemoveField( + model_name='promotedgroup', + name='group_id', + ), + ] diff --git a/src/olympia/promoted/models.py b/src/olympia/promoted/models.py index 8e02e7c091e1..ab93c6a74f90 100644 --- a/src/olympia/promoted/models.py +++ b/src/olympia/promoted/models.py @@ -39,7 +39,7 @@ def approved_for(self, addon): if not addon.current_version: return self.none() approved_promotions = addon.approved_promotions().values_list( - 'promoted_group_pk', flat=True + 'promoted_group_id', flat=True ) return self.all_for(addon=addon).filter(id__in=approved_promotions) diff --git a/src/olympia/promoted/tasks.py b/src/olympia/promoted/tasks.py index 6e27550a784c..9506a121976b 100644 --- a/src/olympia/promoted/tasks.py +++ b/src/olympia/promoted/tasks.py @@ -41,7 +41,7 @@ def add_high_adu_extensions_to_notable(): log.info( 'Starting adding %s addons to %s', count, - notable_group.api_value, + notable_group.api_name, ) for addon_id, addon_slug, adu in addons_ids_and_slugs: due_date = next(due_date_generator) @@ -55,7 +55,7 @@ def add_high_adu_extensions_to_notable(): 'With addon id[%s], attempt to overwrite %s with %s. Skipping', addon_id, [promo.group.name for promo in promotions], - notable_group.api_value, + notable_group.api_name, ) else: raise PromotedAddon.DoesNotExist @@ -76,6 +76,6 @@ def add_high_adu_extensions_to_notable(): addon_id, addon_slug, adu, - notable_group.api_value, + notable_group.api_name, ) - log.info('Done adding %s addons to %s', count, notable_group.api_value) + log.info('Done adding %s addons to %s', count, notable_group.api_name) diff --git a/src/olympia/promoted/tests/test_admin.py b/src/olympia/promoted/tests/test_admin.py index d3d5066e678f..e13d9c77fc39 100644 --- a/src/olympia/promoted/tests/test_admin.py +++ b/src/olympia/promoted/tests/test_admin.py @@ -10,7 +10,6 @@ version_factory, ) from olympia.amo.tests.test_helpers import get_uploaded_file -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES from olympia.hero.models import PrimaryHero, PrimaryHeroImage from olympia.promoted.models import ( PromotedApproval, @@ -160,32 +159,24 @@ def test_list_filtering_by_promoted_group(self): another_absent = addon_factory(name='Another Absent') PromotedAddon.objects.create( addon=another_absent, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.SPOTLIGHT - ), + promoted_group=PromotedGroup.objects.get(api_name='spotlight'), application_id=amo.FIREFOX.id, ) addon = addon_factory(name='Present') PromotedAddon.objects.create( addon=addon, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED - ), + promoted_group=PromotedGroup.objects.get(api_name='recommended'), application_id=amo.FIREFOX.id, ) also_present = addon_factory(name='Also Present') PromotedAddon.objects.create( addon=also_present, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED - ), + promoted_group=PromotedGroup.objects.get(api_name='recommended'), application_id=amo.FIREFOX.id, ) PromotedAddon.objects.create( addon=also_present, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.SPOTLIGHT - ), + promoted_group=PromotedGroup.objects.get(api_name='spotlight'), application_id=amo.FIREFOX.id, ) user = user_factory(email='someone@mozilla.com') @@ -195,7 +186,7 @@ def test_list_filtering_by_promoted_group(self): filters = { # See DiscoveryAddonAdmin.list_filter 'promotedaddon__promoted_group__id__exact': ( - PROMOTED_GROUP_CHOICES.RECOMMENDED.value + PromotedGroup.objects.get(api_name='recommended').pk ) } response = self.client.get(self.list_url, filters, follow=True) @@ -229,9 +220,7 @@ def test_can_edit_with_discovery_edit_permission(self): addon = addon_factory() promotion = PromotedAddon.objects.create( addon=addon, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED - ), + promoted_group=PromotedGroup.objects.get(api_name='recommended'), application_id=amo.FIREFOX.id, ) ver1 = addon.current_version @@ -242,30 +231,22 @@ def test_can_edit_with_discovery_edit_permission(self): approvals = [ PromotedApproval.objects.create( version=ver1, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED - ), + promoted_group=PromotedGroup.objects.get(api_name='recommended'), application_id=amo.FIREFOX.id, ), PromotedApproval.objects.create( version=ver2, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED - ), + promoted_group=PromotedGroup.objects.get(api_name='recommended'), application_id=amo.ANDROID.id, ), PromotedApproval.objects.create( version=ver2, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.LINE - ), + promoted_group=PromotedGroup.objects.get(api_name='line'), application_id=amo.FIREFOX.id, ), PromotedApproval.objects.create( version=ver3, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED - ), + promoted_group=PromotedGroup.objects.get(api_name='recommended'), application_id=amo.FIREFOX.id, ), ] @@ -288,9 +269,7 @@ def test_can_edit_with_discovery_edit_permission(self): self._get_approval_form(approvals), **self._get_promotedgrouppromotionform( promotion=promotion, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.LINE - ), + promoted_group=PromotedGroup.objects.get(api_name='line'), application_id=amo.FIREFOX.id, ), **self._get_heroform(''), @@ -302,11 +281,13 @@ def test_can_edit_with_discovery_edit_permission(self): promotion.reload() assert 'errors' not in response.context_data, response.context_data['errors'] assert PromotedAddon.objects.count() == 1 - assert promotion.promoted_group.group_id == PROMOTED_GROUP_CHOICES.LINE + assert promotion.promoted_group.api_name == 'line' assert PromotedApproval.objects.count() == 4 # same # now it's not promoted because the current_version isn't approved for # LINE group - assert not addon.approved_applications_for(PROMOTED_GROUP_CHOICES.LINE) + assert not addon.approved_applications_for( + PromotedGroup.objects.get(api_name='line') + ) # Try to delete one of the approvals response = self.client.post( @@ -331,17 +312,13 @@ def test_cannot_add_or_change_approval(self): addon = addon_factory() promotion = PromotedAddon.objects.create( addon=addon, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED - ), + promoted_group=PromotedGroup.objects.get(api_name='recommended'), application_id=amo.FIREFOX.id, ) ver1 = addon.current_version approval = PromotedApproval.objects.create( version=ver1, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED - ), + promoted_group=PromotedGroup.objects.get(api_name='recommended'), application_id=amo.FIREFOX.id, ) detail_url = reverse(self.detail_url_name, args=(addon.pk,)) @@ -357,16 +334,14 @@ def test_cannot_add_or_change_approval(self): **self._get_heroform(''), **self._get_promotedgrouppromotionform( promotion=promotion, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.LINE - ), + promoted_group=PromotedGroup.objects.get(api_name='line'), application_id=amo.FIREFOX.id, ), ), follow=True, ) approval.reload() - assert approval.promoted_group.group_id == PROMOTED_GROUP_CHOICES.RECOMMENDED + assert approval.promoted_group.api_name == 'recommended' assert response.status_code == 200 assert PromotedAddon.objects.count() == 1 @@ -378,9 +353,7 @@ def test_cannot_add_or_change_approval(self): **self._get_heroform(''), **{ 'form-1-id': '', - 'form-1-promoted_group': PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.LINE - ), + 'form-1-promoted_group': PromotedGroup.objects.get(api_name='line'), 'form-1-version': str(ver1.id), }, ), @@ -394,18 +367,14 @@ def test_cannot_edit_without_discovery_edit_permission(self): addon = addon_factory() promotion = PromotedAddon.objects.create( addon=addon, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED - ), + promoted_group=PromotedGroup.objects.get(api_name='recommended'), application_id=amo.FIREFOX.id, ) ver1 = addon.current_version approvals = [ PromotedApproval.objects.create( version=ver1, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED - ), + promoted_group=PromotedGroup.objects.get(api_name='recommended'), application_id=amo.FIREFOX.id, ), ] @@ -426,9 +395,7 @@ def test_cannot_edit_without_discovery_edit_permission(self): self._get_approval_form(approvals), **self._get_promotedgrouppromotionform( promotion=promotion, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.LINE - ), + promoted_group=PromotedGroup.objects.get(api_name='line'), application_id=amo.FIREFOX.id, ), ), @@ -438,7 +405,7 @@ def test_cannot_edit_without_discovery_edit_permission(self): promotion.reload() assert PromotedAddon.objects.count() == 1 - assert promotion.promoted_group.group_id == PROMOTED_GROUP_CHOICES.RECOMMENDED + assert promotion.promoted_group.api_name == 'recommended' assert PromotedApproval.objects.count() == 1 # Try to delete the approval instead @@ -460,16 +427,12 @@ def test_can_delete_with_discovery_edit_permission(self): addon = addon_factory() promotion = PromotedAddon.objects.create( addon=addon, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED - ), + promoted_group=PromotedGroup.objects.get(api_name='recommended'), application_id=amo.FIREFOX.id, ) PromotedApproval.objects.create( version=addon.current_version, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED - ), + promoted_group=PromotedGroup.objects.get(api_name='recommended'), application_id=amo.FIREFOX.id, ) detail_url = reverse(self.detail_url_name, args=(addon.pk,)) @@ -499,9 +462,7 @@ def test_cannot_delete_without_discovery_edit_permission(self): addon = addon_factory() promotion = PromotedAddon.objects.create( addon=addon, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED - ), + promoted_group=PromotedGroup.objects.get(api_name='recommended'), application_id=amo.FIREFOX.id, ) detail_url = reverse(self.detail_url_name, args=(addon.pk,)) @@ -531,7 +492,7 @@ def test_can_add_with_discovery_edit_permission(self): self.grant_permission(user, 'Discovery:Edit') self.client.force_login(user) # create an approval that doesn't have a matching PromotedAddon yet - group = PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED) + group = PromotedGroup.objects.get(api_name='recommended') response = self.client.get(detail_url, follow=True) assert response.status_code == 200 assert PromotedAddon.objects.count() == 0 @@ -551,7 +512,7 @@ def test_can_add_with_discovery_edit_permission(self): assert PromotedAddon.objects.count() == 1 item = PromotedAddon.objects.get() assert item.addon == addon - assert item.promoted_group.group_id == PROMOTED_GROUP_CHOICES.RECOMMENDED + assert item.promoted_group.api_name == 'recommended' assert item.application_id == amo.FIREFOX.id assert item.application == amo.FIREFOX assert PromotedApproval.objects.count() == 0 # we didn't create any approvals @@ -564,7 +525,7 @@ def test_can_add_when_existing_approval(self): self.grant_permission(user, 'Discovery:Edit') self.client.force_login(user) # create an approval that doesn't have a matching PromotedAddon yet - group = PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.LINE) + group = PromotedGroup.objects.get(api_name='line') PromotedApproval.objects.create( version=addon.current_version, promoted_group=group, @@ -589,9 +550,7 @@ def test_can_add_when_existing_approval(self): assert 'errors' not in response.context_data assert PromotedApproval.objects.count() == 1 # still one assert PromotedAddon.objects.count() == 1 - assert ( - PROMOTED_GROUP_CHOICES.LINE in addon.promoted_groups().group_id - ) # now approved + assert 'line' in addon.promoted_groups().api_name # now approved def test_cannot_add_without_discovery_edit_permission(self): addon = addon_factory() diff --git a/src/olympia/promoted/tests/test_commands.py b/src/olympia/promoted/tests/test_commands.py index b5d9626287ce..f1e2173feeaa 100644 --- a/src/olympia/promoted/tests/test_commands.py +++ b/src/olympia/promoted/tests/test_commands.py @@ -3,7 +3,6 @@ from olympia import amo from olympia.amo.tests import TestCase, addon_factory, user_factory -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES from olympia.promoted.models import ( PromotedAddon, PromotedApproval, @@ -31,7 +30,11 @@ def test_basic(self): already_promoted = addon_factory( type=amo.ADDON_STATICTHEME, users=[firefox_user], - promoted_id=PROMOTED_GROUP_CHOICES.LINE, + promoted_kwargs={ + 'api_name': 'line', + 'listed_pre_review': True, + 'badged': True, + }, ) expected_affected = addon_factory( @@ -52,5 +55,5 @@ def test_basic(self): ) assert list(addon.promoted_groups()) == [ - PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.LINE) + PromotedGroup.objects.get(api_name='line') ] diff --git a/src/olympia/promoted/tests/test_models.py b/src/olympia/promoted/tests/test_models.py index 57f9f014e635..1e8ab9e88f0e 100644 --- a/src/olympia/promoted/tests/test_models.py +++ b/src/olympia/promoted/tests/test_models.py @@ -1,10 +1,8 @@ -from django.core.exceptions import ValidationError from django.db import IntegrityError, transaction from olympia.addons.models import Addon from olympia.amo.tests import TestCase, addon_factory, version_factory from olympia.constants import applications -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES from olympia.promoted.models import ( PromotedAddon, PromotedApproval, @@ -17,9 +15,7 @@ def setUp(self): self.addon: Addon = addon_factory() self.application_id = applications.FIREFOX.id - self.promoted_group = PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.SPOTLIGHT - ) + self.promoted_group = PromotedGroup.objects.get(api_name='spotlight') self.promotion = PromotedAddon.objects.create( addon=self.addon, promoted_group=self.promoted_group, @@ -46,9 +42,7 @@ def test_approved_promoted_addon_promotion(self): def test_promoted_group_non_pre_reviewed(self): # alternatively, addon has a non-pre-reviewed promoted group - strategic_group = PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.STRATEGIC - ) + strategic_group = PromotedGroup.objects.get(api_name='strategic') self.promotion.promoted_group = strategic_group self.promotion.save() assert strategic_group in PromotedGroup.objects.approved_for(self.addon) @@ -81,16 +75,12 @@ def setUp(self): self.addon = addon_factory() self.promotion1 = PromotedAddon.objects.create( addon=self.addon, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.NOTABLE - ), + promoted_group=PromotedGroup.objects.get(api_name='notable'), application_id=applications.FIREFOX.id, ) self.promotion2 = PromotedAddon.objects.create( addon=self.addon, - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED - ), + promoted_group=PromotedGroup.objects.get(api_name='recommended'), application_id=applications.FIREFOX.id, ) @@ -105,17 +95,6 @@ def test_getattr(self): class TestPromotedGroup(TestCase): - def test_deactived_group_ids_raise(self): - for group in PROMOTED_GROUP_CHOICES: - if group in PROMOTED_GROUP_CHOICES.ACTIVE: - continue - with self.assertRaises(ValidationError): - PromotedGroup.objects.create( - group_id=group.value, - name='Test', - api_name='test', - ) - def test_str_method(self): # Ensure the __str__ method returns the name for pg in PromotedGroup.objects.all(): @@ -125,9 +104,7 @@ def test_str_method(self): class TestPromotedAddon(TestCase): def setUp(self): self.addon: Addon = addon_factory() - self.promoted_group = PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.SPOTLIGHT - ) + self.promoted_group = PromotedGroup.objects.get(api_name='spotlight') self.application_id = applications.FIREFOX.id self.required_fields = { 'addon': self.addon, @@ -205,9 +182,7 @@ def test_duplicate_raises(self): class TestPromotedApproval(TestCase): def setUp(self): self.addon = addon_factory() - self.promoted_group = PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.SPOTLIGHT - ) + self.promoted_group = PromotedGroup.objects.get(api_name='spotlight') self.application_id = applications.FIREFOX.id self.required_fields = { 'promoted_group': self.promoted_group, diff --git a/src/olympia/promoted/tests/test_tasks.py b/src/olympia/promoted/tests/test_tasks.py index 77016b9d90a5..cf2d439bffd4 100644 --- a/src/olympia/promoted/tests/test_tasks.py +++ b/src/olympia/promoted/tests/test_tasks.py @@ -8,13 +8,13 @@ from olympia import amo from olympia.addons.serializers import PromotedGroup from olympia.amo.tests import addon_factory, user_factory, version_factory -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import NOTABLE_API_NAME from olympia.promoted.models import PromotedAddon from olympia.reviewers.models import UsageTier from olympia.versions.utils import get_staggered_review_due_date_generator from olympia.zadmin.models import set_config -from ..tasks import NOTABLE_TIER_SLUG, add_high_adu_extensions_to_notable +from ..tasks import add_high_adu_extensions_to_notable @pytest.mark.django_db @@ -31,17 +31,17 @@ def test_add_high_adu_extensions_to_notable_tier_absent_or_no_threshold(): assert ( not extension_with_high_adu.reload() .promoted_groups(currently_approved=False) - .group_id + .api_name ) - UsageTier.objects.create(slug=NOTABLE_TIER_SLUG, lower_adu_threshold=None) + UsageTier.objects.create(slug=NOTABLE_API_NAME, lower_adu_threshold=None) add_high_adu_extensions_to_notable() assert ( not extension_with_high_adu.reload() .promoted_groups(currently_approved=False) - .group_id + .api_name ) @@ -51,7 +51,7 @@ def test_add_high_adu_extensions_to_notable(): # Arbitrary_lower_adu_threshold lower_adu_threshold = 1234 UsageTier.objects.create( - slug=NOTABLE_TIER_SLUG, lower_adu_threshold=lower_adu_threshold + slug=NOTABLE_API_NAME, lower_adu_threshold=lower_adu_threshold ) # arbitrary target per day target_per_day = 12 @@ -71,9 +71,7 @@ def test_add_high_adu_extensions_to_notable(): ) PromotedAddon.objects.create( addon=already_promoted, - promoted_group=PromotedGroup.objects.create( - group_id=PROMOTED_GROUP_CHOICES.LINE - ), + promoted_group=PromotedGroup.objects.get(api_name='line'), application_id=amo.FIREFOX.id, ) promoted_record_exists = addon_factory( @@ -105,35 +103,31 @@ def test_add_high_adu_extensions_to_notable(): assert ( not extension_with_low_adu.reload() .promoted_groups(currently_approved=False) - .group_id + .api_name ) assert ( - PROMOTED_GROUP_CHOICES.NOTABLE + 'notable' in extension_with_high_adu.reload() .promoted_groups(currently_approved=False) - .group_id - ) - assert not ignored_theme.reload().promoted_groups(currently_approved=False).group_id - assert ( - PROMOTED_GROUP_CHOICES.LINE - in already_promoted.promoted_groups(currently_approved=False).group_id + .api_name ) + assert not ignored_theme.reload().promoted_groups(currently_approved=False).api_name + assert 'line' in already_promoted.promoted_groups(currently_approved=False).api_name promoted_record_exists.reload() assert ( - PROMOTED_GROUP_CHOICES.NOTABLE - in promoted_record_exists.promoted_groups(currently_approved=False).group_id + 'notable' + in promoted_record_exists.promoted_groups(currently_approved=False).api_name ) assert ( - PROMOTED_GROUP_CHOICES.NOTABLE - in unlisted_only_extension.promoted_groups(currently_approved=False).group_id + 'notable' + in unlisted_only_extension.promoted_groups(currently_approved=False).api_name ) assert ( - PROMOTED_GROUP_CHOICES.NOTABLE - in mixed_extension.promoted_groups(currently_approved=False).group_id + 'notable' in mixed_extension.promoted_groups(currently_approved=False).api_name ) assert ( - PROMOTED_GROUP_CHOICES.NOTABLE - in deleted_extension.promoted_groups(currently_approved=False).group_id + 'notable' + in deleted_extension.promoted_groups(currently_approved=False).api_name ) generator = get_staggered_review_due_date_generator(starting=now) diff --git a/src/olympia/reviewers/cron.py b/src/olympia/reviewers/cron.py index b5a01881a8a5..23aaaef82388 100644 --- a/src/olympia/reviewers/cron.py +++ b/src/olympia/reviewers/cron.py @@ -16,7 +16,7 @@ def record_reviewer_queues_counts(): # Also drill down manual review queue by promoted class (there is no real # queue for each, but we still want that data). for group in PromotedGroup.objects.active(): - querysets[f'{PendingManualApprovalQueueTable.name}/{group.api_value}'] = ( + querysets[f'{PendingManualApprovalQueueTable.name}/{group.api_name}'] = ( PendingManualApprovalQueueTable.get_queryset(None).filter( promotedaddon__promoted_group=group ) diff --git a/src/olympia/reviewers/tests/test_cron.py b/src/olympia/reviewers/tests/test_cron.py index a056d7abc4b4..3b4d7bed4160 100644 --- a/src/olympia/reviewers/tests/test_cron.py +++ b/src/olympia/reviewers/tests/test_cron.py @@ -6,7 +6,7 @@ from olympia import amo from olympia.amo.tests import TestCase, addon_factory, user_factory -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.promoted.models import PromotedGroup from olympia.reviewers.cron import record_reviewer_queues_counts from olympia.reviewers.models import NeedsHumanReview, QueueCount from olympia.reviewers.views import reviewer_tables_registry @@ -18,8 +18,8 @@ def setUp(self): def _test_expected_count(self, date): # We are recording every queue, plus drilling down in every promoted group - expected_count = len(reviewer_tables_registry) + len( - PROMOTED_GROUP_CHOICES.ACTIVE + expected_count = ( + len(reviewer_tables_registry) + PromotedGroup.objects.active().count() ) assert QueueCount.objects.filter(date=date).count() == expected_count @@ -48,21 +48,33 @@ def test_basic(self): ) self.addon_recommended_1 = addon_factory( file_kw={'status': amo.STATUS_AWAITING_REVIEW}, - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={ + 'api_name': 'recommended', + 'listed_pre_review': True, + 'active': True, + }, needshumanreview_kw={ 'reason': NeedsHumanReview.REASONS.BELONGS_TO_PROMOTED_GROUP }, ) addon_factory( file_kw={'status': amo.STATUS_AWAITING_REVIEW}, - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={ + 'api_name': 'recommended', + 'listed_pre_review': True, + 'active': True, + }, needshumanreview_kw={ 'reason': NeedsHumanReview.REASONS.BELONGS_TO_PROMOTED_GROUP }, ) addon_factory( file_kw={'status': amo.STATUS_AWAITING_REVIEW}, - promoted_id=PROMOTED_GROUP_CHOICES.NOTABLE, + promoted_kwargs={ + 'api_name': 'notable', + 'listed_pre_review': True, + 'active': True, + }, needshumanreview_kw={ 'reason': NeedsHumanReview.REASONS.BELONGS_TO_PROMOTED_GROUP }, diff --git a/src/olympia/reviewers/tests/test_models.py b/src/olympia/reviewers/tests/test_models.py index 552e50f7db9e..93ca351c84c8 100644 --- a/src/olympia/reviewers/tests/test_models.py +++ b/src/olympia/reviewers/tests/test_models.py @@ -26,7 +26,6 @@ version_factory, ) from olympia.blocklist.models import BlockVersion -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES from olympia.constants.scanners import WEBHOOK, WEBHOOK_DURING_VALIDATION from olympia.files.models import File, FileValidation, WebextPermission from olympia.promoted.models import ( @@ -1175,20 +1174,20 @@ def test_check_is_promoted_prereview(self): assert AutoApprovalSummary.check_is_promoted_prereview(self.version) is False self.make_addon_promoted( - addon=self.addon, group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + addon=self.addon, api_name='recommended', listed_pre_review=True ) assert AutoApprovalSummary.check_is_promoted_prereview(self.version) is True PromotedAddon.objects.filter(addon=self.addon).delete() self.make_addon_promoted( - addon=self.addon, group_id=PROMOTED_GROUP_CHOICES.STRATEGIC - ) # STRATEGIC isn't prereview + addon=self.addon, api_name='strategic' + ) # strategic isn't prereview assert AutoApprovalSummary.check_is_promoted_prereview(self.version) is False PromotedAddon.objects.filter(addon=self.addon).delete() self.make_addon_promoted( - addon=self.addon, group_id=PROMOTED_GROUP_CHOICES.LINE - ) # LINE is though + addon=self.addon, api_name='line', listed_pre_review=True + ) # line is though assert AutoApprovalSummary.check_is_promoted_prereview(self.version) is True self.version.update(channel=amo.CHANNEL_UNLISTED) # not for unlisted though @@ -1196,8 +1195,11 @@ def test_check_is_promoted_prereview(self): PromotedAddon.objects.filter(addon=self.addon).delete() self.make_addon_promoted( - addon=self.addon, group_id=PROMOTED_GROUP_CHOICES.NOTABLE - ) # NOTABLE is + addon=self.addon, + api_name='notable', + listed_pre_review=True, + unlisted_pre_review=True, + ) # notable is assert AutoApprovalSummary.check_is_promoted_prereview(self.version) is True self.version.update(channel=amo.CHANNEL_LISTED) # and for listed too diff --git a/src/olympia/reviewers/tests/test_utils.py b/src/olympia/reviewers/tests/test_utils.py index d24be20610e5..33b2fccc3c20 100644 --- a/src/olympia/reviewers/tests/test_utils.py +++ b/src/olympia/reviewers/tests/test_utils.py @@ -38,7 +38,6 @@ from olympia.amo.utils import send_mail from olympia.blocklist.models import Block, BlocklistSubmission from olympia.constants.abuse import DECISION_ACTIONS -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES from olympia.files.models import File from olympia.lib.crypto.signing import SigningError from olympia.lib.crypto.tests.test_signing import ( @@ -316,11 +315,21 @@ def test_actions_public_post_review(self): == expected ) - # Now make add a recommended promoted addon. The user should lose all - # approve/reject actions (they are no longer considered an - # "appropriate" reviewer for that add-on). - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED) - expected = ['reply', 'comment'] + # Now add a recommended promoted addon. It's a listed pre-review group + # so the individual approve/reject actions aren't offered, but the + # multiple-versions and confirm actions still are for an + # Addons:Review holder. + self.make_addon_promoted( + self.addon, api_name='recommended', listed_pre_review=True, badged=True + ) + expected = [ + 'confirm_auto_approved', + 'reject_multiple_versions', + 'set_needs_human_review_multiple_versions', + 'reply', + 'request_legal_review', + 'comment', + ] assert ( list( self.get_review_actions( @@ -476,181 +485,6 @@ def test_actions_no_version(self): == expected ) - def test_actions_recommended(self): - # Having Addons:Review is not enough to review - # recommended extensions. - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED) - self.grant_permission(self.user, 'Addons:Review') - expected = ['reply', 'comment'] - assert ( - list( - self.get_review_actions( - addon_status=amo.STATUS_APPROVED, file_status=amo.STATUS_APPROVED - ).keys() - ) - == expected - ) - - assert ( - list( - self.get_review_actions( - addon_status=amo.STATUS_NOMINATED, - file_status=amo.STATUS_AWAITING_REVIEW, - ).keys() - ) - == expected - ) - - # Having Addons:RecommendedReview allows you to do it. - self.grant_permission(self.user, 'Addons:RecommendedReview') - expected = [ - 'public', - 'reject', - 'reject_multiple_versions', - 'set_needs_human_review_multiple_versions', - 'reply', - 'request_legal_review', - 'comment', - ] - assert ( - list( - self.get_review_actions( - addon_status=amo.STATUS_APPROVED, - file_status=amo.STATUS_AWAITING_REVIEW, - ).keys() - ) - == expected - ) - - def test_actions_recommended_content_review(self): - # Having Addons:ContentReview is not enough to content review - # recommended extensions. - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED) - self.grant_permission(self.user, 'Addons:ContentReview') - expected = ['reply', 'comment'] - assert ( - list( - self.get_review_actions( - addon_status=amo.STATUS_APPROVED, - file_status=amo.STATUS_APPROVED, - content_review=True, - ).keys() - ) - == expected - ) - - # Having Addons:RecommendedReview allows you to do it (though you'd - # be better off just do a full review). - self.grant_permission(self.user, 'Addons:RecommendedReview') - expected = [ - 'approve_listing_content', - 'reject_multiple_versions', - 'set_needs_human_review_multiple_versions', - 'reply', - 'request_legal_review', - 'comment', - ] - assert ( - list( - self.get_review_actions( - addon_status=amo.STATUS_APPROVED, - file_status=amo.STATUS_APPROVED, - content_review=True, - ).keys() - ) - == expected - ) - - def test_actions_promoted_admin_review_needs_admin_permission(self): - # Having Addons:Review or Addons:RecommendedReview - # is not enough to review promoted addons that are in a group that is - # admin_review=True. - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.LINE) - self.grant_permission(self.user, 'Addons:Review') - expected = ['comment'] - assert ( - list( - self.get_review_actions( - addon_status=amo.STATUS_APPROVED, file_status=amo.STATUS_APPROVED - ).keys() - ) - == expected - ) - assert ( - list( - self.get_review_actions( - addon_status=amo.STATUS_APPROVED, - file_status=amo.STATUS_AWAITING_REVIEW, - ).keys() - ) - == expected - ) - - # only for groups that are admin_review though - self.addon.promotedaddon.all().delete() - self.make_addon_promoted( - self.addon, PROMOTED_GROUP_CHOICES.NOTABLE, approve_version=True - ) - expected = [ - 'public', - 'reject', - 'reject_multiple_versions', - 'set_needs_human_review_multiple_versions', - 'reply', - 'request_legal_review', - 'comment', - ] - assert ( - list( - self.get_review_actions( - addon_status=amo.STATUS_APPROVED, - file_status=amo.STATUS_AWAITING_REVIEW, - ).keys() - ) - == expected - ) - - # change it back to an admin_review group - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.SPOTLIGHT) - - self.grant_permission(self.user, 'Addons:RecommendedReview') - expected = ['comment'] - assert ( - list( - self.get_review_actions( - addon_status=amo.STATUS_APPROVED, - file_status=amo.STATUS_AWAITING_REVIEW, - ).keys() - ) - == expected - ) - - # You need admin review permission. Also because it's a promoted add-on - # despite being admin you don't get the enable/disable auto-approval - # action. - self.grant_permission(self.user, 'Reviews:Admin') - expected = [ - 'public', - 'reject', - 'reject_multiple_versions', - 'change_or_clear_pending_rejection_multiple_versions', - 'clear_needs_human_review_multiple_versions', - 'set_needs_human_review_multiple_versions', - 'reply', - 'disable_addon', - 'request_legal_review', - 'comment', - ] - assert ( - list( - self.get_review_actions( - addon_status=amo.STATUS_APPROVED, - file_status=amo.STATUS_AWAITING_REVIEW, - ).keys() - ) - == expected - ) - def test_actions_unlisted(self): # Just regular review permissions don't let you do much on an unlisted # review page. @@ -689,7 +523,9 @@ def test_actions_unlisted(self): ) # unlisted shouldn't be affected by promoted group status either - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.LINE) + self.make_addon_promoted( + self.addon, api_name='line', listed_pre_review=True, admin_review=True + ) assert ( list( self.get_review_actions( @@ -2347,7 +2183,7 @@ def test_confirm_auto_approved_approves_for_promoted(self): self.grant_permission(self.user, 'Addons:Review') self.setup_data(amo.STATUS_APPROVED, file_status=amo.STATUS_APPROVED) self.make_addon_promoted( - addon=self.addon, group_id=PROMOTED_GROUP_CHOICES.NOTABLE + addon=self.addon, api_name='notable', listed_pre_review=True ) self.create_paths() @@ -2359,7 +2195,7 @@ def test_confirm_auto_approved_approves_for_promoted(self): self.helper.handler.confirm_auto_approved() self.addon.reload() - assert PROMOTED_GROUP_CHOICES.NOTABLE in self.addon.promoted_groups().group_id + assert 'notable' in self.addon.promoted_groups().api_name def test_addon_with_version_need_human_review_confirm_auto_approval(self): NeedsHumanReview.objects.create(version=self.addon.current_version) @@ -3636,50 +3472,50 @@ def test_reject_listing_content_review(self): ) def test_nominated_to_approved_recommended(self): - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED) + self.make_addon_promoted( + self.addon, api_name='recommended', listed_pre_review=True + ) assert not self.addon.promoted_groups() self.test_nomination_to_public() assert self.addon.current_version.promoted_versions.filter( - promoted_group__group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + promoted_group__api_name='recommended' ).exists() - assert ( - PROMOTED_GROUP_CHOICES.RECOMMENDED in self.addon.promoted_groups().group_id - ) + assert 'recommended' in self.addon.promoted_groups().api_name def test_nominated_to_approved_other_promoted(self): - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.LINE) + self.make_addon_promoted(self.addon, api_name='line', listed_pre_review=True) assert not self.addon.promoted_groups() self.test_nomination_to_public() assert self.addon.current_version.promoted_versions.filter( - promoted_group__group_id=PROMOTED_GROUP_CHOICES.LINE + promoted_group__api_name='line' ).exists() - assert PROMOTED_GROUP_CHOICES.LINE in self.addon.promoted_groups().group_id + assert 'line' in self.addon.promoted_groups().api_name def test_approved_update_recommended(self): - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED) + self.make_addon_promoted( + self.addon, api_name='recommended', listed_pre_review=True + ) assert not self.addon.promoted_groups() self.test_public_addon_with_version_awaiting_review_to_public() assert self.addon.current_version.promoted_versions.filter( - promoted_group__group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + promoted_group__api_name='recommended' ).exists() - assert ( - PROMOTED_GROUP_CHOICES.RECOMMENDED in self.addon.promoted_groups().group_id - ) + assert 'recommended' in self.addon.promoted_groups().api_name def test_approved_update_other_promoted(self): - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.LINE) + self.make_addon_promoted(self.addon, api_name='line', listed_pre_review=True) assert not self.addon.promoted_groups() self.test_public_addon_with_version_awaiting_review_to_public() assert self.addon.current_version.promoted_versions.filter( - promoted_group__group_id=PROMOTED_GROUP_CHOICES.LINE + promoted_group__api_name='line' ).exists() - assert PROMOTED_GROUP_CHOICES.LINE in self.addon.promoted_groups().group_id + assert 'line' in self.addon.promoted_groups().api_name def test_autoapprove_promoted(self): - group = PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED) - group.listed_pre_review = False - group.save() - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED) + # Non-pre-review group (listed_pre_review defaults to False), so the + # task user is allowed to auto-approve the version. + self.make_addon_promoted(self.addon, api_name='a-non-pre-review-group') + group = PromotedGroup.objects.get(api_name='a-non-pre-review-group') self.user = UserProfile.objects.get(id=settings.TASK_USER_ID) self.test_nomination_to_public() @@ -3693,11 +3529,9 @@ def test_autoapprove_promoted(self): assert group in self.addon.promoted_groups() def test_autoapprove_promoted_notable(self): - # as above with notable - group = PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.NOTABLE) - group.listed_pre_review = False - group.save() - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.NOTABLE) + # as above with another non-pre-review group + self.make_addon_promoted(self.addon, api_name='another-non-pre-review-group') + group = PromotedGroup.objects.get(api_name='another-non-pre-review-group') self.user = UserProfile.objects.get(id=settings.TASK_USER_ID) self.test_nomination_to_public() @@ -5089,7 +4923,15 @@ def test_nomination_to_public(self): def test_nominated_to_public_recommended(self): self.setup_data(amo.STATUS_NOMINATED) - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED) + self.make_addon_promoted( + self.addon, + api_name='recommended', + listed_pre_review=True, + autograph_signing_states={ + 'firefox': 'recommended', + 'android': 'recommended-android', + }, + ) assert not self.addon.promoted_groups() self.helper.handler.approve_latest_version() @@ -5098,11 +4940,9 @@ def test_nominated_to_public_recommended(self): assert self.addon.versions.all()[0].file.status == (amo.STATUS_APPROVED) assert self.addon.current_version.promoted_versions.filter( - promoted_group__group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + promoted_group__api_name='recommended' ).exists() - assert ( - PROMOTED_GROUP_CHOICES.RECOMMENDED in self.addon.promoted_groups().group_id - ) + assert 'recommended' in self.addon.promoted_groups().api_name signature_info, manifest = _get_signature_details(self.file.file.path) diff --git a/src/olympia/reviewers/tests/test_views.py b/src/olympia/reviewers/tests/test_views.py index 2e8f8879002a..a06dfdcc7586 100644 --- a/src/olympia/reviewers/tests/test_views.py +++ b/src/olympia/reviewers/tests/test_views.py @@ -59,7 +59,6 @@ from olympia.blocklist.models import Block, BlocklistSubmission, BlockType, BlockVersion from olympia.blocklist.utils import block_activity_log_save from olympia.constants.abuse import DECISION_ACTIONS -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES from olympia.constants.reviewers import REVIEWER_DELAYED_REJECTION_PERIOD_DAYS_DEFAULT from olympia.constants.scanners import WEBHOOK, YARA from olympia.files.models import File, FileValidation, WebextPermission @@ -641,7 +640,7 @@ def test_super_admin_all_permissions(self): user_factory(pk=settings.TASK_USER_ID) # Recommended extensions version = addon_factory( - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={'api_name': 'recommended', 'listed_pre_review': True}, status=amo.STATUS_NOMINATED, file_kw={'status': amo.STATUS_AWAITING_REVIEW}, ).versions.get() @@ -650,7 +649,10 @@ def test_super_admin_all_permissions(self): ) version = version_factory( addon=addon_factory( - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={ + 'api_name': 'recommended', + 'listed_pre_review': True, + }, version_kw={'promotion_approved': False}, ), promotion_approved=True, @@ -1303,7 +1305,7 @@ def test_flags_promoted(self): file_kw={'status': amo.STATUS_AWAITING_REVIEW}, due_date=datetime.now() + timedelta(hours=24), ) - self.make_addon_promoted(addon, PROMOTED_GROUP_CHOICES.LINE) + self.make_addon_promoted(addon, api_name='line', listed_pre_review=True) r = self.client.get(reverse('reviewers.queue_extension')) @@ -2528,82 +2530,6 @@ def test_dont_need_unlisted_reviewer_for_mixed_channels(self): self.grant_permission(self.reviewer, 'ReviewerTools:ViewUnlisted') assert self.client.head(self.url).status_code == 200 - def test_need_correct_reviewer_for_promoted_addon(self): - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED) - self.file.update(status=amo.STATUS_AWAITING_REVIEW) - response = self.client.get(self.url) - assert response.status_code == 200 - choices = list(dict(response.context['form'].fields['action'].choices).keys()) - expected_choices = ['reply', 'comment'] - assert choices == expected_choices - - doc = pq(response.content) - assert doc('.is_promoted') - for entry in doc('.is_promoted').items(): - assert entry.text() == ( - "This is a Recommended add-on. You don't have permission to review it." - ) - - self.grant_permission(self.reviewer, 'Addons:RecommendedReview') - response = self.client.get(self.url) - assert response.status_code == 200 - choices = list(dict(response.context['form'].fields['action'].choices).keys()) - expected_choices = [ - 'public', - 'reject', - 'reject_multiple_versions', - 'set_needs_human_review_multiple_versions', - 'reply', - 'request_legal_review', - 'comment', - ] - assert choices == expected_choices - - doc = pq(response.content) - assert doc('.is_promoted') - for entry in doc('.is_promoted').items(): - assert entry.text() == ('This is a Recommended add-on.') - - # Change to a different class of promoted addon - self.addon.promotedaddon.all().delete() - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.SPOTLIGHT) - - response = self.client.get(self.url) - assert response.status_code == 200 - choices = list(dict(response.context['form'].fields['action'].choices).keys()) - expected_choices = ['comment'] - assert choices == expected_choices - - doc = pq(response.content) - assert doc('.is_promoted') - for entry in doc('.is_promoted').items(): - assert entry.text() == ( - "This is a Spotlight add-on. You don't have permission to review it." - ) - - self.grant_permission(self.reviewer, 'Reviews:Admin') - response = self.client.get(self.url) - assert response.status_code == 200 - choices = list(dict(response.context['form'].fields['action'].choices).keys()) - expected_choices = [ - 'public', - 'reject', - 'reject_multiple_versions', - 'change_or_clear_pending_rejection_multiple_versions', - 'clear_needs_human_review_multiple_versions', - 'set_needs_human_review_multiple_versions', - 'reply', - 'disable_addon', - 'request_legal_review', - 'comment', - ] - assert choices == expected_choices - - doc = pq(response.content) - assert doc('.is_promoted') - for entry in doc('.is_promoted').items(): - assert entry.text() == ('This is a Spotlight add-on.') - def test_not_recommendable(self): response = self.client.get(self.url) assert response.status_code == 200 @@ -4420,7 +4346,12 @@ def test_approve_recommended_addon(self, mock_sign_file): ) self.version.file.update(status=amo.STATUS_AWAITING_REVIEW) self.addon.update(status=amo.STATUS_NOMINATED) - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED) + self.make_addon_promoted( + self.addon, + api_name='recommended', + listed_pre_review=True, + badged=True, + ) self.grant_permission(self.reviewer, 'Addons:RecommendedReview') response = self.client.post( self.url, @@ -4437,7 +4368,7 @@ def test_approve_recommended_addon(self, mock_sign_file): assert addon.current_version assert addon.current_version.file.status == amo.STATUS_APPROVED assert addon.current_version.promoted_versions.filter( - promoted_group__group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + promoted_group__api_name='recommended' ).exists() assert mock_sign_file.called @@ -4453,7 +4384,9 @@ def test_approve_addon_for_unlisted_pre_review_promoted_groups( ) self.version.file.update(status=amo.STATUS_AWAITING_REVIEW) self.addon.update(status=amo.STATUS_NULL) - self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.NOTABLE) + self.make_addon_promoted( + self.addon, api_name='notable', unlisted_pre_review=True + ) self.make_addon_unlisted(self.addon) self.grant_permission(self.reviewer, 'Addons:Review') self.grant_permission(self.reviewer, 'Addons:ReviewUnlisted') @@ -4472,7 +4405,7 @@ def test_approve_addon_for_unlisted_pre_review_promoted_groups( self.version.file.reload() assert self.version.file.status == amo.STATUS_APPROVED assert self.version.promoted_versions.filter( - promoted_group__group_id=PROMOTED_GROUP_CHOICES.NOTABLE + promoted_group__api_name='notable' ).exists() assert mock_sign_file.called diff --git a/src/olympia/scanners/tests/test_actions.py b/src/olympia/scanners/tests/test_actions.py index 2ea082e197e5..b224af84c63d 100644 --- a/src/olympia/scanners/tests/test_actions.py +++ b/src/olympia/scanners/tests/test_actions.py @@ -21,7 +21,7 @@ ) from olympia.blocklist.models import Block, BlocklistSubmission, BlockType, BlockVersion from olympia.constants.abuse import DECISION_ACTIONS -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.constants.scanners import ( DELAY_AUTO_APPROVAL, DELAY_AUTO_APPROVAL_INDEFINITELY, @@ -31,7 +31,6 @@ YARA, ) from olympia.files.models import FileUpload -from olympia.promoted.models import PromotedGroup from olympia.reviewers.models import UsageTier from olympia.scanners.actions import ( _delay_auto_approval, @@ -1001,12 +1000,12 @@ def test_disable_and_block_second_level_approval(self): UsageTier.objects.create( upper_adu_threshold=10000, disable_and_block_action_available=True ) - PromotedGroup.objects.get_or_create( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, high_profile=True - ) addon = addon_factory( average_daily_users=4242, - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={ + 'api_name': RECOMMENDED_API_NAME, + 'high_profile': True, + }, version_kw={'promotion_approved': False}, ) @@ -1327,7 +1326,7 @@ def test_execute_workflow_action_skips_ex_promoted_if_addon_is_promoted( self, flag_for_human_review_mock, no_action_mock ): self.make_addon_promoted( - self.addon, group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + self.addon, api_name=RECOMMENDED_API_NAME, listed_pre_review=True ) # Create another rule and add it to the current scanner result, but set to # exclude_promoted_addons=True. @@ -1541,7 +1540,8 @@ def test_execute_policy_enforcement_action_skips_exclude_promoted(self): # second level approval - if the rule didn't have # exclude_promoted_addons=True the add-on would be taken down. self.addon, - group_id=PROMOTED_GROUP_CHOICES.STRATEGIC, + api_name='strategic', + admin_review=True, ) ScannerResult.run_actions(self.version) @@ -1792,7 +1792,7 @@ def test_execute_policy_enforcement_action_2nd_level_approval(self): self.scanner_rule.update(policy=policy) # Make the add-on recommended, that will force 2nd level approval. self.make_addon_promoted( - self.addon, group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + self.addon, api_name=RECOMMENDED_API_NAME, high_profile=True ) ScannerResult.run_actions(self.version) diff --git a/src/olympia/scanners/tests/test_tasks.py b/src/olympia/scanners/tests/test_tasks.py index 90ed943c3469..cf6b0b809a87 100644 --- a/src/olympia/scanners/tests/test_tasks.py +++ b/src/olympia/scanners/tests/test_tasks.py @@ -17,7 +17,7 @@ user_factory, version_factory, ) -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import NOTABLE_API_NAME, RECOMMENDED_API_NAME from olympia.constants.scanners import ( ABORTED, ABORTING, @@ -2011,7 +2011,7 @@ def test_run_on_disabled_addons(self): def test_exclude_promoted_addons(self): self.make_addon_promoted( - self.version.addon, group_id=PROMOTED_GROUP_CHOICES.NOTABLE + self.version.addon, api_name=NOTABLE_API_NAME, listed_pre_review=True ) self.rule.update(exclude_promoted_addons=True, state=SCHEDULED) run_scanner_query_rule.delay(self.rule.pk) @@ -2202,7 +2202,7 @@ def test_run_on_chunk_not_blocked(self): def test_run_on_chunk_was_promoted(self): self.rule.update(state=RUNNING) # Pretend we started running the rule. self.make_addon_promoted( - self.version.addon, group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + self.version.addon, api_name=RECOMMENDED_API_NAME, listed_pre_review=True ) run_scanner_query_rule_on_versions_chunk([self.version.pk], self.rule.pk) diff --git a/src/olympia/search/filters.py b/src/olympia/search/filters.py index 6a31089f9c4b..d978d7b317f7 100644 --- a/src/olympia/search/filters.py +++ b/src/olympia/search/filters.py @@ -409,13 +409,12 @@ def __init__(self, request, query_data=None): self.query_data = query_data def get_values(self): - values = super().get_values() - if BADGED_API_NAME in values: - values = [ - *(value for value in values if value != BADGED_API_NAME), - *BADGED_GROUPS, - ] - return values + # Expand the meta "badged" group in place into its constituent groups, + # deduplicating while preserving order. + expanded = [] + for value in super().get_values(): + expanded.extend(BADGED_GROUPS if value == BADGED_API_NAME else [value]) + return list(dict.fromkeys(expanded)) def get_app(self): return ( diff --git a/src/olympia/search/tests/test_filters.py b/src/olympia/search/tests/test_filters.py index 0eeff6ee585e..9034d4087523 100644 --- a/src/olympia/search/tests/test_filters.py +++ b/src/olympia/search/tests/test_filters.py @@ -186,10 +186,7 @@ def _test_q(self, qs, query): }, 'weight': 4.0, } - assert functions[2] == { - 'filter': {'terms': {'promoted.category': BADGED_GROUPS}}, - 'weight': 5.0, - } + assert functions[2] == {'field_value_factor': {'field': 'ranking_bump'}} return qs def test_no_rescore_if_not_sorting_by_relevance(self): diff --git a/src/olympia/search/tests/test_search_ranking.py b/src/olympia/search/tests/test_search_ranking.py index e654041262b6..f1d8dae2c425 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 PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.constants.search import SEARCH_LANGUAGE_TO_ANALYZER @@ -464,7 +464,11 @@ def setUpTestData(cls): slug='tabby-cat-friend', summary='A new friend in every new tab.', weekly_downloads=350, - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={ + 'api_name': RECOMMENDED_API_NAME, + 'listed_pre_review': True, + 'search_ranking_bump': 5.0, + }, ) amo.tests.addon_factory( average_daily_users=5819, @@ -622,7 +626,11 @@ def setUpTestData(cls): slug='stripy-dog-1', summary='A new friend in every new window.', weekly_downloads=350, - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={ + 'api_name': RECOMMENDED_API_NAME, + 'listed_pre_review': True, + 'search_ranking_bump': 5.0, + }, ) amo.tests.addon_factory( average_daily_users=4089, @@ -631,7 +639,11 @@ def setUpTestData(cls): slug='stripy-dog-2', summary='A new friend in every new window.', weekly_downloads=350, - promoted_id=PROMOTED_GROUP_CHOICES.LINE, + promoted_kwargs={ + 'api_name': 'line', + 'listed_pre_review': True, + 'search_ranking_bump': 5.0, + }, ) amo.tests.addon_factory( average_daily_users=4089, @@ -640,7 +652,10 @@ def setUpTestData(cls): slug='stripy-dog-3', summary='A new friend in every new window.', weekly_downloads=350, - promoted_id=PROMOTED_GROUP_CHOICES.SPOTLIGHT, + promoted_kwargs={ + 'api_name': 'spotlight', + 'listed_pre_review': True, + }, ) amo.tests.addon_factory( average_daily_users=4089, diff --git a/src/olympia/shelves/tests/test_serializers.py b/src/olympia/shelves/tests/test_serializers.py index ea04abc6d683..ca484da20a87 100644 --- a/src/olympia/shelves/tests/test_serializers.py +++ b/src/olympia/shelves/tests/test_serializers.py @@ -18,7 +18,7 @@ ) from olympia.amo.urlresolvers import get_outgoing_url from olympia.bandwagon.models import CollectionAddon -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.users.models import UserProfile from ..models import Shelf @@ -55,7 +55,10 @@ def setUpTestData(cls): average_daily_users=482, weekly_downloads=506, summary=None, - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={ + 'api_name': RECOMMENDED_API_NAME, + 'listed_pre_review': True, + }, tags=('baa',), ) addon_factory( @@ -64,7 +67,10 @@ def setUpTestData(cls): average_daily_users=8838, weekly_downloads=358, summary=None, - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={ + 'api_name': RECOMMENDED_API_NAME, + 'listed_pre_review': True, + }, tags=('foo',), ) diff --git a/src/olympia/shelves/tests/test_views.py b/src/olympia/shelves/tests/test_views.py index 963f90fa4958..95d2ca25c3e3 100644 --- a/src/olympia/shelves/tests/test_views.py +++ b/src/olympia/shelves/tests/test_views.py @@ -16,7 +16,7 @@ reverse_ns, ) from olympia.bandwagon.models import CollectionAddon -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.hero.models import PrimaryHero, SecondaryHero, SecondaryHeroModule from olympia.hero.serializers import ( PrimaryHeroShelfSerializer, @@ -69,7 +69,11 @@ def setUpTestData(cls): summary=None, ) - group = PromotedGroup.objects.get(group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED) + group, _ = PromotedGroup.objects.get_or_create( + api_name=RECOMMENDED_API_NAME, + name='Recommended', + listed_pre_review=True, + ) PromotedAddon.objects.create( addon=addon_ext, promoted_group=group, application_id=amo.FIREFOX.id ) diff --git a/src/olympia/versions/tests/test_commands.py b/src/olympia/versions/tests/test_commands.py index e6ff6b11e3ec..b53371cfe58a 100644 --- a/src/olympia/versions/tests/test_commands.py +++ b/src/olympia/versions/tests/test_commands.py @@ -16,7 +16,6 @@ ) from olympia.applications.models import AppVersion from olympia.blocklist.models import Block, BlockType, BlockVersion -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES from olympia.versions.compare import version_int from olympia.versions.management.commands.force_min_android_compatibility import ( Command as ForceMinAndroidCompatibility, @@ -83,7 +82,10 @@ def test_full(self): 'min_app_version': '48.0', 'max_app_version': '*', }, - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={ + 'api_name': 'recommended', + 'can_be_compatible_with_all_fenix_versions': True, + }, ), addon_factory( name='Line for all', @@ -92,7 +94,10 @@ def test_full(self): 'min_app_version': '48.0', 'max_app_version': '*', }, - promoted_id=PROMOTED_GROUP_CHOICES.LINE, + promoted_kwargs={ + 'api_name': 'line', + 'can_be_compatible_with_all_fenix_versions': True, + }, ), ] addons_to_ignore_not_in_csv = [addon_factory(name='Not in csv')] @@ -225,7 +230,10 @@ def test_basic(self): 'min_app_version': '119.0a1', 'max_app_version': '*', }, - promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED, + promoted_kwargs={ + 'api_name': 'recommended', + 'can_be_compatible_with_all_fenix_versions': True, + }, ) to_bump = [ diff --git a/src/olympia/versions/tests/test_models.py b/src/olympia/versions/tests/test_models.py index 920ea75c0e0d..6ee58ae834f4 100644 --- a/src/olympia/versions/tests/test_models.py +++ b/src/olympia/versions/tests/test_models.py @@ -27,7 +27,6 @@ from olympia.applications.models import AppVersion from olympia.blocklist.models import Block, BlockVersion from olympia.constants.blocklist import BlockReason, BlockType -from olympia.constants.promoted import PROMOTED_GROUP_CHOICES from olympia.constants.scanners import YARA from olympia.files.models import File from olympia.files.tests.test_models import UploadMixin @@ -440,7 +439,7 @@ def test_should_have_due_date(self): # Or if it's in a pre-review promoted group it will. recommended = addon_factory(**addon_kws).current_version self.make_addon_promoted( - addon=recommended.addon, group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + addon=recommended.addon, api_name='recommended', listed_pre_review=True ) NeedsHumanReview.objects.create( version=recommended, @@ -448,9 +447,7 @@ def test_should_have_due_date(self): ) # And not if it's a non-pre-review group - self.make_addon_promoted( - addon=recommended.addon, group_id=PROMOTED_GROUP_CHOICES.STRATEGIC - ) + self.make_addon_promoted(addon=recommended.addon, api_name='strategic') # A disabled version with a developer reply developer_reply = addon_factory( @@ -479,7 +476,7 @@ def test_should_have_due_date(self): # And a version with multiple reasons multiple = addon_factory(**addon_kws).current_version self.make_addon_promoted( - addon=recommended.addon, group_id=PROMOTED_GROUP_CHOICES.LINE + addon=recommended.addon, api_name='line', listed_pre_review=True ) NeedsHumanReview.objects.create( version=multiple, reason=NeedsHumanReview.REASONS.BELONGS_TO_PROMOTED_GROUP @@ -1416,10 +1413,14 @@ def test_promoted_can_be_disabled_and_deleted(self): assert addon.current_version.can_be_disabled_and_deleted() self.make_addon_promoted( - addon, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + addon, + api_name='recommended', + listed_pre_review=True, + badged=True, + approve_version=True, ) addon = addon.reload() - assert PROMOTED_GROUP_CHOICES.RECOMMENDED in addon.promoted_groups().group_id + assert 'recommended' in addon.promoted_groups().api_name # But a promoted one, that's in a prereview group, can't be disabled assert not addon.current_version.can_be_disabled_and_deleted() @@ -1428,26 +1429,24 @@ def test_promoted_can_be_disabled_and_deleted(self): addon = addon.reload() assert previous_version != addon.current_version assert addon.current_version.promoted_versions.filter( - promoted_group__group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + promoted_group__api_name='recommended' ).exists() assert previous_version.promoted_versions.filter( - promoted_group__group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + promoted_group__api_name='recommended' ).exists() # unless the previous version is also approved for the same group assert addon.current_version.can_be_disabled_and_deleted() assert previous_version.can_be_disabled_and_deleted() # double-check by changing the approval of previous version - previous_version.promoted_versions.update( - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.LINE - ) + line_group, _ = PromotedGroup.objects.get_or_create( + api_name='line', + defaults={'name': 'line', 'listed_pre_review': True, 'badged': True}, ) + previous_version.promoted_versions.update(promoted_group=line_group) assert not addon.current_version.can_be_disabled_and_deleted() previous_version.promoted_versions.update( - promoted_group=PromotedGroup.objects.get( - group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED - ) + promoted_group=PromotedGroup.objects.get(api_name='recommended') ) # Check the scenario when some of the previous versions are approved @@ -1462,61 +1461,67 @@ def test_promoted_can_be_disabled_and_deleted(self): version_b = version_b.reload() assert version_d == addon.current_version assert version_a.promoted_versions.filter( - promoted_group__group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + promoted_group__api_name='recommended' ).exists() assert version_b.promoted_versions.filter( - promoted_group__group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + promoted_group__api_name='recommended' ).exists() # ignored because disabled assert not version_c.promoted_versions.filter( - promoted_group__group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + promoted_group__api_name='recommended' ).exists() assert version_d.promoted_versions.filter( - promoted_group__group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED + promoted_group__api_name='recommended' ).exists() assert version_a.can_be_disabled_and_deleted() assert version_b.can_be_disabled_and_deleted() assert version_c.can_be_disabled_and_deleted() assert version_d.can_be_disabled_and_deleted() - assert PROMOTED_GROUP_CHOICES.RECOMMENDED in addon.promoted_groups().group_id + assert 'recommended' in addon.promoted_groups().api_name # now un-approve version_b version_b.promoted_versions.all().delete() assert version_a.can_be_disabled_and_deleted() assert version_b.can_be_disabled_and_deleted() assert version_c.can_be_disabled_and_deleted() assert not version_d.can_be_disabled_and_deleted() - assert PROMOTED_GROUP_CHOICES.RECOMMENDED in addon.promoted_groups().group_id + assert 'recommended' in addon.promoted_groups().api_name def test_unbadged_non_prereview_promoted_can_be_disabled_and_deleted(self): addon = Addon.objects.get(id=3615) self.make_addon_promoted( - addon, PROMOTED_GROUP_CHOICES.LINE, approve_version=True + addon, + api_name='line', + listed_pre_review=True, + badged=True, + approve_version=True, ) - assert PROMOTED_GROUP_CHOICES.LINE in addon.promoted_groups().group_id + assert 'line' in addon.promoted_groups().api_name # it's the only version of a group that requires pre-review and is # badged, so can't be deleted. assert not addon.current_version.can_be_disabled_and_deleted() - # STRATEGIC isn't pre-reviewd or badged, so it's okay though + # strategic isn't pre-reviewd or badged, so it's okay though addon.promotedaddon.all().delete() - self.make_addon_promoted( - addon, PROMOTED_GROUP_CHOICES.STRATEGIC, approve_version=True - ) - assert PROMOTED_GROUP_CHOICES.STRATEGIC in addon.promoted_groups().group_id + self.make_addon_promoted(addon, api_name='strategic', approve_version=True) + assert 'strategic' in addon.promoted_groups().api_name assert addon.current_version.can_be_disabled_and_deleted() - # SPOTLIGHT is pre-reviewed but not badged, so it's okay too + # spotlight is pre-reviewed but not badged, so it's okay too self.make_addon_promoted( - addon, PROMOTED_GROUP_CHOICES.SPOTLIGHT, approve_version=True + addon, api_name='spotlight', listed_pre_review=True, approve_version=True ) - assert PROMOTED_GROUP_CHOICES.SPOTLIGHT in addon.promoted_groups().group_id + assert 'spotlight' in addon.promoted_groups().api_name assert addon.current_version.can_be_disabled_and_deleted() def test_can_be_disabled_and_deleted_querycount(self): addon = Addon.objects.get(id=3615) version_factory(addon=addon) self.make_addon_promoted( - addon, PROMOTED_GROUP_CHOICES.RECOMMENDED, approve_version=True + addon, + api_name='recommended', + listed_pre_review=True, + badged=True, + approve_version=True, ) addon.reload() with self.assertNumQueries(3): @@ -3196,7 +3201,12 @@ def test_not_extension(self): assert not avs.version.file.reload().strict_compatibility def test_recommended_for_android(self): - addon = addon_factory(promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED) + addon = addon_factory( + promoted_kwargs={ + 'api_name': 'recommended', + 'can_be_compatible_with_all_fenix_versions': True, + } + ) assert amo.ANDROID in addon.approved_applications avs = ApplicationsVersions( application=amo.ANDROID.id, @@ -3209,7 +3219,12 @@ def test_recommended_for_android(self): assert not avs.version.file.reload().strict_compatibility def test_line_for_android(self): - addon = addon_factory(promoted_id=PROMOTED_GROUP_CHOICES.LINE) + addon = addon_factory( + promoted_kwargs={ + 'api_name': 'line', + 'can_be_compatible_with_all_fenix_versions': True, + } + ) assert amo.ANDROID in addon.approved_applications avs = ApplicationsVersions( application=amo.ANDROID.id, @@ -3222,7 +3237,7 @@ def test_line_for_android(self): assert not avs.version.file.reload().strict_compatibility def test_other_promotion_for_android(self): - addon = addon_factory(promoted_id=PROMOTED_GROUP_CHOICES.NOTABLE) + addon = addon_factory(promoted_kwargs={'api_name': 'notable'}) assert amo.ANDROID in addon.approved_applications avs = ApplicationsVersions( application=amo.ANDROID.id, @@ -3236,7 +3251,13 @@ def test_other_promotion_for_android(self): assert not avs.version.file.reload().strict_compatibility def test_recommended_or_line_for_desktop(self): - addon = addon_factory(promoted_id=PROMOTED_GROUP_CHOICES.RECOMMENDED) + addon = addon_factory( + promoted_kwargs={ + 'api_name': 'recommended', + 'listed_pre_review': True, + 'can_be_compatible_with_all_fenix_versions': True, + } + ) android_approval = ( addon.current_version.promoted_versions.all() .filter(application_id=amo.ANDROID.id) From 93458ac64e1ff47b0aeb0953e9c75bdb07ca6b79 Mon Sep 17 00:00:00 2001 From: eviljeff Date: Tue, 7 Jul 2026 17:16:01 +0100 Subject: [PATCH 10/14] revert some of the search_ranking_bump changes --- src/olympia/addons/indexers.py | 7 ++----- src/olympia/search/filters.py | 6 +++++- src/olympia/search/tests/test_filters.py | 4 +++- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/olympia/addons/indexers.py b/src/olympia/addons/indexers.py index 3f24c6f9222a..4edbeedcb37d 100644 --- a/src/olympia/addons/indexers.py +++ b/src/olympia/addons/indexers.py @@ -693,11 +693,8 @@ def extract_document(cls, obj): ), default=1.0, ) - # ranking_bump is a multiplicative factor in the search ranking, so a - # group without a bump (or no promotion at all) must map to a neutral - # 1.0 rather than 0.0/absent, which would zero the score or make - # field_value_factor fail on the missing field. - data['ranking_bump'] = max_promoted_ranking_bump or 1.0 + if max_promoted_ranking_bump: + data['ranking_bump'] = max_promoted_ranking_bump data['ratings'] = { 'average': obj.average_rating, diff --git a/src/olympia/search/filters.py b/src/olympia/search/filters.py index d978d7b317f7..97acecbe8e07 100644 --- a/src/olympia/search/filters.py +++ b/src/olympia/search/filters.py @@ -900,7 +900,11 @@ def apply_search_query(self, search_query, qs, sort=None): ), } ), - query.SF('field_value_factor', field='ranking_bump'), + # ranking_bump is a multiplicative factor. Documents without a bump + # (non-promoted add-ons, or promoted groups with no search ranking + # bump) don't have the field indexed, so treat it as a neutral 1.0 + # rather than letting field_value_factor fail on the missing field. + query.SF('field_value_factor', field='ranking_bump', missing=1), ] # Assemble everything together diff --git a/src/olympia/search/tests/test_filters.py b/src/olympia/search/tests/test_filters.py index 9034d4087523..0fda0f3fd583 100644 --- a/src/olympia/search/tests/test_filters.py +++ b/src/olympia/search/tests/test_filters.py @@ -186,7 +186,9 @@ def _test_q(self, qs, query): }, 'weight': 4.0, } - assert functions[2] == {'field_value_factor': {'field': 'ranking_bump'}} + assert functions[2] == { + 'field_value_factor': {'field': 'ranking_bump', 'missing': 1} + } return qs def test_no_rescore_if_not_sorting_by_relevance(self): From f6d414644996bfcd4db19035e4058345df50d009 Mon Sep 17 00:00:00 2001 From: eviljeff Date: Tue, 7 Jul 2026 18:05:07 +0100 Subject: [PATCH 11/14] change promote_by_firefox_themes to use unique api_name --- .../promoted/management/commands/promote_by_firefox_themes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/olympia/promoted/management/commands/promote_by_firefox_themes.py b/src/olympia/promoted/management/commands/promote_by_firefox_themes.py index 1b0f7fa28339..dd4d2f346add 100644 --- a/src/olympia/promoted/management/commands/promote_by_firefox_themes.py +++ b/src/olympia/promoted/management/commands/promote_by_firefox_themes.py @@ -21,7 +21,7 @@ def handle(self, *args, **options): ) for addon in addons: self.stdout.write(f'Promoting {addon.slug}') - group = PromotedGroup.objects.get(name='By Firefox') + group = PromotedGroup.objects.get(api_name='line') PromotedAddon.objects.get_or_create( addon=addon, application_id=amo.FIREFOX.id, From ed8041ac82e921d86914165b5199176c7504f7ac Mon Sep 17 00:00:00 2001 From: eviljeff Date: Tue, 7 Jul 2026 18:24:13 +0100 Subject: [PATCH 12/14] refactor AddonPromotedQueryParam.get_values --- src/olympia/constants/promoted.py | 2 +- src/olympia/search/filters.py | 13 +++++++------ src/olympia/search/tests/test_filters.py | 4 ++-- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/olympia/constants/promoted.py b/src/olympia/constants/promoted.py index b9d73361b9ca..d82c5d79de44 100644 --- a/src/olympia/constants/promoted.py +++ b/src/olympia/constants/promoted.py @@ -1,4 +1,4 @@ BADGED_API_NAME = 'badged' # Special alias for all badged groups -BADGED_GROUPS = ['recommended', 'line'] # special badged groups +BADGED_GROUPS = ['line', 'recommended'] # special badged groups RECOMMENDED_API_NAME = 'recommended' # Name of the "original" promoted group NOTABLE_API_NAME = 'notable' # Name of the group we automatically add add-ons to diff --git a/src/olympia/search/filters.py b/src/olympia/search/filters.py index 97acecbe8e07..551f4575de0a 100644 --- a/src/olympia/search/filters.py +++ b/src/olympia/search/filters.py @@ -409,12 +409,13 @@ def __init__(self, request, query_data=None): self.query_data = query_data def get_values(self): - # Expand the meta "badged" group in place into its constituent groups, - # deduplicating while preserving order. - expanded = [] - for value in super().get_values(): - expanded.extend(BADGED_GROUPS if value == BADGED_API_NAME else [value]) - return list(dict.fromkeys(expanded)) + # Expand the meta "badged" group into its constituent groups if present + values = super().get_values() + processed_values = { + *(val for val in values if val != BADGED_API_NAME), + *(BADGED_GROUPS if BADGED_API_NAME in values else ()), + } + return sorted(processed_values) def get_app(self): return ( diff --git a/src/olympia/search/tests/test_filters.py b/src/olympia/search/tests/test_filters.py index 0fda0f3fd583..c1c78ee14ed7 100644 --- a/src/olympia/search/tests/test_filters.py +++ b/src/olympia/search/tests/test_filters.py @@ -933,7 +933,7 @@ def test_search_by_promoted(self): # test multiple param values qs = self._filter(data={'promoted': 'recommended,line'}) filter_ = qs['query']['bool']['filter'] - assert [{'terms': {'promoted.category': ['recommended', 'line']}}] == filter_ + assert [{'terms': {'promoted.category': ['line', 'recommended']}}] == filter_ # test combining multiple values with the meta "badged" group qs = self._filter(data={'promoted': 'badged,recommended,strategic'}) @@ -943,8 +943,8 @@ def test_search_by_promoted(self): 'terms': { 'promoted.category': [ # recommended shouldn't be there twice - 'recommended', 'line', + 'recommended', 'strategic', ] } From 85ed628e8dc1a0dbe191348af0bc822a25e796c7 Mon Sep 17 00:00:00 2001 From: eviljeff Date: Tue, 14 Jul 2026 18:56:00 +0100 Subject: [PATCH 13/14] use appropriate api_names, and only use RECOMMENDED_API_NAME where needed --- src/olympia/abuse/tests/test_actions.py | 30 ++++++++------- src/olympia/abuse/tests/test_cinder.py | 3 +- src/olympia/abuse/tests/test_models.py | 10 ++--- src/olympia/abuse/tests/test_utils.py | 2 +- src/olympia/addons/tests/test_models.py | 38 +++++++++---------- src/olympia/addons/tests/test_serializers.py | 6 +-- src/olympia/addons/tests/test_views.py | 14 +++---- src/olympia/devhub/tests/test_forms.py | 4 +- src/olympia/devhub/tests/test_views.py | 7 ++-- src/olympia/devhub/tests/test_views_submit.py | 5 +-- .../devhub/tests/test_views_versions.py | 17 +++------ src/olympia/hero/tests/test_models.py | 12 +++--- src/olympia/hero/tests/test_serializers.py | 9 ++--- src/olympia/lib/crypto/tests/test_signing.py | 11 +++--- src/olympia/promoted/tests/test_admin.py | 37 +++++++++--------- src/olympia/promoted/tests/test_models.py | 3 +- src/olympia/reviewers/tests/test_cron.py | 18 ++++----- src/olympia/reviewers/tests/test_models.py | 2 +- src/olympia/reviewers/tests/test_utils.py | 29 +++++++------- src/olympia/reviewers/tests/test_views.py | 10 ++--- src/olympia/scanners/tests/test_actions.py | 11 ++---- src/olympia/scanners/tests/test_tasks.py | 4 +- src/olympia/search/tests/test_filters.py | 14 ++++--- .../search/tests/test_search_ranking.py | 5 +-- src/olympia/shelves/tests/test_serializers.py | 8 ++-- src/olympia/versions/tests/test_commands.py | 4 +- src/olympia/versions/tests/test_models.py | 30 +++++++-------- 27 files changed, 167 insertions(+), 176 deletions(-) diff --git a/src/olympia/abuse/tests/test_actions.py b/src/olympia/abuse/tests/test_actions.py index 511113bf3f37..a8c1c41e3e3e 100644 --- a/src/olympia/abuse/tests/test_actions.py +++ b/src/olympia/abuse/tests/test_actions.py @@ -583,7 +583,7 @@ def test_should_hold_action(self): assert action_helper.should_hold_action() is False addon = addon_factory(users=[self.user]) assert action_helper.should_hold_action() is False - self.make_addon_promoted(addon, api_name='recommended', high_profile=True) + self.make_addon_promoted(addon, api_name='high_profile', high_profile=True) assert action_helper.should_hold_action() is True self.user.banned = datetime.now() @@ -865,7 +865,7 @@ def test_should_hold_action(self): action_helper = self.ActionClass(self.decision) assert action_helper.should_hold_action() is False - self.make_addon_promoted(self.addon, api_name='recommended', high_profile=True) + self.make_addon_promoted(self.addon, api_name='high_profile', high_profile=True) assert action_helper.should_hold_action() is True self.addon.status = amo.STATUS_DISABLED @@ -1471,7 +1471,7 @@ def test_execute_action_with_stakeholder_email(self): ) self.version.file.update(is_signed=True) self.another_version.file.update(approval_date=datetime(2025, 2, 3)) - self.make_addon_promoted(self.addon, api_name='recommended', high_profile=True) + self.make_addon_promoted(self.addon, api_name='high_profile', high_profile=True) self._test_reject_version(content_review=False, expected_emails_from_action=1) assert len(mail.outbox) == 4 assert mail.outbox[0].recipients() == [stakeholder.email] @@ -1627,7 +1627,7 @@ def test_execute_action_delayed_with_stakeholder_email(self): ) self.version.file.update(is_signed=True) self.another_version.file.update(approval_date=datetime(2025, 2, 3)) - self.make_addon_promoted(self.addon, api_name='recommended', high_profile=True) + self.make_addon_promoted(self.addon, api_name='high_profile', high_profile=True) self._test_reject_version_delayed( content_review=False, expected_emails_from_action=1 ) @@ -1741,7 +1741,7 @@ def test_should_hold_action(self): action_helper = self.ActionClass(self.decision) assert action_helper.should_hold_action() is False - self.make_addon_promoted(self.addon, api_name='recommended', high_profile=True) + self.make_addon_promoted(self.addon, api_name='high_profile', high_profile=True) self.decision.target_versions.add(self.another_version) assert self.decision.target_versions.filter(file__is_signed=True).exists() assert action_helper.should_hold_action() is True @@ -1753,7 +1753,7 @@ def test_should_hold_action(self): def test_should_hold_action_some_versions_remain(self): self.decision.update(action=DECISION_ACTIONS.AMO_REJECT_VERSION_ADDON) - self.make_addon_promoted(self.addon, api_name='recommended', high_profile=True) + self.make_addon_promoted(self.addon, api_name='high_profile', high_profile=True) self.version.file.update(is_signed=True) # While there are more public listed versions that wouldn't be affected @@ -1828,7 +1828,7 @@ def test_notify_stakeholders(self): assert len(mail.outbox) == 0 # make the addon promoted - self.make_addon_promoted(self.addon, api_name='recommended', high_profile=True) + self.make_addon_promoted(self.addon, api_name='high_profile', high_profile=True) action_helper.notify_stakeholders('teh reason') assert len(mail.outbox) == 1 body = mail.outbox[0].body @@ -1910,7 +1910,7 @@ def test_notify_stakeholders_with_private_notes(self): self.decision.update(private_notes='These are the private notes.') # make the addon promoted - self.make_addon_promoted(self.addon, api_name='recommended', high_profile=True) + self.make_addon_promoted(self.addon, api_name='high_profile', high_profile=True) action_helper.notify_stakeholders('teh reason') assert len(mail.outbox) == 1 assert mail.outbox[0].recipients() == [stakeholder.email] @@ -1941,7 +1941,7 @@ def test_notify_stakeholders_with_policy_texts(self): ) # make the addon promoted - self.make_addon_promoted(self.addon, api_name='recommended', high_profile=True) + self.make_addon_promoted(self.addon, api_name='high_profile', high_profile=True) action_helper.notify_stakeholders('teh reason') assert len(mail.outbox) == 1 assert mail.outbox[0].recipients() == [stakeholder.email] @@ -2082,7 +2082,7 @@ def test_should_hold_action(self): action_helper = self.ActionClass(self.decision) assert action_helper.should_hold_action() is False - self.make_addon_promoted(self.addon, api_name='recommended', high_profile=True) + self.make_addon_promoted(self.addon, api_name='high_profile', high_profile=True) assert action_helper.should_hold_action() is True # if one version is not blocked we still hold the action @@ -2944,15 +2944,17 @@ def test_execute_action_unlisted(self): assert 'has been approved' in mail.outbox[0].body def test_execute_action_promoted(self): - self.make_addon_promoted(self.addon, api_name='recommended', high_profile=True) + self.make_addon_promoted( + self.addon, api_name='pre_review', listed_pre_review=True + ) assert not self.addon.promoted_groups() self.test_execute_action() assert self.addon.promoted_groups() assert self.version.promoted_versions.filter( - promoted_group__api_name='recommended' + promoted_group__api_name='pre_review' ).exists() assert self.old_version.promoted_versions.filter( - promoted_group__api_name='recommended' + promoted_group__api_name='pre_review' ).exists() def test_execute_action_not_human(self): @@ -3563,7 +3565,7 @@ def test_should_hold_action(self): AddonUser.objects.create(addon=self.rating.addon, user=self.rating.user) assert action_helper.should_hold_action() is False self.make_addon_promoted( - self.rating.addon, api_name='recommended', high_profile=True + self.rating.addon, api_name='high_profile_rating', high_profile_rating=True ) assert action_helper.should_hold_action() is False self.rating.update( diff --git a/src/olympia/abuse/tests/test_cinder.py b/src/olympia/abuse/tests/test_cinder.py index ef145f4e1fbf..b9117ce9cab7 100644 --- a/src/olympia/abuse/tests/test_cinder.py +++ b/src/olympia/abuse/tests/test_cinder.py @@ -30,6 +30,7 @@ ILLEGAL_CATEGORIES, ILLEGAL_SUBCATEGORIES, ) +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.promoted.models import PromotedAddon from olympia.ratings.models import Rating from olympia.reviewers.models import NeedsHumanReview @@ -388,7 +389,7 @@ def test_build_report_payload_promoted_recommended(self): privacy_policy='Söme privacy policy', version_kw={'release_notes': 'Søme release notes'}, ) - self.make_addon_promoted(addon, api_name='recommended', high_profile=True) + self.make_addon_promoted(addon, api_name=RECOMMENDED_API_NAME) message = ' bad addon!' cinder_addon = self.CinderClass(addon) encoded_message = cinder_addon.get_str(message) diff --git a/src/olympia/abuse/tests/test_models.py b/src/olympia/abuse/tests/test_models.py index 99f5d58897f0..95411a873d88 100644 --- a/src/olympia/abuse/tests/test_models.py +++ b/src/olympia/abuse/tests/test_models.py @@ -3414,7 +3414,7 @@ def test_execute_action_disable_addon_held(self): self.grant_permission(user_factory(), ':'.join(ADDONS_HIGH_IMPACT_APPROVE)) addon = addon_factory(users=[user_factory()]) self.make_addon_promoted( - addon, api_name='recommended', high_profile=True, approve_version=True + addon, api_name='high_profile', high_profile=True, approve_version=True ) decision = ContentDecision.objects.create( addon=addon, @@ -3546,7 +3546,7 @@ def test_execute_action_reject_version_held(self): addon = addon_factory(users=[user_factory()], file_kw={'is_signed': True}) version = addon.current_version self.make_addon_promoted( - addon, api_name='recommended', high_profile=True, approve_version=True + addon, api_name='high_profile', high_profile=True, approve_version=True ) version.needshumanreview_set.create( reason=NeedsHumanReview.REASONS.ABUSE_ADDON_VIOLATION @@ -3737,7 +3737,7 @@ def test_execute_action_reject_version_delayed_held(self): addon = addon_factory(users=[user_factory()], file_kw={'is_signed': True}) version = addon.current_version self.make_addon_promoted( - addon, api_name='recommended', high_profile=True, approve_version=True + addon, api_name='high_profile', high_profile=True, approve_version=True ) some_time_ago = self.days_ago(13) little_over_fourteen_days = timedelta(days=14, minutes=1) @@ -4020,8 +4020,8 @@ def test_execute_action_delete_rating_held(self): ) self.make_addon_promoted( rating.addon, - api_name='recommended', - high_profile=True, + api_name='high_profile_rating', + high_profile_rating=True, approve_version=True, ) assert decision.action_date is None diff --git a/src/olympia/abuse/tests/test_utils.py b/src/olympia/abuse/tests/test_utils.py index 39461687ea5e..31956585b2ca 100644 --- a/src/olympia/abuse/tests/test_utils.py +++ b/src/olympia/abuse/tests/test_utils.py @@ -38,7 +38,7 @@ def test_reject_and_block_addons(): normal_addon = addon_factory(users=[user_factory()]) normal_addon_version = normal_addon.current_version recommended_addon = addon_factory( - promoted_kwargs={'api_name': 'recommended', 'high_profile': True}, + promoted_kwargs={'api_name': 'high_profile', 'high_profile': True}, users=[user_factory()], version_kw={'promotion_approved': False}, ) diff --git a/src/olympia/addons/tests/test_models.py b/src/olympia/addons/tests/test_models.py index 0be7ec01c026..6d45d401ad49 100644 --- a/src/olympia/addons/tests/test_models.py +++ b/src/olympia/addons/tests/test_models.py @@ -1763,7 +1763,7 @@ def test_no_promoted_groups(self): def test_unapproved_promoted_groups(self): addon = addon_factory() self.make_addon_promoted( - addon=addon, api_name='recommended', listed_pre_review=True + addon=addon, api_name='pre_review', listed_pre_review=True ) self.make_addon_promoted(addon=addon, api_name='line', listed_pre_review=True) assert addon.promoted_groups(currently_approved=False) @@ -1772,7 +1772,7 @@ def test_unapproved_promoted_groups(self): def test_approved_promoted_groups(self): addon = addon_factory() self.make_addon_promoted( - addon=addon, api_name='recommended', listed_pre_review=True + addon=addon, api_name='pre_review', listed_pre_review=True ) self.make_addon_promoted(addon=addon, api_name='line', listed_pre_review=True) # spotlight group used later to swap a promotion to an unapproved group @@ -1781,13 +1781,13 @@ def test_approved_promoted_groups(self): ) addon.approve_for_version() assert addon.promoted_groups() - assert 'recommended' in addon.promoted_groups().api_name + assert 'pre_review' in addon.promoted_groups().api_name assert 'line' in addon.promoted_groups().api_name # if the group for one approved group changes, its # approval for the current version isn't valid, # but other groups remain valid PromotedAddon.objects.filter( - addon=addon, promoted_group__api_name='recommended' + addon=addon, promoted_group__api_name='pre_review' ).update(promoted_group=spotlight_group) assert 'spotlight' in addon.promoted_groups(currently_approved=False).api_name assert 'line' in addon.promoted_groups(currently_approved=False).api_name @@ -1797,7 +1797,7 @@ def test_approved_promoted_groups(self): def test_unapproved_group_after_approval_promoted_groups(self): addon = addon_factory() self.make_addon_promoted( - addon=addon, api_name='recommended', listed_pre_review=True + addon=addon, api_name='pre_review', listed_pre_review=True ) self.make_addon_promoted(addon=addon, api_name='line', listed_pre_review=True) addon.approve_for_version() @@ -1805,13 +1805,13 @@ def test_unapproved_group_after_approval_promoted_groups(self): addon=addon, api_name='notable', listed_pre_review=True ) assert addon.promoted_groups() - assert 'recommended' in addon.promoted_groups().api_name + assert 'pre_review' in addon.promoted_groups().api_name assert 'line' in addon.promoted_groups().api_name assert 'notable' not in addon.promoted_groups().api_name assert 'notable' in addon.promoted_groups(currently_approved=False).api_name # Unless approved. addon.approve_for_version() - assert 'recommended' in addon.promoted_groups().api_name + assert 'pre_review' in addon.promoted_groups().api_name assert 'line' in addon.promoted_groups().api_name assert 'notable' in addon.promoted_groups().api_name @@ -1819,7 +1819,7 @@ def test_application_specific_multiple_promoted_groups(self): addon = addon_factory() self.make_addon_promoted( addon=addon, - api_name='recommended', + api_name='pre_review', listed_pre_review=True, approve_version=True, ) @@ -1830,7 +1830,7 @@ def test_application_specific_multiple_promoted_groups(self): approve_version=True, ) # Application specific group membership should still work - assert 'recommended' in addon.promoted_groups().api_name + assert 'pre_review' in addon.promoted_groups().api_name assert 'line' in addon.promoted_groups().api_name # update to android only PromotedAddon.objects.filter( @@ -1838,14 +1838,14 @@ def test_application_specific_multiple_promoted_groups(self): promoted_group__api_name='line', application_id=amo.FIREFOX.id, ).delete() - assert 'recommended' in addon.promoted_groups().api_name + assert 'pre_review' in addon.promoted_groups().api_name assert 'line' in addon.promoted_groups().api_name # but if there's no approval for Android it's not promoted addon.current_version.promoted_versions.filter( application_id=amo.ANDROID.id ).delete() - assert 'recommended' in addon.promoted_groups().api_name + assert 'pre_review' in addon.promoted_groups().api_name assert 'line' not in addon.promoted_groups().api_name def test_no_promoted(self): @@ -1856,7 +1856,7 @@ def test_no_promoted(self): def test_promoted_groups_doesnt_error_with_no_version(self): addon = addon_factory() self.make_addon_promoted( - addon=addon, api_name='recommended', listed_pre_review=True + addon=addon, api_name='pre_review', listed_pre_review=True ) addon.current_version.file.update(status=amo.STATUS_DISABLED) addon.update_version() @@ -1867,7 +1867,7 @@ def test_promoted_groups_doesnt_error_with_no_version(self): def test_unapproved_promoted(self): addon = addon_factory() self.make_addon_promoted( - addon=addon, api_name='recommended', listed_pre_review=True + addon=addon, api_name='pre_review', listed_pre_review=True ) self.make_addon_promoted(addon=addon, api_name='line', listed_pre_review=True) assert addon.publicly_promoted_groups == [] @@ -1875,7 +1875,7 @@ def test_unapproved_promoted(self): def test_approved_promoted(self): addon = addon_factory() self.make_addon_promoted( - addon=addon, api_name='recommended', listed_pre_review=True + addon=addon, api_name='pre_review', listed_pre_review=True ) self.make_addon_promoted(addon=addon, api_name='line', listed_pre_review=True) # spotlight group used later to swap a promotion to an unapproved group @@ -1884,7 +1884,7 @@ def test_approved_promoted(self): ) addon.approve_for_version(addon.current_version) assert ( - PromotedGroup.objects.get(api_name='recommended') + PromotedGroup.objects.get(api_name='pre_review') in addon.publicly_promoted_groups ) assert ( @@ -1893,7 +1893,7 @@ def test_approved_promoted(self): # If the group changes the approval for that group # in the current version isn't valid. PromotedAddon.objects.filter( - addon=addon, promoted_group__api_name='recommended' + addon=addon, promoted_group__api_name='pre_review' ).update(promoted_group=spotlight_group) del addon.publicly_promoted_groups assert spotlight_group not in addon.publicly_promoted_groups @@ -1904,7 +1904,7 @@ def test_approved_promoted(self): def test_unapproved_group_after_approval_promoted(self): addon = addon_factory() self.make_addon_promoted( - addon=addon, api_name='recommended', listed_pre_review=True + addon=addon, api_name='pre_review', listed_pre_review=True ) self.make_addon_promoted(addon=addon, api_name='line', listed_pre_review=True) addon.approve_for_version(addon.current_version) @@ -1915,7 +1915,7 @@ def test_unapproved_group_after_approval_promoted(self): PromotedGroup.objects.get(api_name='line') in addon.publicly_promoted_groups ) assert ( - PromotedGroup.objects.get(api_name='recommended') + PromotedGroup.objects.get(api_name='pre_review') in addon.publicly_promoted_groups ) assert ( @@ -1929,7 +1929,7 @@ def test_unapproved_group_after_approval_promoted(self): PromotedGroup.objects.get(api_name='line') in addon.publicly_promoted_groups ) assert ( - PromotedGroup.objects.get(api_name='recommended') + PromotedGroup.objects.get(api_name='pre_review') in addon.publicly_promoted_groups ) assert ( diff --git a/src/olympia/addons/tests/test_serializers.py b/src/olympia/addons/tests/test_serializers.py index 17dbd47cb61d..ec526e5c39f3 100644 --- a/src/olympia/addons/tests/test_serializers.py +++ b/src/olympia/addons/tests/test_serializers.py @@ -614,8 +614,8 @@ def test_promoted(self): def test_promoted_shim(self): group, _ = PromotedGroup.objects.get_or_create( - api_name=RECOMMENDED_API_NAME, - name='Recommended', + api_name='promoted', + name='Promoted', listed_pre_review=True, ) self.addon = addon_factory() @@ -638,7 +638,7 @@ def test_promoted_shim(self): with override_settings(DRF_API_GATES=gates): result = self.serialize() promoted = result['promoted'] - assert promoted['category'] == RECOMMENDED_API_NAME + assert promoted['category'] == 'promoted' assert promoted['apps'] == [amo.FIREFOX.short] def test_private_promoted_api_hidden(self): diff --git a/src/olympia/addons/tests/test_views.py b/src/olympia/addons/tests/test_views.py index 49cb521d618e..1eaa8b6e9288 100644 --- a/src/olympia/addons/tests/test_views.py +++ b/src/olympia/addons/tests/test_views.py @@ -3531,7 +3531,7 @@ def test_compatibility_forbidden_range_android(self): # Recommended add-ons for Android don't have that restriction. self.make_addon_promoted( self.addon, - api_name='recommended', + api_name='all_fenix_versions', listed_pre_review=True, can_be_compatible_with_all_fenix_versions=True, approve_version=True, @@ -3559,7 +3559,7 @@ def test_compatibility_forbidden_range_android_only_min_specified(self): # Recommended add-ons for Android don't have that restriction. self.make_addon_promoted( self.addon, - api_name='recommended', + api_name='all_fenix_versions', listed_pre_review=True, can_be_compatible_with_all_fenix_versions=True, approve_version=True, @@ -4679,7 +4679,7 @@ def test_disable_enabled_version(self): def test_cannot_disable_if_promoted(self): self.make_addon_promoted( self.addon, - api_name='recommended', + api_name='pre_review', name='Recommended', listed_pre_review=True, badged=True, @@ -4842,7 +4842,7 @@ def test_author_developer_can_delete(self): def test_cannot_delete_if_promoted(self): self.make_addon_promoted( self.addon, - api_name='recommended', + api_name='pre_review', name='Recommended', listed_pre_review=True, badged=True, @@ -6327,7 +6327,7 @@ def test_filter_by_promoted(self): self.reindex(Addon) - data = self.perform_search(self.url, {'promoted': 'recommended'}) + data = self.perform_search(self.url, {'promoted': RECOMMENDED_API_NAME}) assert data['count'] == 4 assert len(data['results']) == 4 assert {res['id'] for res in data['results']} == { @@ -6339,7 +6339,7 @@ def test_filter_by_promoted(self): # And with app filtering too data = self.perform_search( - self.url, {'promoted': 'recommended', 'app': 'firefox'} + self.url, {'promoted': RECOMMENDED_API_NAME, 'app': 'firefox'} ) assert data['count'] == 3 assert len(data['results']) == 3 @@ -6351,7 +6351,7 @@ def test_filter_by_promoted(self): # That will filter out for a different app data = self.perform_search( - self.url, {'promoted': 'recommended', 'app': 'android'} + self.url, {'promoted': RECOMMENDED_API_NAME, 'app': 'android'} ) assert data['count'] == 2 assert len(data['results']) == 2 diff --git a/src/olympia/devhub/tests/test_forms.py b/src/olympia/devhub/tests/test_forms.py index 9096fdd99e66..e5c9a36dbe77 100644 --- a/src/olympia/devhub/tests/test_forms.py +++ b/src/olympia/devhub/tests/test_forms.py @@ -29,7 +29,6 @@ from olympia.amo.utils import rm_local_tmp_dir from olympia.api.models import APIKey, APIKeyConfirmation from olympia.applications.models import AppVersion -from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.devhub import forms from olympia.files.models import FileUpload from olympia.tags.models import AddonTag, Tag @@ -482,8 +481,7 @@ def test_fenix_range_not_disabled_for_recommended_android_extensions(self): ) self.make_addon_promoted( addon=self.addon, - api_name=RECOMMENDED_API_NAME, - name='Recommended', + api_name='all_fenix_versions', can_be_compatible_with_all_fenix_versions=True, approve_version=True, ) diff --git a/src/olympia/devhub/tests/test_views.py b/src/olympia/devhub/tests/test_views.py index 776a055837da..99cd14a87f28 100644 --- a/src/olympia/devhub/tests/test_views.py +++ b/src/olympia/devhub/tests/test_views.py @@ -41,7 +41,6 @@ from olympia.amo.tests.test_helpers import get_image_path from olympia.api.models import SYMMETRIC_JWT_TYPE, APIKey, APIKeyConfirmation from olympia.applications.models import AppVersion -from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.devhub.decorators import dev_required from olympia.devhub.forms import APIKeyForm, SupportForm from olympia.devhub.models import BlogPost, SurveyResponse @@ -337,7 +336,7 @@ def test_disabled_post_admin(self): def test_dev_promoted_status(self): self.make_addon_promoted( addon=self.addon, - api_name=RECOMMENDED_API_NAME, + api_name='badged', name='Recommended', badged=True, ) @@ -348,7 +347,7 @@ def test_dev_promoted_status(self): addon=self.addon, api_name='spotlight', name='Spotlight', badged=False ) self.addon.approve_for_version() - assert 'recommended' in self.addon.promoted_groups().api_name + assert 'badged' in self.addon.promoted_groups().api_name assert 'line' in self.addon.promoted_groups().api_name assert 'spotlight' in self.addon.promoted_groups().api_name @@ -574,7 +573,7 @@ def test_my_addons(self): def test_my_addons_recommended(self): self.make_addon_promoted( addon=self.addon, - api_name=RECOMMENDED_API_NAME, + api_name='badged', name='Recommended', badged=True, approve_version=True, diff --git a/src/olympia/devhub/tests/test_views_submit.py b/src/olympia/devhub/tests/test_views_submit.py index 227203389d87..160f7de7b15b 100644 --- a/src/olympia/devhub/tests/test_views_submit.py +++ b/src/olympia/devhub/tests/test_views_submit.py @@ -32,7 +32,7 @@ ) from olympia.constants.categories import CATEGORIES from olympia.constants.licenses import LICENSE_CC_COPYRIGHT -from olympia.constants.promoted import NOTABLE_API_NAME, RECOMMENDED_API_NAME +from olympia.constants.promoted import NOTABLE_API_NAME from olympia.devhub import views from olympia.files.tests.test_models import UploadMixin from olympia.files.utils import parse_addon @@ -2773,8 +2773,7 @@ def test_android_compatibility_modal(self): self.make_addon_promoted( addon=self.addon, - api_name=RECOMMENDED_API_NAME, - name='Recommended', + api_name='all_fenix_versions', can_be_compatible_with_all_fenix_versions=True, approve_version=True, ) diff --git a/src/olympia/devhub/tests/test_views_versions.py b/src/olympia/devhub/tests/test_views_versions.py index e8fa91a1451b..69f1da0abac2 100644 --- a/src/olympia/devhub/tests/test_views_versions.py +++ b/src/olympia/devhub/tests/test_views_versions.py @@ -29,7 +29,6 @@ ) from olympia.applications.models import AppVersion from olympia.blocklist.models import BlockType -from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.files.models import File from olympia.reviewers.models import AutoApprovalSummary from olympia.users.models import Group, UserProfile @@ -221,8 +220,7 @@ def test_cant_disable_or_delete_current_version_recommended(self): # version. self.make_addon_promoted( addon=self.addon, - api_name=RECOMMENDED_API_NAME, - name='Recommended', + api_name='pre_review', listed_pre_review=True, badged=True, approve_version=True, @@ -250,8 +248,7 @@ def test_can_disable_or_delete_current_ver_if_previous_recommended(self): # version if the previous version is approved for recommendation too. self.make_addon_promoted( addon=self.addon, - api_name=RECOMMENDED_API_NAME, - name='Recommended', + api_name='pre_review', listed_pre_review=True, approve_version=True, ) @@ -281,16 +278,15 @@ def test_can_disable_or_delete_current_ver_if_previous_recommended(self): self.addon.reload() assert self.addon.current_version == previous_version - # It's still recommended. - assert 'recommended' in self.addon.promoted_groups().api_name + # It's still promoted. + assert 'pre_review' in self.addon.promoted_groups().api_name def test_can_still_disable_or_delete_old_version_recommended(self): # If the add-on is recommended, you can still disable or delete older # versions than the current one. self.make_addon_promoted( addon=self.addon, - api_name=RECOMMENDED_API_NAME, - name='Recommended', + api_name='pre_review', listed_pre_review=True, approve_version=True, ) @@ -320,8 +316,7 @@ def test_can_still_disable_or_delete_current_version_unapproved(self): # then deleting the current version is fine. self.make_addon_promoted( addon=self.addon, - api_name=RECOMMENDED_API_NAME, - name='Recommended', + api_name='pre_review', listed_pre_review=True, ) assert self.version == self.addon.current_version diff --git a/src/olympia/hero/tests/test_models.py b/src/olympia/hero/tests/test_models.py index 6e3e88873e80..b26a15906806 100644 --- a/src/olympia/hero/tests/test_models.py +++ b/src/olympia/hero/tests/test_models.py @@ -46,7 +46,7 @@ def test_clean_requires_approved_can_primary_hero_group(self): # The error message below lists every active can_primary_hero group by # name, so create the three that can be added to a primary shelf. recommended_group, _ = PromotedGroup.objects.get_or_create( - api_name='recommended', + api_name=RECOMMENDED_API_NAME, name='Recommended', can_primary_hero=True, active=True, @@ -86,7 +86,7 @@ def test_clean_requires_approved_can_primary_hero_group(self): ph.addon.approve_for_version(ph.addon.current_version) ph.reload() ph.enabled = True - assert 'recommended' in ph.addon.promoted_groups().api_name + assert RECOMMENDED_API_NAME in ph.addon.promoted_groups().api_name ph.clean() # it raises if there's an error # change to a different group @@ -135,11 +135,11 @@ def test_clean_external_requires_homepage(self): def test_clean_gradient_and_image(self): # Currently, gradient is required and image isn't. addon = addon_factory() - recommended_group, _ = PromotedGroup.objects.get_or_create( - api_name=RECOMMENDED_API_NAME, name='Recommended', can_primary_hero=True + promoted_group, _ = PromotedGroup.objects.get_or_create( + api_name='primary_hero', name='Primary Hero', can_primary_hero=True ) PromotedAddon.objects.create( - addon=addon, promoted_group=recommended_group, application_id=amo.FIREFOX.id + addon=addon, promoted_group=promoted_group, application_id=amo.FIREFOX.id ) ph = PrimaryHero.objects.create(addon=addon) ph.addon.approve_for_version(ph.addon.current_version) @@ -164,7 +164,7 @@ def test_clean_gradient_and_image(self): def test_clean_only_enabled(self): addon = addon_factory() promoted_group, _ = PromotedGroup.objects.get_or_create( - api_name=RECOMMENDED_API_NAME, name='Recommended', can_primary_hero=True + api_name='primary_hero', name='Primary Hero', can_primary_hero=True ) PromotedAddon.objects.create( addon=addon, promoted_group=promoted_group, application_id=amo.FIREFOX.id diff --git a/src/olympia/hero/tests/test_serializers.py b/src/olympia/hero/tests/test_serializers.py index f44cb638354a..fcdc850cde0a 100644 --- a/src/olympia/hero/tests/test_serializers.py +++ b/src/olympia/hero/tests/test_serializers.py @@ -4,9 +4,6 @@ from olympia.amo.tests import TestCase, addon_factory from olympia.amo.tests.test_helpers import get_uploaded_file from olympia.amo.urlresolvers import get_outgoing_url -from olympia.constants.promoted import ( - RECOMMENDED_API_NAME, -) from ..models import ( GRADIENT_START_COLOR, @@ -36,8 +33,8 @@ def setUp(self): def test_basic(self): addon = addon_factory( promoted_kwargs={ - 'api_name': RECOMMENDED_API_NAME, - 'name': 'Recommended', + 'api_name': 'primary_hero', + 'name': 'Primary Hero', 'can_primary_hero': True, } ) @@ -57,7 +54,7 @@ def test_basic(self): assert data['addon']['promoted'] == [ { 'apps': [amo.FIREFOX.short, amo.ANDROID.short], - 'category': RECOMMENDED_API_NAME, + 'category': 'primary_hero', } ] diff --git a/src/olympia/lib/crypto/tests/test_signing.py b/src/olympia/lib/crypto/tests/test_signing.py index 9c61686559a0..18768de6f18e 100644 --- a/src/olympia/lib/crypto/tests/test_signing.py +++ b/src/olympia/lib/crypto/tests/test_signing.py @@ -28,7 +28,6 @@ version_factory, ) from olympia.amo.tests.test_helpers import get_addon_file -from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.lib.crypto import signing, tasks from olympia.versions.compare import VersionString, version_int @@ -444,7 +443,7 @@ def test_call_signing_promoted(self): def test_call_signing_promoted_recommended(self): self.make_addon_promoted( self.file_.version.addon, - api_name=RECOMMENDED_API_NAME, + api_name='signed_recommended', listed_pre_review=True, autograph_signing_states={ amo.FIREFOX.short: 'recommended', @@ -458,7 +457,7 @@ def test_call_signing_promoted_recommended(self): def test_call_signing_promoted_recommended_android_only(self): self.make_addon_promoted( self.file_.version.addon, - api_name=RECOMMENDED_API_NAME, + api_name='signed_recommended', listed_pre_review=True, autograph_signing_states={ amo.FIREFOX.short: 'recommended', @@ -475,7 +474,7 @@ def test_call_signing_promoted_unlisted(self): # never be signed as promoted. self.make_addon_promoted( self.file_.version.addon, - api_name=RECOMMENDED_API_NAME, + api_name='signed_recommended', listed_pre_review=True, autograph_signing_states={ amo.FIREFOX.short: 'recommended', @@ -687,7 +686,7 @@ def test_resign_bypass_name_checks(self, mock_resolve_message, mock_sign_file): def test_resign_carry_over_promotion(self, mock_sign_file): self.make_addon_promoted( self.addon, - api_name=RECOMMENDED_API_NAME, + api_name='pre_review', listed_pre_review=True, approve_version=True, ) @@ -705,7 +704,7 @@ def test_resign_carry_over_promotion(self, mock_sign_file): def test_resign_doesnt_carry_over_unapproved_promotion(self, mock_sign_file): self.make_addon_promoted( self.addon, - api_name=RECOMMENDED_API_NAME, + api_name='pre_review', listed_pre_review=True, approve_version=False, ) diff --git a/src/olympia/promoted/tests/test_admin.py b/src/olympia/promoted/tests/test_admin.py index e13d9c77fc39..203d3d17cfc3 100644 --- a/src/olympia/promoted/tests/test_admin.py +++ b/src/olympia/promoted/tests/test_admin.py @@ -10,6 +10,7 @@ version_factory, ) from olympia.amo.tests.test_helpers import get_uploaded_file +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.hero.models import PrimaryHero, PrimaryHeroImage from olympia.promoted.models import ( PromotedApproval, @@ -165,13 +166,13 @@ def test_list_filtering_by_promoted_group(self): addon = addon_factory(name='Present') PromotedAddon.objects.create( addon=addon, - promoted_group=PromotedGroup.objects.get(api_name='recommended'), + promoted_group=PromotedGroup.objects.get(api_name=RECOMMENDED_API_NAME), application_id=amo.FIREFOX.id, ) also_present = addon_factory(name='Also Present') PromotedAddon.objects.create( addon=also_present, - promoted_group=PromotedGroup.objects.get(api_name='recommended'), + promoted_group=PromotedGroup.objects.get(api_name=RECOMMENDED_API_NAME), application_id=amo.FIREFOX.id, ) PromotedAddon.objects.create( @@ -186,7 +187,7 @@ def test_list_filtering_by_promoted_group(self): filters = { # See DiscoveryAddonAdmin.list_filter 'promotedaddon__promoted_group__id__exact': ( - PromotedGroup.objects.get(api_name='recommended').pk + PromotedGroup.objects.get(api_name=RECOMMENDED_API_NAME).pk ) } response = self.client.get(self.list_url, filters, follow=True) @@ -220,7 +221,7 @@ def test_can_edit_with_discovery_edit_permission(self): addon = addon_factory() promotion = PromotedAddon.objects.create( addon=addon, - promoted_group=PromotedGroup.objects.get(api_name='recommended'), + promoted_group=PromotedGroup.objects.get(api_name=RECOMMENDED_API_NAME), application_id=amo.FIREFOX.id, ) ver1 = addon.current_version @@ -231,12 +232,12 @@ def test_can_edit_with_discovery_edit_permission(self): approvals = [ PromotedApproval.objects.create( version=ver1, - promoted_group=PromotedGroup.objects.get(api_name='recommended'), + promoted_group=PromotedGroup.objects.get(api_name=RECOMMENDED_API_NAME), application_id=amo.FIREFOX.id, ), PromotedApproval.objects.create( version=ver2, - promoted_group=PromotedGroup.objects.get(api_name='recommended'), + promoted_group=PromotedGroup.objects.get(api_name=RECOMMENDED_API_NAME), application_id=amo.ANDROID.id, ), PromotedApproval.objects.create( @@ -246,7 +247,7 @@ def test_can_edit_with_discovery_edit_permission(self): ), PromotedApproval.objects.create( version=ver3, - promoted_group=PromotedGroup.objects.get(api_name='recommended'), + promoted_group=PromotedGroup.objects.get(api_name=RECOMMENDED_API_NAME), application_id=amo.FIREFOX.id, ), ] @@ -312,13 +313,13 @@ def test_cannot_add_or_change_approval(self): addon = addon_factory() promotion = PromotedAddon.objects.create( addon=addon, - promoted_group=PromotedGroup.objects.get(api_name='recommended'), + promoted_group=PromotedGroup.objects.get(api_name=RECOMMENDED_API_NAME), application_id=amo.FIREFOX.id, ) ver1 = addon.current_version approval = PromotedApproval.objects.create( version=ver1, - promoted_group=PromotedGroup.objects.get(api_name='recommended'), + promoted_group=PromotedGroup.objects.get(api_name=RECOMMENDED_API_NAME), application_id=amo.FIREFOX.id, ) detail_url = reverse(self.detail_url_name, args=(addon.pk,)) @@ -341,7 +342,7 @@ def test_cannot_add_or_change_approval(self): follow=True, ) approval.reload() - assert approval.promoted_group.api_name == 'recommended' + assert approval.promoted_group.api_name == RECOMMENDED_API_NAME assert response.status_code == 200 assert PromotedAddon.objects.count() == 1 @@ -367,14 +368,14 @@ def test_cannot_edit_without_discovery_edit_permission(self): addon = addon_factory() promotion = PromotedAddon.objects.create( addon=addon, - promoted_group=PromotedGroup.objects.get(api_name='recommended'), + promoted_group=PromotedGroup.objects.get(api_name=RECOMMENDED_API_NAME), application_id=amo.FIREFOX.id, ) ver1 = addon.current_version approvals = [ PromotedApproval.objects.create( version=ver1, - promoted_group=PromotedGroup.objects.get(api_name='recommended'), + promoted_group=PromotedGroup.objects.get(api_name=RECOMMENDED_API_NAME), application_id=amo.FIREFOX.id, ), ] @@ -405,7 +406,7 @@ def test_cannot_edit_without_discovery_edit_permission(self): promotion.reload() assert PromotedAddon.objects.count() == 1 - assert promotion.promoted_group.api_name == 'recommended' + assert promotion.promoted_group.api_name == RECOMMENDED_API_NAME assert PromotedApproval.objects.count() == 1 # Try to delete the approval instead @@ -427,12 +428,12 @@ def test_can_delete_with_discovery_edit_permission(self): addon = addon_factory() promotion = PromotedAddon.objects.create( addon=addon, - promoted_group=PromotedGroup.objects.get(api_name='recommended'), + promoted_group=PromotedGroup.objects.get(api_name=RECOMMENDED_API_NAME), application_id=amo.FIREFOX.id, ) PromotedApproval.objects.create( version=addon.current_version, - promoted_group=PromotedGroup.objects.get(api_name='recommended'), + promoted_group=PromotedGroup.objects.get(api_name=RECOMMENDED_API_NAME), application_id=amo.FIREFOX.id, ) detail_url = reverse(self.detail_url_name, args=(addon.pk,)) @@ -462,7 +463,7 @@ def test_cannot_delete_without_discovery_edit_permission(self): addon = addon_factory() promotion = PromotedAddon.objects.create( addon=addon, - promoted_group=PromotedGroup.objects.get(api_name='recommended'), + promoted_group=PromotedGroup.objects.get(api_name=RECOMMENDED_API_NAME), application_id=amo.FIREFOX.id, ) detail_url = reverse(self.detail_url_name, args=(addon.pk,)) @@ -492,7 +493,7 @@ def test_can_add_with_discovery_edit_permission(self): self.grant_permission(user, 'Discovery:Edit') self.client.force_login(user) # create an approval that doesn't have a matching PromotedAddon yet - group = PromotedGroup.objects.get(api_name='recommended') + group = PromotedGroup.objects.get(api_name=RECOMMENDED_API_NAME) response = self.client.get(detail_url, follow=True) assert response.status_code == 200 assert PromotedAddon.objects.count() == 0 @@ -512,7 +513,7 @@ def test_can_add_with_discovery_edit_permission(self): assert PromotedAddon.objects.count() == 1 item = PromotedAddon.objects.get() assert item.addon == addon - assert item.promoted_group.api_name == 'recommended' + assert item.promoted_group.api_name == RECOMMENDED_API_NAME assert item.application_id == amo.FIREFOX.id assert item.application == amo.FIREFOX assert PromotedApproval.objects.count() == 0 # we didn't create any approvals diff --git a/src/olympia/promoted/tests/test_models.py b/src/olympia/promoted/tests/test_models.py index 1e8ab9e88f0e..446abfda87e4 100644 --- a/src/olympia/promoted/tests/test_models.py +++ b/src/olympia/promoted/tests/test_models.py @@ -3,6 +3,7 @@ from olympia.addons.models import Addon from olympia.amo.tests import TestCase, addon_factory, version_factory from olympia.constants import applications +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.promoted.models import ( PromotedAddon, PromotedApproval, @@ -80,7 +81,7 @@ def setUp(self): ) self.promotion2 = PromotedAddon.objects.create( addon=self.addon, - promoted_group=PromotedGroup.objects.get(api_name='recommended'), + promoted_group=PromotedGroup.objects.get(api_name=RECOMMENDED_API_NAME), application_id=applications.FIREFOX.id, ) diff --git a/src/olympia/reviewers/tests/test_cron.py b/src/olympia/reviewers/tests/test_cron.py index 3b4d7bed4160..59da256ff24d 100644 --- a/src/olympia/reviewers/tests/test_cron.py +++ b/src/olympia/reviewers/tests/test_cron.py @@ -46,10 +46,10 @@ def test_basic(self): 'reason': NeedsHumanReview.REASONS.AUTO_APPROVAL_DISABLED }, ) - self.addon_recommended_1 = addon_factory( + self.addon_pre_review_1 = addon_factory( file_kw={'status': amo.STATUS_AWAITING_REVIEW}, promoted_kwargs={ - 'api_name': 'recommended', + 'api_name': 'pre_review', 'listed_pre_review': True, 'active': True, }, @@ -60,7 +60,7 @@ def test_basic(self): addon_factory( file_kw={'status': amo.STATUS_AWAITING_REVIEW}, promoted_kwargs={ - 'api_name': 'recommended', + 'api_name': 'pre_review', 'listed_pre_review': True, 'active': True, }, @@ -90,7 +90,7 @@ def test_basic(self): assert metric.date == expected_date assert metric.value == 5 - metric = QueueCount.objects.get(name='queue_extension/recommended') + metric = QueueCount.objects.get(name='queue_extension/pre_review') assert metric.date == expected_date assert metric.value == 2 @@ -106,8 +106,8 @@ def test_twice_different_day(self): self.test_basic() previous_date = QueueCount.objects.latest('pk').date - self.addon_recommended_1.current_version.file.update(status=amo.STATUS_APPROVED) - self.addon_recommended_1.current_version.needshumanreview_set.all()[0].update( + self.addon_pre_review_1.current_version.file.update(status=amo.STATUS_APPROVED) + self.addon_pre_review_1.current_version.needshumanreview_set.all()[0].update( is_active=False ) @@ -123,7 +123,7 @@ def test_twice_different_day(self): ) assert ( QueueCount.objects.get( - date=previous_date, name='queue_extension/recommended' + date=previous_date, name='queue_extension/pre_review' ).value == 2 ) @@ -143,10 +143,10 @@ def test_twice_different_day(self): == 4 ) - # One fewer add-on in the queue that was recommended. + # One fewer add-on in the queue that was pre-review. assert ( QueueCount.objects.get( - date=expected_date, name='queue_extension/recommended' + date=expected_date, name='queue_extension/pre_review' ).value == 1 ) diff --git a/src/olympia/reviewers/tests/test_models.py b/src/olympia/reviewers/tests/test_models.py index 93ca351c84c8..7d9373fa71df 100644 --- a/src/olympia/reviewers/tests/test_models.py +++ b/src/olympia/reviewers/tests/test_models.py @@ -1174,7 +1174,7 @@ def test_check_is_promoted_prereview(self): assert AutoApprovalSummary.check_is_promoted_prereview(self.version) is False self.make_addon_promoted( - addon=self.addon, api_name='recommended', listed_pre_review=True + addon=self.addon, api_name='pre_review', listed_pre_review=True ) assert AutoApprovalSummary.check_is_promoted_prereview(self.version) is True diff --git a/src/olympia/reviewers/tests/test_utils.py b/src/olympia/reviewers/tests/test_utils.py index 33b2fccc3c20..7d928c77d68d 100644 --- a/src/olympia/reviewers/tests/test_utils.py +++ b/src/olympia/reviewers/tests/test_utils.py @@ -38,6 +38,7 @@ from olympia.amo.utils import send_mail from olympia.blocklist.models import Block, BlocklistSubmission from olympia.constants.abuse import DECISION_ACTIONS +from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.files.models import File from olympia.lib.crypto.signing import SigningError from olympia.lib.crypto.tests.test_signing import ( @@ -315,12 +316,12 @@ def test_actions_public_post_review(self): == expected ) - # Now add a recommended promoted addon. It's a listed pre-review group - # so the individual approve/reject actions aren't offered, but the + # Now add a listed pre-review promoted addon. It's a listed pre-review + # group so the individual approve/reject actions aren't offered, but the # multiple-versions and confirm actions still are for an # Addons:Review holder. self.make_addon_promoted( - self.addon, api_name='recommended', listed_pre_review=True, badged=True + self.addon, api_name='pre_review', listed_pre_review=True, badged=True ) expected = [ 'confirm_auto_approved', @@ -3471,16 +3472,16 @@ def test_reject_listing_content_review(self): in message.body ) - def test_nominated_to_approved_recommended(self): + def test_nominated_to_approved_pre_review(self): self.make_addon_promoted( - self.addon, api_name='recommended', listed_pre_review=True + self.addon, api_name='pre_review', listed_pre_review=True ) assert not self.addon.promoted_groups() self.test_nomination_to_public() assert self.addon.current_version.promoted_versions.filter( - promoted_group__api_name='recommended' + promoted_group__api_name='pre_review' ).exists() - assert 'recommended' in self.addon.promoted_groups().api_name + assert 'pre_review' in self.addon.promoted_groups().api_name def test_nominated_to_approved_other_promoted(self): self.make_addon_promoted(self.addon, api_name='line', listed_pre_review=True) @@ -3491,16 +3492,16 @@ def test_nominated_to_approved_other_promoted(self): ).exists() assert 'line' in self.addon.promoted_groups().api_name - def test_approved_update_recommended(self): + def test_approved_update_pre_review(self): self.make_addon_promoted( - self.addon, api_name='recommended', listed_pre_review=True + self.addon, api_name='pre_review', listed_pre_review=True ) assert not self.addon.promoted_groups() self.test_public_addon_with_version_awaiting_review_to_public() assert self.addon.current_version.promoted_versions.filter( - promoted_group__api_name='recommended' + promoted_group__api_name='pre_review' ).exists() - assert 'recommended' in self.addon.promoted_groups().api_name + assert 'pre_review' in self.addon.promoted_groups().api_name def test_approved_update_other_promoted(self): self.make_addon_promoted(self.addon, api_name='line', listed_pre_review=True) @@ -4925,7 +4926,7 @@ def test_nominated_to_public_recommended(self): self.make_addon_promoted( self.addon, - api_name='recommended', + api_name=RECOMMENDED_API_NAME, listed_pre_review=True, autograph_signing_states={ 'firefox': 'recommended', @@ -4940,9 +4941,9 @@ def test_nominated_to_public_recommended(self): assert self.addon.versions.all()[0].file.status == (amo.STATUS_APPROVED) assert self.addon.current_version.promoted_versions.filter( - promoted_group__api_name='recommended' + promoted_group__api_name=RECOMMENDED_API_NAME ).exists() - assert 'recommended' in self.addon.promoted_groups().api_name + assert RECOMMENDED_API_NAME in self.addon.promoted_groups().api_name signature_info, manifest = _get_signature_details(self.file.file.path) diff --git a/src/olympia/reviewers/tests/test_views.py b/src/olympia/reviewers/tests/test_views.py index a06dfdcc7586..e5ce4f593c57 100644 --- a/src/olympia/reviewers/tests/test_views.py +++ b/src/olympia/reviewers/tests/test_views.py @@ -640,7 +640,7 @@ def test_super_admin_all_permissions(self): user_factory(pk=settings.TASK_USER_ID) # Recommended extensions version = addon_factory( - promoted_kwargs={'api_name': 'recommended', 'listed_pre_review': True}, + promoted_kwargs={'api_name': 'pre_review', 'listed_pre_review': True}, status=amo.STATUS_NOMINATED, file_kw={'status': amo.STATUS_AWAITING_REVIEW}, ).versions.get() @@ -650,7 +650,7 @@ def test_super_admin_all_permissions(self): version = version_factory( addon=addon_factory( promoted_kwargs={ - 'api_name': 'recommended', + 'api_name': 'pre_review', 'listed_pre_review': True, }, version_kw={'promotion_approved': False}, @@ -4337,7 +4337,7 @@ def test_translations(self): assert translations == expected @mock.patch('olympia.reviewers.utils.sign_file') - def test_approve_recommended_addon(self, mock_sign_file): + def test_approve_pre_review_addon(self, mock_sign_file): policy = CinderPolicy.objects.create( uuid='1', name='policy 1', @@ -4348,7 +4348,7 @@ def test_approve_recommended_addon(self, mock_sign_file): self.addon.update(status=amo.STATUS_NOMINATED) self.make_addon_promoted( self.addon, - api_name='recommended', + api_name='pre_review', listed_pre_review=True, badged=True, ) @@ -4368,7 +4368,7 @@ def test_approve_recommended_addon(self, mock_sign_file): assert addon.current_version assert addon.current_version.file.status == amo.STATUS_APPROVED assert addon.current_version.promoted_versions.filter( - promoted_group__api_name='recommended' + promoted_group__api_name='pre_review' ).exists() assert mock_sign_file.called diff --git a/src/olympia/scanners/tests/test_actions.py b/src/olympia/scanners/tests/test_actions.py index b224af84c63d..a52bf87cd9c0 100644 --- a/src/olympia/scanners/tests/test_actions.py +++ b/src/olympia/scanners/tests/test_actions.py @@ -21,7 +21,6 @@ ) from olympia.blocklist.models import Block, BlocklistSubmission, BlockType, BlockVersion from olympia.constants.abuse import DECISION_ACTIONS -from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.constants.scanners import ( DELAY_AUTO_APPROVAL, DELAY_AUTO_APPROVAL_INDEFINITELY, @@ -1003,7 +1002,7 @@ def test_disable_and_block_second_level_approval(self): addon = addon_factory( average_daily_users=4242, promoted_kwargs={ - 'api_name': RECOMMENDED_API_NAME, + 'api_name': 'high_profile', 'high_profile': True, }, version_kw={'promotion_approved': False}, @@ -1326,7 +1325,7 @@ def test_execute_workflow_action_skips_ex_promoted_if_addon_is_promoted( self, flag_for_human_review_mock, no_action_mock ): self.make_addon_promoted( - self.addon, api_name=RECOMMENDED_API_NAME, listed_pre_review=True + self.addon, api_name='pre_review', listed_pre_review=True ) # Create another rule and add it to the current scanner result, but set to # exclude_promoted_addons=True. @@ -1790,10 +1789,8 @@ def test_execute_policy_enforcement_action_2nd_level_approval(self): enforcement_actions=[DECISION_ACTIONS.AMO_DISABLE_ADDON.api_value], ) self.scanner_rule.update(policy=policy) - # Make the add-on recommended, that will force 2nd level approval. - self.make_addon_promoted( - self.addon, api_name=RECOMMENDED_API_NAME, high_profile=True - ) + # Make the add-on high-profile, that will force 2nd level approval. + self.make_addon_promoted(self.addon, api_name='high_profile', high_profile=True) ScannerResult.run_actions(self.version) diff --git a/src/olympia/scanners/tests/test_tasks.py b/src/olympia/scanners/tests/test_tasks.py index cf6b0b809a87..61f4926d2fba 100644 --- a/src/olympia/scanners/tests/test_tasks.py +++ b/src/olympia/scanners/tests/test_tasks.py @@ -17,7 +17,7 @@ user_factory, version_factory, ) -from olympia.constants.promoted import NOTABLE_API_NAME, RECOMMENDED_API_NAME +from olympia.constants.promoted import NOTABLE_API_NAME from olympia.constants.scanners import ( ABORTED, ABORTING, @@ -2202,7 +2202,7 @@ def test_run_on_chunk_not_blocked(self): def test_run_on_chunk_was_promoted(self): self.rule.update(state=RUNNING) # Pretend we started running the rule. self.make_addon_promoted( - self.version.addon, api_name=RECOMMENDED_API_NAME, listed_pre_review=True + self.version.addon, api_name='pre_review', listed_pre_review=True ) run_scanner_query_rule_on_versions_chunk([self.version.pk], self.rule.pk) diff --git a/src/olympia/search/tests/test_filters.py b/src/olympia/search/tests/test_filters.py index c1c78ee14ed7..891bfe2f0b15 100644 --- a/src/olympia/search/tests/test_filters.py +++ b/src/olympia/search/tests/test_filters.py @@ -13,7 +13,7 @@ from olympia import amo from olympia.constants.categories import CATEGORIES -from olympia.constants.promoted import BADGED_GROUPS +from olympia.constants.promoted import BADGED_GROUPS, RECOMMENDED_API_NAME from olympia.search.filters import ( AddonCreatedQueryParam, AddonRatingQueryParam, @@ -581,7 +581,7 @@ def test_sort_random_seed(self): @time_machine.travel('2023-12-24', tick=False) def test_sort_random(self): - qs = self._filter(data={'promoted': 'recommended', 'sort': 'random'}) + qs = self._filter(data={'promoted': RECOMMENDED_API_NAME, 'sort': 'random'}) # Note: this test does not call AddonPromotedQueryParam so it won't # apply the recommended filtering. That's tested below in # TestCombinedFilter.test_filter_promoted_sort_random @@ -931,12 +931,14 @@ def test_search_by_promoted(self): assert {'term': {'promoted.approved_for_apps': amo.FIREFOX.id}} in filter_ # test multiple param values - qs = self._filter(data={'promoted': 'recommended,line'}) + qs = self._filter(data={'promoted': f'{RECOMMENDED_API_NAME},line'}) filter_ = qs['query']['bool']['filter'] - assert [{'terms': {'promoted.category': ['line', 'recommended']}}] == filter_ + assert [ + {'terms': {'promoted.category': ['line', RECOMMENDED_API_NAME]}} + ] == filter_ # test combining multiple values with the meta "badged" group - qs = self._filter(data={'promoted': 'badged,recommended,strategic'}) + qs = self._filter(data={'promoted': f'badged,{RECOMMENDED_API_NAME},strategic'}) filter_ = qs['query']['bool']['filter'] assert [ { @@ -944,7 +946,7 @@ def test_search_by_promoted(self): 'promoted.category': [ # recommended shouldn't be there twice 'line', - 'recommended', + RECOMMENDED_API_NAME, 'strategic', ] } diff --git a/src/olympia/search/tests/test_search_ranking.py b/src/olympia/search/tests/test_search_ranking.py index f1d8dae2c425..e112c86bb8fd 100644 --- a/src/olympia/search/tests/test_search_ranking.py +++ b/src/olympia/search/tests/test_search_ranking.py @@ -5,7 +5,6 @@ from olympia import amo from olympia.amo.tests import APITestClientSessionID, ESTestCase, reverse_ns -from olympia.constants.promoted import RECOMMENDED_API_NAME from olympia.constants.search import SEARCH_LANGUAGE_TO_ANALYZER @@ -465,7 +464,7 @@ def setUpTestData(cls): summary='A new friend in every new tab.', weekly_downloads=350, promoted_kwargs={ - 'api_name': RECOMMENDED_API_NAME, + 'api_name': 'search_bump', 'listed_pre_review': True, 'search_ranking_bump': 5.0, }, @@ -627,7 +626,7 @@ def setUpTestData(cls): summary='A new friend in every new window.', weekly_downloads=350, promoted_kwargs={ - 'api_name': RECOMMENDED_API_NAME, + 'api_name': 'search_bump', 'listed_pre_review': True, 'search_ranking_bump': 5.0, }, diff --git a/src/olympia/shelves/tests/test_serializers.py b/src/olympia/shelves/tests/test_serializers.py index ca484da20a87..9a4bb62786d7 100644 --- a/src/olympia/shelves/tests/test_serializers.py +++ b/src/olympia/shelves/tests/test_serializers.py @@ -121,7 +121,7 @@ def setUp(self): title='Recommended extensions', endpoint='search', addon_type=amo.ADDON_EXTENSION, - criteria='?promoted=recommended&sort=random&type=extension', + criteria=f'?promoted={RECOMMENDED_API_NAME}&sort=random&type=extension', footer_text='See more recommended extensions', footer_pathname='/somewhere', ) @@ -204,7 +204,7 @@ def test_url_and_addons_search(self): assert pop_data['addons'][0]['type'] == 'statictheme' assert pop_data['addons'][1]['name'] == {'en-US': 'test addon test04'} - assert pop_data['addons'][1]['promoted'][0]['category'] == ('recommended') + assert pop_data['addons'][1]['promoted'][0]['category'] == RECOMMENDED_API_NAME assert pop_data['addons'][1]['type'] == 'statictheme' # Test 'Recommended Extensions' shelf - should include 1 addon @@ -213,7 +213,7 @@ def test_url_and_addons_search(self): assert len(rec_data['addons']) == 1 assert rec_data['addons'][0]['name'] == {'en-US': 'test addon test03'} - assert rec_data['addons'][0]['promoted'][0]['category'] == ('recommended') + assert rec_data['addons'][0]['promoted'][0]['category'] == RECOMMENDED_API_NAME assert rec_data['addons'][0]['type'] == 'extension' def test_url_and_addons_collections(self): @@ -243,7 +243,7 @@ def test_footer_url(self): url = ( 'http://testserver' + reverse('search.search') - + '?promoted=recommended&sort=random&type=extension' + + f'?promoted={RECOMMENDED_API_NAME}&sort=random&type=extension' ) assert data['footer'] == { 'url': url, diff --git a/src/olympia/versions/tests/test_commands.py b/src/olympia/versions/tests/test_commands.py index b53371cfe58a..617ffb52d622 100644 --- a/src/olympia/versions/tests/test_commands.py +++ b/src/olympia/versions/tests/test_commands.py @@ -83,7 +83,7 @@ def test_full(self): 'max_app_version': '*', }, promoted_kwargs={ - 'api_name': 'recommended', + 'api_name': 'all_fenix_versions', 'can_be_compatible_with_all_fenix_versions': True, }, ), @@ -231,7 +231,7 @@ def test_basic(self): 'max_app_version': '*', }, promoted_kwargs={ - 'api_name': 'recommended', + 'api_name': 'all_fenix_versions', 'can_be_compatible_with_all_fenix_versions': True, }, ) diff --git a/src/olympia/versions/tests/test_models.py b/src/olympia/versions/tests/test_models.py index 6ee58ae834f4..2f401b9c9bf0 100644 --- a/src/olympia/versions/tests/test_models.py +++ b/src/olympia/versions/tests/test_models.py @@ -439,7 +439,7 @@ def test_should_have_due_date(self): # Or if it's in a pre-review promoted group it will. recommended = addon_factory(**addon_kws).current_version self.make_addon_promoted( - addon=recommended.addon, api_name='recommended', listed_pre_review=True + addon=recommended.addon, api_name='pre_review', listed_pre_review=True ) NeedsHumanReview.objects.create( version=recommended, @@ -1414,13 +1414,13 @@ def test_promoted_can_be_disabled_and_deleted(self): self.make_addon_promoted( addon, - api_name='recommended', + api_name='badged_pre_review', listed_pre_review=True, badged=True, approve_version=True, ) addon = addon.reload() - assert 'recommended' in addon.promoted_groups().api_name + assert 'badged_pre_review' in addon.promoted_groups().api_name # But a promoted one, that's in a prereview group, can't be disabled assert not addon.current_version.can_be_disabled_and_deleted() @@ -1429,10 +1429,10 @@ def test_promoted_can_be_disabled_and_deleted(self): addon = addon.reload() assert previous_version != addon.current_version assert addon.current_version.promoted_versions.filter( - promoted_group__api_name='recommended' + promoted_group__api_name='badged_pre_review' ).exists() assert previous_version.promoted_versions.filter( - promoted_group__api_name='recommended' + promoted_group__api_name='badged_pre_review' ).exists() # unless the previous version is also approved for the same group assert addon.current_version.can_be_disabled_and_deleted() @@ -1446,7 +1446,7 @@ def test_promoted_can_be_disabled_and_deleted(self): previous_version.promoted_versions.update(promoted_group=line_group) assert not addon.current_version.can_be_disabled_and_deleted() previous_version.promoted_versions.update( - promoted_group=PromotedGroup.objects.get(api_name='recommended') + promoted_group=PromotedGroup.objects.get(api_name='badged_pre_review') ) # Check the scenario when some of the previous versions are approved @@ -1461,30 +1461,30 @@ def test_promoted_can_be_disabled_and_deleted(self): version_b = version_b.reload() assert version_d == addon.current_version assert version_a.promoted_versions.filter( - promoted_group__api_name='recommended' + promoted_group__api_name='badged_pre_review' ).exists() assert version_b.promoted_versions.filter( - promoted_group__api_name='recommended' + promoted_group__api_name='badged_pre_review' ).exists() # ignored because disabled assert not version_c.promoted_versions.filter( - promoted_group__api_name='recommended' + promoted_group__api_name='badged_pre_review' ).exists() assert version_d.promoted_versions.filter( - promoted_group__api_name='recommended' + promoted_group__api_name='badged_pre_review' ).exists() assert version_a.can_be_disabled_and_deleted() assert version_b.can_be_disabled_and_deleted() assert version_c.can_be_disabled_and_deleted() assert version_d.can_be_disabled_and_deleted() - assert 'recommended' in addon.promoted_groups().api_name + assert 'badged_pre_review' in addon.promoted_groups().api_name # now un-approve version_b version_b.promoted_versions.all().delete() assert version_a.can_be_disabled_and_deleted() assert version_b.can_be_disabled_and_deleted() assert version_c.can_be_disabled_and_deleted() assert not version_d.can_be_disabled_and_deleted() - assert 'recommended' in addon.promoted_groups().api_name + assert 'badged_pre_review' in addon.promoted_groups().api_name def test_unbadged_non_prereview_promoted_can_be_disabled_and_deleted(self): addon = Addon.objects.get(id=3615) @@ -1518,7 +1518,7 @@ def test_can_be_disabled_and_deleted_querycount(self): version_factory(addon=addon) self.make_addon_promoted( addon, - api_name='recommended', + api_name='badged_pre_review', listed_pre_review=True, badged=True, approve_version=True, @@ -3203,7 +3203,7 @@ def test_not_extension(self): def test_recommended_for_android(self): addon = addon_factory( promoted_kwargs={ - 'api_name': 'recommended', + 'api_name': 'all_fenix_versions', 'can_be_compatible_with_all_fenix_versions': True, } ) @@ -3253,7 +3253,7 @@ def test_other_promotion_for_android(self): def test_recommended_or_line_for_desktop(self): addon = addon_factory( promoted_kwargs={ - 'api_name': 'recommended', + 'api_name': 'all_fenix_versions', 'listed_pre_review': True, 'can_be_compatible_with_all_fenix_versions': True, } From f246e1abf71a65c49f45c4eb58bf036819e2bb99 Mon Sep 17 00:00:00 2001 From: eviljeff Date: Wed, 15 Jul 2026 11:05:46 +0100 Subject: [PATCH 14/14] restore test_need_correct_reviewer_for_promoted_addon --- src/olympia/reviewers/tests/test_views.py | 42 +++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/olympia/reviewers/tests/test_views.py b/src/olympia/reviewers/tests/test_views.py index e5ce4f593c57..ac28f97c944c 100644 --- a/src/olympia/reviewers/tests/test_views.py +++ b/src/olympia/reviewers/tests/test_views.py @@ -2530,6 +2530,48 @@ def test_dont_need_unlisted_reviewer_for_mixed_channels(self): self.grant_permission(self.reviewer, 'ReviewerTools:ViewUnlisted') assert self.client.head(self.url).status_code == 200 + def test_need_correct_reviewer_for_promoted_addon(self): + self.file.update(status=amo.STATUS_AWAITING_REVIEW) + self.make_addon_promoted( + self.addon, name='Admin Only', admin_review=True, listed_pre_review=True + ) + + response = self.client.get(self.url) + assert response.status_code == 200 + choices = list(dict(response.context['form'].fields['action'].choices).keys()) + expected_choices = ['comment'] + assert choices == expected_choices + + doc = pq(response.content) + assert doc('.is_promoted') + for entry in doc('.is_promoted').items(): + assert entry.text() == ( + "This is a Admin Only add-on. You don't have permission to review it." + ) + + self.grant_permission(self.reviewer, 'Reviews:Admin') + response = self.client.get(self.url) + assert response.status_code == 200 + choices = list(dict(response.context['form'].fields['action'].choices).keys()) + expected_choices = [ + 'public', + 'reject', + 'reject_multiple_versions', + 'change_or_clear_pending_rejection_multiple_versions', + 'clear_needs_human_review_multiple_versions', + 'set_needs_human_review_multiple_versions', + 'reply', + 'disable_addon', + 'request_legal_review', + 'comment', + ] + assert choices == expected_choices + + doc = pq(response.content) + assert doc('.is_promoted') + for entry in doc('.is_promoted').items(): + assert entry.text() == ('This is a Admin Only add-on.') + def test_not_recommendable(self): response = self.client.get(self.url) assert response.status_code == 200