Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Multiple Partner Addons #22888

Open
wants to merge 38 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
16d4576
Allow multiple PromotedAddons to a single Addon
chrstinalin Nov 25, 2024
91cc3a5
format
chrstinalin Nov 25, 2024
e8e905c
test changes
chrstinalin Nov 25, 2024
570d40d
no prefetch?
chrstinalin Nov 25, 2024
728795a
refresh from db
chrstinalin Nov 26, 2024
29cb597
multi state
chrstinalin Nov 26, 2024
6eaa568
refresh
chrstinalin Nov 26, 2024
40dd1d5
tests
chrstinalin Nov 26, 2024
0ee69e4
more tests
chrstinalin Nov 26, 2024
6431d51
test
chrstinalin Nov 27, 2024
549fb43
how badly does this break
chrstinalin Nov 28, 2024
dc00e17
prefetch
chrstinalin Nov 28, 2024
b254f27
fakedatetime object
chrstinalin Nov 28, 2024
4cc40f9
lint
chrstinalin Nov 28, 2024
188c101
catch dne
chrstinalin Nov 28, 2024
cf8f017
tests
chrstinalin Nov 28, 2024
9957a1f
more tests
chrstinalin Nov 28, 2024
baf0edd
main tests
chrstinalin Nov 29, 2024
3d22f81
elastic
chrstinalin Nov 29, 2024
7bee3dc
queryparam
chrstinalin Nov 29, 2024
5a4c9af
es
chrstinalin Nov 29, 2024
b4bf689
query count comments
chrstinalin Nov 29, 2024
6603877
refactor promoted_group to promoted_groups
chrstinalin Dec 2, 2024
d49f84e
codestyle
chrstinalin Dec 2, 2024
25396f6
approvals
chrstinalin Dec 3, 2024
9fb4e99
iterate all
chrstinalin Dec 3, 2024
d2ca683
all_apps
chrstinalin Dec 3, 2024
341ecbd
addon model tests
chrstinalin Dec 4, 2024
7d2f9e6
test fix
chrstinalin Dec 4, 2024
5ec5b4b
tests
chrstinalin Dec 4, 2024
8019034
test
chrstinalin Dec 4, 2024
12efa09
docs, conflict
chrstinalin Dec 19, 2024
0d6ad41
lint
chrstinalin Dec 19, 2024
d29a0ce
tests
chrstinalin Dec 19, 2024
3d7b7a7
add shim for api v3/v4
chrstinalin Dec 19, 2024
9c60b7b
whoops
chrstinalin Dec 19, 2024
5034b0f
tests
chrstinalin Dec 19, 2024
cde026a
safer tests
chrstinalin Jan 10, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions src/olympia/abuse/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from olympia.amo.templatetags.jinja_helpers import absolutify
from olympia.amo.utils import send_mail
from olympia.bandwagon.models import Collection
from olympia.constants.promoted import HIGH_PROFILE, HIGH_PROFILE_RATING
from olympia.ratings.models import Rating
from olympia.users.models import UserProfile

Expand Down Expand Up @@ -249,10 +250,7 @@ def should_hold_action(self):
self.target.is_staff # mozilla.com
or self.target.groups_list # has any permissions
# owns a high profile add-on
or any(
addon.promoted_group().high_profile
for addon in self.target.addons.all()
)
or any(addon.get(HIGH_PROFILE) for addon in self.target.addons.all())
)
)

Expand Down Expand Up @@ -281,7 +279,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):
Expand Down Expand Up @@ -369,7 +367,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):
Expand Down
8 changes: 5 additions & 3 deletions src/olympia/abuse/cinder.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,9 @@ class CinderUser(CinderEntity):
def __init__(self, user):
self.user = user
self.related_addons = (
self.user.addons.all().only_translations().select_related('promotedaddon')
self.user.addons.all()
.only_translations()
.prefetch_related('promoted_addons')
)

@property
Expand Down Expand Up @@ -331,7 +333,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,
Expand All @@ -341,7 +343,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

Expand Down
8 changes: 3 additions & 5 deletions src/olympia/abuse/tests/test_cinder.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,10 @@ def test_get_str(self):

class TestCinderAddon(BaseTestCinderCase, TestCase):
CinderClass = CinderAddon
# 2 queries expected:
# 1 queries expected:
# - Authors (can't use the listed_authors transformer, we want non-listed as well,
# and we have custom limits for batch-sending relationships)
# - Promoted add-on
expected_queries_for_report = 2
expected_queries_for_report = 1
expected_queue_suffix = 'listings'

