Add the approve_addons management command (bug 1199234)

apps/addons/management/commands/approve_addons.py

@@ -0,0 +1,100 @@
+# -*- coding: utf-8 -*-
+import logging
+
+from django.core.management.base import BaseCommand, CommandError
+
+import amo
+from addons.models import Addon
+from amo.utils import chunked
+from editors.helpers import ReviewHelper
+
+log = logging.getLogger('z.addons')
+
+
+class Command(BaseCommand):
+    args = u'<addon_guid addon_guid ...>'
+    help = u'Approve a list of add-ons, given their GUIDs.'
+
+    def handle(self, *args, **options):
+        if len(args) == 0:  # No GUID provided?
+            raise CommandError(
+                u'Please provide at least one add-on guid to approve.')
+
+        confirm = raw_input(
+            u'Are you sure you want to bulk approve and sign all those {0} '
+            u'addons? (yes/no)'.format(len(args)))
+        if confirm != 'yes':
+            raise CommandError(u'Aborted.')
+
+        for chunk in chunked(args, 100):
+            files = get_files(chunk)
+            log.info(u'Bulk approving chunk of %s files', len(files))
+
+            files_with_review_type = [
+                (file_, get_review_type(file_)) for file_ in files]
+
+            approve_files(files_with_review_type)
+
+
+def get_files(addon_guids):
+    """Return the list of files that need approval, given a list of GUIDs.
+
+    A file needs approval if:
+    - it's unreviewed
+    - it's preliminary reviewed, but its addon is requesting a full review
+    """
+    # Get all the add-ons that have a GUID from the list, and which are either
+    # reviewed or awaiting a review.
+    addons = Addon.with_unlisted.filter(
+        guid__in=addon_guids,
+        status__in=amo.UNDER_REVIEW_STATUSES + amo.REVIEWED_STATUSES)
+    # Of all those add-ons, we return the list of latest_version files that are
+    # under review, or add-ons that are under review.
+    files = []
+    for addon in addons:
+        for file_ in addon.latest_version.files.all():
+            if (file_.status in amo.UNDER_REVIEW_STATUSES or
+                    addon.status in amo.UNDER_REVIEW_STATUSES):
+                files.append(file_)
+    return files
+
+
+def approve_files(files_with_review_type):
+    """Approve the files (and sign them).
+
+    A file will be fully approved if:
+    - it's waiting for a full review
+    - it's preliminary reviewed, and its addon is waiting for a full review
+    A file will be prelim approved if:
+    - it's waiting for a prelim review
+    """
+    for file_, review_type in files_with_review_type:
+        version = file_.version
+        addon = version.addon
+        helper = ReviewHelper(request=None, addon=addon,
+                              version=file_.version)
+        # Provide the file to review/sign to the helper.
+        helper.set_data({'addon_files': [file_],
+                         'comments': u'bulk approval'})
+        if review_type == 'full':
+            # Already fully reviewed, or waiting for a full review.
+            helper.handler.process_public()
+            log.info(u'File %s (addon %s) fully reviewed', file_.pk, addon.pk)
+        elif review_type == 'prelim':
+            # Already prelim reviewed, or waiting for a prelim review.
+            helper.handler.process_preliminary()
+            log.info(u'File %s (addon %s) prelim reviewed', file_.pk, addon.pk)
+        else:
+            log.info(u'File %s (addon %s) not reviewed: '
+                     u'addon status: %s, file status: %s',
+                     file_.pk, addon.pk, addon.status, file_.status)
+
+
+def get_review_type(file_):
+    """Return 'full', 'prelim' or None depending on the file/addon status."""
+    if file_.status == amo.STATUS_NOMINATED:
+        return 'full'
+    if file_.status == amo.STATUS_UNREVIEWED:
+        return 'prelim'
+    if file_.version.addon.status == amo.STATUS_LITE_AND_NOMINATED:
+        return 'full'

apps/addons/tests/test_commands.py

@@ -1,11 +1,21 @@
+import pytest
+
 from django.conf import settings
 from django.core.management import call_command
 
+import amo
+import amo.tests
+from addons.management.commands import approve_addons
+from devhub.models import AddonLog
+from editors.models import ReviewerScore
+
 
 # Where to monkeypatch "lib.crypto.tasks.sign_addons" so it's correctly mocked.
 SIGN_ADDONS = 'addons.management.commands.sign_addons.sign_addons'
 
 
