From 408c7ec28eae01377b2b3e27d90b8014463c806a Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Tue, 10 Dec 2024 16:47:51 +0100 Subject: [PATCH] :fire: Delete migration test code The tests rely too much on the real models, which breaks other PRs that add additional fields. Refactoring the test code to freeze the price calculation is not reasonable, because it relies on the logic engine evaluation, which can't work with historic models and requires access to the real models and their models. We'll have to trust that the tests were sufficient when the code cleanup was initially authored :fingers_crossed: --- src/openforms/forms/tests/test_migrations.py | 98 +------------------- src/openforms/submissions/pricing.py | 37 +------- 2 files changed, 5 insertions(+), 130 deletions(-) diff --git a/src/openforms/forms/tests/test_migrations.py b/src/openforms/forms/tests/test_migrations.py index 95676f0d8a..fea9e26022 100644 --- a/src/openforms/forms/tests/test_migrations.py +++ b/src/openforms/forms/tests/test_migrations.py @@ -2,37 +2,10 @@ 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" @@ -53,8 +26,8 @@ def setUpBeforeMigration(self, apps: StateApps): 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( + FormStep.objects.create(form=form, form_definition=fd, order=1) + FormVariable.objects.create( form=form, name="Amount", key="amount", @@ -73,73 +46,6 @@ def setUpBeforeMigration(self, apps: StateApps): 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() diff --git a/src/openforms/submissions/pricing.py b/src/openforms/submissions/pricing.py index 7099c69016..48bccf8a6e 100644 --- a/src/openforms/submissions/pricing.py +++ b/src/openforms/submissions/pricing.py @@ -2,12 +2,9 @@ import decimal import logging -import warnings from decimal import Decimal from typing import TYPE_CHECKING -from json_logic import jsonLogic - from openforms.logging import logevent from openforms.typing import JSONValue @@ -72,40 +69,12 @@ def get_submission_price(submission: Submission) -> Decimal: return price # - # 2. Check if there are any logic rules defined that match. - # - data = submission.data - # test the rules one by one, if relevant - price_rules = form.formpricelogic_set.all() - if price_rules: - warnings.warn( - "Price logic rules are no longer supported. The left-over implementation " - "only exists for migration testing purposes.", - RuntimeWarning, - ) - for rule in price_rules: - # logic does not match, no point in bothering - if not jsonLogic(rule.json_logic_trigger, data): - continue - logger.debug( - "Price for submission %s calculated using logic trigger %d: %r", - submission.uuid, - rule.id, - rule.json_logic_trigger, - ) - # first logic match wins - # TODO: validate on API/backend/frontend that logic triggers must be unique for - # a form - return rule.price - - # - # 3. More specific modes didn't produce anything, fall back to the linked product - # price. + # 2. No price stored in a variable, fall back to the linked product price. # + assert price is None logger.debug( - "Falling back to product price for submission %s after trying %d price rules.", + "Falling back to product price for submission %s, there is no price variable.", submission.uuid, - len(price_rules), ) return form.product.price