VariationField form logic

doc/development/api/restriction.rst

@@ -185,8 +185,7 @@ In our example, the implementation could look like this::
                     if 'variation' not in v or v['variation'] not in applied_to:
                         continue
 
-                if (restriction.timeframe_from <= now()
+                if restriction.timeframe_from <= now() <= restriction.timeframe_to:
-                        and restriction.timeframe_to >= now()):
                     # Selling this item is currently possible
                     available = True
                     # If multiple time frames are currently active, make sure to

src/tixlbase/models.py

@@ -665,6 +665,32 @@ class ItemVariation(models.Model):
         return super().save(*args, **kwargs)
 
 
+class VariationsField(models.ManyToManyField):
+    """
+    This is a ManyToManyField using the tixlcontrol.views.forms.VariationsField
+    form field by default.
+    """
+
+    def formfield(self, **kwargs):
+        from tixlcontrol.views.forms import VariationsField as FVariationsField
+        from django.db.models.fields.related import RelatedField
+        defaults = {
+            'form_class': FVariationsField,
+            # We don't need a queryset
+            'queryset': ItemVariation.objects.none(),
+        }
+        defaults.update(kwargs)
+        # If initial is passed in, it's a list of related objects, but the
+        # MultipleChoiceField takes a list of IDs.
+        if defaults.get('initial') is not None:
+            initial = defaults['initial']
+            if callable(initial):
+                initial = initial()
+            defaults['initial'] = [i.pk for i in initial]
+        # Skip ManyToManyField in dependency chain
+        return super(RelatedField, self).formfield(**defaults)
+
+
 class BaseRestriction(models.Model):
     """
     A restriction is the abstract concept of a rule that limits the availability
@@ -683,7 +709,7 @@ class BaseRestriction(models.Model):
         null=True,
         related_name="restrictions_%(app_label)s_%(class)s",
     )
-    variations = models.ManyToManyField(
+    variations = VariationsField(
         ItemVariation,
         blank=True,
         related_name="restrictions_%(app_label)s_%(class)s",

src/tixlcontrol/views/forms.py

@@ -1,8 +1,12 @@
 from django import forms
+from django.core.exceptions import ValidationError
+from django.db import transaction, IntegrityError
+from django.utils.encoding import force_text
+
+from tixlbase.models import ItemVariation, PropertyValue
 
 
 class TolerantFormsetModelForm(forms.ModelForm):
-
     def has_changed(self):
         """
         Returns True if data differs from initial. Contrary to the default
@@ -34,18 +38,33 @@ class TolerantFormsetModelForm(forms.ModelForm):
         return False
 
 
-class RestrictionForm(forms.ModelForm):
+class RestrictionForm(TolerantFormsetModelForm):
+    """
+    The restriction form provides useful functionality for all forms
+    representing a restriction instance. To be concret, this form does
+    the necessary magic to make the 'variations' field work correctly
+     and look beautiful.
+    """
 
     def __init__(self, *args, **kwargs):
         if 'item' in kwargs:
             self.item = kwargs['item']
             del kwargs['item']
             super().__init__(*args, **kwargs)
-            if 'variations' in self.fields:
+            if 'variations' in self.fields and isinstance(self.fields['variations'], VariationsField):
-                self.fields['variations'] = VariationsField(item=self.item)
+                self.fields['variations'].set_item(self.item)
 
 
 class RestrictionInlineFormset(forms.BaseInlineFormSet):
+    """
+    This is the base class you should use for any formset you return
+    from a ``restriction_formset`` signal receiver that contains
+    RestrictionForm objects as its forms, as it correcly handles the
+    necessary item parameter for the RestrictionForm. While this could
+    be achieved with a regular formset, this also adds a
+    ``initialized_empty_form`` method which is the only way to correctly
+    render a working empty form for a JavaScript-enabled restriction formset.
+    """
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
@@ -57,23 +76,156 @@ class RestrictionInlineFormset(forms.BaseInlineFormSet):
             empty_permitted=True,
             item=self.instance
         )
+        self.add_fields(form, None)
         return form
 
     def _construct_form(self, i, **kwargs):
         kwargs['item'] = self.instance
         return super()._construct_form(i, **kwargs)
 
+    class Meta:
+        exclude = ['item']
+
 
 class VariationsField(forms.ModelMultipleChoiceField):
+    """
+    This form field is intended to be used to let the user select a
+    variation of a certain item, for example in a restriction plugin.
 
-    def __init__(self, item=None, **kwargs):
+    As this field expects the non-standard keyword parameter ``item``
+    at initialization time, this is field is normally named ``variations``
+    and lives inside a ``tixlcontrol.views.forms.RestrictionForm``, which
+    does some magic to provide this parameter.
+    """
+
+    def __init__(self, *args, item=None, **kwargs):
         self.item = item
-        super().__init__(self, **kwargs)
+        super().__init__(*args, **kwargs)
+
+    def set_item(self, item):
+        self.item = item
+        self._set_choices(self._get_choices())
 
     def _get_choices(self):
