From d9d2f7be2b13c17b7de2189b142d1291dd08f721 Mon Sep 17 00:00:00 2001
From: Valentin Deniaud <vdeniaud@entrouvert.com>
Date: Tue, 20 Oct 2020 17:29:51 +0200
Subject: [PATCH] manager: move roles and booking delays to own form (#46774)

---
 chrono/manager/forms.py                       | 33 ++++++++++++++-----
 .../chrono/manager_agenda_add_form.html       | 18 ++++++++++
 .../templates/chrono/manager_agenda_form.html | 16 ++++-----
 .../chrono/manager_agenda_reminder_form.html  | 22 -------------
 .../chrono/manager_agenda_settings.html       |  2 ++
 chrono/manager/urls.py                        |  6 ++++
 chrono/manager/views.py                       | 31 ++++++++++++++---
 tests/test_manager.py                         | 33 +++++++++++++++++--
 8 files changed, 116 insertions(+), 45 deletions(-)
 create mode 100644 chrono/manager/templates/chrono/manager_agenda_add_form.html
 delete mode 100644 chrono/manager/templates/chrono/manager_agenda_reminder_form.html

diff --git a/chrono/manager/forms.py b/chrono/manager/forms.py
index 53cc2e4..8c66388 100644
--- a/chrono/manager/forms.py
+++ b/chrono/manager/forms.py
@@ -66,17 +66,13 @@ class AgendaAddForm(forms.ModelForm):
     )
 
 
