From 6118bc30e5559de2a9e2a6d1be8f1caf00811b71 Mon Sep 17 00:00:00 2001
From: Valentin Deniaud <vdeniaud@entrouvert.com>
Date: Thu, 10 Dec 2020 11:28:10 +0100
Subject: [PATCH] manager: do without disabled add user button on users page
 (#45338)

---
 src/authentic2/manager/forms.py               |  5 ++--
 .../templates/authentic2/manager/users.html   | 12 ++------
 src/authentic2/manager/urls.py                |  2 ++
 src/authentic2/manager/user_views.py          | 29 ++++++++++++++++++-
 tests/test_user_manager.py                    | 15 ++++++++++
 5 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/src/authentic2/manager/forms.py b/src/authentic2/manager/forms.py
index 34362eb2..28f082eb 100644
--- a/src/authentic2/manager/forms.py
+++ b/src/authentic2/manager/forms.py
@@ -420,7 +420,7 @@ class OUSearchForm(FormWithRequest):
         # - show all if show_all_ou is True and user has ou_permission over all OUs or more than
         #   one,
         # - show searchable OUs
-        # - show none if user has ou_permission over all OUs
+        # - show none if user has ou_permission over all OUs and show_none_ou is True
         # - when no choice is made,
         #   - show all ou is show_all_ou is True (including None if user has ou_permission over all
         #   OUs)
@@ -431,6 +431,7 @@ class OUSearchForm(FormWithRequest):
         all_ou_label = kwargs.pop('all_ou_label', pgettext('organizational unit', 'All'))
         self.queryset = kwargs.pop('queryset', None)
         self.show_all_ou = kwargs.pop('show_all_ou', True)
+        self.show_none_ou = kwargs.pop('show_none_ou', True)
         request = kwargs['request']
         self.ou_count = utils.get_ou_count()
 
@@ -462,7 +463,7 @@ class OUSearchForm(FormWithRequest):
                 choices.append(('all', all_ou_label))
             for ou in self.ou_qs:
                 choices.append((str(ou.pk), six.text_type(ou)))
-            if self.search_all_ous:
+            if self.show_none_ou and self.search_all_ous:
                 choices.append(('none', pgettext('organizational unit', 'None')))
 
             # if user does not have ou_permission over all OUs, select user OU as default selected
diff --git a/src/authentic2/manager/templates/authentic2/manager/users.html b/src/authentic2/manager/templates/authentic2/manager/users.html
index 7d4ff5be..a648a6b7 100644
--- a/src/authentic2/manager/templates/authentic2/manager/users.html
+++ b/src/authentic2/manager/templates/authentic2/manager/users.html
@@ -7,20 +7,12 @@
   {{ block.super }}
   <span class="actions">
     <a class="extra-actions-menu-opener"></a>
-    {% if add_ou %}
      <a
-        href="{% url "a2-manager-user-add" ou_pk=add_ou.pk %}"
+        {% if add_ou %}href="{% url "a2-manager-user-add" ou_pk=add_ou.pk %}"{% else %}
+        href="{% url "a2-manager-user-add-choose-ou" %}" rel="popup"{% endif %}
         id="add-user-btn">
          {% trans "Add user" %}
      </a>
-   {% else %}
-     <a
-        href="#"
-        class="disabled"
-        id="add-user-btn">
-         {% trans "Add user" %}
-     </a>
-   {% endif %}
    {% if extra_actions %}
      <ul class="extra-actions-menu">
        {% for extra_action in extra_actions %}
diff --git a/src/authentic2/manager/urls.py b/src/authentic2/manager/urls.py
index f6d4eb33..244f242d 100644
--- a/src/authentic2/manager/urls.py
+++ b/src/authentic2/manager/urls.py
@@ -40,6 +40,8 @@ urlpatterns = required(
             user_views.users_export, name='a2-manager-users-export'),
         url(r'^users/add/$', user_views.user_add_default_ou,
             name='a2-manager-user-add-default-ou'),
+        url(r'^users/add/choose-ou/$', user_views.user_add_choose_ou,
+            name='a2-manager-user-add-choose-ou'),
         url(r'^users/import/$',
             user_views.user_imports, name='a2-manager-users-imports'),
         url(r'^users/import/(?P<uuid>[a-z0-9]+)/download/(?P<filename>.*)$',
diff --git a/src/authentic2/manager/user_views.py b/src/authentic2/manager/user_views.py
index a6826d92..052609b6 100644
--- a/src/authentic2/manager/user_views.py
+++ b/src/authentic2/manager/user_views.py
@@ -55,7 +55,7 @@ from .tables import UserTable, UserRolesTable, OuUserRolesTable, UserAuthorizati
 from .forms import (UserSearchForm, UserAddForm, UserEditForm,
                     UserChangePasswordForm, ChooseUserRoleForm,
                     UserRoleSearchForm, UserChangeEmailForm, UserNewImportForm,
-                    UserEditImportForm, ChooseUserAuthorizationsForm)
+                    UserEditImportForm, ChooseUserAuthorizationsForm, OUSearchForm)
 from .resources import UserResource
 from .utils import get_ou_count, has_show_username
 from .journal_views import BaseJournalView
@@ -240,6 +240,33 @@ def user_add_default_ou(request):
     return redirect(request, 'a2-manager-user-add', kwargs={'ou_pk': ou.id}, keep_params=True)
 
 
+class UserAddChooseOU(TitleMixin, FormNeedsRequest, FormView):
+    template_name = 'authentic2/manager/form.html'
+    title = _('Choose organizational unit in which to create user')
+    form_class = OUSearchForm
+
+    def get_success_url(self):
+        return reverse('a2-manager-user-add', kwargs={'ou_pk': self.ou_pk})
+
+    def form_valid(self, form):
+        self.ou_pk = form.cleaned_data['ou'].pk
+        return super().form_valid(form)
+
+    def get_form_kwargs(self):
+        kwargs = super().get_form_kwargs()
+        kwargs['show_all_ou'] = False
+        kwargs['show_none_ou'] = False
+        return kwargs
+
+    def get_context_data(self, **kwargs):
+        context = super().get_context_data(**kwargs)
+        context['action'] = _('Validate')
+        return context
+
+
+user_add_choose_ou = UserAddChooseOU.as_view()
+
+
 class UserDetailView(OtherActionsMixin, BaseDetailView):
     model = get_user_model()
     fields = ['username', 'ou', 'first_name', 'last_name', 'email']
diff --git a/tests/test_user_manager.py b/tests/test_user_manager.py
index dcecc408..0e3d7d70 100644
--- a/tests/test_user_manager.py
+++ b/tests/test_user_manager.py
@@ -204,6 +204,21 @@ def test_create_user_no_password(app, superuser):
     assert user.has_usable_password()
 
 
+def test_create_user_choose_ou(app, superuser, ou1, ou2):
+    response = login(app, superuser, '/manage/users/')
+    response = response.click('Add user')
+    assert 'Choose organizational unit' in response.text
+
+    response = response.form.submit()
+    assert str(get_default_ou().pk) in response.url
+
+    response = app.get('/manage/users/')
+    response = response.click('Add user')
+    response.form['ou'] = ou1.pk
+    response = response.form.submit()
+    assert str(ou1.pk) in response.url
+
+
 def test_manager_user_change_email(app, superuser_or_admin, simple_user, mailoutbox):
     ou = get_default_ou()
     ou.validate_emails = True
-- 
2.20.1