chore!: refactor code for better maintainability

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

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

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,
         }