Waiting list: Add edit view for entry (Z#23215496) (#5712)

* add edit view for waitinglist entry

* add test and fix behaviour when name isn't asked for

* fix linting

* add testcases for new edit view

* fix test

* fix linting

* add search to the waitinglist view

* repair settings check

Co-authored-by: Richard Schreiber <schreiber@pretix.eu>

* make name and phone field optional by removing them

* remove item and variation fields from form

rather set those values during clean

* change label from "Item and Variation" to "Product"

* include only products with an enabled waitinglist in the product field

* combine edit.html and transfer.html

* change transfer to edit

* add tests

* code style

* Update src/pretix/control/forms/waitinglist.py

Co-authored-by: Richard Schreiber <schreiber@pretix.eu>

* Update src/pretix/control/forms/waitinglist.py

Co-authored-by: Richard Schreiber <schreiber@pretix.eu>

* Update src/pretix/control/urls.py

Co-authored-by: Richard Schreiber <schreiber@pretix.eu>

* Update src/pretix/control/templates/pretixcontrol/waitinglist/edit.html

Co-authored-by: Richard Schreiber <schreiber@pretix.eu>

* Update src/pretix/control/templates/pretixcontrol/waitinglist/index.html

Co-authored-by: Richard Schreiber <schreiber@pretix.eu>

* Update src/pretix/control/views/waitinglist.py

Co-authored-by: Richard Schreiber <schreiber@pretix.eu>

* Update src/pretix/control/views/waitinglist.py

Co-authored-by: Richard Schreiber <schreiber@pretix.eu>

* Update src/pretix/control/views/waitinglist.py

Co-authored-by: Richard Schreiber <schreiber@pretix.eu>

* remove validations

* remove validations

* replace widget

* implement small review items

* add better assertions

* add test for the different edit form variations

* add queryset to prefetch only active ItemVariations

* add queryset to prefetch only active ItemVariations

* propper use of WrappedPhoneNumberPrefixWidget

* cleanup

* add validation tests

* small review changes

* handle products with only inactive variations

* styling

---------

Co-authored-by: Richard Schreiber <schreiber@pretix.eu>

src/pretix/control/forms/waitinglist.py

@@ -19,17 +19,44 @@
 # You should have received a copy of the GNU Affero General Public License along with this program.  If not, see
 # <https://www.gnu.org/licenses/>.
 #
+from django import forms
 from django.urls import reverse
+from django.utils.translation import gettext_lazy as _
 from django_scopes.forms import SafeModelChoiceField
+from phonenumber_field.formfields import PhoneNumberField
 
 from pretix.base.forms import I18nModelForm
+from pretix.base.forms.questions import (
+    NamePartsFormField, WrappedPhoneNumberPrefixWidget,
+)
 from pretix.base.models import WaitingListEntry
 from pretix.control.forms.widgets import Select2
 
 
-class WaitingListEntryTransferForm(I18nModelForm):
+class WaitingListEntryEditForm(I18nModelForm):
+    itemvar = forms.ChoiceField(
+        error_messages={
+            'invalid_choice': _("Select a valid choice.")
+        }
+    )
 
     def __init__(self, *args, **kwargs):
+        self.instance = kwargs.get('instance', None)
+        initial = kwargs.get('initial', {})
+
+        choices = []
+        if self.instance and self.instance.pk and 'itemvar' not in initial:
+            if self.instance.variation is not None:
+                initial['itemvar'] = f'{self.instance.item.pk}-{self.instance.variation.pk}'
+                if self.instance.variation.active is False:
+                    choices.append((initial['itemvar'], str(self.instance.variation)))
+            else:
+                initial['itemvar'] = self.instance.item.pk
+                if self.instance.item.active is False:
+                    choices.append((initial['itemvar'], str(self.instance)))
+
+        kwargs['initial'] = initial
+
         super().__init__(*args, **kwargs)
 
         if self.event.has_subevents:
@@ -45,12 +72,73 @@ class WaitingListEntryTransferForm(I18nModelForm):
                 }
             )
             self.fields['subevent'].widget.choices = self.fields['subevent'].choices
