diff --git a/docs/installation/index.rst b/docs/installation/index.rst
index 1789b0b16e..f3de65fdea 100644
--- a/docs/installation/index.rst
+++ b/docs/installation/index.rst
@@ -29,6 +29,6 @@ and expertise.
form_hosting
redis
issues/index
- upgrade-240
- upgrade-250
upgrade-300
+ upgrade-250
+ upgrade-240
diff --git a/docs/installation/upgrade-300.rst b/docs/installation/upgrade-300.rst
index e8c34854c5..c1594a2c4d 100644
--- a/docs/installation/upgrade-300.rst
+++ b/docs/installation/upgrade-300.rst
@@ -4,16 +4,66 @@
Upgrade details to Open Forms 3.0.0
===================================
-Backwards compatibility in form import changes
-==============================================
-Removal of Object API object type convertion
---------------------------------------------
+Open Forms 3.0 is a major version release that contains some breaking changes. As always
+we try to limit the impact of breaking changes through automatic migrations, and this
+release is no different, but there are some subtle changes in behaviour that you should
+be aware of, as they may require additional manual actions.
-With the UX changes of OF 2.8.0, the Object Api registration no longer lets you use
-hyperlinks when configuring the object type. The usage of hyperlinks for the object type
-is now also disallowed when importing a form.
+.. contents:: Jump to
+ :depth: 1
+ :local:
-Previously the hyperlinks would be converted to the expected format, and saved as such.
-The convertion will no-longer take place, and the 'to be imported' form is expected to
-use the new UUID format for the object type.
+Removal of price logic
+======================
+
+Price logic rules are removed in favour of setting the submission price via a form
+variable and normal logic rules. The conversion is automatic.
+
+There is a slight change in behaviour. When no price logic rules matched the trigger
+condition, the price set on the related product was used. This can lead to surprises
+and wrong amounts being paid due to logical errors in the form itself.
+
+The new behaviour will (deliberately) cause a crash that will show to the end-user
+as "something unexpectedly went wrong", since we refuse to make any (likely wrong)
+assumptions about the amount that needs to be paid. Crash information is visible
+in the error monitoring if that's set up correctly.
+
+.. note:: Existing, automatically converted forms are crash-free because we create an
+ explicit fallback logic rule that mimicks the old behaviour.
+
+Form components/fields changes
+==============================
+
+Password component removed
+--------------------------
+
+The password component was deprecated a long time ago, and has now been removed. If you
+need to replace it anywhere, use a regular textfield component instead, but you really
+shouldn't be asking users for passwords.
+
+Removed import conversions
+==========================
+
+For a number of changes, Open Forms ensured that old form exports could still be
+imported and would automatically convert some data. Some of these conversion have been
+removed.
+
+Removal of objecttype URL conversion in the Objects API registration options
+----------------------------------------------------------------------------
+
+Since the UX improvments in Open Forms 2.8.0 you can select the object type in a
+dropdown, and under the hood we save the unique identifier rather than the full API
+resource URL (which you used to have to copy-paste in a text field). The usage of
+hyperlinks for the object type is now also disallowed when importing a form.
+
+Previously the hyperlinks would be converted to the expected format, and saved as such,
+which was quite complex and not ideal for exports using the new format. We
+recommend re-creating the exports on a newer version of Open Forms.
+
+Removal of legacy translations conversion
+-----------------------------------------
+
+Old (from before Open Forms 2.4) form export files containing form field translations
+in the legacy format are now ignored instead of converted to the new format. We
+recommend re-creating the exports on a newer version of Open Forms.
diff --git a/docs/manual/forms/basics.rst b/docs/manual/forms/basics.rst
index 89b0ffb8bd..1c580737d2 100644
--- a/docs/manual/forms/basics.rst
+++ b/docs/manual/forms/basics.rst
@@ -199,7 +199,7 @@ product bevat een prijs die gebruikt kan worden als betaald moet worden voor
het product. Betaling kan ingesteld worden door de juiste **Betaalprovider** te
selecteren.
-Er zijn drie manieren om de prijs van een inzending te bepalen:
+Er zijn twee manieren om de prijs van een inzending te bepalen:
**Gebruik de prijs van het gekoppeld product**
@@ -226,10 +226,9 @@ te activeren.
**Gebruik prijslogica**
-Voor eenvoudige condities kan je prijslogic instellen. Onder een bepaalde conditie geldt
-een bepaalde, vaste, prijs. Indien aan geen enkele conditie voldaan is, dan wordt de
-prijs van het gekoppeld product gebruikt. De **Prijslogica** volgt verder dezelfde
-regels als reguliere **Logica**.
+.. versionremoved:: 3.0
+
+ De prijslogica is vervangen door gewone logica + gebruik van een variabele.
Zie ook: :ref:`configuration_payment_index`
diff --git a/src/openapi.yaml b/src/openapi.yaml
index 7eb8b7adf9..2a1e223c33 100644
--- a/src/openapi.yaml
+++ b/src/openapi.yaml
@@ -2512,229 +2512,6 @@ paths:
$ref: '#/components/headers/X-Is-Form-Designer'
Content-Language:
$ref: '#/components/headers/Content-Language'
- /api/v2/forms/{uuid_or_slug}/price-logic-rules:
- get:
- operationId: forms_price_logic_rules_list
- description: List all price logic rules defined for a form.
- summary: List price logic rules
- parameters:
- - in: path
- name: uuid_or_slug
- schema:
- type: integer
- description: A unique integer value identifying this form.
- required: true
- tags:
- - logic-rules
- security:
- - tokenAuth: []
- - cookieAuth: []
- responses:
- '200':
- content:
- application/json:
- schema:
- type: array
- items:
- $ref: '#/components/schemas/FormPriceLogic'
- description: ''
- headers:
- X-Session-Expires-In:
- $ref: '#/components/headers/X-Session-Expires-In'
- X-CSRFToken:
- $ref: '#/components/headers/X-CSRFToken'
- X-Is-Form-Designer:
- $ref: '#/components/headers/X-Is-Form-Designer'
- Content-Language:
- $ref: '#/components/headers/Content-Language'
- '401':
- content:
- application/json:
- schema:
- $ref: '#/components/schemas/Exception'
- description: ''
- headers:
- X-Session-Expires-In:
- $ref: '#/components/headers/X-Session-Expires-In'
- X-CSRFToken:
- $ref: '#/components/headers/X-CSRFToken'
- X-Is-Form-Designer:
- $ref: '#/components/headers/X-Is-Form-Designer'
- Content-Language:
- $ref: '#/components/headers/Content-Language'
- '403':
- content:
- application/json:
- schema:
- $ref: '#/components/schemas/Exception'
- description: ''
- headers:
- X-Session-Expires-In:
- $ref: '#/components/headers/X-Session-Expires-In'
- X-CSRFToken:
- $ref: '#/components/headers/X-CSRFToken'
- X-Is-Form-Designer:
- $ref: '#/components/headers/X-Is-Form-Designer'
- Content-Language:
- $ref: '#/components/headers/Content-Language'
- '404':
- content:
- application/json:
- schema:
- $ref: '#/components/schemas/Exception'
- description: ''
- headers:
- X-Session-Expires-In:
- $ref: '#/components/headers/X-Session-Expires-In'
- X-CSRFToken:
- $ref: '#/components/headers/X-CSRFToken'
- X-Is-Form-Designer:
- $ref: '#/components/headers/X-Is-Form-Designer'
- Content-Language:
- $ref: '#/components/headers/Content-Language'
- '405':
- content:
- application/json:
- schema:
- $ref: '#/components/schemas/Exception'
- description: ''
- headers:
- X-Session-Expires-In:
- $ref: '#/components/headers/X-Session-Expires-In'
- X-CSRFToken:
- $ref: '#/components/headers/X-CSRFToken'
- X-Is-Form-Designer:
- $ref: '#/components/headers/X-Is-Form-Designer'
- Content-Language:
- $ref: '#/components/headers/Content-Language'
- put:
- operationId: forms_price_logic_rules_update
- description: By sending a list of FormPriceLogic to this endpoint, all the FormPriceLogic
- related to the form will be replaced with the data sent to the endpoint.
- summary: Bulk configure price logic rules
- parameters:
- - in: header
- name: X-CSP-Nonce
- schema:
- type: string
- description: The value of the CSP nonce generated by the page embedding the
- SDK. If provided, fields containing rich text from WYSIWYG editors will
- be post-processed to allow inline styles with the provided nonce. If the
- embedding page emits a `style-src` policy containing `unsafe-inline`, then
- you can omit this header without losing functionality. We recommend favouring
- the nonce mechanism though.
- - in: path
- name: uuid_or_slug
- schema:
- type: integer
- description: A unique integer value identifying this form.
- required: true
- tags:
- - logic-rules
- requestBody:
- content:
- application/json:
- schema:
- type: array
- items:
- $ref: '#/components/schemas/FormPriceLogic'
- required: true
- security:
- - tokenAuth: []
- - cookieAuth: []
- responses:
- '200':
- content:
- application/json:
- schema:
- type: array
- items:
- $ref: '#/components/schemas/FormPriceLogic'
- description: ''
- headers:
- X-Session-Expires-In:
- $ref: '#/components/headers/X-Session-Expires-In'
- X-CSRFToken:
- $ref: '#/components/headers/X-CSRFToken'
- X-Is-Form-Designer:
- $ref: '#/components/headers/X-Is-Form-Designer'
- Content-Language:
- $ref: '#/components/headers/Content-Language'
- '400':
- content:
- application/json:
- schema:
- $ref: '#/components/schemas/ValidationError'
- description: ''
- headers:
- X-Session-Expires-In:
- $ref: '#/components/headers/X-Session-Expires-In'
- X-CSRFToken:
- $ref: '#/components/headers/X-CSRFToken'
- X-Is-Form-Designer:
- $ref: '#/components/headers/X-Is-Form-Designer'
- Content-Language:
- $ref: '#/components/headers/Content-Language'
- '401':
- content:
- application/json:
- schema:
- $ref: '#/components/schemas/Exception'
- description: ''
- headers:
- X-Session-Expires-In:
- $ref: '#/components/headers/X-Session-Expires-In'
- X-CSRFToken:
- $ref: '#/components/headers/X-CSRFToken'
- X-Is-Form-Designer:
- $ref: '#/components/headers/X-Is-Form-Designer'
- Content-Language:
- $ref: '#/components/headers/Content-Language'
- '403':
- content:
- application/json:
- schema:
- $ref: '#/components/schemas/Exception'
- description: ''
- headers:
- X-Session-Expires-In:
- $ref: '#/components/headers/X-Session-Expires-In'
- X-CSRFToken:
- $ref: '#/components/headers/X-CSRFToken'
- X-Is-Form-Designer:
- $ref: '#/components/headers/X-Is-Form-Designer'
- Content-Language:
- $ref: '#/components/headers/Content-Language'
- '404':
- content:
- application/json:
- schema:
- $ref: '#/components/schemas/Exception'
- description: ''
- headers:
- X-Session-Expires-In:
- $ref: '#/components/headers/X-Session-Expires-In'
- X-CSRFToken:
- $ref: '#/components/headers/X-CSRFToken'
- X-Is-Form-Designer:
- $ref: '#/components/headers/X-Is-Form-Designer'
- Content-Language:
- $ref: '#/components/headers/Content-Language'
- '405':
- content:
- application/json:
- schema:
- $ref: '#/components/schemas/Exception'
- description: ''
- headers:
- X-Session-Expires-In:
- $ref: '#/components/headers/X-Session-Expires-In'
- X-CSRFToken:
- $ref: '#/components/headers/X-CSRFToken'
- X-Is-Form-Designer:
- $ref: '#/components/headers/X-Is-Form-Designer'
- Content-Language:
- $ref: '#/components/headers/Content-Language'
/api/v2/forms/{uuid_or_slug}/variables:
get:
operationId: forms_variables_list
@@ -8471,36 +8248,6 @@ components:
title: Explanation template [nl]
description: Content that will be shown on the start page of the form, below
the title and above the log in text.
- FormPriceLogic:
- type: object
- properties:
- uuid:
- type: string
- format: uuid
- readOnly: true
- url:
- type: string
- format: uri
- readOnly: true
- form:
- type: string
- format: uri
- description: Form to which the pricing JSON logic applies.
- jsonLogicTrigger:
- title: JSON logic
- description: The trigger expression to determine if the actions should execute
- or not. Note that this must be a valid JsonLogic expression, and the first
- operand must be a reference to a variable in the form.
- price:
- type: string
- format: decimal
- pattern: ^-?\d{0,8}(?:\.\d{0,2})?$
- required:
- - form
- - jsonLogicTrigger
- - price
- - url
- - uuid
FormRegistrationBackend:
type: object
properties:
diff --git a/src/openforms/forms/api/serializers/__init__.py b/src/openforms/forms/api/serializers/__init__.py
index 8e7ed98316..33d1fb91c6 100644
--- a/src/openforms/forms/api/serializers/__init__.py
+++ b/src/openforms/forms/api/serializers/__init__.py
@@ -5,11 +5,9 @@
from .form_variable import FormVariableListSerializer, FormVariableSerializer
from .form_version import FormVersionSerializer
from .logic.form_logic import FormLogicSerializer
-from .logic.form_logic_price import FormPriceLogicSerializer
__all__ = [
"FormLogicSerializer",
- "FormPriceLogicSerializer",
"FormSerializer",
"FormExportSerializer",
"FormImportSerializer",
diff --git a/src/openforms/forms/api/serializers/logic/form_logic_price.py b/src/openforms/forms/api/serializers/logic/form_logic_price.py
deleted file mode 100644
index 760a587258..0000000000
--- a/src/openforms/forms/api/serializers/logic/form_logic_price.py
+++ /dev/null
@@ -1,22 +0,0 @@
-from openforms.api.serializers import ListWithChildSerializer
-from openforms.forms.api.serializers.logic.form_logic import FormLogicBaseSerializer
-from openforms.forms.models import FormPriceLogic
-
-
-class FormPriceLogicListSerializer(ListWithChildSerializer):
- child_serializer_class = "openforms.forms.api.serializers.logic.form_logic_price.FormPriceLogicSerializer"
-
-
-class FormPriceLogicSerializer(FormLogicBaseSerializer):
- class Meta(FormLogicBaseSerializer.Meta):
- model = FormPriceLogic
- list_serializer_class = FormPriceLogicListSerializer
- fields = FormLogicBaseSerializer.Meta.fields + ("price",)
- extra_kwargs = {
- **FormLogicBaseSerializer.Meta.extra_kwargs,
- "url": {
- "view_name": "api:form-price-logic-rules",
- "lookup_field": "uuid",
- "lookup_url_kwarg": "uuid_or_slug",
- },
- }
diff --git a/src/openforms/forms/api/viewsets.py b/src/openforms/forms/api/viewsets.py
index 2f6617ad1b..e0a81e87e2 100644
--- a/src/openforms/forms/api/viewsets.py
+++ b/src/openforms/forms/api/viewsets.py
@@ -48,7 +48,6 @@
FormDefinitionSerializer,
FormImportSerializer,
FormLogicSerializer,
- FormPriceLogicSerializer,
FormSerializer,
FormStepSerializer,
FormVariableListSerializer,
@@ -56,7 +55,6 @@
FormVersionSerializer,
)
from .serializers.logic.form_logic import FormLogicListSerializer
-from .serializers.logic.form_logic_price import FormPriceLogicListSerializer
@extend_schema(
@@ -294,23 +292,6 @@ def configuration(self, request: Request, *args, **kwargs):
status.HTTP_405_METHOD_NOT_ALLOWED: ExceptionSerializer,
},
),
- price_logic_rules_bulk_update=extend_schema(
- summary=_("Bulk configure price logic rules"),
- description=_(
- "By sending a list of FormPriceLogic to this endpoint, all the FormPriceLogic related to the form will be "
- "replaced with the data sent to the endpoint."
- ),
- tags=["logic-rules"],
- request=FormPriceLogicListSerializer,
- responses={
- status.HTTP_200_OK: FormPriceLogicListSerializer,
- status.HTTP_400_BAD_REQUEST: ValidationErrorSerializer,
- status.HTTP_401_UNAUTHORIZED: ExceptionSerializer,
- status.HTTP_403_FORBIDDEN: ExceptionSerializer,
- status.HTTP_404_NOT_FOUND: ExceptionSerializer,
- status.HTTP_405_METHOD_NOT_ALLOWED: ExceptionSerializer,
- },
- ),
)
class FormViewSet(viewsets.ModelViewSet):
"""
@@ -364,8 +345,6 @@ def get_queryset(self):
"variables_list",
"logic_rules_bulk_update",
"logic_rules_list",
- "price_logic_rules_bulk_update",
- "price_logic_rules_list",
):
queryset = queryset.select_related(None).prefetch_related(None)
@@ -615,61 +594,6 @@ def logic_rules_list(self, request, *args, **kwargs):
)
return Response(serializer.data, status=status.HTTP_200_OK)
- @action(
- detail=True,
- methods=["put"],
- url_path="price-logic-rules",
- url_name="price-logic-rules",
- )
- @transaction.atomic
- def price_logic_rules_bulk_update(self, request, *args, **kwargs):
- form = self.get_object()
- price_logic_rules = form.formpricelogic_set.all()
- # We expect that all the price logic rules associated with a form come in the request.
- # So we can delete any existing rule because they will be replaced.
- price_logic_rules.delete()
-
- serializer = FormPriceLogicSerializer(
- data=request.data,
- many=True,
- context={
- "request": request,
- "form": form,
- # context for :class:`openforms.api.fields.RelatedFieldFromContext` lookups
- "forms": {str(form.uuid): form},
- "form_variables": FormVariableWrapper(form),
- },
- )
- serializer.is_valid(raise_exception=True)
- serializer.save()
-
- return Response(serializer.data, status=status.HTTP_200_OK)
-
- @extend_schema(
- summary=_("List price logic rules"),
- description=_("List all price logic rules defined for a form."),
- tags=["logic-rules"],
- request=FormPriceLogicListSerializer,
- responses={
- status.HTTP_200_OK: FormPriceLogicListSerializer,
- status.HTTP_401_UNAUTHORIZED: ExceptionSerializer,
- status.HTTP_403_FORBIDDEN: ExceptionSerializer,
- status.HTTP_404_NOT_FOUND: ExceptionSerializer,
- status.HTTP_405_METHOD_NOT_ALLOWED: ExceptionSerializer,
- },
- )
- @price_logic_rules_bulk_update.mapping.get
- def price_logic_rules_list(self, request, *args, **kwargs):
- form = self.get_object()
- price_logic_rules = form.formpricelogic_set.all()
-
- serializer = FormPriceLogicSerializer(
- instance=price_logic_rules,
- many=True,
- context={"request": request, "form": form},
- )
- return Response(serializer.data, status=status.HTTP_200_OK)
-
FormViewSet.__doc__ = inspect.getdoc(FormViewSet).format(
admin_fields=_FORM_ADMIN_FIELDS_MARKDOWN
diff --git a/src/openforms/forms/migrations/0106_convert_price_logic_rules.py b/src/openforms/forms/migrations/0106_convert_price_logic_rules.py
new file mode 100644
index 0000000000..885c92a45e
--- /dev/null
+++ b/src/openforms/forms/migrations/0106_convert_price_logic_rules.py
@@ -0,0 +1,107 @@
+# Generated by Django 4.2.16 on 2024-11-25 15:32
+from decimal import Decimal
+
+from django.db import migrations
+from django.db.migrations.state import StateApps
+
+from openforms.forms.constants import LogicActionTypes
+from openforms.variables.constants import FormVariableDataTypes, FormVariableSources
+
+VARIABLE_NAME = "Total price"
+VARIABLE_KEY = "totalPrice"
+
+
+def _assignment_action(key: str, value: Decimal):
+ return {
+ "variable": key,
+ "action": {
+ "type": LogicActionTypes.variable,
+ "value": str(value),
+ },
+ }
+
+
+def convert_price_logic_rules_to_price_variable(apps: StateApps, _):
+ """
+ For each form that has price logic rules, create a variable to hold the price and
+ add normal logic rules.
+ """
+ Form = apps.get_model("forms", "Form")
+ forms_with_pricelogic = (
+ Form.objects.filter(formpricelogic__isnull=False)
+ .exclude(product__isnull=True)
+ .distinct()
+ )
+
+ for form in forms_with_pricelogic.iterator():
+ product = form.product
+ rules = form.formpricelogic_set.all()
+
+ # create a variable to hold the result.
+ variable_keys = set(form.formvariable_set.values_list("key", flat=True))
+ variable_key = VARIABLE_KEY
+ variable_name = VARIABLE_NAME
+ counter = 0
+ while variable_key in variable_keys:
+ counter += 1
+ variable_key = f"{variable_key}{counter}"
+ variable_name = f"{variable_name}{counter}"
+ if counter > 100:
+ raise RuntimeError(
+ "Could not generate a unique key without looping too long"
+ )
+
+ price_variable = form.formvariable_set.create(
+ form_definition=None,
+ name=variable_name,
+ key=variable_key,
+ source=FormVariableSources.user_defined,
+ data_type=FormVariableDataTypes.float,
+ )
+ form.price_variable_key = price_variable.key
+ form.save()
+
+ max_order = (
+ last_rule.order
+ if (last_rule := form.formlogic_set.order_by("order").last())
+ else 0
+ )
+
+ # set up regular logic rules for each price logic rule
+ for rule in rules:
+ max_order += 1
+ form.formlogic_set.create(
+ description="Converted price logic rule",
+ order=max_order,
+ is_advanced=True,
+ json_logic_trigger=rule.json_logic_trigger,
+ actions=[_assignment_action(form.price_variable_key, rule.price)],
+ )
+
+ # create one fallback rule in case none of the triggers hit
+ composite_negated_trigger = {
+ "!": {"or": [rule.json_logic_trigger for rule in rules]}
+ }
+ max_order += 1
+ form.formlogic_set.create(
+ description="Converted price logic rule",
+ order=max_order,
+ is_advanced=True,
+ json_logic_trigger=composite_negated_trigger,
+ actions=[_assignment_action(form.price_variable_key, product.price)],
+ )
+
+ rules.delete()
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ("forms", "0105_alter_form_all_submissions_removal_limit_and_more"),
+ ]
+
+ operations = [
+ migrations.RunPython(
+ convert_price_logic_rules_to_price_variable, migrations.RunPython.noop
+ ),
+ ]
diff --git a/src/openforms/forms/models/pricing_logic.py b/src/openforms/forms/models/pricing_logic.py
index e0fcc70b88..a5a40af393 100644
--- a/src/openforms/forms/models/pricing_logic.py
+++ b/src/openforms/forms/models/pricing_logic.py
@@ -16,15 +16,7 @@ class FormPriceLogic(models.Model):
The data model is similar to :class:`openforms.forms.models.FormLogic`.
- .. todo::
-
- Document logic evaluation - rules are evaluated and as soon as one matches,
- that's the winner. If multiple rules match, the first one wins.
-
- .. todo::
-
- Support complex conditions (AND/OR them together). This is a broader logic
- editing/evaluation issue that applies for regular form logic too.
+ .. warning:: This feature is no longer supported.
"""
uuid = models.UUIDField(_("UUID"), unique=True, default=_uuid.uuid4)
diff --git a/src/openforms/forms/tests/factories.py b/src/openforms/forms/tests/factories.py
index a747d15051..73ad058afb 100644
--- a/src/openforms/forms/tests/factories.py
+++ b/src/openforms/forms/tests/factories.py
@@ -2,11 +2,12 @@
import factory
+from openforms.forms.constants import LogicActionTypes
from openforms.products.tests.factories import ProductFactory
from openforms.registrations.registry import register as registration_registry
from openforms.variables.constants import FormVariableDataTypes, FormVariableSources
-from ..models import FormDefinition, FormStep, FormVariable
+from ..models import Form, FormDefinition, FormStep, FormVariable
from ..utils import form_to_json
@@ -64,6 +65,45 @@ def registration_backend(form, create, extracted, **kwargs):
else FormRegistrationBackendFactory.build
)(form=form, backend=extracted, **kwargs)
+ @factory.post_generation
+ def price_logic(
+ form: Form, # pyright: ignore[reportGeneralTypeIssues]
+ create,
+ extracted,
+ **kwargs,
+ ):
+ """
+ Configure the form to evaluate the price using logic rules and read it from
+ the respective variable.
+
+ .. note:: submissions for the form must call
+ ``openforms.submissions.utils.persist_user_defined_variables``
+
+ .. note:: forms with price logic must have at least one step
+ """
+ if not (extracted or kwargs):
+ return
+
+ if not create:
+ raise ValueError(
+ "Price logic can only be specified with the create strategy"
+ )
+
+ form_logic = extracted or FormLogicFactory.create(
+ form=form,
+ json_logic_trigger=kwargs.get("json_logic_trigger", True),
+ for_submission_price=True,
+ price_variable=kwargs.get("price_variable", "totalPrice"),
+ price_value=kwargs.get("price_value", 5.00),
+ order=kwargs.get(
+ "order", 1000
+ ), # should typically be one of the last rules evaluated
+ )
+ variable_key = form_logic.actions[0]["variable"]
+ FormVariableFactory.create(form=form, for_price=True, key=variable_key)
+ form.price_variable_key = variable_key
+ form.save()
+
class FormRegistrationBackendFactory(factory.django.DjangoModelFactory):
key = factory.Sequence(lambda n: f"backend{n}")
@@ -175,26 +215,29 @@ def post(obj, create, extracted, **kwargs):
class FormLogicFactory(factory.django.DjangoModelFactory):
json_logic_trigger = {"==": [{"var": "test-key"}, 1]}
- actions = [{"action": {"type": "disable-next"}}]
class Meta:
model = "forms.FormLogic"
+ class Params:
+ # generate a logic rule that sets a submission price
+ for_submission_price = False
+ price_variable = "totalPrice"
+ price_value = 5.00 # literal value or JSON logic expression
-class FormPriceLogicFactory(factory.django.DjangoModelFactory):
- form = factory.SubFactory(FormFactory)
- json_logic_trigger = {"==": [{"var": "test-key"}, 1]}
- price = factory.Faker(
- "pydecimal",
- left_digits=2,
- right_digits=2,
- positive=True,
- min_value=5.00,
- max_value=100.00,
- )
-
- class Meta:
- model = "forms.FormPriceLogic"
+ @factory.lazy_attribute
+ def actions(self):
+ if self.for_submission_price: # type: ignore
+ return [
+ {
+ "variable": self.price_variable, # type: ignore
+ "action": {
+ "type": LogicActionTypes.variable,
+ "value": self.price_value, # type: ignore
+ },
+ }
+ ]
+ return [{"action": {"type": LogicActionTypes.disable_next}}]
class FormVariableFactory(factory.django.DjangoModelFactory):
@@ -213,6 +256,13 @@ class Params:
source=FormVariableSources.user_defined,
form_definition=None,
)
+ for_price = factory.Trait(
+ name="Total price",
+ key="totalPrice",
+ form_definition=None,
+ source=FormVariableSources.user_defined,
+ data_type=FormVariableDataTypes.float,
+ )
@factory.post_generation
def form_definition(obj, create, extracted, **kwargs):
diff --git a/src/openforms/forms/tests/test_api_form_price_logic_bulk.py b/src/openforms/forms/tests/test_api_form_price_logic_bulk.py
deleted file mode 100644
index 5ad4fffd2f..0000000000
--- a/src/openforms/forms/tests/test_api_form_price_logic_bulk.py
+++ /dev/null
@@ -1,192 +0,0 @@
-from decimal import Decimal
-
-from rest_framework import status
-from rest_framework.reverse import reverse, reverse_lazy
-from rest_framework.test import APITestCase
-
-from openforms.accounts.tests.factories import SuperUserFactory, UserFactory
-
-from ..models import FormPriceLogic
-from .factories import FormFactory, FormPriceLogicFactory
-
-
-class FormPriceLogicBulkAPITests(APITestCase):
- @classmethod
- def setUpTestData(cls):
- super().setUpTestData()
-
- cls.superuser = SuperUserFactory.create()
- cls.form = FormFactory.create(
- generate_minimal_setup=True,
- formstep__form_definition__configuration={
- "components": [
- {
- "type": "textfield",
- "key": "step1_textfield1",
- }
- ]
- },
- )
- cls.form_url = reverse(
- "api:form-detail", kwargs={"uuid_or_slug": cls.form.uuid}
- )
-
- def test_auth_required(self):
- response = self.client.get(
- reverse_lazy(
- "api:form-price-logic-rules", kwargs={"uuid_or_slug": self.form.uuid}
- )
- )
-
- self.assertEqual(status.HTTP_401_UNAUTHORIZED, response.status_code)
-
- def test_staff_user_required(self):
- user = UserFactory.create(is_staff=False)
- self.client.force_authenticate(user)
-
- response = self.client.get(
- reverse_lazy(
- "api:form-price-logic-rules", kwargs={"uuid_or_slug": self.form.uuid}
- )
- )
-
- self.assertEqual(status.HTTP_403_FORBIDDEN, response.status_code)
-
- def test_list_and_filter_price_logic(self):
- self.client.force_authenticate(self.superuser)
- fpl1, fpl2 = FormPriceLogicFactory.create_batch(2)
- assert fpl1.form != fpl2.form
-
- url = reverse(
- "api:form-price-logic-rules", kwargs={"uuid_or_slug": fpl1.form.uuid}
- )
- response = self.client.get(url)
-
- self.assertEqual(status.HTTP_200_OK, response.status_code)
- response_data = response.json()
- self.assertEqual(len(response_data), 1)
- self.assertEqual(response_data[0]["uuid"], str(fpl1.uuid))
-
- def test_create_price_logic(self):
- self.client.force_authenticate(user=self.superuser)
- price_logic_data = [
- {
- "form": f"http://testserver{self.form_url}",
- "json_logic_trigger": {
- "==": [
- {"var": "step1_textfield1"},
- "test",
- ]
- },
- "price": "15.00",
- }
- ]
-
- url = reverse(
- "api:form-price-logic-rules", kwargs={"uuid_or_slug": self.form.uuid}
- )
- response = self.client.put(url, data=price_logic_data)
-
- self.assertEqual(status.HTTP_200_OK, response.status_code)
- price_logics_qs = FormPriceLogic.objects.all()
- self.assertEqual(price_logics_qs.count(), 1)
- price_logic = price_logics_qs.get()
- self.assertEqual(price_logic.form, self.form)
- self.assertEqual(price_logic.price, Decimal("15.00"))
-
- def test_create_logic_with_dates(self):
- self.client.force_authenticate(user=self.superuser)
- form = FormFactory.create(
- generate_minimal_setup=True,
- formstep__form_definition__configuration={
- "components": [
- {
- "type": "datetime",
- "key": "dateOfBirth",
- }
- ]
- },
- )
- form_url = reverse("api:form-detail", kwargs={"uuid_or_slug": form.uuid})
- price_logic_data = [
- {
- "form": f"http://testserver{form_url}",
- "json_logic_trigger": {
- ">": [
- {"date": {"var": "dateOfBirth"}},
- {"-": [{"today": []}, {"rdelta": [18]}]},
- ]
- },
- "price": "15.00",
- }
- ]
-
- url = reverse("api:form-price-logic-rules", kwargs={"uuid_or_slug": form.uuid})
- response = self.client.put(url, data=price_logic_data)
-
- self.assertEqual(status.HTTP_200_OK, response.status_code)
-
- def test_delete_price_logic(self):
- self.client.force_authenticate(user=self.superuser)
- price_rule = FormPriceLogicFactory.create(form__generate_minimal_setup=True)
-
- url = reverse(
- "api:form-price-logic-rules", kwargs={"uuid_or_slug": price_rule.form.uuid}
- )
-
- response = self.client.put(url, data=[])
-
- self.assertEqual(status.HTTP_200_OK, response.status_code)
- self.assertFalse(FormPriceLogic.objects.exists())
-
- def test_invalid_logic_trigger(self):
- self.client.force_authenticate(user=self.superuser)
- price_logic_data = [
- {
- "form": f"http://testserver{self.form_url}",
- "json_logic_trigger": {
- "invalid_op": [
- {"var": "step1_textfield1"},
- "hide step 1",
- ]
- },
- "price": "123.15",
- }
- ]
-
- url = reverse(
- "api:form-price-logic-rules", kwargs={"uuid_or_slug": self.form.uuid}
- )
- response = self.client.put(url, data=price_logic_data)
-
- self.assertEqual(status.HTTP_400_BAD_REQUEST, response.status_code)
- self.assertEqual(
- response.json()["invalidParams"][0]["name"],
- "0.jsonLogicTrigger",
- )
-
- def test_invalid_price(self):
- self.client.force_authenticate(user=self.superuser)
- price_logic_data = [
- {
- "form": f"http://testserver{self.form_url}",
- "json_logic_trigger": {
- "==": [
- {"var": "step1_textfield1"},
- "hide step 1",
- ]
- },
- "price": "",
- }
- ]
-
- url = reverse(
- "api:form-price-logic-rules", kwargs={"uuid_or_slug": self.form.uuid}
- )
- response = self.client.put(url, data=price_logic_data)
-
- self.assertEqual(status.HTTP_400_BAD_REQUEST, response.status_code)
- self.assertEqual(
- response.json()["invalidParams"][0]["name"],
- "0.price",
- )
diff --git a/src/openforms/forms/tests/test_migrations.py b/src/openforms/forms/tests/test_migrations.py
new file mode 100644
index 0000000000..95676f0d8a
--- /dev/null
+++ b/src/openforms/forms/tests/test_migrations.py
@@ -0,0 +1,210 @@
+from decimal import Decimal
+
+from django.db.migrations.state import StateApps
+
+from openforms.submissions.form_logic import check_submission_logic
+from openforms.submissions.models import (
+ Submission,
+ SubmissionStep,
+ SubmissionValueVariable,
+)
+from openforms.submissions.pricing import get_submission_price
+from openforms.utils.tests.test_migrations import TestMigrations
+from openforms.variables.constants import FormVariableDataTypes, FormVariableSources
+
+
+def _persist_user_defined_variables(submission):
+ # inspired by openforms.submissions.utils.persist_user_defined_variables
+ data = submission.data
+ check_submission_logic(submission, data)
+ state = submission.load_submission_value_variables_state()
+ variables = state.variables
+
+ user_defined_vars_data = {
+ variable.key: variable.value
+ for variable in variables.values()
+ if variable.form_variable
+ and variable.form_variable.source == FormVariableSources.user_defined
+ }
+
+ if user_defined_vars_data:
+ SubmissionValueVariable.objects.bulk_create_or_update_from_data(
+ user_defined_vars_data, submission
+ )
+
+
+class FormLogicMigrationTests(TestMigrations):
+ app = "forms"
+ migrate_from = "0105_alter_form_all_submissions_removal_limit_and_more"
+ migrate_to = "0106_convert_price_logic_rules"
+
+ def setUpBeforeMigration(self, apps: StateApps):
+ # set up some variants that will each be hit for different submissions. After
+ # migrating to form variable pricing, the result must be the same.
+ Product = apps.get_model("products", "Product")
+ Form = apps.get_model("forms", "Form")
+ FormDefinition = apps.get_model("forms", "FormDefinition")
+ FormStep = apps.get_model("forms", "FormStep")
+ FormPriceLogic = apps.get_model("forms", "FormPriceLogic")
+ FormVariable = apps.get_model("forms", "FormVariable")
+
+ product = Product.objects.create(name="Test product", price=Decimal("4.12"))
+ fd = FormDefinition.objects.create(
+ name="Pricing tests", configuration={"components": []}
+ )
+ form = Form.objects.create(name="Pricing tests", product=product, slug="step-1")
+ form_step = FormStep.objects.create(form=form, form_definition=fd, order=1)
+ amount_variable = FormVariable.objects.create(
+ form=form,
+ name="Amount",
+ key="amount",
+ source=FormVariableSources.user_defined,
+ data_type=FormVariableDataTypes.float,
+ initial_value=1.0,
+ )
+ FormPriceLogic.objects.create(
+ form=form,
+ json_logic_trigger={"==": [{"var": "amount"}, 3]},
+ price=Decimal("11.99"),
+ )
+ FormPriceLogic.objects.create(
+ form=form,
+ json_logic_trigger={"==": [{"var": "amount"}, 5]},
+ price=Decimal("19.99"),
+ )
+
+ # we deliberately use the real models here to get access to the properties and
+ # custom methods so that we can call get_submission_price. This requires
+ # migrations to be properly orchestrated so that no schema migrations in the
+ # submissions app happen at the wrong time! It could be a possible future
+ # failure point.
+
+ submission1 = Submission.objects.create(form_id=form.id)
+ SubmissionStep.objects.create(submission=submission1, form_step_id=form_step.id)
+ self.submission1_pk = submission1.pk
+ SubmissionValueVariable.objects.create(
+ submission=submission1,
+ form_variable_id=amount_variable.id,
+ key="amount",
+ value=1.0,
+ )
+ price1 = get_submission_price(submission1)
+ assert price1 == Decimal("4.12")
+
+ submission2 = Submission.objects.create(form_id=form.id)
+ SubmissionStep.objects.create(submission=submission2, form_step_id=form_step.id)
+ self.submission2_pk = submission2.pk
+ SubmissionValueVariable.objects.create(
+ submission=submission2,
+ form_variable_id=amount_variable.id,
+ key="amount",
+ value=3,
+ )
+ price2 = get_submission_price(submission2)
+ assert price2 == Decimal("11.99")
+
+ submission3 = Submission.objects.create(form_id=form.id)
+ SubmissionStep.objects.create(submission=submission3, form_step_id=form_step.id)
+ self.submission3_pk = submission3.pk
+ SubmissionValueVariable.objects.create(
+ submission=submission3,
+ form_variable_id=amount_variable.id,
+ key="amount",
+ value=5,
+ )
+ price3 = get_submission_price(submission3)
+ assert price3 == Decimal("19.99")
+
+ def test_prices_still_the_same_after_migration(self):
+ with self.subTest("submission 1"):
+ submission1 = Submission.objects.get(pk=self.submission1_pk)
+ _persist_user_defined_variables(submission1)
+
+ price1 = get_submission_price(submission1)
+
+ self.assertEqual(price1, Decimal("4.12"))
+
+ with self.subTest("submission 2"):
+ submission2 = Submission.objects.get(pk=self.submission2_pk)
+ _persist_user_defined_variables(submission2)
+
+ price2 = get_submission_price(submission2)
+
+ self.assertEqual(price2, Decimal("11.99"))
+
+ with self.subTest("submission 3"):
+ submission3 = Submission.objects.get(pk=self.submission3_pk)
+ _persist_user_defined_variables(submission3)
+
+ price3 = get_submission_price(submission3)
+
+ self.assertEqual(price3, Decimal("19.99"))
+
+ def test_price_variable_created(self):
+ Form = self.apps.get_model("forms", "Form")
+ form = Form.objects.get()
+
+ variables = {variable.key: variable for variable in form.formvariable_set.all()}
+
+ self.assertIn("totalPrice", variables)
+ variable = variables["totalPrice"]
+ self.assertEqual(variable.data_type, FormVariableDataTypes.float)
+ self.assertEqual(variable.source, FormVariableSources.user_defined)
+
+
+class DuplicatePriceVariableMigrationTests(TestMigrations):
+ app = "forms"
+ migrate_from = "0105_alter_form_all_submissions_removal_limit_and_more"
+ migrate_to = "0106_convert_price_logic_rules"
+
+ def setUpBeforeMigration(self, apps: StateApps):
+ # set up some variants that will each be hit for different submissions. After
+ # migrating to form variable pricing, the result must be the same.
+ Product = apps.get_model("products", "Product")
+ Form = apps.get_model("forms", "Form")
+ FormDefinition = apps.get_model("forms", "FormDefinition")
+ FormStep = apps.get_model("forms", "FormStep")
+ FormPriceLogic = apps.get_model("forms", "FormPriceLogic")
+ FormVariable = apps.get_model("forms", "FormVariable")
+
+ product = Product.objects.create(name="Test product", price=Decimal("4.12"))
+ fd = FormDefinition.objects.create(
+ name="Pricing tests", configuration={"components": []}
+ )
+ form = Form.objects.create(name="Pricing tests", product=product, slug="step-1")
+ FormStep.objects.create(form=form, form_definition=fd, order=1)
+ FormPriceLogic.objects.create(
+ form=form,
+ json_logic_trigger={"==": [{"var": "amount"}, 3]},
+ price=Decimal("11.99"),
+ )
+ FormVariable.objects.create(
+ form=form,
+ name="Amount",
+ key="amount",
+ source=FormVariableSources.user_defined,
+ data_type=FormVariableDataTypes.float,
+ initial_value=1.0,
+ )
+ # causes conflicts
+ FormVariable.objects.create(
+ form=form,
+ name="Total price",
+ key="totalPrice",
+ source=FormVariableSources.user_defined,
+ data_type=FormVariableDataTypes.float,
+ initial_value=10.0,
+ )
+
+ def test_price_variable_created(self):
+ Form = self.apps.get_model("forms", "Form")
+ form = Form.objects.get()
+
+ variables = {variable.key: variable for variable in form.formvariable_set.all()}
+
+ self.assertIn("totalPrice", variables) # pre-existing
+ self.assertIn("totalPrice1", variables) # newly created
+ variable = variables["totalPrice1"]
+ self.assertEqual(variable.data_type, FormVariableDataTypes.float)
+ self.assertEqual(variable.source, FormVariableSources.user_defined)
+ self.assertEqual(form.price_variable_key, "totalPrice1")
diff --git a/src/openforms/js/compiled-lang/en.json b/src/openforms/js/compiled-lang/en.json
index c0c5ce575d..f0f116a9aa 100644
--- a/src/openforms/js/compiled-lang/en.json
+++ b/src/openforms/js/compiled-lang/en.json
@@ -1383,12 +1383,6 @@
"value": "plus"
}
],
- "BrsG4w": [
- {
- "type": 0,
- "value": "Then the price is"
- }
- ],
"BvRBef": [
{
"type": 0,
@@ -2909,12 +2903,6 @@
"value": "Output mapping"
}
],
- "QAbqb6": [
- {
- "type": 0,
- "value": "Add rule"
- }
- ],
"QDBI2H": [
{
"type": 0,
@@ -4439,12 +4427,6 @@
"value": "Attachments"
}
],
- "eJEu3L": [
- {
- "type": 0,
- "value": "Are you sure you want to delete this rule?"
- }
- ],
"eLDxD5": [
{
"type": 0,
@@ -6665,12 +6647,6 @@
"value": "Plugin configuration: Ogone legacy"
}
],
- "zeXZzX": [
- {
- "type": 0,
- "value": "Use logic rules to determine the price"
- }
- ],
"zwOwrD": [
{
"type": 0,
diff --git a/src/openforms/js/compiled-lang/nl.json b/src/openforms/js/compiled-lang/nl.json
index 6b5d2f2d9f..6e50fb8430 100644
--- a/src/openforms/js/compiled-lang/nl.json
+++ b/src/openforms/js/compiled-lang/nl.json
@@ -1387,12 +1387,6 @@
"value": "plus"
}
],
- "BrsG4w": [
- {
- "type": 0,
- "value": "dan is de prijs"
- }
- ],
"BvRBef": [
{
"type": 0,
@@ -2926,12 +2920,6 @@
"value": "Uitvoerparameters"
}
],
- "QAbqb6": [
- {
- "type": 0,
- "value": "Regel toevoegen"
- }
- ],
"QDBI2H": [
{
"type": 0,
@@ -4461,12 +4449,6 @@
"value": "Bijlagen"
}
],
- "eJEu3L": [
- {
- "type": 0,
- "value": "Weet u zeker dat u deze regel wil verwijderen?"
- }
- ],
"eLDxD5": [
{
"type": 0,
@@ -6693,12 +6675,6 @@
"value": "Plugin-instellingen: Ogone legacy"
}
],
- "zeXZzX": [
- {
- "type": 0,
- "value": "Gebruik prijsregels om de prijs te bepalen"
- }
- ],
"zwOwrD": [
{
"type": 0,
diff --git a/src/openforms/js/components/admin/form_design/PriceLogic.js b/src/openforms/js/components/admin/form_design/PriceLogic.js
index 650c0ffa09..c4c9d1ece2 100644
--- a/src/openforms/js/components/admin/form_design/PriceLogic.js
+++ b/src/openforms/js/components/admin/form_design/PriceLogic.js
@@ -1,30 +1,14 @@
import PropTypes from 'prop-types';
-import React, {useContext, useState} from 'react';
+import React, {useState} from 'react';
import {FormattedMessage, defineMessage, useIntl} from 'react-intl';
-import ButtonContainer from 'components/admin/forms/ButtonContainer';
import Field from 'components/admin/forms/Field';
import Fieldset from 'components/admin/forms/Fieldset';
import FormRow from 'components/admin/forms/FormRow';
-import {NumberInput} from 'components/admin/forms/Inputs';
import Select from 'components/admin/forms/Select';
-import {ValidationErrorContext} from 'components/admin/forms/ValidationErrors';
import VariableSelection from 'components/admin/forms/VariableSelection';
-import {DeleteIcon} from 'components/admin/icons';
import {getTranslatedChoices} from 'utils/i18n';
-import DSLEditorNode from './logic/DSLEditorNode';
-import Trigger from './logic/Trigger';
-import {parseValidationErrors} from './utils';
-
-export const EMPTY_PRICE_RULE = {
- uuid: '',
- _generatedId: '', // consumers should generate this, as it's used for the React key prop if no uuid exists
- form: '',
- jsonLogicTrigger: {},
- price: '',
-};
-
const PRICING_MODES = [
[
'static',
@@ -40,13 +24,6 @@ const PRICING_MODES = [
defaultMessage: 'Use a variable for the price',
}),
],
- [
- 'dynamic',
- defineMessage({
- description: 'dynamic pricing mode label',
- defaultMessage: 'Use logic rules to determine the price',
- }),
- ],
];
const PricingMode = ({mode = 'static', onChange}) => {
@@ -61,57 +38,21 @@ PricingMode.propTypes = {
onChange: PropTypes.func.isRequired,
};
-export const PriceLogic = ({variableKey, rules = [], onChange, onDelete, onAdd, onFieldChange}) => {
- const initialPricingMode =
- variableKey !== '' ? 'variable' : rules.length > 0 ? 'dynamic' : 'static';
+export const PriceLogic = ({variableKey, onFieldChange}) => {
+ const initialPricingMode = variableKey !== '' ? 'variable' : 'static';
const [pricingMode, setPricingMode] = useState(initialPricingMode);
- const validationErrors = parseValidationErrors(useContext(ValidationErrorContext), 'priceRules');
-
- // TODO: de-duplicate/validate duplicate rules (identical triggers?)
-
const onPricingModeChange = event => {
const {value} = event.target;
- const resetVariableKey = () => {
- if (variableKey) {
- onFieldChange({target: {name: 'form.priceVariableKey', value: ''}});
- }
- };
-
- const resetRules = () => {
- if (rules.length > 0) {
- // XXX: iterate in reverse so we delete all rules by removing the last one
- // every time.
- // State updates in event handlers are batched by React, so removing index 0, 1,...
- // in success causes issues since the local `rules` does no langer match the
- // parent component state - draft.priceRules.length !== rules.length.
- // By reversing, we essentially pop the last element every time which
- // works around this.
- const maxIndex = rules.length - 1;
- for (let offset = 0; offset < rules.length; offset++) {
- onDelete(maxIndex - offset);
- }
- }
- };
-
switch (value) {
case 'variable': {
- resetRules();
- break;
- }
- case 'dynamic': {
- // toggle from static to dynamic -> ensure at least one rule exists
- if (rules.length === 0) {
- onAdd();
- }
- resetVariableKey();
break;
}
case 'static': {
- // toggle from dynamic to static -> delete all the rules
- resetRules();
- resetVariableKey();
+ if (variableKey) {
+ onFieldChange({target: {name: 'form.priceVariableKey', value: ''}});
+ }
break;
}
}
@@ -138,22 +79,6 @@ export const PriceLogic = ({variableKey, rules = [], onChange, onDelete, onAdd,
- {rules.map((rule, index) => (
-
- ))}
-
- {pricingMode === 'dynamic' && (
-
-
-
- )}
-
{pricingMode === 'variable' && (
{
- const intl = useIntl();
- const deleteConfirmMessage = intl.formatMessage({
- description: 'Price rule deletion confirm message',
- defaultMessage: 'Are you sure you want to delete this rule?',
- });
- return (
-
-
-
-
-
-
-
-
-
- €
-
-
-
-
-