Apply suggestions from code review

Co-authored-by: Raphael Michel <mail@raphaelmichel.de>

src/pretix/base/reldate.py

@@ -21,7 +21,7 @@
 #
 import datetime
 from collections import namedtuple
-from typing import Union
+from typing import Tuple, Union
 from zoneinfo import ZoneInfo
 
 from dateutil import parser
@@ -34,7 +34,7 @@ from django.utils.timezone import now
 from django.utils.translation import gettext_lazy as _
 from rest_framework import serializers
 
-BASE_CHOICES = (
+EVENT_CHOICES = (
     ('date_from', _('Event start')),
     ('date_to', _('Event end')),
     ('date_admission', _('Event admission')),
@@ -42,6 +42,12 @@ BASE_CHOICES = (
     ('presale_end', _('Presale end')),
 )
 
+# extend NO_BEFORE_VALUES in reldate.js if changed
+ORDER_CHOICES = (
+    ('datetime', _('Order creation')),
+)
+ORDER_CHOICES_KEYS = {choice[0] for choice in ORDER_CHOICES}
+
 RelativeDate = namedtuple('RelativeDate', ['days', 'minutes', 'time', 'is_after', 'base_date_name'], defaults=(0, None, None, False, 'date_from'))
 
 
@@ -51,17 +57,43 @@ class RelativeDateWrapper:
     that the underlying data is either a fixed date or a number of days and a wall clock
     time to calculate the date based on a base point.
 
-    The base point can be the date_from, date_to, date_admission, presale_start or presale_end
-    attribute of an event or subevent. If the respective attribute is not set, ``date_from``
-    will be used.
+    The base point can be the ``date_from``, ``date_to``, ``date_admission``, ``presale_start``
+    or ``presale_end`` attribute of an event or subevent, as well as a ``datetime`` of an order.
+    If the respective attribute is not set, ``date_from`` will be used.
     """
 
     def __init__(self, data: Union[datetime.datetime, RelativeDate]):
         self.data = data
 
-    def date(self, event) -> datetime.date:
-        from .models import SubEvent
+    def _resolve_base_date(self, reference) -> Tuple[datetime.datetime, ZoneInfo]:
+        """
 
+        :param reference:
+        :return:
+        """
+        from .models import Event, Order, SubEvent
+
+        if self.data.base_date_name in ORDER_CHOICES_KEYS:
+            if not isinstance(reference, Order):
+                raise ValueError('A order-based relative datetime choice must be used with an order object')
+            event = reference.event
+            base_date = getattr(reference, self.data.base_date_name)
+        elif isinstance(reference, SubEvent):
+            event = reference.event
+            base_date = (getattr(reference, self.data.base_date_name) or
+                         getattr(reference.event, self.data.base_date_name) or
+                         reference.date_from)
+        elif isinstance(reference, Event):
+            event = reference
+            base_date = getattr(reference, self.data.base_date_name) or event.date_from
+        else:
+            raise TypeError("Only event, subevent or order objects are supported")
+
+        tz = ZoneInfo(event.settings.timezone)
+
+        return base_date, tz
+
+    def date(self, reference) -> datetime.date:
         if isinstance(self.data, datetime.datetime):
             return self.data.date()
         elif isinstance(self.data, datetime.date):
@@ -70,15 +102,7 @@ class RelativeDateWrapper:
             if self.data.minutes is not None:
                 raise ValueError('A minute-based relative datetime can not be used as a date')
 
-            tz = ZoneInfo(event.settings.timezone)
-            if isinstance(event, SubEvent):
-                base_date = (
-                    getattr(event, self.data.base_date_name)
-                    or getattr(event.event, self.data.base_date_name)
-                    or event.date_from
-                )
-            else:
-                base_date = getattr(event, self.data.base_date_name) or event.date_from
+            base_date, tz = self._resolve_base_date(reference)
 
             if self.data.is_after:
                 new_date = base_date.astimezone(tz) + datetime.timedelta(days=self.data.days)
@@ -86,21 +110,11 @@ class RelativeDateWrapper:
                 new_date = base_date.astimezone(tz) - datetime.timedelta(days=self.data.days)
             return new_date.date()
 
-    def datetime(self, event) -> datetime.datetime:
-        from .models import SubEvent
-
+    def datetime(self, reference) -> datetime.datetime:
         if isinstance(self.data, (datetime.datetime, datetime.date)):
             return self.data
         else:
-            tz = ZoneInfo(event.settings.timezone)
-            if isinstance(event, SubEvent):
-                base_date = (
-                    getattr(event, self.data.base_date_name)
-                    or getattr(event.event, self.data.base_date_name)
-                    or event.date_from
-                )
-            else:
-                base_date = getattr(event, self.data.base_date_name) or event.date_from
+            base_date, tz = self._resolve_base_date(reference)
 
             if self.data.minutes is not None:
                 if self.data.is_after:
@@ -172,7 +186,9 @@ class RelativeDateWrapper:
                         minutes=None,
                         is_after=len(parts) > 4 and parts[4] == "after",
                     )
-            if data.base_date_name not in [k[0] for k in BASE_CHOICES]:
+            if data.base_date_name in ORDER_CHOICES_KEYS and parts[4] != "after":
+                raise ValueError('ORDER_CHOICE: {} cannot be combined with "before"'.format(data.base_date_name))
+            if data.base_date_name not in [k[0] for k in EVENT_CHOICES + ORDER_CHOICES]:
                 raise ValueError('{} is not a valid base date'.format(data.base_date_name))
         else:
             data = parser.parse(input)
@@ -311,11 +327,17 @@ class RelativeDateTimeField(forms.MultiValueField):
         ]
         if kwargs.get('limit_choices'):
             limit = kwargs.pop('limit_choices')
-            choices = [(k, v) for k, v in BASE_CHOICES if k in limit]
+            choices = [(k, v) for k, v in EVENT_CHOICES if k in limit]
         else:
-            choices = BASE_CHOICES
+            choices = EVENT_CHOICES
+
+        self.relative_to_order = kwargs.pop('relative_to_order', False)
+        if self.relative_to_order:
+            choices += ORDER_CHOICES
+
         if not kwargs.get('required', True):
             status_choices.insert(0, ('unset', _('Not set')))
+
         fields = reldatetimeparts(
             status=forms.ChoiceField(
                 choices=status_choices,
@@ -359,12 +381,13 @@ class RelativeDateTimeField(forms.MultiValueField):
         )
 
     def set_event(self, event):
-        self.widget.widgets[reldatetimeparts.indizes.rel_days_relationto].choices = [
-            (k, v) for k, v in BASE_CHOICES if getattr(event, k, None)
-        ]
-        self.widget.widgets[reldatetimeparts.indizes.rel_mins_relationto].choices = [
-            (k, v) for k, v in BASE_CHOICES if getattr(event, k, None)
+        choices = [
+            (k, v) for k, v in EVENT_CHOICES if getattr(event, k, None)
         ]
+        if self.relative_to_order:
+            choices += ORDER_CHOICES
+        self.widget.widgets[reldateparts.indizes.rel_days_relationto].choices = choices
+        self.widget.widgets[reldatetimeparts.indizes.rel_mins_relationto].choices = choices
 
     def compress(self, data_list):
         if not data_list:
@@ -404,6 +427,10 @@ class RelativeDateTimeField(forms.MultiValueField):
             raise ValidationError(self.error_messages['incomplete'])
         elif data.status == 'relative_minutes' and (data.rel_mins_number is None or not data.rel_mins_relationto):
             raise ValidationError(self.error_messages['incomplete'])
+        elif data.status == 'relative' and data.rel_days_relationto in ORDER_CHOICES_KEYS and data.rel_days_relation == 'before':
+            raise ValidationError(_('A relative date in relation to an order can only be after the order has been placed'))
+        elif data.status == 'relative' and data.rel_mins_relationto in ORDER_CHOICES_KEYS and data.rel_mins_relation == 'before':
+            raise ValidationError(_('A relative date in relation to an order can only be after the order has been placed'))
 
         return super().clean(value)
 
@@ -424,13 +451,14 @@ class RelativeDateWidget(RelativeDateTimeWidget):
 
     def __init__(self, *args, **kwargs):
         self.status_choices = kwargs.pop('status_choices')
+        base_choices = kwargs.pop('base_choices')
         widgets = reldateparts(
             status=forms.RadioSelect(choices=self.status_choices),
             absolute=forms.DateInput(
                 attrs={'class': 'datepickerfield'}
             ),
             rel_days_number=forms.NumberInput(),
-            rel_days_relationto=forms.Select(choices=kwargs.pop('base_choices')),
+            rel_days_relationto=forms.Select(choices=base_choices),
             rel_days_relation=forms.Select(choices=BEFORE_AFTER_CHOICE),
         )
         forms.MultiWidget.__init__(self, widgets=widgets, *args, **kwargs)
@@ -474,6 +502,12 @@ class RelativeDateField(RelativeDateTimeField):
         ]
         if not kwargs.get('required', True):
             status_choices.insert(0, ('unset', _('Not set')))
+
+        choices = EVENT_CHOICES
+        self.relative_to_order = kwargs.pop('relative_to_order', False)
+        if self.relative_to_order:
+            choices += ORDER_CHOICES
+
         fields = reldateparts(
             status=forms.ChoiceField(
                 choices=status_choices,
@@ -486,7 +520,7 @@ class RelativeDateField(RelativeDateTimeField):
                 required=False
             ),
             rel_days_relationto=forms.ChoiceField(
-                choices=BASE_CHOICES,
+                choices=choices,
                 required=False
             ),
             rel_days_relation=forms.ChoiceField(
@@ -495,15 +529,18 @@ class RelativeDateField(RelativeDateTimeField):
             ),
         )
         if 'widget' not in kwargs:
-            kwargs['widget'] = RelativeDateWidget(status_choices=status_choices, base_choices=BASE_CHOICES)
+            kwargs['widget'] = RelativeDateWidget(status_choices=status_choices, base_choices=choices)
         forms.MultiValueField.__init__(
             self, fields=fields, require_all_fields=False, *args, **kwargs
         )
 
     def set_event(self, event):
-        self.widget.widgets[reldateparts.indizes.rel_days_relationto].choices = [
-            (k, v) for k, v in BASE_CHOICES if getattr(event, k, None)
+        choices = [
+            (k, v) for k, v in EVENT_CHOICES if getattr(event, k, None)
         ]
+        if self.relative_to_order:
+            choices += ORDER_CHOICES
+        self.widget.widgets[reldateparts.indizes.rel_days_relationto].choices = choices
 
     def compress(self, data_list):
         if not data_list:
@@ -527,6 +564,8 @@ class RelativeDateField(RelativeDateTimeField):
             raise ValidationError(self.error_messages['incomplete'])
         elif data.status == 'relative' and (data.rel_days_number is None or not data.rel_days_relationto):
             raise ValidationError(self.error_messages['incomplete'])
+        elif data.status == 'relative' and data.rel_days_relationto in ORDER_CHOICES_KEYS and data.rel_days_relation == 'before':
+            raise ValidationError(_('A relative date in relation to an order can only be after the order has been placed'))
 
         return forms.MultiValueField.clean(self, value)
 

src/pretix/base/templates/pretixbase/forms/widgets/reldate.html

@@ -1,4 +1,6 @@
 {% load i18n %}
+{% load static %}
+<script src="{% static 'pretixbase/js/reldate.js' %}" defer></script>
 <div class="reldatetime">
     {% for group_name, group_choices, group-index in widget.subwidgets.0.optgroups %}
         {% for selopt in group_choices %}

src/pretix/base/templates/pretixbase/forms/widgets/reldatetime.html

@@ -1,4 +1,6 @@
 {% load i18n %}
+{% load static %}
+<script src="{% static 'pretixbase/js/reldate.js' %}" defer></script>
 <div class="reldatetime">
     {% for group_name, group_choices, group-index in widget.subwidgets.0.optgroups %}
         {% for selopt in group_choices %}

src/pretix/static/pretixbase/js/reldate.js

@@ -0,0 +1,44 @@
+if (!window.__reldateInitialized) {
+    window.__reldateInitialized = true;
+    document.addEventListener('DOMContentLoaded', () => {
+         const NO_BEFORE_VALUES = ['datetime'];
+
+        document.querySelectorAll('.reldatetime, .reldate').forEach(container => {
+            const groups = container.querySelectorAll('.radio');
+
+            groups.forEach(group => {
+                const selects = group.querySelectorAll('select');
+                if (selects.length < 2) return;
+
+                let referenceSelect = null;
+                let beforeAfterSelect = null;
+
+                selects.forEach(sel => {
+                    const values = Array.from(sel.options).map(o => o.value);
+                    // only attach to selects that contain problematic values
+                    if (NO_BEFORE_VALUES.some(v => values.includes(v))) {
+                        referenceSelect = sel;
+                    } else if (values.includes('before') && values.includes('after')) {
+                        beforeAfterSelect = sel;
+                    }
+                });
+                if (!referenceSelect || !beforeAfterSelect) return;
+
+                const beforeOption = beforeAfterSelect.querySelector('option[value="before"]');
+                const updateBeforeOption = () => {
+                    if (NO_BEFORE_VALUES.includes(referenceSelect.value)) {
+                        beforeOption.disabled = true;
+                        if (beforeAfterSelect.value === 'before') {
+                            beforeAfterSelect.value = 'after';
+                            beforeAfterSelect.dispatchEvent(new Event('change', { bubbles: true }));
+                        }
+                    } else {
+                        beforeOption.disabled = false;
+                    }
+                };
+                referenceSelect.addEventListener('change', updateBeforeOption);
+                updateBeforeOption();
+            });
+        });
+    });
+}

src/tests/base/test_reldate.py

@@ -19,13 +19,13 @@
 # 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 datetime import datetime, time
+from datetime import datetime, time, timedelta
 from zoneinfo import ZoneInfo
 
 import pytest
 from django_scopes import scope
 
-from pretix.base.models import Event, Organizer
+from pretix.base.models import Event, Order, Organizer
 from pretix.base.reldate import RelativeDate, RelativeDateWrapper
 
 TOKYO = ZoneInfo('Asia/Tokyo')
@@ -147,3 +147,29 @@ def test_unserialize():
 
     rdw = RelativeDateWrapper.from_string('RELDATE/minutes/60/date_from/')
     assert rdw.data == RelativeDate(days=0, time=None, base_date_name='date_from', minutes=60)
+
+
+@pytest.mark.django_db
+def test_relative_to_order(event):
+    with scope(organizer=event.organizer):
+        order_moment = datetime(2020, 3, 29, 18, 0, 0, tzinfo=TOKYO)
+
+        order = Order.objects.create(
+            code='FOO', event=event, email='dummy@dummy.test',
+            status=Order.STATUS_PENDING, secret="k24fiuwvu8kxz3y1",
+            datetime=order_moment,
+            expires=order_moment + timedelta(days=10),
+            sales_channel=event.organizer.sales_channels.get(identifier="web"),
+            total=23, locale='en'
+        )
+
+        rdw = RelativeDateWrapper(RelativeDate(days=1, time=None, base_date_name='datetime', minutes=None))
+        assert rdw.datetime(order).astimezone(TOKYO) == datetime(2020, 3, 28, 18, 0, 0, tzinfo=TOKYO)
+        assert rdw.to_string() == 'RELDATE/1/-/datetime/'
+        # this is expressible as a RelativeDate but the Wrapper should catch it as invalid when parsing
+        with pytest.raises(ValueError):
+            rdw.from_string(rdw.to_string())
+
+        rdw = RelativeDateWrapper(RelativeDate(days=1, time=None, base_date_name='datetime', minutes=None, is_after=True))
+        assert rdw.datetime(order).astimezone(TOKYO) == datetime(2020, 3, 30, 18, 0, 0, tzinfo=TOKYO)
+        assert rdw.to_string() == 'RELDATE/1/-/datetime/after'