File.status cleanup (bug 778779)

This commit is contained in:
Rob Hudson 2012-07-31 15:28:50 -07:00
Родитель beac7ff2e8
Коммит 84128308aa
11 изменённых файлов: 140 добавлений и 121 удалений

Просмотреть файл

@ -111,6 +111,10 @@ WEBAPPS_UNREVIEWED_STATUS = STATUS_PENDING
WEBAPPS_UNLISTED_STATUSES = (STATUS_DISABLED, STATUS_PENDING,
STATUS_PUBLIC_WAITING, STATUS_REJECTED)
# The only statuses we use in the marketplace.
MARKET_STATUSES = (STATUS_NULL, STATUS_PENDING, STATUS_PUBLIC, STATUS_DISABLED,
STATUS_DELETED, STATUS_REJECTED, STATUS_PUBLIC_WAITING)
# Types of administrative review queues for an add-on:
ADMIN_REVIEW_FULL = 1
ADMIN_REVIEW_PRELIM = 2

Просмотреть файл

@ -150,20 +150,19 @@ class File(amo.models.OnChangeMixin, amo.models.ModelBase):
@classmethod
def from_upload(cls, upload, version, platform, parse_data={}):
is_webapp = version.addon.is_webapp()
f = cls(version=version, platform=platform)
upload.path = amo.utils.smart_path(nfd_str(upload.path))
ext = os.path.splitext(upload.path)[1]
f.filename = f.generate_filename(extension=ext or '.xpi')
f.filename = f.generate_filename(os.path.splitext(upload.path)[1])
# Size in kilobytes.
f.size = int(max(1, round(storage.size(upload.path) / 1024, 0)))
f.size = int(max(1, round(storage.size(upload.path) / 1024)))
data = cls.get_jetpack_metadata(upload.path)
f.jetpack_version = data['sdkVersion']
f.builder_version = data['builderVersion']
f.no_restart = parse_data.get('no_restart', False)
f.strict_compatibility = parse_data.get('strict_compatibility', False)
if version.addon.is_webapp():
# Files don't really matter for webapps, just make them public.
f.status = amo.STATUS_PUBLIC
if is_webapp:
f.status = amo.STATUS_PENDING
elif version.addon.status == amo.STATUS_PUBLIC:
if amo.VERSION_BETA.search(parse_data.get('version', '')):
f.status = amo.STATUS_BETA
@ -220,15 +219,22 @@ class File(amo.models.OnChangeMixin, amo.models.ModelBase):
hash.update(chunk)
return 'sha256:%s' % hash.hexdigest()
def generate_filename(self, extension='.xpi'):
def generate_filename(self, extension=None):
"""
Files are in the format of:
{addon_name}-{version}-{apps}-{platform}
"""
parts = []
addon = self.version.addon
# slugify drops unicode so we may end up with an empty string.
# Apache did not like serving unicode filenames (bug 626587).
name = slugify(self.version.addon.name).replace('-', '_') or 'addon'
if addon.is_webapp():
extension = extension or '.webapp'
parts.append(addon.app_slug)
parts.append(self.version.version)
else:
extension = extension or '.xpi'
name = slugify(addon.name).replace('-', '_') or 'addon'
parts.append(name)
parts.append(self.version.version)

Просмотреть файл