+# Test the "sign_addons" command.
+
 def test_no_overridden_settings(monkeypatch):
     assert not settings.SIGNING_SERVER
     assert not settings.PRELIMINARY_SIGNING_SERVER
@@ -52,3 +62,137 @@ def test_force_signing(monkeypatch):
         assert force
     monkeypatch.setattr(SIGN_ADDONS, is_forced)
     call_command('sign_addons', 123, force=True)
+
+
+# Test the "approve_addons" command.
+
+@pytest.mark.django_db
+def test_approve_addons_get_files_incomplete():
+    """An incomplete add-on can't be approved."""
+    addon = amo.tests.addon_factory(status=amo.STATUS_NULL)
+    assert approve_addons.get_files([addon.guid]) == []
+
+
+@pytest.mark.django_db
+def test_approve_addons_get_files_bad_guid():
+    """An add-on with another guid doesn't get approved."""
+    addon1 = amo.tests.addon_factory(status=amo.STATUS_UNREVIEWED, guid='foo')
+    addon1_file = addon1.latest_version.files.get()
+    addon1_file.update(status=amo.STATUS_UNREVIEWED)
+    # Create another add-on that we won't get the files for.
+    addon2 = amo.tests.addon_factory(status=amo.STATUS_UNREVIEWED, guid='bar')
+    addon2_file = addon2.latest_version.files.get()
+    addon2_file.update(status=amo.STATUS_UNREVIEWED)
+    # There's only the addon1's file returned, no other.
+    assert approve_addons.get_files(['foo']) == [addon1_file]
+
+
+@pytest.fixture(
+    params=[(amo.STATUS_UNREVIEWED, amo.STATUS_UNREVIEWED, 'prelim'),
+            (amo.STATUS_LITE, amo.STATUS_UNREVIEWED, 'prelim'),
+            (amo.STATUS_NOMINATED, amo.STATUS_NOMINATED, 'full'),
+            (amo.STATUS_PUBLIC, amo.STATUS_NOMINATED, 'full'),
+            (amo.STATUS_LITE_AND_NOMINATED, amo.STATUS_LITE, 'full')],
+    ids=['1-1-prelim',  # Those are used to build better test names, for the
+         '8-1-prelim',  # tests using this fixture, eg:
+         '3-3-full',    # > test_approve_addons_get_files[1-1-prelim]
+         '4-3-full',    # instead of simply:
+         '9-8-full'])   # > test_approve_addons_get_files[usecase0]
+def use_case(request, db):
+    """This fixture will return quadruples for different use cases.
+
+    Addon                   | File1 and 2        | Review type
+    ==============================================================
+    waiting for prelim      | waiting for prelim | prelim reviewed
+    prelim reviewed         | waiting for prelim | prelim reviewed
+    waiting for full        | waiting for full   | fully reviewed
+    fully reviewed          | waiting for full   | fully reviewed
+    prelim waiting for full | prelim reviewed    | fully reviewed
+    """
+    addon_status, file_status, review_type = request.param
+
+    addon = amo.tests.addon_factory(status=addon_status, guid='foo')
+    version = addon.latest_version
+    file1 = version.files.get()
+    file1.update(status=file_status)
+    # A second file for good measure.
+    file2 = amo.tests.file_factory(version=version, status=file_status)
+    # If the addon is public, and we change its only file to something else
+    # than public, it'll change to unreviewed.
+    addon.update(status=addon_status)
+    assert addon.reload().status == addon_status
+    assert file1.reload().status == file_status
+    assert file2.reload().status == file_status
+
+    return (addon, file1, file2, review_type)
+
+
+@pytest.fixture
+def mozilla_user(db):
+    """Create and return the "mozilla" user used to auto approve addons."""
+    return amo.tests.user_factory(id=settings.TASK_USER_ID)
+
+
+def test_approve_addons_get_files(use_case):
+    """Files that need to get approved are returned in the list.
+
+    Use cases are quadruples taken from the "use_case" fixture above.
+    """
+    addon, file1, file2, review_type = use_case
+    assert approve_addons.get_files([addon.guid]) == [file1, file2]
+
+
+@pytest.mark.django_db
+def test_approve_addons_approve_files_no_review_type():
+    """Files which don't need approval don't change status."""
+    # Create the "mozilla" user, needed for the log.
+    amo.tests.user_factory(id=settings.TASK_USER_ID)
+    addon = amo.tests.addon_factory(status=amo.STATUS_PUBLIC)
+    file_ = addon.versions.get().files.get()
+    file_.update(status=amo.STATUS_PUBLIC)
+    approve_addons.approve_files([(file_, None)])
+    # Nothing changed.
+    assert addon.reload().status == amo.STATUS_PUBLIC
+    assert file_.reload().status == amo.STATUS_PUBLIC
+
+
+def test_approve_addons_approve_files(use_case, mozilla_user):
+    """Files are approved using the correct review type.
+
+    Use cases are quadruples taken from the "use_case" fixture above.
+    """
+    addon, file1, file2, review_type = use_case
+    approve_addons.approve_files([(file1, review_type),
+                                  (file2, review_type)])
+    assert file1.reload().status == (
+        amo.STATUS_LITE if review_type == 'prelim' else amo.STATUS_PUBLIC)
+    assert file2.reload().status == (
+        amo.STATUS_LITE if review_type == 'prelim' else amo.STATUS_PUBLIC)
+    logs = AddonLog.objects.filter(addon=addon)
+    assert len(logs) == 2  # One per file.
+    file1_log, file2_log = logs
+    # An AddonLog has been created for each approval.
+    assert file1_log.activity_log.details['comments'] == u'bulk approval'
+    assert file1_log.activity_log.user == mozilla_user
+    assert file2_log.activity_log.details['comments'] == u'bulk approval'
+    assert file2_log.activity_log.user == mozilla_user
+    # No ReviewerScore was granted, it's an automatic approval.
+    assert not ReviewerScore.objects.all()
+
+
+@pytest.mark.django_db
+def test_approve_addons_get_review_type_already_approved():
+    """The review type for a file that doesn't need approval is None."""
+    addon = amo.tests.addon_factory(status=amo.STATUS_PUBLIC)
+    file_ = addon.versions.get().files.get()
+    file_.update(status=amo.STATUS_PUBLIC)
+    assert approve_addons.get_review_type(file_) is None
+
+
+def test_approve_addons_get_review_type(use_case):
+    """Review type depends on the file and addon status.
+
+    Use cases are quadruples taken from the "use_case" fixture above.
+    """
+    addon, file1, _, review_type = use_case
+    assert approve_addons.get_review_type(file1) == review_type