def _create_dummy_target(self, **kwargs):
Expand Down Expand Up @@ -1096,8 +1095,7 @@ class TestCinderAddonHandledByReviewers(TestCinderAddon):
CinderClass = CinderAddonHandledByReviewers
# For rendering the payload to Cinder like CinderAddon:
# - 1 Fetch Addon authors
# - 2 Fetch Promoted Addon
expected_queries_for_report = 2
expected_queries_for_report = 1
expected_queue_suffix = 'addon-infringement'

def test_queue_with_theme(self):
Expand Down
6 changes: 4 additions & 2 deletions src/olympia/activity/tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def test_list(self):
self.grant_permission(user, '*:*')
self.client.force_login(user)

with self.assertNumQueries(11):
with self.assertNumQueries(12):
# - 2 savepoints/release
# - 2 user and groups
# - 1 count for pagination
Expand All @@ -40,6 +40,7 @@ def test_list(self):
# - 1 all versions from activities
# - 1 all translations from those versions
# - 1 all add-ons from activities
# - 1 add-ons' promoted addon
# - 1 all translations for those add-ons
response = self.client.get(self.list_url)
assert response.status_code == 200
Expand All @@ -65,7 +66,7 @@ def test_search_for_single_ip(self):
with core.override_remote_addr('127.0.0.1'):
extra_user = user_factory() # Extra user that shouldn't match
ActivityLog.objects.create(amo.LOG.LOG_IN, user=extra_user)
with self.assertNumQueries(11):
with self.assertNumQueries(12):
# - 2 savepoints/release
# - 2 user and groups
# - 1 count for pagination
Expand All @@ -74,6 +75,7 @@ def test_search_for_single_ip(self):
# - 1 all versions from activities
# - 1 all translations from those versions
# - 1 all add-ons from activities
# - 1 add-ons' promoted addon
# - 1 all translations for those add-ons
response = self.client.get(self.list_url, {'q': '127.0.0.2'}, follow=True)
assert response.status_code == 200
Expand Down
9 changes: 6 additions & 3 deletions src/olympia/activity/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,10 +328,11 @@ def test_arguments_old_reviews_app_to_ratings(self):
assert activity.arguments == [addon, rating]
activity.save()

with self.assertNumQueries(5):
with self.assertNumQueries(6):
# - 1 for all activities
# - 1 for all users
# - 1 for all addons
# - 1 for addons' promoted addon
# - 1 for all add-on translations
# - 1 for all ratings
activity = ActivityLog.objects.latest('pk')
Expand All @@ -342,10 +343,11 @@ def test_arguments_old_reviews_app_to_ratings(self):
addon2 = addon_factory(slug='foo')
user2 = user_factory()
rating2 = Rating.objects.create(addon=addon2, user=user2, rating=2)
with self.assertNumQueries(5):
with self.assertNumQueries(6):
# - 1 for all activities
# - 1 for all users
# - 1 for all addons
# - 1 for addons' promoted addon
# - 1 for all add-on translations
# - 1 for all ratings
activities = ActivityLog.objects.for_addons([addon, addon2]).order_by('pk')
Expand Down Expand Up @@ -387,10 +389,11 @@ def test_to_string_num_queries_model_depending_on_addon(self):
addon2.current_version,
user=user_factory(),
)
with self.assertNumQueries(6):
with self.assertNumQueries(7):
# - 1 for all activities
# - 1 for all users
# - 1 for all addons
# - 1 for addons' promoted addon
# - 1 for all add-on translations
# - 1 for all versions
# - 1 for all versions translations
Expand Down
3 changes: 2 additions & 1 deletion src/olympia/activity/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,10 +302,11 @@ def test_queries(self):
'fiiiine', amo.LOG.REVIEWER_REPLY_VERSION, self.days_ago(0)
)
self._login_developer()
with self.assertNumQueries(16):
with self.assertNumQueries(18):
# - 2 savepoints because of tests
# - 2 user and groups
# - 2 addon and its translations
# - 2 addons' promoted addons
# - 1 addon author lookup (permission check)
# - 1 version (no transforms at all)
# - 1 count of activity logs
Expand Down
12 changes: 5 additions & 7 deletions src/olympia/addons/indexers.py
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ def get_mapping(cls):
'promoted': {
'type': 'object',
'properties': {
'group_id': {'type': 'byte'},
'group_ids': {'type': 'byte'},
'approved_for_apps': {'type': 'byte'},
},
},
Expand Down Expand Up @@ -661,8 +661,8 @@ def extract_document(cls, obj):
data['has_eula'] = bool(obj.eula)
data['has_privacy_policy'] = bool(obj.privacy_policy)