@ -225,6 +225,12 @@ class TestFile(amo.tests.TestCase, amo.tests.AMOPaths):
f = File.objects.get(id=67442)
eq_(f.generate_filename(), 'delicious_bookmarks-2.1.072-fx.xpi')
def test_generate_filename_webapp(self):
f = File.objects.get(id=67442)
f.version.addon.app_slug = 'testing-123'
f.version.addon.type = amo.ADDON_WEBAPP
eq_(f.generate_filename(), 'testing-123-2.1.072.webapp')
def test_pretty_filename(self):
f = File.objects.get(id=67442)
f.generate_filename()
@ -260,7 +266,6 @@ class TestFile(amo.tests.TestCase, amo.tests.AMOPaths):
with storage.open(f.file_path, 'w') as fp:
fp.write('sample data\n')
def test_copy_to_mirror(self):
f = File.objects.get(id=67442)
self.clean_files(f)
@ -574,7 +579,8 @@ class TestFileUpload(UploadTest):
"id": "gkobes@gkobes",
}
})
upload = self.get_upload(filename='extension.xpi', validation=validation)
upload = self.get_upload(filename='extension.xpi',
validation=validation)
version = Version.objects.filter(addon__pk=3615)[0]
plat = Platform.objects.get(pk=amo.PLATFORM_LINUX.id)
file_ = File.from_upload(upload, version, plat)
@ -595,7 +601,8 @@ class TestFileUpload(UploadTest):
"id": "gkobes@gkobes",
}
})
upload = self.get_upload(filename='extension.xpi', validation=validation)
upload = self.get_upload(filename='extension.xpi',
validation=validation)
version = Version.objects.filter(addon__pk=3615)[0]
plat = Platform.objects.get(pk=amo.PLATFORM_LINUX.id)
file_ = File.from_upload(upload, version, plat)
@ -616,7 +623,8 @@ class TestFileUpload(UploadTest):
"requires_chrome": True
}
})
upload = self.get_upload(filename='extension.xpi', validation=validation)
upload = self.get_upload(filename='extension.xpi',
validation=validation)
version = Version.objects.filter(addon__pk=3615)[0]
plat = Platform.objects.get(pk=amo.PLATFORM_LINUX.id)
file_ = File.from_upload(upload, version, plat)

Просмотреть файл

