allow China-rejected apps to be requested against for inclusion in China (bug 922252, bug @clouserw)

mkt/developers/forms.py

@@ -89,15 +89,10 @@ def toggle_app_for_special_regions(request, app, enabled_regions=None):
     if not waffle.flag_is_active(request, 'special-regions'):
         return
 
-    # We omit `amo.STATUS_REJECTED` because we shouldn't be altering that
-    # status. If a reviewer rejects that app in China, that's permanent!
-    valid_statuses = (amo.STATUS_NULL, amo.STATUS_PENDING, amo.STATUS_PUBLIC)
-
     for region in mkt.regions.SPECIAL_REGIONS:
         status = app.geodata.get_status(region)
 
-        if (status in valid_statuses and
-            enabled_regions is not None):
+        if enabled_regions is not None:
             if region.id in enabled_regions:
                 # If it's not already enabled, mark as pending.
                 if status != amo.STATUS_PUBLIC:
@@ -764,13 +759,6 @@ class RegionForm(forms.Form):
                 if not self.product.content_ratings_in(region):
                     disabled_regions.add(region.id)
 
-        # If it's been rejected for China, for example, don't ever allow
-        # the developer to opt in again.
-        for region in self.regions_before:
-            status = self.product.geodata.get_status(region)
-            if status == amo.STATUS_REJECTED:
-                disabled_regions.add(region)
-
         return disabled_regions
 
     @property
@@ -821,16 +809,6 @@ class RegionForm(forms.Form):
                     raise region_error(region)
         return regions
 
-    def clean_special_regions(self):
-        special_regions = self.cleaned_data['special_regions']
-        if not self.is_toggling():
-            # Handle special regions.
-            for region in special_regions:
-                status = self.product.geodata.get_status(region)
-                if status == amo.STATUS_REJECTED:
-                    raise region_error(region)
-        return special_regions
-
     def save(self):
         # Don't save regions if we are toggling.
         if self.is_toggling():

mkt/developers/tests/test_forms.py

@@ -291,33 +291,6 @@ class TestRegionForm(amo.tests.WebappTestCase):
             {'regions': [mkt.regions.WORLDWIDE.id]}, **self.kwargs)
         assert form.is_valid(), form.errors
 
-    def test_china_initially_excluded_and_disallowed_if_rejected(self):
-        self.create_flag('special-regions')
-
-        # Mark app as rejected in China.
-        status = amo.STATUS_REJECTED
-        self.app.geodata.set_status(mkt.regions.CN, status, save=True)
-        eq_(self.app.geodata.get_status(mkt.regions.CN), status)
-
-        # Post the form.
-        form = forms.RegionForm({'regions': mkt.regions.ALL_REGION_IDS,
-                                 'special_regions': [mkt.regions.CN.id]},
-                                **self.kwargs)
-        cn = mkt.regions.CN.id
-        assert cn not in form.initial['regions']
-        assert cn in dict(form.fields['regions'].choices).keys()
-        eq_(form.disabled_regions, [cn])
-
-        # Form should be invalid because this app was already rejected in
-        # China.
-        assert not form.is_valid(), 'Form should be invalid'
-        error_msg = u'You cannot select %s.' % unicode(mkt.regions.CN.name)
-        eq_(form.errors, {'special_regions': [error_msg]})
-
-        # App should still be rejected.
-        self.app = self.app.reload()
-        eq_(self.app.geodata.get_status(mkt.regions.CN), status)
-
     def test_china_initially_excluded_if_null(self):
         self.create_flag('special-regions')
         form = forms.RegionForm(None, **self.kwargs)
@@ -325,31 +298,11 @@ class TestRegionForm(amo.tests.WebappTestCase):
         assert cn not in form.initial['regions']
         assert cn in dict(form.fields['regions'].choices).keys()
 
-    def test_china_disabled_and_excluded_if_rejected(self):
+    def _test_china_excluded_if_pending_or_rejected(self):
         self.create_flag('special-regions')
 
-        # Mark app as rejected in China.
-        status = amo.STATUS_REJECTED
-        self.app.geodata.set_status(mkt.regions.CN, status, save=True)
-        eq_(self.app.geodata.get_status(mkt.regions.CN), status)
-
-        # Post the form.
-        form = forms.RegionForm({'regions': mkt.regions.ALL_REGION_IDS},
-                                **self.kwargs)
-        eq_(form.disabled_regions, [mkt.regions.CN.id])
-        assert form.is_valid(), form.errors
-        form.save()
-
-        # App should be unlisted in China and still rejected.
-        self.app = self.app.reload()
-        eq_(self.app.listed_in(mkt.regions.CN), False)
-        eq_(self.app.geodata.get_status(mkt.regions.CN), status)
-
-    def _test_china_excluded_if_pending(self):
-        self.create_flag('special-regions')
-
-        # Mark app as pending in China.
-        status = amo.STATUS_PENDING
+        # Mark app as pending/rejected in China.
+        for status in (amo.STATUS_PENDING, amo.STATUS_REJECTED):
             self.app.geodata.set_status(mkt.regions.CN, status, save=True)
             eq_(self.app.geodata.get_status(mkt.regions.CN), status)
 
@@ -358,30 +311,39 @@ class TestRegionForm(amo.tests.WebappTestCase):
                                      'special_regions': [mkt.regions.CN.id]},
                                     **self.kwargs)
 
-        # China should be checked if it's pending.
+            # China should be checked if it's pending and
+            # unchecked if rejected.
             cn = mkt.regions.CN.id
-        assert cn in form.initial['regions']
-        assert cn in dict(form.fields['regions'].choices).keys()
+            if status == amo.STATUS_PENDING:
+                assert cn in form.initial['regions'], (
+                    status, form.initial['regions'])
+            else:
+                assert cn not in form.initial['regions'], (
+                    status, form.initial['regions'])
+            choices = dict(form.fields['regions'].choices).keys()
+            assert cn in choices, (status, choices)
 
             eq_(form.disabled_regions, [])
             assert form.is_valid(), form.errors
             form.save()
 
-        # App should be unlisted in China and still pending.
+            # App should be unlisted in China and always pending after
+            # requesting China.
             self.app = self.app.reload()
             eq_(self.app.listed_in(mkt.regions.CN), False)
-        eq_(self.app.geodata.get_status(mkt.regions.CN), status)
+            eq_(self.app.geodata.get_status(mkt.regions.CN),
+                amo.STATUS_PENDING)
 
-    def test_china_excluded_if_pending(self):
-        self._test_china_excluded_if_pending()
+    def test_china_excluded_if_pending_or_rejected(self):
+        self._test_china_excluded_if_pending_or_rejected()
 
-    def test_china_already_excluded_and_pending(self):
+    def test_china_already_excluded_and_pending_or_rejected(self):
         cn = mkt.regions.CN.id
         self.app.addonexcludedregion.create(region=cn)
 
         # If the app was already excluded in China, the checkbox should still
         # be checked if the app's been requested for approval in China now.
-        self._test_china_excluded_if_pending()
+        self._test_china_excluded_if_pending_or_rejected()
 
     def test_china_excluded_if_pending_cancelled(self):
         """