diff --git a/src/olympia/abuse/actions.py b/src/olympia/abuse/actions.py index fc9bd9d2fd8..31f7cd0e2e6 100644 --- a/src/olympia/abuse/actions.py +++ b/src/olympia/abuse/actions.py @@ -8,6 +8,7 @@ from django.utils.safestring import mark_safe from django.utils.translation import gettext_lazy as _ +from olympia.constants.promoted import HIGH_PROFILE, HIGH_PROFILE_RATING import waffle import olympia @@ -250,7 +251,7 @@ def should_hold_action(self): or self.target.groups_list # has any permissions # owns a high profile add-on or any( - addon.promoted_group().high_profile + addon.get(HIGH_PROFILE) for addon in self.target.addons.all() ) ) @@ -281,7 +282,7 @@ def should_hold_action(self): return bool( self.target.status != amo.STATUS_DISABLED # is a high profile add-on - and self.target.promoted_group().high_profile + and self.target.get(HIGH_PROFILE) ) def process_action(self): @@ -369,7 +370,7 @@ def should_hold_action(self): return bool( not self.target.deleted and self.target.reply_to - and self.target.addon.promoted_group().high_profile_rating + and self.target.addon.get(HIGH_PROFILE_RATING) ) def process_action(self): diff --git a/src/olympia/abuse/cinder.py b/src/olympia/abuse/cinder.py index 5660bd90c64..0dae13c3ca2 100644 --- a/src/olympia/abuse/cinder.py +++ b/src/olympia/abuse/cinder.py @@ -331,7 +331,7 @@ def get_attributes(self): # promoted in any way, but we don't care about the promotion being # approved for the current version, it would make more queries and it's # not useful for moderation purposes anyway. - promoted_group = self.addon.promoted_group(currently_approved=False) + group_name = self.addon.group_name(currently_approved=False) data = { 'id': self.id, 'average_daily_users': self.addon.average_daily_users, @@ -341,7 +341,7 @@ def get_attributes(self): 'name': self.get_str(self.addon.name), 'slug': self.addon.slug, 'summary': self.get_str(self.addon.summary), - 'promoted': self.get_str(promoted_group.name if promoted_group else ''), + 'promoted': self.get_str(group_name), } return data diff --git a/src/olympia/addons/indexers.py b/src/olympia/addons/indexers.py index bfbf7d37503..2d9dc8b51c0 100644 --- a/src/olympia/addons/indexers.py +++ b/src/olympia/addons/indexers.py @@ -662,7 +662,7 @@ def extract_document(cls, obj): data['has_privacy_policy'] = bool(obj.privacy_policy) data['is_recommended'] = bool( - obj.promoted and obj.promoted.group == RECOMMENDED + RECOMMENDED in obj.promoted_group() ) data['previews'] = [ @@ -677,10 +677,10 @@ def extract_document(cls, obj): data['promoted'] = ( { - 'group_id': obj.promoted.group_id, + 'group_ids': obj.group_ids, # store the app approvals because .approved_applications needs it. 'approved_for_apps': [ - app.id for app in obj.promoted.approved_applications + app.id for app in obj.approved_applications ], } if obj.promoted diff --git a/src/olympia/addons/models.py b/src/olympia/addons/models.py index 8125e9ce767..acdf277407d 100644 --- a/src/olympia/addons/models.py +++ b/src/olympia/addons/models.py @@ -57,7 +57,7 @@ ) from olympia.constants.browsers import BROWSERS from olympia.constants.categories import CATEGORIES_BY_ID -from olympia.constants.promoted import NOT_PROMOTED, RECOMMENDED +from olympia.constants.promoted import CAN_BE_COMPATIBLE_WITH_ALL_FENIX_VERSIONS, NOT_PROMOTED, RECOMMENDED, PromotedClass from olympia.constants.reviewers import REPUTATION_CHOICES from olympia.files.models import File from olympia.files.utils import extract_translations, resolve_i18n_message @@ -279,7 +279,6 @@ def get_base_queryset_for_queue( select_related_fields = [ 'reviewerflags', 'addonapprovalscounter', - 'promotedaddon', ] if select_related_fields_for_listed: # Most listed queues need these to avoid extra queries because @@ -1556,8 +1555,7 @@ def _is_recommended_theme(self): def promoted_group(self, *, currently_approved=True): """Is the addon currently promoted for the current applications? - Returns the group constant, or NOT_PROMOTED (which is falsey) - otherwise. + Returns the list of group constants. `currently_approved=True` means only returns True if self.current_version is approved for the current promotion & apps. @@ -1567,22 +1565,72 @@ def promoted_group(self, *, currently_approved=True): from olympia.promoted.models import PromotedAddon try: - promoted = self.promotedaddon + promoted_addons = self.promoted_addons.all() except PromotedAddon.DoesNotExist: - return NOT_PROMOTED - is_promoted = not currently_approved or promoted.approved_applications - return promoted.group if is_promoted else NOT_PROMOTED + return [] + + return [promoted.group for promoted in promoted_addons if not currently_approved or promoted.approved_applications] + + def group_name(self, *, currently_approved=True): + """ Returns the string name of the currently groups, comma separated. + + `currently_approved=True` means only returns True if + self.current_version is approved for the current promotion & apps. + If currently_approved=False then promotions where there isn't approval + are returned too. + """ + groups = self.promoted_group(currently_approved=currently_approved) + return ', '.join(group.name for group in groups) + + def get(self, permission, currently_approved=True): + """ Fetch the given permission. + + Based on the type of the permission, returns -- + Bool -> If any group is true + Int -> The maximum value from the groups + Dict -> return the first truthy value, or {} if none. + + `currently_approved=True` means only returns True if + self.current_version is approved for the current promotion & apps. + If currently_approved=False then promotions where there isn't approval + are returned too. + """ + groups = self.promoted_group(currently_approved=currently_approved) + type = PromotedClass.type(permission) + + if type == int: + return max(getattr(group, permission) for group in groups if getattr(group, permission) is not None) + if type == bool: + return any(getattr(group, permission, False) for group in groups) + + for group in groups: + value = getattr(group, permission, None) + if value: + return value + return {} + + @property + def group_ids(self): + groups = self.promoted_group() + return [group.id for group in groups] + + @property + def approved_applications(self): + approved_apps = set() + for promoted in self.promoted_addons.all(): + approved_apps.update(promoted.approved_applications) + return approved_apps @cached_property def promoted(self): promoted_group = self.promoted_group() if promoted_group: - return self.promotedaddon + return self.promoted_addons else: from olympia.promoted.models import PromotedTheme if self._is_recommended_theme(): - return PromotedTheme(addon=self, group_id=RECOMMENDED.id) + return [PromotedTheme(addon=self, group_id=RECOMMENDED.id)] return None @cached_property @@ -1608,7 +1656,7 @@ def can_be_compatible_with_all_fenix_versions(self): versions (i.e. it's a recommended/line extension for Android).""" return ( self.promoted - and self.promoted.group.can_be_compatible_with_all_fenix_versions + and self.get(CAN_BE_COMPATIBLE_WITH_ALL_FENIX_VERSIONS) and amo.ANDROID in self.promoted.approved_applications ) diff --git a/src/olympia/addons/serializers.py b/src/olympia/addons/serializers.py index 312c8523299..89ddf2a6211 100644 --- a/src/olympia/addons/serializers.py +++ b/src/olympia/addons/serializers.py @@ -513,12 +513,12 @@ def validate_is_disabled(self, disable): ): raise exceptions.ValidationError(gettext('File is already disabled.')) if not version.can_be_disabled_and_deleted(): - group = version.addon.promoted_group() + name = version.addon.group_name() msg = gettext( 'The latest approved version of this %s add-on cannot be deleted ' 'because the previous version was not approved for %s promotion. ' 'Please contact AMO Admins if you need help with this.' - ) % (group.name, group.name) + ) % (name, name) raise exceptions.ValidationError(msg) return disable @@ -1562,12 +1562,13 @@ def fake_object(self, data): # set .approved_for_groups cached_property because it's used in # .approved_applications. approved_for_apps = promoted.get('approved_for_apps') - obj.promoted = PromotedAddon( - addon=obj, - approved_application_ids=approved_for_apps, - created=None, - group_id=promoted['group_id'], - ) + for group_id in promoted['group_ids']: + obj.promoted = PromotedAddon( + addon=obj, + approved_application_ids=approved_for_apps, + created=None, + group_id=group_id, + ) # we can safely regenerate these tuples because # .appproved_applications only cares about the current group obj._current_version.approved_for_groups = ( diff --git a/src/olympia/addons/tests/test_indexers.py b/src/olympia/addons/tests/test_indexers.py index 83a7a355c18..6a9ced51cad 100644 --- a/src/olympia/addons/tests/test_indexers.py +++ b/src/olympia/addons/tests/test_indexers.py @@ -513,7 +513,7 @@ def test_extract_promoted(self): self.addon = addon_factory(promoted=RECOMMENDED) extracted = self._extract() assert extracted['promoted'] - assert extracted['promoted']['group_id'] == RECOMMENDED.id + assert RECOMMENDED.id in extracted['promoted']['group_ids'] assert extracted['promoted']['approved_for_apps'] == [ amo.FIREFOX.id, amo.ANDROID.id, @@ -521,7 +521,7 @@ def test_extract_promoted(self): assert extracted['is_recommended'] is True # Specific application. - self.addon.promotedaddon.update(application_id=amo.FIREFOX.id) + self.addon.promoted_addons.first().update(application_id=amo.FIREFOX.id) extracted = self._extract() assert extracted['promoted']['approved_for_apps'] == [amo.FIREFOX.id] assert extracted['is_recommended'] is True @@ -534,7 +534,7 @@ def test_extract_promoted(self): featured_collection.add_addon(self.addon) extracted = self._extract() assert extracted['promoted'] - assert extracted['promoted']['group_id'] == RECOMMENDED.id + assert RECOMMENDED.id in extracted['promoted']['group_ids'] assert extracted['promoted']['approved_for_apps'] == [ amo.FIREFOX.id, amo.ANDROID.id, diff --git a/src/olympia/addons/tests/test_models.py b/src/olympia/addons/tests/test_models.py index a6c7a17e680..acd7597e2ed 100644 --- a/src/olympia/addons/tests/test_models.py +++ b/src/olympia/addons/tests/test_models.py @@ -1727,34 +1727,31 @@ def test_promoted_group(self): addon = addon_factory() # default case - no group so not recommended assert not addon.promoted_group() - # NOT_PROMOTED is falsey - assert addon.promoted_group() == NOT_PROMOTED assert not addon.promoted_group(currently_approved=False) # It's promoted but nothing has been approved promoted = PromotedAddon.objects.create(addon=addon, group_id=LINE.id) assert addon.promoted_group(currently_approved=False) - assert addon.promoted_group() == NOT_PROMOTED assert not addon.promoted_group() # The latest version is approved for the same group. promoted.approve_for_version(version=addon.current_version) assert addon.promoted_group() - assert addon.promoted_group() == LINE + assert LINE in addon.promoted_group() # if the group has changes the approval for the current version isn't # valid promoted.update(group_id=SPOTLIGHT.id) assert not addon.promoted_group() assert addon.promoted_group(currently_approved=False) - assert addon.promoted_group(currently_approved=False) == SPOTLIGHT + assert SPOTLIGHT in addon.promoted_group(currently_approved=False) promoted.approve_for_version(version=addon.current_version) - assert addon.promoted_group() == SPOTLIGHT + assert SPOTLIGHT in addon.promoted_group() # Application specific group membership should work too # if no app is specifed in the PromotedAddon everything should match - assert addon.promoted_group() == SPOTLIGHT + assert SPOTLIGHT in addon.promoted_group() # update to mobile app promoted.update(application_id=amo.ANDROID.id) assert addon.promoted_group() @@ -1765,14 +1762,13 @@ def test_promoted_group(self): del addon.current_version.approved_for_groups assert not addon.promoted_group() promoted.update(application_id=amo.FIREFOX.id) - assert addon.promoted_group() == SPOTLIGHT + assert SPOTLIGHT in addon.promoted_group() # check it doesn't error if there's no current_version addon.current_version.file.update(status=amo.STATUS_DISABLED) addon.update_version() assert not addon.current_version assert not addon.promoted_group() - assert addon.promoted_group() == NOT_PROMOTED assert addon.promoted_group(currently_approved=False) def test_promoted(self): @@ -1821,7 +1817,6 @@ def test_promoted_theme(self): featured_collection.remove_addon(addon) del addon.promoted addon = Addon.objects.get(id=addon.id) - # assert not addon.promotedaddon # but not when it's removed. assert addon.promoted is None diff --git a/src/olympia/addons/tests/test_serializers.py b/src/olympia/addons/tests/test_serializers.py index 3672f4aef69..bfcd8653b3e 100644 --- a/src/olympia/addons/tests/test_serializers.py +++ b/src/olympia/addons/tests/test_serializers.py @@ -552,7 +552,7 @@ def test_promoted(self): assert result['promoted']['apps'] == [amo.FIREFOX.short] # With a recommended theme. - self.addon.promotedaddon.delete() + self.addon.promoted_addons.all().delete() self.addon.update(type=amo.ADDON_STATICTHEME) featured_collection, _ = Collection.objects.get_or_create( id=settings.COLLECTION_FEATURED_THEMES_ID diff --git a/src/olympia/addons/tests/test_views.py b/src/olympia/addons/tests/test_views.py index bfa87bfe237..ea7e5d34431 100644 --- a/src/olympia/addons/tests/test_views.py +++ b/src/olympia/addons/tests/test_views.py @@ -5610,7 +5610,7 @@ def test_filter_by_featured_no_app_no_lang(self): slug='my-addon', name='Featured Addôn', promoted=RECOMMENDED ) addon_factory(slug='other-addon', name='Other Addôn') - assert addon.promoted_group() == RECOMMENDED + assert RECOMMENDED in addon.promoted_group() self.reindex(Addon) data = self.perform_search(self.url, {'featured': 'true'}) @@ -5633,9 +5633,9 @@ def test_filter_by_promoted(self): min=av_min, max=av_max, ) - assert addon.promoted_group() == RECOMMENDED - assert addon.promotedaddon.application_id is None # i.e. all - assert addon.promotedaddon.approved_applications == [amo.FIREFOX, amo.ANDROID] + assert RECOMMENDED in addon.promoted_group() + assert addon.promoted_addons.first().application_id is None # i.e. all + assert addon.approved_applications == [amo.FIREFOX, amo.ANDROID] addon2 = addon_factory(name='Fírefox Addôn', promoted=RECOMMENDED) ApplicationsVersions.objects.get_or_create( @@ -5645,10 +5645,11 @@ def test_filter_by_promoted(self): max=av_max, ) # This case is approved for all apps, but now only set for Firefox - addon2.promotedaddon.update(application_id=amo.FIREFOX.id) - assert addon2.promoted_group() == RECOMMENDED - assert addon2.promotedaddon.application_id is amo.FIREFOX.id - assert addon2.promotedaddon.approved_applications == [amo.FIREFOX] + addon2.promoted_addons.first().update(application_id=amo.FIREFOX.id) + assert RECOMMENDED in addon2.promoted_group() + assert addon2.promoted_addons.first().application_id is amo.FIREFOX.id + assert addon2.promoted_addons.first().approved_applications == [amo.FIREFOX] + assert addon2.approved_applications == [amo.FIREFOX] addon3 = addon_factory(slug='other-addon', name='Other Addôn') ApplicationsVersions.objects.get_or_create( @@ -5668,12 +5669,12 @@ def test_filter_by_promoted(self): max=av_max, ) self.make_addon_promoted(addon4, RECOMMENDED) - addon4.promotedaddon.update(application_id=amo.FIREFOX.id) - addon4.promotedaddon.approve_for_version(addon4.current_version) - addon4.promotedaddon.update(application_id=None) - assert addon4.promoted_group() == RECOMMENDED - assert addon4.promotedaddon.application_id is None # i.e. all - assert addon4.promotedaddon.approved_applications == [amo.FIREFOX] + addon4.promoted_addons.first().update(application_id=amo.FIREFOX.id) + addon4.promoted_addons.first().approve_for_version(addon4.current_version) + addon4.promoted_addons.first().update(application_id=None) + assert RECOMMENDED in addon4.promoted_group() + assert addon4.promoted_addons.first().application_id is None # i.e. all + assert addon4.promoted_addons.first().approved_applications == [amo.FIREFOX] # And repeat with Android rather than Firefox addon5 = addon_factory(name='Andróid Addôn') @@ -5684,12 +5685,12 @@ def test_filter_by_promoted(self): max=av_max, ) self.make_addon_promoted(addon5, RECOMMENDED) - addon5.promotedaddon.update(application_id=amo.ANDROID.id) - addon5.promotedaddon.approve_for_version(addon5.current_version) - addon5.promotedaddon.update(application_id=None) - assert addon5.promoted_group() == RECOMMENDED - assert addon5.promotedaddon.application_id is None # i.e. all - assert addon5.promotedaddon.approved_applications == [amo.ANDROID] + addon5.promoted_addons.first().update(application_id=amo.ANDROID.id) + addon5.promoted_addons.first().approve_for_version(addon5.current_version) + addon5.promoted_addons.first().update(application_id=None) + assert RECOMMENDED in addon5.promoted_group() + assert addon5.promoted_addons.first().application_id is None # i.e. all + assert addon5.promoted_addons.first().approved_applications == [amo.ANDROID] self.reindex(Addon) @@ -6555,8 +6556,8 @@ def tearDown(self): def test_basic(self): addon1 = addon_factory(promoted=RECOMMENDED) addon2 = addon_factory(promoted=RECOMMENDED) - assert addon1.promoted_group() == RECOMMENDED - assert addon2.promoted_group() == RECOMMENDED + assert RECOMMENDED in addon1.promoted_group() + assert RECOMMENDED in addon2.promoted_group() addon_factory() # not recommended so shouldn't show up self.refresh() diff --git a/src/olympia/addons/views.py b/src/olympia/addons/views.py index 1efd7910bb5..6a5b7d43eed 100644 --- a/src/olympia/addons/views.py +++ b/src/olympia/addons/views.py @@ -732,12 +732,12 @@ def update(self, request, *args, **kwargs): def destroy(self, request, *args, **kwargs): instance = self.get_object() if not instance.can_be_disabled_and_deleted(): - group = self.get_addon_object().promoted_group() + name = self.get_addon_object().group_name() msg = gettext( 'The latest approved version of this %s add-on cannot be deleted ' 'because the previous version was not approved for %s promotion. ' 'Please contact AMO Admins if you need help with this.' - ) % (group.name, group.name) + ) % (name, name) raise exceptions.ValidationError(msg) return super().destroy(request, *args, **kwargs) diff --git a/src/olympia/amo/tests/__init__.py b/src/olympia/amo/tests/__init__.py index b1bb0d50af3..a411697843c 100644 --- a/src/olympia/amo/tests/__init__.py +++ b/src/olympia/amo/tests/__init__.py @@ -590,7 +590,7 @@ def make_addon_promoted(cls, addon, group, approve_version=False): if approve_version: obj.approve_for_version(addon.current_version) if not created: - addon.promotedaddon.reload() + addon.promoted_addons.first().reload() return obj def _add_fake_throttling_action( @@ -969,7 +969,7 @@ def version_factory(*, file_kw=None, needshumanreview_kw=None, **kw): file_kw = file_kw or {} file_factory(version=ver, **file_kw) if promotion_approved: - kw['addon'].promotedaddon.approve_for_version(version=ver) + kw['addon'].promoted_addons.first().approve_for_version(version=ver) av_min, _ = AppVersion.objects.get_or_create( application=application, version=min_app_version ) diff --git a/src/olympia/bandwagon/views.py b/src/olympia/bandwagon/views.py index 23e98c80f07..629ca8e81ef 100644 --- a/src/olympia/bandwagon/views.py +++ b/src/olympia/bandwagon/views.py @@ -201,7 +201,7 @@ def get_queryset(self): qs = ( CollectionAddon.objects.filter(collection=self.get_collection()) .prefetch_related( - 'addon__promotedaddon', + 'addon__promoted_addons', 'addon___current_version__file___webext_permissions', ) .transform(self._transformer) diff --git a/src/olympia/constants/promoted.py b/src/olympia/constants/promoted.py index f9d90e87030..5a3032541ea 100644 --- a/src/olympia/constants/promoted.py +++ b/src/olympia/constants/promoted.py @@ -4,44 +4,58 @@ from olympia.constants import applications - -_PromotedSuperClass = namedtuple( - '_PromotedSuperClass', - [ - # Be careful when adding to this list to adjust defaults too. - 'id', - 'name', - 'api_name', - 'search_ranking_bump', - 'listed_pre_review', - 'unlisted_pre_review', - 'admin_review', - 'badged', # See BADGE_CATEGORIES in frontend too: both need changing - 'autograph_signing_states', - 'can_primary_hero', # can be added to a primary hero shelf - 'immediate_approval', # will addon be auto-approved once added - 'flag_for_human_review', # will be add-on be flagged for another review - 'can_be_compatible_with_all_fenix_versions', # If addon is promoted for Android - 'high_profile', # the add-on is considered high-profile for review purposes - 'high_profile_rating', # developer replies are considered high-profile - ], - defaults=( - # "Since fields with a default value must come after any fields without - # a default, the defaults are applied to the rightmost parameters" - # No defaults for: id, name, api_name. +ID = 'id' +NAME = 'name' +API_NAME = 'api_name' +SEARCH_RANKING_BUMP = 'search_ranking_bump' +LISTED_PRE_REVIEW = 'listed_pre_review' +UNLISTED_PRE_REVIEW = 'unlisted_pre_review' +ADMIN_REVIEW = 'admin_review' +BADGED = 'badged' +AUTOGRAPH_SIGNING_STATES = 'autograph_signing_states' +CAN_PRIMARY_HERO = 'can_primary_hero' +IMMEDIATE_APPROVAL = 'immediate_approval' +FLAG_FOR_HUMAN_REVIEW = 'flag_for_human_review' +CAN_BE_COMPATIBLE_WITH_ALL_FENIX_VERSIONS = 'can_be_compatible_with_all_fenix_versions' +HIGH_PROFILE = 'high_profile' +HIGH_PROFILE_RATING = 'high_profile_rating' + +defaults=( 0.0, # search_ranking_bump False, # listed_pre_review False, # unlisted_pre_review False, # admin_review False, # badged - {}, # autograph_signing_states - should be a dict of App.short: state + {}, # autograph_signing_states False, # can_primary_hero False, # immediate_approval False, # flag_for_human_review False, # can_be_compatible_with_all_fenix_versions False, # high_profile False, # high_profile_rating - ), + ) + +_PromotedSuperClass = namedtuple( + '_PromotedSuperClass', + [ + # Be careful when adding to this list to adjust defaults too. + ID, + NAME, + API_NAME, + SEARCH_RANKING_BUMP, + LISTED_PRE_REVIEW, + UNLISTED_PRE_REVIEW, + ADMIN_REVIEW, + BADGED, # See BADGE_CATEGORIES in frontend too: both need changing + AUTOGRAPH_SIGNING_STATES, + CAN_PRIMARY_HERO, # can be added to a primary hero shelf + IMMEDIATE_APPROVAL, # will addon be auto-approved once added + FLAG_FOR_HUMAN_REVIEW, # will be add-on be flagged for another review + CAN_BE_COMPATIBLE_WITH_ALL_FENIX_VERSIONS, # If addon is promoted for Android + HIGH_PROFILE, # the add-on is considered high-profile for review purposes + HIGH_PROFILE_RATING # developer replies are considered high-profile + ], + defaults=defaults, ) @@ -50,6 +64,14 @@ class PromotedClass(_PromotedSuperClass): def __bool__(self): return bool(self.id) + + @classmethod + def type(cls, attribute): + try: + index = cls._fields.index(attribute) + return type(defaults[index]) + except ValueError: + raise AttributeError(f"{attribute} is not a valid parameter.") NOT_PROMOTED = PromotedClass( diff --git a/src/olympia/devhub/templates/devhub/includes/addon_details.html b/src/olympia/devhub/templates/devhub/includes/addon_details.html index 2815c98a8e0..2efdfd9e565 100644 --- a/src/olympia/devhub/templates/devhub/includes/addon_details.html +++ b/src/olympia/devhub/templates/devhub/includes/addon_details.html @@ -6,9 +6,8 @@ {% set status_text = _('Invisible') %} {% set tooltip_text = status_tips['invisible'] %} {% else %} - {% set promoted_group = addon.promoted_group() %} - {% if addon.status == amo.STATUS_APPROVED and promoted_group.badged %} - {% set status_text = _('Approved and %s' % promoted_group.name) %} + {% if addon.status == amo.STATUS_APPROVED and addon.get('badged') %} + {% set status_text = _('Approved and %s' % addon.group_name()) %} {% else %} {% set status_text = addon.get_status_display() %} {% endif %} diff --git a/src/olympia/devhub/templates/devhub/new-landing/components/my-addons.html b/src/olympia/devhub/templates/devhub/new-landing/components/my-addons.html index 443d32c234d..40c3592b53c 100644 --- a/src/olympia/devhub/templates/devhub/new-landing/components/my-addons.html +++ b/src/olympia/devhub/templates/devhub/new-landing/components/my-addons.html @@ -1,9 +1,8 @@ {% macro render_status(addon) %} - {% set promoted_group = addon.promoted_group() %} {% if addon.status != amo.STATUS_DISABLED and addon.disabled_by_user %} {% set status_text = _('Invisible') %} - {% elif addon.status == amo.STATUS_APPROVED and promoted_group.badged %} - {% set status_text = _('Approved and %s' % promoted_group.name) %} + {% elif addon.status == amo.STATUS_APPROVED and addon.get('badged') %} + {% set status_text = _('Approved and %s' % addon.group_name()) %} {% else %} {% set status_text = addon.get_status_display() %} {% endif %} diff --git a/src/olympia/devhub/templates/devhub/versions/list.html b/src/olympia/devhub/templates/devhub/versions/list.html index b1017762565..84255bf9108 100644 --- a/src/olympia/devhub/templates/devhub/versions/list.html +++ b/src/olympia/devhub/templates/devhub/versions/list.html @@ -297,7 +297,7 @@
{# will be hidden by default, shown by javascript if data-can-be-disabled is falsey #} - {% trans promoted_group_name = addon.promoted_group().name %} + {% trans promoted_group_name = addon.group_name() %} The latest approved version of this {{ promoted_group_name }} add-on cannot be deleted or disabled because the previous version was not approved for {{ promoted_group_name }} promotion. Please contact AMO Admins if you need help with this. diff --git a/src/olympia/devhub/tests/test_views_versions.py b/src/olympia/devhub/tests/test_views_versions.py index 6d87879042e..1f2ff7b55f7 100644 --- a/src/olympia/devhub/tests/test_views_versions.py +++ b/src/olympia/devhub/tests/test_views_versions.py @@ -238,7 +238,7 @@ 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 self.addon.promoted_group() == RECOMMENDED + assert RECOMMENDED in self.addon.promoted_group() def test_can_still_disable_or_delete_old_version_recommended(self): # If the add-on is recommended, you can still disable or delete older diff --git a/src/olympia/devhub/views.py b/src/olympia/devhub/views.py index 7d71c94b55b..158a9b8241e 100644 --- a/src/olympia/devhub/views.py +++ b/src/olympia/devhub/views.py @@ -20,6 +20,7 @@ from django.views.decorators.cache import never_cache from django.views.decorators.csrf import csrf_exempt +from olympia.constants.promoted import LISTED_PRE_REVIEW, UNLISTED_PRE_REVIEW import waffle from csp.decorators import csp_update from django_statsd.clients import statsd @@ -1207,13 +1208,13 @@ def version_delete(request, addon_id, addon): if not version.can_be_disabled_and_deleted(): # Developers shouldn't be able to delete/disable the current version # of a promoted approved add-on. - group = addon.promoted_group() + name = addon.group_name() msg = gettext( 'The latest approved version of this %s add-on cannot ' 'be deleted or disabled because the previous version was not ' 'approved for %s promotion. ' 'Please contact AMO Admins if you need help with this.' - ) % (group.name, group.name) + ) % (name, name) messages.error(request, msg) elif 'disable_version' in request.POST: messages.success(request, gettext('Version %s disabled.') % version.version) @@ -1585,9 +1586,8 @@ def _submit_upload( # If we're not showing the generic submit notification warning, show # one specific to pre review if the developer would be affected because # of its promoted group. - promoted_group = addon.promoted_group(currently_approved=False) - if (channel == amo.CHANNEL_LISTED and promoted_group.listed_pre_review) or ( - channel == amo.CHANNEL_UNLISTED and promoted_group.unlisted_pre_review + if (channel == amo.CHANNEL_LISTED and addon.get(LISTED_PRE_REVIEW, currently_approved=False)) or ( + channel == amo.CHANNEL_UNLISTED and addon.get(UNLISTED_PRE_REVIEW, currently_approved=False) ): submit_notification_warning = get_config( 'submit_notification_warning_pre_review' diff --git a/src/olympia/discovery/views.py b/src/olympia/discovery/views.py index ed2f9764b4a..40b6cd7fba0 100644 --- a/src/olympia/discovery/views.py +++ b/src/olympia/discovery/views.py @@ -102,7 +102,7 @@ def filter_queryset(self, qs): if self.request.query_params.get('recommended', False) == 'true': qs = qs.filter( **{ - 'addon__promotedaddon__group_id': RECOMMENDED.id, + 'addon__promoted_addons__group_id': RECOMMENDED.id, 'addon___current_version__promoted_approvals__group_id': RECOMMENDED.id, # noqa } ).distinct() diff --git a/src/olympia/hero/tests/test_models.py b/src/olympia/hero/tests/test_models.py index 73f2cf8fe88..28f6f80daec 100644 --- a/src/olympia/hero/tests/test_models.py +++ b/src/olympia/hero/tests/test_models.py @@ -58,7 +58,7 @@ def test_clean_requires_approved_can_primary_hero_group(self): ph.promoted_addon.approve_for_version(ph.promoted_addon.addon.current_version) ph.reload() ph.enabled = True - assert ph.promoted_addon.addon.promoted_group() == RECOMMENDED + assert RECOMMENDED in ph.promoted_addon.addon.promoted_group() ph.clean() # it raises if there's an error # change to a different group @@ -66,7 +66,7 @@ def test_clean_requires_approved_can_primary_hero_group(self): ph.promoted_addon.approve_for_version(ph.promoted_addon.addon.current_version) ph.reload() ph.enabled = True - assert ph.promoted_addon.addon.promoted_group() == STRATEGIC + assert STRATEGIC in ph.promoted_addon.addon.promoted_group() with self.assertRaises(ValidationError) as context: # STRATEGIC isn't a group that can be added as a primary hero ph.clean() @@ -80,7 +80,7 @@ def test_clean_requires_approved_can_primary_hero_group(self): ph.promoted_addon.approve_for_version(ph.promoted_addon.addon.current_version) ph.reload() ph.enabled = True - assert ph.promoted_addon.addon.promoted_group() == SPOTLIGHT + assert SPOTLIGHT in ph.promoted_addon.addon.promoted_group() ph.clean() # it raises if there's an error def test_clean_external_requires_homepage(self): diff --git a/src/olympia/hero/tests/test_serializers.py b/src/olympia/hero/tests/test_serializers.py index f482921beb0..28951669e65 100644 --- a/src/olympia/hero/tests/test_serializers.py +++ b/src/olympia/hero/tests/test_serializers.py @@ -35,7 +35,7 @@ def setUp(self): def test_basic(self): addon = addon_factory(promoted=RECOMMENDED) hero = PrimaryHero.objects.create( - promoted_addon=addon.promotedaddon, + promoted_addon=addon.promoted_addons.first(), description='Déscription', select_image=self.phi, gradient_color='#008787', diff --git a/src/olympia/landfill/serializers.py b/src/olympia/landfill/serializers.py index d3c3338ec6a..2edf1ec9dbc 100644 --- a/src/olympia/landfill/serializers.py +++ b/src/olympia/landfill/serializers.py @@ -69,7 +69,7 @@ def create_generic_featured_addons(self): ) AddonUser.objects.create(user=user_factory(), addon=addon) - PrimaryHero.objects.create(promoted_addon=addon.promotedaddon, enabled=True) + PrimaryHero.objects.create(promoted_addon=addon.promoted_addons.first(), enabled=True) SecondaryHero.objects.create( enabled=True, headline='This is a headline', diff --git a/src/olympia/lib/crypto/signing.py b/src/olympia/lib/crypto/signing.py index f66d6a92a29..e036f447c14 100644 --- a/src/olympia/lib/crypto/signing.py +++ b/src/olympia/lib/crypto/signing.py @@ -13,6 +13,7 @@ import requests from asn1crypto import cms from django_statsd.clients import statsd +from olympia.constants.promoted import AUTOGRAPH_SIGNING_STATES from requests_hawk import HawkAuth import olympia.core.logger @@ -51,10 +52,10 @@ def get_id(addon): return force_str(hashlib.sha256(guid).hexdigest()) -def use_promoted_signer(file_obj, promo_group): +def use_promoted_signer(file_obj, addon): return ( file_obj.version.channel == amo.CHANNEL_LISTED - and promo_group.autograph_signing_states + and addon.get(AUTOGRAPH_SIGNING_STATES) ) @@ -120,11 +121,11 @@ def call_signing(file_obj): # We are using a separate signer that adds the mozilla-recommendation.json # file. - promo_group = file_obj.addon.promoted_group(currently_approved=False) - if use_promoted_signer(file_obj, promo_group): + states = file_obj.addon.get(AUTOGRAPH_SIGNING_STATES, currently_approved=False) + if use_promoted_signer(file_obj, file_obj.addon): signing_states = { - promo_group.autograph_signing_states.get(app.short) - for app in file_obj.addon.promotedaddon.all_applications + states.get(app.short) + for app in file_obj.addon.promoted_addons.first().all_applications } signing_data['keyid'] = conf['recommendation_signer'] diff --git a/src/olympia/lib/crypto/tasks.py b/src/olympia/lib/crypto/tasks.py index fd267da3681..6f634aee56b 100644 --- a/src/olympia/lib/crypto/tasks.py +++ b/src/olympia/lib/crypto/tasks.py @@ -6,6 +6,7 @@ from django.db import transaction from django.utils import translation +from olympia.constants.promoted import LISTED_PRE_REVIEW import olympia.core.logger from olympia import amo from olympia.activity.models import ActivityLog @@ -122,7 +123,6 @@ def bump_addon_version(old_version): task_user = get_task_user() addon = old_version.addon old_file_obj = old_version.file - promoted_group = addon.promoted_group(currently_approved=True) # We only sign files that have been reviewed if old_file_obj.status not in amo.APPROVED_STATUSES: log.info( @@ -200,8 +200,8 @@ def bump_addon_version(old_version): ) # Carry over promotion if necessary. - if promoted_group and promoted_group.listed_pre_review: - addon.promotedaddon.approve_for_version(new_version) + if addon.get(LISTED_PRE_REVIEW, currently_approved=True): + addon.promoted_addons.first().approve_for_version(new_version) except Exception: log.exception(f'Failed re-signing file {old_file_obj.pk}', exc_info=True) diff --git a/src/olympia/lib/crypto/tests/test_signing.py b/src/olympia/lib/crypto/tests/test_signing.py index 04462f4f70a..e2fcd4907a8 100644 --- a/src/olympia/lib/crypto/tests/test_signing.py +++ b/src/olympia/lib/crypto/tests/test_signing.py @@ -440,7 +440,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, RECOMMENDED) - self.file_.version.addon.promotedaddon.update(application_id=amo.ANDROID.id) + self.file_.version.addon.promoted_addons.first().update(application_id=amo.ANDROID.id) # Recommended has different states for desktop and android self._check_signed_correctly(states=['recommended-android']) diff --git a/src/olympia/promoted/admin.py b/src/olympia/promoted/admin.py index 43f77d489a8..b32be2a084f 100644 --- a/src/olympia/promoted/admin.py +++ b/src/olympia/promoted/admin.py @@ -52,7 +52,7 @@ def get_queryset(self, request): if not self.instance: return self.model.objects.none() qs = super().get_queryset(request) - qs = qs.filter(version__addon__promotedaddon=self.instance).order_by( + qs = qs.filter(version__addon__promoted_addons=self.instance).order_by( '-version_id' ) return qs diff --git a/src/olympia/promoted/migrations/0022_alter_promotedaddon_addon_and_more.py b/src/olympia/promoted/migrations/0022_alter_promotedaddon_addon_and_more.py new file mode 100644 index 00000000000..48e033a4241 --- /dev/null +++ b/src/olympia/promoted/migrations/0022_alter_promotedaddon_addon_and_more.py @@ -0,0 +1,20 @@ +# Generated by Django 4.2.16 on 2024-11-25 16:59 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('addons', '0053_alter_addon_summary_alter_addon_type'), + ('promoted', '0021_auto_20240919_0952'), + ] + + operations = [ + migrations.AlterField( + model_name='promotedaddon', + name='addon', + field=models.ForeignKey(help_text='Add-on id this item will point to (If you do not know the id, paste the slug instead and it will be transformed automatically for you. If you have access to the add-on admin page, you can use the magnifying glass to see all available add-ons.', on_delete=django.db.models.deletion.CASCADE, related_name='promoted_addons', to='addons.addon'), + ), + ] diff --git a/src/olympia/promoted/models.py b/src/olympia/promoted/models.py index 29d8369dedc..a9b96935b19 100644 --- a/src/olympia/promoted/models.py +++ b/src/olympia/promoted/models.py @@ -24,7 +24,7 @@ class PromotedAddon(ModelBase): 'deleting it. Note: changing the group does *not* change ' 'approvals of versions.', ) - addon = models.OneToOneField( + addon = models.ForeignKey( Addon, on_delete=models.CASCADE, null=False, @@ -33,6 +33,7 @@ class PromotedAddon(ModelBase): 'automatically for you. If you have access to the add-on ' 'admin page, you can use the magnifying glass to see ' 'all available add-ons.', + related_name='promoted_addons' ) application_id = models.SmallIntegerField( choices=APPLICATION_CHOICES, null=True, verbose_name='Application', blank=True diff --git a/src/olympia/promoted/tests/test_admin.py b/src/olympia/promoted/tests/test_admin.py index b31cf057d15..466253510fb 100644 --- a/src/olympia/promoted/tests/test_admin.py +++ b/src/olympia/promoted/tests/test_admin.py @@ -376,7 +376,7 @@ def test_can_add_when_existing_approval(self): assert response.status_code == 200 assert 'errors' not in response.context_data assert PromotedApproval.objects.count() == 1 # still one - assert addon.promoted_group() == LINE # now approved + assert LINE in addon.promoted_group() # now approved def test_cannot_add_without_discovery_edit_permission(self): addon = addon_factory() diff --git a/src/olympia/promoted/tests/test_models.py b/src/olympia/promoted/tests/test_models.py index 0655f5c5de2..46fa3c98158 100644 --- a/src/olympia/promoted/tests/test_models.py +++ b/src/olympia/promoted/tests/test_models.py @@ -33,21 +33,21 @@ def test_is_approved_applications(self): promoted_addon = PromotedAddon.objects.create( addon=addon, group_id=promoted.LINE.id ) - assert addon.promotedaddon + assert addon.promoted_addons.first() # Just having the PromotedAddon instance isn't enough - assert addon.promotedaddon.approved_applications == [] + assert addon.promoted_addons.first().approved_applications == [] # the current version needs to be approved also promoted_addon.approve_for_version(addon.current_version) addon.reload() - assert addon.promotedaddon.approved_applications == [ + assert addon.promoted_addons.first().approved_applications == [ applications.FIREFOX, applications.ANDROID, ] # but not if it's for a different type of promotion promoted_addon.update(group_id=promoted.SPOTLIGHT.id) - assert addon.promotedaddon.approved_applications == [] + assert addon.promoted_addons.first().approved_applications == [] # unless that group has an approval too PromotedApproval.objects.create( version=addon.current_version, @@ -55,13 +55,13 @@ def test_is_approved_applications(self): application_id=applications.FIREFOX.id, ) addon.reload() - assert addon.promotedaddon.approved_applications == [applications.FIREFOX] + assert addon.promoted_addons.first().approved_applications == [applications.FIREFOX] # for promoted groups that don't require pre-review though, there isn't # a per version approval, so a current_version is sufficient and all # applications are seen as approved. promoted_addon.update(group_id=promoted.STRATEGIC.id) - assert addon.promotedaddon.approved_applications == [ + assert addon.promoted_addons.first().approved_applications == [ applications.FIREFOX, applications.ANDROID, ] @@ -81,7 +81,7 @@ def test_auto_approves_addon_when_saved_for_immediate_approval(self): promo.addon.reload() assert promo.approved_applications == [] assert not PromotedApproval.objects.exists() - assert promo.addon.promoted_group() == promoted.NOT_PROMOTED + assert promoted.NOT_PROMOTED in promo.addon.promoted_group() # then with a group thats immediate_approval == True promo.group_id = promoted.SPOTLIGHT.id @@ -89,7 +89,7 @@ def test_auto_approves_addon_when_saved_for_immediate_approval(self): promo.addon.reload() assert promo.approved_applications == [amo.FIREFOX] assert PromotedApproval.objects.count() == 1 - assert promo.addon.promoted_group() == promoted.SPOTLIGHT + assert promoted.SPOTLIGHT in promo.addon.promoted_group() # test the edge case where the application was changed afterwards promo.application_id = 0 @@ -116,7 +116,7 @@ def test_addon_flagged_for_human_review_when_saved(self): promo.addon.reload() assert promo.approved_applications == [] assert not PromotedApproval.objects.exists() - assert promo.addon.promoted_group() == promoted.NOT_PROMOTED + assert promoted.NOT_PROMOTED in promo.addon.promoted_group() assert unlisted_ver.needshumanreview_set.count() == 0 assert listed_ver.needshumanreview_set.count() == 0 @@ -133,7 +133,7 @@ def test_addon_flagged_for_human_review_when_saved(self): promo.addon.reload() assert promo.approved_applications == [] # doesn't approve immediately assert not PromotedApproval.objects.exists() - assert promo.addon.promoted_group() == promoted.NOT_PROMOTED + assert promoted.NOT_PROMOTED in promo.addon.promoted_group() assert not listed_ver.reload().due_date assert not unlisted_ver.reload().due_date assert unlisted_ver.needshumanreview_set.count() == 0 @@ -152,7 +152,7 @@ def test_addon_flagged_for_human_review_when_saved(self): promo.addon.reload() assert promo.approved_applications == [] # doesn't approve immediately assert not PromotedApproval.objects.exists() - assert promo.addon.promoted_group() == promoted.NOT_PROMOTED + assert promoted.NOT_PROMOTED in promo.addon.promoted_group() assert not listed_ver.reload().due_date assert not unlisted_ver.reload().due_date assert unlisted_ver.needshumanreview_set.count() == 0 @@ -169,7 +169,7 @@ def test_addon_flagged_for_human_review_when_saved(self): promo.addon.reload() assert promo.approved_applications == [] # doesn't approve immediately assert not PromotedApproval.objects.exists() - assert promo.addon.promoted_group() == promoted.NOT_PROMOTED + assert promoted.NOT_PROMOTED in promo.addon.promoted_group() self.assertCloseToNow(listed_ver.reload().due_date, now=get_review_due_date()) self.assertCloseToNow(unlisted_ver.reload().due_date, now=get_review_due_date()) assert unlisted_ver.needshumanreview_set.filter(is_active=True).count() == 1 @@ -191,7 +191,7 @@ def test_disabled_and_deleted_versions_flagged_for_human_review(self): promo = PromotedAddon.objects.create( addon=addon, application_id=amo.FIREFOX.id, group_id=promoted.NOTABLE.id ) - assert promo.addon.promoted_group() == promoted.NOT_PROMOTED + assert promoted.NOT_PROMOTED in promo.addon.promoted_group() self.assertCloseToNow(version.reload().due_date, now=get_review_due_date()) assert version.needshumanreview_set.filter(is_active=True).count() == 1 assert ( @@ -239,5 +239,5 @@ def test_approve_for_addon(self): # SPOTLIGHT doesnt have special signing states so won't be resigned # approve_for_addon is called automatically - SPOTLIGHT has immediate_approval promo.addon.reload() - assert promo.addon.promoted_group() == promoted.SPOTLIGHT + assert promoted.SPOTLIGHT in promo.addon.promoted_group() assert promo.addon.current_version.version == '0.123a' diff --git a/src/olympia/promoted/tests/test_tasks.py b/src/olympia/promoted/tests/test_tasks.py index bac16036d0d..90632941364 100644 --- a/src/olympia/promoted/tests/test_tasks.py +++ b/src/olympia/promoted/tests/test_tasks.py @@ -28,8 +28,7 @@ def test_add_high_adu_extensions_to_notable_tier_absent_or_no_threshold(): add_high_adu_extensions_to_notable() assert ( - extension_with_high_adu.reload().promoted_group(currently_approved=False) - == NOT_PROMOTED + not extension_with_high_adu.reload().promoted_group(currently_approved=False) ) UsageTier.objects.create(slug=NOTABLE_TIER_SLUG, lower_adu_threshold=None) @@ -37,8 +36,7 @@ def test_add_high_adu_extensions_to_notable_tier_absent_or_no_threshold(): add_high_adu_extensions_to_notable() assert ( - extension_with_high_adu.reload().promoted_group(currently_approved=False) - == NOT_PROMOTED + not extension_with_high_adu.reload().promoted_group(currently_approved=False) ) @@ -95,23 +93,21 @@ def test_add_high_adu_extensions_to_notable(): add_high_adu_extensions_to_notable() assert ( - extension_with_low_adu.reload().promoted_group(currently_approved=False) - == NOT_PROMOTED + not extension_with_low_adu.reload().promoted_group(currently_approved=False) ) assert ( - extension_with_high_adu.reload().promoted_group(currently_approved=False) - == NOTABLE + NOTABLE in extension_with_high_adu.reload().promoted_group(currently_approved=False) ) assert ( - ignored_theme.reload().promoted_group(currently_approved=False) == NOT_PROMOTED - ) - already_promoted.reload().promotedaddon.reload() - assert already_promoted.promoted_group(currently_approved=False) == LINE - promoted_record_exists.reload().promotedaddon.reload() - assert promoted_record_exists.promoted_group(currently_approved=False) == NOTABLE - assert unlisted_only_extension.promoted_group(currently_approved=False) == NOTABLE - assert mixed_extension.promoted_group(currently_approved=False) == NOTABLE - assert deleted_extension.promoted_group(currently_approved=False) == NOTABLE + not ignored_theme.reload().promoted_group(currently_approved=False) + ) + already_promoted.reload().promoted_addons.first().reload() + assert LINE in already_promoted.promoted_group(currently_approved=False) + promoted_record_exists.reload().promoted_addons.first().reload() + assert NOTABLE in promoted_record_exists.promoted_group(currently_approved=False) + assert NOTABLE in unlisted_only_extension.promoted_group(currently_approved=False) + assert NOTABLE in mixed_extension.promoted_group(currently_approved=False) + assert NOTABLE in deleted_extension.promoted_group(currently_approved=False) generator = get_staggered_review_due_date_generator(starting=now) diff --git a/src/olympia/reviewers/models.py b/src/olympia/reviewers/models.py index a0db883576f..948afca3e4e 100644 --- a/src/olympia/reviewers/models.py +++ b/src/olympia/reviewers/models.py @@ -11,6 +11,7 @@ from extended_choices import Choices +from olympia.constants.promoted import LISTED_PRE_REVIEW, UNLISTED_PRE_REVIEW import olympia.core.logger from olympia import activity, amo, core from olympia.abuse.models import AbuseReport, CinderPolicy @@ -147,7 +148,8 @@ def get_flags(addon, version): ] # add in the promoted group flag and return if promoted := addon.promoted_group(currently_approved=False): - flags.append((f'promoted-{promoted.api_name}', promoted.name)) + for group in promoted: + flags.append((f'promoted-{group.api_name}', group.name)) return flags @@ -631,16 +633,13 @@ def check_is_promoted_prereview(cls, version): """Check whether the add-on is a promoted addon group that requires pre-review.""" return bool( - (promo_group := version.addon.promoted_group(currently_approved=False)) - and ( - ( - version.channel == amo.CHANNEL_LISTED - and promo_group.listed_pre_review - ) - or ( - version.channel == amo.CHANNEL_UNLISTED - and promo_group.unlisted_pre_review - ) + ( + version.channel == amo.CHANNEL_LISTED + and version.addon.get(LISTED_PRE_REVIEW, currently_approved=False) + ) + or ( + version.channel == amo.CHANNEL_UNLISTED + and version.addon.get(UNLISTED_PRE_REVIEW, currently_approved=False) ) ) diff --git a/src/olympia/reviewers/templates/reviewers/review.html b/src/olympia/reviewers/templates/reviewers/review.html index 4db648aabf4..75b934fc54d 100644 --- a/src/olympia/reviewers/templates/reviewers/review.html +++ b/src/olympia/reviewers/templates/reviewers/review.html @@ -349,7 +349,7 @@