From 3122b606981ff7982657e37957081ccdc6342d84 Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Wed, 4 Oct 2023 10:32:12 +0200 Subject: [PATCH] Show modal warning at submission time when Android compatibility is checked (#21224) * Show modal warning at submission time when Android compatibility is checked --- src/olympia/addons/models.py | 10 +++++++ src/olympia/addons/tests/test_models.py | 19 ++++++++++++ src/olympia/constants/promoted.py | 8 ++--- src/olympia/devhub/forms.py | 7 ++--- .../templates/devhub/addons/submit/base.html | 3 ++ .../includes/android_compatibility_modal.html | 10 +++++++ src/olympia/devhub/tests/test_views_submit.py | 21 ++++++++++++- .../force_max_android_compatibility.py | 2 +- .../force_min_android_compatibility.py | 9 ++---- src/olympia/versions/models.py | 6 +--- static/css/devhub/popups.less | 4 +++ static/css/restyle/restyle.less | 2 +- static/js/zamboni/devhub.js | 30 +++++++++++++++++++ static/js/zamboni/global.js | 6 ++-- 14 files changed, 112 insertions(+), 25 deletions(-) create mode 100644 src/olympia/devhub/templates/devhub/includes/android_compatibility_modal.html diff --git a/src/olympia/addons/models.py b/src/olympia/addons/models.py index 9e7c58dda8..887d3c8d24 100644 --- a/src/olympia/addons/models.py +++ b/src/olympia/addons/models.py @@ -1545,6 +1545,16 @@ class Addon(OnChangeMixin, ModelBase): def can_set_compatibility(self): return self.type_can_set_compatibility(self.type) + @property + def can_be_compatible_with_all_fenix_versions(self): + """Whether or not the addon is allowed to be compatible with all Fenix + versions (i.e. it's a recommended/line extension for Android).""" + return ( + self.promoted + and self.promoted.group.can_be_compatible_with_all_fenix_versions + and amo.ANDROID in self.promoted.approved_applications + ) + def has_author(self, user): """True if ``user`` is an author of the add-on.""" if user is None or user.is_anonymous: diff --git a/src/olympia/addons/tests/test_models.py b/src/olympia/addons/tests/test_models.py index 3a9b27098f..716a605793 100644 --- a/src/olympia/addons/tests/test_models.py +++ b/src/olympia/addons/tests/test_models.py @@ -1847,6 +1847,25 @@ class TestAddonModels(TestCase): AddonGUID.objects.create(addon=addon, guid='not-a-guid') assert list(addon.blocklistsubmissions) == [submission] + def test_can_be_compatible_with_all_fenix_versions_property(self): + addon = addon_factory() + assert not addon.can_be_compatible_with_all_fenix_versions + + # It's promoted but nothing has been approved. + promoted = PromotedAddon.objects.create(addon=addon, group_id=LINE.id) + assert not addon.can_be_compatible_with_all_fenix_versions + + # The latest version is approved. + promoted.approve_for_version(addon.current_version) + del addon.promoted + assert addon.can_be_compatible_with_all_fenix_versions + + addon.promoted.update(application_id=amo.FIREFOX.id) + assert not addon.can_be_compatible_with_all_fenix_versions + + addon.promoted.update(application_id=amo.ANDROID.id) + assert addon.can_be_compatible_with_all_fenix_versions + class TestAddonUser(TestCase): def test_delete(self): diff --git a/src/olympia/constants/promoted.py b/src/olympia/constants/promoted.py index 3753346a9b..306260be81 100644 --- a/src/olympia/constants/promoted.py +++ b/src/olympia/constants/promoted.py @@ -22,7 +22,7 @@ _PromotedSuperClass = namedtuple( 'can_primary_hero', 'immediate_approval', 'flag_for_human_review', - 'can_be_compatible_with_fenix', # Note: we also check it's promoted for Android + 'can_be_compatible_with_all_fenix_versions', # If addon is promoted for Android ], defaults=( # "Since fields with a default value must come after any fields without @@ -38,7 +38,7 @@ _PromotedSuperClass = namedtuple( False, # can_primary_hero - can be added to a primary hero shelf False, # immediate_approval - will addon be auto-approved once added False, # flag_for_human_review - will be add-on be flagged for another review - False, # can_be_compatible_with_fenix + False, # can_be_compatible_with_all_fenix_versions ), ) @@ -69,7 +69,7 @@ RECOMMENDED = PromotedClass( applications.ANDROID.short: 'recommended-android', }, can_primary_hero=True, - can_be_compatible_with_fenix=True, + can_be_compatible_with_all_fenix_versions=True, ) SPONSORED = PromotedClass( @@ -113,7 +113,7 @@ LINE = PromotedClass( applications.ANDROID.short: 'line', }, can_primary_hero=True, - can_be_compatible_with_fenix=True, + can_be_compatible_with_all_fenix_versions=True, ) SPOTLIGHT = PromotedClass( diff --git a/src/olympia/devhub/forms.py b/src/olympia/devhub/forms.py index 438e83642e..34bb9f2435 100644 --- a/src/olympia/devhub/forms.py +++ b/src/olympia/devhub/forms.py @@ -885,10 +885,9 @@ class CompatForm(AMOModelForm): # On Android, disable Fenix-pre GA version range unless the add-on is # recommended or line. - if app == amo.ANDROID.id and not ( - addon.promoted - and addon.promoted.group.can_be_compatible_with_fenix - and amo.ANDROID in addon.promoted.approved_applications + if ( + app == amo.ANDROID.id + and not addon.can_be_compatible_with_all_fenix_versions ): self.fields[ 'min' diff --git a/src/olympia/devhub/templates/devhub/addons/submit/base.html b/src/olympia/devhub/templates/devhub/addons/submit/base.html index 337b714eaf..d6557e6015 100644 --- a/src/olympia/devhub/templates/devhub/addons/submit/base.html +++ b/src/olympia/devhub/templates/devhub/addons/submit/base.html @@ -42,5 +42,8 @@ + {% if not addon or not addon.can_be_compatible_with_all_fenix_versions %} + {% include 'devhub/includes/android_compatibility_modal.html' %} + {% endif %} {% endblock content %} diff --git a/src/olympia/devhub/templates/devhub/includes/android_compatibility_modal.html b/src/olympia/devhub/templates/devhub/includes/android_compatibility_modal.html new file mode 100644 index 0000000000..3a16ba6cbf --- /dev/null +++ b/src/olympia/devhub/templates/devhub/includes/android_compatibility_modal.html @@ -0,0 +1,10 @@ + diff --git a/src/olympia/devhub/tests/test_views_submit.py b/src/olympia/devhub/tests/test_views_submit.py index 7c0620449b..69f4a5ec28 100644 --- a/src/olympia/devhub/tests/test_views_submit.py +++ b/src/olympia/devhub/tests/test_views_submit.py @@ -32,7 +32,7 @@ from olympia.amo.tests import ( version_factory, ) from olympia.constants.licenses import LICENSES_BY_BUILTIN -from olympia.constants.promoted import NOTABLE +from olympia.constants.promoted import NOTABLE, RECOMMENDED from olympia.devhub import views from olympia.files.tests.test_models import UploadMixin from olympia.files.utils import parse_addon @@ -858,6 +858,13 @@ class TestAddonSubmitUpload(UploadMixin, TestCase): ) self.assert3xx(response, expected_location) + def test_android_compatibility_modal(self): + url = reverse('devhub.submit.upload', args=['listed']) + modal_selector = '#modals #modal-confirm-android-compatibility.modal' + response = self.client.get(url) + doc = pq(response.content) + assert doc(modal_selector) + class TestAddonSubmitSource(TestSubmitBase): def setUp(self): @@ -2508,6 +2515,18 @@ class TestVersionSubmitUploadListed(VersionSubmitUploadMixin, UploadMixin, TestC 'Version 0.0.1 must be greater than the previous approved version 0.0.1.0.' ) + def test_android_compatibility_modal(self): + url = reverse('devhub.submit.version.upload', args=[self.addon.slug, 'listed']) + modal_selector = '#modals #modal-confirm-android-compatibility.modal' + response = self.client.get(url) + doc = pq(response.content) + assert doc(modal_selector) + + self.make_addon_promoted(self.addon, RECOMMENDED, approve_version=True) + response = self.client.get(url) + doc = pq(response.content) + assert not doc(modal_selector) + class TestVersionSubmitUploadUnlisted(VersionSubmitUploadMixin, UploadMixin, TestCase): channel = amo.CHANNEL_UNLISTED diff --git a/src/olympia/versions/management/commands/force_max_android_compatibility.py b/src/olympia/versions/management/commands/force_max_android_compatibility.py index 896a5c5fef..fae934e57e 100644 --- a/src/olympia/versions/management/commands/force_max_android_compatibility.py +++ b/src/olympia/versions/management/commands/force_max_android_compatibility.py @@ -37,7 +37,7 @@ class Command(BaseCommand): application=amo.ANDROID.id, version=amo.MAX_VERSION_FENNEC ) promoted_groups_ids = [ - p.id for p in PROMOTED_GROUPS if p.can_be_compatible_with_fenix + p.id for p in PROMOTED_GROUPS if p.can_be_compatible_with_all_fenix_versions ] qs = ( # We only care about listed extensions already marked as compatible diff --git a/src/olympia/versions/management/commands/force_min_android_compatibility.py b/src/olympia/versions/management/commands/force_min_android_compatibility.py index 4b2f7903bf..3b16e8eec4 100644 --- a/src/olympia/versions/management/commands/force_min_android_compatibility.py +++ b/src/olympia/versions/management/commands/force_min_android_compatibility.py @@ -62,13 +62,10 @@ class Command(BaseCommand): count = 0 skipped = 0 for addon in addons: - if ( - addon.promoted - and addon.promoted.group.can_be_compatible_with_fenix - and amo.ANDROID in addon.promoted.approved_applications - ): + if addon.can_be_compatible_with_all_fenix_versions: log.info( - 'Skipping add-on id %d because of its promoted group.', addon.pk + 'Skipping add-on id %d because it can be compatible with Fenix.', + addon.pk, ) skipped += 1 continue diff --git a/src/olympia/versions/models.py b/src/olympia/versions/models.py index 4b14492935..3a6e534044 100644 --- a/src/olympia/versions/models.py +++ b/src/olympia/versions/models.py @@ -1467,11 +1467,7 @@ class ApplicationsVersions(models.Model): # Recommended/line for Android are allowed to set compatibility in the # limited range. - if ( - addon.promoted - and addon.promoted.group.can_be_compatible_with_fenix - and amo.ANDROID in addon.promoted.approved_applications - ): + if addon.can_be_compatible_with_all_fenix_versions: return False # Range is expressed as a [closed, open) interval. diff --git a/static/css/devhub/popups.less b/static/css/devhub/popups.less index 0e1903ba28..ea0bc22461 100644 --- a/static/css/devhub/popups.less +++ b/static/css/devhub/popups.less @@ -33,3 +33,7 @@ input.ui-autocomplete-loading, } } } + +.modal-actions button { + margin: 5px 0; +} diff --git a/static/css/restyle/restyle.less b/static/css/restyle/restyle.less index 1a0967ec08..93679479fb 100644 --- a/static/css/restyle/restyle.less +++ b/static/css/restyle/restyle.less @@ -1888,7 +1888,7 @@ h3.author .transfer-ownership { .contrib-overlay, .modal-overlay { - background: rgba(0, 0, 0, 0.5); + background: rgba(0, 0, 0, 0.7); } .primary.island { diff --git a/static/js/zamboni/devhub.js b/static/js/zamboni/devhub.js index f0a2003432..3761c53ba8 100644 --- a/static/js/zamboni/devhub.js +++ b/static/js/zamboni/devhub.js @@ -1350,4 +1350,34 @@ function initSubmitModals() { return false; // don't follow the a.href link }); } + + // Warn about Android compatibility (if selected). + if ($('#modal-confirm-android-compatibility').length > 0) { + let confirmedOnce = false; + let $input = $('#id_compatible_apps label.android input[type=checkbox]'); + + const $modalAndroidConfirm = $( + '#modal-confirm-android-compatibility', + ).modal('#id_compatible_apps label.android', { + width: 630, + callback: function shouldShowAndroidModal(options) { + if ($input.prop('disabled')) { + return false; + } + if (confirmedOnce) { + $input.prop('checked', !$input.prop('checked')); + } + return !confirmedOnce; + }, + }); + + $('#modal-confirm-android-compatibility') + .find('form') + .on('submit', function onSubmit(e) { + e.preventDefault(); + $input.prop('checked', true); + $modalAndroidConfirm.trigger('close'); + confirmedOnce = true; + }); + } } diff --git a/static/js/zamboni/global.js b/static/js/zamboni/global.js index 3eb0f84d11..6aacb21a2b 100644 --- a/static/js/zamboni/global.js +++ b/static/js/zamboni/global.js @@ -304,10 +304,10 @@ $.fn.modal = function (click_target, o) { offset = offset || $modal.o.offset; $modal.detach().appendTo('body'); - var toX = ($(window).width() - $modal.outerWidth(false)) / 2, - toY = $(window).scrollTop() + 26; //distance from top of the window + let toX = offset.x || ($(window).width() - $modal.outerWidth(false)) / 2, + toY = offset.y || 160; $modal.css({ - left: toX, + left: toX + 'px', top: toY + 'px', right: 'inherit', bottom: 'inherit',