From 2c3f67a12f6c20eecd82eb7f4295b4093609420c Mon Sep 17 00:00:00 2001 From: Chris Van Date: Tue, 3 Sep 2013 11:52:42 -0700 Subject: [PATCH] Revert "manage packaged-app submission for Android and Desktop via waffle flag (bug 907203, bug 910778)" This reverts commit a5a9b3df66dbc219702e5f3f3dd04d842cd6c9b1. --- media/js/devreg/submit.js | 30 +++---------- migrations/655-waffle-packaged-apps.sql | 5 --- mkt/developers/forms_payments.py | 11 ++--- mkt/developers/tests/test_forms_payments.py | 30 +++---------- mkt/submit/forms.py | 33 +++++---------- mkt/submit/templates/submit/manifest.html | 4 +- mkt/submit/tests/test_forms.py | 47 ++++++++++----------- 7 files changed, 53 insertions(+), 107 deletions(-) delete mode 100644 migrations/655-waffle-packaged-apps.sql diff --git a/media/js/devreg/submit.js b/media/js/devreg/submit.js index 5850e1735b0..d70ac8bc174 100644 --- a/media/js/devreg/submit.js +++ b/media/js/devreg/submit.js @@ -54,7 +54,7 @@ // When a big device button is clicked, update the form. var $upload_form = $('#upload-webapp'), $qhd = $('#id_has_qhd'); - z.body.on('change', '#upload-webapp select', function() { + $(document.body).on('change', '#upload-webapp select', function() { // IT'S FINE. IT'S FINE. if (!$upload_form.find('option[value$="-desktop"]:selected, option[value$="-tablet"]:selected').length) { $qhd.prop('checked', true).trigger('change'); @@ -102,29 +102,11 @@ // Condition to show packaged tab...ugly but works. function showPackagedTab() { - // If the Android flag is disabled, and you tried to select - // Android Mobile or Tablet... no packaged apps for you. - // (This lets us prevent you from marking your app as compatible - // with both Firefox OS *and* Android when Android support - // hasn't landed yet.) - if (!$('[data-packaged-platforms~="android"]').length && - $('option[value*="-android-"]:selected').length) { - return false; - } - - // If the Desktop flag is disabled, and you tried to select - // Desktop... no packaged apps for you. - if (!$('[data-packaged-platforms~="desktop"]').length && - $('option[value$="-desktop"]:selected').length) { - return false; - } - return ($('#id_free_platforms option[value="free-firefoxos"]:selected').length && - $('#id_free_platforms option:selected').length == 1) || - $('#id_paid_platforms option[value="paid-firefoxos"]:selected').length || - $('[data-packaged-platforms~="android"] option[value*="-android-"]:selected').length || - $('[data-packaged-platforms~="desktop"] option[value$="-desktop"]:selected').length || - allTabsDeselected(); + $('#id_free_platforms option:selected').length == 1) || + $('#id_paid_platforms option[value="paid-firefoxos"]:selected').length || + $('option[value*=android]:selected').length || + allTabsDeselected(); } // Toggle packaged/hosted tab state. @@ -142,7 +124,7 @@ } } - z.body.on('tabs-changed', function(e, tab) { + $(document).on('tabs-changed', function(e, tab) { if (tab.id == 'packaged-tab-header') { $('.learn-mdn.active').removeClass('active'); $('.learn-mdn.packaged').addClass('active'); diff --git a/migrations/655-waffle-packaged-apps.sql b/migrations/655-waffle-packaged-apps.sql deleted file mode 100644 index 077e005eadb..00000000000 --- a/migrations/655-waffle-packaged-apps.sql +++ /dev/null @@ -1,5 +0,0 @@ -INSERT INTO waffle_flag_mkt (name, everyone, percent, superusers, staff, authenticated, rollout, testing, languages, note, created, modified) - VALUES ('android-packaged', 0, NULL, 0, 0, 0, 0, 0, '', 'ON: packaged apps for Android can be submitted and show up in search results; OFF: packaged apps for Android are disallowed', NOW(), NOW()); - -INSERT INTO waffle_flag_mkt (name, everyone, percent, superusers, staff, authenticated, rollout, testing, languages, note, created, modified) - VALUES ('desktop-packaged', 0, NULL, 0, 0, 0, 0, 0, '', 'ON: packaged apps for Desktop can be submitted and show up in search results; OFF: packaged apps for Desktop are disallowed', NOW(), NOW()); diff --git a/mkt/developers/forms_payments.py b/mkt/developers/forms_payments.py index 2eb88e4394c..fdd20b1d85c 100644 --- a/mkt/developers/forms_payments.py +++ b/mkt/developers/forms_payments.py @@ -170,14 +170,14 @@ def is_toggling(self): return value if value in ('free', 'paid') else False def clean(self): + is_toggling = self.is_toggling() - if self.addon.is_packaged: - self._set_packaged_errors() - if self._errors.get('free_platforms'): - return self.cleaned_data + if self.addon.is_packaged and 'desktop' in self._get_combined(): + self._errors['free_platforms'] = self._errors['paid_platforms'] = ( + self.ERRORS['packaged']) - if not is_toggling: + elif not is_toggling: # If a platform wasn't selected, raise an error. if not self.cleaned_data[ '%s_platforms' % ('paid' if self.is_paid() else 'free')]: @@ -198,6 +198,7 @@ def clean(self): return self.cleaned_data def clean_price(self): + price_value = self.cleaned_data.get('price') premium_type = self.cleaned_data.get('premium_type') if ((premium_type in amo.ADDON_PREMIUMS diff --git a/mkt/developers/tests/test_forms_payments.py b/mkt/developers/tests/test_forms_payments.py index 9d8680bbae6..16e73dd0221 100644 --- a/mkt/developers/tests/test_forms_payments.py +++ b/mkt/developers/tests/test_forms_payments.py @@ -1,3 +1,5 @@ +from django.conf import settings + import mock from nose.tools import eq_, ok_ from pyquery import PyQuery as pq @@ -222,11 +224,8 @@ def test_optgroups_in_price_choices(self): Price.objects.create(price='1.00', method=PAYMENT_METHOD_ALL) Price.objects.create(price='2.00', method=PAYMENT_METHOD_ALL) form = forms_payments.PremiumForm(self.platforms, **self.kwargs) - # 1 x Free with inapp - # + 1 x price tier 0 - # + 3 x values grouped by billing - # + 1 x 'Please select' - # = 6 + # 1 x Free with inapp + 1 x price tier 0 + 3 x values grouped by billing + + # 1 x 'Please select' = 6. eq_(len(form.fields['price'].choices), 6) html = form.as_p() eq_(len(pq(html)('#id_price optgroup')), 3, 'Should be 3 optgroups') @@ -248,13 +247,6 @@ def test_cannot_set_desktop_for_packaged_app(self): form = forms_payments.PremiumForm(data=self.platforms, **self.kwargs) assert not form.is_valid() - def test_can_set_desktop_for_packaged_app(self): - self.create_flag('desktop-packaged') - self.platforms = {'free_platforms': ['free-desktop']} - self.addon.update(is_packaged=True) - form = forms_payments.PremiumForm(data=self.platforms, **self.kwargs) - assert form.is_valid(), form.errors - def test_can_change_devices_for_hosted_app(self): # Specify the free and paid. It shouldn't fail because you can't change # payment types without explicitly specifying that. @@ -266,17 +258,7 @@ def test_can_change_devices_for_hosted_app(self): self.assertSetEqual(self.addon.device_types, [amo.DEVICE_DESKTOP]) - def test_cannot_change_android_devices_for_packaged_app(self): - self.platforms = {'free_platforms': ['free-android-mobile'], - 'paid_platforms': ['paid-firefoxos']} # Ignored. - self.addon.update(is_packaged=True) - form = forms_payments.PremiumForm(data=self.platforms, **self.kwargs) - assert not form.is_valid() - - self.assertSetEqual(self.addon.device_types, [amo.DEVICE_GAIA]) - - def test_can_change_devices_for_packaged_app_behind_flag(self): - self.create_flag('android-packaged') + def test_can_change_devices_for_packaged_app(self): self.platforms = {'free_platforms': ['free-android-mobile'], 'paid_platforms': ['paid-firefoxos']} # Ignored. self.addon.update(is_packaged=True) @@ -524,7 +506,7 @@ def test_rereview(self, client): eq_(RereviewQueue.objects.count(), 1) form = forms_payments.BangoAccountListForm(None, **self.kwargs) - eq_(form.fields['accounts'].empty_label, None) + assert form.fields['accounts'].empty_label == None @mock.patch('mkt.developers.models.client') def test_disagreed_tos_rereview(self, client): diff --git a/mkt/submit/forms.py b/mkt/submit/forms.py index 1c9f4ca5c9d..dbb49a91f0f 100644 --- a/mkt/submit/forms.py +++ b/mkt/submit/forms.py @@ -47,8 +47,8 @@ class DeviceTypeForm(happyforms.Form): ERRORS = { 'both': _lazy(u'Cannot be free and paid.'), 'none': _lazy(u'Please select a device.'), - 'packaged': _lazy(u'Packaged apps are not yet supported for those ' - u'platforms.'), + 'packaged': _lazy(u'Packaged apps are valid for only Firefox OS ' + 'and Android.'), } free_platforms = forms.MultipleChoiceField( @@ -85,21 +85,6 @@ def _get_combined(self): self.cleaned_data.get('paid_platforms', [])) return set(d.split('-', 1)[1] for d in devices) - def _set_packaged_errors(self): - """Add packaged-app submission errors for incompatible platforms.""" - devices = self._get_combined() - bad_android = ( - not waffle.flag_is_active(self.request, 'android-packaged') and - ('android-mobile' in devices or 'android-tablet' in devices) - ) - bad_desktop = ( - not waffle.flag_is_active(self.request, 'desktop-packaged') and - 'desktop' in devices - ) - if bad_android or bad_desktop: - self._errors['free_platforms'] = self._errors['paid_platforms'] = ( - self.ERRORS['packaged']) - def clean(self): data = self.cleaned_data paid = data.get('paid_platforms', []) @@ -176,11 +161,13 @@ def __init__(self, *args, **kw): del self.fields['paid_platforms'] def clean(self): + data = self.cleaned_data if 'upload' not in self.cleaned_data: self._errors['upload'] = self.upload_error return + # Packaged apps are only valid for firefox os. if self.is_packaged(): # Now run the packaged app check, done in clean, because # clean_packaged needs to be processed first. @@ -233,8 +220,7 @@ def __init__(self, *args, **kwargs): self.request = kwargs.pop('request', None) super(NewWebappForm, self).__init__(*args, **kwargs) if 'paid_platforms' in self.fields: - self.fields['paid_platforms'].choices = PAID_PLATFORMS( - self.request) + self.fields['paid_platforms'].choices = PAID_PLATFORMS(self.request) def _add_error(self, msg): self._errors['free_platforms'] = self._errors['paid_platforms'] = ( @@ -245,10 +231,11 @@ def clean(self): if not data: return - if self.is_packaged(): - self._set_packaged_errors() - if self._errors.get('free_platforms'): - return + combined_platforms = self._get_combined() + if self.is_packaged() and 'desktop' in combined_platforms: + self._errors['free_platforms'] = self._errors['paid_platforms'] = ( + self.ERRORS['packaged']) + return return data diff --git a/mkt/submit/templates/submit/manifest.html b/mkt/submit/templates/submit/manifest.html index b4a8733fbf1..4337e3c0f34 100644 --- a/mkt/submit/templates/submit/manifest.html +++ b/mkt/submit/templates/submit/manifest.html @@ -26,6 +26,7 @@

