From f532853021ea863e435affead8f6afbbc16cab8f Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Wed, 27 Mar 2024 12:11:20 +0100 Subject: [PATCH] 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 * Respect variations --------- Co-authored-by: Mira Weller --- src/pretix/base/forms/questions.py | 23 +++++++--- src/pretix/base/forms/widgets.py | 6 +-- src/pretix/base/models/memberships.py | 8 +++- src/pretix/base/services/memberships.py | 44 ++++++++++++------- .../templates/pretixcontrol/item/index.html | 1 + src/pretix/presale/forms/checkout.py | 4 +- src/pretix/static/pretixcontrol/js/ui/main.js | 4 +- src/pretix/static/pretixpresale/js/ui/main.js | 4 +- src/tests/base/test_memberships.py | 18 ++++++++ 9 files changed, 82 insertions(+), 30 deletions(-) diff --git a/src/pretix/base/forms/questions.py b/src/pretix/base/forms/questions.py index 993d0458a7..f1c525b5ea 100644 --- a/src/pretix/base/forms/questions.py +++ b/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.'), - required=False, + help_text='' if initial else _('If you keep this empty, the ticket will be valid starting at the time of purchase.'), + 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.'), - required=False, + help_text='' if initial else _('If you keep this empty, the ticket will be valid starting at the time of purchase.'), + 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 = {} diff --git a/src/pretix/base/forms/widgets.py b/src/pretix/base/forms/widgets.py index c86ff75d62..917b339d38 100644 --- a/src/pretix/base/forms/widgets.py +++ b/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(): diff --git a/src/pretix/base/models/memberships.py b/src/pretix/base/models/memberships.py index 6cdcf20cae..b444509722 100644 --- a/src/pretix/base/models/memberships.py +++ b/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): - if ev: + def is_valid(self, ev=None, ticket_valid_from=None, valid_from_not_chosen=False): + 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() diff --git a/src/pretix/base/services/memberships.py b/src/pretix/base/services/memberships.py index a66130a13c..e85c728b41 100644 --- a/src/pretix/base/services/memberships.py +++ b/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): - 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 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 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]: diff --git a/src/pretix/control/templates/pretixcontrol/item/index.html b/src/pretix/control/templates/pretixcontrol/item/index.html index 4927c32d93..3ab926bc65 100644 --- a/src/pretix/control/templates/pretixcontrol/item/index.html +++ b/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 %} +
{% if f.template and not "template" in f.fields %} {% include f.template with form=f %} {% else %} diff --git a/src/pretix/presale/forms/checkout.py b/src/pretix/presale/forms/checkout.py index ce6071bbdd..6133d15258 100644 --- a/src/pretix/presale/forms/checkout.py +++ b/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: diff --git a/src/pretix/static/pretixcontrol/js/ui/main.js b/src/pretix/static/pretixcontrol/js/ui/main.js index d50f8486d5..2d62af0e8e 100644 --- a/src/pretix/static/pretixcontrol/js/ui/main.js +++ b/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")); diff --git a/src/pretix/static/pretixpresale/js/ui/main.js b/src/pretix/static/pretixpresale/js/ui/main.js index 26ad321213..7e71dbed4e 100644 --- a/src/pretix/static/pretixpresale/js/ui/main.js +++ b/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')) { diff --git a/src/tests/base/test_memberships.py b/src/tests/base/test_memberships.py index 9cb6ac0d0e..0d50c0567a 100644 --- a/src/tests/base/test_memberships.py +++ b/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,