data['is_recommended'] = bool(
obj.promoted and obj.promoted.group == RECOMMENDED
data['is_recommended'] = any(
RECOMMENDED.id == promoted.group_id for promoted in obj.promoted
)

data['previews'] = [
Expand All @@ -677,11 +677,9 @@ 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
],
'approved_for_apps': [app.id for app in obj.approved_applications],
}
if obj.promoted
else None
Expand Down
121 changes: 101 additions & 20 deletions src/olympia/addons/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
from django.urls import reverse
from django.utils import translation
from django.utils.functional import cached_property
from django.utils.translation import gettext_lazy as _, trans_real
from django.utils.translation import gettext, gettext_lazy as _, trans_real

from django_statsd.clients import statsd

Expand Down Expand Up @@ -57,7 +57,12 @@
)
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
Expand Down Expand Up @@ -237,7 +242,7 @@ def __init__(self, include_deleted=False):
self.include_deleted = include_deleted

def get_queryset(self):
qs = super().get_queryset()
qs = super().get_queryset().prefetch_related('promoted_addons')
if not self.include_deleted:
qs = qs.exclude(status=amo.STATUS_DELETED)
return qs.transform(Addon.transformer)
Expand Down Expand Up @@ -279,7 +284,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
Expand Down Expand Up @@ -1553,37 +1557,114 @@ def _is_recommended_theme(self):
).exists()
)

def promoted_group(self, *, currently_approved=True):
def promoted_groups(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.
If currently_approved=False then promotions where there isn't approval
are returned too.
"""
from olympia.promoted.models import PromotedAddon

try:
promoted = self.promotedaddon
except PromotedAddon.DoesNotExist:
return NOT_PROMOTED
is_promoted = not currently_approved or promoted.approved_applications
return promoted.group if is_promoted else NOT_PROMOTED
promoted_addons = self.promoted_addons.all()
except ValueError:
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_groups(currently_approved=currently_approved)
return ', '.join(
gettext(group.name) for group in groups if group != NOT_PROMOTED
)

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
Default -> set of truthy permissions (ex. set of dicts)

`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_groups(currently_approved=currently_approved)
type = PromotedClass.type(permission)

if type is int:
return max(
[
getattr(group, permission)
for group in groups
if getattr(group, permission) is not None
],
default=0,
)
if type is bool:
return any(getattr(group, permission, False) for group in groups)
list = []
for group in groups:
value = getattr(group, permission, None)
if value:
list.append(value)
return list

@property
def group_ids(self):
groups = self.promoted
return [group.group_id for group in groups]

@property
def all_applications(self):
all_apps = set()
for promoted in self.promoted_addons.all():
all_apps.update(promoted.all_applications)
return list(all_apps)

@property
def approved_applications(self):
approved_apps = set()
for promoted in self.promoted:
approved_apps.update(promoted.approved_applications)
return list(approved_apps)

@cached_property
def promoted(self):
promoted_group = self.promoted_group()
if promoted_group:
return self.promotedaddon
promoted_groups = self.promoted_groups()
if promoted_groups:
return [
promoted
for promoted in self.promoted_addons.all()
if promoted.approved_applications
]
else:
from olympia.promoted.models import PromotedTheme

if self._is_recommended_theme():
return PromotedTheme(addon=self, group_id=RECOMMENDED.id)
return None
return [PromotedTheme(addon=self, group_id=RECOMMENDED.id)]
return []

def approve_for_version(self, version):
"""Approves all associated PromotedAddons to the addon."""
for promoted in self.promoted_addons.all():
promoted.approve_for_version(version)

@cached_property
def compatible_apps(self):
Expand All @@ -1608,8 +1689,8 @@ 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 amo.ANDROID in self.promoted.approved_applications
and self.get(CAN_BE_COMPATIBLE_WITH_ALL_FENIX_VERSIONS)
and amo.ANDROID in self.approved_applications
)

def has_author(self, user):
Expand Down
Loading
Loading