Parse existing apps manifest to fill developer name (bug 880188)

apps/addons/management/commands/process_addons.py

@@ -11,9 +11,9 @@ from amo.utils import chunked
 from devhub.tasks import convert_purified, flag_binary, get_preview_sizes
 from market.tasks import check_paypal, check_paypal_multiple
 
-from mkt.webapps.tasks import (add_uuids, dump_apps, update_features,
-                               update_manifests, update_supported_locales,
-                               zip_apps)
+from mkt.webapps.tasks import (add_uuids, dump_apps, update_developer_name,
+                               update_features, update_manifests,
+                               update_supported_locales, zip_apps)
 
 
 tasks = {
@@ -56,7 +56,13 @@ tasks = {
                                   status__in=[amo.STATUS_PENDING,
                                               amo.STATUS_PUBLIC,
                                               amo.STATUS_PUBLIC_WAITING],
-                                  disabled_by_user=False)]}
+                                  disabled_by_user=False)]},
+    'update_developer_name': {'method': update_developer_name,
+                         'qs': [Q(type=amo.ADDON_WEBAPP,
+                                  status__in=[amo.STATUS_PENDING,
+                                              amo.STATUS_PUBLIC,
+                                              amo.STATUS_PUBLIC_WAITING],
+                                  disabled_by_user=False)]},
 }
 
 

mkt/webapps/tasks.py

@@ -8,6 +8,7 @@ from collections import defaultdict
 
 from django.conf import settings
 from django.core.files.storage import default_storage as storage
+from django.forms import ValidationError
 from django.template import Context, loader
 
 from celery.exceptions import RetryTaskError
@@ -22,6 +23,7 @@ from amo.urlresolvers import reverse
 from amo.utils import chunked, JSONEncoder
 from editors.models import RereviewQueue
 from files.models import FileUpload
+from files.utils import WebAppParser
 from lib.es.utils import get_indices
 from users.utils import get_task_user
 
@@ -401,3 +403,39 @@ def _update_features(id):
 def update_features(ids, **kw):
     for id in ids:
         _update_features(id)
+
+
+def _update_developer_name(id):
+    try:
+        webapp = Webapp.objects.get(pk=id)
+    except Webapp.DoesNotExist:
+        _log(id, u'Webapp does not exist')
+        return
+
+    version = webapp.current_version
+
+    # If the app doesn't have a current_version, don't bother.
+    if not version:
+        _log(id, u'Webapp does not have a current_version')
+        return
+
+    # If the current_version already has a non-empty developer_name set, don't
+    # touch it and bail.
+    if version._developer_name:
+        _log(id, u'Webapp already has a non-empty developer_name')
+        return
+
+    try:
+        data = WebAppParser().parse(webapp.get_latest_file().file_path)
+    except ValidationError:
+        _log(id, u'Webapp manifest can not be parsed')
+        return
+
+    max_len = version._meta.get_field_by_name('_developer_name')[0].max_length
+    version.update(_developer_name=data['developer_name'][:max_len])
+
+
+@task
+def update_developer_name(ids, **kw):
+    for id in ids:
+        _update_developer_name(id)

mkt/webapps/tests/test_tasks.py

@@ -7,6 +7,7 @@ import stat
 from django.conf import settings
 from django.core.files.storage import default_storage as storage
 from django.core.management import call_command
+from django.forms import ValidationError
 
 import mock
 from nose.tools import eq_, ok_
@@ -22,8 +23,8 @@ from versions.models import Version
 
 from mkt.site.fixtures import fixture
 from mkt.webapps.models import AppFeatures, Webapp
-from mkt.webapps.tasks import (dump_app, update_features, update_manifests,
-                               zip_apps)
+from mkt.webapps.tasks import (dump_app, update_developer_name, update_features,
+                               update_manifests, zip_apps)
 
 
 original = {
@@ -555,3 +556,68 @@ class TestUpdateFeatures(amo.tests.TestCase):
         eq_(features['has_apps'], True)
         eq_(features['has_activity'], True)
         eq_(features['has_sms'], False)
+
+
+class TestUpdateDeveloperName(amo.tests.TestCase):
+    fixtures = fixture('webapp_337141')
+
+    def setUp(self):
+        self.app = Webapp.objects.get(pk=337141)
+
+    @mock.patch('mkt.webapps.tasks._update_developer_name')
+    def test_ignore_not_webapp(self, mock_):
+        self.app.update(type=amo.ADDON_EXTENSION)
+        call_command('process_addons', task='update_developer_name')
+        assert not mock_.called
+
+    @mock.patch('mkt.webapps.tasks._update_developer_name')
+    def test_pending(self, mock_):
+        self.app.update(status=amo.STATUS_PENDING)
+        call_command('process_addons', task='update_developer_name')
+        assert mock_.called
+
+    @mock.patch('mkt.webapps.tasks._update_developer_name')
+    def test_public_waiting(self, mock_):
+        self.app.update(status=amo.STATUS_PUBLIC_WAITING)
+        call_command('process_addons', task='update_developer_name')
+        assert mock_.called
+
+    @mock.patch('mkt.webapps.tasks._update_developer_name')
+    def test_ignore_disabled(self, mock_):
+        self.app.update(status=amo.STATUS_DISABLED)
+        call_command('process_addons', task='update_developer_name')
+        assert not mock_.called
+
+    @mock.patch('files.utils.WebAppParser.parse')
+    def test_ignore_no_current_version(self, mock_parser):
+        self.app.current_version.all_files[0].update(status=amo.STATUS_DISABLED)
+        self.app.update_version()
+        update_developer_name(ids=(self.app.pk,))
+        assert not mock_parser.called
+
+    @mock.patch('files.utils.WebAppParser.parse')
+    def test_ignore_if_existing_developer_name(self, mock_parser):
+        version = self.app.current_version
+        version.update(_developer_name=u"Mï")
+        update_developer_name(ids=(self.app.pk,))
+        assert not mock_parser.called
+
+    @mock.patch('files.utils.WebAppParser.parse')
+    def test_update_developer_name(self, mock_parser):
+        mock_parser.return_value = {
+            'developer_name': u'New Dêv'
+        }
+        update_developer_name(ids=(self.app.pk,))
+        version = self.app.current_version.reload()
+        eq_(version._developer_name, u'New Dêv')
+        eq_(version.developer_name, u'New Dêv')
+
+    @mock.patch('files.utils.WebAppParser.parse')
+    @mock.patch('mkt.webapps.tasks._log')
+    def test_update_developer_name_validation_error(self, _log, mock_parser):
+        mock_parser.side_effect = ValidationError('dummy validation error')
+        update_developer_name(ids=(self.app.pk,))
+        assert _log.called_with(337141, u'Webapp manifest can not be parsed')
+
+        version = self.app.current_version.reload()
+        eq_(version._developer_name, '')