@ -1,5 +1,4 @@
import collections
import cPickle as pickle
import glob
import hashlib
import json
@ -57,6 +56,7 @@ def get_filepath(fileorpath):
return fileorpath.path
return fileorpath
def get_file(fileorpath):
"""Get a file-like object, whether given a FileUpload object or a path."""
if hasattr(fileorpath, 'path'): # FileUpload
@ -438,8 +438,7 @@ def parse_addon(pkg, addon=None):
or files.models.FileUpload.
"""
name = getattr(pkg, 'name', pkg)
if (getattr(pkg, 'is_webapp', False) or
name.endswith('.webapp') or name.endswith('.json')):
if getattr(pkg, 'is_webapp', False) or name.endswith(('.webapp', '.json')):
parsed = WebAppParser().parse(pkg, addon)
elif name.endswith('.xml'):
parsed = parse_search(pkg, addon)
@ -447,8 +446,7 @@ def parse_addon(pkg, addon=None):
parsed = parse_xpi(pkg, addon)
if addon and addon.type != parsed['type']:
raise forms.ValidationError(
_("<em:type> doesn't match add-on"))
raise forms.ValidationError(_("<em:type> doesn't match add-on"))
return parsed

Просмотреть файл

@ -83,7 +83,7 @@ class Version(amo.models.ModelBase):
AV(version=v, min=app.min, max=app.max,
application_id=app.id).save()
if addon.type in [amo.ADDON_SEARCH, amo.ADDON_WEBAPP]:
# Search extensions are always for all platforms.
# Search extensions and webapps are always for all platforms.
platforms = [Platform.objects.get(id=amo.PLATFORM_ALL.id)]
else:
platforms = cls._make_safe_platform_files(platforms)
@ -92,8 +92,7 @@ class Version(amo.models.ModelBase):
File.from_upload(upload, v, platform, parse_data=data)
v.disable_old_files()
# After the upload has been copied to all
# platforms, remove the upload.
# After the upload has been copied to all platforms, remove the upload.
storage.delete(upload.path)
if send_signal:
version_uploaded.send(sender=v)
@ -385,8 +384,9 @@ class Version(amo.models.ModelBase):
def disable_old_files(self):
if not self.files.filter(status=amo.STATUS_BETA).exists():
qs = File.objects.filter(version__addon=self.addon_id,
version__lt=self,
status=amo.STATUS_UNREVIEWED)
version__lt=self.id,
status__in=[amo.STATUS_UNREVIEWED,
amo.STATUS_PENDING])
# Use File.update so signals are triggered.
for f in qs:
f.update(status=amo.STATUS_DISABLED)

Просмотреть файл

@ -25,7 +25,7 @@ from files.models import FileUpload
import mkt
from mkt.developers import tasks
from mkt.developers.tests.test_views import BaseWebAppTest
from mkt.submit.tests.test_views import BaseWebAppTest
from mkt.webapps.models import AddonExcludedRegion as AER

Просмотреть файл

@ -29,7 +29,6 @@ from browse.tests import test_default_sort, test_listing_sort
from devhub.models import UserLog
from files.models import FileUpload
from files.tests.test_models import UploadTest as BaseUploadTest
from lib.pay_server import client
from market.models import AddonPremium, Price, Refund
from mkt.developers import tasks
from mkt.developers.models import ActivityLog
@ -1457,69 +1456,6 @@ def assert_json_field(request, field, msg):
eq_(content[field], msg)
class BaseWebAppTest(BaseUploadTest, amo.tests.TestCase):
fixtures = ['base/apps', 'base/users', 'base/platforms']
def setUp(self):
super(BaseWebAppTest, self).setUp()
self.manifest = os.path.join(settings.ROOT, 'apps', 'devhub', 'tests',
'addons', 'mozball.webapp')
self.upload = self.get_upload(abspath=self.manifest)
self.url = reverse('submit.app.manifest')
assert self.client.login(username='regular@mozilla.com',
password='password')
self.client.post(reverse('submit.app.terms'),
{'read_dev_agreement': True})
def post(self, desktop_platforms=[amo.PLATFORM_ALL], mobile_platforms=[],
expect_errors=False):
d = dict(upload=self.upload.pk,
desktop_platforms=[p.id for p in desktop_platforms],
mobile_platforms=[p.id for p in mobile_platforms])
r = self.client.post(self.url, d, follow=True)
eq_(r.status_code, 200)
if not expect_errors:
# Show any unexpected form errors.
if r.context and 'new_addon_form' in r.context:
eq_(r.context['new_addon_form'].errors.as_text(), '')
return r
def post_addon(self):
eq_(Addon.objects.count(), 0)
self.post()
return Addon.objects.get()
class TestCreateWebApp(BaseWebAppTest):
def test_post_app_redirect(self):
r = self.post()
addon = Addon.objects.get()
self.assertRedirects(r, reverse('submit.app.details',
args=[addon.app_slug]))
def test_addon_from_uploaded_manifest(self):
addon = self.post_addon()
eq_(addon.type, amo.ADDON_WEBAPP)
eq_(addon.guid, None)
eq_(unicode(addon.name), 'MozillaBall')
eq_(addon.slug, 'app-%s' % addon.id)
eq_(addon.app_slug, 'mozillaball')
eq_(addon.summary, u'Exciting Open Web development action!')
eq_(Translation.objects.get(id=addon.summary.id, locale='it'),
u'Azione aperta emozionante di sviluppo di fotoricettore!')
def test_version_from_uploaded_manifest(self):
addon = self.post_addon()
eq_(addon.current_version.version, '1.0')
def test_file_from_uploaded_manifest(self):
addon = self.post_addon()
files = addon.current_version.files
eq_(files.count(), 1)
eq_(files.all()[0].status, amo.STATUS_PUBLIC)
class TestDeleteApp(amo.tests.TestCase):
fixtures = ['base/apps', 'base/users', 'webapps/337141-steamcube']

Просмотреть файл

@ -17,11 +17,13 @@ import reviews
from abuse.models import AbuseReport
from access.models import GroupUser
from addons.models import AddonDeviceType, AddonUser
from amo.tests import app_factory, check_links, formset, initial
from amo.tests import (app_factory, check_links, formset, initial,
version_factory)
from amo.urlresolvers import reverse
from amo.utils import urlparams
from devhub.models import ActivityLog, AppLog
from editors.models import CannedResponse, ReviewerScore
from files.models import File
from mkt.reviewers.models import EscalationQueue, RereviewQueue
from mkt.webapps.models import Webapp
from reviews.models import Review, ReviewFlag
@ -196,6 +198,7 @@ class TestAppQueue(AppReviewerTest, AccessMixin):
def test_action_buttons_rejected(self):
# Check action buttons for a previously rejected app.
self.apps[0].update(status=amo.STATUS_REJECTED)
self.apps[0].latest_version.files.update(status=amo.STATUS_REJECTED)
r = self.client.get(self.review_url(self.apps[0]))
eq_(r.status_code, 200)
actions = pq(r.content)('#review-actions input')
@ -359,6 +362,7 @@ class TestRereviewQueue(AppReviewerTest, AccessMixin):
def test_action_buttons_reject(self):
self.apps[0].update(status=amo.STATUS_REJECTED)
self.apps[0].latest_version.files.update(status=amo.STATUS_REJECTED)
r = self.client.get(self.review_url(self.apps[0]))
eq_(r.status_code, 200)
actions = pq(r.content)('#review-actions input')
@ -473,6 +477,7 @@ class TestEscalationQueue(AppReviewerTest, AccessMixin):
def test_action_buttons_reject(self):
self.apps[0].update(status=amo.STATUS_REJECTED)
self.apps[0].latest_version.files.update(status=amo.STATUS_REJECTED)
r = self.client.get(self.review_url(self.apps[0]))
eq_(r.status_code, 200)
actions = pq(r.content)('#review-actions input')
@ -532,6 +537,7 @@ class TestReviewApp(AppReviewerTest, AccessMixin):
self.app.update(status=amo.STATUS_PENDING,
mozilla_contact=self.mozilla_contact)
self.version = self.app.current_version
self.version.files.all().update(status=amo.STATUS_PENDING)
self.url = reverse('reviewers.apps.review', args=[self.app.app_slug])
def get_app(self):
@ -539,7 +545,6 @@ class TestReviewApp(AppReviewerTest, AccessMixin):
def post(self, data, queue='pending'):
res = self.client.post(self.url, data)
# Purposefully not using assertRedirects.
self.assert3xx(res, reverse('reviewers.apps.queue_%s' % queue))
def test_review_viewing_ping(self):
@ -602,7 +607,9 @@ class TestReviewApp(AppReviewerTest, AccessMixin):
'comments': 'something',
'addon_files': files,
})
eq_(self.get_app().status, amo.STATUS_PUBLIC)
app = self.get_app()
eq_(app.status, amo.STATUS_PUBLIC)
eq_(app.current_version.files.all()[0].status, amo.STATUS_PUBLIC)
self._check_log(amo.LOG.APPROVE_VERSION)
eq_(len(mail.outbox), 1)
@ -623,7 +630,9 @@ class TestReviewApp(AppReviewerTest, AccessMixin):
'comments': 'something',
'addon_files': files,
})
eq_(self.get_app().status, amo.STATUS_PUBLIC)
app = self.get_app()
eq_(app.status, amo.STATUS_PUBLIC)
eq_(app.current_version.files.all()[0].status, amo.STATUS_PUBLIC)
self._check_log(amo.LOG.APPROVE_VERSION)
eq_(len(mail.outbox), 1)
@ -644,7 +653,10 @@ class TestReviewApp(AppReviewerTest, AccessMixin):
'comments': 'something',
'addon_files': files,
})
eq_(self.get_app().status, amo.STATUS_PUBLIC_WAITING)
app = self.get_app()
eq_(app.status, amo.STATUS_PUBLIC_WAITING)
eq_(app.current_version.files.all()[0].status,
amo.STATUS_PUBLIC_WAITING)
self._check_log(amo.LOG.APPROVE_VERSION_WAITING)
eq_(len(mail.outbox), 1)
@ -654,8 +666,34 @@ class TestReviewApp(AppReviewerTest, AccessMixin):
self._check_score(amo.REVIEWED_WEBAPP)
def test_pending_to_reject(self):
files = list(self.version.files.values_list('id', flat=True))
self.post({'action': 'reject', 'comments': 'suxor'})
eq_(self.get_app().status, amo.STATUS_REJECTED)
app = self.get_app()
eq_(app.status, amo.STATUS_REJECTED)
eq_(File.objects.filter(id__in=files)[0].status, amo.STATUS_DISABLED)
self._check_log(amo.LOG.REJECT_VERSION)
eq_(len(mail.outbox), 1)
msg = mail.outbox[0]
self._check_email(msg, 'Submission Update')
self._check_email_body(msg)
def test_multiple_versions_reject(self):
self.app.update(status=amo.STATUS_PUBLIC)
self.app.current_version.files.update(status=amo.STATUS_PUBLIC)
new_version = version_factory(addon=self.app)
new_version.files.all().update(status=amo.STATUS_PENDING)
files = list(new_version.files.values_list('id', flat=True))
self.post({
'action': 'reject',
'operating_systems': '',
'applications': '',
'comments': 'something',
'addon_files': files,
})
app = self.get_app()
eq_(app.status, amo.STATUS_PUBLIC)
eq_(new_version.files.all()[0].status, amo.STATUS_DISABLED)
self._check_log(amo.LOG.REJECT_VERSION)
eq_(len(mail.outbox), 1)
@ -679,7 +717,9 @@ class TestReviewApp(AppReviewerTest, AccessMixin):
self.app.update(status=amo.STATUS_PUBLIC)
self.post({'action': 'disable', 'comments': 'disabled ur app'})
eq_(self.get_app().status, amo.STATUS_DISABLED)
app = self.get_app()
eq_(app.status, amo.STATUS_DISABLED)
eq_(app.current_version.files.all()[0].status, amo.STATUS_DISABLED)
self._check_log(amo.LOG.APP_DISABLED)
eq_(len(mail.outbox), 1)
self._check_email(mail.outbox[0], 'App disabled by reviewer')
@ -704,7 +744,9 @@ class TestReviewApp(AppReviewerTest, AccessMixin):
'comments': 'something',
'addon_files': files,
}, queue='escalated')
eq_(self.get_app().status, amo.STATUS_PUBLIC)
app = self.get_app()
eq_(app.status, amo.STATUS_PUBLIC)
eq_(app.current_version.files.all()[0].status, amo.STATUS_PUBLIC)
self._check_log(amo.LOG.APPROVE_VERSION)
eq_(EscalationQueue.objects.count(), 0)
@ -724,7 +766,9 @@ class TestReviewApp(AppReviewerTest, AccessMixin):
'comments': 'something',
'addon_files': files,
}, queue='escalated')
eq_(self.get_app().status, amo.STATUS_REJECTED)
app = self.get_app()
eq_(app.status, amo.STATUS_REJECTED)
eq_(File.objects.filter(id__in=files)[0].status, amo.STATUS_DISABLED)
self._check_log(amo.LOG.REJECT_VERSION)
eq_(EscalationQueue.objects.count(), 0)
@ -740,7 +784,9 @@ class TestReviewApp(AppReviewerTest, AccessMixin):
self.app.update(status=amo.STATUS_PUBLIC)
self.post({'action': 'disable', 'comments': 'disabled ur app'},
queue='escalated')
eq_(self.get_app().status, amo.STATUS_DISABLED)
app = self.get_app()
eq_(app.status, amo.STATUS_DISABLED)
eq_(app.current_version.files.all()[0].status, amo.STATUS_DISABLED)
self._check_log(amo.LOG.APP_DISABLED)
eq_(EscalationQueue.objects.count(), 0)
eq_(len(mail.outbox), 1)

Просмотреть файл

@ -12,6 +12,7 @@ from amo.helpers import absolutify
from amo.urlresolvers import reverse
from amo.utils import send_mail_jinja
from editors.models import ReviewerScore
from files.models import File
from .models import EscalationQueue, RereviewQueue
@ -145,8 +146,9 @@ class ReviewApp(ReviewBase):
def process_public_waiting(self):
"""Make an app pending."""
self.set_files(amo.STATUS_PUBLIC_WAITING, self.version.files.all())
self.set_addon(highest_status=amo.STATUS_PUBLIC_WAITING,
status=amo.STATUS_PUBLIC_WAITING)
if self.addon.status != amo.STATUS_PUBLIC:
self.set_addon(status=amo.STATUS_PUBLIC_WAITING,
highest_status=amo.STATUS_PUBLIC_WAITING)
self.log_action(amo.LOG.APPROVE_VERSION_WAITING)
self.notify_email('pending_to_public_waiting',
@ -160,8 +162,9 @@ class ReviewApp(ReviewBase):
# Save files first, because set_addon checks to make sure there
# is at least one public file or it won't make the addon public.
self.set_files(amo.STATUS_PUBLIC, self.version.files.all())
self.set_addon(highest_status=amo.STATUS_PUBLIC,
status=amo.STATUS_PUBLIC)
if self.addon.status != amo.STATUS_PUBLIC:
self.set_addon(status=amo.STATUS_PUBLIC,
highest_status=amo.STATUS_PUBLIC)
self.log_action(amo.LOG.APPROVE_VERSION)
self.notify_email('pending_to_public', u'App Approved: %s')
@ -171,13 +174,17 @@ class ReviewApp(ReviewBase):
def process_sandbox(self):
"""Reject an app."""
self.set_files(amo.STATUS_DISABLED, self.version.files.all(),
hide_disabled_file=True)
# If there aren't other versions with already reviewed files, it's ok
# to reject this addon. Otherwise only reject the files.
if (not self.addon.versions.exclude(id=self.version.id)
.filter(files__status__in=amo.REVIEWED_STATUSES).exists()):
self.set_addon(status=amo.STATUS_REJECTED)
if self.in_escalate:
EscalationQueue.objects.filter(addon=self.addon).delete()
if self.in_rereview:
RereviewQueue.objects.filter(addon=self.addon).delete()
self.set_files(amo.STATUS_DISABLED, self.version.files.all(),
hide_disabled_file=True)
self.log_action(amo.LOG.REJECT_VERSION)
self.notify_email('pending_to_sandbox', u'Submission Update: %s')
@ -213,6 +220,10 @@ class ReviewApp(ReviewBase):
if not acl.action_allowed(self.request, 'Addons', 'Edit'):
return
# Disable disables all files, not just those in this version.
self.set_files(amo.STATUS_DISABLED,
File.objects.filter(version__addon=self.addon),
hide_disabled_file=True)
self.addon.update(status=amo.STATUS_DISABLED)
if self.in_escalate:
EscalationQueue.objects.filter(addon=self.addon).delete()
@ -238,7 +249,8 @@ class ReviewHelper(object):
self.handler = None
self.required = {}
self.addon = addon
self.all_files = version.files.all()
self.version = version
self.all_files = version and version.files.all()
self.get_review_type(request, addon, version)
self.actions = self.get_actions()
@ -290,13 +302,15 @@ class ReviewHelper(object):
'label': _lazy(u'Clear Escalation'),
'minimal': True,
'details': _lazy(u'Clear this app from the escalation queue. The '
u'author will get no email or see comments here.')}
u'author will get no email or see comments '
u'here.')}
clear_rereview = {
'method': self.handler.process_clear_rereview,
'label': _lazy(u'Clear Re-review'),
'minimal': True,
'details': _lazy(u'Clear this app from the re-review queue. The '
u'author will get no email or see comments here.')}
u'author will get no email or see comments '
u'here.')}
disable = {
'method': self.handler.process_disable,
'label': _lazy(u'Disable app'),
@ -306,17 +320,24 @@ class ReviewHelper(object):
actions = SortedDict()
file_status = self.version.files.values_list('status', flat=True)
# Public.
if self.addon.status != amo.STATUS_PUBLIC:
if (self.addon.status != amo.STATUS_PUBLIC or
amo.STATUS_PUBLIC not in file_status):
actions['public'] = public
# Reject.
if self.addon.status not in [amo.STATUS_REJECTED, amo.STATUS_DISABLED]:
if (self.addon.status != amo.STATUS_REJECTED and
self.addon.status != amo.STATUS_DISABLED or (
amo.STATUS_REJECTED not in file_status and
amo.STATUS_DISABLED not in file_status)):
actions['reject'] = reject
# Disable.
if (acl.action_allowed(self.handler.request, 'Addons', 'Edit') and
self.addon.status != amo.STATUS_DISABLED):
if (acl.action_allowed(self.handler.request, 'Addons', 'Edit') and (
self.addon.status != amo.STATUS_DISABLED or
amo.STATUS_DISABLED not in file_status)):
actions['disable'] = disable
# Clear escalation.

Просмотреть файл

@ -278,7 +278,7 @@ class TestCreateWebApp(BaseWebAppTest):
addon = self.post_addon()
files = addon.current_version.files.all()
eq_(len(files), 1)
eq_(files[0].status, amo.STATUS_PUBLIC)
eq_(files[0].status, amo.STATUS_PENDING)
class TestCreateWebAppFromManifest(BaseWebAppTest):

Просмотреть файл

@ -19,7 +19,7 @@ from files.models import File
from versions.models import Version
import mkt
from mkt.developers.tests.test_views import BaseWebAppTest
from mkt.submit.tests.test_views import BaseWebAppTest
from mkt.webapps.models import AddonExcludedRegion, Webapp