+        else:
+            del self.fields['subevent']
+
+        if self.event.settings.waiting_list_names_asked:
+            self.fields['name_parts'] = NamePartsFormField(
+                max_length=255,
+                required=self.event.settings.waiting_list_names_required,
+                scheme=self.event.organizer.settings.name_scheme,
+                titles=self.event.organizer.settings.name_scheme_titles,
+                label=_('Name'),
+            )
+        else:
+            del self.fields['name_parts']
+
+        if not self.event.settings.waiting_list_phones_asked:
+            del self.fields['phone']
+
+        items = self.event.items.filter(active=True).prefetch_related(
+            'variations'
+        )
+
+        for item in items:
+            if len(item.variations.all()) > 0:
+                for variation in item.variations.all():
+                    if variation.active:
+                        choices.append(
+                            ('{}-{}'.format(item.pk, variation.pk), '{} - {}'.format(str(item), str(variation)))
+                        )
+            else:
+                choices.append(('{}'.format(item.pk), str(item)))
+
+        self.fields['itemvar'].label = _("Product")
+        self.fields['itemvar'].help_text = _("Only includes active products.")
+        self.fields['itemvar'].required = True
+        self.fields['itemvar'].choices = choices
+
+    def clean(self):
+        cleaned_data = super().clean()
+
+        if self.instance.voucher is not None:
+            raise forms.ValidationError(_('A voucher for this waiting list entry was already sent out.'))
+
+        itemvar = cleaned_data.get('itemvar')
+        if itemvar:
+            self.instance.item = self.event.items.get(pk=itemvar.split('-')[0])
+            if '-' in itemvar:
+                self.instance.variation = self.instance.item.variations.get(pk=itemvar.split('-')[1])
+
+        if ((self.instance.item and not self.instance.item.active) or
+                (self.instance.variation and not self.instance.variation.active)):
+            self.add_error('itemvar', _('The selected product is not active.'))
+
+        return cleaned_data
 
     class Meta:
         model = WaitingListEntry
         fields = [
+            'email',
+            'name_parts',
+            'phone',
             'subevent',
         ]
         field_classes = {
             'subevent': SafeModelChoiceField,
+            'email': forms.EmailField,
+            'phone': PhoneNumberField,
+        }
+        widgets = {
+            'phone': WrappedPhoneNumberPrefixWidget,
         }

src/pretix/control/templates/pretixcontrol/waitinglist/edit.html

@@ -0,0 +1,33 @@
+{% extends "pretixcontrol/event/base.html" %}
+{% load i18n %}
+{% load bootstrap3 %}
+{% block title %}{% trans "Edit entry" %}{% endblock %}
+{% block content %}
+    <h1>{% trans "Edit entry" %}</h1>
+    <form action="" method="post" class="form-horizontal">
+        {% csrf_token %}
+
+        {% if form.subevent %}
+            {% bootstrap_field form.subevent layout="control" %}
+        {% endif %}
+
+        {% bootstrap_field form.email layout="control" %}
+
+        {% if form.name_parts %}
+            {% bootstrap_field form.name_parts layout="control" %}
+        {% endif %}
+
+        {% if form.phone %}
+            {% bootstrap_field form.phone layout="control" %}
+        {% endif %}
+        {% bootstrap_field form.itemvar layout="control" %}
+        <div class="form-group submit-group">
+            <a href="{% url "control:event.orders.waitinglist" organizer=request.event.organizer.slug event=request.event.slug %}" class="btn btn-default btn-cancel">
+                {% trans "Cancel" %}
+            </a>
+            <button type="submit" class="btn btn-primary btn-save">
+                {% trans "Save" %}
+            </button>
+        </div>
+    </form>
+{% endblock %}

src/pretix/control/templates/pretixcontrol/waitinglist/index.html

@@ -124,6 +124,7 @@
                 </option>
             {% endfor %}
         </select>
