From cda66c10ff57c9a85dbe4ff413a6d4c986f6ae3e Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Mon, 23 Oct 2023 15:40:54 +0200 Subject: [PATCH 01/10] =?UTF-8?q?=E2=9C=A8=20Add=20the=20ability=20to=20se?= =?UTF-8?q?t=20a=20form=20as=20non=20applicable?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../forms/api/serializers/form_step.py | 2 + .../serializers/logic/action_serializers.py | 1 + src/openforms/forms/constants.py | 1 + .../migrations/0097_formstep_is_applicable.py | 22 ++++++++++ src/openforms/forms/models/form_step.py | 5 +++ .../components/admin/form_design/FormStep.js | 2 + .../admin/form_design/FormStepDefinition.js | 23 +++++++++++ .../admin/form_design/data/steps.js | 1 + .../form_design/logic/actions/Actions.js | 12 ++++++ .../admin/form_design/logic/constants.js | 7 ++++ .../admin/form_design/types/FormStep.js | 1 + src/openforms/submissions/logic/actions.py | 41 ++++++++++++++++++- .../submissions/models/submission_step.py | 12 +++++- 13 files changed, 126 insertions(+), 4 deletions(-) create mode 100644 src/openforms/forms/migrations/0097_formstep_is_applicable.py diff --git a/src/openforms/forms/api/serializers/form_step.py b/src/openforms/forms/api/serializers/form_step.py index a5f6d08d48..d98de0d739 100644 --- a/src/openforms/forms/api/serializers/form_step.py +++ b/src/openforms/forms/api/serializers/form_step.py @@ -108,6 +108,7 @@ class Meta: "name", "internal_name", "url", + "is_applicable", "login_required", "is_reusable", "literals", @@ -123,6 +124,7 @@ class Meta: "name", "internal_name", "url", + "is_applicable", "login_required", "is_reusable", "literals", diff --git a/src/openforms/forms/api/serializers/logic/action_serializers.py b/src/openforms/forms/api/serializers/logic/action_serializers.py index dc7886bf7e..e8c4eb5fba 100644 --- a/src/openforms/forms/api/serializers/logic/action_serializers.py +++ b/src/openforms/forms/api/serializers/logic/action_serializers.py @@ -91,6 +91,7 @@ class LogicActionPolymorphicSerializer(PolymorphicSerializer): str(LogicActionTypes.disable_next): DummySerializer, str(LogicActionTypes.property): LogicPropertyActionSerializer, str(LogicActionTypes.step_not_applicable): DummySerializer, + str(LogicActionTypes.step_applicable): DummySerializer, str(LogicActionTypes.variable): LogicValueActionSerializer, str(LogicActionTypes.fetch_from_service): LogicFetchActionSerializer, str( diff --git a/src/openforms/forms/constants.py b/src/openforms/forms/constants.py index 519117a394..5228179d4d 100644 --- a/src/openforms/forms/constants.py +++ b/src/openforms/forms/constants.py @@ -10,6 +10,7 @@ class LogicActionTypes(models.TextChoices): step_not_applicable = "step-not-applicable", _( "Mark the form step as not-applicable" ) + step_applicable = "step-applicable", _("Mark the form step as applicable") disable_next = "disable-next", _("Disable the next step") property = "property", _("Modify a component property") variable = "variable", _("Set the value of a variable") diff --git a/src/openforms/forms/migrations/0097_formstep_is_applicable.py b/src/openforms/forms/migrations/0097_formstep_is_applicable.py new file mode 100644 index 0000000000..db553d958a --- /dev/null +++ b/src/openforms/forms/migrations/0097_formstep_is_applicable.py @@ -0,0 +1,22 @@ +# Generated by Django 3.2.21 on 2023-10-23 12:44 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("forms", "0096_move_time_component_validators"), + ] + + operations = [ + migrations.AddField( + model_name="formstep", + name="is_applicable", + field=models.BooleanField( + default=True, + help_text="Whether the step is applicable by default.", + verbose_name="is applicable", + ), + ), + ] diff --git a/src/openforms/forms/models/form_step.py b/src/openforms/forms/models/form_step.py index a4752118af..fc707a14a3 100644 --- a/src/openforms/forms/models/form_step.py +++ b/src/openforms/forms/models/form_step.py @@ -61,6 +61,11 @@ class FormStep(OrderedModel): "Leave blank to get value from global configuration." ), ) + is_applicable = models.BooleanField( + _("is applicable"), + default=True, + help_text=_("Whether the step is applicable by default."), + ) order_with_respect_to = "form" diff --git a/src/openforms/js/components/admin/form_design/FormStep.js b/src/openforms/js/components/admin/form_design/FormStep.js index 9b3eeffcfc..e7e023eb81 100644 --- a/src/openforms/js/components/admin/form_design/FormStep.js +++ b/src/openforms/js/components/admin/form_design/FormStep.js @@ -16,6 +16,7 @@ const FormStep = ({data, onEdit, onComponentMutated, onFieldChange, onReplace}) slug, loginRequired, translations, + isApplicable, isReusable, isNew, validationErrors = [], @@ -45,6 +46,7 @@ const FormStep = ({data, onEdit, onComponentMutated, onFieldChange, onReplace}) translations={translations} componentTranslations={componentTranslations} configuration={configuration} + isApplicable={isApplicable} loginRequired={loginRequired} isReusable={isReusable} onFieldChange={onFieldChange} diff --git a/src/openforms/js/components/admin/form_design/FormStepDefinition.js b/src/openforms/js/components/admin/form_design/FormStepDefinition.js index 5935c1462c..6573752346 100644 --- a/src/openforms/js/components/admin/form_design/FormStepDefinition.js +++ b/src/openforms/js/components/admin/form_design/FormStepDefinition.js @@ -41,6 +41,7 @@ const FormStepDefinition = ({ generatedId = '', internalName = '', slug = '', + isApplicable = true, loginRequired = false, isReusable = false, translations = {}, @@ -317,6 +318,28 @@ const FormStepDefinition = ({ /> + + + + } + name="isApplicable" + checked={isApplicable} + onChange={e => + onFieldChange({target: {name: 'isApplicable', value: !isApplicable}}) + } + disabled={langCode !== defaultLang} + /> + + { ); }; +const ActionStepApplicable = ({action, errors, onChange}) => { + return ( + + + + ); +}; + const ActionSetRegistrationBackend = ({action, errors, onChange}) => { return ( @@ -240,6 +248,10 @@ const ActionComponent = ({action, errors, onChange}) => { Component = ActionStepNotApplicable; break; } + case 'step-applicable': { + Component = ActionStepApplicable; + break; + } case 'set-registration-backend': { Component = ActionSetRegistrationBackend; break; diff --git a/src/openforms/js/components/admin/form_design/logic/constants.js b/src/openforms/js/components/admin/form_design/logic/constants.js index 885971aeea..979c94e9be 100644 --- a/src/openforms/js/components/admin/form_design/logic/constants.js +++ b/src/openforms/js/components/admin/form_design/logic/constants.js @@ -91,6 +91,13 @@ const ACTION_TYPES = [ defaultMessage: 'Mark the form step as not-applicable', }), ], + [ + 'step-applicable', + defineMessage({ + description: 'action type "step-applicable" label', + defaultMessage: 'Mark the form step as applicable', + }), + ], [ 'set-registration-backend', defineMessage({ diff --git a/src/openforms/js/components/admin/form_design/types/FormStep.js b/src/openforms/js/components/admin/form_design/types/FormStep.js index 85b515dfba..c21ed0e88d 100644 --- a/src/openforms/js/components/admin/form_design/types/FormStep.js +++ b/src/openforms/js/components/admin/form_design/types/FormStep.js @@ -7,6 +7,7 @@ const FormStep = PropTypes.shape({ name: PropTypes.string, internalName: PropTypes.string, slug: PropTypes.string, + isApplicable: PropTypes.bool, loginRequired: PropTypes.bool, isReusable: PropTypes.bool, url: PropTypes.string, diff --git a/src/openforms/submissions/logic/actions.py b/src/openforms/submissions/logic/actions.py index 8d3c7bee1a..65579a49a1 100644 --- a/src/openforms/submissions/logic/actions.py +++ b/src/openforms/submissions/logic/actions.py @@ -167,13 +167,13 @@ def apply( submission_step_to_modify = execution_state.resolve_step( self.form_step_identifier ) - submission_step_to_modify._is_applicable = False + submission_step_to_modify.is_applicable = False # This clears data in the database to make sure that saved steps which later become # not-applicable don't have old data submission_step_to_modify.data = {} if submission_step_to_modify == step: - step._is_applicable = False + step.is_applicable = False step.data = DirtyData({}) def get_action_log_data( @@ -191,6 +191,42 @@ def get_action_log_data( } +@dataclass +class StepApplicableAction(ActionOperation): + form_step_identifier: str + + @classmethod + def from_action(cls, action: ActionDict) -> "StepApplicableAction": + return cls( + form_step_identifier=action["form_step_uuid"], + ) + + def apply( + self, step: SubmissionStep, configuration: FormioConfigurationWrapper + ) -> None: + execution_state = ( + step.submission.load_execution_state() + ) # typically cached already + submission_step_to_modify = execution_state.resolve_step( + self.form_step_identifier + ) + submission_step_to_modify.is_applicable = True + + def get_action_log_data( + self, + component_map: Dict[str, ComponentMeta], + all_variables: Dict[str, FormVariable], + initial_data: dict, + log_data: dict[str, JSONValue], + ) -> JSONObject: + return { + "type_display": LogicActionTypes.get_label( + LogicActionTypes.step_applicable + ), + "step_name": self.form_step_identifier, + } + + @dataclass class VariableAction(ActionOperation): variable: str @@ -299,6 +335,7 @@ def get_action_log_data(self, *args, **kwargs) -> JSONObject: LogicActionTypes.property: PropertyAction, LogicActionTypes.disable_next: DisableNextAction, LogicActionTypes.step_not_applicable: StepNotApplicableAction, + LogicActionTypes.step_applicable: StepApplicableAction, LogicActionTypes.variable: VariableAction, LogicActionTypes.fetch_from_service: ServiceFetchAction, LogicActionTypes.set_registration_backend: SetRegistrationBackendAction, diff --git a/src/openforms/submissions/models/submission_step.py b/src/openforms/submissions/models/submission_step.py index 3995090359..ac27c6a1a7 100644 --- a/src/openforms/submissions/models/submission_step.py +++ b/src/openforms/submissions/models/submission_step.py @@ -118,7 +118,7 @@ class SubmissionStep(models.Model): # can be modified by logic evaluations/checks _can_submit = True - _is_applicable = True + _is_applicable: bool | None = None _unsaved_data = None @@ -196,7 +196,15 @@ def can_submit(self) -> bool: @property def is_applicable(self) -> bool: - return self._is_applicable + if self._is_applicable is not None: + return self._is_applicable + return self.form_step.is_applicable + + @is_applicable.setter + def is_applicable(self, value: bool) -> None: + if not isinstance(value, bool): + raise ValueError(f"'is_applicable' expects a boolean value, got {value}") + self._is_applicable = value def reset(self): self.data = {} From e1765dbcc4fc19c30f9813bfb19d5d0417f442c8 Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Mon, 23 Oct 2023 16:33:49 +0200 Subject: [PATCH 02/10] =?UTF-8?q?=E2=9C=85=20Add=20basic=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../tests/form_logic/test_modify_step.py | 68 +++++++++++++++++++ .../tests/test_submission_steps_state.py | 15 ++++ 2 files changed, 83 insertions(+) diff --git a/src/openforms/submissions/tests/form_logic/test_modify_step.py b/src/openforms/submissions/tests/form_logic/test_modify_step.py index be8d9223a8..f140a875da 100644 --- a/src/openforms/submissions/tests/form_logic/test_modify_step.py +++ b/src/openforms/submissions/tests/form_logic/test_modify_step.py @@ -138,6 +138,74 @@ def test_step_not_applicable(self): self.assertFalse(updated_step_2.is_applicable) + def test_step_applicable(self): + form = FormFactory.create() + step1 = FormStepFactory.create( + form=form, + form_definition__configuration={ + "components": [ + { + "type": "number", + "key": "age", + } + ] + }, + ) + step2 = FormStepFactory.create( + form=form, + form_definition__configuration={ + "components": [ + { + "type": "textfield", + "key": "driverId", + } + ] + }, + is_applicable=False, + ) + FormLogicFactory.create( + form=form, + json_logic_trigger={ + "<": [ + {"var": "age"}, + 18, + ] + }, + actions=[ + { + "form_step_uuid": f"{step2.uuid}", + "action": { + "name": "Step is applicable", + "type": "step-applicable", + }, + } + ], + ) + + submission = SubmissionFactory.create(form=form) + submission_step_1 = SubmissionStepFactory.create( + submission=submission, + form_step=step1, + data={"age": 16}, + ) + # not saved in DB! + submission_step_2 = SubmissionStepFactory.build( + submission=submission, + form_step=step2, + ) + + self.assertTrue(submission_step_1.is_applicable) + # _is_applicable = None by default -> should look at form_step.is_applicable: + self.assertFalse(submission_step_2.is_applicable) + + evaluate_form_logic(submission, submission_step_1, submission.data) + submission_state = submission.load_execution_state() + updated_step_2 = submission_state.get_submission_step( + form_step_uuid=str(step2.uuid) + ) + + self.assertTrue(updated_step_2.is_applicable) + def test_date_trigger(self): form = FormFactory.create() step = FormStepFactory.create( diff --git a/src/openforms/submissions/tests/test_submission_steps_state.py b/src/openforms/submissions/tests/test_submission_steps_state.py index 65a64a15f3..149f61ca9b 100644 --- a/src/openforms/submissions/tests/test_submission_steps_state.py +++ b/src/openforms/submissions/tests/test_submission_steps_state.py @@ -73,3 +73,18 @@ def test_step_completed(self): self.assertTrue(steps[0].completed) self.assertTrue(steps[1].completed) + + def test_step_set_applicable(self): + form = FormFactory.create() + step = FormStepFactory.create(form=form) + submission = SubmissionFactory.create(form=form) + submission_step = SubmissionStepFactory.create( + form_step=step, submission=submission, data={} + ) + + self.assertIsNone(submission_step._is_applicable) + self.assertTrue(submission_step.is_applicable) + submission_step.is_applicable = False + self.assertFalse(submission_step.is_applicable) + with self.assertRaises(ValueError): + submission_step.is_applicable = None From ec64ae82834750b2d14b4b023f56b62bc1733609 Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Mon, 23 Oct 2023 17:07:15 +0200 Subject: [PATCH 03/10] =?UTF-8?q?=E2=9C=A8=20Ensure=20first=20step=20is=20?= =?UTF-8?q?applicable?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/openforms/forms/models/form_step.py | 9 +++++++++ src/openforms/forms/tests/test_models.py | 7 +++++++ .../js/components/admin/form_design/FormStep.js | 3 +++ .../components/admin/form_design/FormStepDefinition.js | 3 ++- 4 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/openforms/forms/models/form_step.py b/src/openforms/forms/models/form_step.py index fc707a14a3..bf04dad122 100644 --- a/src/openforms/forms/models/form_step.py +++ b/src/openforms/forms/models/form_step.py @@ -1,5 +1,6 @@ import uuid +from django.core.exceptions import ValidationError from django.db import models from django.utils.translation import gettext_lazy as _ @@ -107,3 +108,11 @@ def delete(self, *args, **kwargs): def iter_components(self, recursive=True, **kwargs): yield from self.form_definition.iter_components(recursive=recursive, **kwargs) + + def clean(self) -> None: + if not self.is_applicable and self.order == 0: + raise ValidationError( + {"is_applicable": _("First form step must be applicable.")}, + code="invalid", + ) + return super().clean() diff --git a/src/openforms/forms/tests/test_models.py b/src/openforms/forms/tests/test_models.py index d810b1da50..357482d2dd 100644 --- a/src/openforms/forms/tests/test_models.py +++ b/src/openforms/forms/tests/test_models.py @@ -417,6 +417,13 @@ def test_str_no_relation(self): step = FormStep() self.assertEqual(str(step), "FormStep object (None)") + def test_clean(self): + step = FormStepFactory.create( + order=0, + is_applicable=False, + ) + self.assertRaises(ValidationError, step.clean) + class FormLogicTests(TestCase): def test_block_form_logic_trigger_step_other_form(self): diff --git a/src/openforms/js/components/admin/form_design/FormStep.js b/src/openforms/js/components/admin/form_design/FormStep.js index e7e023eb81..f77a714fcd 100644 --- a/src/openforms/js/components/admin/form_design/FormStep.js +++ b/src/openforms/js/components/admin/form_design/FormStep.js @@ -9,6 +9,7 @@ import TYPES from './types'; const FormStep = ({data, onEdit, onComponentMutated, onFieldChange, onReplace}) => { const { _generatedId, + index, configuration, formDefinition, name, @@ -22,6 +23,7 @@ const FormStep = ({data, onEdit, onComponentMutated, onFieldChange, onReplace}) validationErrors = [], componentTranslations, } = data; + console.log(_generatedId); const previousFormDefinition = usePrevious(formDefinition); let forceBuilderUpdate = false; if (previousFormDefinition && previousFormDefinition != formDefinition) { @@ -40,6 +42,7 @@ const FormStep = ({data, onEdit, onComponentMutated, onFieldChange, onReplace}) return ( onFieldChange({target: {name: 'isApplicable', value: !isApplicable}}) } - disabled={langCode !== defaultLang} + disabled={index === 0 || langCode !== defaultLang} // First step can't be n/a by default /> From 47176f201ba28b055276600988f321479e5a662d Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Tue, 24 Oct 2023 12:11:14 +0200 Subject: [PATCH 04/10] =?UTF-8?q?=F0=9F=92=84=20Add=20`title`=20prop=20to?= =?UTF-8?q?=20`Field`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../js/components/admin/form_design/FormStepDefinition.js | 1 + src/openforms/js/components/admin/forms/Field.js | 4 +++- src/openforms/js/components/admin/forms/Field.stories.mdx | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/openforms/js/components/admin/form_design/FormStepDefinition.js b/src/openforms/js/components/admin/form_design/FormStepDefinition.js index 158b99e27b..f2aaf5cffb 100644 --- a/src/openforms/js/components/admin/form_design/FormStepDefinition.js +++ b/src/openforms/js/components/admin/form_design/FormStepDefinition.js @@ -324,6 +324,7 @@ const FormStepDefinition = ({ name="isApplicable" errorClassPrefix={'checkbox'} errorClassModifier={'no-padding'} + title={index === 0 ? 'First step must be applicable' : null} > ) : null} -
+
{fieldBox && hasErrors ? ( {formattedErrors} @@ -86,6 +87,7 @@ Field.propTypes = { label: PropTypes.node, children: PropTypes.element.isRequired, helpText: PropTypes.node, + title: PropTypes.string, required: PropTypes.bool, errors: PropTypes.oneOfType([ PropTypes.arrayOf(PropTypes.oneOfType([PropTypes.string, PropTypes.array])), diff --git a/src/openforms/js/components/admin/forms/Field.stories.mdx b/src/openforms/js/components/admin/forms/Field.stories.mdx index e3e6af9e5a..7b8c2e9677 100644 --- a/src/openforms/js/components/admin/forms/Field.stories.mdx +++ b/src/openforms/js/components/admin/forms/Field.stories.mdx @@ -25,6 +25,7 @@ export const Template = args => ( name: 'Field', label: 'Input field', helpText: 'Lorem ipsum', + title: 'Title', errorClassPrefix: '', errorClassModifier: '', }} From 28d5bb69ede5940ed5d220fe7441253fac890b23 Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Tue, 24 Oct 2023 12:16:14 +0200 Subject: [PATCH 05/10] =?UTF-8?q?=E2=9C=85=20Add=20another=20test=20for=20?= =?UTF-8?q?coverage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/openforms/forms/tests/test_models.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/openforms/forms/tests/test_models.py b/src/openforms/forms/tests/test_models.py index 357482d2dd..b9621f259e 100644 --- a/src/openforms/forms/tests/test_models.py +++ b/src/openforms/forms/tests/test_models.py @@ -418,11 +418,18 @@ def test_str_no_relation(self): self.assertEqual(str(step), "FormStep object (None)") def test_clean(self): - step = FormStepFactory.create( + step_raises = FormStepFactory.create( order=0, is_applicable=False, ) - self.assertRaises(ValidationError, step.clean) + step_ok = FormStepFactory.create( + order=0, + is_applicable=True, + ) + with self.subTest("clean raises"): + self.assertRaises(ValidationError, step_raises.clean) + with self.subTest("clean does not raises"): + step_ok.clean() class FormLogicTests(TestCase): From 27982f39b2526298823a28e876706c1a5458a56d Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Wed, 25 Oct 2023 10:53:49 +0200 Subject: [PATCH 06/10] =?UTF-8?q?=E2=8F=AA=EF=B8=8F=20Revert=20usage=20of?= =?UTF-8?q?=20`title`=20HTML=20attribute?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../js/components/admin/form_design/FormStepDefinition.js | 1 - src/openforms/js/components/admin/forms/Field.js | 4 +--- src/openforms/js/components/admin/forms/Field.stories.mdx | 1 - 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/openforms/js/components/admin/form_design/FormStepDefinition.js b/src/openforms/js/components/admin/form_design/FormStepDefinition.js index f2aaf5cffb..158b99e27b 100644 --- a/src/openforms/js/components/admin/form_design/FormStepDefinition.js +++ b/src/openforms/js/components/admin/form_design/FormStepDefinition.js @@ -324,7 +324,6 @@ const FormStepDefinition = ({ name="isApplicable" errorClassPrefix={'checkbox'} errorClassModifier={'no-padding'} - title={index === 0 ? 'First step must be applicable' : null} > ) : null} -
+
{fieldBox && hasErrors ? ( {formattedErrors} @@ -87,7 +86,6 @@ Field.propTypes = { label: PropTypes.node, children: PropTypes.element.isRequired, helpText: PropTypes.node, - title: PropTypes.string, required: PropTypes.bool, errors: PropTypes.oneOfType([ PropTypes.arrayOf(PropTypes.oneOfType([PropTypes.string, PropTypes.array])), diff --git a/src/openforms/js/components/admin/forms/Field.stories.mdx b/src/openforms/js/components/admin/forms/Field.stories.mdx index 7b8c2e9677..e3e6af9e5a 100644 --- a/src/openforms/js/components/admin/forms/Field.stories.mdx +++ b/src/openforms/js/components/admin/forms/Field.stories.mdx @@ -25,7 +25,6 @@ export const Template = args => ( name: 'Field', label: 'Input field', helpText: 'Lorem ipsum', - title: 'Title', errorClassPrefix: '', errorClassModifier: '', }} From 3534fb32fdd8dcd251373902a238d06d0286a3d0 Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Thu, 26 Oct 2023 17:19:45 +0200 Subject: [PATCH 07/10] =?UTF-8?q?=E2=9C=A8=20Apply=20feedback?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../forms/api/serializers/form_step.py | 3 ++ src/openforms/forms/api/validators.py | 13 +++++++ .../forms/tests/test_api_formsteps.py | 39 +++++++++++++++++++ src/openforms/forms/tests/test_models.py | 7 +++- .../components/admin/form_design/FormStep.js | 1 - 5 files changed, 61 insertions(+), 2 deletions(-) diff --git a/src/openforms/forms/api/serializers/form_step.py b/src/openforms/forms/api/serializers/form_step.py index d98de0d739..56061c0a75 100644 --- a/src/openforms/forms/api/serializers/form_step.py +++ b/src/openforms/forms/api/serializers/form_step.py @@ -12,6 +12,7 @@ from ...models import FormDefinition, FormStep from ...validators import validate_no_duplicate_keys_across_steps from .button_text import ButtonTextSerializer +from ..validators import FormStepIsApplicableIfFirstValidator class FormStepLiteralsSerializer(serializers.Serializer): @@ -54,6 +55,7 @@ class Meta: "index", "literals", "url", + "is_applicable", ) extra_kwargs = { "uuid": { @@ -140,6 +142,7 @@ class Meta: "read_only": True, }, } + validators = [FormStepIsApplicableIfFirstValidator()] def create(self, validated_data): validated_data["form"] = self.context["form"] diff --git a/src/openforms/forms/api/validators.py b/src/openforms/forms/api/validators.py index 3f18fa2bb4..350d71e896 100644 --- a/src/openforms/forms/api/validators.py +++ b/src/openforms/forms/api/validators.py @@ -190,6 +190,19 @@ def __call__(self, attrs: dict, serializer: serializers.Serializer): ) +class FormStepIsApplicableIfFirstValidator: + def __call__(self, attrs: dict): + if not attrs.get("is_applicable", True) and attrs.get("order") == 0: + raise serializers.ValidationError( + { + "is_applicable": serializers.ErrorDetail( + _("First form step must be applicable."), + code="invalid", + ), + } + ) + + def validate_template_expressions(configuration: JSONObject) -> None: """ Validate that any template expressions in supported properties are correct. diff --git a/src/openforms/forms/tests/test_api_formsteps.py b/src/openforms/forms/tests/test_api_formsteps.py index 193af20435..59a299f2fd 100644 --- a/src/openforms/forms/tests/test_api_formsteps.py +++ b/src/openforms/forms/tests/test_api_formsteps.py @@ -1092,3 +1092,42 @@ def test_update_with_translations_validate_literals(self, _mock): for error in expected: with self.subTest(field=error["name"], code=error["code"]): self.assertIn(error, invalid_params) + + +class FormStepsAPIApplicabilityTests(APITestCase): + def setUp(self): + super().setUp() + + self.user = UserFactory.create() + self.form = FormFactory.create() + self.form_definition = FormDefinitionFactory.create() + self.client.force_authenticate(user=self.user) + + def test_create_form_step_not_applicable_as_first_unsucessful(self): + self.user.user_permissions.add(Permission.objects.get(codename="change_form")) + self.user.is_staff = True + self.user.save() + url = reverse( + "api:form-steps-list", kwargs={"form_uuid_or_slug": self.form.uuid} + ) + + form_detail_url = reverse( + "api:formdefinition-detail", + kwargs={"uuid": self.form_definition.uuid}, + ) + data = { + "formDefinition": f"http://testserver{form_detail_url}", + "index": 0, + "isApplicable": False, + } + response = self.client.post(url, data=data) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual( + response.json()["invalidParams"][0], + { + "name": "isApplicable", + "code": "invalid", + "reason": "First form step must be applicable.", + }, + ) diff --git a/src/openforms/forms/tests/test_models.py b/src/openforms/forms/tests/test_models.py index b9621f259e..da11720415 100644 --- a/src/openforms/forms/tests/test_models.py +++ b/src/openforms/forms/tests/test_models.py @@ -426,10 +426,15 @@ def test_clean(self): order=0, is_applicable=True, ) + step_ok_order_1 = FormStepFactory.create( + order=1, + is_applicable=False, + ) with self.subTest("clean raises"): self.assertRaises(ValidationError, step_raises.clean) - with self.subTest("clean does not raises"): + with self.subTest("clean does not raise"): step_ok.clean() + step_ok_order_1.clean() class FormLogicTests(TestCase): diff --git a/src/openforms/js/components/admin/form_design/FormStep.js b/src/openforms/js/components/admin/form_design/FormStep.js index f77a714fcd..4dd4202c62 100644 --- a/src/openforms/js/components/admin/form_design/FormStep.js +++ b/src/openforms/js/components/admin/form_design/FormStep.js @@ -23,7 +23,6 @@ const FormStep = ({data, onEdit, onComponentMutated, onFieldChange, onReplace}) validationErrors = [], componentTranslations, } = data; - console.log(_generatedId); const previousFormDefinition = usePrevious(formDefinition); let forceBuilderUpdate = false; if (previousFormDefinition && previousFormDefinition != formDefinition) { From 02b6724a364b186323047208716bf597b75d9f83 Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Fri, 27 Oct 2023 09:24:02 +0200 Subject: [PATCH 08/10] =?UTF-8?q?=F0=9F=90=9B=20Fix=20OpenAPI=20spec?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/openapi.yaml | 11 +++++++++++ src/openforms/forms/api/serializers/form_step.py | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/openapi.yaml b/src/openapi.yaml index 08e06d7b43..8bc9fa9a28 100644 --- a/src/openapi.yaml +++ b/src/openapi.yaml @@ -7422,6 +7422,9 @@ components: type: string format: uri readOnly: true + isApplicable: + type: boolean + description: Whether the step is applicable by default. loginRequired: type: boolean readOnly: true @@ -7763,6 +7766,7 @@ components: disable-next: '#/components/schemas/LogicActionPolymorphicGenericObject' property: '#/components/schemas/LogicActionPolymorphicLogicPropertyAction' step-not-applicable: '#/components/schemas/LogicActionPolymorphicGenericObject' + step-applicable: '#/components/schemas/LogicActionPolymorphicGenericObject' variable: '#/components/schemas/LogicActionPolymorphicLogicValueAction' fetch-from-service: '#/components/schemas/LogicActionPolymorphicLogicFetchAction' set-registration-backend: '#/components/schemas/LogicActionPolymorphicLogicSetRegistrationBackendAction' @@ -7816,6 +7820,7 @@ components: LogicActionPolymorphicSharedTypeEnum: enum: - step-not-applicable + - step-applicable - disable-next - property - variable @@ -8003,6 +8008,9 @@ components: url: type: string format: uri + isApplicable: + type: boolean + description: Whether the step is applicable by default. required: - formDefinition - index @@ -8421,6 +8429,9 @@ components: type: string format: uri readOnly: true + isApplicable: + type: boolean + description: Whether the step is applicable by default. loginRequired: type: boolean readOnly: true diff --git a/src/openforms/forms/api/serializers/form_step.py b/src/openforms/forms/api/serializers/form_step.py index 56061c0a75..f66bb439ee 100644 --- a/src/openforms/forms/api/serializers/form_step.py +++ b/src/openforms/forms/api/serializers/form_step.py @@ -11,8 +11,8 @@ from ...models import FormDefinition, FormStep from ...validators import validate_no_duplicate_keys_across_steps -from .button_text import ButtonTextSerializer from ..validators import FormStepIsApplicableIfFirstValidator +from .button_text import ButtonTextSerializer class FormStepLiteralsSerializer(serializers.Serializer): From 01261436d0801b7d717b8dc776d744b4932c4cb7 Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Fri, 27 Oct 2023 11:52:49 +0200 Subject: [PATCH 09/10] =?UTF-8?q?=E2=9C=A8=20Apply=20new=20feedback?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../forms/tests/test_api_formsteps.py | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/src/openforms/forms/tests/test_api_formsteps.py b/src/openforms/forms/tests/test_api_formsteps.py index 59a299f2fd..7312bef000 100644 --- a/src/openforms/forms/tests/test_api_formsteps.py +++ b/src/openforms/forms/tests/test_api_formsteps.py @@ -1095,25 +1095,19 @@ def test_update_with_translations_validate_literals(self, _mock): class FormStepsAPIApplicabilityTests(APITestCase): - def setUp(self): - super().setUp() - - self.user = UserFactory.create() - self.form = FormFactory.create() - self.form_definition = FormDefinitionFactory.create() - self.client.force_authenticate(user=self.user) - def test_create_form_step_not_applicable_as_first_unsucessful(self): - self.user.user_permissions.add(Permission.objects.get(codename="change_form")) - self.user.is_staff = True - self.user.save() - url = reverse( - "api:form-steps-list", kwargs={"form_uuid_or_slug": self.form.uuid} - ) + user = UserFactory.create() + form = FormFactory.create() + form_definition = FormDefinitionFactory.create() + self.client.force_authenticate(user=user) + user.user_permissions.add(Permission.objects.get(codename="change_form")) + user.is_staff = True + user.save() + url = reverse("api:form-steps-list", kwargs={"form_uuid_or_slug": form.uuid}) form_detail_url = reverse( "api:formdefinition-detail", - kwargs={"uuid": self.form_definition.uuid}, + kwargs={"uuid": form_definition.uuid}, ) data = { "formDefinition": f"http://testserver{form_detail_url}", @@ -1131,3 +1125,12 @@ def test_create_form_step_not_applicable_as_first_unsucessful(self): "reason": "First form step must be applicable.", }, ) + + data = { + "formDefinition": f"http://testserver{form_detail_url}", + "index": 0, + "isApplicable": True, + } + response = self.client.post(url, data=data) + + self.assertEqual(response.status_code, status.HTTP_201_CREATED) From badfe8407e34bea3d796e934cbf378a2b616b178 Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Fri, 27 Oct 2023 15:27:45 +0200 Subject: [PATCH 10/10] =?UTF-8?q?=E2=8F=AA=EF=B8=8F=20Do=20not=20raise=20o?= =?UTF-8?q?n=20property=20setter?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/openforms/submissions/models/submission_step.py | 2 -- src/openforms/submissions/tests/test_submission_steps_state.py | 2 -- 2 files changed, 4 deletions(-) diff --git a/src/openforms/submissions/models/submission_step.py b/src/openforms/submissions/models/submission_step.py index ac27c6a1a7..e457a2d46d 100644 --- a/src/openforms/submissions/models/submission_step.py +++ b/src/openforms/submissions/models/submission_step.py @@ -202,8 +202,6 @@ def is_applicable(self) -> bool: @is_applicable.setter def is_applicable(self, value: bool) -> None: - if not isinstance(value, bool): - raise ValueError(f"'is_applicable' expects a boolean value, got {value}") self._is_applicable = value def reset(self): diff --git a/src/openforms/submissions/tests/test_submission_steps_state.py b/src/openforms/submissions/tests/test_submission_steps_state.py index 149f61ca9b..6baf240ba0 100644 --- a/src/openforms/submissions/tests/test_submission_steps_state.py +++ b/src/openforms/submissions/tests/test_submission_steps_state.py @@ -86,5 +86,3 @@ def test_step_set_applicable(self): self.assertTrue(submission_step.is_applicable) submission_step.is_applicable = False self.assertFalse(submission_step.is_applicable) - with self.assertRaises(ValueError): - submission_step.is_applicable = None