diff --git a/forms_pro/api/form.py b/forms_pro/api/form.py index 80700c4..730ef99 100644 --- a/forms_pro/api/form.py +++ b/forms_pro/api/form.py @@ -5,6 +5,7 @@ from forms_pro.api.user import get_user from forms_pro.forms_pro.doctype.form.form import Form +from forms_pro.utils.constants import FORMS_PRO_SYSTEM_FIELDNAMES, UNSUPPORTED_FRAPPE_FIELDTYPES class FormSharedWithResponse(BaseModel): @@ -36,9 +37,11 @@ def is_login_required(route: str) -> bool: return bool(login_enabled) -@frappe.whitelist(allow_guest=True) +@frappe.whitelist(allow_guest=True) # nosemgrep: frappe-semgrep-rules.rules.security.guest-whitelisted-method def get_form_by_route(route: str) -> dict: form_id = frappe.db.get_value("Form", {"route": route}, pluck="name") + if not form_id: + frappe.throw(_("Form not found"), frappe.DoesNotExistError) return get_form(form_id) @@ -230,25 +233,13 @@ def get_doctype_list() -> list[str]: ) -@frappe.whitelist(allow_guest=True) -def get_doctype_fields(doctype: str) -> dict: +@frappe.whitelist(allow_guest=True) # nosemgrep: frappe-semgrep-rules.rules.security.guest-whitelisted-method +def get_doctype_fields(doctype: str) -> list: doctype = frappe.get_doc("DocType", doctype) - fields = doctype.fields - - FIELDTYPES_TO_REMOVE = [ - "Section Break", - "HTML", - "Button", - "Column Break", - "Tab Break", - "Barcode", - "Dynamic Link", - "Fold", + fields = [ + field + for field in doctype.fields + if field.fieldtype not in UNSUPPORTED_FRAPPE_FIELDTYPES + and field.fieldname not in FORMS_PRO_SYSTEM_FIELDNAMES ] - - FIELDS_TO_REMOVE = ["fp_submission_status", "fp_linked_form"] - - fields = [field for field in fields if field.fieldtype not in FIELDTYPES_TO_REMOVE] - fields = [field for field in fields if field.fieldname not in FIELDS_TO_REMOVE] - return fields diff --git a/forms_pro/api/submission.py b/forms_pro/api/submission.py index 859389d..73d303e 100644 --- a/forms_pro/api/submission.py +++ b/forms_pro/api/submission.py @@ -1,3 +1,4 @@ +import json from datetime import datetime from typing import Any @@ -32,11 +33,98 @@ def parse_datetime(cls, v: Any) -> datetime: raise ValueError(f"Invalid datetime value: {v}") -@frappe.whitelist(allow_guest=True) +def _coerce_field_value(value: Any, fieldtype: str) -> Any: + """Coerce a submitted value to its comparable type, matching frontend conditionals.ts logic.""" + if value is None or value == "": + return None + # Matches isBoolean types in the frontend registry (Switch, Checkbox) + if fieldtype in ("Switch", "Checkbox"): + return bool(value) + if fieldtype == "Number": + try: + return float(value) + except (TypeError, ValueError): + return None + return str(value) + + +def _evaluate_conditions(conditions: list[dict], form_data: dict, field_map: dict) -> bool: + """Evaluate AND-joined conditions against submitted form data.""" + for condition in conditions: + fieldname = condition.get("fieldname") + operator = condition.get("operator") + expected = condition.get("value") + field = field_map.get(fieldname) + if not field: + return False + actual = _coerce_field_value(form_data.get(fieldname), field.fieldtype) + # Coerce the condition's expected value through the same function so + # types match — avoids str(True)=="True" vs "true" mismatches and + # str(3.0)=="3.0" vs "3" mismatches for Number fields. + expected_coerced = _coerce_field_value(expected, field.fieldtype) + if operator == "Is" and actual != expected_coerced: + return False + if operator == "Is Not" and actual == expected_coerced: + return False + if operator == "Is Empty" and actual is not None and actual != "": + return False + if operator == "Is Not Empty" and (actual is None or actual == ""): + return False + return True + + +def _validate_form_response(form: "Form", form_data: dict) -> None: + """ + Validate required and conditionally-required fields server-side. + + Mirrors the shouldFieldBeVisible / shouldFieldBeRequired logic in + frontend/src/utils/conditionals.ts so that direct API calls cannot + bypass frontend validation. + """ + field_map = {f.fieldname: f for f in form.fields} + errors: list[str] = [] + + for field in form.fields: + is_visible = not field.hidden + is_required = bool(field.reqd) + + for other in form.fields: + if not other.conditional_logic: + continue + try: + logic = json.loads(other.conditional_logic) + except (json.JSONDecodeError, TypeError): + continue + + if logic.get("target_field") != field.fieldname: + continue + + conditions_met = _evaluate_conditions(logic.get("conditions", []), form_data, field_map) + if conditions_met: + action = logic.get("action") + if action == "Show Field": + is_visible = True + elif action == "Hide Field": + is_visible = False + elif action == "Require Answer": + is_required = True + + if not is_visible: + continue + + value = form_data.get(field.fieldname) + if is_required and (value is None or value == ""): + errors.append(_("{0} is required").format(field.label)) + + if errors: + frappe.throw("\n".join(errors), frappe.ValidationError) + + +@frappe.whitelist(allow_guest=True) # nosemgrep: frappe-semgrep-rules.rules.security.guest-whitelisted-method def submit_form_response( form_id: str, form_data: list[dict], - submission_status: SubmissionStatus = SubmissionStatus.SUBMITTED, + submission_status: str = SubmissionStatus.SUBMITTED.value, ) -> str: """ Submit a form response @@ -49,6 +137,14 @@ def submit_form_response( Returns: The name of the submission """ + try: + status = SubmissionStatus(submission_status) + except ValueError: + frappe.throw( + _("Invalid submission status: {0}").format(submission_status), + frappe.ValidationError, + ) + try: form: Form = frappe.get_doc("Form", form_id) linked_doctype = form.linked_doctype @@ -59,12 +155,22 @@ def submit_form_response( frappe.PermissionError, ) + form_data_dict = {item["fieldname"]: item["value"] for item in form_data} + + # Whitelist to declared form fields only — prevents injecting system fields + # like owner, docstatus, etc. into the linked DocType. + allowed_fieldnames = {f.fieldname for f in form.fields} + form_data_dict = {k: v for k, v in form_data_dict.items() if k in allowed_fieldnames} + + if status == SubmissionStatus.SUBMITTED: + _validate_form_response(form, form_data_dict) + submission = frappe.new_doc(linked_doctype) - for data in form_data: - submission.set(data["fieldname"], data["value"]) + for fieldname, value in form_data_dict.items(): + submission.set(fieldname, value) submission.fp_linked_form = form_id - submission.fp_submission_status = submission_status.value + submission.fp_submission_status = status.value submission.insert(ignore_permissions=True, ignore_mandatory=True) # Share the submission with the owner diff --git a/forms_pro/forms_pro/doctype/form_field/form_field.py b/forms_pro/forms_pro/doctype/form_field/form_field.py index f347b6d..209b21d 100644 --- a/forms_pro/forms_pro/doctype/form_field/form_field.py +++ b/forms_pro/forms_pro/doctype/form_field/form_field.py @@ -4,6 +4,32 @@ # import frappe from frappe.model.document import Document +# Maps Forms Pro field types to Frappe CustomField fieldtypes. +# When adding a new field type: +# 1. Add the option to form_field.json → DF.Literal regenerates automatically +# 2. Add an entry here +# 3. Add an entry to FIELD_TYPE_DEFINITIONS in frontend/src/config/fieldTypes.ts +FORM_TO_FRAPPE_FIELDTYPE: dict[str, dict] = { + "Attach": {"fieldtype": "Attach"}, + "Data": {"fieldtype": "Data"}, + "Number": {"fieldtype": "Int"}, + "Email": {"fieldtype": "Data", "options": "Email"}, + "Date": {"fieldtype": "Date"}, + "Date Time": {"fieldtype": "Datetime"}, + "Date Range": {"fieldtype": "Data"}, + "Time Picker": {"fieldtype": "Time"}, + "Password": {"fieldtype": "Password"}, + "Select": {"fieldtype": "Select"}, + "Phone": {"fieldtype": "Phone"}, + "Switch": {"fieldtype": "Check"}, + "Textarea": {"fieldtype": "Text"}, + "Text Editor": {"fieldtype": "Text Editor"}, + "Link": {"fieldtype": "Link"}, + "Checkbox": {"fieldtype": "Check"}, + "Rating": {"fieldtype": "Rating"}, + "Table": {"fieldtype": "Table"}, +} + class FormField(Document): # begin: auto-generated types @@ -49,30 +75,13 @@ class FormField(Document): @property def to_frappe_field(self) -> dict: - _fieldtype = self.fieldtype - - if self.fieldtype == "Email": - _fieldtype = "Data" - self.options = "Email" - elif self.fieldtype == "Number": - _fieldtype = "Int" - elif self.fieldtype == "Date Time": - _fieldtype = "Datetime" - elif self.fieldtype == "Date Range": - _fieldtype = "Data" - elif self.fieldtype == "Time Picker": - _fieldtype = "Time" - elif self.fieldtype == "Switch" or self.fieldtype == "Checkbox": - _fieldtype = "Check" - elif self.fieldtype == "Textarea": - _fieldtype = "Text" - + mapping = FORM_TO_FRAPPE_FIELDTYPE.get(self.fieldtype, {}) return { "fieldname": self.fieldname, - "fieldtype": _fieldtype, + "fieldtype": mapping.get("fieldtype", self.fieldtype), "label": self.label, "reqd": self.reqd, - "options": self.options, + "options": mapping.get("options", self.options), "description": self.description, "default": self.default, } diff --git a/forms_pro/tests/test_submission_validation.py b/forms_pro/tests/test_submission_validation.py new file mode 100644 index 0000000..9aaff25 --- /dev/null +++ b/forms_pro/tests/test_submission_validation.py @@ -0,0 +1,215 @@ +# Copyright (c) 2025, harsh@buildwithhussain.com and contributors +# For license information, please see license.txt + +import json +import unittest +from types import SimpleNamespace + +from forms_pro.api.submission import _coerce_field_value, _evaluate_conditions, _validate_form_response +from forms_pro.utils.constants import FORMS_PRO_SYSTEM_FIELDNAMES, UNSUPPORTED_FRAPPE_FIELDTYPES +from forms_pro.utils.form_generator import LINKED_FORM_FIELDOPTIONS, SUBMISSION_STATUS_FIELDOPTIONS + + +def _field(fieldname, fieldtype="Data", label=None, reqd=0, hidden=0, conditional_logic=None): + """Build a minimal field stub matching what _validate_form_response expects.""" + return SimpleNamespace( + fieldname=fieldname, + fieldtype=fieldtype, + label=label or fieldname.replace("_", " ").title(), + reqd=reqd, + hidden=hidden, + conditional_logic=conditional_logic, + ) + + +def _form(*fields): + """Build a minimal form stub.""" + return SimpleNamespace(fields=list(fields)) + + +class TestCoerceFieldValue(unittest.TestCase): + def test_empty_string_returns_none(self): + self.assertIsNone(_coerce_field_value("", "Data")) + + def test_none_returns_none(self): + self.assertIsNone(_coerce_field_value(None, "Data")) + + def test_switch_coerces_to_bool(self): + self.assertTrue(_coerce_field_value(1, "Switch")) + self.assertFalse(_coerce_field_value(0, "Switch")) + self.assertFalse(_coerce_field_value("", "Switch")) + + def test_checkbox_coerces_to_bool(self): + self.assertTrue(_coerce_field_value("true", "Checkbox")) + self.assertFalse(_coerce_field_value(0, "Checkbox")) + + def test_number_coerces_to_float(self): + self.assertEqual(_coerce_field_value("42", "Number"), 42.0) + self.assertEqual(_coerce_field_value(7, "Number"), 7.0) + self.assertEqual(_coerce_field_value("3.14", "Number"), 3.14) + self.assertEqual(_coerce_field_value(3.14, "Number"), 3.14) + + def test_number_invalid_returns_none(self): + self.assertIsNone(_coerce_field_value("abc", "Number")) + + def test_data_returns_string(self): + self.assertEqual(_coerce_field_value("hello", "Data"), "hello") + self.assertEqual(_coerce_field_value(123, "Data"), "123") + + +class TestEvaluateConditions(unittest.TestCase): + def _field_map(self, *fields): + return {f.fieldname: f for f in fields} + + def test_is_operator_match(self): + fields = self._field_map(_field("status", "Data")) + conditions = [{"fieldname": "status", "operator": "Is", "value": "Active"}] + self.assertTrue(_evaluate_conditions(conditions, {"status": "Active"}, fields)) + self.assertFalse(_evaluate_conditions(conditions, {"status": "Inactive"}, fields)) + + def test_is_not_operator(self): + fields = self._field_map(_field("status", "Data")) + conditions = [{"fieldname": "status", "operator": "Is Not", "value": "Active"}] + self.assertTrue(_evaluate_conditions(conditions, {"status": "Inactive"}, fields)) + self.assertFalse(_evaluate_conditions(conditions, {"status": "Active"}, fields)) + + def test_is_empty_operator(self): + fields = self._field_map(_field("note", "Data")) + conditions = [{"fieldname": "note", "operator": "Is Empty", "value": None}] + self.assertTrue(_evaluate_conditions(conditions, {"note": ""}, fields)) + self.assertTrue(_evaluate_conditions(conditions, {"note": None}, fields)) + self.assertFalse(_evaluate_conditions(conditions, {"note": "some text"}, fields)) + + def test_is_not_empty_operator(self): + fields = self._field_map(_field("note", "Data")) + conditions = [{"fieldname": "note", "operator": "Is Not Empty", "value": None}] + self.assertTrue(_evaluate_conditions(conditions, {"note": "hi"}, fields)) + self.assertFalse(_evaluate_conditions(conditions, {"note": ""}, fields)) + + def test_all_conditions_must_pass(self): + """AND logic — all conditions must be true.""" + fields = self._field_map(_field("a", "Data"), _field("b", "Data")) + conditions = [ + {"fieldname": "a", "operator": "Is", "value": "yes"}, + {"fieldname": "b", "operator": "Is", "value": "yes"}, + ] + self.assertTrue(_evaluate_conditions(conditions, {"a": "yes", "b": "yes"}, fields)) + self.assertFalse(_evaluate_conditions(conditions, {"a": "yes", "b": "no"}, fields)) + + def test_unknown_field_returns_false(self): + conditions = [{"fieldname": "nonexistent", "operator": "Is", "value": "x"}] + self.assertFalse(_evaluate_conditions(conditions, {}, {})) + + def test_number_condition_matches_float(self): + """float("3.14") must equal float(3.14) — old int() coercion would have failed.""" + fields = self._field_map(_field("score", "Number")) + conditions = [{"fieldname": "score", "operator": "Is", "value": 3.14}] + self.assertTrue(_evaluate_conditions(conditions, {"score": "3.14"}, fields)) + self.assertFalse(_evaluate_conditions(conditions, {"score": "3.15"}, fields)) + + def test_number_condition_int_vs_float_parity(self): + """Condition value stored as int 42 must match submitted value "42".""" + fields = self._field_map(_field("age", "Number")) + conditions = [{"fieldname": "age", "operator": "Is", "value": 42}] + self.assertTrue(_evaluate_conditions(conditions, {"age": "42"}, fields)) + + def test_boolean_condition_type_aware(self): + """True (bool) condition must match truthy submitted value without str() mismatch.""" + fields = self._field_map(_field("agreed", "Switch")) + conditions = [{"fieldname": "agreed", "operator": "Is", "value": True}] + self.assertTrue(_evaluate_conditions(conditions, {"agreed": 1}, fields)) + self.assertFalse(_evaluate_conditions(conditions, {"agreed": 0}, fields)) + + +class TestValidateFormResponse(unittest.TestCase): + def test_required_field_missing_raises(self): + import frappe + + form = _form(_field("name", reqd=1)) + with self.assertRaises(frappe.ValidationError): + _validate_form_response(form, {}) + + def test_required_field_present_passes(self): + form = _form(_field("name", reqd=1)) + _validate_form_response(form, {"name": "John"}) # must not raise + + def test_hidden_required_field_is_skipped(self): + """A hidden required field must not be validated.""" + form = _form(_field("secret", reqd=1, hidden=1)) + _validate_form_response(form, {}) # must not raise + + def test_conditional_show_makes_field_required(self): + """A field conditionally required via require_answer must be checked.""" + import frappe + + logic = json.dumps( + { + "target_field": "phone", + "action": "Require Answer", + "conditions": [{"fieldname": "contact_method", "operator": "Is", "value": "Phone"}], + } + ) + contact = _field("contact_method") + phone = _field("phone", reqd=0) + phone_with_logic = _field("contact_method", conditional_logic=logic) + + form = _form(contact, phone, phone_with_logic) + with self.assertRaises(frappe.ValidationError): + _validate_form_response(form, {"contact_method": "Phone", "phone": ""}) + + def test_conditional_hide_skips_required_field(self): + """A conditionally hidden field must not be validated even if reqd=1.""" + logic = json.dumps( + { + "target_field": "extra", + "action": "Hide Field", + "conditions": [{"fieldname": "flag", "operator": "Is", "value": "yes"}], + } + ) + flag = _field("flag") + extra = _field("extra", reqd=1) + flag_with_logic = _field("flag", conditional_logic=logic) + + form = _form(flag, extra, flag_with_logic) + # extra is required but should be hidden when flag=yes — no error expected + _validate_form_response(form, {"flag": "yes", "extra": ""}) + + def test_multiple_missing_fields_all_reported(self): + """All missing required fields should appear in the error, not just the first.""" + import frappe + + form = _form( + _field("first_name", reqd=1), + _field("last_name", reqd=1), + ) + with self.assertRaises(frappe.ValidationError) as ctx: + _validate_form_response(form, {}) + + error_msg = str(ctx.exception) + self.assertIn("First Name", error_msg) + self.assertIn("Last Name", error_msg) + + def test_required_field_with_zero_passes(self): + """Numeric 0 is a valid value and must not trigger a required-field error.""" + form = _form(_field("score", fieldtype="Number", reqd=1)) + _validate_form_response(form, {"score": 0}) # must not raise + + def test_required_field_with_false_passes(self): + """Boolean False (unchecked Switch) is a valid value for a required field.""" + form = _form(_field("agreed", fieldtype="Switch", reqd=1)) + _validate_form_response(form, {"agreed": False}) # must not raise + + +class TestConstants(unittest.TestCase): + def test_system_fieldnames_derived_from_form_generator(self): + """Constants must stay in sync with form_generator field definitions.""" + self.assertIn(SUBMISSION_STATUS_FIELDOPTIONS["fieldname"], FORMS_PRO_SYSTEM_FIELDNAMES) + self.assertIn(LINKED_FORM_FIELDOPTIONS["fieldname"], FORMS_PRO_SYSTEM_FIELDNAMES) + + def test_unsupported_fieldtypes_contains_layout_types(self): + for fieldtype in ("Section Break", "Column Break", "Tab Break", "Fold"): + self.assertIn(fieldtype, UNSUPPORTED_FRAPPE_FIELDTYPES) + + def test_unsupported_fieldtypes_contains_non_input_types(self): + for fieldtype in ("HTML", "Button", "Barcode", "Dynamic Link"): + self.assertIn(fieldtype, UNSUPPORTED_FRAPPE_FIELDTYPES) diff --git a/forms_pro/utils/constants.py b/forms_pro/utils/constants.py new file mode 100644 index 0000000..7675945 --- /dev/null +++ b/forms_pro/utils/constants.py @@ -0,0 +1,27 @@ +from forms_pro.utils.form_generator import LINKED_FORM_FIELDOPTIONS, SUBMISSION_STATUS_FIELDOPTIONS + +# Frappe fieldtypes that have no Forms Pro equivalent and must be excluded +# when importing fields from an existing DocType into a form. +# These are layout/structural types (Section Break, Column Break, etc.) +# or types with no meaningful input representation (Button, Barcode, Fold). +UNSUPPORTED_FRAPPE_FIELDTYPES: frozenset[str] = frozenset( + [ + "Section Break", + "Column Break", + "Tab Break", + "Fold", + "HTML", + "Button", + "Barcode", + "Dynamic Link", + ] +) + +# Fieldnames injected by Forms Pro itself — must never appear in the +# user-visible field list returned to the form builder or submission page. +FORMS_PRO_SYSTEM_FIELDNAMES: frozenset[str] = frozenset( + [ + SUBMISSION_STATUS_FIELDOPTIONS["fieldname"], # fp_submission_status + LINKED_FORM_FIELDOPTIONS["fieldname"], # fp_linked_form + ] +) diff --git a/frontend/src/components/builder/FieldLabel.vue b/frontend/src/components/builder/FieldLabel.vue new file mode 100644 index 0000000..8572ffa --- /dev/null +++ b/frontend/src/components/builder/FieldLabel.vue @@ -0,0 +1,30 @@ + + + diff --git a/frontend/src/components/builder/FieldRenderer.vue b/frontend/src/components/builder/FieldRenderer.vue index 9af4ea7..748c19f 100644 --- a/frontend/src/components/builder/FieldRenderer.vue +++ b/frontend/src/components/builder/FieldRenderer.vue @@ -1,9 +1,10 @@ + diff --git a/frontend/src/components/builder/field-editor/FieldPropertiesForm.vue b/frontend/src/components/builder/field-editor/FieldPropertiesForm.vue index 4183359..38cc19a 100644 --- a/frontend/src/components/builder/field-editor/FieldPropertiesForm.vue +++ b/frontend/src/components/builder/field-editor/FieldPropertiesForm.vue @@ -1,7 +1,8 @@