+        <input name="search" type="text" placeholder="{% trans "Search" %}" class="form-control" value="{{ request.GET.search }}">
         {% if request.event.has_subevents %}
             <select name="subevent" class="form-control">
                 <option value="">{% trans "All dates" context "subevent" %}</option>
@@ -267,13 +268,13 @@
                                     data-toggle="tooltip" title="{% trans "Move to the end of the list" %}">
                                     <span class="fa fa-thumbs-down"></span>
                                 </button>
-                                {% if request.event.has_subevents %}
+
-                                    <a href="{% url "control:event.orders.waitinglist.transfer" organizer=request.event.organizer.slug event=request.event.slug entry=e.id %}"
+                                <a href="{% url "control:event.orders.waitinglist.edit" organizer=request.event.organizer.slug event=request.event.slug entry=e.id %}"
-                                       class="btn btn-default btn-sm" title="{% trans "Transfer to other date" context "subevent" %}"
+                                   class="btn btn-default btn-sm" title="{% trans "Edit entry" %}"
-                                        data-toggle="tooltip">
+                                    data-toggle="tooltip">
-                                        <i class="fa fa-calendar" aria-hidden="true"></i>
+                                    <i class="fa fa-edit" aria-hidden="true"></i>
-                                    </a>
+                                </a>
-                                {% endif %}
+
                                 <a href="{% url "control:event.orders.waitinglist.delete" organizer=request.event.organizer.slug event=request.event.slug entry=e.id %}?next={{ request.get_full_path|urlencode }}" class="btn btn-danger btn-sm"><i class="fa fa-trash"></i></a>
                             {% else %}
                                 <button class="btn btn-default btn-sm disabled">

src/pretix/control/templates/pretixcontrol/waitinglist/transfer.html

@@ -1,23 +0,0 @@
-{% extends "pretixcontrol/event/base.html" %}
-{% load i18n %}
-{% load bootstrap3 %}
-{% block title %}{% trans "Transfer entry" %}{% endblock %}
-{% block content %}
-    <h1>{% trans "Transfer entry" %}</h1>
-    <form action="" method="post" class="form-horizontal">
-        {% csrf_token %}
-        <p>{% blocktrans trimmed context "subevent" %}
-            Please select the date to which the following waiting list entry should be
-            transferred: <strong>{{ entry }}</strong>?
-        {% endblocktrans %}</p>
-        {% bootstrap_field form.subevent layout="control" %}
-        <div class="form-group submit-group">
-            <a href="{% url "control:event.orders.waitinglist" organizer=request.event.organizer.slug event=request.event.slug %}" class="btn btn-default btn-cancel">
-                {% trans "Cancel" %}
-            </a>
-            <button type="submit" class="btn btn-primary btn-save">
-                {% trans "Transfer" %}
-            </button>
-        </div>
-    </form>
-{% endblock %}

src/pretix/control/urls.py