-class AgendaEditForm(AgendaAddForm):
+class AgendaEditForm(forms.ModelForm):
     class Meta:
         model = Agenda
         fields = [
             'label',
             'slug',
             'category',
-            'edit_role',
-            'view_role',
-            'minimal_booking_delay',
-            'maximal_booking_delay',
             'anonymize_delay',
             'default_view',
             'booking_form_url',
@@ -84,14 +80,35 @@ class AgendaEditForm(AgendaAddForm):
 
     def __init__(self, *args, **kwargs):
         super(AgendaEditForm, self).__init__(*args, **kwargs)
-        if kwargs['instance'].kind != 'virtual':
-            self.fields['minimal_booking_delay'].required = True
-            self.fields['maximal_booking_delay'].required = True
         if kwargs['instance'].kind != 'events':
             del self.fields['default_view']
             del self.fields['booking_form_url']
 
 
+class AgendaBookingDelaysForm(AgendaAddForm):
+    class Meta:
+        model = Agenda
+        fields = [
+            'minimal_booking_delay',
+            'maximal_booking_delay',
+        ]
+
+    def __init__(self, *args, **kwargs):
+        super(AgendaBookingDelaysForm, self).__init__(*args, **kwargs)
+        if kwargs['instance'].kind != 'virtual':
+            self.fields['minimal_booking_delay'].required = True
+            self.fields['maximal_booking_delay'].required = True
+
+
+class AgendaRolesForm(AgendaAddForm):
+    class Meta:
+        model = Agenda
+        fields = [
+            'edit_role',
+            'view_role',
+        ]
+
+
 class UnavailabilityCalendarAddForm(forms.ModelForm):
     class Meta:
         model = UnavailabilityCalendar
diff --git a/chrono/manager/templates/chrono/manager_agenda_add_form.html b/chrono/manager/templates/chrono/manager_agenda_add_form.html
new file mode 100644
index 0000000..7a40949
--- /dev/null
+++ b/chrono/manager/templates/chrono/manager_agenda_add_form.html
@@ -0,0 +1,18 @@
+{% extends "chrono/manager_home.html" %}
+{% load i18n %}
+
+{% block appbar %}
+<h2>{% trans "New Agenda" %}</h2>
+{% endblock %}
+
+{% block content %}
+
+<form method="post" enctype="multipart/form-data">
+  {% csrf_token %}
+  {{ form.as_p }}
+  <div class="buttons">
+    <button class="submit-button">{% trans "Save" %}</button>
+    <a class="cancel" href="{% url 'chrono-manager-homepage' %}">{% trans 'Cancel' %}</a>
+  </div>
+</form>
+{% endblock %}
diff --git a/chrono/manager/templates/chrono/manager_agenda_form.html b/chrono/manager/templates/chrono/manager_agenda_form.html
index 9d71ca6..15830d7 100644
--- a/chrono/manager/templates/chrono/manager_agenda_form.html
+++ b/chrono/manager/templates/chrono/manager_agenda_form.html
@@ -1,22 +1,22 @@
-{% extends "chrono/manager_home.html" %}
+{% extends "chrono/manager_agenda_view.html" %}
 {% load i18n %}
 
+{% block breadcrumb %}
+{{ block.super }}
+<a href="">{{ title }}</a>
+{% endblock %}
+
 {% block appbar %}
-{% if object.id %}
-<h2>{% trans "Edit Agenda" %}</h2>
-{% else %}
-<h2>{% trans "New Agenda" %}</h2>
-{% endif %}
+<h2>{{ title }}</h2>
 {% endblock %}
 
 {% block content %}
-
 <form method="post" enctype="multipart/form-data">
   {% csrf_token %}
   {{ form.as_p }}
   <div class="buttons">
     <button class="submit-button">{% trans "Save" %}</button>
-    <a class="cancel" href="{% url 'chrono-manager-homepage' %}">{% trans 'Cancel' %}</a>
+    <a class="cancel" href="{% url 'chrono-manager-agenda-settings' pk=agenda.id %}">{% trans 'Cancel' %}</a>
   </div>
 </form>
 {% endblock %}
diff --git a/chrono/manager/templates/chrono/manager_agenda_reminder_form.html b/chrono/manager/templates/chrono/manager_agenda_reminder_form.html
deleted file mode 100644
index 4501fb2..0000000
--- a/chrono/manager/templates/chrono/manager_agenda_reminder_form.html
+++ /dev/null
@@ -1,22 +0,0 @@
-{% extends "chrono/manager_agenda_view.html" %}
-{% load i18n %}
-
-{% block breadcrumb %}
-{{ block.super }}
-<a href="">{% trans "Reminder settings" %}</a>
-{% endblock %}
-
-{% block appbar %}
-<h2>{% trans "Reminder settings" %}</h2>
-{% endblock %}
-
-{% block content %}
-<form method="post" enctype="multipart/form-data">
-  {% csrf_token %}
-  {{ form.as_p }}
-  <div class="buttons">
-    <button class="submit-button">{% trans "Save" %}</button>
-    <a class="cancel" href="{% url 'chrono-manager-agenda-settings' pk=agenda.id %}">{% trans 'Cancel' %}</a>
-  </div>
-</form>
-{% endblock %}
diff --git a/chrono/manager/templates/chrono/manager_agenda_settings.html b/chrono/manager/templates/chrono/manager_agenda_settings.html
index b396515..4789746 100644
--- a/chrono/manager/templates/chrono/manager_agenda_settings.html
+++ b/chrono/manager/templates/chrono/manager_agenda_settings.html
@@ -69,6 +69,7 @@
   <li>{% trans "Edit Role:" %} {% if agenda.edit_role %}{{ agenda.edit_role }}{% else %}<i>{% trans "undefined" %}</i>{% endif %}</li>
   <li>{% trans "View Role:" %} {% if agenda.view_role %}{{ agenda.view_role }}{% else %}<i>{% trans "undefined" %}</i>{% endif %}</li>
 </ul>
+<a rel="popup" class="button" href="{% url 'chrono-manager-agenda-roles' pk=object.id %}">{% trans 'Configure' %}</a>
 </div>
 </div>
 {% endblock %}
@@ -85,6 +86,7 @@
       {% if agenda.maximal_booking_delay is not None %}{{ agenda.maximal_booking_delay }} {% trans "days" %}
       {% else %}<i>{% trans "undefined" %}</i>{% endif %}</li>
 </ul>
+<a rel="popup" class="button" href="{% url 'chrono-manager-agenda-booking-delays' pk=object.id %}">{% trans 'Configure' %}</a>
 </div>
 </div>
 {% endblock %}
diff --git a/chrono/manager/urls.py b/chrono/manager/urls.py
index 45d57d4..bf7928a 100644
--- a/chrono/manager/urls.py
+++ b/chrono/manager/urls.py
@@ -99,6 +99,12 @@ urlpatterns = [
     ),
     url(r'^agendas/(?P<pk>\d+)/settings$', views.agenda_settings, name='chrono-manager-agenda-settings'),
     url(r'^agendas/(?P<pk>\d+)/edit$', views.agenda_edit, name='chrono-manager-agenda-edit'),
+    url(
+        r'^agendas/(?P<pk>\d+)/booking-delays$',
+        views.agenda_booking_delays,
+        name='chrono-manager-agenda-booking-delays',
+    ),
+    url(r'^agendas/(?P<pk>\d+)/roles$', views.agenda_roles, name='chrono-manager-agenda-roles'),
     url(r'^agendas/(?P<pk>\d+)/delete$', views.agenda_delete, name='chrono-manager-agenda-delete'),
     url(r'^agendas/(?P<pk>\d+)/export$', views.agenda_export, name='chrono-manager-agenda-export'),
     url(r'^agendas/(?P<pk>\d+)/add-event$', views.agenda_add_event, name='chrono-manager-agenda-add-event'),
diff --git a/chrono/manager/views.py b/chrono/manager/views.py
index 01d5207..9f580e6 100644
--- a/chrono/manager/views.py
+++ b/chrono/manager/views.py
@@ -72,6 +72,8 @@ from chrono.agendas.models import (
 from .forms import (
     AgendaAddForm,
     AgendaEditForm,
+    AgendaBookingDelaysForm,
+    AgendaRolesForm,
     NewEventForm,
     EventForm,
     NewMeetingTypeForm,
@@ -568,7 +570,7 @@ category_delete = CategoryDeleteView.as_view()
 
 
 class AgendaAddView(CreateView):
-    template_name = 'chrono/manager_agenda_form.html'
+    template_name = 'chrono/manager_agenda_add_form.html'
     model = Agenda
     form_class = AgendaAddForm
 
@@ -679,16 +681,35 @@ class ManagedAgendaMixin(ViewableAgendaMixin):
 
 class AgendaEditView(ManagedAgendaMixin, UpdateView):
     template_name = 'chrono/manager_agenda_form.html'
+    title = _('Edit Agenda')
     model = Agenda
     form_class = AgendaEditForm
 
-    def get_success_url(self):
-        return reverse('chrono-manager-agenda-settings', kwargs={'pk': self.object.id})
+    def get_context_data(self, **kwargs):
+        context = super().get_context_data(**kwargs)
+        context['title'] = self.title
+        return context
 
 
 agenda_edit = AgendaEditView.as_view()
 
 
+class AgendaBookingDelaysView(AgendaEditView):
+    form_class = AgendaBookingDelaysForm
+    title = _('Configure booking delays')
+
+
+agenda_booking_delays = AgendaBookingDelaysView.as_view()
+
+
+class AgendaRolesView(AgendaEditView):
+    form_class = AgendaRolesForm
+    title = _('Configure roles')
+
+
+agenda_roles = AgendaRolesView.as_view()
+
+
 class AgendaDeleteView(DeleteView):
     template_name = 'chrono/manager_confirm_delete.html'
     model = Agenda
@@ -1475,8 +1496,8 @@ class AgendaNotificationsSettingsView(ManagedAgendaMixin, UpdateView):
 agenda_notifications_settings = AgendaNotificationsSettingsView.as_view()
 
 
-class AgendaReminderSettingsView(ManagedAgendaMixin, UpdateView):
-    template_name = 'chrono/manager_agenda_reminder_form.html'
+class AgendaReminderSettingsView(AgendaEditView):
+    title = _('Reminder Settings')
     model = AgendaReminderSettings
     form_class = AgendaReminderForm
 
diff --git a/tests/test_manager.py b/tests/test_manager.py
index 49d905d..993d8be 100644
--- a/tests/test_manager.py
+++ b/tests/test_manager.py
@@ -915,7 +915,7 @@ def test_options_agenda_cant_unset_delays(app, admin_user):
     agenda = Agenda.objects.create(label=u'Foo bar')
     assert agenda.minimal_booking_delay == 1
     app = login(app)
-    url = '/manage/agendas/%s/edit' % agenda.pk
+    url = '/manage/agendas/%s/booking-delay' % agenda.pk
     resp = app.get(url)
     resp.form['minimal_booking_delay'] = None
     resp = resp.form.submit()
@@ -927,7 +927,7 @@ def test_options_virtuale_agenda_can_unset_delays(app, admin_user):
     agenda = Agenda.objects.create(label=u'Foo bar', kind='virtual', maximal_booking_delay=2)
     assert agenda.maximal_booking_delay == 2
     app = login(app)
-    url = '/manage/agendas/%s/edit' % agenda.pk
+    url = '/manage/agendas/%s/booking-delays' % agenda.pk
     resp = app.get(url)
     resp.form['maximal_booking_delay'] = None
     resp = resp.form.submit()
@@ -4866,3 +4866,32 @@ def test_unavailability_calendar_delete_unavailability_permissions(app, manager_
     unavailability_calendar.edit_role = group
     unavailability_calendar.save()
     app.get(url)
+
+
+def test_manager_agenda_roles(app, admin_user, manager_user):
+    agenda = Agenda.objects.create(label='Events', kind='events')
+
+    login(app)
+    resp = app.get('/manage/agendas/%s/settings' % agenda.id)
+    resp = resp.click('Configure', href='roles')
+
+    role = manager_user.groups.all()[0]
+    resp.form['edit_role'] = manager_user.groups.all()[0].pk
+    resp = resp.form.submit().follow()
+
+    assert 'Edit Role: Managers' in resp.text
+
+
+def test_manager_agenda_booking_delays(app, admin_user):
+    agenda = Agenda.objects.create(label='Events', kind='events')
+
+    login(app)
+    resp = app.get('/manage/agendas/%s/settings' % agenda.id)
+    resp = resp.click('Configure', href='delays')
+
+    resp.form['maximal_booking_delay'] = 42
+    resp = resp.form.submit().follow()
+
+    assert '42 days' in resp.text
+    agenda.refresh_from_db()
+    assert agenda.maximal_booking_delay == 42
-- 
2.20.1