{{ _('Submit an App') }}

{{ progress(request, addon=None, step=step) }} +
{% if waffle.flag('allow-b2g-paid-submission') %} @@ -50,8 +51,7 @@ {% endif %}
-
+

{{ _('Hosted') }}

{{ _("Submit your app manifest URL") }}

diff --git a/mkt/submit/tests/test_forms.py b/mkt/submit/tests/test_forms.py index cc29a7e8c24..b4018baf0e3 100644 --- a/mkt/submit/tests/test_forms.py +++ b/mkt/submit/tests/test_forms.py @@ -96,33 +96,32 @@ def test_not_packaged_allowed(self): assert form.is_valid(), form.errors assert not form.is_packaged() - @mock.patch('mkt.submit.forms.parse_addon', - lambda *args: {'version': None}) - def test_packaged_disallowed_behind_flag(self): - for device in ('free-desktop', - 'free-android-mobile', - 'free-android-tablet'): - form = forms.NewWebappForm({'free_platforms': [device], - 'upload': self.file.uuid, - 'packaged': True}) - assert not form.is_valid(), form.errors - eq_(form.ERRORS['packaged'], form.errors['paid_platforms']) + @mock.patch('mkt.submit.forms.parse_addon') + def test_packaged_allowed(self, parse_addon): + parse_addon.return_value = {} + form = forms.NewWebappForm({'free_platforms': ['free-firefoxos'], + 'upload': self.file.uuid, + 'packaged': True}) + assert form.is_valid(), form.errors + assert form.is_packaged() + + @mock.patch('mkt.submit.forms.parse_addon') + def test_packaged_allowed_android(self, parse_addon): + parse_addon.return_value = {} + form = forms.NewWebappForm({'free_platforms': ['free-android-mobile'], + 'upload': self.file.uuid, + 'packaged': True}) + assert form.is_valid(), form.errors + assert form.is_packaged() @mock.patch('mkt.submit.forms.parse_addon', lambda *args: {'version': None}) - def test_packaged_allowed_everywhere(self): - self.create_flag('android-packaged') - self.create_flag('desktop-packaged') - for device in ('free-firefoxos', - 'free-desktop', - 'free-android-tablet', - 'free-android-mobile'): - form = forms.NewWebappForm({'free_platforms': [device], - 'upload': self.file.uuid, - 'packaged': True}, - request=self.request) - assert form.is_valid(), form.errors - assert form.is_packaged() + def test_packaged_wrong_device(self): + form = forms.NewWebappForm({'free_platforms': ['free-desktop'], + 'upload': self.file.uuid, + 'packaged': True}) + assert not form.is_valid(), form.errors + eq_(form.ERRORS['packaged'], form.errors['paid_platforms']) class TestNewWebappVersionForm(amo.tests.TestCase):