diff --git a/doc/development/api/restriction.rst b/doc/development/api/restriction.rst index 56fa92da9..55fe771d6 100644 --- a/doc/development/api/restriction.rst +++ b/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() - and restriction.timeframe_to >= now()): + 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 diff --git a/src/tixlbase/models.py b/src/tixlbase/models.py index 71ce5e64c..cc7a0cc3d 100644 --- a/src/tixlbase/models.py +++ b/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", diff --git a/src/tixlcontrol/views/forms.py b/src/tixlcontrol/views/forms.py index b40934fed..06f8535e8 100644 --- a/src/tixlcontrol/views/forms.py +++ b/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: - self.fields['variations'] = VariationsField(item=self.item) + if 'variations' in self.fields and isinstance(self.fields['variations'], VariationsField): + 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'): + """ + We can't use a normal QuerySet as there theoretically might be + 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 () - print(self.item.pk) - 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) diff --git a/src/tixlcontrol/views/item.py b/src/tixlcontrol/views/item.py index d0bea70de..169f192fa 100644 --- a/src/tixlcontrol/views/item.py +++ b/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) diff --git a/src/tixlplugins/timerestriction/signals.py b/src/tixlplugins/timerestriction/signals.py index e9e1c4144..f2ac32c4f 100644 --- a/src/tixlplugins/timerestriction/signals.py +++ b/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