From 2dadef5bf39726e1329a7b8283d82fa17ce679af Mon Sep 17 00:00:00 2001
From: Emmanuel Cazenave <ecazenave@entrouvert.com>
Date: Wed, 17 Jun 2020 17:39:26 +0200
Subject: [PATCH] agendas: mark MeetingType for deletion (#44132)

---
 .../0048_meeting_type_deleted_flag.py         | 20 +++++++
 chrono/agendas/models.py                      |  9 +--
 chrono/api/views.py                           |  2 +-
 chrono/manager/forms.py                       |  4 +-
 .../manager_meetings_agenda_settings.html     |  6 +-
 chrono/manager/views.py                       | 53 +++++++++--------
 tests/test_api.py                             | 33 ++++++++++-
 tests/test_manager.py                         | 57 +++++++------------
 8 files changed, 108 insertions(+), 76 deletions(-)
 create mode 100644 chrono/agendas/migrations/0048_meeting_type_deleted_flag.py

diff --git a/chrono/agendas/migrations/0048_meeting_type_deleted_flag.py b/chrono/agendas/migrations/0048_meeting_type_deleted_flag.py
new file mode 100644
index 0000000..6d0e8f0
--- /dev/null
+++ b/chrono/agendas/migrations/0048_meeting_type_deleted_flag.py
@@ -0,0 +1,20 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.18 on 2020-06-17 13:23
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('agendas', '0047_auto_20200617_1521'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='meetingtype',
+            name='deleted',
+            field=models.BooleanField(default=False, verbose_name='Deleted'),
+        ),
+    ]
diff --git a/chrono/agendas/models.py b/chrono/agendas/models.py
index 4ef6d39..6beca09 100644
--- a/chrono/agendas/models.py
+++ b/chrono/agendas/models.py
@@ -186,7 +186,7 @@ class Agenda(models.Model):
         the real ones shared by every real agendas.
         """
         if self.kind == 'virtual':
-            base_qs = MeetingType.objects.filter(agenda__virtual_agendas__in=[self])
+            base_qs = MeetingType.objects.filter(agenda__virtual_agendas__in=[self], deleted=False)
             real_agendas = self.real_agendas
             if excluded_agenda:
                 base_qs = base_qs.exclude(agenda=excluded_agenda)
@@ -201,7 +201,7 @@ class Agenda(models.Model):
                 for mt in queryset.order_by('slug')
             ]
 
-        return self.meetingtype_set.all().order_by('slug')
+        return self.meetingtype_set.filter(deleted=False).all().order_by('slug')
 
     def get_meetingtype(self, id_=None, slug=None):
         match = id_ or slug
@@ -219,8 +219,8 @@ class Agenda(models.Model):
             return meeting_type
 
         if id_:
-            return MeetingType.objects.get(id=id_, agenda=self)
-        return MeetingType.objects.get(slug=slug, agenda=self)
+            return MeetingType.objects.get(id=id_, agenda=self, deleted=False)
+        return MeetingType.objects.get(slug=slug, agenda=self, deleted=False)
 
     def get_virtual_members(self):
         return VirtualMember.objects.filter(virtual_agenda=self)
@@ -679,6 +679,7 @@ class MeetingType(models.Model):
     label = models.CharField(_('Label'), max_length=150)
     slug = models.SlugField(_('Identifier'), max_length=160)
     duration = models.IntegerField(_('Duration (in minutes)'), default=30)
+    deleted = models.BooleanField(_('Deleted'), default=False)
 
     class Meta:
         ordering = ['duration', 'label']
diff --git a/chrono/api/views.py b/chrono/api/views.py
index 13b5662..a7e136b 100644
--- a/chrono/api/views.py
+++ b/chrono/api/views.py
@@ -451,7 +451,7 @@ class MeetingDatetimes(APIView):
         try:
             if agenda_identifier is None:
                 # legacy access by meeting id
-                meeting_type = MeetingType.objects.get(id=meeting_identifier)
+                meeting_type = MeetingType.objects.get(id=meeting_identifier, deleted=False)
                 agenda = meeting_type.agenda
             else:
                 agenda = Agenda.objects.get(slug=agenda_identifier)
diff --git a/chrono/manager/forms.py b/chrono/manager/forms.py
index 95c6ab0..60c529a 100644
--- a/chrono/manager/forms.py
+++ b/chrono/manager/forms.py
@@ -117,7 +117,7 @@ class NewMeetingTypeForm(forms.ModelForm):
         widgets = {
             'agenda': forms.HiddenInput(),
         }
-        exclude = ['slug']
+        exclude = ['slug', 'deleted']
 
     def clean(self):
         super().clean()
@@ -136,7 +136,7 @@ class MeetingTypeForm(forms.ModelForm):
         widgets = {
             'agenda': forms.HiddenInput(),
         }
-        exclude = []
+        exclude = ['deleted']
 
     def clean(self):
         super().clean()
diff --git a/chrono/manager/templates/chrono/manager_meetings_agenda_settings.html b/chrono/manager/templates/chrono/manager_meetings_agenda_settings.html
index 9f6542d..662d274 100644
--- a/chrono/manager/templates/chrono/manager_meetings_agenda_settings.html
+++ b/chrono/manager/templates/chrono/manager_meetings_agenda_settings.html
@@ -23,10 +23,9 @@
 <div class="section">
 <h3>{% trans 'Meeting Types' %}</h3>
 <div>
-{% with object.meetingtype_set.all as meetingtypes %}
-{% if meetingtypes %}
+{% if meeting_types %}
   <ul class="objects-list single-links">
-    {% for meeting_type in meetingtypes %}
+    {% for meeting_type in meeting_types %}
     <li><a rel="popup" href="{% url 'chrono-manager-meeting-type-edit' pk=meeting_type.id %}">
         {{meeting_type.label}}
         <span class="duration">({{meeting_type.duration}} {% trans "minutes" %})</span>
@@ -44,7 +43,6 @@
   {% endblocktrans %}
 </div>
 {% endif %}
-{% endwith %}
 </div>
 </div>
 
diff --git a/chrono/manager/views.py b/chrono/manager/views.py
index d0f304c..15cea5a 100644
--- a/chrono/manager/views.py
+++ b/chrono/manager/views.py
@@ -1110,12 +1110,13 @@ class AgendaSettings(ManagedAgendaMixin, DetailView):
 
     def get_context_data(self, **kwargs):
         context = super(AgendaSettings, self).get_context_data(**kwargs)
+        if self.agenda.accept_meetings():
+            context['meeting_types'] = self.object.iter_meetingtypes()
         if self.agenda.kind == 'virtual':
             context['virtual_members'] = [
                 (virtual_member, virtual_member.real_agenda.can_be_managed(self.request.user))
                 for virtual_member in self.object.get_virtual_members()
             ]
-            context['meeting_types'] = self.object.iter_meetingtypes()
         if self.agenda.kind == 'meetings':
             context['has_resources'] = Resource.objects.exists()
         return context
@@ -1340,32 +1341,24 @@ class MeetingTypeDeleteView(ManagedAgendaSubobjectMixin, DeleteView):
     def get_context_data(self, **kwargs):
         context = super(MeetingTypeDeleteView, self).get_context_data(**kwargs)
         meeting_type = self.get_object()
+        context['cannot_delete'] = False
+
+        for virtual_agenda in self.get_object().agenda.virtual_agendas.all():
+            if virtual_agenda.real_agendas.count() == 1:
+                continue
+            for mt in virtual_agenda.iter_meetingtypes():
+                if (
+                    meeting_type.slug == mt.slug
+                    and meeting_type.label == mt.label
+                    and meeting_type.duration == mt.duration
+                ):
+                    context['cannot_delete'] = True
+                    context['cannot_delete_msg'] = _(
+                        'This cannot be removed as it used by a virtual agenda: %(agenda)s'
+                        % {'agenda': virtual_agenda}
+                    )
+                    break
 
-        cannot_delete = Booking.objects.filter(
-            event__meeting_type=meeting_type,
-            event__start_datetime__gt=now(),
-            cancellation_datetime__isnull=True,
-        ).exists()
-        if cannot_delete:
-            context['cannot_delete_msg'] = FUTURE_BOOKING_ERROR_MSG
-        else:
-            for virtual_agenda in self.get_object().agenda.virtual_agendas.all():
-                if virtual_agenda.real_agendas.count() == 1:
-                    continue
-                for mt in virtual_agenda.iter_meetingtypes():
-                    if (
-                        meeting_type.slug == mt.slug
-                        and meeting_type.label == mt.label
-                        and meeting_type.duration == mt.duration
-                    ):
-                        cannot_delete = True
-                        context['cannot_delete_msg'] = _(
-                            'This cannot be removed as it used by a virtual agenda: %(agenda)s'
-                            % {'agenda': virtual_agenda}
-                        )
-                        break
-
-        context['cannot_delete'] = cannot_delete
         return context
 
     def delete(self, request, *args, **kwargs):
@@ -1373,7 +1366,13 @@ class MeetingTypeDeleteView(ManagedAgendaSubobjectMixin, DeleteView):
         context = self.get_context_data()
         if context['cannot_delete']:
             raise PermissionDenied()
-        return super(MeetingTypeDeleteView, self).delete(request, *args, **kwargs)
+
+        # rewrite django/views/generic/edit.py::DeletionMixin.delete
+        # to mark for deletion instead of actually delete
+        success_url = self.get_success_url()
+        self.object.deleted = True
+        self.object.save()
+        return HttpResponseRedirect(success_url)
 
 
 meeting_type_delete = MeetingTypeDeleteView.as_view()
diff --git a/tests/test_api.py b/tests/test_api.py
index f22a767..e4757ac 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -231,7 +231,7 @@ def test_agendas_api(app):
 
 def test_agendas_meetingtypes_api(app, some_data, meetings_agenda):
     resp = app.get('/api/agenda/%s/meetings/' % meetings_agenda.slug)
-    assert resp.json == {
+    expected_resp = {
         'data': [
             {
                 'text': 'Blah',
@@ -243,6 +243,12 @@ def test_agendas_meetingtypes_api(app, some_data, meetings_agenda):
             }
         ]
     }
+    assert resp.json == expected_resp
+
+    # deleted meeting type does not show up
+    MeetingType.objects.create(agenda=meetings_agenda, slug='deleted-meeting-type', duration=43, deleted=True)
+    resp = app.get('/api/agenda/%s/meetings/' % meetings_agenda.slug)
+    assert resp.json == expected_resp
 
     # wrong kind
     agenda1 = Agenda.objects.filter(label=u'Foo bar')[0]
@@ -2950,6 +2956,31 @@ def test_agenda_meeting_gcd_durations_and_exceptions(app, meetings_agenda, user)
     assert len(resp.json['data']) == 0
 
 
+def test_agenda_meeting_deleted_meetingtype(app, meetings_agenda, user):
+    MeetingType.objects.all().delete()
+    meeting_type = MeetingType.objects.create(
+        agenda=meetings_agenda, label='Blah 20', duration=20, deleted=True
+    )
+    resp = app.get(
+        '/api/agenda/%s/meetings/%s/datetimes/' % (meetings_agenda.slug, meeting_type.slug), status=404
+    )
+
+    meeting_type.deleted = False
+    meeting_type.save()
+    resp = app.get('/api/agenda/%s/meetings/%s/datetimes/' % (meetings_agenda.slug, meeting_type.slug))
+    data = resp.json['data']
+    assert len(data) == 216
+
+    # try to book if disabled
+    meeting_type.deleted = True
+    meeting_type.save()
+
+    fillslot_url = data[0]['api']['fillslot_url']
+    app.authorization = ('Basic', ('john.doe', 'password'))
+    resp_booking = app.post(fillslot_url, status=400)
+    assert 'invalid meeting type id' in resp_booking.json['err_desc']
+
+
 def test_datetimes_api_meetings_agenda_start_hour_change(app, meetings_agenda):
     meeting_type = MeetingType.objects.get(agenda=meetings_agenda)
     api_url = '/api/agenda/%s/meetings/%s/datetimes/' % (meeting_type.agenda.slug, meeting_type.slug)
diff --git a/tests/test_manager.py b/tests/test_manager.py
index 6647c1f..6e3d1ce 100644
--- a/tests/test_manager.py
+++ b/tests/test_manager.py
@@ -850,38 +850,6 @@ def test_delete_busy_agenda(app, admin_user):
     resp = resp.form.submit(status=403)
 
 
-def test_delete_busy_meeting_type(app, admin_user):
-    agenda = Agenda.objects.create(label='Foo', kind='meetings')
-    meeting_type = MeetingType.objects.create(agenda=agenda, label='Meeting Type Foo', duration=30)
-    desk = Desk.objects.create(agenda=agenda, label='Desk', slug='desk')
-
-    app = login(app)
-    resp = app.get('/manage/', status=200)
-    resp = resp.click('Foo').follow()
-    resp = resp.click('Settings')
-    mt_page = resp.click('Meeting Type Foo')
-    mt_delete_page = mt_page.click('Delete')
-    assert 'Are you sure you want to delete this?' in mt_delete_page.text
-    # make sure the deleting is not disabled
-    assert 'disabled' not in mt_delete_page.text
-
-    event = Event(
-        start_datetime=now() + datetime.timedelta(days=10),
-        meeting_type=meeting_type,
-        desk=desk,
-        agenda=agenda,
-        full=False,
-        places=1,
-    )
-    event.save()
-    booking = Booking(event=event)
-    booking.save()
-    assert Booking.objects.count() == 1
-
-    resp = mt_page.click('Delete')
-    assert 'This cannot be removed' in resp.text
-
-
 def test_delete_agenda_as_manager(app, manager_user):
     agenda = Agenda(label=u'Foo bar')
     agenda.edit_role = manager_user.groups.all()[0]
@@ -1416,17 +1384,22 @@ def test_meetings_agenda_add_meeting_type(app, admin_user):
     resp = resp.click('New Meeting Type')
     resp.form['label'] = 'Blah'
     resp.form['duration'] = '60'
+    assert 'deleted' not in resp.form.fields
     resp = resp.form.submit()
-    assert MeetingType.objects.get(agenda=agenda).label == 'Blah'
-    assert MeetingType.objects.get(agenda=agenda).duration == 60
+    meeeting_type = MeetingType.objects.get(agenda=agenda)
+    assert meeeting_type.label == 'Blah'
+    assert meeeting_type.duration == 60
+    assert meeeting_type.deleted is False
     resp = resp.follow()
     assert 'Blah' in resp.text
 
     # and edit
     resp = resp.click('Blah')
     resp.form['duration'] = '30'
+    assert 'deleted' not in resp.form.fields
     resp = resp.form.submit()
-    assert MeetingType.objects.get(agenda=agenda).duration == 30
+    meeeting_type = MeetingType.objects.get(agenda=agenda)
+    assert meeeting_type.duration == 30
 
 
 def test_meetings_agenda_delete_meeting_type(app, admin_user):
@@ -1441,9 +1414,18 @@ def test_meetings_agenda_delete_meeting_type(app, admin_user):
     resp = resp.click('Settings')
     resp = resp.click('Blah')
     resp = resp.click('Delete')
+    assert 'Are you sure you want to delete this?' in resp.text
+    assert 'disabled' not in resp.text
     resp = resp.form.submit()
     assert resp.location.endswith('/manage/agendas/%s/settings' % agenda.id)
-    assert MeetingType.objects.count() == 0
+    meeting_type.refresh_from_db()
+    assert meeting_type.deleted is True
+
+    # meeting type not showing up anymore
+    resp = app.get('/manage/', status=200)
+    resp = resp.click('Foo').follow()
+    resp = resp.click('Settings')
+    assert 'Meeting Type Foo' not in resp.text
 
 
 def test_meetings_agenda_add_time_period(app, admin_user):
@@ -3224,7 +3206,8 @@ def test_cant_delete_meetingtype_used_by_virtual_agenda(app, admin_user):
     resp = resp.click('Delete')
     resp = resp.form.submit()
     assert not meeting_agenda_1.iter_meetingtypes()
-    MeetingType.objects.create(agenda=meeting_agenda_1, label='MT', slug='mt', duration=10)
+    mt1.deleted = False
+    mt1.save()
 
     meeting_agenda_2 = Agenda.objects.create(label='Meeting agenda 2', kind='meetings')
     mt2 = MeetingType.objects.create(agenda=meeting_agenda_2, label='MT', slug='mt', duration=10)
-- 
2.20.1

