More org review tweaks (#17890)

* add a lil visual for applications with outstanding/fulfilled info requests

* org request more info template: remove leading www. from parsed domain

* handle borked urls

* Allow admins to edit applications on behalf of users

Author: ewdurbin

tests/unit/admin/views/test_organizations.py

@@ -14,6 +14,7 @@
 import pytest
 
 from pyramid.httpexceptions import HTTPBadRequest, HTTPNotFound
+from webob.multidict import MultiDict
 
 from warehouse.admin.views import organizations as views
 from warehouse.organizations.interfaces import IOrganizationService
@@ -545,11 +546,63 @@ def test_detail(self, db_request):
         db_request.matchdict["organization_application_id"] = (
             organization_application.id
         )
-        assert views.organization_application_detail(db_request) == {
-            "user": organization_application.submitted_by,
-            "conflicting_applications": [],
-            "organization_application": organization_application,
-        }
+        result = views.organization_application_detail(db_request)
+        assert result["user"] == organization_application.submitted_by
+        assert result["form"].name.data == organization_application.name
+        assert result["conflicting_applications"] == []
+        assert result["organization_application"] == organization_application
+
+    @pytest.mark.usefixtures("_enable_organizations")
+    def test_detail_edit(self, db_request):
+        organization_application = OrganizationApplicationFactory.create()
+        db_request.matchdict["organization_application_id"] = (
+            organization_application.id
+        )
+
+        new_org_name = f"New-Org-Name-{organization_application.name}"
+        db_request.method = "POST"
+        db_request.POST["name"] = new_org_name
+        db_request.POST["description"] = organization_application.description
+        db_request.POST["display_name"] = organization_application.display_name
+        db_request.POST["link_url"] = organization_application.link_url
+        db_request.POST["orgtype"] = organization_application.orgtype
+        db_request.POST = MultiDict(db_request.POST)
+
+        db_request.session.flash = pretend.call_recorder(lambda *a, **kw: None)
+        db_request.current_route_path = lambda *a, **kw: "/the/url/"
+
+        result = views.organization_application_detail(db_request)
+
+        assert result.status_code == 303
+        assert result.location == "/the/url/"
+        assert db_request.session.flash.calls == [
+            pretend.call(
+                f"Application for {organization_application.name!r} updated",
+                queue="success",
+            )
+        ]
+
+        assert organization_application.name == new_org_name
+
+    @pytest.mark.usefixtures("_enable_organizations")
+    def test_detail_edit_invalid(self, db_request):
+        existing_organization = OrganizationFactory.create()
+        organization_application = OrganizationApplicationFactory.create()
+
+        db_request.matchdict["organization_application_id"] = (
+            organization_application.id
+        )
+        db_request.method = "POST"
+        db_request.POST["name"] = existing_organization.name
+        db_request.POST = MultiDict(db_request.POST)
+
+        result = views.organization_application_detail(db_request)
+
+        assert result["user"] == organization_application.submitted_by
+        assert result["form"].name.data == existing_organization.name
+        assert result["form"].name.errors != []
+        assert result["conflicting_applications"] == []
+        assert result["organization_application"] == organization_application
 
     @pytest.mark.usefixtures("_enable_organizations")
     def test_detail_is_approved_true(self, db_request):
@@ -559,11 +612,11 @@ def test_detail_is_approved_true(self, db_request):
         db_request.matchdict["organization_application_id"] = (
             organization_application.id
         )
-        assert views.organization_application_detail(db_request) == {
-            "user": organization_application.submitted_by,
-            "conflicting_applications": [],
-            "organization_application": organization_application,
-        }
+        result = views.organization_application_detail(db_request)
+        assert result["user"] == organization_application.submitted_by
+        assert result["form"].name.data == organization_application.name
+        assert result["conflicting_applications"] == []
+        assert result["organization_application"] == organization_application
 
     @pytest.mark.usefixtures("_enable_organizations")
     def test_detail_is_approved_false(self, db_request):
@@ -573,11 +626,11 @@ def test_detail_is_approved_false(self, db_request):
         db_request.matchdict["organization_application_id"] = (
             organization_application.id
         )
-        assert views.organization_application_detail(db_request) == {
-            "user": organization_application.submitted_by,
-            "conflicting_applications": [],
-            "organization_application": organization_application,
-        }
+        result = views.organization_application_detail(db_request)
+        assert result["user"] == organization_application.submitted_by
+        assert result["form"].name.data == organization_application.name
+        assert result["conflicting_applications"] == []
+        assert result["organization_application"] == organization_application
 
     @pytest.mark.usefixtures("_enable_organizations")
     @pytest.mark.parametrize(
@@ -601,11 +654,11 @@ def test_detail_conflicting_applications(self, db_request, name, conflicts):
         db_request.matchdict["organization_application_id"] = (
             organization_application.id
         )
-        assert views.organization_application_detail(db_request) == {
-            "user": organization_application.submitted_by,
-            "conflicting_applications": conflicting_applications,
-            "organization_application": organization_application,
-        }
+        result = views.organization_application_detail(db_request)
+        assert result["user"] == organization_application.submitted_by
+        assert result["form"].name.data == organization_application.name
+        assert result["conflicting_applications"] == conflicting_applications
+        assert result["organization_application"] == organization_application
 
     @pytest.mark.usefixtures("_enable_organizations")
     def test_detail_not_found(self):

warehouse/admin/static/js/warehouse.js

@@ -240,6 +240,13 @@ if (requestMoreInformationEmailTemplateButton !== "undefined") {
   });
 }
 