@@ -480,8 +480,8 @@ urlpatterns = [
         re_path(r'^waitinglist/auto_assign$', waitinglist.AutoAssign.as_view(), name='event.orders.waitinglist.auto'),
         re_path(r'^waitinglist/(?P<entry>\d+)/delete$', waitinglist.EntryDelete.as_view(),
                 name='event.orders.waitinglist.delete'),
-        re_path(r'^waitinglist/(?P<entry>\d+)/transfer$', waitinglist.EntryTransfer.as_view(),
+        re_path(r'^waitinglist/(?P<entry>\d+)/edit$', waitinglist.EntryEdit.as_view(),
-                name='event.orders.waitinglist.transfer'),
+                name='event.orders.waitinglist.edit'),
         re_path(r'^checkins/$', checkin.CheckinListView.as_view(), name='event.orders.checkins'),
         re_path(r'^checkinlists/$', checkin.CheckinListList.as_view(), name='event.orders.checkinlists'),
         re_path(r'^checkinlists/add$', checkin.CheckinListCreate.as_view(), name='event.orders.checkinlists.add'),

src/pretix/control/views/waitinglist.py

@@ -53,7 +53,7 @@ from pretix.base.models import Item, LogEntry, Quota, WaitingListEntry
 from pretix.base.models.waitinglist import WaitingListException
 from pretix.base.services.waitinglist import assign_automatically
 from pretix.base.views.tasks import AsyncAction
-from pretix.control.forms.waitinglist import WaitingListEntryTransferForm
+from pretix.control.forms.waitinglist import WaitingListEntryEditForm
 from pretix.control.permissions import EventPermissionRequiredMixin
 from pretix.control.views import PaginationMixin
 
@@ -138,6 +138,17 @@ class WaitingListQuerySetMixin:
         elif force_filtered and '__ALL' not in self.request_data:
             qs = qs.none()
 
+        if self.request_data.get("search", "") != "":
+            s = self.request_data.get("search", "")
+            search_q = Q(email__icontains=s)
+
+            if self.request.event.settings.waiting_list_names_asked:
+                search_q = search_q | Q(name_cached__icontains=s)
+            if self.request.event.settings.waiting_list_phones_asked:
+                search_q = search_q | Q(phone__icontains=s)
+
+            qs = qs.filter(search_q)
+
         return qs
 
 
@@ -238,7 +249,7 @@ class WaitingListView(EventPermissionRequiredMixin, WaitingListQuerySetMixin, Pa
     def get_context_data(self, **kwargs):
         ctx = super().get_context_data(**kwargs)
         ctx['items'] = Item.objects.filter(event=self.request.event)
-        ctx['filtered'] = ("status" in self.request.GET or "item" in self.request.GET)
+        ctx['filtered'] = any(param in self.request.GET for param in ("status", "item", "search"))
 
         itemvar_cache = {}
         quota_cache = {}
@@ -390,25 +401,20 @@ class EntryDelete(EventPermissionRequiredMixin, CompatDeleteView):
         })
 
 
-class EntryTransfer(EventPermissionRequiredMixin, UpdateView):
+class EntryEdit(EventPermissionRequiredMixin, UpdateView):
     model = WaitingListEntry
-    template_name = 'pretixcontrol/waitinglist/transfer.html'
+    template_name = 'pretixcontrol/waitinglist/edit.html'
     permission = 'can_change_orders'
-    form_class = WaitingListEntryTransferForm
+    form_class = WaitingListEntryEditForm
     context_object_name = 'entry'
 
-    def dispatch(self, request, *args, **kwargs):
-        if not self.request.event.has_subevents:
-            raise Http404(_("This is not an event series."))
-        return super().dispatch(request, *args, **kwargs)
-
     def get_object(self, queryset=None) -> WaitingListEntry:
         return get_object_or_404(WaitingListEntry, pk=self.kwargs['entry'], event=self.request.event, voucher__isnull=True)
 
     @transaction.atomic
     def form_valid(self, form):
-        messages.success(self.request, _('The waitinglist entry has been transferred.'))
         if form.has_changed():
