Skip to content
This repository has been archived by the owner on Mar 15, 2018. It is now read-only.

Commit

Permalink
Added ability to upload new versions of packaged apps (bug 783669)
Browse files Browse the repository at this point in the history
  • Loading branch information
robhudson committed Aug 29, 2012
1 parent f2ebe46 commit 451c870
Show file tree
Hide file tree
Showing 9 changed files with 198 additions and 22 deletions.
4 changes: 3 additions & 1 deletion apps/amo/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
from files.helpers import copyfileobj
from files.models import File, Platform
from market.models import AddonPremium, Price, PriceCurrency
from translations.models import Translation
from versions.models import ApplicationsVersions, Version

import mkt
Expand Down Expand Up @@ -372,6 +371,9 @@ def grant_permission(self, user_obj, rules):
group = Group.objects.create(name='Test Group', rules=rules)
GroupUser.objects.create(group=group, user=user_obj)

def days_ago(self, days):
return datetime.now() - timedelta(days=days)


class AMOPaths(object):
"""Mixin for getting common AMO Paths."""
Expand Down
2 changes: 1 addition & 1 deletion mkt/constants/submit.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

APP_STEPS = [
('terms', _('Developer Agreement')),
('manifest', _('Upload')),
('manifest', _('Submit')),

This comment has been minimized.

Copy link
@cvan

cvan Aug 29, 2012

Contributor

:(

('details', _('Details')),
('payments', _('Payments')),
('done', _('Finished!')),
Expand Down
9 changes: 2 additions & 7 deletions mkt/developers/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,6 @@ def file_validator(file_id, **kw):
def run_validator(file_path):
"""A pre-configured wrapper around the app validator."""

# TODO(Kumar) remove this when validator is fixed, see bug 620503
from validator.testcases import scripting
scripting.SPIDERMONKEY_INSTALLATION = settings.SPIDERMONKEY
import validator.constants
validator.constants.SPIDERMONKEY_INSTALLATION = settings.SPIDERMONKEY

with statsd.timer('mkt.developers.validator'):
is_packaged = zipfile.is_zipfile(file_path)
if is_packaged:
Expand All @@ -93,7 +87,8 @@ def run_validator(file_path):
with statsd.timer('mkt.developers.validate_packaged_app'):
return validate_packaged_app(file_path,
market_urls=settings.VALIDATOR_IAF_URLS,
timeout=settings.VALIDATOR_TIMEOUT)
timeout=settings.VALIDATOR_TIMEOUT,
spidermonkey=settings.SPIDERMONKEY)
else:
log.info(u'Running `validate_app` for path: %s' % (file_path))
with statsd.timer('mkt.developers.validate_app'):
Expand Down
29 changes: 26 additions & 3 deletions mkt/developers/templates/developers/apps/status.html
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ <h3>
</p>
{% endif %}
{{ form_field(form.release_notes, opt=True) }}
<p><button type="submit">{{ _('Resubmit App') }}</button></p>
<p><button type="submit" name="resubmit-app">{{ _('Resubmit App') }}</button></p>
</form>
{% elif addon.status == amo.STATUS_PUBLIC_WAITING %}
<form method="post" action="{{ addon.get_dev_url('publicise') }}">
Expand Down Expand Up @@ -103,7 +103,7 @@ <h2>{{ _('Packaged Versions') }}</h2>
<ul id="version-list">
{% for version in versions %}
<li>
<h4>Version {{ version.version }}</h4>
<h4><a href="{{ addon.get_dev_url('versions.edit', [version.pk]) }}">Version {{ version.version }}</a></h4>

This comment has been minimized.

Copy link
@cvan

cvan Aug 29, 2012

Contributor

we don't care about localizing this?

This comment has been minimized.

Copy link
@robhudson

robhudson Aug 29, 2012

Author Member

It wasn't before and I missed it. I'll follow up and fix it.

<small>
{% if addon.disabled_by_user %}
<span class="{{ mkt_file_status_class(addon, version) }}"><b>{{ _('Disabled') }}</b></span>
Expand All @@ -113,14 +113,37 @@ <h4>Version {{ version.version }}</h4>
</small>
<small>Submitted <span title="{{ version.created|isotime }}">{{ version.created|datetime }}</span></small>
<div class="buttons">
<a href="{{ version.all_files[0].get_url_path('devhub') }}" class="button download">Download</a>
<a href="{{ version.all_files[0].get_url_path('devhub') }}" class="button download">{{ _('Download') }}</a>
{# TODO: Install and delete buttons #}
</div>
</li>
{% endfor %}
</ul>
</div>
{% endif %}

<h2>{{ _('Upload New Version') }}</h2>
<div class="island">

This comment has been minimized.

Copy link
@cvan

cvan Aug 29, 2012

Contributor

maybe you can include this so you're DRY?

<form method="post" id="create-addon" class="item">
{{ csrf() }}
<p>
{% trans %}
Use the fields below to upload your packaged app. After upload, a
series of automated validation tests will be run on your file.
{% endtrans %}
</p>
<div id="submit-upload upload-file">
<div class="hidden">{{ upload_form.upload }}</div>
<input type="file" id="upload-app" data-upload-url="{{ url('mkt.developers.upload') }}">
{{ upload_form.errors }}
<div class="submission-buttons addon-submission-field">
<button class="prominent addon-upload-dependant" disabled id="submit-upload-file-finish" type="submit" name="upload-version">
{{ _('Continue') }}
</button>
</div>
</div>
</form>
</div>
{% endif %}

</section>
Expand Down
57 changes: 57 additions & 0 deletions mkt/developers/templates/developers/apps/version_edit.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
{% extends 'developers/base_impala.html' %}
{% from 'developers/includes/macros.html' import some_html_tip, tip %}

{% set title = _('Manage Version') + ' %s' % version.version %}

This comment has been minimized.

Copy link
@cvan

cvan Aug 29, 2012

Contributor

gross

This comment has been minimized.

Copy link
@robhudson

robhudson Aug 29, 2012

Author Member

LOL 🙈

{% block title %}{{ hub_page_title(title, addon) }}{% endblock %}

{% macro status(msg) %}
<strong class="status {{ mkt_status_class(addon) }}">{{ msg }}</strong>
{% endmacro %}

{% block content %}
<header>
{{ hub_breadcrumbs(addon, items=[(None, title)]) }}
<h1>{{ title }}</h1>
</header>
<section id="edit-version" class="primary devhub-form manage">
<div class="island">
<form method="post">
{{ csrf() }}
<table>
<tr>
<th>
<label data-for="{{ form.releasenotes.auto_id }}">{{ _('Version Notes') }}
{{ tip(None, _('Information about changes in this release, new features,
known bugs, and other useful information specific to this
release/version. This information is also shown in the
Apps Manager when updating.')) }}

This comment has been minimized.

Copy link
@cvan

cvan Aug 29, 2012

Contributor

eh, not sure that's accurate at all

This comment has been minimized.

Copy link
@robhudson

robhudson Aug 29, 2012

Author Member

I know. But it's easy to change when we do know.

</label>
</th>
<td>
{{ form.releasenotes.errors }}
{{ form.releasenotes }}
{{ some_html_tip() }}
</td>
</tr>
<tr>
<th>
<label for="{{ form.approvalnotes.auto_id }}">{{ _('Notes for Reviewers') }}</label>
{{ tip(None, _('Optionally, enter any information that may be useful
to the Reviewer reviewing this app, such as test
account information.')) }}
</th>
<td>
{{ form.approvalnotes.errors }}
{{ form.approvalnotes }}
</td>
</tr>
</table>
<div class="listing-footer">
<button type="submit">{{ _('Save Changes') }}</button> {{ _('or') }}
<a href="{{ addon.get_dev_url('versions') }}">{{ _('Cancel') }}</a>
</div>
</form>
</div>
</section>
{% include 'developers/includes/addons_edit_nav.html' %}
{% endblock %}
64 changes: 61 additions & 3 deletions mkt/developers/tests/test_views_versions.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@

import amo
import amo.tests
from addons.models import Addon
from addons.models import Addon, AddonUser
from users.models import UserProfile

from mkt.submit.tests.test_views import BasePackagedAppTest


class TestAppStatus(amo.tests.TestCase):
fixtures = ['base/apps', 'base/users', 'webapps/337141-steamcube']
Expand Down Expand Up @@ -79,7 +81,8 @@ def test_rejected(self):
eq_(doc('#rejection blockquote').text(), comments)

my_reply = 'fixed just for u, brah'
r = self.client.post(self.url, {'release_notes': my_reply})
r = self.client.post(self.url, {'release_notes': my_reply,
'resubmit-app': ''})
self.assertRedirects(r, self.url, 302)

webapp = self.get_webapp()
Expand Down Expand Up @@ -108,10 +111,65 @@ def test_version_list_packaged(self):
eq_(doc('#version-list span.status-pending').length, 1)
eq_(doc('#version-list span.status-public').length, 1)
# Check version strings and order of versions.
eq_(map(lambda x: x.text, doc('#version-list h4')),
eq_(map(lambda x: x.text, doc('#version-list h4 a')),
['Version 2.0', 'Version 1.0'])
# Check download url.
eq_(doc('#version-list a.button.download').eq(0).attr('href'),
self.webapp.versions.all()[0].all_files[0].get_url_path('devhub'))
eq_(doc('#version-list a.button.download').eq(1).attr('href'),
self.webapp.versions.all()[1].all_files[0].get_url_path('devhub'))


class TestAddVersion(BasePackagedAppTest):

def setUp(self):
super(TestAddVersion, self).setUp()
self.app = amo.tests.app_factory(is_packaged=True,
version_kw=dict(version='1.0'))
self.url = self.app.get_dev_url('versions')
self.user = UserProfile.objects.get(username='regularuser')
AddonUser.objects.create(user=self.user, addon=self.app)

def _post(self, expected_status=200):
res = self.client.post(self.url, {'upload': self.upload.pk,
'upload-version': ''})
eq_(res.status_code, expected_status)
return res

def test_post(self):
self.app.current_version.update(version='0.9',
created=self.days_ago(1))
self._post(302)
version = self.app.versions.latest()
eq_(version.version, '1.0')
eq_(version.all_files[0].status, amo.STATUS_PENDING)

def test_unique_version(self):
res = self._post(200)
self.assertFormError(res, 'upload_form', 'upload',
'Version 1.0 already exists')


class TestEditVersion(amo.tests.TestCase):
fixtures = ['base/users']

def setUp(self):
self.app = amo.tests.app_factory(is_packaged=True,
version_kw=dict(version='1.0'))
version = self.app.current_version
self.url = self.app.get_dev_url('versions.edit', [version.pk])
self.user = UserProfile.objects.get(username='regularuser')
AddonUser.objects.create(user=self.user, addon=self.app)
self.client.login(username='regular@mozilla.com',
password='password')
eq_(self.client.get(self.url).status_code, 200)

def test_post(self):
rn = u'Release Notes'
an = u'Approval Notes'
res = self.client.post(self.url, {'releasenotes': rn,
'approvalnotes': an})
eq_(res.status_code, 302)
ver = self.app.versions.latest()
eq_(ver.releasenotes, rn)
eq_(ver.approvalnotes, an)
4 changes: 4 additions & 0 deletions mkt/developers/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ def paypal_patterns(prefix):
url('^publicise$', views.publicise, name='mkt.developers.apps.publicise'),
url('^status$', views.status, name='mkt.developers.apps.versions'),

# TODO: '^versions/$'
url('^versions/(?P<version_id>\d+)$', views.version_edit,
name='mkt.developers.apps.versions.edit'),

url('^payments$', views.payments, name='mkt.developers.apps.payments'),
# PayPal-specific stuff.
url('^paypal/', include(paypal_patterns('apps'))),
Expand Down
38 changes: 33 additions & 5 deletions mkt/developers/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
from amo.helpers import absolutify, urlparams
from amo.urlresolvers import reverse
from amo.utils import escape_all
from devhub.forms import VersionForm
from devhub.models import AppLog
from files.models import File, FileUpload
from files.utils import parse_addon
Expand All @@ -46,6 +47,7 @@
from translations.models import delete_translation
from users.models import UserProfile
from users.views import _login
from versions.models import Version

from mkt.developers.decorators import dev_required
from mkt.developers.forms import (AppFormBasic, AppFormDetails, AppFormMedia,
Expand All @@ -54,6 +56,7 @@
PreviewFormSet, RegionForm, trap_duplicate)
from mkt.developers.utils import check_upload
from mkt.inapp_pay.models import InappConfig
from mkt.submit.forms import NewWebappForm
from mkt.webapps.tasks import update_manifests, _update_manifest
from mkt.webapps.models import Webapp

Expand Down Expand Up @@ -185,13 +188,24 @@ def publicise(request, addon_id, addon):
@dev_required(webapp=True)
def status(request, addon_id, addon, webapp=False):
form = forms.AppAppealForm(request.POST, product=addon)
upload_form = NewWebappForm(request.POST or None, is_packaged=True,
addon=addon)

if request.method == 'POST' and form.is_valid():
form.save()
messages.success(request, _('App successfully resubmitted.'))
return redirect(addon.get_dev_url('versions'))
if request.method == 'POST':
if 'resubmit-app' in request.POST and form.is_valid():
form.save()
messages.success(request, _('App successfully resubmitted.'))
return redirect(addon.get_dev_url('versions'))

ctx = {'addon': addon, 'webapp': webapp, 'form': form}
elif 'upload-version' in request.POST and upload_form.is_valid():
ver = Version.from_upload(upload_form.cleaned_data['upload'],
addon, [amo.PLATFORM_ALL])
log.info('[Webapp:%s] New version created id=%s from upload: %s'
% (addon, ver.pk, upload_form.cleaned_data['upload']))
return redirect(addon.get_dev_url('versions.edit', args=[ver.pk]))

ctx = {'addon': addon, 'webapp': webapp, 'form': form,
'upload_form': upload_form}

if addon.status == amo.STATUS_REJECTED:
try:
Expand All @@ -207,6 +221,20 @@ def status(request, addon_id, addon, webapp=False):
return jingo.render(request, 'developers/apps/status.html', ctx)


@dev_required
def version_edit(request, addon_id, addon, version_id):
version = get_object_or_404(Version, pk=version_id, addon=addon)
form = VersionForm(request.POST or None, instance=version)

if request.method == 'POST' and form.is_valid():
form.save()
messages.success(request, _('Changes successfully saved.'))
return redirect(addon.get_dev_url('versions.edit', args=[version.pk]))

return jingo.render(request, 'developers/apps/version_edit.html', {
'addon': addon, 'version': version, 'form': form})


@dev_required(owner_for_post=True, webapp=True)
def ownership(request, addon_id, addon, webapp=False):
# Authors.
Expand Down
13 changes: 11 additions & 2 deletions mkt/submit/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from django import forms

import happyforms
from tower import ugettext_lazy as _lazy
from tower import ugettext as _, ugettext_lazy as _lazy

from addons.forms import AddonFormBasic
from addons.models import Addon, AddonUpsell
Expand All @@ -12,6 +12,7 @@
from apps.users.notifications import app_surveys
from apps.users.models import UserNotification
from files.models import FileUpload
from files.utils import parse_addon
from market.models import AddonPremium, Price
from translations.widgets import TransInput, TransTextarea
from translations.fields import TransField
Expand Down Expand Up @@ -47,12 +48,20 @@ class NewWebappForm(happyforms.Form):
' upload. Please try again.')})

def __init__(self, *args, **kw):
self.addon = kw.pop('addon', None)
self.is_packaged = kw.pop('is_packaged', False)
super(NewWebappForm, self).__init__(*args, **kw)

def clean_upload(self):
upload = self.cleaned_data['upload']
if not self.is_packaged:
if self.is_packaged:
pkg = parse_addon(upload, self.addon)
ver = pkg.get('version')
if (ver and self.addon and
self.addon.versions.filter(version=ver).exists()):
raise forms.ValidationError(
_(u'Version %s already exists') % ver)
else:
# Throw an error if this is a dupe.
verify_app_domain(upload.name) # JS sets manifest as `upload.name`.
return upload
Expand Down

0 comments on commit 451c870

Please sign in to comment.