Skip to content

Commit 94394f7

Browse files
eviljeffeviljeff
committed
Replace last_content_review_pass with last_content_review_status
1 parent a4b726f commit 94394f7

File tree

7 files changed

+116
-39
lines changed

7 files changed

+116
-39
lines changed

‎src/olympia/abuse/actions.py‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -700,7 +700,12 @@ def process_action(self, release_hold=False):
700700
if self.target.status not in (amo.STATUS_DISABLED, amo.STATUS_REJECTED):
701701
self.target.update(status=amo.STATUS_REJECTED)
702702
AddonApprovalsCounter.objects.update_or_create(
703-
addon=self.target, defaults={'last_content_review_pass': False}
703+
addon=self.target,
704+
defaults={
705+
'last_content_review_status': (
706+
AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL
707+
)
708+
},
704709
)
705710
return self.log_action(amo.LOG.REJECT_LISTING_CONTENT)
706711

@@ -889,7 +894,9 @@ def process_action(self, release_hold=False):
889894
addon=target,
890895
defaults={
891896
'last_content_review': datetime.now(),
892-
'last_content_review_pass': True,
897+
'last_content_review_status': (
898+
AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PASS
899+
),
893900
},
894901
)
895902
# Call the function to correct it the status
@@ -970,7 +977,9 @@ def process_action(self, release_hold=False):
970977
addon=self.target,
971978
defaults={
972979
'last_content_review': datetime.now(),
973-
'last_content_review_pass': True,
980+
'last_content_review_status': (
981+
AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PASS
982+
),
974983
},
975984
)
976985
if self.status == amo.STATUS_REJECTED:

‎src/olympia/abuse/tests/test_actions.py‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -786,7 +786,10 @@ def _test_reporter_content_approved_action_taken(self):
786786
assert second_activity.details == {'comments': self.decision.private_notes}
787787

788788
counter = AddonApprovalsCounter.objects.get(addon=self.addon)
789-
assert counter.last_content_review_pass is True
789+
assert (
790+
counter.last_content_review_status
791+
== AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PASS
792+
)
790793
self.assertCloseToNow(counter.last_content_review)
791794

792795
# get this again, to replicate how send_notifications works
@@ -798,7 +801,8 @@ def _test_reporter_content_approved_action_taken(self):
798801

799802
def test_content_approve_rejected_listing_content(self):
800803
AddonApprovalsCounter.objects.create(
801-
addon=self.addon, last_content_review_pass=False
804+
addon=self.addon,
805+
last_content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.REQUESTED,
802806
)
803807
self.addon.update(status=amo.STATUS_REJECTED)
804808
action = DECISION_ACTIONS.AMO_APPROVE
@@ -828,7 +832,10 @@ def test_content_approve_rejected_listing_content(self):
828832
assert third_activity.log == amo.LOG.CHANGE_STATUS
829833

830834
counter = AddonApprovalsCounter.objects.get(addon=self.addon)
831-
assert counter.last_content_review_pass is True
835+
assert (
836+
counter.last_content_review_status
837+
== AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PASS
838+
)
832839
self.assertCloseToNow(counter.last_content_review)
833840

