Don't allow for empty 'more information' message (#17990)

Author: di

tests/unit/admin/views/test_organizations.py

@@ -1074,6 +1074,44 @@ def test_request_more_information_for_not_found(self):
         with pytest.raises(HTTPNotFound):
             views.organization_application_request_more_information(request)
 
+    @pytest.mark.usefixtures("_enable_organizations")
+    def test_request_more_information_no_message(self, db_request):
+        admin = UserFactory.create()
+        user = UserFactory.create()
+        organization_application = OrganizationApplicationFactory.create(
+            name="example", submitted_by=user
+        )
+
+        organization_service = pretend.stub(
+            get_organization_application=lambda *a, **kw: organization_application,
+            request_more_information=pretend.call_recorder(pretend.raiser(ValueError)),
+        )
+
+        db_request.matchdict = {
+            "organization_application_id": organization_application.id
+        }
+        db_request.params = {}
+        db_request.user = admin
+        db_request.route_path = pretend.call_recorder(_organization_application_routes)
+        db_request.find_service = pretend.call_recorder(
+            lambda iface, context: organization_service
+        )
+        db_request.session.flash = pretend.call_recorder(lambda *a, **kw: None)
+
+        result = views.organization_application_request_more_information(db_request)
+
+        assert organization_service.request_more_information.calls == [
+            pretend.call(organization_application.id, db_request),
+        ]
+        assert db_request.session.flash.calls == [
+            pretend.call("No message provided", queue="error"),
+        ]
+        assert result.status_code == 303
+        assert (
+            result.location
+            == f"/admin/organization_applications/{organization_application.id}/"
+        )
+
     @pytest.mark.usefixtures("_enable_organizations")
     def test_decline(self, db_request):
         admin = UserFactory.create()

tests/unit/organizations/test_services.py

@@ -271,6 +271,7 @@ def test_request_more_information_organization_application(
 
         admin = UserFactory(username="admin", is_superuser=True)
         db_request.user = admin
+        db_request.params["message"] = "some message"
 
         organization_application = OrganizationApplicationFactory.create()
         organization_service.request_more_information(
@@ -288,10 +289,30 @@ def test_request_more_information_organization_application(
                 organization_application.submitted_by,
                 organization_name=organization_application.name,
                 organization_application_id=organization_application.id,
-                message="",
+                message="some message",
             ),
         ]
 
+    def test_request_more_information_organization_application_no_message(
+        self, db_request, organization_service, monkeypatch
+    ):
+        send_email = pretend.call_recorder(lambda *a, **kw: None)
+        monkeypatch.setattr(
+            services, "send_new_organization_moreinformationneeded_email", send_email
+        )
+
+        admin = UserFactory(username="admin", is_superuser=True)
+        db_request.user = admin
+
+        organization_application = OrganizationApplicationFactory.create()
+        with pytest.raises(ValueError):  # noqa
+            organization_service.request_more_information(
+                organization_application.id, db_request
+            )
+
+        assert len(organization_application.observations) == 0
+        assert send_email.calls == []
+
     def test_decline_organization_application(
         self, db_request, organization_service, monkeypatch
     ):

warehouse/admin/static/js/warehouse.js

@@ -275,22 +275,28 @@ $(document).ready(function() {
     document.onkeyup = function(){};
   }
   function hotKeys(e) {
-    console.log(e);
     if (keyBindings.has(String(e.which))) {
       unbindHotKeys();
-      $(keyBindings.get(String(e.which))).modal("show");
-      $(keyBindings.get(String(e.which))).on("hidden.bs.modal", bindHotKeys);
+      const targetModal = $(keyBindings.get(String(e.which)));
+      targetModal.one("shown.bs.modal", function () {
+        const firstFocusableElement = $(this).find("input:visible, textarea:visible").first();
+        if (firstFocusableElement.length) {
+          firstFocusableElement.focus();
+        }
+      });
+      targetModal.modal("show");
+      targetModal.on("hidden.bs.modal", bindHotKeys);
     }
   }
   modalHotKeyBindings.forEach(function(modalHotKeyBinding) {
     if (! modalHotKeyBinding.disabled) {
       keyBindings.set(modalHotKeyBinding.dataset.hotkeyBinding, modalHotKeyBinding.dataset.target);
     }
   });
-  const inputs = document.querySelectorAll("input");
-  inputs.forEach(function(input) {
-    input.addEventListener("focusin", unbindHotKeys);
-    input.addEventListener("focusout", bindHotKeys);
+  const focusable = document.querySelectorAll("input, textarea");
+  focusable.forEach(function(element) {
+    element.addEventListener("focusin", unbindHotKeys);
+    element.addEventListener("focusout", bindHotKeys);
   });
   bindHotKeys();
 });

warehouse/admin/views/organizations.py

@@ -470,18 +470,22 @@ def organization_application_request_more_information(request):
     if organization_application is None:
         raise HTTPNotFound
 
-    organization_service.request_more_information(organization_application.id, request)
-
-    request.session.flash(
-        (
-            f'Request for more info from "{organization_application.name}" '
-            "organization sent"
-        ),
-        queue="success",
-    )
+    try:
+        organization_service.request_more_information(
+            organization_application.id, request
+        )
+        request.session.flash(
+            (
+                f'Request for more info from "{organization_application.name}" '
+                "organization sent"
+            ),
+            queue="success",
+        )
 
-    if request.params.get("organization_applications_turbo_mode") == "true":
-        return _turbo_mode(request)
+        if request.params.get("organization_applications_turbo_mode") == "true":
+            return _turbo_mode(request)
+    except ValueError:
+        request.session.flash("No message provided", queue="error")
 
     return HTTPSeeOther(
         request.route_path(

warehouse/organizations/services.py

@@ -270,6 +270,9 @@ def request_more_information(self, organization_application_id, request):
 
         message = request.params.get("message", "")
 
+        if not message:
+            raise ValueError
+
         organization_application.record_observation(
             request=request,
             actor=request.user,