+            messages.success(self.request, _('The waitinglist entry has been changed.'))
             self.object.log_action(
                 'pretix.event.orders.waitinglist.changed', user=self.request.user, data={
                     k: form.cleaned_data.get(k) for k in form.changed_data

src/tests/control/test_waitinglist.py

@@ -19,19 +19,6 @@
 # You should have received a copy of the GNU Affero General Public License along with this program.  If not, see
 # <https://www.gnu.org/licenses/>.
 #
-
-# This file is based on an earlier version of pretix which was released under the Apache License 2.0. The full text of
-# the Apache License 2.0 can be obtained at <http://www.apache.org/licenses/LICENSE-2.0>.
-#
-# This file may have since been changed and any changes are released under the terms of AGPLv3 as described above. A
-# full history of changes and contributors is available at <https://github.com/pretix/pretix>.
-#
-# This file contains Apache-licensed contributions copyrighted by: Daniel
-#
-# Unless required by applicable law or agreed to in writing, software distributed under the Apache License 2.0 is
-# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
-# License for the specific language governing permissions and limitations under the License.
-
 from datetime import timedelta
 
 import pytest
@@ -39,7 +26,8 @@ from django.utils.timezone import now
 from django_scopes import scopes_disabled
 
 from pretix.base.models import (
-    Event, Item, Organizer, Quota, Team, User, Voucher, WaitingListEntry,
+    Event, Item, ItemVariation, Organizer, Quota, Team, User, Voucher,
+    WaitingListEntry,
 )
 from pretix.control.views.dashboards import waitinglist_widgets
 
@@ -52,11 +40,12 @@ def env():
         date_from=now(), plugins='pretix.plugins.banktransfer,tests.testdummy'
     )
     event.settings.set('ticketoutput_testdummy__enabled', True)
+    event.settings.set('waiting_list_names_asked', False)
+    event.settings.set('waiting_list_names_required', False)
+    event.settings.set('waiting_list_phones_asked', False)
     user = User.objects.create_user('dummy@dummy.dummy', 'dummy')
-    item1 = Item.objects.create(event=event, name="Ticket", default_price=23,
+    item1 = Item.objects.create(event=event, name="Ticket", default_price=23, admission=True, allow_waitinglist=True)
-                                admission=True)
+    item2 = Item.objects.create(event=event, name="Ticket", default_price=23, admission=True)
-    item2 = Item.objects.create(event=event, name="Ticket", default_price=23,
-                                admission=True)
 
     for i in range(5):
         WaitingListEntry.objects.create(
@@ -78,7 +67,16 @@ def env():
     t = Team.objects.create(organizer=o, can_view_orders=True, can_change_orders=True)
     t.members.add(user)
     t.limit_events.add(event)
-    return event, user, o, item1
+
+    wle = WaitingListEntry.objects.filter(item=item1).first()
+    variation = ItemVariation.objects.create(item=item1)
+
+    return {
+        "event": event,
+        "item1": item1,
+        "wle": wle,
+        "variation": variation,
+    }
 
 
 @pytest.mark.django_db
@@ -122,7 +120,7 @@ def test_list(client, env):
     assert 'foo0@bar.com' not in response.content.decode()
     assert 'valid@example.org' not in response.content.decode()
 
-    response = client.get('/control/event/dummy/dummy/waitinglist/?item=%d' % env[3].pk)
+    response = client.get('/control/event/dummy/dummy/waitinglist/?item=%d' % env['item1'].pk)
     assert 'item2@example.org' not in response.content.decode()
     assert 'foo0@bar.com' in response.content.decode()
 
@@ -191,11 +189,161 @@ def test_delete_bulk(client, env):
             WaitingListEntry.objects.get(id=wle.id)
 
 
+@pytest.mark.django_db
+def test_edit_settings(client, env):
+    event = env['event']
+    wle = env['wle']
+    client.login(email='dummy@dummy.dummy', password='dummy')
+
+    response = client.get('/control/event/dummy/dummy/waitinglist/%s/edit' % wle.id)
+    assert ['email', 'itemvar'] == list(response.context_data['form'].fields.keys())
+
+    event.settings.set('waiting_list_names_asked', True)
+    response = client.get('/control/event/dummy/dummy/waitinglist/%s/edit' % wle.id)
+    assert 'name_parts' in list(response.context_data['form'].fields.keys())
+
+    event.settings.set('waiting_list_names_required', True)
+    response = client.get('/control/event/dummy/dummy/waitinglist/%s/edit' % wle.id)
+    assert response.context_data['form'].fields['name_parts'].required is True
+
+    event.settings.set('waiting_list_phones_asked', True)
+    response = client.get('/control/event/dummy/dummy/waitinglist/%s/edit' % wle.id)
+    assert 'phone' in list(response.context_data['form'].fields.keys())
+
+
+@pytest.mark.django_db
+def test_edit_itemvariation(client, env):
+    item = env['item1']
+    variation = env['variation']
+    wle = env['wle']
+
+    client.login(email='dummy@dummy.dummy', password='dummy')
+
+    itemvar = f"{item.pk}-{variation.pk}"
+
+    client.post(
+        '/control/event/dummy/dummy/waitinglist/%s/edit' % wle.id,
+        data={
+            "email": f"1_{wle.email}",
+            "itemvar": itemvar
+        }
+    )
+
+    wle.refresh_from_db()
+    assert wle.variation == variation
+
+
+@pytest.mark.django_db
+def test_edit_validations_only_valid_item(client, env):
+    item = env['item1']
+    wle = env['wle']
+
+    client.login(email='dummy@dummy.dummy', password='dummy')
+
+    itemvar = f"{item.pk + 10000}"
+
+    response = client.post(
+        '/control/event/dummy/dummy/waitinglist/%s/edit' % wle.id,
+        data={
+            "email": f"1_{wle.email}",
+            "itemvar": itemvar
+        }
+    )
+    assert response.context_data['form'].errors['itemvar'] == ["Select a valid choice."]
+
+
+@pytest.mark.django_db
+def test_edit_validations_only_valid_variation(client, env):
+    item = env['item1']
+    wle = env['wle']
+    variation = env['variation']
+
+    client.login(email='dummy@dummy.dummy', password='dummy')
+
+    itemvar = f"{item.pk}-{variation.pk + 1}"
+
+    response = client.post(
+        '/control/event/dummy/dummy/waitinglist/%s/edit' % wle.id,
+        data={
+            "email": f"1_{wle.email}",
+            "itemvar": itemvar
+        }
+    )
+    assert response.context_data['form'].errors['itemvar'] == ["Select a valid choice."]
+
+
+@pytest.mark.django_db
+def test_edit_validations_inactive_item(client, env):
+    item = env['item1']
+    wle = env['wle']
+    item.active = False
+    item.save()
+
+    client.login(email='dummy@dummy.dummy', password='dummy')
+
+    response = client.post(
+        '/control/event/dummy/dummy/waitinglist/%s/edit' % wle.id,
+        data={
+            "email": f"1_{wle.email}",
+            "itemvar": f"{item.pk}"
+        }
+    )
+    assert response.context_data['form'].errors['itemvar'] == ["The selected product is not active."]
+
+
+@pytest.mark.django_db
+def test_edit_validations_inactive_variation(client, env):
+    item = env['item1']
+    wle = env['wle']
+    variation = env['variation']
+    wle.variation = variation
+    wle.save()
+
+    variation.active = False
+    variation.save()
+
+    client.login(email='dummy@dummy.dummy', password='dummy')
+    response = client.post(
+        '/control/event/dummy/dummy/waitinglist/%s/edit' % wle.id,
+        data={
+            "email": f"1_{wle.email}",
+            "itemvar": f"{item.pk}-{variation.pk}"
+        }
+    )
+    assert response.context_data['form'].errors['itemvar'] == ["The selected product is not active."]
+
+
+@pytest.mark.django_db
+def test_edit_voucher_send_out(client, env):
+    event = env['event']
+    item = env['item1']
+    wle = env['wle']
+
+    quota = Quota.objects.create(event=event, size=100)
+    quota.items.add(item)
+
+    client.login(email='dummy@dummy.dummy', password='dummy')
+
+    with scopes_disabled():
+        wle.send_voucher()
+
+    response = client.post(
+        '/control/event/dummy/dummy/waitinglist/%s/edit' % wle.id,
+        data={
+            "email": f"1_{wle.email}",
+            "itemvar": item.pk
+        },
+        follow=True
+    )
+    assert response.status_code == 404
+
+
 @pytest.mark.django_db
 def test_dashboard(client, env):
     with scopes_disabled():
-        quota = Quota.objects.create(name="Test", size=2, event=env[0])
+        quota = Quota.objects.create(name="Test", size=2, event=env['event'])
-        quota.items.add(env[3])
+        quota.items.add(env['item1'])
-        w = waitinglist_widgets(env[0])
+        w = waitinglist_widgets(env['event'])
+
     assert '1' in w[0]['content']
     assert '5' in w[1]['content']