-        if not hasattr(self, 'item'):
+        """
-            return ()
+        We can't use a normal QuerySet as there theoretically might be
-        print(self.item.pk)
+        two types of variations: Some who already have a ItemVariation
+        object associated with tham and some who don't. We therefore use
+        the item's ``get_all_variations`` method. In the first case, we
+        use the ItemVariation objects primary key as our choice, key,
+        in the latter case we use a string constructed from the values
+        (see VariationDict.key() for implementation details).
+        """
+        if self.item is None:
             return ()
+        variations = self.item.get_all_variations()
+        return (
+            (
+                v['variation'].pk if 'variation' in v else v.key(),
+                v
+            ) for v in variations
+        )
+
+    def clean(self, value):
+        """
+        At cleaning time, we have to clean up the mess we produced with our
+        _get_choices implementation. In the case of ItemVariation object ids
+        we don't to anything to them, but if one of the selected items is a
+        list of PropertyValue objects (see _get_choices), we need to create
+        a new ItemVariation object for this combination and then add this to
+        our list of selected items.
+        """
+        if self.item is None:
+            raise ValueError(
+                "VariationsField object was not properly initialized. Please"
+                "use a tixlcontrol.views.forms.RestrictionForm form instead of"
+                "a plain Django ModelForm"
+            )
+
+        # Standard validation foo
+        if self.required and not value:
+            raise ValidationError(self.error_messages['required'], code='required')
+        elif not self.required and not value:
+            return self.queryset.none()
+        if not isinstance(value, (list, tuple)):
+            raise ValidationError(self.error_messages['list'], code='list')
+
+        # Build up a cache of variations having an ItemVariation object
+        # For implementation details, see ItemVariation.get_all_variations()
+        # which uses a very similar method
+        all_variations = self.item.variations.all().prefetch_related("values")
+        variations_cache = {}
+        for var in all_variations:
+            key = []
+            for v in var.values.all():
+                key.append((v.prop_id, v.pk))
+            key = tuple(sorted(key))
+            variations_cache[key] = var.pk
+
+        cleaned_value = []
+
+        # Wrap this in a transaction to prevent strange database state if we
+        # get a ValidationError half-way through
+        with transaction.atomic():
+            for pk in value:
+                if ":" in pk:
+                    # A combination of PropertyValues was given
+
+                    # Hash the combination in the same way as in our cache above
+                    key = []
+                    for pair in pk.split(","):
+                        key.append(tuple([int(i) for i in pair.split(":")]))
+                    key = tuple(sorted(key))
+
+                    if key in variations_cache:
+                        # An ItemVariation object already exists for this variation,
+                        # so use this. (This might occur if the variation object was
+                        # created _after_ the user loaded the form but _before_ he
+                        # submitted it.)
+                        cleaned_value.append(str(variations_cache[key]))
+                        continue
+
+                    # No ItemVariation present, create one!
+                    var = ItemVariation()
+                    var.item = self.item
+                    var.save()
+                    # Add the values to the ItemVariation object
+                    for pair in pk.split(","):
+                        prop, value = pair.split(":")
+                        try:
+                            var.values.add(
+                                PropertyValue.objects.get(
+                                    pk=value,
+                                    prop_pk=prop
+                                )
+                            )
+                        except PropertyValue.DoesNotExist:
+                            raise ValidationError(
+                                self.error_messages['invalid_pk_value'],
+                                code='invalid_pk_value',
+                                params={'pk': value},
+                            )
+                    variations_cache[key] = var.pk
+                    cleaned_value.append(str(var.pk))
+                else:
+                    # An ItemVariation id was given
+                    cleaned_value.append(pk)
+
+        qs = ItemVariation.objects.filter(item=self.item, pk__in=cleaned_value)
+
+        # Re-check for consistency
+        pks = set(force_text(getattr(o, "pk")) for o in qs)
+        for val in cleaned_value:
+            if force_text(val) not in pks:
+                raise ValidationError(
+                    self.error_messages['invalid_choice'],
+                    code='invalid_choice',
+                    params={'value': val},
+                )
+
+        # Since this overrides the inherited ModelChoiceField.clean
+        # we run custom validators here
+        self.run_validators(cleaned_value)
+        return qs
 
     choices = property(_get_choices, forms.ChoiceField._set_choices)

src/tixlcontrol/views/item.py

@@ -680,7 +680,7 @@ class ItemRestrictions(ItemDetailMixin, EventPermissionRequiredMixin, TemplateVi
                     else:
                         form.instance.event = request.event
                         form.instance.item = self.object
-                        form.instance.save()
+                        form.save()
             return redirect(self.get_success_url())
         else:
             context = self.get_context_data(object=self.object)

src/tixlplugins/timerestriction/signals.py

@@ -88,7 +88,7 @@ def availability_handler(sender, **kwargs):
                 if 'variation' not in v or v['variation'] not in applied_to:
                     continue
 
-            if (restriction.timeframe_from <= now() <= restriction.timeframe_to):
+            if restriction.timeframe_from <= now() <= restriction.timeframe_to:
                 # Selling this item is currently possible
                 available = True
                 # If multiple time frames are currently active, make sure to