Memberships: Prefer valid_from over event date for .is_valid() (#4003)

* Memberships: Prefer valid_from over event date for .is_valid()

* Fix tests

* Add parameter description

* Use reasonable default for requested_valid_from if membership starts in the future

* Set datetimepicker viewDate to closest allowed date

* Keep current value on going back to QuestionsStep

* Fix min_date/max_date in SplitDateTimePickerWidget

* Remove unused import

* Update src/pretix/base/models/memberships.py

Co-authored-by: Mira <weller@rami.io>

* Respect variations

---------

Co-authored-by: Mira Weller <weller@rami.io>

src/pretix/base/forms/questions.py

@@ -609,27 +609,38 @@ class BaseQuestionsForm(forms.Form):
                 max_date = now().astimezone(event.timezone) + timedelta(days=item.validity_dynamic_start_choice_day_limit)
             else:
                 max_date = None
+            min_date = now()
+            initial = None
+            if (item.require_membership or (pos.variation and pos.variation.require_membership)) and pos.used_membership:
+                if pos.used_membership.date_start >= now():
+                    initial = min_date = pos.used_membership.date_start
+                max_date = min(max_date, pos.used_membership.date_end) if max_date else pos.used_membership.date_end
             if item.validity_dynamic_duration_months or item.validity_dynamic_duration_days:
                 attrs = {}
                 if max_date:
                     attrs['data-max'] = max_date.date().isoformat()
+                if min_date:
+                    attrs['data-min'] = min_date.date().isoformat()
                 self.fields['requested_valid_from'] = forms.DateField(
                     label=_('Start date'),
-                    help_text=_('If you keep this empty, the ticket will be valid starting at the time of purchase.'),
+                    help_text='' if initial else _('If you keep this empty, the ticket will be valid starting at the time of purchase.'),
-                    required=False,
+                    required=bool(initial),
+                    initial=pos.requested_valid_from or initial,
                     widget=DatePickerWidget(attrs),
-                    validators=[MaxDateValidator(max_date.date())] if max_date else []
+                    validators=([MaxDateValidator(max_date.date())] if max_date else []) + [MinDateValidator(min_date.date())]
                 )
             else:
                 self.fields['requested_valid_from'] = forms.SplitDateTimeField(
                     label=_('Start date'),
-                    help_text=_('If you keep this empty, the ticket will be valid starting at the time of purchase.'),
+                    help_text='' if initial else _('If you keep this empty, the ticket will be valid starting at the time of purchase.'),
-                    required=False,
+                    required=bool(initial),
+                    initial=pos.requested_valid_from or initial,
                     widget=SplitDateTimePickerWidget(
                         time_format=get_format_without_seconds('TIME_INPUT_FORMATS'),
+                        min_date=min_date,
                         max_date=max_date
                     ),
-                    validators=[MaxDateTimeValidator(max_date)] if max_date else []
+                    validators=([MaxDateTimeValidator(max_date)] if max_date else []) + [MinDateTimeValidator(min_date)]
                 )
 
         add_fields = {}

src/pretix/base/forms/widgets.py

@@ -33,7 +33,7 @@
 # License for the specific language governing permissions and limitations under the License.
 
 import os
-from datetime import date
+from datetime import datetime
 
 from django import forms
 from django.utils.formats import get_format
@@ -188,11 +188,11 @@ class SplitDateTimePickerWidget(forms.SplitDateTimeWidget):
         time_attrs['autocomplete'] = 'off'
         if min_date:
             date_attrs['data-min'] = (
-                min_date if isinstance(min_date, date) else min_date.astimezone(get_current_timezone()).date()
+                min_date if not isinstance(min_date, datetime) else min_date.astimezone(get_current_timezone()).date()
             ).isoformat()
         if max_date:
             date_attrs['data-max'] = (
-                max_date if isinstance(max_date, date) else max_date.astimezone(get_current_timezone()).date()
+                max_date if not isinstance(max_date, datetime) else max_date.astimezone(get_current_timezone()).date()
             ).isoformat()
 
         def date_placeholder():

src/pretix/base/models/memberships.py

@@ -163,8 +163,12 @@ class Membership(models.Model):
     def attendee_name(self):
         return build_name(self.attendee_name_parts, fallback_scheme=lambda: self.customer.organizer.settings.name_scheme)
 
-    def is_valid(self, ev=None):
+    def is_valid(self, ev=None, ticket_valid_from=None, valid_from_not_chosen=False):
-        if ev:
+        if valid_from_not_chosen:
+            return not self.canceled and self.date_end >= now()
+        elif ticket_valid_from:
+            dt = ticket_valid_from
+        elif ev:
             dt = ev.date_from
         else:
             dt = now()

src/pretix/base/services/memberships.py

@@ -93,6 +93,8 @@ def validate_memberships_in_order(customer: Customer, positions: List[AbstractPo
     :param lock: Whether to place a SELECT FOR UPDATE lock on the selected memberships
     :param ignored_order: An order that should be ignored for usage counting
     :param testmode: If ``True``, only test mode memberships are allowed. If ``False``, test mode memberships are not allowed.
+    :param valid_from_not_chosen: Set to ``True`` to indicate that the customer is in an early step of the checkout flow
+                                  where the valid_from date is not selected yet. In this case, the valid_from date is not checked.
     """
     tz = event.timezone
     applicable_positions = [
@@ -163,15 +165,33 @@ def validate_memberships_in_order(customer: Customer, positions: List[AbstractPo
 
         ev = p.subevent or event
 
-        if not m.is_valid(ev):
+        if isinstance(p, (OrderPosition, CartPosition)):
-            raise ValidationError(
+            # override_ variants are for usage of fake cart in OrderChangeManager
-                _('You selected a membership that is valid from {start} to {end}, but selected an event '
+            valid_from = getattr(p, 'override_valid_from', p.valid_from)
-                  'taking place at {date}.').format(
+            valid_until = getattr(p, 'override_valid_until', p.valid_until)
-                    start=date_format(m.date_start.astimezone(tz), 'SHORT_DATETIME_FORMAT'),
+        else:  # future safety, not technically defined on AbstractPosition
-                    end=date_format(m.date_end.astimezone(tz), 'SHORT_DATETIME_FORMAT'),
+            valid_from = None
-                    date=date_format(ev.date_from.astimezone(tz), 'SHORT_DATETIME_FORMAT'),
+            valid_until = None
+
+        if not m.is_valid(ev, valid_from, valid_from_not_chosen=p.item.validity_dynamic_start_choice and valid_from_not_chosen):
+            if valid_from:
+                raise ValidationError(
+                    _('You selected a membership that is valid from {start} to {end}, but selected a ticket that '
+                      'starts to be valid on {date}.').format(
+                        start=date_format(m.date_start.astimezone(tz), 'SHORT_DATETIME_FORMAT'),
+                        end=date_format(m.date_end.astimezone(tz), 'SHORT_DATETIME_FORMAT'),
+                        date=date_format(valid_from.astimezone(tz), 'SHORT_DATETIME_FORMAT'),
+                    )
+                )
+            else:
+                raise ValidationError(
+                    _('You selected a membership that is valid from {start} to {end}, but selected an event '
+                      'taking place at {date}.').format(
+                        start=date_format(m.date_start.astimezone(tz), 'SHORT_DATETIME_FORMAT'),
+                        end=date_format(m.date_end.astimezone(tz), 'SHORT_DATETIME_FORMAT'),
+                        date=date_format(ev.date_from.astimezone(tz), 'SHORT_DATETIME_FORMAT'),
+                    )
                 )
-            )
 
         if p.variation and p.variation.require_membership:
             types = p.variation.require_membership_types.all()
@@ -197,14 +217,6 @@ def validate_memberships_in_order(customer: Customer, positions: List[AbstractPo
             m.usages += 1
 
         if not m.membership_type.allow_parallel_usage:
-            if isinstance(p, (OrderPosition, CartPosition)):
-                # override_ variants are for usage of fake cart in OrderChangeManager
-                valid_from = getattr(p, 'override_valid_from', p.valid_from)
-                valid_until = getattr(p, 'override_valid_until', p.valid_until)
-            else:  # future safety, not technically defined on AbstractPosition
-                valid_from = None
-                valid_until = None
-
             if (valid_from or valid_until) and not (p.item.validity_dynamic_start_choice and valid_from_not_chosen):
                 for used_range in m._used_for_ranges:
                     if valid_from and valid_from > used_range[1]:

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

@@ -267,6 +267,7 @@
                         {% bootstrap_field form.show_quota_left layout="control" %}
                         {% for f in plugin_forms %}
                             {% if not f.is_layouts and not f.title %}
+                                <hr />
                                 {% if f.template and not "template" in f.fields %}
                                     {% include f.template with form=f %}
                                 {% else %}

src/pretix/presale/forms/checkout.py

@@ -225,7 +225,9 @@ class MembershipForm(forms.Form):
 
         memberships = [
             m for m in self.memberships
-            if m.is_valid(ev) and m.membership_type in types
+            if m.membership_type in types and (
+                m.is_valid(ev, self.position.valid_from, valid_from_not_chosen=self.position.item.validity_dynamic_start_choice)
+            )
         ]
 
         if len(memberships) == 1:

src/pretix/static/pretixcontrol/js/ui/main.js

@@ -190,7 +190,9 @@ var form_handlers = function (el) {
         }
         if ($(this).is('[data-max]')) {
             opts["maxDate"] = $(this).attr("data-max");
-            opts["viewDate"] = $(this).attr("data-max");
+            opts["viewDate"] = (opts.minDate &&   // if minDate and maxDate are set, use the one closer to now as viewDate
+                    Math.abs(+new Date(opts.minDate) - new Date()) < Math.abs(+new Date(opts.maxDate) - new Date())
+            ) ? opts.minDate : opts.maxDate;
         }
         if ($(this).is('[data-is-payment-date]'))
             opts["daysOfWeekDisabled"] = JSON.parse($("body").attr("data-payment-weekdays-disabled"));

src/pretix/static/pretixpresale/js/ui/main.js

@@ -78,7 +78,9 @@ var form_handlers = function (el) {
         }
         if ($(this).is('[data-max]')) {
             opts["maxDate"] = $(this).attr("data-max");
-            opts["viewDate"] = $(this).attr("data-max");
+            opts["viewDate"] = (opts.minDate &&   // if minDate and maxDate are set, use the one closer to now as viewDate
+                    Math.abs(+new Date(opts.minDate) - new Date()) < Math.abs(+new Date(opts.maxDate) - new Date())
+            ) ? opts.minDate : opts.maxDate;
         }
         $(this).datetimepicker(opts);
         if ($(this).parent().is('.splitdatetimerow')) {

src/tests/base/test_memberships.py

@@ -180,6 +180,7 @@ def test_validate_membership_not_required(event, customer, membership, granting_
             customer,
             [
                 CartPosition(
+                    event=event,
                     item=granting_ticket,
                     used_membership=membership,
                 )
@@ -198,6 +199,7 @@ def test_validate_membership_required(event, customer, membership, requiring_tic
             customer,
             [
                 CartPosition(
+                    event=event,
                     item=requiring_ticket,
                 )
             ],
@@ -237,6 +239,7 @@ def test_validate_membership_canceled(event, customer, membership, requiring_tic
             customer,
             [
                 CartPosition(
+                    event=event,
                     item=requiring_ticket,
                     used_membership=membership
                 )
@@ -258,6 +261,7 @@ def test_validate_membership_test_mode(event, customer, membership, requiring_ti
             customer,
             [
                 CartPosition(
+                    event=event,
                     item=requiring_ticket,
                     used_membership=membership
                 )
@@ -275,6 +279,7 @@ def test_validate_membership_test_mode(event, customer, membership, requiring_ti
             customer,
             [
                 CartPosition(
+                    event=event,
                     item=requiring_ticket,
                     used_membership=membership
                 )
@@ -295,6 +300,7 @@ def test_validate_membership_wrong_customer(event, customer, membership, requiri
             customer2,
             [
                 CartPosition(
+                    event=event,
                     item=requiring_ticket,
                     used_membership=membership
                 )
@@ -316,6 +322,7 @@ def test_validate_membership_wrong_date(event, customer, membership, requiring_t
             customer,
             [
                 CartPosition(
+                    event=event,
                     item=requiring_ticket,
                     used_membership=membership
                 )
@@ -335,6 +342,7 @@ def test_validate_membership_wrong_type(event, customer, membership, requiring_t
             customer,
             [
                 CartPosition(
+                    event=event,
                     item=requiring_ticket,
                     used_membership=membership
                 )
@@ -373,6 +381,7 @@ def test_validate_membership_max_usages(event, customer, membership, requiring_t
             customer,
             [
                 CartPosition(
+                    event=event,
                     item=requiring_ticket,
                     used_membership=membership
                 )
@@ -388,6 +397,7 @@ def test_validate_membership_max_usages(event, customer, membership, requiring_t
         customer,
         [
             CartPosition(
+                event=event,
                 item=requiring_ticket,
                 used_membership=membership
             )
@@ -402,10 +412,12 @@ def test_validate_membership_max_usages(event, customer, membership, requiring_t
             customer,
             [
                 CartPosition(
+                    event=event,
                     item=requiring_ticket,
                     used_membership=membership
                 ),
                 CartPosition(
+                    event=event,
                     item=requiring_ticket,
                     used_membership=membership
                 ),
@@ -527,6 +539,8 @@ def test_validate_membership_parallel_validity_dynamic(event, customer, membersh
 
     membership_type.allow_parallel_usage = False
     membership_type.save()
+    membership.date_end = now() + timedelta(days=900)
+    membership.save()
 
     o1 = Order.objects.create(
         status=Order.STATUS_PENDING,
@@ -631,6 +645,8 @@ def test_validate_membership_parallel_validity_dynamic(event, customer, membersh
 
 @pytest.mark.django_db
 def test_validate_membership_parallel_validity_fixed(event, customer, membership, requiring_ticket, membership_type):
+    event.date_from = datetime(2021, 1, 1, 0, 0, 0, 0, tzinfo=TZ)
+    event.save()
     requiring_ticket.validity_mode = Item.VALIDITY_MODE_FIXED
     requiring_ticket.validity_fixed_from = now().replace(hour=2, minute=20, second=0)
     requiring_ticket.validity_fixed_until = now().replace(hour=6, minute=20, second=0)
@@ -638,6 +654,8 @@ def test_validate_membership_parallel_validity_fixed(event, customer, membership
 
     membership_type.allow_parallel_usage = False
     membership_type.save()
+    membership.date_end = now() + timedelta(days=900)
+    membership.save()
 
     o1 = Order.objects.create(
         status=Order.STATUS_PENDING,