Skip to content

Commit

Permalink
Don't flag add-ons for growth threshold if hotness is negative (#23031)
Browse files Browse the repository at this point in the history
* Don't flag add-ons for growth threshold if hotness is negative
  • Loading branch information
diox authored Jan 30, 2025
1 parent 3c769ea commit ad4569b
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 19 deletions.
2 changes: 0 additions & 2 deletions src/olympia/addons/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -540,8 +540,6 @@ def flag_high_hotness_according_to_review_tier():
base_qs = UsageTier.get_base_addons()
hotness_per_tier_filters = Q()
for usage_tier in usage_tiers:
# Note: get_growth_threshold_q_object can return an empty Q() object
# if the average hotness for that tier is exactly 0.
hotness_per_tier_filters |= usage_tier.get_growth_threshold_q_object()
if not hotness_per_tier_filters:
return
Expand Down
12 changes: 7 additions & 5 deletions src/olympia/addons/tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -451,13 +451,15 @@ def test_flag_high_hotness_according_to_review_tier_threshold_check_negative():
upper_adu_threshold=1000,
growth_threshold_before_flagging=10,
)
# Average hotness should be negative, -0.1.
# Average hotness should be negative, -0.2.
# growth_threshold_before_flagging for that tier is 10 so we should flag
# add-ons with hotness above 0
# add-ons with hotness above -0.1, but we explicitly only flag positive
# growth add-ons, so only the last one should be flagged.

addon_factory(average_daily_users=251, hotness=-0.1)
addon_factory(average_daily_users=251, hotness=-0.2)
addon = addon_factory(average_daily_users=251, hotness=0.0)
addon_factory(average_daily_users=251, hotness=-0.05)
addon_factory(average_daily_users=251, hotness=-0.85)
addon_factory(average_daily_users=251, hotness=0.0)
addon = addon_factory(average_daily_users=251, hotness=0.1)
File.objects.update(is_signed=True)

flag_high_hotness_according_to_review_tier()
Expand Down
22 changes: 10 additions & 12 deletions src/olympia/reviewers/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -832,28 +832,26 @@ def average_growth(self):
self.get_base_addons()
.filter(**self.get_tier_boundaries())
.aggregate(Avg('hotness', default=0))
.get('hotness__avg')
.get('hotness__avg', 0)
)

def get_growth_threshold(self):
"""Return the growth threshold for that tier, or None.
"""Return the growth threshold for that tier.
The value is computed from the average growth (hotness) of add-ons in
that tier plus the growth_threshold_before_flagging converted to
decimal."""
return self.average_growth + self.growth_threshold_before_flagging / 100
decimal.
It has a floor of 0."""
return max(0, self.average_growth + self.growth_threshold_before_flagging / 100)

def get_growth_threshold_q_object(self):
"""Return Q object containing filters to apply to find add-ons over the
computed growth threshold for that tier."""
hotness_ceiling_before_flagging = self.get_growth_threshold()
if hotness_ceiling_before_flagging is not None:
return Q(
hotness__gt=hotness_ceiling_before_flagging,
**self.get_tier_boundaries(),
)
else:
return Q()
return Q(
hotness__gt=self.get_growth_threshold(),
**self.get_tier_boundaries(),
)


class NeedsHumanReview(ModelBase):
Expand Down
26 changes: 26 additions & 0 deletions src/olympia/reviewers/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1919,6 +1919,16 @@ def test_get_growth_threshold(self):
del self.tier.average_growth
assert round(self.tier.get_growth_threshold(), ndigits=2) == 0.77

def test_get_growth_threshold_zero_floor_instead_of_negative(self):
addon_factory(hotness=-0.4, average_daily_users=100)
addon_factory(hotness=-0.4, average_daily_users=999)
assert round(self.tier.get_growth_threshold(), ndigits=2) == 0.1

addon_factory(hotness=-0.9, average_daily_users=999)
addon_factory(hotness=-0.9, average_daily_users=999)
del self.tier.average_growth
assert round(self.tier.get_growth_threshold(), ndigits=2) == 0 # Not -0.15

def test_get_growth_threshold_q_object(self):
addon_factory(hotness=0.01, average_daily_users=100)
addon_factory(hotness=0.01, average_daily_users=999)
Expand All @@ -1928,3 +1938,19 @@ def test_get_growth_threshold_q_object(self):
list(Addon.objects.filter(self.tier.get_growth_threshold_q_object()))
== expected
)

def test_get_growth_threshold_q_object_hotness_needs_to_be_higher_than(self):
addon_factory(hotness=0.5, average_daily_users=999)
expected = [addon_factory(hotness=0.501, average_daily_users=999)]

# Override computed average growth to force the growth threshold to
# 0.5 (0.0 + 50/100)
self.tier.average_growth = 0.0
assert self.tier.get_growth_threshold() == 0.5

# We filter on hotness_gt in get_growth_threshold_q_object() so the
# first add-on shouldn't be returned, only the second one.
assert (
list(Addon.objects.filter(self.tier.get_growth_threshold_q_object()))
== expected
)

0 comments on commit ad4569b

Please sign in to comment.