apps/constants/base.py

@@ -109,6 +109,12 @@ LITE_STATUSES = (STATUS_LITE, STATUS_LITE_AND_NOMINATED)
 MIRROR_STATUSES = (STATUS_PUBLIC, STATUS_BETA,
                    STATUS_LITE, STATUS_LITE_AND_NOMINATED)
 
+# Fully reviewed of waiting for a full review.
+FULL_REVIEW_STATUSES = [STATUS_NOMINATED, STATUS_LITE_AND_NOMINATED,
+                        STATUS_PUBLIC]
+# Prelim reviewed of waiting for a prelim review.
+PRELIM_REVIEW_STATUSES = [STATUS_UNREVIEWED, STATUS_LITE]
+
 # An add-on in one of these statuses can become premium.
 PREMIUM_STATUSES = (STATUS_NULL,) + UNDER_REVIEW_STATUSES
 

apps/editors/helpers.py

@@ -711,7 +711,9 @@ class ReviewAddon(ReviewBase):
         log.info(u'Sending email for %s' % (self.addon))
 
         # Assign reviewer incentive scores.
-        ReviewerScore.award_points(self.request.amo_user, self.addon, status)
+        if self.request:
+            ReviewerScore.award_points(self.request.amo_user, self.addon,
+                                       status)
 
     def process_sandbox(self):
         """Set an addon back to sandbox."""
@@ -741,7 +743,9 @@ class ReviewAddon(ReviewBase):
         log.info(u'Sending email for %s' % (self.addon))
 
         # Assign reviewer incentive scores.
-        ReviewerScore.award_points(self.request.amo_user, self.addon, status)
+        if self.request:
+            ReviewerScore.award_points(self.request.amo_user, self.addon,
+                                       status)
 
     def process_preliminary(self, auto_validation=False):
         """Set an addon to preliminary."""

settings_test.py

@@ -105,7 +105,7 @@ AMO_LANGUAGES = (
 )
 LANGUAGES = lazy(lazy_langs, dict)(AMO_LANGUAGES)
 LANGUAGE_URL_MAP = dict([(i.lower(), i) for i in AMO_LANGUAGES])
-TASK_USER_ID = '4043307'
+TASK_USER_ID = 4043307
 
 PASSWORD_HASHERS = (
     'django.contrib.auth.hashers.MD5PasswordHasher',