834841
# get this again, to replicate how send_notifications works
@@ -1165,7 +1172,8 @@ def _test_approve_appeal_or_override_but_listing_rejected(self, ContentActionCla
11651172

11661173
def test_approve_appeal_success_but_listing_rejected(self):
11671174
AddonApprovalsCounter.objects.create(
1168-
addon=self.addon, last_content_review_pass=False
1175+
addon=self.addon,
1176+
last_content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL,
11691177
)
11701178
self.past_negative_decision.update(appeal_job=self.cinder_job)
11711179
self._test_approve_appeal_or_override_but_listing_rejected(
@@ -1176,7 +1184,8 @@ def test_approve_appeal_success_but_listing_rejected(self):
11761184

11771185
def test_approve_override_success_but_listing_rejected(self):
11781186
AddonApprovalsCounter.objects.create(
1179-
addon=self.addon, last_content_review_pass=False
1187+
addon=self.addon,
1188+
last_content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL,
11801189
)
11811190
self.decision.update(override_of=self.past_negative_decision)
11821191
self._test_approve_appeal_or_override_but_listing_rejected(
@@ -1305,7 +1314,8 @@ def _test_approve_appeal_or_override(self, ContentActionClass):
13051314
def test_approve_appeal_success_but_listing_rejected(self):
13061315
self.addon.update(status=amo.STATUS_REJECTED)
13071316
AddonApprovalsCounter.objects.create(
1308-
addon=self.addon, last_content_review_pass=False
1317+
addon=self.addon,
1318+
last_content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL,
13091319
)
13101320
self.past_negative_decision.update(appeal_job=self.cinder_job)
13111321
self._test_approve_appeal_or_override(ContentActionTargetAppealApprove)
@@ -1315,7 +1325,8 @@ def test_approve_appeal_success_but_listing_rejected(self):
13151325
def test_approve_override_success_but_listing_rejected(self):
13161326
self.addon.update(status=amo.STATUS_REJECTED)
13171327
AddonApprovalsCounter.objects.create(
1318-
addon=self.addon, last_content_review_pass=False
1328+
addon=self.addon,
1329+
last_content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL,
13191330
)
13201331
self.decision.update(override_of=self.past_negative_decision)
13211332
self._test_approve_appeal_or_override(ContentActionOverrideApprove)

‎src/olympia/addons/models.py‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
from olympia import activity, amo, core
3535
from olympia.addons.utils import generate_addon_guid
3636
from olympia.amo.decorators import use_primary_db
37+
from olympia.amo.enum import EnumChoices
3738
from olympia.amo.fields import PositiveAutoField
3839
from olympia.amo.models import (
3940
BasePreview,
@@ -1298,7 +1299,8 @@ def update_status(self, ignore_version=None):
12981299
correct_status = self.status
12991300
force_update = False
13001301
if AddonApprovalsCounter.objects.filter(
1301-
addon=self, last_content_review_pass=False
1302+
addon=self,
1303+
last_content_review_status__in=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.NEGATIVES,
13021304
).exists():
13031305
# If the content review failed, the status should stay as rejected
13041306
correct_status = amo.STATUS_REJECTED
@@ -2408,14 +2410,26 @@ class AddonApprovalsCounter(ModelBase):
24082410
add-on
24092411
- last_content_review, the date of the last time a human fully reviewed the
24102412
add-on content (not code).
2411-
- last_content_review_pass, a boolean indicating if the last content review passed.
2413+
- last_content_review_status, the status of the last content review.
24122414
"""
24132415

2416+
class CONTENT_REVIEW_STATUSES(EnumChoices):
2417+
UNREVIEWED = 0, 'Unreviewed'
2418+
PASS = 1, 'Pass'
2419+
FAIL = 2, 'Fail'
2420+
REQUESTED = 3, 'New review requested'
2421+
2422+
CONTENT_REVIEW_STATUSES.add_subset('NEGATIVES', ('FAIL', 'REQUESTED'))
2423+
24142424
addon = models.OneToOneField(Addon, primary_key=True, on_delete=models.CASCADE)
24152425
counter = models.PositiveIntegerField(default=0)
24162426
last_human_review = models.DateTimeField(null=True)
24172427
last_content_review = models.DateTimeField(null=True, db_index=True)
2418-
last_content_review_pass = models.BooleanField(null=True, db_index=True)
2428+
last_content_review_status = models.SmallIntegerField(
2429+
choices=CONTENT_REVIEW_STATUSES.choices,
2430+
null=True,
2431+
db_index=True,
2432+
)
24192433

24202434
def __str__(self):
24212435
return '%s: %d' % (str(self.pk), self.counter) if self.pk else ''
@@ -2453,13 +2467,16 @@ def reset_for_addon(cls, addon):
24532467
@classmethod
24542468
def approve_content_for_addon(cls, addon):
24552469
"""
2456-
Set last_content_review and last_content_review_pass to True for this addon.
2470+
Set last_content_review and last_content_review_status to
2471+
CONTENT_REVIEW_STATUSES.PASS for this addon.
24572472
"""
24582473
obj, _ = cls.objects.update_or_create(
24592474
addon=addon,
24602475
defaults={
24612476
'last_content_review': datetime.now(),
2462-
'last_content_review_pass': True,
2477+
'last_content_review_status': (
2478+
AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PASS
2479+
),
24632480
},
24642481
)
24652482
return obj

‎src/olympia/addons/tests/test_models.py‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -906,7 +906,8 @@ def test_force_enable_back_to_rejected(
906906
v2 = version_factory(addon=addon)
907907
v3 = version_factory(addon=addon)
908908
AddonApprovalsCounter.objects.create(
909-
addon=addon, last_content_review_pass=False
909+
addon=addon,
910+
last_content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL,
910911
)
911912
addon.update(status=amo.STATUS_DISABLED)
912913
v1.file.update(
@@ -2941,7 +2942,8 @@ def test_rejected_listing_addons_do_not_update(self):
29412942
status=amo.STATUS_REJECTED, file_kw={'status': amo.STATUS_DISABLED}
29422943
)
29432944
AddonApprovalsCounter.objects.create(
2944-
addon=addon, last_content_review_pass=False
2945+
addon=addon,
2946+
last_content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL,
29452947
)
29462948
assert addon.status == amo.STATUS_REJECTED
29472949

@@ -2957,7 +2959,7 @@ def test_rejected_listing_addons_do_not_update(self):
29572959

29582960
# but if the content review passed, the status will update
29592961
AddonApprovalsCounter.objects.get(addon=addon).update(
2960-
last_content_review_pass=True
2962+
last_content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PASS
29612963
)
29622964
addon.update_status()
29632965
assert addon.status == amo.STATUS_APPROVED

‎src/olympia/devhub/tests/test_views_versions.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,8 @@ def test_version_delete_status_null(self):
191191

192192
def test_version_delete_with_rejected_listing(self):
193193
AddonApprovalsCounter.objects.create(
194-
addon=self.addon, last_content_review_pass=False
194+
addon=self.addon,
195+
last_content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL,
195196
)
196197
response = self.client.post(self.delete_url, self.delete_data)
197198
assert response.status_code == 302
@@ -209,7 +210,8 @@ def test_disable_version(self):
209210

210211
def test_disable_version_with_rejected_listing(self):
211212
AddonApprovalsCounter.objects.create(
212-
addon=self.addon, last_content_review_pass=False
213+
addon=self.addon,
214+
last_content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL,
213215
)
214216
self.test_disable_version()
215217
assert Addon.objects.get(id=3615).status == amo.STATUS_REJECTED

‎src/olympia/reviewers/tests/test_utils.py‎

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,7 @@ def test_actions_content_review_rejected(self):
417417
]
418418
AddonApprovalsCounter.objects.create(
419419
addon=self.addon,
420-
last_content_review_pass=False,
420+
last_content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL,
421421
)
422422
assert (
423423
list(
@@ -1922,7 +1922,8 @@ def test_nomination_but_rejected_to_public(self):
19221922
self.sign_file_mock.reset()
19231923
self.setup_data(amo.STATUS_REJECTED)
19241924
AddonApprovalsCounter.objects.create(
1925-
addon=self.addon, last_content_review_pass=False
1925+
addon=self.addon,
1926+
last_content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL,
19261927
)
19271928
AutoApprovalSummary.objects.update_or_create(
19281929
version=self.review_version,
@@ -1942,7 +1943,10 @@ def test_nomination_but_rejected_to_public(self):
19421943
# AddonApprovalsCounter counter is now at 1 for this addon.
19431944
approval_counter = AddonApprovalsCounter.objects.get(addon=self.addon)
19441945
assert approval_counter.counter == 1
1945-
assert approval_counter.last_content_review_pass is False # hasn't changed
1946+
assert (
1947+
approval_counter.last_content_review_status
1948+
== AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL
1949+
) # hasn't changed
19461950

19471951
self.sign_file_mock.assert_called_with(self.file)
19481952
assert storage.exists(self.file.file.path)
@@ -1995,7 +1999,8 @@ def test_nomination_to_public_not_human(self):
19951999
def test_nomination_but_listing_rejected_to_public_not_human(self):
19962000
self.setup_data(amo.STATUS_REJECTED, human_review=False)
19972001
approval_counter = AddonApprovalsCounter.objects.create(
1998-
addon=self.addon, last_content_review_pass=False
2002+
addon=self.addon,
2003+
last_content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL,
19992004
)
20002005
self.sign_file_mock.reset()
20012006

@@ -3559,7 +3564,10 @@ def test_approve_listing_content_review(self):
35593564
assert approvals_counter.counter == 0
35603565
assert approvals_counter.last_human_review is None
35613566
self.assertCloseToNow(approvals_counter.last_content_review)
3562-
assert approvals_counter.last_content_review_pass is True
3567+
assert (
3568+
approvals_counter.last_content_review_status
3569+
== AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PASS
3570+
)
35633571
assert self.check_log_count(amo.LOG.CONFIRM_AUTO_APPROVED.id) == 0
35643572
assert self.check_log_count(amo.LOG.APPROVE_LISTING_CONTENT.id) == 1
35653573
activity = (
@@ -3604,7 +3612,10 @@ def test_approve_listing_content_review_with_policies(self):
36043612
assert approvals_counter.counter == 0
36053613
assert approvals_counter.last_human_review is None
36063614
self.assertCloseToNow(approvals_counter.last_content_review)
3607-
assert approvals_counter.last_content_review_pass is True
3615+
assert (
3616+
approvals_counter.last_content_review_status
3617+
== AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PASS
3618+
)
36083619
assert self.check_log_count(amo.LOG.CONFIRM_AUTO_APPROVED.id) == 0
36093620
assert self.check_log_count(amo.LOG.APPROVE_LISTING_CONTENT.id) == 1
36103621
activity = (
@@ -3622,7 +3633,8 @@ def test_approve_listing_content_review_with_policies(self):
36223633
def test_approve_rejected_listing_content_review(self):
36233634
self.grant_permission(self.user, 'Addons:ContentReview')
36243635
approvals_counter = AddonApprovalsCounter.objects.create(
3625-
addon=self.addon, last_content_review_pass=False
3636+
addon=self.addon,
3637+
last_content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.REQUESTED,
36263638
)
36273639
self.setup_data(
36283640
amo.STATUS_REJECTED, file_status=amo.STATUS_APPROVED, content_review=True
@@ -3647,7 +3659,10 @@ def test_approve_rejected_listing_content_review(self):
36473659
assert approvals_counter.reload().counter == 0
36483660
assert approvals_counter.last_human_review is None
36493661
self.assertCloseToNow(approvals_counter.last_content_review)
3650-
assert approvals_counter.last_content_review_pass is True
3662+
assert (
3663+
approvals_counter.last_content_review_status
3664+
== AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PASS
3665+
)
36513666
assert self.check_log_count(amo.LOG.CONFIRM_AUTO_APPROVED.id) == 0
36523667
assert self.check_log_count(amo.LOG.APPROVE_REJECTED_LISTING_CONTENT.id) == 1
36533668
activity = (
@@ -3668,7 +3683,8 @@ def test_approve_rejected_listing_content_review(self):
36683683
def test_approve_rejected_listing_content_review_with_policies(self):
36693684
self.grant_permission(self.user, 'Addons:ContentReview')
36703685
approvals_counter = AddonApprovalsCounter.objects.create(
3671-
addon=self.addon, last_content_review_pass=False
3686+
addon=self.addon,
3687+
last_content_review_status=AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.REQUESTED,
36723688
)
36733689
self.setup_data(
36743690
amo.STATUS_REJECTED, file_status=amo.STATUS_APPROVED, content_review=True
@@ -3697,7 +3713,10 @@ def test_approve_rejected_listing_content_review_with_policies(self):
36973713
assert approvals_counter.reload().counter == 0
36983714
assert approvals_counter.last_human_review is None
36993715
self.assertCloseToNow(approvals_counter.last_content_review)
3700-
assert approvals_counter.last_content_review_pass is True
3716+
assert (
3717+
approvals_counter.last_content_review_status
3718+
== AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.PASS
3719+
)
37013720
assert self.check_log_count(amo.LOG.CONFIRM_AUTO_APPROVED.id) == 0
37023721
assert self.check_log_count(amo.LOG.APPROVE_REJECTED_LISTING_CONTENT.id) == 1
37033722
activity = (
@@ -3743,7 +3762,10 @@ def test_reject_listing_content_review(self):
37433762
assert approvals_counter.counter == 0
37443763
assert approvals_counter.last_human_review is None
37453764
assert approvals_counter.last_content_review is None
3746-
assert approvals_counter.last_content_review_pass is False
3765+
assert (
3766+
approvals_counter.last_content_review_status
3767+
== AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL
3768+
)
37473769
assert self.check_log_count(amo.LOG.REJECT_LISTING_CONTENT.id) == 1
37483770
activity = (
37493771
ActivityLog.objects.for_addons(self.addon)
@@ -3800,7 +3822,10 @@ def test_reject_listing_content_review_with_policies(self):
38003822
assert approvals_counter.counter == 0
38013823
assert approvals_counter.last_human_review is None
38023824
assert approvals_counter.last_content_review is None
3803-
assert approvals_counter.last_content_review_pass is False
3825+
assert (
3826+
approvals_counter.last_content_review_status
3827+
== AddonApprovalsCounter.CONTENT_REVIEW_STATUSES.FAIL
3828+
)
38043829
assert self.check_log_count(amo.LOG.REJECT_LISTING_CONTENT.id) == 1
38053830
activity = (
38063831
ActivityLog.objects.for_addons(self.addon)

0 commit comments

Comments
 (0)