+let editModalForm = document.getElementById("editModalForm");
+if (editModalForm !== "undefined") {
+  if (editModalForm.classList.contains("edit-form-errors")) {
+    document.getElementById("editModalButton").click();
+  }
+}
+
 // Link Checking
 const links = document.querySelectorAll("a[data-check-link-url]");
 links.forEach(function(link){

warehouse/admin/templates/admin/organization_applications/detail.html

@@ -20,13 +20,31 @@
   <li class="breadcrumb-item active">{{ organization_application.name }}</li>
 {% endblock %}
 
+{% macro render_field(label, field, input_id, placeholder=None, class=None) %}
+<div class="form-group{% if field.errors %} has-error{% endif %}">
+  <label>
+    {{ label }}
+  </label>
+  {{ field(id=input_id, class=class, placeholder=placeholder) }}
+
+  {% if field.errors %}
+  <div class="help-block">
+  {% for error in field.errors %}
+    <div class="text-red">{{ error }}</div>
+  {% endfor %}
+  </div>
+  {% endif %}
+</div>
+{% endmacro %}
+
 {% block content %}
   <div class="row">
     <div class="col-md-3">
       <div class="card card-primary">
         <div class="card-body card-widget widget-user-1">
           <div class="widget-user-header">
             <h3 class="widget-user-username text-center">{{ organization_application.name }}</h3>
+            <h6 class="widget-user-username text-center">{{ organization_application.display_name }}</h6>
           </div>
         </div>
         <div class="card-body">
@@ -41,6 +59,19 @@ <h3 class="widget-user-username text-center">{{ organization_application.name }}
           <h2 class="card-title">Actions</h2>
         </div>
         <div class="card-body">
+          <div class="form-group">
+            <button
+              type="button"
+              class="btn btn-warning btn-block"
+              id="editModalButton"
+              data-toggle="modal"
+              data-target="#editModal" {{ 'disabled' if not request.has_permission(Permissions.AdminOrganizationsWrite) or organization_application.status.value in ['approved', 'declined'] }}
+              title="Edit organization request"
+            >
+              <i class="fa-solid fa-pen-to-square"></i> Edit
+            </button>
+          </div>
+
           <div class="form-group">
             <button
               type="button"
@@ -83,6 +114,37 @@ <h2 class="card-title">Actions</h2>
             </button>
 
             {% if user %}
+            <div class="modal fade" id="editModal" tabindex="-1" role="dialog">
+              <form method="POST" action="{{ request.current_route_path() }}" id="editModalForm" class="{% if form.errors %}edit-form-errors{% endif %}">
+                <input name="csrf_token" type="hidden" value="{{ request.session.get_csrf_token() }}">
+                <div class="modal-dialog" role="document">
+                  <div class="modal-content">
+                    <div class="modal-header">
+                      <h4 class="modal-title" id="editModalLabel">
+                        Edit {{ organization_application.name }}?
+                      </h4>
+                      <button type="button" class="close" data-dismiss="modal" aria-label="Close">
+                        <span aria-hidden="true">&times;</span>
+                      </button>
+                    </div>
+                    <div class="modal-body">
+                      {{ render_field("Organization Display Name", form.display_name, "organization-display-name", class="form-control") }}
+                      {{ render_field("Organization Name", form.name, "organization-name", class="form-control") }}
+                      {{ render_field("Organization URL", form.link_url, "organization-link-url", class="form-control") }}
+                      {{ render_field("Organization Description", form.description, "organization-description", class="form-control") }}
+                      {{ render_field("Organization Type", form.orgtype, "organization-type", class="form-control") }}
+                    </div>
+                    <div class="modal-footer justify-content-between">
+                      <button type="button" class="btn btn-secondary" data-dismiss="modal">Close</button>
+                      <button type="submit" class="btn btn-primary">
+                        <i class="icon fa fa-edit"></i> Approve organization request
+                      </button>
+                    </div>
+                  </div>
+                </div>
+              </form>
+            </div>
+
             <div class="modal fade" id="approveModal" tabindex="-1" role="dialog">
               <form method="POST" action="{{ request.route_path('admin.organization_application.approve', organization_application_id=organization_application.id) }}">
                 <input name="csrf_token" type="hidden" value="{{ request.session.get_csrf_token() }}">
@@ -169,7 +231,7 @@ <h4 class="modal-title" id="deferModalLabel">
                 <div class="hidden" id="requestMoreInformationEmailTemplate">
                     Hello {{ organization_application.submitted_by.username }}!
                     We'd be happy to approve this organization request,
-                    but ask that you first add an @{% with parsed = organization_application.link_url|urlparse %}{{ parsed.host }}{% endwith %}
+                    but ask that you first add an @{% with parsed = organization_application.link_url|urlparse %}{% if parsed.host %}{{ parsed.host.lstrip("www.") }}{% else %}NONE{% endif %}{% endwith %}
                     email address to your PyPI account,
                     complete the verification,
                     and respond using the linked form.
@@ -277,10 +339,12 @@ <h4 class="modal-title" id="declineModalLabel">
       </div>
     </div>
 
+    {% set information_requests = organization_application.information_requests %}
+    {% set outstanding_information_requests = organization_application.information_requests|selectattr("additional", "defined")|map(attribute="additional")|selectattr("response", "undefined")|list %}
     <div class="col-md-9">
       <div class="card">
         <div class="card-header with-border">
-          <h3 class="card-title">Organization Request</h3>
+          <h3 class="card-title">Organization Request{% if information_requests %}{% if outstanding_information_requests %}  <i class="fa-solid fa-envelope text-info"></i>{% else %}  <i class="fa-solid fa-envelope-open text-green"></i><i class="fa-solid fa-reply text-green"></i>{% endif %}{% endif %}</h3>
         </div>
         <div class="card-body">
           <div class="form-horizontal">
@@ -374,7 +438,7 @@ <h3 class="card-title">Organization Request</h3>
           <h3 class="card-title">Information Requests</h3>
         </div>
         <div class="card-body">
-        {% for information_request in organization_application.information_requests %}
+        {% for information_request in information_requests %}
         <div class="card {% if information_request.additional.response %}card-neutral{% else %}card-warning{% endif %} card-outline direct-chat direct-chat-primary"</div>
           <div class="card-body">
             <div class="direct-chat-messages unset-height">

warehouse/admin/templates/admin/organization_applications/list.html

@@ -69,8 +69,11 @@
 
       <tbody>
       {% for organization_application in organization_applications %}
+      {% set information_requests = organization_application.information_requests %}
+      {% set outstanding_information_requests = organization_application.information_requests|selectattr("additional", "defined")|map(attribute="additional")|selectattr("response", "undefined")|list %}
       <tr>
         <td>
+          {% if information_requests %}{% if outstanding_information_requests %}  <i class="fa-solid fa-envelope text-info"></i>{% else %}  <i class="fa-solid fa-envelope-open text-green"></i><i class="fa-solid fa-reply text-green"></i>{% endif %}{% endif %}
           <a href="{{ request.route_path('admin.organization_application.detail', organization_application_id=organization_application.id) }}">{{ organization_application.name }}</a>
         </td>
         <td>

warehouse/admin/views/organizations.py

@@ -16,9 +16,11 @@
 from pyramid.httpexceptions import HTTPBadRequest, HTTPNotFound, HTTPSeeOther
 from pyramid.view import view_config
 from sqlalchemy import or_
+from sqlalchemy.orm import joinedload
 
 from warehouse.accounts.interfaces import IUserService
 from warehouse.authnz import Permissions
+from warehouse.manage.forms import OrganizationNameMixin, SaveOrganizationForm
 from warehouse.organizations.interfaces import IOrganizationService
 from warehouse.organizations.models import (
     Organization,
@@ -265,13 +267,24 @@ def organization_applications_list(request):
                     organization_applications_query.filter(filter_or_subfilters)
                 )
 
+    organization_applications_query = organization_applications_query.options(
+        joinedload(OrganizationApplication.observations)
+    )
+
     return {
         "organization_applications": organization_applications_query.all(),
         "query": q,
         "terms": terms,
     }
 
 
+class OrganizationApplicationForm(OrganizationNameMixin, SaveOrganizationForm):
+    def __init__(self, *args, organization_service, user, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.organization_service = organization_service
+        self.user = user
+
+
 @view_config(
     route_name="admin.organization_application.detail",
     require_methods=False,
@@ -292,6 +305,21 @@ def organization_application_detail(request):
     if organization_application is None:
         raise HTTPNotFound
 
+    form = OrganizationApplicationForm(
+        request.POST if request.method == "POST" else None,
+        organization_application,
+        organization_service=organization_service,
+        user=request.user,
+    )
+
+    if request.method == "POST" and form.validate():
+        form.populate_obj(organization_application)
+        request.session.flash(
+            f"Application for {organization_application.name!r} updated",
+            queue="success",
+        )
+        return HTTPSeeOther(location=request.current_route_path())
+
     conflicting_applications = (
         request.db.query(OrganizationApplication)
         .filter(
@@ -307,6 +335,7 @@ def organization_application_detail(request):
 
     return {
         "organization_application": organization_application,
+        "form": form,
         "conflicting_applications": conflicting_applications,
         "user": user,
     }