From 58dbfd420367fc9338d8df1d72ebc7dfc99b50ff Mon Sep 17 00:00:00 2001 From: Kevin Shenk Date: Fri, 16 Oct 2026 23:44:13 -0400 Subject: [PATCH 01/14] chore: ignore the app standards formatting commit in blame Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SvdEeMiewirZ3ifFoUix5F --- .git-blame-ignore-revs | 1 + 1 file changed, 1 insertion(+) diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs index a4746b9..99efecc 100644 --- a/.git-blame-ignore-revs +++ b/.git-blame-ignore-revs @@ -1,2 +1,3 @@ # Mass reformats (style: PRs), one squash SHA per line with its subject as a comment. # GitHub reads this file; locally the dev shell sets blame.ignoreRevsFile. +0f2bacb5c3f826e683a21223e4adf81ad0eba101 # style: adopt the app standards (managed config and formatting) (#7) From 8dc18f5de59e3b982b8f391e0f5a16abd31c8367 Mon Sep 17 00:00:00 2001 From: Kevin Shenk Date: Fri, 16 Oct 2026 23:44:26 -0400 Subject: [PATCH 02/14] refactor: drop the Frappe v15 controller override path The app targets Frappe 16 only, so Web Form and Email Template are always registered with extend_doctype_class. EsignWebForm subclasses the stock WebForm unconditionally; the v15 branch that inherited from payments' PaymentWebForm and the override_doctype_class registration are gone. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SvdEeMiewirZ3ifFoUix5F --- esign/esign/custom/web_form.py | 40 ++++++++-------------------------- esign/hooks.py | 20 +++++------------ pyproject.toml | 2 +- 3 files changed, 16 insertions(+), 46 deletions(-) diff --git a/esign/esign/custom/web_form.py b/esign/esign/custom/web_form.py index 2a459fc..9184591 100644 --- a/esign/esign/custom/web_form.py +++ b/esign/esign/custom/web_form.py @@ -5,7 +5,7 @@ import json import uuid from datetime import datetime -from typing import TYPE_CHECKING, TypedDict, cast +from typing import TypedDict, cast import frappe from frappe import _ @@ -62,34 +62,6 @@ def validate_esign_key(key: str, doc: Document, require_key: bool = False) -> bo return True -FRAPPE_MAJOR = int(frappe.__version__.split(".")[0]) - -# Choose the base class for EsignWebForm. -# -# Frappe v16+ registers this class via `extend_doctype_class` (see hooks.py) and composes -# the final controller as `type(..., (*extensions_in_reverse_install_order, WebForm))`. -# The payments app registers its PaymentWebForm the same way, so its behaviour is still -# mixed into the final class. If we also inherited from PaymentWebForm here, installing -# esign before payments would produce bases (PaymentWebForm, EsignWebForm, WebForm), -# which has no consistent MRO and breaks loading every Web Form. So on v16+ we only -# subclass the stock WebForm. -# -# Frappe v15 uses `override_doctype_class`, where the last app wins; there we keep -# inheriting from PaymentWebForm (when installed) so payment behaviour isn't lost. -if TYPE_CHECKING or FRAPPE_MAJOR >= 16: - _EsignWebFormBase = BaseWebForm -else: - try: - from payments.overrides.payment_webform import PaymentWebForm # type: ignore - except ImportError: - # Dummy mixin when payments isn't installed - class PaymentWebForm: - pass - - class _EsignWebFormBase(PaymentWebForm, BaseWebForm): - pass - - class RequestData(TypedDict): host_url: str | None scheme: str @@ -149,8 +121,14 @@ def _find_email_fieldname(wf: EsignWebForm) -> str | None: return None -class EsignWebForm(_EsignWebFormBase): - """Custom WebForm override for eSign-specific functionality with custom field types""" +class EsignWebForm(BaseWebForm): + """eSign behaviour for Web Form, registered through `extend_doctype_class`. + + Frappe composes the final controller from the stock WebForm and every installed app's + extension (payments registers one too), so this subclasses the stock WebForm only: + subclassing another app's extension would break the class hierarchy whenever that app is + installed after esign. + """ # Custom fields from ../custom/web_form.json esign_enabled: DF.Check | None diff --git a/esign/hooks.py b/esign/hooks.py index 14dadf5..1edc128 100644 --- a/esign/hooks.py +++ b/esign/hooks.py @@ -1,10 +1,6 @@ # Copyright (c) 2026, Avunu LLC and contributors # For license information, please see license.txt -import frappe - -frappe_version = int(frappe.__version__.split(".")[0]) - app_name = "esign" app_title = "eSign" app_publisher = "Avunu LLC" @@ -33,16 +29,12 @@ "Web Form": "public/js/web_form.js", } -if frappe_version >= 16: - extend_doctype_class = { - "Web Form": "esign.esign.custom.web_form.EsignWebForm", - "Email Template": "esign.esign.custom.email_template.EsignEmailTemplate", - } -else: - override_doctype_class = { - "Web Form": "esign.esign.custom.web_form.EsignWebForm", - "Email Template": "esign.esign.custom.email_template.EsignEmailTemplate", - } +# Frappe v16 composes these onto the stock controllers (and any other app's extensions), +# so each class subclasses the stock controller only. +extend_doctype_class = { + "Web Form": "esign.esign.custom.web_form.EsignWebForm", + "Email Template": "esign.esign.custom.email_template.EsignEmailTemplate", +} jinja = { diff --git a/pyproject.toml b/pyproject.toml index c8b0ae9..79f8001 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -7,7 +7,7 @@ readme = "README.md" classifiers = ["License :: OSI Approved :: MIT License"] dynamic = ["version"] dependencies = [ - # "frappe~=15.0.0" # Installed and managed by bench. + # frappe is installed and managed by bench; see [tool.bench.frappe-dependencies]. ] [build-system] From 8532c6a2fa279956b4340cc757b9680e0b404d72 Mon Sep 17 00:00:00 2001 From: Kevin Shenk Date: Fri, 16 Oct 2026 23:44:57 -0400 Subject: [PATCH 03/14] fix: check access before rendering, updating or sharing a document - get_print_html set frappe.flags.ignore_print_permissions before validate_esign_key, so without a key the fallback print permission check was skipped and any visitor could read any document's print HTML. The key or the permission check now runs first. - accept updated an existing document with ignore_permissions whenever no key was sent. An update now needs the share key or web form write permission, through has_web_form_permission, as frappe's own accept does. - send_esign_email and get_esign_link (called with a document reference) mint a share key for the document, so they now need the document's email permission, as frappe's email dialog does. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SvdEeMiewirZ3ifFoUix5F --- esign/esign/__init__.py | 4 ++++ esign/esign/custom/web_form.py | 21 ++++++++++++++++----- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/esign/esign/__init__.py b/esign/esign/__init__.py index 09703f5..594fafb 100644 --- a/esign/esign/__init__.py +++ b/esign/esign/__init__.py @@ -126,6 +126,10 @@ def send_esign_email( except frappe.DoesNotExistError: frappe.throw(_("Document {0} {1} not found").format(doctype, name)) + # The email carries a share key that opens the document to its recipients, so the sender + # needs the same permission frappe's own email dialog asks for. + frappe.has_permission(doctype, ptype="email", doc=name, throw=True) + # Validate web form from esign.esign.custom.web_form import EsignWebForm, get_esign_link diff --git a/esign/esign/custom/web_form.py b/esign/esign/custom/web_form.py index 9184591..55aa40d 100644 --- a/esign/esign/custom/web_form.py +++ b/esign/esign/custom/web_form.py @@ -694,6 +694,10 @@ def get_esign_link( doc = frappe.get_doc(doc["doctype"], doc["name"]) except Exception as e: return str(e) + # A reference from a request: the link carries a share key that opens the document to + # whoever holds it, so the caller must be allowed to email the document (as for + # send_esign_email). Server-side callers (Jinja, Python) pass the Document itself. + frappe.has_permission(doc.doctype, ptype="email", doc=doc, throw=True) try: web_form = EsignWebForm("Web Form", web_form_name) @@ -757,13 +761,15 @@ def get_print_html( doc = frappe.get_doc(doctype, docname) doc.flags.ignore_permissions = True - # Skip internal permission checks since we're using key-based auth - # This prevents msgprint warnings from has_permission checks - frappe.flags.ignore_print_permissions = True - - # Validate access via share key or fallback to standard permissions + # Validate access via share key or fallback to standard permissions. This has to run + # before ignore_print_permissions is set: without a key, the fallback is the standard + # print permission check, which that flag turns off. validate_esign_key(key, doc) + # Access is granted: skip the renderer's own permission checks, which would only add + # msgprint warnings for a Guest holding a valid key. + frappe.flags.ignore_print_permissions = True + # Get print format document meta = frappe.get_meta(doctype) print_format_doc = get_print_format_doc(print_format, meta=meta) @@ -1091,6 +1097,11 @@ def accept(web_form, data): except frappe.PermissionError, frappe.exceptions.LinkExpired: frappe.throw(_("Invalid or expired access key"), frappe.PermissionError) + # An update needs the document's share key or web form write permission, as in frappe's + # own accept; otherwise anyone who knows a document name could overwrite its form fields. + if data.name and not wf.has_web_form_permission(doctype, data.name, "write"): + frappe.throw(_("You are not permitted to update this document"), frappe.PermissionError) + # Now proceed with the form submission using ignore_permissions files = [] files_to_delete = [] From 1859be2b15997da8aac11d04d8396eab53420d1f Mon Sep 17 00:00:00 2001 From: Kevin Shenk Date: Fri, 16 Oct 2026 23:46:15 -0400 Subject: [PATCH 04/14] fix: clear the registry's blocking semgrep findings frappe-manual-commit (2): - attach_print_to_document committed mid-job so the signed PDF's File row was saved before the Communication touched the document. It now ends after the File is attached and queues a second job, record_signature, with enqueue_after_commit: the queue starts it once the first job's transaction has committed, which keeps the same boundary without a manual commit. The request mocking both jobs need is one context manager, _job_request. - download_signed_pdf committed the new cloud storage sharing link before redirecting. It sets frappe.local.flags.commit instead, so the request commits at its end, before the response is sent. It also redirected with the raw UUID while storing the UUID's top 64 bits, so the first download of a file without a link went to a key that matched nothing; the stored key is now the one in the redirect. frappe-ssti (2): the notification's subject and message are rendered from a System Manager-authored Notification, as frappe's own Notification does; the two calls carry nosemgrep with that reason. The context is built as Notification.send builds it (get_context(doc) was computed and then thrown away), and the audit trail values appended to the email are HTML-escaped, since the user agent and the email parameter come from the signer's request. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SvdEeMiewirZ3ifFoUix5F --- esign/esign/custom/web_form.py | 231 ++++++++++++++++++++------------- 1 file changed, 143 insertions(+), 88 deletions(-) diff --git a/esign/esign/custom/web_form.py b/esign/esign/custom/web_form.py index 55aa40d..3e99de2 100644 --- a/esign/esign/custom/web_form.py +++ b/esign/esign/custom/web_form.py @@ -4,6 +4,8 @@ import hashlib import json import uuid +from collections.abc import Iterator +from contextlib import contextmanager from datetime import datetime from typing import TypedDict, cast @@ -19,7 +21,7 @@ from frappe.rate_limiter import rate_limit from frappe.twofactor import get_qr_svg_code from frappe.types import DF -from frappe.utils import cint, get_frappe_version, now +from frappe.utils import cint, escape_html, get_frappe_version, now from frappe.website.doctype.web_form.web_form import WebForm as BaseWebForm from frappe.www.printview import validate_print_permission @@ -377,9 +379,9 @@ def _send_esign_notification( ) return - # Build context similar to how Notification.send() does it + # Build context the way Notification.send() does context = get_context(doc) - context = {"doc": doc, "alert": notification, "comments": None} + context.update({"alert": notification, "comments": None}) if doc.get("_comments"): context["comments"] = json.loads(str(doc.get("_comments", "[]"))) @@ -394,12 +396,14 @@ def _send_esign_notification( ) return - # Render subject and message + # Render subject and message. Both come from the Notification, which only System + # Managers author: the same trusted source frappe's own Notification.send_an_email + # renders them from, with the same sandboxed environment. subject = notification.subject if subject and "{" in subject: - subject = frappe.render_template(notification.subject, context) + subject = frappe.render_template(subject, context) # nosemgrep: frappe-ssti - message = frappe.render_template(notification.message, context) or "" + message = frappe.render_template(notification.message, context) or "" # nosemgrep: frappe-ssti # Append audit trail information to the notification email if audit_data: @@ -420,9 +424,11 @@ def _send_esign_notification( audit_rows.append((_("PDF SHA-256"), pdf_hash)) if audit_rows: + # The values come from the signer's request (user agent, email parameter), so + # they are escaped before going into the HTML body. table_rows = "".join( f'{label}' - f'{value}' + f'{escape_html(value)}' for label, value in audit_rows ) message += ( @@ -513,21 +519,14 @@ def _send_esign_notification( ) -def attach_print_to_document( - doc: Document, - print_format: str, - request_data: RequestData, - audit_data: AuditData, - notification_name: str | None = None, -): - """Background job to attach the signed PDF to the document. +@contextmanager +def _job_request(request_data: RequestData) -> Iterator[None]: + """Give a background job the signing request's host, scheme and headers. - Also records audit trail and SHA-256 hash for document integrity verification. - Creates a Communication record for timeline display via additional_timeline_content hook. - Optionally triggers a notification with the validated signed PDF attached. + The PDF renderer and URL helpers read them from `frappe.local.request`. Only the + context-local request is set: assigning `frappe.request` would replace the module-level + LocalProxy for the rest of the worker process and leak into later jobs. """ - from frappe import attach_print - mock_request = frappe._dict( { "headers": request_data.get("headers", {}), @@ -538,72 +537,55 @@ def attach_print_to_document( "scheme": request_data.get("scheme", "https"), } ) - - # Only set the context-local request. Assigning `frappe.request` would replace the - # module-level LocalProxy for the rest of this worker process and leak into later jobs. prev_request = getattr(frappe.local, "request", _UNSET) frappe.local.request = mock_request - try: - # Refetch the document to ensure latest data - doc.reload() + yield + finally: + if prev_request is _UNSET: + if hasattr(frappe.local, "request"): + del frappe.local.request + else: + frappe.local.request = prev_request - # Generate the PDF - timestamp_str = datetime.now().strftime("%Y%m%d_%H%M%S") - print_data = attach_print( - doctype=doc.doctype, - name=doc.name, - file_name=f"{doc.name}_signed_{timestamp_str}", - print_format=print_format, - doc=doc, - print_letterhead=False, - ) - # Compute SHA-256 hash of the PDF content - pdf_content = print_data["fcontent"] - pdf_hash = hashlib.sha256(pdf_content).hexdigest() - frappe_content_hash = get_content_hash(pdf_content) +def _read_file_bytes(file_doc: File) -> bytes: + """The stored bytes of a local File. File.get_content decodes anything that decodes as + text (a PDF often does, as windows-1252), and writing that back would change the bytes.""" + with open(file_doc.get_full_path(), "rb") as f: + return f.read() - # Attach the signed PDF to the document - # Note: content_hash must be set explicitly for cloud_storage deduplication to work, - # as cloud_storage's validate() doesn't call the core File validate which generates it - file_doc = cast( - File, - frappe.get_doc( - { - "doctype": "File", - "file_name": print_data["fname"], - "attached_to_doctype": doc.doctype, - "attached_to_name": doc.name, - "folder": "Home/Attachments", - "is_private": True, - "content": pdf_content, - "content_hash": frappe_content_hash, - } - ), - ) - file_doc.insert(ignore_permissions=True) - # Check if this is a cloud_storage-enhanced File (has s3_key and file_association attributes) - is_cloud_storage = hasattr(file_doc, "s3_key") and hasattr(file_doc, "file_association") +def _is_cloud_storage_file(file_doc: File) -> bool: + """Whether cloud_storage's File extension (s3_key, file_association) is installed.""" + return hasattr(file_doc, "s3_key") and hasattr(file_doc, "file_association") - if is_cloud_storage: - # Extract s3_key from the file_url and persist it - s3_key = str(file_doc.file_url or "").replace("/api/method/retrieve?key=", "") - if s3_key: - file_doc.db_set("s3_key", s3_key) - # Commit the file transaction to avoid deadlock with Communication's _comments update. - # Cloud storage's associate_files() may have modified the parent document, and the - # Communication insert will try to update _comments on the same document. - frappe.db.commit() +def record_signature( + doctype: str, + name: str, + file_name: str, + pdf_hash: str, + request_data: RequestData, + audit_data: AuditData, + notification_name: str | None = None, +): + """Background job: record the signing on the document's timeline. + + Creates the eSign Communication (audit data as JSON, shown through the + additional_timeline_content hook), links the signed PDF to it and, when the web form + names one, sends the notification with that PDF attached. + """ + with _job_request(request_data): + doc = frappe.get_doc(doctype, name) + file_doc = cast(File, frappe.get_doc("File", file_name)) # Create Communication record for timeline display with audit data as JSON audit_content = json.dumps( { **audit_data, "pdf_hash": pdf_hash, - "file_name": print_data["fname"], + "file_name": file_doc.file_name, } ) @@ -624,11 +606,10 @@ def attach_print_to_document( comm.insert(ignore_permissions=True) # Associate the PDF with the Communication - if is_cloud_storage: + if _is_cloud_storage_file(file_doc): # For cloud_storage, add a file_association instead of duplicating the file from frappe.utils import get_datetime - file_doc.reload() file_doc.append( "file_association", { @@ -646,13 +627,13 @@ def attach_print_to_document( frappe.get_doc( { "doctype": "File", - "file_name": print_data["fname"], + "file_name": file_doc.file_name, "attached_to_doctype": "Communication", "attached_to_name": comm.name, "folder": "Home/Attachments", "is_private": True, - "content": pdf_content, - "content_hash": frappe_content_hash, + "content": _read_file_bytes(file_doc), + "content_hash": file_doc.content_hash, } ), ) @@ -661,12 +642,83 @@ def attach_print_to_document( # Trigger notification with the validated signed PDF attached if notification_name: _send_esign_notification(notification_name, doc, file_doc, audit_data, pdf_hash) - finally: - if prev_request is _UNSET: - if hasattr(frappe.local, "request"): - del frappe.local.request - else: - frappe.local.request = prev_request + + +def attach_print_to_document( + doc: Document, + print_format: str, + request_data: RequestData, + audit_data: AuditData, + notification_name: str | None = None, +): + """Background job: render the signed PDF and attach it to the document. + + The audit trail (the Communication, its copy of the PDF and the notification) is written + by `record_signature`, a second job queued to start once this job's transaction has + committed. The File has to be committed before the Communication updates the document's + `_comments`: cloud storage's file association may have changed the document too, and + doing both in one transaction deadlocked. + """ + from frappe import attach_print + + with _job_request(request_data): + # Refetch the document to ensure latest data + doc.reload() + + # Generate the PDF + timestamp_str = datetime.now().strftime("%Y%m%d_%H%M%S") + print_data = attach_print( + doctype=doc.doctype, + name=doc.name, + file_name=f"{doc.name}_signed_{timestamp_str}", + print_format=print_format, + doc=doc, + print_letterhead=False, + ) + + # Compute SHA-256 hash of the PDF content + pdf_content = print_data["fcontent"] + pdf_hash = hashlib.sha256(pdf_content).hexdigest() + + # Attach the signed PDF to the document + # Note: content_hash must be set explicitly for cloud_storage deduplication to work, + # as cloud_storage's validate() doesn't call the core File validate which generates it + file_doc = cast( + File, + frappe.get_doc( + { + "doctype": "File", + "file_name": print_data["fname"], + "attached_to_doctype": doc.doctype, + "attached_to_name": doc.name, + "folder": "Home/Attachments", + "is_private": True, + "content": pdf_content, + "content_hash": get_content_hash(pdf_content), + } + ), + ) + file_doc.insert(ignore_permissions=True) + + if _is_cloud_storage_file(file_doc): + # Extract s3_key from the file_url and persist it + s3_key = str(file_doc.file_url or "").replace("/api/method/retrieve?key=", "") + if s3_key: + file_doc.db_set("s3_key", s3_key) + + frappe.enqueue( + record_signature, + queue="short", + timeout=300, + enqueue_after_commit=True, + doctype=doc.doctype, + name=doc.name, + file_name=file_doc.name, + pdf_hash=pdf_hash, + request_data=request_data, + audit_data=audit_data, + notification_name=notification_name, + ) @frappe.whitelist() @@ -890,13 +942,16 @@ def download_signed_pdf(doctype: str, docname: str, key: str = ""): # there instead. if file_url.startswith("/api/method/retrieve"): full_file = frappe.get_doc("File", file_doc.get("name")) - sharing_uuid = full_file.get("sharing_link") - if not sharing_uuid: - sharing_uuid = uuid.uuid4() - full_file.db_set("sharing_link", str(int(sharing_uuid) >> 64)) - frappe.db.commit() # ensure sharing_link is saved before redirect + sharing_link = full_file.get("sharing_link") + if not sharing_link: + # The same key format cloud_storage's own get_sharing_link writes. + sharing_link = str(uuid.uuid4().int >> 64) + full_file.db_set("sharing_link", sharing_link) + # A GET request is rolled back at its end unless it asks to commit; the link must + # exist by the time the browser follows the redirect. + frappe.local.flags.commit = True frappe.local.response["type"] = "redirect" - frappe.local.response["location"] = f"/api/method/share?key={sharing_uuid}" + frappe.local.response["location"] = f"/api/method/share?key={sharing_link}" return # Handle private vs public files (local storage) From e9acaab9a7871f72b91e105aaf896e2d4b2c0be3 Mon Sep 17 00:00:00 2001 From: Kevin Shenk Date: Fri, 16 Oct 2026 23:47:46 -0400 Subject: [PATCH 05/14] fix: make ty clean - Read WebFormField.fieldname and Notification.set_property_after_alert through get(): frappe types both as Literal[None], so ty treated every branch that used them as unreachable. - esign_update_field is typed as the string it holds; its Select options are the target doctype's fields, set at runtime. - The request snapshot for the signing job is built key by key instead of looping over a TypedDict with str keys. - The image upload check in accept read `"data:" and "base64" in value`, where "data:" was always true; it now checks for both substrings. - Drop two unused type: ignore comments, and the blanket one on the attachments lookup (frappe.desk.form.load is imported as a module so a patched get_attachments is still the one called). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SvdEeMiewirZ3ifFoUix5F --- esign/esign/__init__.py | 6 ++- esign/esign/custom/web_form.py | 74 ++++++++++++++++++---------------- 2 files changed, 45 insertions(+), 35 deletions(-) diff --git a/esign/esign/__init__.py b/esign/esign/__init__.py index 594fafb..8a77bc3 100644 --- a/esign/esign/__init__.py +++ b/esign/esign/__init__.py @@ -5,6 +5,10 @@ import json import frappe + +# Imported as a module and looked up at call time, so a patched get_attachments (cloud_storage +# replaces it) is the one called. +import frappe.desk.form.load from frappe import _ from frappe.core.doctype.communication.communication import Communication from frappe.utils import now @@ -48,7 +52,7 @@ def get_timeline_content(doctype: str, docname: str) -> list[dict]: # Get the attached signed PDF file URL using the patched get_attachments # This works for both standard Frappe and cloud_storage setups file_url = None - attachments = frappe.desk.form.load.get_attachments("Communication", comm.name) # type: ignore + attachments = frappe.desk.form.load.get_attachments("Communication", comm.name) for attachment in attachments: if attachment.get("file_name", "").endswith(".pdf"): file_url = attachment.get("file_url") diff --git a/esign/esign/custom/web_form.py b/esign/esign/custom/web_form.py index 3e99de2..48410c9 100644 --- a/esign/esign/custom/web_form.py +++ b/esign/esign/custom/web_form.py @@ -4,7 +4,7 @@ import hashlib import json import uuid -from collections.abc import Iterator +from collections.abc import Generator from contextlib import contextmanager from datetime import datetime from typing import TypedDict, cast @@ -106,6 +106,16 @@ def extract_param_from_referrer(param: str = "") -> str: return "" +def _fieldname(row: Document) -> str | None: + """A web form row's fieldname. + + Frappe declares WebFormField.fieldname (and Notification.set_property_after_alert) as + `Literal[None]`, its type for a Select whose options are only filled in at runtime, so + the value is read through `get`, which is untyped. + """ + return row.get("fieldname") + + def _find_email_fieldname(wf: EsignWebForm) -> str | None: """Find the fieldname of a Data field with Options='Email' on the web form. @@ -116,10 +126,11 @@ def _find_email_fieldname(wf: EsignWebForm) -> str | None: """ meta = frappe.get_meta(wf.doc_type) for field in wf.web_form_fields: - if field.fieldtype == "Data" and field.fieldname: - df = meta.get_field(field.fieldname) + fieldname = _fieldname(field) + if field.fieldtype == "Data" and fieldname: + df = meta.get_field(fieldname) if df and df.fieldtype == "Data" and (df.options or "").strip().lower() == "email": - return field.fieldname + return fieldname return None @@ -135,7 +146,8 @@ class EsignWebForm(BaseWebForm): # Custom fields from ../custom/web_form.json esign_enabled: DF.Check | None esign_force_mobile: DF.Check | None - esign_update_field: DF.Select[""] | None # noqa: F722 -- options are set per doctype at runtime, as frappe generates them + # A Select whose options are the target doctype's fields, set at runtime (web_form.js). + esign_update_field: DF.Data | None esign_update_value: DF.Data | None esign_submit_on_response: DF.Check | None esign_send_notification: DF.Check | None @@ -195,7 +207,7 @@ def _get_signed_pdf(self, doc: Document) -> dict | None: def _check_signature_exists(self, doc: Document) -> bool: """Check if all signature fields in the web form already have values.""" signature_fields = [ - field.fieldname for field in self.web_form_fields if field.fieldtype == "Signature" + _fieldname(field) for field in self.web_form_fields if field.fieldtype == "Signature" ] # If no signature fields, not applicable @@ -245,7 +257,9 @@ def get_context(self, context): context.doctype = self.doc_type context.name = cur_doc_name context.print_format = print_format - form_fields: set[str] = {field.fieldname for field in self.web_form_fields if field.fieldname} + form_fields: set[str] = { + fieldname for field in self.web_form_fields if (fieldname := _fieldname(field)) + } form_fields.add("name") form_fields.add("doctype") context.reference_doc = {k: v for k, v in doc.as_dict().items() if k in form_fields} @@ -428,7 +442,7 @@ def _send_esign_notification( # they are escaped before going into the HTML body. table_rows = "".join( f'{label}' - f'{escape_html(value)}' + f'{escape_html(str(value))}' for label, value in audit_rows ) message += ( @@ -487,14 +501,14 @@ def _send_esign_notification( ) # Handle set_property_after_alert if configured - if notification.set_property_after_alert: + fieldname = notification.get("set_property_after_alert") # Literal[None] in frappe's types + if fieldname: allow_update = True - field_df = doc.meta.get_field(notification.set_property_after_alert) + field_df = doc.meta.get_field(fieldname) if doc.docstatus.is_submitted() and field_df and not field_df.allow_on_submit: allow_update = False try: if allow_update and not doc.flags.in_notification_update: - fieldname = notification.set_property_after_alert value: str | int | float | None = notification.property_value if field_df and field_df.fieldtype in numeric_fieldtypes: value = cint(value or 0) @@ -520,7 +534,7 @@ def _send_esign_notification( @contextmanager -def _job_request(request_data: RequestData) -> Iterator[None]: +def _job_request(request_data: RequestData) -> Generator[None]: """Give a background job the signing request's host, scheme and headers. The PDF renderer and URL helpers read them from `frappe.local.request`. Only the @@ -830,7 +844,7 @@ def get_print_html( # Get rendered print HTML print_html = get_rendered_template( doc=doc, - print_format=print_format_doc, # type: ignore + print_format=print_format_doc, meta=meta, trigger_print=False, no_letterhead=bool(frappe.form_dict.get("no_letterhead")), @@ -841,7 +855,7 @@ def get_print_html( # Get print styles print_style = get_print_style( style=frappe.form_dict.get("style"), - print_format=print_format_doc, # type: ignore + print_format=print_format_doc, ) # Clean up any local message log @@ -1050,9 +1064,9 @@ def _collect_audit_data(doc: Document, wf: EsignWebForm, print_format: str) -> A signature_fieldtypes = ("Signature", "SignaturePad") for field in wf.web_form_fields: if field.fieldtype in signature_fieldtypes: - value = doc.get(field.fieldname) - if value: - signed_fields.append(field.label or field.fieldname) + fieldname = _fieldname(field) + if fieldname and doc.get(fieldname): + signed_fields.append(field.label or fieldname) return AuditData( timestamp=now(), @@ -1074,24 +1088,16 @@ def _enqueue_print_attachment(doc: Document, wf: EsignWebForm): print_format = wf.print_format or "standard" # Extract only serializable data from request + request = getattr(frappe.local, "request", None) + headers = getattr(request, "headers", None) request_data: RequestData = { - "host_url": None, - "scheme": "https", - "host": None, - "headers": {}, - "method": None, + "host_url": getattr(request, "host_url", None), + "scheme": getattr(request, "scheme", "https"), + "host": getattr(request, "host", None), + "headers": dict(headers) if headers is not None else {}, + "method": getattr(request, "method", None), } - for attr, default in request_data.items(): - if hasattr(frappe, "request") and hasattr(frappe.request, attr): - value = getattr(frappe.request, attr) - # Convert headers to dict if it's the headers attribute - if attr == "headers" and value is not None: - value = dict(value) - request_data[attr] = value - else: - request_data[attr] = default - # Collect audit data audit_data = _collect_audit_data(doc, wf, print_format) @@ -1186,12 +1192,12 @@ def accept(web_form, data): # Set web form field values for field in wf.web_form_fields: - fieldname = field.fieldname + fieldname = _fieldname(field) df = meta.get_field(fieldname) value = data.get(fieldname, "") if df and df.fieldtype in ("Attach", "Attach Image"): - if value and "data:" and "base64" in value: + if value and "data:" in value and "base64" in value: files.append((fieldname, value)) if not doc.name: doc.set(fieldname, "") From 1b72fba6fd87d1fc793c82939a97691f162c27fd Mon Sep 17 00:00:00 2001 From: Kevin Shenk Date: Fri, 16 Oct 2026 23:59:50 -0400 Subject: [PATCH 06/14] fix: validate the share key before marking a document permission-free get_context, get_print_html and download_signed_pdf set doc.flags.ignore_permissions before validate_esign_key. Without a key, that falls back to frappe's print permission check, whose website permission step passes any document carrying the flag, so a visitor with no key could open the signing page and print HTML of any document, and a logged-in user without read permission could download its signed PDF. The flag is now set only after the key or the permission check has passed (and not at all where nothing needs it). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SvdEeMiewirZ3ifFoUix5F --- esign/esign/custom/web_form.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/esign/esign/custom/web_form.py b/esign/esign/custom/web_form.py index 48410c9..70a60e4 100644 --- a/esign/esign/custom/web_form.py +++ b/esign/esign/custom/web_form.py @@ -232,10 +232,12 @@ def get_context(self, context): # Get document without permission check - we'll validate via share key doc = frappe.get_doc(self.doc_type, cur_doc_name) - doc.flags.ignore_permissions = True - # Validate access via share key or fallback to standard permissions + # Validate access via share key or fallback to standard permissions. The document is + # only marked permission-free afterwards: frappe's website permission check (part of + # the fallback) passes any document that already carries ignore_permissions. validate_esign_key(key, doc) + doc.flags.ignore_permissions = True # Allow public access to the web form if validation passes print_format = frappe.form_dict.get("format", self.print_format) or "standard" @@ -336,7 +338,6 @@ def has_web_form_permission(self, doctype, name, ptype="read"): if key: try: doc = frappe.get_doc(doctype, name) - doc.flags.ignore_permissions = True validate_esign_key(key, doc, require_key=True) return True except frappe.PermissionError: @@ -825,12 +826,13 @@ def get_print_html( # Get the document without permission checks - we'll validate via share key doc = frappe.get_doc(doctype, docname) - doc.flags.ignore_permissions = True # Validate access via share key or fallback to standard permissions. This has to run - # before ignore_print_permissions is set: without a key, the fallback is the standard - # print permission check, which that flag turns off. + # before the document or the request is marked permission-free: without a key, the + # fallback is the standard print permission check, which ignore_print_permissions turns + # off and which passes a document that carries ignore_permissions. validate_esign_key(key, doc) + doc.flags.ignore_permissions = True # Access is granted: skip the renderer's own permission checks, which would only add # msgprint warnings for a Guest holding a valid key. @@ -888,9 +890,9 @@ def download_signed_pdf(doctype: str, docname: str, key: str = ""): # Get the document without permission checks - we'll validate via share key doc = frappe.get_doc(doctype, docname) - doc.flags.ignore_permissions = True - # Validate access via share key - require key for guest access + # Validate access via share key - require key for guest access. Without a key a user + # needs print permission, which a document marked ignore_permissions would skip. validate_esign_key(key, doc, require_key=(frappe.session.user == "Guest")) # Find the signed PDF for this document From 2f2b0a4b46917bf78743a02a0790de8b80cc9a56 Mon Sep 17 00:00:00 2001 From: Kevin Shenk Date: Sat, 17 Oct 2026 00:00:02 -0400 Subject: [PATCH 07/14] fix: build working eSign links from a document reference - get_esign_link annotated doc as Document, so frappe v16's argument type check refused the JSON or dict the desk sends (the link preview in the "Send for eSign" dialog). It now accepts all three, as its body always did. - The link joined get_url() and the route without a slash ("https://hostesign-form/..."), unless the configured host name ended in one. It now always has exactly one. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SvdEeMiewirZ3ifFoUix5F --- esign/esign/custom/web_form.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/esign/esign/custom/web_form.py b/esign/esign/custom/web_form.py index 70a60e4..8f66664 100644 --- a/esign/esign/custom/web_form.py +++ b/esign/esign/custom/web_form.py @@ -738,7 +738,7 @@ def attach_print_to_document( @frappe.whitelist() def get_esign_link( - doc: Document, web_form_name: str, print_format_name: str = "", recipient_email: str = "" + doc: Document | str | dict, web_form_name: str, print_format_name: str = "", recipient_email: str = "" ) -> str: """Get the eSign Web Form link for a given document and web form name. @@ -774,9 +774,9 @@ def get_esign_link( # Generate the URL from frappe.utils import get_url - base_url = get_url() + base_url = get_url().rstrip("/") share_key = doc.get_document_share_key() - web_form_path = f"{web_form.route}/{doc.name}/edit" + web_form_path = f"/{(web_form.route or '').lstrip('/')}/{doc.name}/edit" query_string = f"?key={share_key}" if print_format_name: query_string += f"&format={print_format_name}" From 0632fc9067c3dc549587b6b4779554607693f036 Mon Sep 17 00:00:00 2001 From: Kevin Shenk Date: Sat, 17 Oct 2026 00:00:02 -0400 Subject: [PATCH 08/14] test: cover the signing flow, its key checks and the desk hooks 77 tests on a submittable test doctype (created by the tests), an eSign web form on it, and requests built the way frappe builds them: - the signing page for a Guest with and without a key, completed documents, force-mobile and the stock web form when eSign is off; - validate_esign_key, has_web_form_permission (key from the form or the referrer), get_print_html and download_signed_pdf (private, public and cloud storage files) with valid, wrong and missing keys; - accept: signing as a Guest with a key, refusals without one or without permission, the anonymous form's switch to Guest and back (also when saving fails), submit on response, the update field, attachments, new documents, and the audit data and request snapshot it queues; - both signing jobs: the signed PDF, the Communication and its copy of the PDF, cloud storage association, and the notification with the escaped audit trail, its property update and every misconfiguration it skips; - can_esign, send_esign_email, the timeline cards, the Email Template extension, the Jinja method and the install, migrate and uninstall hooks. Coverage is 97.9%; fail_under goes from 0 to 80, the profile's target. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SvdEeMiewirZ3ifFoUix5F --- esign/tests/__init__.py | 2 + esign/tests/test_esign.py | 312 +++++++++++++ esign/tests/test_web_form.py | 839 +++++++++++++++++++++++++++++++++++ esign/tests/utils.py | 227 ++++++++++ pyproject.toml | 2 +- 5 files changed, 1381 insertions(+), 1 deletion(-) create mode 100644 esign/tests/__init__.py create mode 100644 esign/tests/test_esign.py create mode 100644 esign/tests/test_web_form.py create mode 100644 esign/tests/utils.py diff --git a/esign/tests/__init__.py b/esign/tests/__init__.py new file mode 100644 index 0000000..0a2b4be --- /dev/null +++ b/esign/tests/__init__.py @@ -0,0 +1,2 @@ +# Copyright (c) 2026, Avunu LLC and contributors +# For license information, please see license.txt diff --git a/esign/tests/test_esign.py b/esign/tests/test_esign.py new file mode 100644 index 0000000..e3175ac --- /dev/null +++ b/esign/tests/test_esign.py @@ -0,0 +1,312 @@ +# Copyright (c) 2026, Avunu LLC and contributors +# For license information, please see license.txt + +"""The desk side: sending a document for signature, the timeline, the hooks and install.""" + +import json +from typing import Any, cast +from unittest.mock import patch + +import frappe +from frappe.core.doctype.communication.communication import Communication +from frappe.core.doctype.file.file import File +from frappe.model.base_document import get_controller +from frappe.tests import IntegrationTestCase + +from esign.config import ( + add_field_options, + after_install, + after_migrate, + after_uninstall, + revert_field_options, +) +from esign.esign import can_esign, get_timeline_content, send_esign_email +from esign.esign.custom.email_template import EsignEmailTemplate, get_esign_email_templates +from esign.esign.custom.web_form import EsignWebForm +from esign.tests.utils import ( + OUTSIDER, + TEST_DOCTYPE, + ensure_test_doctype, + ensure_user, + make_agreement, + make_email_account, + make_pdf, + make_web_form, +) + + +def communication_type_options() -> set[str]: + return set((frappe.get_meta("Communication").get_field("communication_type").options or "").split("\n")) + + +class EsignDeskTestCase(IntegrationTestCase): + def setUp(self) -> None: + # Idempotent: the doctype and users are created once per site and kept. + ensure_test_doctype() + ensure_user(OUTSIDER) + frappe.set_user("Administrator") + self.web_form = make_web_form() + self.doc = make_agreement() + + def tearDown(self) -> None: + frappe.set_user("Administrator") + + +class TestHooks(EsignDeskTestCase): + def test_controllers_are_extended_not_overridden(self): + hooks = frappe.get_hooks(app_name="esign") + self.assertEqual( + hooks["extend_doctype_class"], + { + "Web Form": ["esign.esign.custom.web_form.EsignWebForm"], + "Email Template": ["esign.esign.custom.email_template.EsignEmailTemplate"], + }, + ) + self.assertNotIn("override_doctype_class", hooks) + self.assertTrue(issubclass(get_controller("Web Form"), EsignWebForm)) + self.assertTrue(issubclass(get_controller("Email Template"), EsignEmailTemplate)) + + def test_jinja_and_timeline_hooks(self): + hooks = frappe.get_hooks(app_name="esign") + self.assertIn("esign.esign.custom.web_form.get_esign_link", hooks["jinja"]["methods"]) + self.assertEqual(hooks["additional_timeline_content"]["*"], ["esign.esign.get_timeline_content"]) + + def test_esign_link_in_a_template(self): + html = frappe.render_template( + "{{ get_esign_link(doc, web_form) }}", {"doc": self.doc, "web_form": self.web_form.name} + ) + self.assertIn(f"/esign-test-agreement/{self.doc.name}/edit?key=", html) + + +class TestCanEsign(EsignDeskTestCase): + def test_enabled_by_a_published_esign_web_form(self): + self.assertTrue(can_esign(TEST_DOCTYPE, self.doc.name)) + self.web_form.db_set("published", 0) + self.assertFalse(can_esign(TEST_DOCTYPE, self.doc.name)) + + def test_can_esign_hooks_decide(self): + real_get_hooks = frappe.get_hooks + + def get_hooks(hook: str | None = None, *args: Any, **kwargs: Any): + if hook == "can_esign": + return {TEST_DOCTYPE: ["esign.tests.utils.allow_esign"]} + return real_get_hooks(hook, *args, **kwargs) + + with patch("frappe.get_hooks", side_effect=get_hooks): + self.assertTrue(can_esign(TEST_DOCTYPE, json.dumps({"title": "Allowed"}))) + self.assertFalse(can_esign(TEST_DOCTYPE, {"title": "Refused"})) + + +class TestSendEsignEmail(EsignDeskTestCase): + def send(self, **kwargs: Any) -> dict: + args: dict[str, Any] = { + "doctype": TEST_DOCTYPE, + "name": self.doc.name, + "recipients": "signer@example.com, other@example.com", + "subject": "Please sign", + "message": "

Hello

", + "web_form": self.web_form.name, + } + args.update(kwargs) + with patch("frappe.sendmail") as sendmail: + result = send_esign_email(**args) + self.sendmail = sendmail + return result + + def test_sends_the_link_and_records_a_communication(self): + result = self.send(cc="cc@example.com", bcc=["bcc@example.com"], print_format="Standard") + self.assertTrue(result["success"]) + self.assertIn(f"/esign-test-agreement/{self.doc.name}/edit?key=", result["esign_link"]) + self.assertIn("&format=Standard", result["esign_link"]) + + comm = cast(Communication, frappe.get_doc("Communication", result["communication"])) + self.assertEqual(comm.recipients, "signer@example.com, other@example.com") + self.assertEqual(comm.cc, "cc@example.com") + self.assertEqual(comm.bcc, "bcc@example.com") + # The stored HTML escapes the link's "&". + self.assertIn(result["esign_link"].split("&")[0], comm.content or "") + + mail = self.sendmail.call_args.kwargs + self.assertTrue(mail["now"]) + self.assertIn(result["esign_link"], mail["content"]) + + def test_link_already_in_the_message_is_not_added_again(self): + link = "https://example.com/already-there" + with patch("esign.esign.custom.web_form.get_esign_link", return_value=link): + result = self.send(message=f"sign", recipients=["signer@example.com"]) + content = frappe.db.get_value("Communication", result["communication"], "content") + self.assertEqual(content.count(link), 1) + self.assertNotIn("Sign Document", content) + + def test_copy_to_the_sender_and_email_account(self): + frappe.db.set_value("User", "Administrator", "email", "admin@example.com") + result = self.send(send_me_a_copy=True, email_account=make_email_account()) + comm = cast(Communication, frappe.get_doc("Communication", result["communication"])) + self.assertEqual(comm.sender, "esign-out@example.com") + self.assertIn("admin@example.com", comm.bcc or "") + + def test_invalid_requests_are_refused(self): + with self.assertRaises(frappe.ValidationError): + self.send(subject="") + with self.assertRaises(frappe.ValidationError): + self.send(name="missing") + with self.assertRaises(frappe.ValidationError): + self.send(web_form="No Such Web Form") + with self.assertRaises(frappe.ValidationError): + self.send(recipients=" , ") + self.web_form.db_set("esign_enabled", 0) + with self.assertRaises(frappe.ValidationError): + self.send() + + def test_failed_link_is_refused(self): + with ( + patch("esign.esign.custom.web_form.get_esign_link", return_value=""), + self.assertRaises(frappe.ValidationError), + ): + self.send() + + def test_sending_failure_is_logged(self): + with ( + patch( + "frappe.core.doctype.communication.communication.Communication.insert", + side_effect=RuntimeError("smtp"), + ), + self.assertRaises(frappe.ValidationError), + ): + self.send() + self.assertTrue( + frappe.db.exists( + "Error Log", {"method": f"eSign: Failed to send email for {TEST_DOCTYPE} {self.doc.name}"} + ) + ) + + def test_sender_needs_email_permission(self): + frappe.set_user(OUTSIDER) + with self.assertRaises(frappe.PermissionError): + self.send() + + +class TestTimeline(EsignDeskTestCase): + def test_esign_communications_become_timeline_cards(self): + audit = {"signer_name": "Sam Signer", "signer_email": "sam@example.com", "pdf_hash": "ab" * 32} + comm = frappe.get_doc( + { + "doctype": "Communication", + "communication_type": "eSign", + "subject": "Signed", + "content": json.dumps(audit), + "reference_doctype": TEST_DOCTYPE, + "reference_name": self.doc.name, + } + ).insert(ignore_permissions=True) + pdf = cast( + File, + frappe.get_doc( + { + "doctype": "File", + "file_name": "signed.pdf", + "attached_to_doctype": "Communication", + "attached_to_name": comm.name, + "content": make_pdf(), + "is_private": 1, + } + ).insert(ignore_permissions=True), + ) + frappe.get_doc( + { + "doctype": "Communication", + "communication_type": "eSign", + "subject": "Unreadable", + "content": "not json", + "sender": "fallback@example.com", + "reference_doctype": TEST_DOCTYPE, + "reference_name": self.doc.name, + } + ).insert(ignore_permissions=True) + + cards = get_timeline_content(TEST_DOCTYPE, str(self.doc.name)) + self.assertEqual(len(cards), 2) + by_signer = {card["template_data"]["signer_email"]: card for card in cards} + signed = by_signer["sam@example.com"]["template_data"] + self.assertEqual(signed["signer_name"], "Sam Signer") + self.assertEqual(signed["pdf_hash"], "ab" * 32) + self.assertEqual(signed["file_url"], pdf.file_url) + self.assertEqual(by_signer["sam@example.com"]["template"], "esign_timeline") + self.assertIsNone(by_signer["fallback@example.com"]["template_data"]["file_url"]) + self.assertEqual(get_timeline_content("ToDo", "nothing"), []) + + +class TestEmailTemplate(EsignDeskTestCase): + def test_esign_templates_and_context(self): + template = frappe.get_doc( + { + "doctype": "Email Template", + "name": "eSign Test Request", + "subject": "Please sign {{ name }}", + "response": "Sign here", + "esign_request": 1, + } + ) + if frappe.db.exists("Email Template", template.name): + frappe.delete_doc("Email Template", template.name, force=True) + template.insert(ignore_permissions=True) + + self.assertIn("Please sign {{ name }}", [t.subject for t in get_esign_email_templates()]) + loaded = cast(EsignEmailTemplate, frappe.get_doc("Email Template", str(template.name))) + self.assertEqual(loaded.get_context(frappe._dict())["esign_request"], 1) + + +class TestInstall(EsignDeskTestCase): + def test_install_and_migrate_add_the_communication_type(self): + after_install() + after_migrate() + self.assertIn("eSign", communication_type_options()) + + def test_uninstall_removes_it(self): + self.addCleanup(after_install) + after_uninstall() + frappe.clear_cache(doctype="Communication") + self.assertNotIn("eSign", communication_type_options()) + + def test_field_options_are_added_once_and_reverted(self): + add_field_options(TEST_DOCTYPE, "status", ["Countersigned"]) + add_field_options(TEST_DOCTYPE, "status", ["Countersigned"]) + setter = frappe.db.get_value( + "Property Setter", + {"doc_type": TEST_DOCTYPE, "field_name": "status", "property": "options"}, + "value", + ) + self.assertEqual(setter.split("\n").count("Countersigned"), 1) + + revert_field_options(TEST_DOCTYPE, "status", ["Countersigned"]) + setter = frappe.db.get_value( + "Property Setter", + {"doc_type": TEST_DOCTYPE, "field_name": "status", "property": "options"}, + "value", + ) + self.assertNotIn("Countersigned", setter.split("\n")) + + def test_reverting_the_last_option_deletes_the_property_setter(self): + frappe.make_property_setter( + { + "doctype": TEST_DOCTYPE, + "doctype_or_field": "DocField", + "fieldname": "title", + "property": "options", + "value": "Only", + "property_type": "Text", + } + ) + revert_field_options(TEST_DOCTYPE, "title", ["Only"]) + self.assertFalse( + frappe.db.exists( + "Property Setter", {"doc_type": TEST_DOCTYPE, "field_name": "title", "property": "options"} + ) + ) + revert_field_options(TEST_DOCTYPE, "title", ["Only"]) + + def test_unknown_field_is_ignored(self): + add_field_options(TEST_DOCTYPE, "no_such_field", ["X"]) + self.assertFalse( + frappe.db.exists("Property Setter", {"doc_type": TEST_DOCTYPE, "field_name": "no_such_field"}) + ) diff --git a/esign/tests/test_web_form.py b/esign/tests/test_web_form.py new file mode 100644 index 0000000..22ed6eb --- /dev/null +++ b/esign/tests/test_web_form.py @@ -0,0 +1,839 @@ +# Copyright (c) 2026, Avunu LLC and contributors +# For license information, please see license.txt + +"""The signing flow: the web form page, its key checks, accept and the signing jobs.""" + +import json +from typing import Any, cast +from unittest.mock import MagicMock, patch + +import frappe +from frappe.model.document import Document +from frappe.model.meta import Meta +from frappe.tests import IntegrationTestCase +from frappe.website.serve import get_response_content + +from esign.esign.custom import web_form as esign_web_form +from esign.esign.custom.web_form import ( + AuditData, + EsignWebForm, + RequestData, + accept, + attach_print_to_document, + download_signed_pdf, + extract_param_from_referrer, + get_esign_link, + get_esign_web_forms, + get_print_html, + record_signature, + validate_esign_key, +) +from esign.tests.utils import ( + OUTSIDER, + PLACEHOLDER_SIGNATURE, + SIGNATURE, + SIGNER, + TEST_DOCTYPE, + WEB_FORM_ROUTE, + ensure_test_doctype, + ensure_user, + form_data, + get_agreement, + make_agreement, + make_email_account, + make_pdf, + make_web_form, + share_key, + signing_request, +) + +PDF = make_pdf() + +# The signing request as _enqueue_print_attachment snapshots it for the jobs. +REQUEST_DATA: RequestData = { + "host_url": "https://erp.example.com/", + "scheme": "https", + "host": "erp.example.com", + "headers": {"User-Agent": "TestBrowser/1.0"}, + "method": "POST", +} + + +def fake_attach_print(**kwargs: Any) -> dict: + return {"fname": f"{kwargs['file_name']}.pdf", "fcontent": PDF} + + +class EsignTestCase(IntegrationTestCase): + def setUp(self) -> None: + # Idempotent: the doctype and users are created once per site and kept. + ensure_test_doctype() + ensure_user(OUTSIDER) + ensure_user(SIGNER, ["System Manager"]) + frappe.set_user("Administrator") + frappe.conf.disable_website_cache = True + frappe.local.form_dict = frappe._dict() + self.web_form = cast(EsignWebForm, make_web_form()) + self.web_form_name = str(self.web_form.name) + self.doc = make_agreement() + self.key = share_key(self.doc) + + def tearDown(self) -> None: + frappe.set_user("Administrator") + frappe.conf.disable_website_cache = False + frappe.local.request = None + frappe.local.form_dict = frappe._dict() + frappe.flags.ignore_print_permissions = False + frappe.local.message_log = [] + + def reload_web_form(self) -> EsignWebForm: + return cast(EsignWebForm, frappe.get_doc("Web Form", self.web_form_name)) + + +class TestEsignWebFormController(EsignTestCase): + def test_controller_is_composed_onto_web_form(self): + self.assertIsInstance(frappe.get_doc("Web Form", self.web_form_name), EsignWebForm) + + def test_validate_forces_allow_edit(self): + self.assertTrue(self.reload_web_form().allow_edit) + + plain = cast(EsignWebForm, make_web_form(esign_enabled=0, allow_edit=0)) + self.assertFalse(plain.allow_edit) + + def test_signature_exists_only_when_every_signature_field_is_signed(self): + wf = self.reload_web_form() + self.assertFalse(wf._check_signature_exists(self.doc)) + + self.doc.db_set("signature", PLACEHOLDER_SIGNATURE) + self.assertFalse(wf._check_signature_exists(self.doc)) + + self.doc.db_set("signature", SIGNATURE) + self.assertTrue(wf._check_signature_exists(self.doc)) + + wf.web_form_fields = [row for row in wf.web_form_fields if row.fieldtype != "Signature"] + self.assertFalse(wf._check_signature_exists(self.doc)) + + def test_signed_pdf_lookup_prefers_the_esign_communication(self): + wf = self.reload_web_form() + self.assertIsNone(wf._get_signed_pdf(self.doc)) + + direct = frappe.get_doc( + { + "doctype": "File", + "file_name": f"{self.doc.name}_signed_1.pdf", + "attached_to_doctype": TEST_DOCTYPE, + "attached_to_name": self.doc.name, + "is_private": 1, + "content": PDF, + } + ).insert(ignore_permissions=True) + self.assertEqual((wf._get_signed_pdf(self.doc) or {}).get("name"), direct.name) + + comm = frappe.get_doc( + { + "doctype": "Communication", + "communication_type": "eSign", + "subject": "Signed", + "reference_doctype": TEST_DOCTYPE, + "reference_name": self.doc.name, + } + ).insert(ignore_permissions=True) + on_comm = frappe.get_doc( + { + "doctype": "File", + "file_name": "copy.pdf", + "attached_to_doctype": "Communication", + "attached_to_name": comm.name, + "is_private": 1, + "content": make_pdf(pages=2), + } + ).insert(ignore_permissions=True) + self.assertEqual((wf._get_signed_pdf(self.doc) or {}).get("name"), on_comm.name) + + +class TestKeyValidation(EsignTestCase): + def test_valid_key_grants_access(self): + frappe.set_user("Guest") + self.assertTrue(validate_esign_key(self.key, self.doc)) + + def test_wrong_key_is_refused(self): + frappe.set_user("Guest") + with self.assertRaises(frappe.PermissionError): + validate_esign_key("not-a-key", self.doc) + + def test_missing_key_is_refused_when_required(self): + with self.assertRaises(frappe.PermissionError): + validate_esign_key("", self.doc, require_key=True) + + def test_missing_key_falls_back_to_print_permission(self): + self.assertTrue(validate_esign_key("", self.doc)) + frappe.set_user("Guest") + with self.assertRaises(frappe.PermissionError): + validate_esign_key("", get_agreement(self.doc.name)) + + def test_has_web_form_permission_accepts_the_key_from_the_referrer(self): + wf = self.reload_web_form() + frappe.set_user("Guest") + with signing_request(referrer=f"https://example.com/{WEB_FORM_ROUTE}/x/edit?key={self.key}"): + self.assertTrue(wf.has_web_form_permission(TEST_DOCTYPE, self.doc.name, "write")) + self.assertEqual(frappe.form_dict.key, self.key) + + def test_has_web_form_permission_falls_back_on_a_bad_key(self): + wf = self.reload_web_form() + frappe.set_user("Guest") + with signing_request(key="wrong"): + self.assertFalse(wf.has_web_form_permission(TEST_DOCTYPE, self.doc.name, "write")) + with signing_request(): + self.assertFalse(wf.has_web_form_permission(TEST_DOCTYPE, self.doc.name, "write")) + + def test_referrer_parameters(self): + with signing_request(referrer="https://example.com/form?key=abc&email=a%40b.co&key=ignored"): + self.assertEqual(extract_param_from_referrer("key"), "abc") + self.assertEqual(extract_param_from_referrer("email"), "a@b.co") + self.assertEqual(extract_param_from_referrer("format"), "") + with signing_request(): + self.assertEqual(extract_param_from_referrer("key"), "") + with ( + signing_request(referrer="https://example.com/?key=x"), + patch("urllib.parse.urlparse", side_effect=ValueError("bad url")), + ): + self.assertEqual(extract_param_from_referrer("key"), "") + + +class TestSigningPage(EsignTestCase): + def render(self, **form_dict: Any) -> str: + path = f"{WEB_FORM_ROUTE}/{self.doc.name}/edit" + # The test bench may have no built assets; bundle names then render as they are. + with ( + signing_request(path=path, method="GET", name=self.doc.name, **form_dict), + patch("frappe.utils.get_assets_json", return_value={}), + ): + return get_response_content(path) + + def test_guest_with_key_gets_the_signing_page(self): + frappe.set_user("Guest") + content = self.render(key=self.key, email="signer@example.com", format="Standard") + + self.assertIn("esign-layout", content) + self.assertIn(f'key: "{self.key}"', content) + self.assertIn('print_format: "Standard"', content) + self.assertIn("signer@example.com", content) + self.assertIn("esign.web.bundle", content) + self.assertIn("frappe.utils.print_format.download_pdf", content) + self.assertNotIn("esign-completed", content) + + def test_guest_without_key_is_refused(self): + frappe.set_user("Guest") + content = self.render() + self.assertNotIn("esign-layout", content) + + def test_completed_document_links_the_signed_pdf(self): + self.doc.db_set("signature", SIGNATURE) + frappe.get_doc( + { + "doctype": "File", + "file_name": f"{self.doc.name}_signed_1.pdf", + "attached_to_doctype": TEST_DOCTYPE, + "attached_to_name": self.doc.name, + "is_private": 1, + "content": PDF, + } + ).insert(ignore_permissions=True) + frappe.set_user("Guest") + content = self.render(key=self.key) + + self.assertIn("esign-completed", content) + self.assertIn("esign.esign.custom.web_form.download_signed_pdf", content) + self.assertIn(f"docname={self.doc.name}", content) + + def test_force_mobile_adds_a_qr_code(self): + self.web_form.db_set("esign_force_mobile", 1) + frappe.set_user("Guest") + content = self.render(key=self.key) + self.assertIn("esign-force-mobile", content) + self.assertIn("data:image/svg+xml;base64,", content) + + def test_disabled_esign_renders_the_standard_web_form(self): + self.web_form.db_set("esign_enabled", 0) + content = self.render() + self.assertNotIn("esign-layout", content) + + +class TestEsignLink(EsignTestCase): + def assert_valid_link(self, link: str) -> None: + base = frappe.utils.get_url().rstrip("/") + prefix = f"{base}/{WEB_FORM_ROUTE}/{self.doc.name}/edit?key=" + self.assertTrue(link.startswith(prefix), link) + key = link.removeprefix(prefix).split("&")[0] + frappe.set_user("Guest") + self.assertTrue(validate_esign_key(key, get_agreement(self.doc.name))) + frappe.set_user("Administrator") + + def test_link_carries_key_format_and_email(self): + link = get_esign_link(self.doc, self.web_form_name, "Standard", "a+b@example.com") + self.assert_valid_link(link) + self.assertIn("&format=Standard", link) + self.assertIn("&email=a%2Bb%40example.com", link) + + def test_link_from_a_request_payload(self): + payload = json.dumps({"doctype": TEST_DOCTYPE, "name": self.doc.name}) + self.assert_valid_link(get_esign_link(payload, self.web_form_name)) + + def test_request_payload_needs_email_permission(self): + frappe.set_user(OUTSIDER) + with self.assertRaises(frappe.PermissionError): + get_esign_link({"doctype": TEST_DOCTYPE, "name": self.doc.name}, self.web_form_name) + + def test_bad_payloads_return_the_error(self): + self.assertTrue(get_esign_link("{not json", self.web_form_name)) + self.assertIn("not found", get_esign_link({"doctype": TEST_DOCTYPE, "name": "missing"}, "x").lower()) + + def test_no_link_for_a_form_that_does_not_match(self): + self.web_form.db_set("esign_enabled", 0) + self.assertEqual(get_esign_link(self.doc, self.web_form_name), "") + self.assertEqual(get_esign_link(self.doc, "No Such Web Form"), "") + + def test_web_forms_for_a_doctype(self): + forms = get_esign_web_forms(TEST_DOCTYPE) + self.assertEqual([f.name for f in forms], [self.web_form_name]) + self.assertEqual(get_esign_web_forms("ToDo"), []) + + +class TestPrintHtml(EsignTestCase): + def test_guest_with_key_gets_the_print_html(self): + frappe.set_user("Guest") + result = get_print_html(TEST_DOCTYPE, self.doc.name, key=self.key) + self.assertIn("Service agreement", result["print_html"]) + self.assertIn("print_style", result) + self.assertEqual(frappe.form_dict.key, self.key) + + def test_guest_without_key_is_refused(self): + frappe.set_user("Guest") + with self.assertRaises(frappe.PermissionError): + get_print_html(TEST_DOCTYPE, self.doc.name) + self.assertFalse(frappe.flags.ignore_print_permissions) + + def test_guest_with_a_wrong_key_is_refused(self): + frappe.set_user("Guest") + with self.assertRaises(frappe.PermissionError): + get_print_html(TEST_DOCTYPE, self.doc.name, key="wrong") + + def test_user_with_print_permission_needs_no_key(self): + result = get_print_html(TEST_DOCTYPE, self.doc.name) + self.assertIn("Service agreement", result["print_html"]) + + +class TestDownloadSignedPdf(EsignTestCase): + def attach_signed_pdf(self, **kwargs: Any): + return frappe.get_doc( + { + "doctype": "File", + "file_name": f"{self.doc.name}_signed_1.pdf", + "attached_to_doctype": TEST_DOCTYPE, + "attached_to_name": self.doc.name, + "content": PDF, + "is_private": 1, + **kwargs, + } + ).insert(ignore_permissions=True) + + def test_guest_needs_a_key(self): + self.attach_signed_pdf() + frappe.set_user("Guest") + with signing_request(method="GET"), self.assertRaises(frappe.PermissionError): + download_signed_pdf(TEST_DOCTYPE, self.doc.name) + + def test_no_signed_pdf(self): + with signing_request(method="GET"), self.assertRaises(frappe.DoesNotExistError): + download_signed_pdf(TEST_DOCTYPE, self.doc.name, key=self.key) + + def test_private_file_is_sent_and_logged(self): + file = self.attach_signed_pdf() + frappe.set_user("Guest") + with ( + signing_request(method="GET"), + patch("frappe.utils.response.send_private_file", return_value="sent") as send, + ): + self.assertEqual(download_signed_pdf(TEST_DOCTYPE, self.doc.name, key=self.key), "sent") + self.assertTrue(send.call_args.args[0].startswith("/files/")) + self.assertTrue( + frappe.db.exists("Access Log", {"export_from": "File", "reference_document": file.name}) + ) + + def test_public_file_redirects(self): + file = self.attach_signed_pdf(is_private=0) + with signing_request(method="GET"): + download_signed_pdf(TEST_DOCTYPE, self.doc.name, key=self.key) + self.assertEqual(frappe.local.response["type"], "redirect") + self.assertEqual(frappe.local.response["location"], file.file_url) + + def test_cloud_storage_file_redirects_to_a_stored_sharing_link(self): + file = self.attach_signed_pdf() + frappe.db.set_value( + "File", file.name, "file_url", "/api/method/retrieve?key=abc", update_modified=False + ) + stored: dict[str, str] = {} + full_file = MagicMock() + full_file.get.return_value = None + full_file.db_set.side_effect = lambda field, value: stored.__setitem__(field, value) + real_get_doc = frappe.get_doc + + def get_doc(*args: Any, **kwargs: Any): + if args == ("File", file.name): + return full_file + return real_get_doc(*args, **kwargs) + + frappe.local.flags.commit = False + with signing_request(method="GET"), patch("frappe.get_doc", side_effect=get_doc): + download_signed_pdf(TEST_DOCTYPE, self.doc.name, key=self.key) + + self.assertEqual(frappe.local.response["type"], "redirect") + self.assertEqual(frappe.local.response["location"], f"/api/method/share?key={stored['sharing_link']}") + self.assertTrue(frappe.local.flags.commit) + + # An existing link is reused, not replaced. + full_file.get.return_value = "1234" + full_file.db_set.reset_mock() + with signing_request(method="GET"), patch("frappe.get_doc", side_effect=get_doc): + download_signed_pdf(TEST_DOCTYPE, self.doc.name, key=self.key) + self.assertEqual(frappe.local.response["location"], "/api/method/share?key=1234") + full_file.db_set.assert_not_called() + frappe.local.flags.commit = False + + +def make_notification(**kwargs: Any): + name = "eSign Test Signed" + if frappe.db.exists("Notification", name): + frappe.delete_doc("Notification", name, force=True, ignore_permissions=True) + return frappe.get_doc( + { + "doctype": "Notification", + "name": name, + "subject": "Signed: {{ doc.title }}", + "document_type": TEST_DOCTYPE, + "event": "Custom", + "channel": "Email", + "enabled": 1, + "message": "

{{ doc.name }} was signed

", + "recipients": [{"receiver_by_document_field": "signer_email"}], + **kwargs, + } + ).insert(ignore_permissions=True) + + +class TestAccept(EsignTestCase): + """accept: the signature flow and its Guest session switch.""" + + def accept(self, data: str, key: str | None = None, **request: Any): + referrer = f"https://example.com/{WEB_FORM_ROUTE}/{self.doc.name}/edit?key={key}" if key else None + with signing_request(referrer=referrer, **request), patch("frappe.enqueue") as enqueue: + result = accept(self.web_form_name, data) + return result, enqueue + + def test_guest_with_key_signs_and_queues_the_signed_pdf(self): + frappe.set_user("Guest") + result, enqueue = self.accept( + form_data(self.doc, signature=SIGNATURE, signer_email="signer@example.com"), + key=self.key, + headers={"User-Agent": "TestBrowser/1.0", "X-Forwarded-For": "203.0.113.9, 10.0.0.1"}, + ) + + self.doc.reload() + self.assertEqual(self.doc.signature, SIGNATURE) + self.assertEqual(self.doc.signer_email, "signer@example.com") + self.assertEqual(result.name, self.doc.name) + self.assertEqual(frappe.session.user, "Guest") + self.assertIs(frappe.flags.web_form_doc, result) + + job = enqueue.call_args + self.assertIs(job.args[0], attach_print_to_document) + self.assertEqual(job.kwargs["print_format"], "standard") + self.assertTrue(job.kwargs["enqueue_after_commit"]) + self.assertEqual(job.kwargs["request_data"]["headers"]["User-Agent"], "TestBrowser/1.0") + audit = job.kwargs["audit_data"] + self.assertEqual(audit["ip_address"], "203.0.113.9") + self.assertEqual(audit["user_agent"], "TestBrowser/1.0") + self.assertEqual(audit["signer_email"], "signer@example.com") + self.assertEqual(audit["signed_fields"], ["Signature"]) + self.assertEqual(audit["web_form"], "eSign Test Agreement") + self.assertIsNone(job.kwargs["notification_name"]) + + def test_guest_without_key_cannot_update(self): + frappe.set_user("Guest") + with self.assertRaises(frappe.PermissionError): + self.accept(form_data(self.doc, signature=SIGNATURE)) + self.doc.reload() + self.assertFalse(self.doc.signature) + + def test_guest_with_wrong_key_cannot_update(self): + frappe.set_user("Guest") + with self.assertRaises(frappe.PermissionError): + self.accept(form_data(self.doc, signature=SIGNATURE), key="wrong") + + def test_user_without_permission_cannot_update(self): + frappe.set_user(OUTSIDER) + with self.assertRaises(frappe.PermissionError): + self.accept(form_data(self.doc, signature=SIGNATURE)) + + def test_anonymous_form_saves_as_guest_and_restores_the_user(self): + self.web_form.db_set("anonymous", 1) + frappe.set_user(SIGNER) + seen: list[str] = [] + original_save = Document.save + + def save(doc, *args: Any, **kwargs: Any): + seen.append(frappe.session.user) + return original_save(doc, *args, **kwargs) + + with patch.object(Document, "save", save): + self.accept(form_data(self.doc, signature=SIGNATURE), key=self.key) + + self.assertEqual(seen[0], "Guest") + self.assertEqual(frappe.session.user, SIGNER) + + def test_anonymous_form_restores_the_user_when_saving_fails(self): + self.web_form.db_set("anonymous", 1) + frappe.set_user(SIGNER) + with ( + patch.object(Document, "save", side_effect=frappe.ValidationError("boom")), + self.assertRaises(frappe.ValidationError), + ): + self.accept(form_data(self.doc, signature=SIGNATURE), key=self.key) + self.assertEqual(frappe.session.user, SIGNER) + + def test_submit_on_response_and_update_field(self): + self.web_form.db_set( + {"esign_submit_on_response": 1, "esign_update_field": "status", "esign_update_value": "Signed"} + ) + frappe.set_user("Guest") + self.accept(form_data(self.doc, signature=SIGNATURE), key=self.key) + + self.doc.reload() + self.assertEqual(self.doc.docstatus, 1) + self.assertEqual(self.doc.status, "Signed") + + def test_update_field_of_an_unsupported_type_is_left_alone(self): + self.web_form.db_set({"esign_update_field": "rating", "esign_update_value": "5"}) + self.accept(form_data(self.doc, signature=SIGNATURE)) + self.doc.reload() + self.assertEqual(self.doc.rating, 0) + + def test_update_field_failure_is_logged(self): + self.web_form.db_set({"esign_update_field": "status", "esign_update_value": "Signed"}) + real_has_field = Meta.has_field + + def has_field(meta, fieldname: str) -> bool: + if meta.name == TEST_DOCTYPE and fieldname == "status": + raise RuntimeError("meta") + return real_has_field(meta, fieldname) + + with patch.object(Meta, "has_field", has_field): + self.accept(form_data(self.doc, signature=SIGNATURE)) + self.doc.reload() + self.assertEqual(self.doc.signature, SIGNATURE) + self.assertTrue( + frappe.db.exists("Error Log", {"method": ("like", "eSign: Failed to set field status%")}) + ) + + def test_updates_need_allow_edit(self): + web_form = self.reload_web_form() + web_form.allow_edit = 0 + with patch("frappe.get_lazy_doc", return_value=web_form), self.assertRaises(frappe.ValidationError): + self.accept(form_data(self.doc, signature=SIGNATURE)) + + def test_new_document_from_the_form(self): + data = json.dumps( + {"doctype": TEST_DOCTYPE, "signature": SIGNATURE, "signer_email": "new@example.com"} + ) + frappe.set_user("Guest") + result, enqueue = self.accept(data) + self.assertTrue(frappe.db.exists(TEST_DOCTYPE, result.name)) + self.assertEqual(enqueue.call_count, 1) + + def test_new_document_needs_login_when_the_form_does(self): + self.web_form.db_set("login_required", 1) + frappe.set_user("Guest") + with self.assertRaises(frappe.ValidationError): + self.accept(json.dumps({"doctype": TEST_DOCTYPE, "signature": SIGNATURE})) + + def test_attachments_are_saved_replaced_and_removed(self): + self.web_form.append( + "web_form_fields", {"fieldname": "attachment", "fieldtype": "Attach", "label": "File"} + ) + self.web_form.save(ignore_permissions=True) + + self.accept(form_data(self.doc, attachment="notes.txt,data:text/plain;base64,aGVsbG8=")) + self.doc.reload() + first_url = self.doc.attachment + self.assertTrue(first_url) + self.assertTrue(frappe.db.exists("File", {"file_url": first_url, "attached_to_name": self.doc.name})) + + # frappe also records an attach field's File on save, so each upload has two rows; + # replacing or clearing the field removes one of them. + def files(url: str | None) -> int: + return frappe.db.count("File", {"file_url": url, "attached_to_name": self.doc.name}) + + first_count = files(first_url) + self.accept(form_data(self.doc, attachment="more.txt,data:text/plain;base64,d29ybGQ=")) + self.doc.reload() + self.assertNotEqual(self.doc.attachment, first_url) + self.assertLess(files(first_url), first_count) + + second_url = self.doc.attachment + second_count = files(second_url) + self.accept(form_data(self.doc, attachment="")) + self.doc.reload() + self.assertFalse(self.doc.attachment) + self.assertLess(files(second_url), second_count) + + def test_new_document_with_an_attachment(self): + self.web_form.append( + "web_form_fields", {"fieldname": "attachment", "fieldtype": "Attach", "label": "File"} + ) + self.web_form.save(ignore_permissions=True) + data = json.dumps( + {"doctype": TEST_DOCTYPE, "attachment": "notes.txt,data:text/plain;base64,aGVsbG8="} + ) + result, _enqueue = self.accept(data) + self.assertTrue(frappe.db.get_value(TEST_DOCTYPE, result.name, "attachment")) + + def test_form_without_esign_uses_frappes_accept(self): + self.web_form.db_set("esign_enabled", 0) + with patch("frappe.website.doctype.web_form.web_form.accept", return_value="base") as base: + result, _enqueue = self.accept(form_data(self.doc, signature=SIGNATURE)) + self.assertEqual(result, "base") + self.assertEqual(json.loads(base.call_args.args[1])["name"], self.doc.name) + + def test_signer_email_resolution(self): + wf = self.reload_web_form() + self.doc.signer_email = "field@example.com" + + with signing_request(referrer="https://example.com/form?email=param%40example.com"): + self.assertEqual( + esign_web_form._collect_audit_data(self.doc, wf, "Standard")["signer_email"], + "param@example.com", + ) + with signing_request(): + self.assertEqual( + esign_web_form._collect_audit_data(self.doc, wf, "Standard")["signer_email"], + "field@example.com", + ) + + self.doc.signer_email = None + frappe.set_user(SIGNER) + with signing_request(): + audit = esign_web_form._collect_audit_data(self.doc, wf, "Standard") + self.assertEqual(audit["signer_email"], SIGNER) + self.assertEqual(audit["signer_name"], "esign-signer") + + frappe.set_user("Guest") + with signing_request(): + audit = esign_web_form._collect_audit_data(self.doc, wf, "Standard") + self.assertIsNone(audit["signer_email"]) + self.assertEqual(audit["ip_address"], "127.0.0.1") + self.assertIsNone(audit["user_agent"]) + + def test_client_ip_headers_in_order(self): + with signing_request(headers={"CF-Connecting-IP": "198.51.100.1", "X-Real-IP": "198.51.100.2"}): + self.assertEqual(esign_web_form._get_client_ip(), "198.51.100.1") + with signing_request(headers={"X-Real-IP": " 198.51.100.2 "}): + self.assertEqual(esign_web_form._get_client_ip(), "198.51.100.2") + with signing_request(): + self.assertEqual(esign_web_form._get_client_ip(), "127.0.0.1") + + def test_print_format_and_notification_come_from_the_link_and_form(self): + notification = make_notification() + self.web_form.db_set({"esign_send_notification": 1, "esign_notification": notification.name}) + with ( + signing_request(referrer=f"https://example.com/x?key={self.key}&format=Signed%20Format"), + patch("frappe.enqueue") as enqueue, + ): + accept(self.web_form_name, form_data(self.doc, signature=SIGNATURE)) + self.assertEqual(enqueue.call_args.kwargs["print_format"], "Signed Format") + self.assertEqual(enqueue.call_args.kwargs["notification_name"], notification.name) + + +class TestSigningJobs(EsignTestCase): + """attach_print_to_document and record_signature, the two background jobs.""" + + def audit(self, **kwargs: Any) -> AuditData: + values: dict[str, Any] = { + "timestamp": "2026-10-16 12:00:00", + "ip_address": "203.0.113.9", + "user_agent": "TestBrowser", + "signer_email": "signer@example.com", + "signer_name": "Sam Signer", + "web_form": "eSign Test Agreement", + "print_format": "Standard", + "signed_fields": ["Signature"], + } + values.update(kwargs) + return cast(AuditData, values) + + def run_jobs(self, notification_name: str | None = None) -> MagicMock: + frappe.local.request = None + with ( + patch("frappe.attach_print", side_effect=fake_attach_print), + patch("frappe.enqueue") as enqueue, + ): + attach_print_to_document( + self.doc, "Standard", REQUEST_DATA, self.audit(), notification_name=notification_name + ) + self.assertIsNone(getattr(frappe.local, "request", None)) + job = enqueue.call_args + self.assertIs(job.args[0], record_signature) + self.assertTrue(job.kwargs["enqueue_after_commit"]) + record_signature( + doctype=job.kwargs["doctype"], + name=job.kwargs["name"], + file_name=job.kwargs["file_name"], + pdf_hash=job.kwargs["pdf_hash"], + request_data=job.kwargs["request_data"], + audit_data=job.kwargs["audit_data"], + notification_name=job.kwargs["notification_name"], + ) + return enqueue + + def test_signed_pdf_is_attached_and_recorded(self): + self.doc.db_set("signer_email", "signer@example.com") + self.run_jobs() + + files = frappe.get_all( + "File", + filters={"attached_to_doctype": TEST_DOCTYPE, "attached_to_name": self.doc.name}, + fields=["name", "file_name", "is_private", "content_hash"], + ) + self.assertEqual(len(files), 1) + self.assertTrue(files[0].file_name.startswith(f"{self.doc.name}_signed_")) + self.assertTrue(files[0].is_private) + + comm = frappe.get_last_doc( + "Communication", filters={"reference_name": self.doc.name, "communication_type": "eSign"} + ) + audit = json.loads(comm.content) + self.assertEqual(audit["signer_email"], "signer@example.com") + self.assertEqual(audit["file_name"], files[0].file_name) + self.assertEqual(len(audit["pdf_hash"]), 64) + self.assertEqual(comm.sender, "signer@example.com") + + copy = frappe.get_all( + "File", + filters={"attached_to_doctype": "Communication", "attached_to_name": comm.name}, + fields=["content_hash"], + ) + self.assertEqual(copy[0].content_hash, files[0].content_hash) + + def test_request_is_restored_after_the_job(self): + sentinel = frappe._dict(headers={}) + frappe.local.request = sentinel + with esign_web_form._job_request(REQUEST_DATA): + self.assertEqual(frappe.local.request.host, "erp.example.com") + self.assertIs(frappe.local.request, sentinel) + + def test_notification_is_sent_with_the_signed_pdf_and_audit_trail(self): + notification = make_notification(set_property_after_alert="status", property_value="Notified") + self.doc.db_set("signer_email", "signer@example.com") + with patch("frappe.sendmail") as sendmail: + self.run_jobs(notification.name) + + mail = sendmail.call_args.kwargs + self.assertEqual(mail["recipients"], ["signer@example.com"]) + self.assertEqual(mail["subject"], "Signed: Service agreement") + self.assertIn(f"{self.doc.name} was signed", mail["message"]) + self.assertIn("eSign Audit Trail", mail["message"]) + self.assertIn("<b>TestBrowser</b>", mail["message"]) + self.assertNotIn("TestBrowser", mail["message"]) + self.assertIn("PDF SHA-256", mail["message"]) + self.assertTrue(mail["attachments"][0]["file_url"].startswith("/private/files/")) + self.assertTrue(mail["communication"]) + self.assertEqual(frappe.db.get_value(TEST_DOCTYPE, self.doc.name, "status"), "Notified") + + def test_notification_numeric_property_and_sender(self): + notification = make_notification( + set_property_after_alert="rating", + property_value="4", + sender=make_email_account(), + sender_email="sales@example.com", + ) + self.doc.db_set("signer_email", "signer@example.com") + with patch("frappe.sendmail") as sendmail: + esign_web_form._send_esign_notification( + notification.name, self.doc, MagicMock(file_url="/files/x.pdf") + ) + self.assertEqual(sendmail.call_args.kwargs["sender"], "eSign Test Outgoing ") + self.assertNotIn("eSign Audit Trail", sendmail.call_args.kwargs["message"]) + self.assertEqual(frappe.db.get_value(TEST_DOCTYPE, self.doc.name, "rating"), 4) + + def test_notification_property_update_failure_is_logged(self): + notification = make_notification(set_property_after_alert="status", property_value="Not An Option") + self.doc.db_set("signer_email", "signer@example.com") + with patch("frappe.sendmail"): + esign_web_form._send_esign_notification( + notification.name, self.doc, MagicMock(file_url="/files/x.pdf") + ) + self.assertTrue( + frappe.db.exists("Error Log", {"method": "eSign: Document update failed after notification"}) + ) + + def test_notification_without_recipients_sends_nothing(self): + notification = make_notification() + with patch("frappe.sendmail") as sendmail: + esign_web_form._send_esign_notification( + notification.name, self.doc, MagicMock(file_url="/files/x.pdf") + ) + sendmail.assert_not_called() + + def test_misconfigured_notifications_are_logged_and_skipped(self): + cases = { + "eSign: Notification doctype mismatch": {"document_type": "ToDo"}, + "eSign: Notification event not Custom": {"event": "New"}, + "eSign: Notification disabled": {"enabled": 0}, + "eSign: Notification channel not Email": {"channel": "System Notification"}, + } + for title, values in cases.items(): + with self.subTest(title): + notification = make_notification(**values) + with patch("frappe.sendmail") as sendmail: + esign_web_form._send_esign_notification( + notification.name, self.doc, MagicMock(file_url="/x") + ) + sendmail.assert_not_called() + self.assertTrue(frappe.db.exists("Error Log", {"method": title})) + + def test_notification_errors_are_logged(self): + esign_web_form._send_esign_notification("No Such Notification", self.doc, MagicMock(file_url="/x")) + self.assertTrue( + frappe.db.exists( + "Error Log", + {"method": f"eSign: Failed to send notification for {TEST_DOCTYPE} {self.doc.name}"}, + ) + ) + + def test_cloud_storage_file_gets_an_association(self): + appended: list[tuple[str, dict]] = [] + + def fake_insert(file_doc, *args: Any, **kwargs: Any): + file_doc.name = "cloud-file" + file_doc.file_url = "/api/method/retrieve?key=bucket/key.pdf" + return file_doc + + cloud_file = MagicMock(file_name="x_signed.pdf") + cloud_file.append.side_effect = lambda table, row: appended.append((table, row)) + real_get_doc = frappe.get_doc + + def get_doc(*args: Any, **kwargs: Any): + if args == ("File", "cloud-file"): + return cloud_file + return real_get_doc(*args, **kwargs) + + with ( + patch.object(esign_web_form, "_is_cloud_storage_file", return_value=True), + patch("frappe.core.doctype.file.file.File.insert", fake_insert), + patch("frappe.core.doctype.file.file.File.db_set") as db_set, + patch("frappe.get_doc", side_effect=get_doc), + ): + self.run_jobs() + + db_set.assert_called_once_with("s3_key", "bucket/key.pdf") + self.assertEqual(appended[0][0], "file_association") + self.assertEqual(appended[0][1]["link_doctype"], "Communication") + cloud_file.save.assert_called_once() diff --git a/esign/tests/utils.py b/esign/tests/utils.py new file mode 100644 index 0000000..02abf52 --- /dev/null +++ b/esign/tests/utils.py @@ -0,0 +1,227 @@ +# Copyright (c) 2026, Avunu LLC and contributors +# For license information, please see license.txt + +"""Fixtures shared by the eSign tests: a submittable test doctype, an eSign web form on it, +documents to sign and a request to sign them from.""" + +import json +from collections.abc import Generator +from contextlib import contextmanager +from io import BytesIO +from typing import Any, cast + +import frappe +from frappe.core.doctype.user.user import User +from frappe.model.document import Document +from frappe.utils import set_request +from pypdf import PdfWriter + + +class Agreement(Document): + """The test doctype's fields, for the type checker only: the doctype is custom, so its + documents are plain Documents at runtime and this class is never instantiated.""" + + title: str | None + signer_email: str | None + signature: str | None + status: str | None + rating: int + agreed_on: str | None + attachment: str | None + + +TEST_DOCTYPE = "eSign Test Agreement" +WEB_FORM_ROUTE = "esign-test-agreement" +OUTSIDER = "esign-outsider@example.com" +SIGNER = "esign-signer@example.com" + +# A 1x1 transparent PNG, the shape the signature control posts. +SIGNATURE = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg==" +PLACEHOLDER_SIGNATURE = "/assets/frappe/images/signature-placeholder.png" + + +def make_pdf(pages: int = 1) -> bytes: + """A valid PDF: frappe checks an uploaded PDF parses before it saves the File.""" + writer = PdfWriter() + for _ in range(pages): + writer.add_blank_page(width=72, height=72) + buffer = BytesIO() + writer.write(buffer) + return buffer.getvalue() + + +def ensure_test_doctype() -> None: + """Create the custom, submittable doctype the tests sign (a DDL change, so it commits).""" + if frappe.db.exists("DocType", TEST_DOCTYPE): + return + frappe.get_doc( + { + "doctype": "DocType", + "name": TEST_DOCTYPE, + "module": "eSign", + "custom": 1, + "is_submittable": 1, + "autoname": "hash", + "fields": [ + {"fieldname": "title", "fieldtype": "Data", "label": "Title"}, + { + "fieldname": "signer_email", + "fieldtype": "Data", + "options": "Email", + "label": "Signer Email", + "allow_on_submit": 1, + }, + { + "fieldname": "signature", + "fieldtype": "Signature", + "label": "Signature", + "allow_on_submit": 1, + }, + { + "fieldname": "status", + "fieldtype": "Select", + "options": "\nDraft\nSigned\nNotified", + "label": "Status", + "allow_on_submit": 1, + }, + {"fieldname": "rating", "fieldtype": "Int", "label": "Rating", "allow_on_submit": 1}, + {"fieldname": "agreed_on", "fieldtype": "Date", "label": "Agreed On", "allow_on_submit": 1}, + { + "fieldname": "attachment", + "fieldtype": "Attach", + "label": "Attachment", + "allow_on_submit": 1, + }, + ], + "permissions": [ + { + "role": "System Manager", + "read": 1, + "write": 1, + "create": 1, + "delete": 1, + "submit": 1, + "cancel": 1, + "email": 1, + "print": 1, + "share": 1, + } + ], + } + ).insert(ignore_permissions=True) + + +def ensure_user(email: str, roles: list[str] | None = None) -> None: + """A user with the given roles (none: a website user who can open no desk document).""" + if frappe.db.exists("User", email): + return + user = frappe.get_doc( + { + "doctype": "User", + "email": email, + "first_name": email.split("@")[0], + "send_welcome_email": 0, + "user_type": "System User" if roles else "Website User", + } + ) + user.insert(ignore_permissions=True) + if roles: + cast(User, user).add_roles(*roles) + + +def make_web_form(**kwargs: Any) -> Document: + """An eSign-enabled web form on the test doctype, replacing any left by an earlier test.""" + if frappe.db.exists("Web Form", {"route": WEB_FORM_ROUTE}): + frappe.delete_doc( + "Web Form", + frappe.db.get_value("Web Form", {"route": WEB_FORM_ROUTE}), + force=True, + ignore_permissions=True, + ) + web_form = frappe.get_doc( + { + "doctype": "Web Form", + "title": "eSign Test Agreement", + "route": WEB_FORM_ROUTE, + "doc_type": TEST_DOCTYPE, + "module": "eSign", + "published": 1, + "esign_enabled": 1, + "web_form_fields": [ + {"fieldname": "signer_email", "fieldtype": "Data", "label": "Signer Email"}, + {"fieldname": "signature", "fieldtype": "Signature", "label": "Signature"}, + ], + **kwargs, + } + ) + web_form.insert(ignore_permissions=True) + return web_form + + +def make_agreement(**kwargs: Any) -> Agreement: + """A draft document of the test doctype.""" + doc = frappe.get_doc({"doctype": TEST_DOCTYPE, "title": "Service agreement", **kwargs}) + doc.insert(ignore_permissions=True) + return cast(Agreement, doc) + + +def get_agreement(name: str | None) -> Agreement: + """A fresh copy of a test document, without the flags the creating code left on it.""" + return cast(Agreement, frappe.get_doc(TEST_DOCTYPE, str(name))) + + +def share_key(doc: Document) -> str: + return doc.get_document_share_key() + + +@contextmanager +def signing_request( + path: str = f"/{WEB_FORM_ROUTE}", + referrer: str | None = None, + headers: dict[str, str] | None = None, + method: str = "POST", + **form_dict: Any, +) -> Generator[None]: + """Run the block inside a request from a signer's browser, as frappe would set it up.""" + all_headers = dict(headers or {}) + if referrer is not None: + all_headers["Referer"] = referrer + set_request(method=method, path=path, headers=all_headers, environ_base={"REMOTE_ADDR": "127.0.0.1"}) + frappe.local.request_ip = "127.0.0.1" + frappe.local.form_dict = frappe._dict(form_dict) + # The endpoints are rate limited per IP; the tests are one IP making many requests. + if cache := frappe.cache: + cache.delete_keys("rl:") + try: + yield + finally: + frappe.local.request = None + frappe.local.request_ip = None + frappe.local.form_dict = frappe._dict() + + +def form_data(doc: Document, **values: Any) -> str: + """The JSON the web form posts to accept.""" + return json.dumps({"doctype": doc.doctype, "name": doc.name, **values}) + + +def make_email_account(name: str = "eSign Test Outgoing", email: str = "esign-out@example.com") -> str: + """An email account that sends nothing, for the sender links.""" + if not frappe.db.exists("Email Account", name): + account = frappe.get_doc( + { + "doctype": "Email Account", + "email_account_name": name, + "email_id": email, + "enable_outgoing": 0, + "enable_incoming": 0, + } + ) + account.flags.ignore_validate = True + account.insert(ignore_permissions=True) + return name + + +def allow_esign(doc: frappe._dict) -> bool: + """A can_esign hook target that allows documents titled "Allowed".""" + return doc.get("title") == "Allowed" diff --git a/pyproject.toml b/pyproject.toml index 79f8001..515c709 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -65,7 +65,7 @@ show_missing = true skip_covered = true precision = 1 exclude_also = ["if TYPE_CHECKING:", "raise NotImplementedError", "@(abc\\.)?abstractmethod"] -fail_under = 0 +fail_under = 80 [tool.vulture] exclude = [".venv/", "node_modules/", ".frappe-nix/", ".dev-dist/"] From 8da82313d6f6b3f104cd144c364309b204582cb9 Mon Sep 17 00:00:00 2001 From: Kevin Shenk Date: Sat, 17 Oct 2026 00:11:46 -0400 Subject: [PATCH 09/14] refactor: type the bundles in strict TypeScript and build the fonts as a bundle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Compiled code (the desk, control and web bundles and the controls they import) is now TypeScript, checked by the managed tsconfig.browser.json with the frappe-types strict preset. The Web Form doctype script stays JavaScript with JSDoc, checked by tsconfig.desk.json against types/doctypes.d.ts (frappe-types gen-registry: Web Form and Email Template with esign's custom fields). - controls/base.ts types frappe.ui.form.ControlData as the ControlInput subclass it is at runtime; frappe-types declares it as the base Control. Fields the controls set while frappe's constructor runs are `declare`d, so the compiled classes never re-initialise them. - types/esign.augment.d.ts declares what esign adds to frappe (ControlUpload, ControlFontSelect, frappe.esign_context, frappe.is_completed) and to the page; types/frappe-modules.d.ts declares frappe's web form modules the web bundle imports, now by "frappe/public/js/…" (resolved from the bench's apps directory) instead of five levels of "../". - frappe keys a bundle in assets.json by its source file name, so hooks.py and the eSign page now name esign.desk.bundle.ts, esign.control.bundle.ts and esign.web.bundle.ts. - The signature fonts are a CSS entry (esign-fonts.entry.css) that Vite builds to dist/css/esign-fonts.bundle..css with a manifest, and scripts/vite-register.mjs registers it as esign-fonts.bundle.css. hooks.py and the eSign page use that name instead of the unhashed /assets/esign/dist/esign-fonts.css path, so a release busts the cache. - web_form.js calls its two helpers directly instead of through frm.events. - Small runtime changes, all deliberate: the upload control's button now reads "Change" after a file is chosen (the old code set textContent on a jQuery object, a no-op); the debug console lines on the eSign page are gone. - @types/hast is declared (the controls build hast trees). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SvdEeMiewirZ3ifFoUix5F --- .oxlintrc.json | 10 +- esign/esign/custom/web_form.py | 4 +- esign/hooks.py | 6 +- esign/public/css/esign-fonts.entry.css | 12 + esign/public/js/controls/base.ts | 55 +++++ .../{font_select.js => font_select.ts} | 164 ++++++++----- .../controls/{signature.js => signature.ts} | 191 +++++++++------ .../js/controls/{upload.js => upload.ts} | 71 ++++-- esign/public/js/controls/utils.js | 117 --------- esign/public/js/controls/utils.ts | 163 +++++++++++++ ...trol.bundle.js => esign.control.bundle.ts} | 0 ...gn.desk.bundle.js => esign.desk.bundle.ts} | 177 +++++++++----- esign/public/js/fonts.js | 17 -- ...sign.web.bundle.js => esign.web.bundle.ts} | 14 +- ...form_script.js => esign_webform_script.ts} | 125 +++++----- esign/public/js/web_form.js | 78 +++--- esign/tests/test_esign.py | 6 + esign/tests/test_web_form.py | 3 +- package.json | 1 + tsconfig.base.json | 9 + tsconfig.browser.json | 11 + tsconfig.json | 3 + types/doctypes.d.ts | 226 ++++++++++++++++++ types/esign.augment.d.ts | 50 ++++ types/frappe-modules.d.ts | 47 ++++ vite.config.mjs | 26 +- yarn.lock | 7 + 27 files changed, 1122 insertions(+), 471 deletions(-) create mode 100644 esign/public/css/esign-fonts.entry.css create mode 100644 esign/public/js/controls/base.ts rename esign/public/js/controls/{font_select.js => font_select.ts} (69%) rename esign/public/js/controls/{signature.js => signature.ts} (84%) rename esign/public/js/controls/{upload.js => upload.ts} (57%) delete mode 100644 esign/public/js/controls/utils.js create mode 100644 esign/public/js/controls/utils.ts rename esign/public/js/{esign.control.bundle.js => esign.control.bundle.ts} (100%) rename esign/public/js/{esign.desk.bundle.js => esign.desk.bundle.ts} (61%) delete mode 100644 esign/public/js/fonts.js rename esign/public/js/web/{esign.web.bundle.js => esign.web.bundle.ts} (55%) rename esign/public/js/web/{esign_webform_script.js => esign_webform_script.ts} (77%) create mode 100644 tsconfig.base.json create mode 100644 tsconfig.browser.json create mode 100644 types/doctypes.d.ts create mode 100644 types/esign.augment.d.ts create mode 100644 types/frappe-modules.d.ts diff --git a/.oxlintrc.json b/.oxlintrc.json index e5b3a95..44239a6 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -54,15 +54,7 @@ } }, { - "files": [ - "esign/public/js/controls/font_select.js", - "esign/public/js/controls/signature.js", - "esign/public/js/controls/upload.js", - "esign/public/js/controls/utils.js", - "esign/public/js/fonts.js", - "esign/public/js/web/esign_webform_script.js", - "esign/public/js/web_form.js" - ], + "files": ["esign/public/js/web_form.js"], "globals": { "frappe": "readonly", "__": "readonly", diff --git a/esign/esign/custom/web_form.py b/esign/esign/custom/web_form.py index 8f66664..215dfee 100644 --- a/esign/esign/custom/web_form.py +++ b/esign/esign/custom/web_form.py @@ -285,12 +285,12 @@ def get_context(self, context): [ "esign.control.bundle.css", "esign.web.bundle.css", - "/assets/esign/dist/esign-fonts.css", + "esign-fonts.bundle.css", ] ) context.web_include_js.extend( [ - "esign.web.bundle.js", + "esign.web.bundle.ts", ] ) context.template = "esign/templates/esign.html" diff --git a/esign/hooks.py b/esign/hooks.py index 1edc128..52d3d1c 100644 --- a/esign/hooks.py +++ b/esign/hooks.py @@ -8,10 +8,12 @@ app_email = "mail@avu.nu" app_license = "MIT" -app_include_js = ["esign.desk.bundle.js", "esign.control.bundle.js"] +# Bundles are looked up in assets.json by their source name, so the TypeScript entries are named +# with .ts; frappe still serves them as compiled .js files. +app_include_js = ["esign.desk.bundle.ts", "esign.control.bundle.ts"] app_include_css = [ "esign.control.bundle.css", - "/assets/esign/dist/esign-fonts.css", + "esign-fonts.bundle.css", ] # Install/migrate hooks for communication type registration diff --git a/esign/public/css/esign-fonts.entry.css b/esign/public/css/esign-fonts.entry.css new file mode 100644 index 0000000..4665192 --- /dev/null +++ b/esign/public/css/esign-fonts.entry.css @@ -0,0 +1,12 @@ +/* Copyright (c) 2026, Avunu LLC and contributors + For license information, please see license.txt */ + +/* + * The signature fonts, bundled by Vite (vite.config.mjs) into one stylesheet with its font + * files. scripts/vite-register.mjs registers it as esign-fonts.bundle.css, the name hooks.py + * and the eSign web form use. + */ +@import "@fontsource/pacifico/index.css"; +@import "@fontsource/dancing-script/index.css"; +@import "@fontsource/great-vibes/index.css"; +@import "@fontsource/caveat/index.css"; diff --git a/esign/public/js/controls/base.ts b/esign/public/js/controls/base.ts new file mode 100644 index 0000000..4f58444 --- /dev/null +++ b/esign/public/js/controls/base.ts @@ -0,0 +1,55 @@ +// Copyright (c) 2026, Avunu LLC and contributors +// For license information, please see license.txt + +/** + * @fileoverview The frappe control class the eSign controls extend, typed for them. + * + * frappe-types declares every `frappe.ui.form.Control*` class as the base `Control`. At + * runtime `ControlData` extends `ControlInput` (frappe/public/js/frappe/form/controls/data.js), + * and the eSign controls use its input area and two of its own methods, so this module + * declares those members once and hands out the same class under that type. + * + * @author Avunu LLC + */ + +import type { BaseControl, ControlInput, ControlOptions } from "frappe-types"; + +/** + * A `ControlData` instance as the eSign controls use it. Its focus target (`$input`, + * `input`) is whatever element the control renders (a button, a canvas, a hidden input), + * which frappe only ever treats as an element. + */ +export interface ControlDataInstance extends Omit { + $input?: JQuery; + input?: HTMLElement; + // The methods the eSign controls override, kept as methods (Omit turns them into properties). + make_input(): void; + set_input(value: unknown): void; + get_input_value(): unknown; + get_value(): unknown; + refresh(): void; + refresh_input(): void; + /** frappe/public/js/frappe/form/controls/data.js: copies `df` attributes onto `$input`. */ + set_input_attributes(): void; + /** frappe/public/js/frappe/form/controls/data.js: renders a value into the input. */ + set_formatted_input(value: unknown): unknown; +} + +type ControlMethod = + | "make_input" + | "set_input" + | "get_input_value" + | "get_value" + | "refresh" + | "refresh_input"; + +export interface ControlDataClass { + new (opts: ControlOptions): ControlDataInstance; + prototype: ControlDataInstance; +} + +/** `frappe.ui.form.ControlData`, typed as the class it is at runtime. */ +export const ControlData = frappe.ui.form.ControlData as ControlDataClass; + +/** Every eSign control is a frappe control: the type `frappe.ui.form` registers it under. */ +export type ControlClass = typeof BaseControl; diff --git a/esign/public/js/controls/font_select.js b/esign/public/js/controls/font_select.ts similarity index 69% rename from esign/public/js/controls/font_select.js rename to esign/public/js/controls/font_select.ts index 04d7fa9..17b6404 100644 --- a/esign/public/js/controls/font_select.js +++ b/esign/public/js/controls/font_select.ts @@ -1,7 +1,8 @@ // Copyright (c) 2026, Avunu LLC and contributors // For license information, please see license.txt -import { toDom } from "./utils"; +import { ControlData } from "./base"; +import { toDom, type HastElement } from "./utils"; /** * @fileoverview FontSelect control for Frappe Framework @@ -14,12 +15,27 @@ import { toDom } from "./utils"; * @author Avunu LLC */ +/** A font option as the control shows it. */ +export interface FontOption { + value: string; + label: string; + fontFamily: string; +} + +/** A font option as `df.options` gives it; `variants` are the family names to look for locally. */ +export interface FontOptionInput extends FontOption { + variants?: string[] | undefined; +} + +/** One entry of `df.options`: a family name, or an object with any of the option's keys. */ +type RawFontOption = string | Partial; + /** * Generates a unique anchor name for CSS anchor positioning - * @returns {string} Unique anchor name + * @returns Unique anchor name */ let anchorCounter = 0; -function generateAnchorName() { +function generateAnchorName(): string { return `--font-select-anchor-${++anchorCounter}`; } @@ -27,13 +43,13 @@ function generateAnchorName() { * Checks which fonts from a list are available locally using the Local Font Access API. * Falls back to returning all fonts if the API is unavailable or permission is denied. * - * @param {Array<{value: string, label: string, fontFamily: string, variants?: string[]}>} fonts + * @param fonts * Array of font options. Each option can include: * - value: The value to use when selected * - label: Display label for the option * - fontFamily: CSS font-family value * - variants: Optional array of font family name variants to check for availability - * @returns {Promise>} + * @returns * Filtered array of available fonts * * @example @@ -47,13 +63,14 @@ function generateAnchorName() { * ]; * const available = await filterAvailableFonts(fonts); */ -export async function filterAvailableFonts(fonts) { +export async function filterAvailableFonts(fonts: FontOptionInput[]): Promise { if (!fonts || fonts.length === 0) { return []; } // If Local Font Access API is not available, return all fonts - if (!("queryLocalFonts" in window)) { + const queryLocalFonts = window.queryLocalFonts; + if (!queryLocalFonts) { return fonts.map(({ value, label, fontFamily }) => ({ value, label, @@ -62,10 +79,10 @@ export async function filterAvailableFonts(fonts) { } try { - const localFonts = await window.queryLocalFonts(); + const localFonts = await queryLocalFonts.call(window); const localFontNames = new Set(localFonts.map((f) => f.family.toLowerCase())); - const available = []; + const available: FontOption[] = []; for (const font of fonts) { // Use variants if provided, otherwise use the label as the variant const variants = font.variants || [font.label]; @@ -98,12 +115,27 @@ export async function filterAvailableFonts(fonts) { } } -export class ControlFontSelect extends frappe.ui.form.ControlData { +export class ControlFontSelect extends ControlData { + // Assigned in make_input and _build_ui, which frappe's control constructor reaches: + // `declare` keeps the class from re-initialising them afterwards. + declare _options: FontOptionInput[]; + declare _available_options: FontOption[]; + declare _is_hidden: boolean; + declare _init_promise: Promise | undefined; + declare _anchor_name: string; + declare _last_options_json: string | undefined; + declare font_select_wrapper: HTMLElement; + declare trigger_button: HTMLButtonElement; + declare value_display: HTMLSpanElement; + declare popover_element: HTMLDivElement; + declare options_container: HTMLDivElement; + declare hidden_input: HTMLInputElement; + /** * Initializes the control. Called by Frappe's control lifecycle. * Triggers async font availability check. */ - make_input() { + override make_input(): void { if (this.has_input) return; // Mark as having input to prevent re-entry @@ -120,7 +152,7 @@ export class ControlFontSelect extends frappe.ui.form.ControlData { * Async initialization - parses options, checks font availability, builds UI * @private */ - async _init_async() { + async _init_async(): Promise { // Parse options from df.options this._options = this.parse_options(); @@ -134,7 +166,7 @@ export class ControlFontSelect extends frappe.ui.form.ControlData { // If exactly one font, set it as the value if (this._available_options.length === 1) { - this.value = this._available_options[0].value; + this.value = this._available_options[0]?.value; } return; } @@ -143,7 +175,7 @@ export class ControlFontSelect extends frappe.ui.form.ControlData { this._build_ui(); // Set initial value - const defaultValue = this.df.default || (this._available_options[0] && this._available_options[0].value); + const defaultValue = this.df.default || this._available_options[0]?.value; if (defaultValue) { this.set_input(defaultValue); } @@ -153,7 +185,7 @@ export class ControlFontSelect extends frappe.ui.form.ControlData { * Hides the control when there's no meaningful choice * @private */ - _hide_control() { + _hide_control(): void { // Hide the entire field wrapper if possible if (this.$wrapper) { this.$wrapper.hide(); @@ -166,7 +198,7 @@ export class ControlFontSelect extends frappe.ui.form.ControlData { * Shows the control * @private */ - _show_control() { + _show_control(): void { if (this.$wrapper) { this.$wrapper.show(); } else if (this.wrapper) { @@ -178,14 +210,14 @@ export class ControlFontSelect extends frappe.ui.form.ControlData { * Builds the font select UI * @private */ - _build_ui() { + _build_ui(): void { this._anchor_name = generateAnchorName(); // Generate unique IDs for popover association const popoverId = `font-select-popover-${anchorCounter}`; // Build options list for HAST - const optionItems = this._available_options.map((opt, index) => ({ + const optionItems = this._available_options.map((opt, index): HastElement => ({ type: "element", tagName: "button", properties: { @@ -199,7 +231,7 @@ export class ControlFontSelect extends frappe.ui.form.ControlData { })); // Define complete structure using HAST - const wrapperStructure = { + const wrapperStructure: HastElement = { type: "element", tagName: "div", properties: { @@ -256,28 +288,40 @@ export class ControlFontSelect extends frappe.ui.form.ControlData { this.font_select_wrapper = toDom(wrapperStructure); this.input_area.appendChild(this.font_select_wrapper); - // Get references - this.trigger_button = this.font_select_wrapper.children[0]; - this.value_display = this.trigger_button.children[0]; - this.popover_element = this.font_select_wrapper.children[1]; - this.options_container = this.popover_element.children[0]; + // Get references: [button.font-select-trigger > span.font-select-value, + // div.font-select-popover > div.font-select-options] + const [trigger_button, popover_element] = this.font_select_wrapper.children; + const value_display = trigger_button?.children[0]; + const options_container = popover_element?.children[0]; + if ( + !(trigger_button instanceof HTMLButtonElement) || + !(value_display instanceof HTMLSpanElement) || + !(popover_element instanceof HTMLDivElement) || + !(options_container instanceof HTMLDivElement) + ) { + throw new TypeError("ControlFontSelect: unexpected markup"); + } + this.trigger_button = trigger_button; + this.value_display = value_display; + this.popover_element = popover_element; + this.options_container = options_container; // Apply font-family styles to option buttons - const optionButtons = this.options_container.children; - for (let i = 0; i < optionButtons.length; i++) { - const btn = optionButtons[i]; - btn.style.fontFamily = btn.dataset.fontFamily; + for (const btn of this.option_buttons()) { + btn.style.fontFamily = btn.dataset.fontFamily ?? ""; } // Bind click handlers for options this.options_container.addEventListener("click", (e) => { - const optionButton = e.target.closest(".font-select-option"); + const optionButton = + e.target instanceof Element ? e.target.closest(".font-select-option") : null; if (optionButton) { const value = optionButton.dataset.value; this.set_input(value); this.popover_element.hidePopover(); - if (this.df.onchange) { - this.df.onchange(); + const onchange = this.df.onchange; + if (typeof onchange === "function") { + onchange.call(this.df); } } }); @@ -293,20 +337,23 @@ export class ControlFontSelect extends frappe.ui.form.ControlData { this.input = this.hidden_input; } + /** The option buttons, as rendered into the popover. */ + option_buttons(): HTMLElement[] { + return Array.from(this.options_container.children).filter( + (child): child is HTMLElement => child instanceof HTMLElement, + ); + } + /** * Parses options from df.options * Supports: * - Array of strings: ["Font A", "Font B"] * - Array of objects: [{ value: "font-a", label: "Font A", fontFamily: "...", variants: [...] }] * - Newline-separated string: "Font A\nFont B" - * @returns {Array<{value: string, label: string, fontFamily: string, variants?: string[]}>} */ - parse_options() { - let options = this.df.options || []; - - if (typeof options === "string") { - options = options.split("\n").filter((o) => o.trim()); - } + parse_options(): FontOptionInput[] { + const raw = (this.df.options as string | RawFontOption[] | undefined) || []; + const options: RawFontOption[] = typeof raw === "string" ? raw.split("\n").filter((o) => o.trim()) : raw; return options.map((opt) => { if (typeof opt === "string") { @@ -317,9 +364,9 @@ export class ControlFontSelect extends frappe.ui.form.ControlData { }; } return { - value: opt.value || opt.label, - label: opt.label || opt.value, - fontFamily: opt.fontFamily || opt.value, + value: opt.value || opt.label || "", + label: opt.label || opt.value || "", + fontFamily: opt.fontFamily || opt.value || "", variants: opt.variants, }; }); @@ -327,40 +374,35 @@ export class ControlFontSelect extends frappe.ui.form.ControlData { /** * Gets option by value from available options - * @param {string} value - * @returns {Object|undefined} */ - get_option(value) { + get_option(value: unknown): FontOption | undefined { return this._available_options.find((opt) => opt.value === value); } /** * Gets all available options - * @returns {Array<{value: string, label: string, fontFamily: string}>} */ - get_options() { + get_options(): FontOption[] { return this._available_options; } /** * Returns whether the control is hidden due to insufficient options - * @returns {boolean} */ - is_hidden() { + is_hidden(): boolean { return this._is_hidden; } /** * Waits for async initialization to complete - * @returns {Promise} */ - async ready() { + async ready(): Promise { if (this._init_promise) { await this._init_promise; } } - set_input(value) { + override set_input(value: unknown): void { this.last_value = this.value; this.value = value; @@ -371,14 +413,12 @@ export class ControlFontSelect extends frappe.ui.form.ControlData { } if (this.hidden_input) { - this.hidden_input.value = value || ""; + this.hidden_input.value = value ? String(value) : ""; } // Update selected state in options if (this.options_container) { - const optionButtons = this.options_container.children; - for (let i = 0; i < optionButtons.length; i++) { - const btn = optionButtons[i]; + for (const btn of this.option_buttons()) { if (btn.dataset.value === value) { btn.classList.add("selected"); } else { @@ -388,22 +428,22 @@ export class ControlFontSelect extends frappe.ui.form.ControlData { } } - get_input_value() { + override get_input_value(): unknown { return this.value; } - get_value() { + override get_value(): unknown { return this.value || this.get_model_value(); } - set_formatted_input(value) { + override set_formatted_input(value: unknown): void { this.set_input(value); } /** * Refreshes options if they have changed */ - async refresh() { + override async refresh(): Promise { super.refresh(); // Wait for initial async setup @@ -424,7 +464,7 @@ export class ControlFontSelect extends frappe.ui.form.ControlData { this._is_hidden = true; this._hide_control(); if (this._available_options.length === 1) { - this.value = this._available_options[0].value; + this.value = this._available_options[0]?.value; } } else { this._is_hidden = false; @@ -437,7 +477,7 @@ export class ControlFontSelect extends frappe.ui.form.ControlData { /** * Rebuilds the options list */ - rebuild_options() { + rebuild_options(): void { if (!this.options_container) return; // Clear existing options @@ -447,7 +487,7 @@ export class ControlFontSelect extends frappe.ui.form.ControlData { // Build new options this._available_options.forEach((opt, index) => { - const optionStructure = { + const optionStructure: HastElement = { type: "element", tagName: "button", properties: { diff --git a/esign/public/js/controls/signature.js b/esign/public/js/controls/signature.ts similarity index 84% rename from esign/public/js/controls/signature.js rename to esign/public/js/controls/signature.ts index 8ef3e81..de24ac3 100644 --- a/esign/public/js/controls/signature.js +++ b/esign/public/js/controls/signature.ts @@ -2,7 +2,8 @@ // For license information, please see license.txt import SignaturePad from "signature_pad"; -import { createIconHast, toDom } from "./utils"; +import { ControlData } from "./base"; +import { createIconHast, toDom, type HastElement } from "./utils"; /** * @fileoverview Signature control for Frappe Framework @@ -25,11 +26,19 @@ import { createIconHast, toDom } from "./utils"; * @author Avunu LLC */ +/** A typed-signature font: its value, its label and the CSS font-family to draw it with. */ +export interface SignatureFont { + value: string; + label: string; + fontFamily: string; +} + +type SignatureMode = "draw" | "type" | "upload"; + /** * Bundled signature fonts from @fontsource - always available - * @type {Array<{value: string, label: string, fontFamily: string}>} */ -const SIGNATURE_FONTS = [ +const SIGNATURE_FONTS: readonly [SignatureFont, ...SignatureFont[]] = [ { value: "Pacifico", label: "Pacifico", @@ -63,17 +72,44 @@ let signatureCounter = 0; * @description A signature field control with draw, type, and upload modes. * Uses modern web APIs (Popover, CSS Anchor Positioning) and HAST for DOM. */ -export class ControlSignature extends frappe.ui.form.ControlData { +export class ControlSignature extends ControlData { + // Assigned in make_input (by toDom refs for the elements), which frappe's control + // constructor calls: `declare` keeps the class from re-initialising them afterwards. + declare _id: number; + declare _current_mode: SignatureMode; + declare _font_options: SignatureFont[]; + declare _selected_font: string | null; + declare _font_option_buttons: HTMLElement[]; + declare _upload_data: string | null; + declare _theme_observer: MutationObserver | undefined; + declare signature_pad: SignaturePad | undefined; + declare mode_buttons: HTMLElement[]; + declare mode_sections: HTMLElement[]; + declare display_wrapper: HTMLDivElement; + declare display_canvas: HTMLCanvasElement; + declare display_overlay: HTMLDivElement; + declare dialog: HTMLDivElement; + declare pad_canvas: HTMLCanvasElement; + declare typed_input: HTMLInputElement; + declare font_value_display: HTMLSpanElement; + declare font_popover: HTMLDivElement; + declare typed_preview_text: HTMLDivElement; + declare upload_area: HTMLDivElement; + declare file_input: HTMLInputElement; + declare upload_prompt: HTMLDivElement; + declare upload_preview: HTMLDivElement; + declare upload_img: HTMLImageElement; + /** * Gets the term to use for "signature" from df.options. * Defaults to "Signature" if not specified. - * @returns {string} */ - get_signature_term() { - return this.df.options?.trim() || "Signature"; + get_signature_term(): string { + const options = this.df.options; + return (typeof options === "string" ? options.trim() : "") || "Signature"; } - make_input() { + override make_input(): void { if (this.has_input) return; this._id = ++signatureCounter; @@ -94,14 +130,14 @@ export class ControlSignature extends frappe.ui.form.ControlData { /** * Builds the signature display area with canvas and overlay */ - build_display_area() { + build_display_area(): void { const term = this.get_signature_term().toLowerCase(); const addText = __("Add your {0}", [term]); const replaceText = __("Replace {0}", [term]); const addIcon = createIconHast("es-line-add", "md"); const replaceIcon = createIconHast("es-line-edit", "md"); - const displayStructure = { + const displayStructure: HastElement = { type: "element", tagName: "div", properties: { className: ["signature-display"] }, @@ -183,7 +219,7 @@ export class ControlSignature extends frappe.ui.form.ControlData { /** * Builds the signature dialog as a popover element */ - build_dialog_popover() { + build_dialog_popover(): void { const term = this.get_signature_term(); const termLower = term.toLowerCase(); const popoverId = `signature-dialog-${this._id}`; @@ -193,7 +229,7 @@ export class ControlSignature extends frappe.ui.form.ControlData { this.mode_buttons = []; this.mode_sections = []; - const dialogStructure = { + const dialogStructure: HastElement = { type: "element", tagName: "div", properties: { @@ -203,13 +239,13 @@ export class ControlSignature extends frappe.ui.form.ControlData { }, ref: "dialog", data: { - onclick: (e) => { + onclick: (e: MouseEvent) => { // Close on backdrop click if (e.target === this.dialog) { this.hide_dialog(); } }, - onkeydown: (e) => { + onkeydown: (e: KeyboardEvent) => { if (e.key === "Escape") { this.hide_dialog(); } @@ -269,7 +305,7 @@ export class ControlSignature extends frappe.ui.form.ControlData { dataMode: "draw", }, data: { - constructor: (el) => this.mode_buttons.push(el), + constructor: (el: HTMLElement) => this.mode_buttons.push(el), onclick: () => this.select_mode("draw"), }, children: [{ type: "text", value: __("Draw") }], @@ -283,7 +319,7 @@ export class ControlSignature extends frappe.ui.form.ControlData { dataMode: "type", }, data: { - constructor: (el) => this.mode_buttons.push(el), + constructor: (el: HTMLElement) => this.mode_buttons.push(el), onclick: () => this.select_mode("type"), }, children: [{ type: "text", value: __("Type") }], @@ -297,7 +333,7 @@ export class ControlSignature extends frappe.ui.form.ControlData { dataMode: "upload", }, data: { - constructor: (el) => this.mode_buttons.push(el), + constructor: (el: HTMLElement) => this.mode_buttons.push(el), onclick: () => this.select_mode("upload"), }, children: [{ type: "text", value: __("Upload") }], @@ -321,7 +357,7 @@ export class ControlSignature extends frappe.ui.form.ControlData { dataSection: "draw", }, data: { - constructor: (el) => this.mode_sections.push(el), + constructor: (el: HTMLElement) => this.mode_sections.push(el), }, children: [ { @@ -387,7 +423,7 @@ export class ControlSignature extends frappe.ui.form.ControlData { dataSection: "type", }, data: { - constructor: (el) => this.mode_sections.push(el), + constructor: (el: HTMLElement) => this.mode_sections.push(el), }, children: [ { @@ -511,7 +547,7 @@ export class ControlSignature extends frappe.ui.form.ControlData { dataSection: "upload", }, data: { - constructor: (el) => this.mode_sections.push(el), + constructor: (el: HTMLElement) => this.mode_sections.push(el), }, children: [ { @@ -522,22 +558,22 @@ export class ControlSignature extends frappe.ui.form.ControlData { }, ref: "upload_area", data: { - onclick: (e) => { + onclick: (e: MouseEvent) => { if (e.target !== this.file_input) { this.file_input.click(); } }, - ondragover: (e) => { + ondragover: (e: DragEvent) => { e.preventDefault(); this.upload_area.classList.add("drag-over"); }, ondragleave: () => { this.upload_area.classList.remove("drag-over"); }, - ondrop: (e) => { + ondrop: (e: DragEvent) => { e.preventDefault(); this.upload_area.classList.remove("drag-over"); - const file = e.dataTransfer.files[0]; + const file = e.dataTransfer?.files[0]; if (file && file.type.startsWith("image/")) { this.process_uploaded_file(file); } @@ -549,12 +585,12 @@ export class ControlSignature extends frappe.ui.form.ControlData { tagName: "input", properties: { type: "file", - accept: "image/*", + accept: ["image/*"], className: ["signature-file-input"], }, ref: "file_input", data: { - onchange: (e) => this.handle_file_select(e), + onchange: (e: Event) => this.handle_file_select(e), }, children: [], }, @@ -650,13 +686,14 @@ export class ControlSignature extends frappe.ui.form.ControlData { /** * Shows the signature dialog */ - show_dialog() { + show_dialog(): void { // Initialize font options if not already done (bundled fonts are always available) if (this._font_options.length === 0) { - this._font_options = SIGNATURE_FONTS; + this._font_options = [...SIGNATURE_FONTS]; this.build_font_options(); - if (this._font_options.length > 0) { - this.select_font(this._font_options[0].value); + const first = this._font_options[0]; + if (first) { + this.select_font(first.value); } } @@ -679,15 +716,15 @@ export class ControlSignature extends frappe.ui.form.ControlData { /** * Hides the signature dialog */ - hide_dialog() { + hide_dialog(): void { this.dialog.hidePopover(); } /** * Selects a signature mode (draw, type, upload) - * @param {string} mode - The mode to select + * @param mode - The mode to select */ - select_mode(mode) { + select_mode(mode: SignatureMode): void { this._current_mode = mode; // Update button states @@ -709,7 +746,7 @@ export class ControlSignature extends frappe.ui.form.ControlData { /** * Initializes the signature pad on the draw canvas */ - init_signature_pad() { + init_signature_pad(): void { if (!this.pad_canvas) return; // Resize canvas to match CSS size @@ -728,7 +765,7 @@ export class ControlSignature extends frappe.ui.form.ControlData { /** * Resizes the pad canvas to match CSS dimensions and device pixel ratio */ - resize_pad_canvas() { + resize_pad_canvas(): void { const ratio = Math.max(window.devicePixelRatio || 1, 1); const rect = this.pad_canvas.getBoundingClientRect(); @@ -737,7 +774,7 @@ export class ControlSignature extends frappe.ui.form.ControlData { if (this.pad_canvas.width !== rect.width * ratio || this.pad_canvas.height !== rect.height * ratio) { this.pad_canvas.width = rect.width * ratio; this.pad_canvas.height = rect.height * ratio; - this.pad_canvas.getContext("2d").scale(ratio, ratio); + this.pad_canvas.getContext("2d")?.scale(ratio, ratio); if (this.signature_pad) { this.signature_pad.clear(); @@ -748,15 +785,15 @@ export class ControlSignature extends frappe.ui.form.ControlData { /** * Builds font option buttons in the font popover */ - build_font_options() { + build_font_options(): void { // Track font option buttons for select state updates this._font_option_buttons = []; - const optionsStructure = { + const optionsStructure: HastElement = { type: "element", tagName: "div", properties: { className: ["signature-font-options"] }, - children: this._font_options.map((opt) => ({ + children: this._font_options.map((opt): HastElement => ({ type: "element", tagName: "button", properties: { @@ -765,7 +802,7 @@ export class ControlSignature extends frappe.ui.form.ControlData { dataValue: opt.value, }, data: { - constructor: (el) => { + constructor: (el: HTMLElement) => { this._font_option_buttons.push(el); el.style.fontFamily = opt.fontFamily; }, @@ -784,9 +821,9 @@ export class ControlSignature extends frappe.ui.form.ControlData { /** * Selects a font for typed signatures - * @param {string} value - The font value to select + * @param value - The font value to select */ - select_font(value) { + select_font(value: string): void { this._selected_font = value; const option = this._font_options.find((o) => o.value === value); @@ -808,7 +845,7 @@ export class ControlSignature extends frappe.ui.form.ControlData { /** * Updates the typed signature preview */ - update_typed_preview() { + update_typed_preview(): void { const text = this.typed_input.value || ""; const option = this._font_options.find((o) => o.value === this._selected_font); const fontFamily = option?.fontFamily || SIGNATURE_FONTS[0].fontFamily; @@ -819,10 +856,10 @@ export class ControlSignature extends frappe.ui.form.ControlData { /** * Handles file selection from input - * @param {Event} e - The change event + * @param e - The change event */ - handle_file_select(e) { - const file = e.target.files[0]; + handle_file_select(e: Event): void { + const file = (e.target as HTMLInputElement).files?.[0]; if (file) { this.process_uploaded_file(file); } @@ -830,13 +867,14 @@ export class ControlSignature extends frappe.ui.form.ControlData { /** * Processes an uploaded file and shows preview - * @param {File} file - The uploaded file + * @param file - The uploaded file */ - process_uploaded_file(file) { + process_uploaded_file(file: File): void { const reader = new FileReader(); - reader.addEventListener("load", (e) => { - this._upload_data = e.target.result; - this.upload_img.src = this._upload_data; + reader.addEventListener("load", () => { + const data = reader.result as string; + this._upload_data = data; + this.upload_img.src = data; this.upload_preview.classList.add("active"); this.upload_prompt.classList.add("hidden"); }); @@ -846,7 +884,7 @@ export class ControlSignature extends frappe.ui.form.ControlData { /** * Handles the insert button click */ - handle_insert() { + handle_insert(): void { const term = this.get_signature_term().toLowerCase(); if (this._current_mode === "draw") { @@ -878,7 +916,7 @@ export class ControlSignature extends frappe.ui.form.ControlData { }); return; } - this.normalize_upload_to_canvas(this._upload_data).then((data) => { + void this.normalize_upload_to_canvas(this._upload_data).then((data) => { this.set_signature_value(data); }); return; // Don't hide yet, wait for promise @@ -889,10 +927,10 @@ export class ControlSignature extends frappe.ui.form.ControlData { /** * Converts typed text to base64 PNG - * @param {string} text - The text to convert - * @returns {string} Base64 data URL + * @param text - The text to convert + * @returns Base64 data URL */ - convert_typed_to_base64(text) { + convert_typed_to_base64(text: string): string { const option = this._font_options.find((o) => o.value === this._selected_font); const fontFamily = option?.fontFamily || SIGNATURE_FONTS[0].fontFamily; @@ -900,6 +938,9 @@ export class ControlSignature extends frappe.ui.form.ControlData { canvas.width = 750; canvas.height = 292; const ctx = canvas.getContext("2d"); + if (!ctx) { + return canvas.toDataURL("image/png"); + } ctx.fillStyle = "transparent"; ctx.fillRect(0, 0, canvas.width, canvas.height); @@ -933,15 +974,19 @@ export class ControlSignature extends frappe.ui.form.ControlData { /** * Normalizes an uploaded image to a consistent canvas size - * @param {string} dataUrl - The uploaded image data URL - * @returns {Promise} Normalized base64 data URL + * @param dataUrl - The uploaded image data URL + * @returns Normalized base64 data URL */ - normalize_upload_to_canvas(dataUrl) { + normalize_upload_to_canvas(dataUrl: string): Promise { return new Promise((resolve, reject) => { const canvas = document.createElement("canvas"); canvas.width = 750; canvas.height = 292; const ctx = canvas.getContext("2d"); + if (!ctx) { + reject(new Error("Canvas 2D is not available")); + return; + } ctx.fillStyle = "transparent"; ctx.fillRect(0, 0, canvas.width, canvas.height); @@ -978,11 +1023,11 @@ export class ControlSignature extends frappe.ui.form.ControlData { /** * Sets the signature value and updates the display - * @param {string} data - Base64 signature data + * @param data - Base64 signature data */ - set_signature_value(data) { + set_signature_value(data: string): void { this.value = data; - this.set_value(data); + void this.set_value(data); this.render_display(); this.hide_dialog(); @@ -997,7 +1042,7 @@ export class ControlSignature extends frappe.ui.form.ControlData { * Canvas 2D can't parse `var(--x)`, so we read the resolved color off a * hidden probe element — reading `color` guarantees a concrete rgb() value. */ - resolve_css_color(var_name, fallback = "#d1d8dd") { + resolve_css_color(var_name: string, fallback = "#d1d8dd"): string { const probe = document.createElement("span"); probe.style.color = `var(${var_name}, ${fallback})`; probe.style.display = "none"; @@ -1011,7 +1056,7 @@ export class ControlSignature extends frappe.ui.form.ControlData { * Re-renders the empty-state placeholder when the desk theme changes so its * border tracks the active theme. Set up once per control instance. */ - setup_theme_observer() { + setup_theme_observer(): void { if (this._theme_observer) return; this._theme_observer = new MutationObserver(() => { if (!this.get_value()) this.render_display(); @@ -1025,15 +1070,16 @@ export class ControlSignature extends frappe.ui.form.ControlData { /** * Renders the signature on the display canvas */ - render_display() { + render_display(): void { if (!this.display_canvas) return; this.setup_theme_observer(); const ctx = this.display_canvas.getContext("2d"); + if (!ctx) return; ctx.clearRect(0, 0, this.display_canvas.width, this.display_canvas.height); const value = this.get_value(); - if (value) { + if (typeof value === "string" && value) { const img = new Image(); img.addEventListener("load", () => { ctx.drawImage(img, 0, 0, this.display_canvas.width, this.display_canvas.height); @@ -1049,8 +1095,9 @@ export class ControlSignature extends frappe.ui.form.ControlData { /** * Renders the empty state on the display canvas */ - render_empty_display() { + render_empty_display(): void { const ctx = this.display_canvas.getContext("2d"); + if (!ctx) return; ctx.clearRect(0, 0, this.display_canvas.width, this.display_canvas.height); ctx.setLineDash([5, 5]); @@ -1065,16 +1112,16 @@ export class ControlSignature extends frappe.ui.form.ControlData { // Base class interface methods // ───────────────────────────────────────────────────────────────── - set_input(value) { + override set_input(value: unknown): void { this.value = value; this.render_display(); } - get_input_value() { + override get_input_value(): unknown { return this.value; } - get_value() { + override get_value(): unknown { const value = this.value || this.get_model_value(); // Ignore placeholder image if (value === "/assets/frappe/images/signature-placeholder.png") { @@ -1083,11 +1130,11 @@ export class ControlSignature extends frappe.ui.form.ControlData { return value; } - set_formatted_input(value) { + override set_formatted_input(value: unknown): void { this.set_input(value); } - refresh_input() { + override refresh_input(): void { // Call base class to handle label, description, mandatory styling, etc. super.refresh_input(); diff --git a/esign/public/js/controls/upload.js b/esign/public/js/controls/upload.ts similarity index 57% rename from esign/public/js/controls/upload.js rename to esign/public/js/controls/upload.ts index 8ffed92..5e7dbf4 100644 --- a/esign/public/js/controls/upload.js +++ b/esign/public/js/controls/upload.ts @@ -1,8 +1,6 @@ // Copyright (c) 2026, Avunu LLC and contributors // For license information, please see license.txt -import { toDom } from "hast-util-to-dom"; - /** * @fileoverview Upload control for Frappe Framework * @description This module provides a file upload control with preview functionality. @@ -13,13 +11,28 @@ import { toDom } from "hast-util-to-dom"; * @author Avunu LLC */ -export class ControlUpload extends frappe.ui.form.ControlData { - make_input() { +import { ControlData } from "./base"; +import { toDom, type HastElement } from "./utils"; + +/** `df.options` of an Upload field, when it restricts the file types. */ +interface UploadOptions { + allowed_file_types?: string[]; +} + +export class ControlUpload extends ControlData { + declare file_input: HTMLInputElement; + declare preview_element: HTMLElement; + declare preview_img: HTMLImageElement; + + override make_input(): void { if (this.$input) return; + const options = this.df.options as UploadOptions | string | undefined; + const allowed = typeof options === "object" ? options.allowed_file_types : undefined; + // Define complete structure using hast (HTML Abstract Syntax Tree) // Structure: div.upload-wrapper > [button.btn-upload, input[type=file], div.upload-preview > img] - const uploadStructure = { + const uploadStructure: HastElement = { type: "element", tagName: "div", properties: { className: ["upload-wrapper"] }, @@ -39,7 +52,7 @@ export class ControlUpload extends frappe.ui.form.ControlData { properties: { type: "file", className: ["hidden"], - accept: this.df.options?.allowed_file_types?.join(",") || "image/*", + accept: allowed?.length ? allowed : ["image/*"], }, children: [], }, @@ -63,15 +76,21 @@ export class ControlUpload extends frappe.ui.form.ControlData { const wrapper = toDom(uploadStructure); this.input_area.appendChild(wrapper); - // Get references via property accessors - // wrapper.children[0] = button.btn-upload - // wrapper.children[1] = input[type=file] - // wrapper.children[2] = div.upload-preview - // wrapper.children[2].children[0] = img.upload-preview-img - const button = wrapper.children[0]; - this.file_input = wrapper.children[1]; - this.preview_element = wrapper.children[2]; - this.preview_img = this.preview_element.children[0]; + // Get references by position: + // [button.btn-upload, input[type=file], div.upload-preview > img.upload-preview-img] + const [button, file_input, preview] = wrapper.children; + const preview_img = preview?.children[0]; + if ( + !(button instanceof HTMLButtonElement) || + !(file_input instanceof HTMLInputElement) || + !(preview instanceof HTMLElement) || + !(preview_img instanceof HTMLImageElement) + ) { + throw new TypeError("ControlUpload: unexpected markup"); + } + this.file_input = file_input; + this.preview_element = preview; + this.preview_img = preview_img; // Bind event listeners this.file_input.addEventListener("change", (e) => { @@ -90,40 +109,40 @@ export class ControlUpload extends frappe.ui.form.ControlData { this.set_input_attributes(); } - handle_file_selection(e) { - const file = e.target.files[0]; + handle_file_selection(e: Event): void { + const file = (e.target as HTMLInputElement).files?.[0]; if (!file) return; // Convert to base64 const reader = new FileReader(); - reader.addEventListener("load", (event) => { - const dataurl = event.target.result; - this.set_value(dataurl); + reader.addEventListener("load", () => { + const dataurl = reader.result as string; + void this.set_value(dataurl); this.value = dataurl; this.set_preview(dataurl); }); reader.readAsDataURL(file); } - set_preview(dataurl) { - if (dataurl) { + set_preview(dataurl: unknown): void { + if (typeof dataurl === "string" && dataurl) { this.preview_img.src = dataurl; this.preview_element.classList.add("active"); - this.$input.textContent = __("Change"); + this.$input?.text(__("Change")); } else { this.preview_element.classList.remove("active"); - this.$input.textContent = __("Upload"); + this.$input?.text(__("Upload")); } } - set_input(value) { + override set_input(value: unknown): void { this.last_value = this.value; this.value = value; this.set_formatted_input(value); this.set_preview(value); } - get_value() { + override get_value(): unknown { return this.value || null; } } diff --git a/esign/public/js/controls/utils.js b/esign/public/js/controls/utils.js deleted file mode 100644 index 2b2339c..0000000 --- a/esign/public/js/controls/utils.js +++ /dev/null @@ -1,117 +0,0 @@ -// Copyright (c) 2026, Avunu LLC and contributors -// For license information, please see license.txt - -import { toDom as hastToDom } from "hast-util-to-dom"; - -/** - * @fileoverview Shared utilities for eSign controls - * @description Provides HAST-based utilities for DOM structure creation. - * - * @requires hast-util-to-dom - For efficient DOM structure creation - * - * @author Avunu LLC - */ - -/** - * Converts HAST to DOM with support for element refs and event bindings. - * - * Supports ref pattern: - * - String ref (preferred): `ref: "myElement"` → assigns to `context.myElement` - * - * Supports constructor callback: - * - Callback constructor: `data: { constructor: (el) => ... }` → calls function with element - * - * Event listeners are assigned via `data` property: - * - `data: { onclick: (e) => ... }` - * - * @param {Object} hast - HAST structure - * @param {Object} [context] - Object to assign string refs to (typically `this`) - * @returns {Element} DOM element - */ -export function toDom(hast, context) { - return hastToDom(hast, { - afterTransform(node, element) { - // Handle string ref: assign element to context property - if (node.ref && typeof node.ref === "string" && context) { - context[node.ref] = element; - } - // Handle data properties (callback refs, events, etc.) - if (node.data && element instanceof Element) { - for (const [key, value] of Object.entries(node.data)) { - if (key === "constructor" && typeof value === "function") { - value(element); // Call ref callback with element - } else { - element[key] = value; // Assign event handlers - } - } - } - }, - }); -} - -/** - * Creates a HAST node for an SVG icon compatible with Frappe's icon system. - * Supports both standard icons (icon-*) and Espresso icons (es-*). - * - * @param {string} icon_name - The name of the icon (e.g., "edit", "add", "es-line-reload") - * @param {string|{width: string, height: string}} [size="sm"] - Size preset ("xs", "sm", "md", "lg") or custom dimensions - * @param {string} [icon_class=""] - Additional class for the element - * @param {string} [icon_style=""] - Additional inline styles for the element - * @param {string} [svg_class=""] - Additional class for the element - * @returns {Object} HAST element node representing the SVG icon - * - * @example - * // Standard icon - * createIconHast("edit", "md") - * - * @example - * // Espresso icon - * createIconHast("es-line-reload", "sm") - * - * @example - * // Custom size - * createIconHast("add", { width: "24px", height: "24px" }) - */ -export function createIconHast(icon_name, size = "sm", icon_class = "", icon_style = "", svg_class = "") { - let size_class = ""; - const is_espresso = icon_name.startsWith("es-"); - - const href = is_espresso ? `#${icon_name}` : `#icon-${icon_name}`; - - if (typeof size === "object") { - icon_style += ` width: ${size.width}; height: ${size.height}`; - } else { - size_class = `icon-${size}`; - } - - // Determine SVG class based on icon type - let base_svg_class; - if (is_espresso) { - base_svg_class = icon_name.startsWith("es-solid") ? "es-icon es-solid" : "es-icon es-line"; - } else { - base_svg_class = "icon"; - } - - const classes = [base_svg_class, svg_class, size_class].filter(Boolean); - - return { - type: "element", - tagName: "svg", - properties: { - className: classes, - style: icon_style || undefined, - ariaHidden: "true", - }, - children: [ - { - type: "element", - tagName: "use", - properties: { - className: icon_class ? [icon_class] : undefined, - href: href, - }, - children: [], - }, - ], - }; -} diff --git a/esign/public/js/controls/utils.ts b/esign/public/js/controls/utils.ts new file mode 100644 index 0000000..4e7108a --- /dev/null +++ b/esign/public/js/controls/utils.ts @@ -0,0 +1,163 @@ +// Copyright (c) 2026, Avunu LLC and contributors +// For license information, please see license.txt + +/** + * @fileoverview Shared utilities for eSign controls + * @description Provides HAST-based utilities for DOM structure creation. + * + * @requires hast-util-to-dom - For efficient DOM structure creation + * + * @author Avunu LLC + */ + +import type { Properties } from "hast"; +import { toDom as hastToDom } from "hast-util-to-dom"; + +/** + * Element callbacks: `constructor` is called with the created element, every other key is + * assigned to the element as a property (`onclick`, `oninput`, …). + */ +export interface HastData { + // Each handler takes its own event type, so the parameter is typed at the use site. + [key: string]: ((arg: never) => unknown) | undefined; +} + +/** A hast element with the two additions `toDom` understands: `ref` and `data`. */ +export interface HastElement { + type: "element"; + tagName: string; + properties: Properties; + children: HastChild[]; + /** `toDom` assigns the created element to `context[ref]`. */ + ref?: string; + data?: HastData; +} + +export interface HastText { + type: "text"; + value: string; +} + +export type HastChild = HastElement | HastText; + +/** + * Converts HAST to DOM with support for element refs and event bindings. + * + * Supports ref pattern: + * - String ref (preferred): `ref: "myElement"` → assigns to `context.myElement` + * + * Supports constructor callback: + * - Callback constructor: `data: { constructor: (el) => ... }` → calls function with element + * + * Event listeners are assigned via `data` property: + * - `data: { onclick: (e) => ... }` + * + * @param hast - HAST structure + * @param context - Object to assign string refs to (typically `this`) + * @returns The root element + */ +export function toDom(hast: HastElement, context?: object): HTMLElement { + const root = hastToDom(hast, { + afterTransform(node, element) { + if (node.type !== "element" || !(element instanceof Element)) { + return; + } + const { ref, data } = node as Partial; + // Handle string ref: assign element to context property + if (ref && context) { + Reflect.set(context, ref, element); + } + // Handle data properties (callback refs, events, etc.) + for (const [key, value] of Object.entries(data ?? {})) { + if (key === "constructor" && typeof value === "function") { + (value as (el: Element) => void)(element); // Call ref callback with element + } else { + Reflect.set(element, key, value); // Assign event handlers + } + } + }, + }); + if (!(root instanceof HTMLElement)) { + throw new TypeError(`toDom: <${hast.tagName}> did not produce an HTML element`); + } + return root; +} + +/** A custom icon size, as CSS lengths. */ +export interface IconSize { + width: string; + height: string; +} + +/** + * Creates a HAST node for an SVG icon compatible with Frappe's icon system. + * Supports both standard icons (icon-*) and Espresso icons (es-*). + * + * @param icon_name - The name of the icon (e.g., "edit", "add", "es-line-reload") + * @param size - Size preset ("xs", "sm", "md", "lg") or custom dimensions + * @param icon_class - Additional class for the element + * @param icon_style - Additional inline styles for the element + * @param svg_class - Additional class for the element + * @returns HAST element node representing the SVG icon + * + * @example + * // Standard icon + * createIconHast("edit", "md") + * + * @example + * // Espresso icon + * createIconHast("es-line-reload", "sm") + * + * @example + * // Custom size + * createIconHast("add", { width: "24px", height: "24px" }) + */ +export function createIconHast( + icon_name: string, + size: string | IconSize = "sm", + icon_class = "", + icon_style = "", + svg_class = "", +): HastElement { + let size_class = ""; + const is_espresso = icon_name.startsWith("es-"); + + const href = is_espresso ? `#${icon_name}` : `#icon-${icon_name}`; + + if (typeof size === "object") { + icon_style += ` width: ${size.width}; height: ${size.height}`; + } else { + size_class = `icon-${size}`; + } + + // Determine SVG class based on icon type + let base_svg_class; + if (is_espresso) { + base_svg_class = icon_name.startsWith("es-solid") ? "es-icon es-solid" : "es-icon es-line"; + } else { + base_svg_class = "icon"; + } + + const classes = [base_svg_class, svg_class, size_class].filter(Boolean); + + return { + type: "element", + tagName: "svg", + properties: { + className: classes, + style: icon_style || undefined, + ariaHidden: "true", + }, + children: [ + { + type: "element", + tagName: "use", + properties: { + className: icon_class ? [icon_class] : undefined, + href: href, + }, + children: [], + }, + ], + }; +} diff --git a/esign/public/js/esign.control.bundle.js b/esign/public/js/esign.control.bundle.ts similarity index 100% rename from esign/public/js/esign.control.bundle.js rename to esign/public/js/esign.control.bundle.ts diff --git a/esign/public/js/esign.desk.bundle.js b/esign/public/js/esign.desk.bundle.ts similarity index 61% rename from esign/public/js/esign.desk.bundle.js rename to esign/public/js/esign.desk.bundle.ts index ed7f15a..28c6349 100644 --- a/esign/public/js/esign.desk.bundle.js +++ b/esign/public/js/esign.desk.bundle.ts @@ -1,15 +1,56 @@ // Copyright (c) 2026, Avunu LLC and contributors // For license information, please see license.txt +import type { Dialog, Form, FrappeResponse } from "frappe-types"; + +// oxlint-disable-next-line import/no-unassigned-import -- registers the timeline card's microtemplate import "./templates/esign_timeline.html"; -$(document).on("form-refresh", function (event, frm) { +/** A row of esign.esign.custom.web_form.get_esign_web_forms. */ +interface EsignWebFormRow { + name: string; + title: string; + route: string; + print_format: string | null; +} + +/** A MultiSelect suggestion. */ +interface ContactOption { + value: string; + label: string; + description: string; +} + +/** What esign.esign.send_esign_email returns. */ +interface SendEsignEmailResult { + success: boolean; + message: string; + communication: string; + esign_link: string; +} + +/** + * Optional form events another app can define on a doctype to steer the dialog: the sending + * Email Account and the suggested recipients. + */ +interface EsignFormEvents { + get_email_account?: ( + frm: Form, + fieldname: string, + ) => Promise | string | null | undefined; + get_email_recipients?: ( + frm: Form, + fieldname: string, + ) => Promise | string[] | null | undefined; +} + +$(document).on("form-refresh", function (_event: JQuery.TriggeredEvent, frm: Form) { frappe - .call("esign.esign.can_esign", { + .call("esign.esign.can_esign", { doctype: frm.doctype, doc: frm.doc, }) - .then((r) => { + .then((r: FrappeResponse) => { if (r.message) { const esign_dialog = new ESignDialog(frm); frm.add_custom_button(__("Send for eSign"), () => { @@ -24,16 +65,25 @@ $(document).on("form-refresh", function (event, frm) { }); class ESignDialog { - constructor(frm) { + frm: Form; + web_forms: EsignWebFormRow[]; + dialog: Dialog; + contact_list_cache: ContactOption[]; + + constructor(frm: Form) { this.frm = frm; this.web_forms = []; - this.dialog = null; this.contact_list_cache = []; - this.init_dialog(); + this.dialog = this.init_dialog(); } - init_dialog() { - this.dialog = new frappe.ui.Dialog({ + /** The form's optional eSign events (see EsignFormEvents). */ + get esign_events(): EsignFormEvents { + return this.frm.events as EsignFormEvents; + } + + init_dialog(): Dialog { + return new frappe.ui.Dialog({ title: __("Send for eSign"), fields: [ { @@ -43,7 +93,7 @@ class ESignDialog { fieldname: "email_account", only_select: 1, get_query: () => { - const filters = { enable_outgoing: 1 }; + const filters: Record = { enable_outgoing: 1 }; if (this.frm.doc.company) { filters.company = this.frm.doc.company; } @@ -55,7 +105,7 @@ class ESignDialog { fieldtype: "MultiSelect", reqd: 1, fieldname: "recipients", - get_data: (txt) => { + get_data: (txt?: string) => { return this.get_cached_contact_list(txt); }, }, @@ -69,7 +119,7 @@ class ESignDialog { label: __("CC"), fieldtype: "MultiSelect", fieldname: "cc", - get_data: (txt) => { + get_data: (txt?: string) => { return this.get_cached_contact_list(txt); }, }, @@ -77,7 +127,7 @@ class ESignDialog { label: __("BCC"), fieldtype: "MultiSelect", fieldname: "bcc", - get_data: (txt) => { + get_data: (txt?: string) => { return this.get_cached_contact_list(txt); }, }, @@ -97,7 +147,7 @@ class ESignDialog { onchange: () => { const template = this.dialog.get_value("email_template"); if (template) { - this.load_email_template(template); + this.load_email_template(String(template)); } }, }, @@ -113,6 +163,7 @@ class ESignDialog { }, { fieldtype: "Section Break", + fieldname: "message_section", }, { label: __("Subject"), @@ -164,15 +215,15 @@ class ESignDialog { }); } - show() { - this.load_web_forms(); + show(): void { + void this.load_web_forms(); } - async load_web_forms() { + async load_web_forms(): Promise { // Load contact list cache first await this.load_contact_list_cache(); - frappe.call({ + frappe.call({ method: "esign.esign.custom.web_form.get_esign_web_forms", args: { doctype: this.frm.doctype }, callback: (r) => { @@ -182,10 +233,10 @@ class ESignDialog { label: wf.title, value: wf.name, })); - this.dialog.set_df_property("web_form", "options", [].concat(options)); - this.dialog.set_value("web_form", options[0]?.value); + this.dialog.set_df_property("web_form", "options", [...options]); + void this.dialog.set_value("web_form", options[0]?.value); this.dialog.show(); - this.prefill_email_account(); + void this.prefill_email_account(); } else { frappe.msgprint(__("No eSign-enabled web forms found for {0}", [this.frm.doctype])); } @@ -193,24 +244,25 @@ class ESignDialog { }); } - async prefill_email_account() { + async prefill_email_account(): Promise { // Try custom hook first - if (this.frm?.events.get_email_account) { + const events = this.esign_events; + if (events.get_email_account) { try { - const custom_email_account = await this.frm.events.get_email_account(this.frm, "recipients"); + const custom_email_account = await events.get_email_account(this.frm, "recipients"); if (custom_email_account) { // Ensure proper format for MultiSelect - this.dialog.set_value("email_account", custom_email_account); + void this.dialog.set_value("email_account", custom_email_account); } } catch (error) { console.error("Error fetching custom email accounts:", error); } } else { - const filters = { enable_outgoing: 1 }; + const filters: Record = { enable_outgoing: 1 }; if (this.frm.doc.company) { filters.company = this.frm.doc.company; } - frappe.call({ + frappe.call>({ method: "frappe.client.get_list", args: { doctype: "Email Account", @@ -226,31 +278,33 @@ class ESignDialog { const selected = default_account || (accounts.length === 1 ? accounts[0] : null); if (selected) { - this.dialog.set_value("email_account", selected.name); + void this.dialog.set_value("email_account", selected.name); } }, }); } } - on_web_form_change() { + on_web_form_change(): void { const web_form_name = this.dialog.get_value("web_form"); const web_form = this.web_forms.find((wf) => wf.name === web_form_name); if (web_form && web_form.print_format) { - this.dialog.set_value("print_format", web_form.print_format); + void this.dialog.set_value("print_format", web_form.print_format); } this.update_esign_link_preview(); } - async load_contact_list_cache() { - let contacts = []; + async load_contact_list_cache(): Promise { + let contacts: ContactOption[] = []; + const doc = this.frm.doc; // Try custom hook first - if (this.frm?.events.get_email_recipients) { + const events = this.esign_events; + if (events.get_email_recipients) { try { - const custom_contacts = await this.frm.events.get_email_recipients(this.frm, "recipients"); + const custom_contacts = await events.get_email_recipients(this.frm, "recipients"); if (custom_contacts && custom_contacts.length > 0) { // Ensure proper format for MultiSelect contacts = custom_contacts.map((email) => ({ @@ -264,29 +318,34 @@ class ESignDialog { } } - // Fallback to default contact extraction if no custom contacts + // Fallback to default contact extraction if no custom contacts. These are fields of + // whichever doctype is open (Sales Order, Purchase Order, …), so they are read as strings. + const field = (fieldname: string): string => { + const value = doc[fieldname]; + return typeof value === "string" ? value : ""; + }; if (contacts.length === 0) { - if (this.frm.doc.customer && this.frm.doc.customer_email) { + if (field("customer") && field("customer_email")) { contacts.push({ - value: this.frm.doc.customer_email, - label: this.frm.doc.customer_email, - description: this.frm.doc.customer, + value: field("customer_email"), + label: field("customer_email"), + description: field("customer"), }); } - if (this.frm.doc.supplier && this.frm.doc.supplier_email) { + if (field("supplier") && field("supplier_email")) { contacts.push({ - value: this.frm.doc.supplier_email, - label: this.frm.doc.supplier_email, - description: this.frm.doc.supplier, + value: field("supplier_email"), + label: field("supplier_email"), + description: field("supplier"), }); } - if (this.frm.doc.contact_email) { + if (field("contact_email")) { contacts.push({ - value: this.frm.doc.contact_email, - label: this.frm.doc.contact_email, - description: this.frm.doc.contact_display || "Contact", + value: field("contact_email"), + label: field("contact_email"), + description: field("contact_display") || "Contact", }); } } @@ -294,15 +353,15 @@ class ESignDialog { this.contact_list_cache = contacts; } - get_cached_contact_list(txt) { + get_cached_contact_list(txt?: string): ContactOption[] { // Filter based on search text return this.contact_list_cache.filter( (contact) => !txt || contact.value.toLowerCase().includes(txt.toLowerCase()), ); } - load_email_template(template_name) { - frappe.call({ + load_email_template(template_name: string): void { + frappe.call<{ subject: string; message: string }>({ method: "frappe.email.doctype.email_template.email_template.get_email_template", args: { template_name: template_name, @@ -310,19 +369,19 @@ class ESignDialog { }, callback: (r) => { if (r.message) { - this.dialog.set_value("subject", r.message.subject); - this.dialog.set_value("content", r.message.message); + void this.dialog.set_value("subject", r.message.subject); + void this.dialog.set_value("content", r.message.message); } }, }); } - update_esign_link_preview() { + update_esign_link_preview(): void { const web_form = this.dialog.get_value("web_form"); const print_format = this.dialog.get_value("print_format") || "Standard"; if (web_form) { - frappe.call({ + frappe.call({ method: "esign.esign.custom.web_form.get_esign_link", args: { doc: this.frm.doc, @@ -331,7 +390,11 @@ class ESignDialog { }, callback: (r) => { if (r.message) { - this.dialog.fields_dict.esign_link_preview.$wrapper.html( + const preview = this.dialog.fields_dict.esign_link_preview; + if (!preview || !("$wrapper" in preview)) { + return; + } + preview.$wrapper.html( `

eSign Link:

${r.message} @@ -343,8 +406,8 @@ class ESignDialog { } } - send_esign_request(values) { - frappe.call({ + send_esign_request(values: Record): void { + frappe.call({ method: "esign.esign.send_esign_email", args: { doctype: this.frm.doctype, @@ -367,7 +430,7 @@ class ESignDialog { message: __("eSign request sent successfully"), indicator: "green", }); - this.frm.reload_doc(); + void this.frm.reload_doc(); } }, }); diff --git a/esign/public/js/fonts.js b/esign/public/js/fonts.js deleted file mode 100644 index 4648e40..0000000 --- a/esign/public/js/fonts.js +++ /dev/null @@ -1,17 +0,0 @@ -// Copyright (c) 2026, Avunu LLC and contributors -// For license information, please see license.txt - -/** - * @fileoverview Font bundle entry point for Vite build. - * Imports @fontsource packages so Vite extracts the CSS and font files. - * - * @author Avunu LLC - */ - -// Side-effect imports: each module registers itself or emits CSS and font files. -// oxlint-disable import/no-unassigned-import -import "@fontsource/pacifico"; -import "@fontsource/dancing-script"; -import "@fontsource/great-vibes"; -import "@fontsource/caveat"; -// oxlint-enable import/no-unassigned-import diff --git a/esign/public/js/web/esign.web.bundle.js b/esign/public/js/web/esign.web.bundle.ts similarity index 55% rename from esign/public/js/web/esign.web.bundle.js rename to esign/public/js/web/esign.web.bundle.ts index 4574188..789264c 100644 --- a/esign/public/js/web/esign.web.bundle.js +++ b/esign/public/js/web/esign.web.bundle.ts @@ -9,13 +9,13 @@ */ // Side-effect imports: each module registers itself or emits CSS and font files. // oxlint-disable import/no-unassigned-import -import "../../../../../frappe/frappe/public/js/bootstrap-4-web.bundle.js"; -import "../../../../../frappe/frappe/public/js/controls.bundle.js"; -import "../../../../../frappe/frappe/public/js/dialog.bundle.js"; -import "../../../../../frappe/frappe/public/js/frappe/ui/keyboard.js"; -import "../../../../../frappe/frappe/public/js/frappe/utils/datetime.js"; -import "../../../../../frappe/frappe/public/js/lib/moment.js"; -import "./esign_webform_script.js"; +import "frappe/public/js/bootstrap-4-web.bundle.js"; +import "frappe/public/js/controls.bundle.js"; +import "frappe/public/js/dialog.bundle.js"; +import "frappe/public/js/frappe/ui/keyboard.js"; +import "frappe/public/js/frappe/utils/datetime.js"; +import "frappe/public/js/lib/moment.js"; +import "./esign_webform_script"; // oxlint-enable import/no-unassigned-import import { ControlFontSelect } from "../controls/font_select"; frappe.ui.form.ControlFontSelect = ControlFontSelect; diff --git a/esign/public/js/web/esign_webform_script.js b/esign/public/js/web/esign_webform_script.ts similarity index 77% rename from esign/public/js/web/esign_webform_script.js rename to esign/public/js/web/esign_webform_script.ts index 4a84d3d..e6a12dd 100644 --- a/esign/public/js/web/esign_webform_script.js +++ b/esign/public/js/web/esign_webform_script.ts @@ -8,8 +8,27 @@ * It extends WebForm to use our custom accept endpoint and registers * our custom controls for the web/public frontend. */ -import WebFormList from "../../../../../frappe/frappe/public/js/frappe/web_form/web_form_list"; -import WebForm from "../../../../../frappe/frappe/public/js/frappe/web_form/web_form"; +import type { FrappeDocBase, FrappeResponse } from "frappe-types"; +// Also brings in the website globals frappe adds on a web form page (frappe.ready, …). +import type { WebFormDoc } from "frappe-types/web"; +import WebFormList from "frappe/public/js/frappe/web_form/web_form_list"; +import WebForm from "frappe/public/js/frappe/web_form/web_form"; +import { ControlSignature } from "../controls/signature"; +import { ControlUpload } from "../controls/upload"; + +/** What esign.esign.custom.web_form.get_print_html returns. */ +interface PrintHtmlResponse { + print_html: string; + print_style: string; +} + +/** The fields of a saved web form document that the save handler reads. */ +interface SavedDoc { + name: string; + doctype: string; + attachment?: string; + file?: string; +} // ============================================================================ // Document Preview Functions @@ -18,7 +37,7 @@ import WebForm from "../../../../../frappe/frappe/public/js/frappe/web_form/web_ /** * Scale print format to fit the document pane width */ -function scaleToFitDocumentPane() { +function scaleToFitDocumentPane(): void { const documentPane = document.querySelector(".esign-document-pane"); const shadowHost = document.getElementById("shadow-host"); @@ -26,13 +45,13 @@ function scaleToFitDocumentPane() { return; } - const printFormat = shadowHost.shadowRoot.querySelector(".print-format"); + const printFormat = shadowHost.shadowRoot.querySelector(".print-format"); if (!printFormat) { return; } // Reset zoom to measure natural width - printFormat.style.zoom = "100%"; + printFormat.style.setProperty("zoom", "100%"); // Get the natural width of the print format content const contentWidth = printFormat.scrollWidth; @@ -51,24 +70,24 @@ function scaleToFitDocumentPane() { } // Apply the scale transformation - printFormat.style.zoom = `${scale}%`; + printFormat.style.setProperty("zoom", `${scale}%`); } /** * Load print HTML asynchronously from the server - * @returns {Promise} Promise that resolves when content is loaded and rendered + * @returns Promise that resolves when content is loaded and rendered */ -function loadPrintHtml() { +function loadPrintHtml(): Promise { const ctx = frappe.esign_context; if (!ctx || !ctx.doctype || !ctx.name) { console.error("eSign context not available"); - return Promise.reject("Context not available"); + return Promise.reject(new Error("Context not available")); } const shadowHost = document.getElementById("shadow-host"); if (!shadowHost || !shadowHost.shadowRoot) { console.error("Shadow host not available"); - return Promise.reject("Shadow host not available"); + return Promise.reject(new Error("Shadow host not available")); } const shadowRoot = shadowHost.shadowRoot; @@ -79,8 +98,9 @@ function loadPrintHtml() { if (loadingEl) loadingEl.classList.remove("hide"); if (printContent) printContent.classList.add("hide"); - return frappe - .call({ + // Promise.resolve adopts the jqXHR, so callers get a standard promise. + return Promise.resolve( + frappe.call({ method: "esign.esign.custom.web_form.get_print_html", args: { doctype: ctx.doctype, @@ -89,13 +109,14 @@ function loadPrintHtml() { key: ctx.key || "", }, freeze: false, - }) - .then((response) => { + }), + ) + .then((response: FrappeResponse) => { if (response.message) { renderPrintHtml(response.message.print_html, response.message.print_style); } }) - .catch((error) => { + .catch((error: unknown) => { console.error("Failed to load print HTML:", error); if (loadingEl) { loadingEl.innerHTML = '

Failed to load document preview.

'; @@ -105,10 +126,10 @@ function loadPrintHtml() { /** * Render print HTML into the shadow DOM - * @param {string} printHtml - The HTML content to render - * @param {string} printStyle - Additional CSS styles for the print format + * @param printHtml - The HTML content to render + * @param printStyle - Additional CSS styles for the print format */ -function renderPrintHtml(printHtml, printStyle) { +function renderPrintHtml(printHtml: string, printStyle: string): void { const shadowHost = document.getElementById("shadow-host"); if (!shadowHost || !shadowHost.shadowRoot) return; @@ -136,7 +157,7 @@ function renderPrintHtml(printHtml, printStyle) { // Re-scale when fonts are loaded if (document.fonts) { - document.fonts.ready.then(() => { + void document.fonts.ready.then(() => { setTimeout(scaleToFitDocumentPane, 100); }); } @@ -144,9 +165,9 @@ function renderPrintHtml(printHtml, printStyle) { /** * Refresh the document preview (called after form submission) - * @returns {Promise} Promise that resolves when content is refreshed + * @returns Promise that resolves when content is refreshed */ -function refreshPrintHtml() { +function refreshPrintHtml(): Promise { return loadPrintHtml(); } @@ -157,20 +178,19 @@ window.refreshPrintHtml = refreshPrintHtml; /** * Initialize document preview functionality */ -function initDocumentPreview() { +function initDocumentPreview(): void { const ctx = frappe.esign_context; // Skip loading print HTML if we have a signed PDF (completed documents) if (ctx && ctx.signed_pdf_url) { - console.debug("Signed PDF available, skipping print HTML load"); return; } // Load print HTML for documents that need signing - loadPrintHtml(); + void loadPrintHtml(); // Re-scale on window resize with debouncing - let resizeTimeout; + let resizeTimeout: ReturnType | undefined; window.addEventListener("resize", () => { clearTimeout(resizeTimeout); resizeTimeout = setTimeout(scaleToFitDocumentPane, 150); @@ -181,24 +201,19 @@ function initDocumentPreview() { // Custom Controls Registration // ============================================================================ -// Import and register eSign controls for web forms -import { ControlUpload } from "../controls/upload"; -import { ControlSignature } from "../controls/signature"; - +// Register eSign controls for web forms frappe.ui.form.ControlUpload = ControlUpload; frappe.ui.form.ControlSignature = ControlSignature; class EsignWebForm extends WebForm { - handle_success(data) { + override handle_success(data: unknown): void { // Override to use eSign success card in sidebar const formCard = document.querySelector(".esign-form-card"); const successCard = document.getElementById("esign-success-card"); // Refresh the document preview to show the new signature if (window.refreshPrintHtml) { - window.refreshPrintHtml().then(() => { - console.debug("Document preview refreshed after signing"); - }); + void window.refreshPrintHtml(); } if (formCard && successCard) { @@ -216,34 +231,32 @@ class EsignWebForm extends WebForm { } } - handle_redirect() { + handle_redirect(): void { + const success_url = this.success_url ?? ""; // Countdown and redirect const timeSpan = document.querySelector(".esign-redirect-message .time"); if (!timeSpan) { - window.location.href = this.success_url; + window.location.href = success_url; return; } let countdown = 5; const interval = setInterval(() => { countdown--; - timeSpan.textContent = countdown; + timeSpan.textContent = String(countdown); if (countdown <= 0) { clearInterval(interval); - window.location.href = this.success_url; + window.location.href = success_url; } }, 1000); } - save() { + override save(): false { // Use eSign accept endpoint if esign_enabled, otherwise use default let method = this.esign_enabled ? "esign.esign.custom.web_form.accept" : "frappe.website.doctype.web_form.web_form.accept"; - // debug - console.debug("Using save method:", method); - let is_new = this.is_new; let valid = this.validate && this.validate(); if (!valid && valid !== undefined) { @@ -252,7 +265,7 @@ class EsignWebForm extends WebForm { } // validation hack: get_values will check for missing data - let doc_values = super.get_values(this.allow_incomplete); + let doc_values = this.get_values(Boolean(this.allow_incomplete)); if (!doc_values) return false; @@ -269,7 +282,7 @@ class EsignWebForm extends WebForm { window.saving = true; frappe.form_dirty = false; - frappe.call({ + frappe.call({ type: "POST", method: method, args: { @@ -281,7 +294,7 @@ class EsignWebForm extends WebForm { freeze: true, callback: (response) => { // Check for any exception in response - if (!response.exc) { + if (!response.exc && response.message) { // Success this.handle_success(response.message); frappe.web_form.events.trigger("after_save"); @@ -321,7 +334,6 @@ frappe.ready(function () { // If document is already completed/signed, don't initialize the form if (frappe.is_completed) { - console.debug("Document already signed, skipping form initialization"); return; } @@ -379,15 +391,16 @@ frappe.ready(function () { }); } - function setup_fields(form_doc, doc_data) { + function setup_fields(form_doc: WebFormDoc, doc_data: FrappeDocBase) { form_doc.web_form_fields.forEach((df) => { df.is_web_form = true; - df.read_only = df.read_only || (!form_doc.is_new && !form_doc.in_edit_mode); + df.read_only = df.read_only || (!form_doc.is_new && !form_doc.in_edit_mode ? 1 : 0); if (df.fieldtype === "Table") { df.get_data = () => { - let data = []; - if (doc_data && doc_data[df.fieldname]) { - return doc_data[df.fieldname]; + let data: unknown[] = []; + const rows = doc_data?.[df.fieldname]; + if (Array.isArray(rows) && rows.length) { + return rows; } return data; }; @@ -408,16 +421,18 @@ frappe.ready(function () { delete df.parenttype; delete df.doctype; - return df; + return; } if (df.fieldtype === "Link") { df.only_select = true; } if (["Attach", "Attach Image"].includes(df.fieldtype)) { - if (typeof df.options !== "object") { - df.options = {}; - } - df.options.disable_file_browser = true; + // The Attach controls read an object here; frappe's own web form script does the same. + const options: unknown = df.options; + const attach_options = + typeof options === "object" && options !== null ? (options as Record) : {}; + attach_options.disable_file_browser = true; + Reflect.set(df, "options", attach_options); } }); } diff --git a/esign/public/js/web_form.js b/esign/public/js/web_form.js index 40c27e6..62e29a5 100644 --- a/esign/public/js/web_form.js +++ b/esign/public/js/web_form.js @@ -1,41 +1,53 @@ // Copyright (c) 2026, Avunu LLC and contributors // For license information, please see license.txt -frappe.ui.form.on("Web Form", { - refresh: function (frm) { +/** + * Fill the "Set Property After Response" select with the value fields of the web form's + * doctype. + * @param {frappe.ui.form.Form<"Web Form">} frm + */ +function update_field_options(frm) { + const doc_type = frm.doc.doc_type; + if (!doc_type) { + return; + } + void frappe.model.with_doctype(doc_type, () => { + const fieldnames = (frappe.get_meta(doc_type)?.fields ?? []) + .filter((field) => !frappe.model.no_value_type.includes(field.fieldtype)) + .map((field) => field.fieldname); + + frm.set_df_property("esign_update_field", "options", [""].concat(fieldnames)); + }); +} + +/** + * Offer only enabled Custom-event notifications for the web form's doctype. + * @param {frappe.ui.form.Form<"Web Form">} frm + */ +function set_notification_query(frm) { + frm.set_query("esign_notification", function () { + return { + filters: { + document_type: frm.doc.doc_type, + event: "Custom", + enabled: 1, + }, + }; + }); +} + +/** @type {frappe.ui.form.FormEvents<"Web Form">} */ +const web_form_events = { + refresh(frm) { if (frm.doc.doc_type) { - frm.events.update_field_options(frm); + update_field_options(frm); } - frm.events.set_notification_query(frm); + set_notification_query(frm); }, - doc_type: function (frm) { - frm.events.update_field_options(frm); - frm.events.set_notification_query(frm); + doc_type(frm) { + update_field_options(frm); + set_notification_query(frm); }, - update_field_options: function (frm) { - var doc = frm.doc; - if (!doc.doc_type) { - return; - } - frappe.model.with_doctype(doc.doc_type, () => { - const fieldnames = frappe - .get_meta(doc.doc_type) - .fields.filter((field) => !frappe.model.no_value_type.includes(field.fieldtype)) - .map((field) => field.fieldname); +}; - frm.set_df_property("esign_update_field", "options", [""].concat(fieldnames)); - }); - }, - set_notification_query: function (frm) { - // Filter notifications to only show Custom event notifications for the same doctype - frm.set_query("esign_notification", function () { - return { - filters: { - document_type: frm.doc.doc_type, - event: "Custom", - enabled: 1, - }, - }; - }); - }, -}); +frappe.ui.form.on("Web Form", web_form_events); diff --git a/esign/tests/test_esign.py b/esign/tests/test_esign.py index e3175ac..42a0d26 100644 --- a/esign/tests/test_esign.py +++ b/esign/tests/test_esign.py @@ -66,6 +66,12 @@ def test_controllers_are_extended_not_overridden(self): self.assertTrue(issubclass(get_controller("Web Form"), EsignWebForm)) self.assertTrue(issubclass(get_controller("Email Template"), EsignEmailTemplate)) + def test_assets_are_named_as_their_bundles(self): + hooks = frappe.get_hooks(app_name="esign") + # assets.json keys a bundle by its source file name, .ts included. + self.assertEqual(hooks["app_include_js"], ["esign.desk.bundle.ts", "esign.control.bundle.ts"]) + self.assertEqual(hooks["app_include_css"], ["esign.control.bundle.css", "esign-fonts.bundle.css"]) + def test_jinja_and_timeline_hooks(self): hooks = frappe.get_hooks(app_name="esign") self.assertIn("esign.esign.custom.web_form.get_esign_link", hooks["jinja"]["methods"]) diff --git a/esign/tests/test_web_form.py b/esign/tests/test_web_form.py index 22ed6eb..e25855e 100644 --- a/esign/tests/test_web_form.py +++ b/esign/tests/test_web_form.py @@ -217,7 +217,8 @@ def test_guest_with_key_gets_the_signing_page(self): self.assertIn(f'key: "{self.key}"', content) self.assertIn('print_format: "Standard"', content) self.assertIn("signer@example.com", content) - self.assertIn("esign.web.bundle", content) + self.assertIn("esign.web.bundle.ts", content) + self.assertIn("esign-fonts.bundle.css", content) self.assertIn("frappe.utils.print_format.download_pdf", content) self.assertNotIn("esign-completed", content) diff --git a/package.json b/package.json index 23737ab..84fe0f4 100644 --- a/package.json +++ b/package.json @@ -26,6 +26,7 @@ "signature_pad": "^5.1.3" }, "devDependencies": { + "@types/hast": "^3.0.4", "@types/node": "^26.6.4", "frappe-types": "^16.5.0", "oxfmt": "^0.72.0", diff --git a/tsconfig.base.json b/tsconfig.base.json new file mode 100644 index 0000000..8d49c15 --- /dev/null +++ b/tsconfig.base.json @@ -0,0 +1,9 @@ +// frappe-nix:managed — generated by frappe-nix (`frappe-init --sync`); do not edit. Customise via [tool.frappe-nix.typescript]. +{ + "extends": "frappe-types/tsconfig/base.json", + "compilerOptions": { + "noUnusedLocals": true, + "noUnusedParameters": true, + "noImplicitReturns": true + } +} diff --git a/tsconfig.browser.json b/tsconfig.browser.json new file mode 100644 index 0000000..9f203f1 --- /dev/null +++ b/tsconfig.browser.json @@ -0,0 +1,11 @@ +// frappe-nix:managed — generated by frappe-nix (`frappe-init --sync`); do not edit. Customise via [tool.frappe-nix.typescript]. +{ + "extends": "./tsconfig.base.json", + "compilerOptions": { + "composite": true, + "types": ["frappe-types/global"], + "allowImportingTsExtensions": true + }, + "include": ["esign/public/js/**/*.ts", "esign/public/js/**/*.d.ts", "types/**/*.d.ts"], + "exclude": ["node_modules", "esign/public/dist"] +} diff --git a/tsconfig.json b/tsconfig.json index 62d9943..ca34648 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -2,6 +2,9 @@ { "files": [], "references": [ + { + "path": "./tsconfig.browser.json" + }, { "path": "./tsconfig.desk.json" } diff --git a/types/doctypes.d.ts b/types/doctypes.d.ts new file mode 100644 index 0000000..1e62e9d --- /dev/null +++ b/types/doctypes.d.ts @@ -0,0 +1,226 @@ +// Generated by `frappe-types gen-registry`. Do not edit; re-run the generator. +// --app esign +// DocType JSON read from: frappe +// customisations read from: esign +// +// Each interface lists one DocType's data fields as its JSON files declare them, plus +// the Custom Fields and Property Setters of the apps above. `DocOf` adds the +// standard fields. Fields added in the database (Customize Form) are in no file and +// so are not here: a registered document is closed, so add any your code reads by +// augmenting `FrappeDocTypeFields`. Other generated files (other apps') can share the +// program: see the frappe-types README, "Generating the registry". + +import type * as $ft from "frappe-types"; + +declare global { + /** + * DocType name -> its data fields. Every entry is an interface of the global + * `FrappeDocTypeFields` namespace, so a second generated file that registers the same + * DocType declares the same entry, and its fields merge into the same interface. + */ + interface FrappeDocTypes { + "Email Template": FrappeDocTypeFields.EmailTemplate; + "Web Form": FrappeDocTypeFields.WebForm; + "Web Form Field": FrappeDocTypeFields.WebFormField; + "Web Form List Column": FrappeDocTypeFields.WebFormListColumn; + } + + /** The data fields of each registered DocType, by frappe's controller class name. */ + namespace FrappeDocTypeFields { + /** + * DocType "Email Template" — frappe, module Email. + * `frappe/frappe/email/doctype/email_template/email_template.json` + * Included because it is customised by esign/esign/esign/custom/email_template.json. + */ + interface EmailTemplate { + /** Subject · Data */ + subject?: string | null; + /** Reference DocType · Link → DocType */ + reference_doctype?: string | null; + /** Response · Text Editor */ + response?: string | null; + /** Use HTML · Check */ + use_html?: 0 | 1; + /** Response · Code */ + response_html?: string | null; + /** eSign Request · Check · Custom Field, esign/esign/esign/custom/email_template.json */ + esign_request?: 0 | 1; + } + + /** + * DocType "Web Form" — frappe, module Website. + * `frappe/frappe/website/doctype/web_form/web_form.json` + * Included because it is customised by esign/esign/esign/custom/web_form.json. + */ + interface WebForm { + /** Title · Data */ + title?: string | null; + /** Route · Data */ + route?: string | null; + /** Select DocType · Link → DocType */ + doc_type?: string | null; + /** Module · Link → Module Def */ + module?: string | null; + /** Is standard · Check */ + is_standard?: 0 | 1; + /** Published · Check */ + published?: 0 | 1; + /** Login required · Check */ + login_required?: 0 | 1; + /** Allow editing after submit · Check */ + allow_edit?: 0 | 1; + /** Allow multiple responses · Check */ + allow_multiple?: 0 | 1; + /** Allow delete · Check */ + allow_delete?: 0 | 1; + /** Allow print · Check */ + allow_print?: 0 | 1; + /** Print Format · Link → Print Format */ + print_format?: string | null; + /** Allow comments · Check */ + allow_comments?: 0 | 1; + /** Show attachments · Check */ + show_attachments?: 0 | 1; + /** Allow incomplete forms · Check */ + allow_incomplete?: 0 | 1; + /** Introduction · Text Editor */ + introduction_text?: string | null; + /** Web Form Fields · Table → Web Form Field */ + web_form_fields: FrappeChildRow<"Web Form Field">[]; + /** Max attachment size · Int */ + max_attachment_size?: number | null; + /** Client script · Code */ + client_script?: string | null; + /** Submit button label · Data */ + button_label?: string | null; + /** Success message · Text */ + success_message?: string | null; + /** Success URL · Data */ + success_url?: string | null; + /** Show sidebar · Check */ + show_sidebar?: 0 | 1; + /** Breadcrumbs · Code */ + breadcrumbs?: string | null; + /** Custom CSS · Code */ + custom_css?: string | null; + /** Apply document permissions · Check */ + apply_document_permissions?: 0 | 1; + /** Show list · Check */ + show_list?: 0 | 1; + /** Title · Data */ + list_title?: string | null; + /** List Columns · Table → Web Form List Column */ + list_columns: FrappeChildRow<"Web Form List Column">[]; + /** Website Sidebar · Link → Website Sidebar */ + website_sidebar?: string | null; + /** Success title · Data */ + success_title?: string | null; + /** Banner Image · Attach Image */ + banner_image?: string | null; + /** Meta title · Data */ + meta_title?: string | null; + /** Meta description · Small Text */ + meta_description?: string | null; + /** Meta image · Attach Image */ + meta_image?: string | null; + /** Anonymous responses · Check */ + anonymous?: 0 | 1; + /** Condition JSON · JSON */ + condition_json?: unknown; + /** Dynamic Filters JSON · JSON */ + dynamic_filters_json?: unknown; + /** Hide navbar · Check */ + hide_navbar?: 0 | 1; + /** Hide footer · Check */ + hide_footer?: 0 | 1; + /** Allowed embedding domains · Small Text */ + allowed_embedding_domains?: string | null; + /** Key required · Check */ + key_required?: 0 | 1; + /** Enable eSign · Check · Custom Field, esign/esign/esign/custom/web_form.json */ + esign_enabled?: 0 | 1; + /** Submit on Response · Check · Custom Field, esign/esign/esign/custom/web_form.json */ + esign_submit_on_response?: 0 | 1; + /** Only allow Signing on Mobile · Check · Custom Field, esign/esign/esign/custom/web_form.json */ + esign_force_mobile?: 0 | 1; + /** Set Property After Response · Select · Custom Field, esign/esign/esign/custom/web_form.json */ + esign_update_field?: string | null; + /** Value To Be Set · Data · Custom Field, esign/esign/esign/custom/web_form.json */ + esign_update_value?: string | null; + /** Send Notification on Response · Check · Custom Field, esign/esign/esign/custom/web_form.json */ + esign_send_notification?: 0 | 1; + /** Notification · Link → Notification · Custom Field, esign/esign/esign/custom/web_form.json */ + esign_notification?: string | null; + } + + /** + * DocType "Web Form Field" — frappe, module Website. + * `frappe/frappe/website/doctype/web_form_field/web_form_field.json` + * Included because it is a child table of Web Form.web_form_fields. + */ + interface WebFormField { + /** The DocTypes on this bench with a Table of Web Form Field: the forms its events run on. */ + parenttype: "Web Form"; + /** Field · Select */ + fieldname?: string | null; + /** Fieldtype · Select */ + fieldtype?: "Attach" | "Attach Image" | "Check" | "Currency" | "Color" | "Data" | "Date" | "Datetime" | "Duration" | "Float" | "HTML" | "Int" | "Link" | "Password" | "Phone" | "Rating" | "Select" | "Signature" | "Small Text" | "Text" | "Text Editor" | "Table" | "Time" | "Section Break" | "Column Break" | "Page Break" | "" | null; + /** Custom Label · Data */ + label?: string | null; + /** Allow Read On All Link Options · Check */ + allow_read_on_all_link_options?: 0 | 1; + /** Mandatory · Check */ + reqd?: 0 | 1; + /** Display Depends On · Code */ + depends_on?: string | null; + /** Read Only · Check */ + read_only?: 0 | 1; + /** Show in filter · Check */ + show_in_filter?: 0 | 1; + /** Hidden · Check */ + hidden?: 0 | 1; + /** Options · Text */ + options?: string | null; + /** Max Length · Int */ + max_length?: number | null; + /** Max Value · Int */ + max_value?: number | null; + /** Description · Text */ + description?: string | null; + /** Default · Data */ + default?: string | null; + /** Mandatory Depends On · Code */ + mandatory_depends_on?: string | null; + /** Read Only Depends On · Code */ + read_only_depends_on?: string | null; + /** Precision · Select */ + precision?: "0" | "1" | "2" | "3" | "4" | "5" | "6" | "7" | "8" | "9" | "" | null; + /** Placeholder · Data */ + placeholder?: string | null; + } + + /** + * DocType "Web Form List Column" — frappe, module Website. + * `frappe/frappe/website/doctype/web_form_list_column/web_form_list_column.json` + * Included because it is a child table of Web Form.list_columns. + * `autoname: autoincrement`: the server sends `name` as a number (frappe/model/naming.py:161-162), though the registry types it string. + */ + interface WebFormListColumn { + /** The DocTypes on this bench with a Table of Web Form List Column: the forms its events run on. */ + parenttype: "Web Form"; + /** Field · Select */ + fieldname?: string | null; + /** Custom Label · Data */ + label?: string | null; + /** Fieldtype · Data */ + fieldtype?: string | null; + /** Options · Text */ + options?: string | null; + } + } +} + +export type EmailTemplate = FrappeDocTypeFields.EmailTemplate; +export type WebForm = FrappeDocTypeFields.WebForm; +export type WebFormField = FrappeDocTypeFields.WebFormField; +export type WebFormListColumn = FrappeDocTypeFields.WebFormListColumn; diff --git a/types/esign.augment.d.ts b/types/esign.augment.d.ts new file mode 100644 index 0000000..ccfcd05 --- /dev/null +++ b/types/esign.augment.d.ts @@ -0,0 +1,50 @@ +// Copyright (c) 2026, Avunu LLC and contributors +// For license information, please see license.txt + +// The names esign adds to frappe's namespaces and to the page, for the desk scripts and the +// compiled bundles. A module (it imports), so the `frappe-types` block merges into the package. + +import type { BaseControl } from "frappe-types"; + +/** What the eSign page template (esign/templates/esign.html) passes to the page script. */ +interface EsignContext { + doctype: string; + name: string; + key: string; + print_format: string; + signed_pdf_url: string; +} + +declare module "frappe-types" { + interface FrappeUiFormNamespace { + // app-owned: the image upload control, registered by esign's control bundles + ControlUpload: typeof BaseControl; + // app-owned: the font picker control, registered by esign's control bundles + ControlFontSelect: typeof BaseControl; + } + + interface Frappe { + // app-owned: set by the eSign page template for its document preview + esign_context?: EsignContext; + // app-owned: set by the eSign page template once every signature field is signed + is_completed?: boolean; + } +} + +declare global { + /** A family the Local Font Access API reports (only the member esign reads). */ + interface LocalFontData { + readonly family: string; + } + + interface Window { + // app-owned: the eSign page's document preview loader, for other page scripts + loadPrintHtml?: () => Promise; + // app-owned: reloads the eSign page's document preview after signing + refreshPrintHtml?: () => Promise; + // frappe's web form save guard (frappe/public/js/frappe/web_form/web_form.js) + saving?: boolean; + // The Local Font Access API (Chromium only), not in TypeScript's DOM library + queryLocalFonts?: () => Promise; + } +} diff --git a/types/frappe-modules.d.ts b/types/frappe-modules.d.ts new file mode 100644 index 0000000..e6c2480 --- /dev/null +++ b/types/frappe-modules.d.ts @@ -0,0 +1,47 @@ +// Copyright (c) 2026, Avunu LLC and contributors +// For license information, please see license.txt + +// frappe's own web form modules, which the eSign web bundle imports and frappe-types does not +// declare, and the non-script files the bundles import. frappe's esbuild resolves +// "frappe/public/js/…" from the bench's apps directory. + +/** The website bundles and scripts the eSign page loads for their side effects. */ +declare module "frappe/public/js/*.js"; + +/** A frappe microtemplate: esbuild compiles it into a `frappe.templates` entry. */ +declare module "*.html"; + +declare module "frappe/public/js/frappe/web_form/web_form" { + import type { WebForm } from "frappe-types/web"; + + /** frappe/public/js/frappe/web_form/web_form.js: the members the eSign web form uses. */ + export interface WebFormController extends WebForm { + /** Shows the success message after a save, or redirects to `success_url`. */ + handle_success(data: unknown): void; + /** Builds the fields, the actions and the client script into `parent`. */ + prepare(web_form_doc: unknown, doc: unknown): void; + make(): void; + success_url?: string; + allow_incomplete?: boolean | 0 | 1; + accept_payment?: boolean | 0 | 1; + esign_enabled?: boolean | 0 | 1; + } + + const WebFormClass: { + new (opts: { parent: JQuery }): WebFormController; + prototype: WebFormController; + }; + export default WebFormClass; +} + +declare module "frappe/public/js/frappe/web_form/web_form_list" { + /** frappe/public/js/frappe/web_form/web_form_list.js: renders the list from its constructor. */ + const WebFormList: new (opts: { + doctype: string; + web_form_name: string; + list_columns: unknown; + condition_json: unknown; + settings: { allow_delete: unknown }; + }) => object; + export default WebFormList; +} diff --git a/vite.config.mjs b/vite.config.mjs index 2f179a4..c7f0368 100644 --- a/vite.config.mjs +++ b/vite.config.mjs @@ -1,24 +1,28 @@ +// Copyright (c) 2026, Avunu LLC and contributors +// For license information, please see license.txt + import { defineConfig } from "vite"; -import path from "path"; +// The signature fonts: esign/public/css/esign-fonts.entry.css becomes +// public/dist/css/esign-fonts.bundle..css plus its font files, and the manifest lets +// scripts/vite-register.mjs register it in assets.json as esign-fonts.bundle.css, so it is +// served by name and cache-busted like frappe's own bundles. export default defineConfig({ + // Font URLs relative to the stylesheet, so they resolve under /assets/esign/dist/. base: "./", build: { outDir: "esign/public/dist", + // frappe's esbuild bundles share public/dist. emptyOutDir: false, + manifest: true, assetsInlineLimit: 0, rollupOptions: { - input: path.resolve(__dirname, "esign/public/js/fonts.js"), + input: { "esign-fonts": "esign/public/css/esign-fonts.entry.css" }, output: { - entryFileNames: "esign-fonts.js", - assetFileNames: (assetInfo) => { - // Keep font files in a fonts/ subdirectory - if (/\.(woff2?|ttf|otf|eot)$/.test(assetInfo.names?.[0] ?? "")) { - return "fonts/[name][extname]"; - } - // CSS output - return "esign-fonts[extname]"; - }, + assetFileNames: (assetInfo) => + /\.(woff2?|ttf|otf|eot)$/.test(assetInfo.names?.[0] ?? "") + ? "fonts/[name]-[hash][extname]" + : "css/[name].bundle.[hash][extname]", }, }, }, diff --git a/yarn.lock b/yarn.lock index 2401b2b..b84cdb3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -585,6 +585,13 @@ dependencies: "@types/unist" "*" +"@types/hast@^3.0.4": + version "3.0.5" + resolved "https://registry.yarnpkg.com/@types/hast/-/hast-3.0.5.tgz#48020de4c0e63492f4ca9db42068c108f68b7f8f" + integrity sha512-rp/ezSWaD1m44dPKICGhiskI13nVr7qTloFwDa/IYkhhf5nzwP+zIQcIJh3WIFSBOy/H1PzB40jPjMDksN4F+g== + dependencies: + "@types/unist" "*" + "@types/jquery@^3.5": version "3.5.34" resolved "https://registry.yarnpkg.com/@types/jquery/-/jquery-3.5.34.tgz#c1993eaac0db03cf9db974976dd8f07bbf7c5708" From 4e3d5d7e956f6a0b3bd4295aac2fef6265657d6f Mon Sep 17 00:00:00 2001 From: Kevin Shenk Date: Sat, 17 Oct 2026 00:31:07 -0400 Subject: [PATCH 10/14] fix: let owners and users with write permission through the signing page and accept The update guard in accept refused a desk user whose role may write the doctype but who is not the owner and has no key; frappe's own accept saves for them with permission checks on. accept now allows that case too. The signing page without a key now also admits the users the web form's own rules admit (owner, website permission, document permissions), so it agrees with accept. Adds tests for both, and for download_signed_pdf refusing a logged-in user without print permission when no key is given. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SvdEeMiewirZ3ifFoUix5F --- esign/esign/custom/web_form.py | 19 +++++++++++++----- esign/tests/test_web_form.py | 35 ++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/esign/esign/custom/web_form.py b/esign/esign/custom/web_form.py index 215dfee..1ae4c2b 100644 --- a/esign/esign/custom/web_form.py +++ b/esign/esign/custom/web_form.py @@ -235,8 +235,11 @@ def get_context(self, context): # Validate access via share key or fallback to standard permissions. The document is # only marked permission-free afterwards: frappe's website permission check (part of - # the fallback) passes any document that already carries ignore_permissions. - validate_esign_key(key, doc) + # the fallback) passes any document that already carries ignore_permissions. Without a + # key, the web form's own rules (owner, website permission, document permissions) also + # let a user in, as in frappe's web form and in accept. + if key or not BaseWebForm.has_web_form_permission(self, self.doc_type, cur_doc_name, "read"): + validate_esign_key(key, doc) doc.flags.ignore_permissions = True # Allow public access to the web form if validation passes @@ -1160,9 +1163,15 @@ def accept(web_form, data): except frappe.PermissionError, frappe.exceptions.LinkExpired: frappe.throw(_("Invalid or expired access key"), frappe.PermissionError) - # An update needs the document's share key or web form write permission, as in frappe's - # own accept; otherwise anyone who knows a document name could overwrite its form fields. - if data.name and not wf.has_web_form_permission(doctype, data.name, "write"): + # An update needs the document's share key, web form write permission or DocType write + # permission: the three ways frappe's own accept lets an update through (the last one by + # saving with permission checks on). Otherwise anyone who knows a document name could + # overwrite its form fields. + if ( + data.name + and not wf.has_web_form_permission(doctype, data.name, "write") + and not frappe.has_permission(doctype, "write", data.name) + ): frappe.throw(_("You are not permitted to update this document"), frappe.PermissionError) # Now proceed with the form submission using ignore_permissions diff --git a/esign/tests/test_web_form.py b/esign/tests/test_web_form.py index e25855e..9c76c5a 100644 --- a/esign/tests/test_web_form.py +++ b/esign/tests/test_web_form.py @@ -227,6 +227,16 @@ def test_guest_without_key_is_refused(self): content = self.render() self.assertNotIn("esign-layout", content) + def test_owner_without_key_gets_the_signing_page(self): + # A portal user opening their own record from the web form list, as accept allows. + self.doc.db_set("owner", OUTSIDER) + frappe.set_user(OUTSIDER) + self.assertIn("esign-layout", self.render()) + + def test_user_without_key_or_permission_is_refused(self): + frappe.set_user(OUTSIDER) + self.assertNotIn("esign-layout", self.render()) + def test_completed_document_links_the_signed_pdf(self): self.doc.db_set("signature", SIGNATURE) frappe.get_doc( @@ -343,6 +353,17 @@ def test_guest_needs_a_key(self): with signing_request(method="GET"), self.assertRaises(frappe.PermissionError): download_signed_pdf(TEST_DOCTYPE, self.doc.name) + def test_user_without_key_needs_print_permission(self): + self.attach_signed_pdf(is_private=0) + frappe.set_user(OUTSIDER) + with signing_request(method="GET"), self.assertRaises(frappe.PermissionError): + download_signed_pdf(TEST_DOCTYPE, self.doc.name) + + frappe.set_user(SIGNER) + with signing_request(method="GET"): + download_signed_pdf(TEST_DOCTYPE, self.doc.name) + self.assertEqual(frappe.local.response["type"], "redirect") + def test_no_signed_pdf(self): with signing_request(method="GET"), self.assertRaises(frappe.DoesNotExistError): download_signed_pdf(TEST_DOCTYPE, self.doc.name, key=self.key) @@ -475,6 +496,20 @@ def test_user_without_permission_cannot_update(self): with self.assertRaises(frappe.PermissionError): self.accept(form_data(self.doc, signature=SIGNATURE)) + def test_user_with_doctype_write_permission_updates_without_key(self): + # SIGNER is neither the owner nor holds a key, but their role may write the doctype. + frappe.set_user(SIGNER) + self.accept(form_data(self.doc, signature=SIGNATURE)) + self.doc.reload() + self.assertEqual(self.doc.signature, SIGNATURE) + + def test_owner_updates_without_key(self): + self.doc.db_set("owner", OUTSIDER) + frappe.set_user(OUTSIDER) + self.accept(form_data(self.doc, signature=SIGNATURE)) + self.doc.reload() + self.assertEqual(self.doc.signature, SIGNATURE) + def test_anonymous_form_saves_as_guest_and_restores_the_user(self): self.web_form.db_set("anonymous", 1) frappe.set_user(SIGNER) From 86809221ca44a5e30a5f1c8582c64a1a6a0da32d Mon Sep 17 00:00:00 2001 From: Kevin Shenk Date: Sat, 17 Oct 2026 00:31:30 -0400 Subject: [PATCH 11/14] fix: escape the signer's audit values on the timeline card The desk timeline template prints its data with {{ }}, which is raw output in frappe's microtemplates, and several of the values come from the signer's request (user agent, the email parameter, form-derived names). A Guest with a share key could store markup that ran for every desk user opening the timeline. get_timeline_content now HTML-escapes every value it passes to the card, and the email parameter is recorded only when it is a single valid address. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SvdEeMiewirZ3ifFoUix5F --- esign/esign/__init__.py | 30 +++++++++++++++++++----------- esign/esign/custom/web_form.py | 5 +++-- esign/tests/test_esign.py | 28 ++++++++++++++++++++++++++++ esign/tests/test_web_form.py | 6 ++++++ 4 files changed, 56 insertions(+), 13 deletions(-) diff --git a/esign/esign/__init__.py b/esign/esign/__init__.py index 8a77bc3..3b4c860 100644 --- a/esign/esign/__init__.py +++ b/esign/esign/__init__.py @@ -11,12 +11,17 @@ import frappe.desk.form.load from frappe import _ from frappe.core.doctype.communication.communication import Communication -from frappe.utils import now +from frappe.utils import escape_html, now # Communication type for eSign audit trail ESIGN_COMMUNICATION_TYPE = "eSign" +def _escape(value: object) -> str | None: + """A stored audit value, HTML-escaped for the timeline card (None stays None).""" + return None if value is None else escape_html(str(value)) + + def get_timeline_content(doctype: str, docname: str) -> list[dict]: """Return eSign audit trail entries for the document timeline. @@ -64,18 +69,21 @@ def get_timeline_content(doctype: str, docname: str) -> list[dict]: "is_card": True, "creation": comm.communication_date or comm.creation, "template": "esign_timeline", + # The timeline template prints these unescaped ({{ }} is raw in frappe's + # microtemplates), and the signer controls several of them (user agent, email + # parameter, form fields), so every value is escaped here. "template_data": { "creation": str(comm.creation), - "file_url": file_url, - "signer_name": audit_data.get("signer_name") or comm.sender_full_name, - "signer_email": audit_data.get("signer_email") or comm.sender, - "timestamp": audit_data.get("timestamp"), - "ip_address": audit_data.get("ip_address"), - "user_agent": audit_data.get("user_agent"), - "web_form": audit_data.get("web_form"), - "print_format": audit_data.get("print_format"), - "signed_fields": audit_data.get("signed_fields", []), - "pdf_hash": audit_data.get("pdf_hash"), + "file_url": _escape(file_url), + "signer_name": _escape(audit_data.get("signer_name") or comm.sender_full_name), + "signer_email": _escape(audit_data.get("signer_email") or comm.sender), + "timestamp": _escape(audit_data.get("timestamp")), + "ip_address": _escape(audit_data.get("ip_address")), + "user_agent": _escape(audit_data.get("user_agent")), + "web_form": _escape(audit_data.get("web_form")), + "print_format": _escape(audit_data.get("print_format")), + "signed_fields": [_escape(field) for field in audit_data.get("signed_fields") or []], + "pdf_hash": _escape(audit_data.get("pdf_hash")), }, } ) diff --git a/esign/esign/custom/web_form.py b/esign/esign/custom/web_form.py index 1ae4c2b..5fe95b7 100644 --- a/esign/esign/custom/web_form.py +++ b/esign/esign/custom/web_form.py @@ -21,7 +21,7 @@ from frappe.rate_limiter import rate_limit from frappe.twofactor import get_qr_svg_code from frappe.types import DF -from frappe.utils import cint, escape_html, get_frappe_version, now +from frappe.utils import cint, escape_html, get_frappe_version, now, validate_email_address from frappe.website.doctype.web_form.web_form import WebForm as BaseWebForm from frappe.www.printview import validate_print_permission @@ -1041,8 +1041,9 @@ def _collect_audit_data(doc: Document, wf: EsignWebForm, print_format: str) -> A signer_name = None # 1. From the email URL search param (link was sent via email) + # The parameter is the signer's own input, so only a valid address is recorded. email_param = extract_param_from_referrer("email") - if email_param: + if email_param and validate_email_address(email_param) == email_param: signer_email = email_param # 2. From a Data(Email) field on the web form diff --git a/esign/tests/test_esign.py b/esign/tests/test_esign.py index 42a0d26..f719757 100644 --- a/esign/tests/test_esign.py +++ b/esign/tests/test_esign.py @@ -241,6 +241,34 @@ def test_esign_communications_become_timeline_cards(self): self.assertIsNone(by_signer["fallback@example.com"]["template_data"]["file_url"]) self.assertEqual(get_timeline_content("ToDo", "nothing"), []) + def test_signer_controlled_values_are_escaped(self): + payload = '' + audit = { + "signer_name": payload, + "signer_email": payload, + "user_agent": payload, + "ip_address": payload, + "web_form": payload, + "signed_fields": [payload], + } + frappe.get_doc( + { + "doctype": "Communication", + "communication_type": "eSign", + "subject": "Signed", + "content": json.dumps(audit), + "reference_doctype": TEST_DOCTYPE, + "reference_name": self.doc.name, + } + ).insert(ignore_permissions=True) + + data = get_timeline_content(TEST_DOCTYPE, str(self.doc.name))[0]["template_data"] + escaped = "<img src=x onerror="alert(1)">" + for key in ("signer_name", "signer_email", "user_agent", "ip_address", "web_form"): + self.assertEqual(data[key], escaped, key) + self.assertEqual(data["signed_fields"], [escaped]) + self.assertIsNone(data["timestamp"]) + class TestEmailTemplate(EsignDeskTestCase): def test_esign_templates_and_context(self): diff --git a/esign/tests/test_web_form.py b/esign/tests/test_web_form.py index 9c76c5a..1bf9930 100644 --- a/esign/tests/test_web_form.py +++ b/esign/tests/test_web_form.py @@ -648,6 +648,12 @@ def test_signer_email_resolution(self): esign_web_form._collect_audit_data(self.doc, wf, "Standard")["signer_email"], "param@example.com", ) + # An email parameter that is not a single valid address is not recorded. + with signing_request(referrer="https://example.com/form?email=%3Cimg%20src%3Dx%3E"): + self.assertEqual( + esign_web_form._collect_audit_data(self.doc, wf, "Standard")["signer_email"], + "field@example.com", + ) with signing_request(): self.assertEqual( esign_web_form._collect_audit_data(self.doc, wf, "Standard")["signer_email"], From 7f01c7490229bf652083d260933baad972329940 Mon Sep 17 00:00:00 2001 From: Kevin Shenk Date: Sat, 17 Oct 2026 00:31:30 -0400 Subject: [PATCH 12/14] fix: name the bundle entries .js so prebuilt assets resolve them `bench build` keys assets.json by an entry's source name, but `bench build --using-cached` (used by benches that install prebuilt assets) keys by the built file's name, which is always .js. With .ts entries the desk and the signing page asked for esign.*.bundle.ts there and got a 404. The esbuild entries are now small .js files that import the TypeScript modules, and hooks.py and the signing page name esign.*.bundle.js, which both build paths produce. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SvdEeMiewirZ3ifFoUix5F --- esign/esign/custom/web_form.py | 2 +- esign/hooks.py | 7 ++++--- esign/public/js/esign.control.bundle.js | 6 ++++++ esign/public/js/esign.desk.bundle.js | 6 ++++++ .../js/{esign.control.bundle.ts => esign_controls.ts} | 2 +- esign/public/js/{esign.desk.bundle.ts => esign_desk.ts} | 0 esign/public/js/web/esign.web.bundle.js | 7 +++++++ esign/public/js/web/{esign.web.bundle.ts => esign_web.ts} | 0 esign/tests/test_esign.py | 4 ++-- esign/tests/test_web_form.py | 2 +- 10 files changed, 28 insertions(+), 8 deletions(-) create mode 100644 esign/public/js/esign.control.bundle.js create mode 100644 esign/public/js/esign.desk.bundle.js rename esign/public/js/{esign.control.bundle.ts => esign_controls.ts} (87%) rename esign/public/js/{esign.desk.bundle.ts => esign_desk.ts} (100%) create mode 100644 esign/public/js/web/esign.web.bundle.js rename esign/public/js/web/{esign.web.bundle.ts => esign_web.ts} (100%) diff --git a/esign/esign/custom/web_form.py b/esign/esign/custom/web_form.py index 5fe95b7..e78509c 100644 --- a/esign/esign/custom/web_form.py +++ b/esign/esign/custom/web_form.py @@ -293,7 +293,7 @@ def get_context(self, context): ) context.web_include_js.extend( [ - "esign.web.bundle.ts", + "esign.web.bundle.js", ] ) context.template = "esign/templates/esign.html" diff --git a/esign/hooks.py b/esign/hooks.py index 52d3d1c..2018c5a 100644 --- a/esign/hooks.py +++ b/esign/hooks.py @@ -8,9 +8,10 @@ app_email = "mail@avu.nu" app_license = "MIT" -# Bundles are looked up in assets.json by their source name, so the TypeScript entries are named -# with .ts; frappe still serves them as compiled .js files. -app_include_js = ["esign.desk.bundle.ts", "esign.control.bundle.ts"] +# The esbuild entries are .js files that import the TypeScript modules: `bench build` keys +# assets.json by the entry's name and `bench build --using-cached` by the built file's, and only a +# .js entry gets the same key from both. +app_include_js = ["esign.desk.bundle.js", "esign.control.bundle.js"] app_include_css = [ "esign.control.bundle.css", "esign-fonts.bundle.css", diff --git a/esign/public/js/esign.control.bundle.js b/esign/public/js/esign.control.bundle.js new file mode 100644 index 0000000..397751b --- /dev/null +++ b/esign/public/js/esign.control.bundle.js @@ -0,0 +1,6 @@ +// Copyright (c) 2026, Avunu LLC and contributors +// For license information, please see license.txt + +// The esbuild entry point, kept as .js so assets.json names the bundle "esign.control.bundle.js" both +// in a normal build and in `bench build --using-cached`, which keys by the built file's name. +import "./esign_controls.ts"; diff --git a/esign/public/js/esign.desk.bundle.js b/esign/public/js/esign.desk.bundle.js new file mode 100644 index 0000000..ea5aa1a --- /dev/null +++ b/esign/public/js/esign.desk.bundle.js @@ -0,0 +1,6 @@ +// Copyright (c) 2026, Avunu LLC and contributors +// For license information, please see license.txt + +// The esbuild entry point, kept as .js so assets.json names the bundle "esign.desk.bundle.js" both +// in a normal build and in `bench build --using-cached`, which keys by the built file's name. +import "./esign_desk.ts"; diff --git a/esign/public/js/esign.control.bundle.ts b/esign/public/js/esign_controls.ts similarity index 87% rename from esign/public/js/esign.control.bundle.ts rename to esign/public/js/esign_controls.ts index db3667a..832ec39 100644 --- a/esign/public/js/esign.control.bundle.ts +++ b/esign/public/js/esign_controls.ts @@ -4,7 +4,7 @@ /** * eSign Control Bundle (Desk) * - * This bundle is loaded via app_include_js and runs on the desk/backend. + * Loaded through esign.control.bundle.js (app_include_js) and runs on the desk/backend. * At this point frappe.ui.form is guaranteed to be available, so no polling needed. */ import { ControlUpload } from "./controls/upload"; diff --git a/esign/public/js/esign.desk.bundle.ts b/esign/public/js/esign_desk.ts similarity index 100% rename from esign/public/js/esign.desk.bundle.ts rename to esign/public/js/esign_desk.ts diff --git a/esign/public/js/web/esign.web.bundle.js b/esign/public/js/web/esign.web.bundle.js new file mode 100644 index 0000000..e6ff0a2 --- /dev/null +++ b/esign/public/js/web/esign.web.bundle.js @@ -0,0 +1,7 @@ +// Copyright (c) 2026, Avunu LLC and contributors +// For license information, please see license.txt + +// The esbuild entry point, kept as .js so assets.json names the bundle "esign.web.bundle.js" both +// in a normal build and in `bench build --using-cached`, which keys by the built file's name. +// oxlint-disable-next-line import/no-unassigned-import -- the module registers itself +import "./esign_web.ts"; diff --git a/esign/public/js/web/esign.web.bundle.ts b/esign/public/js/web/esign_web.ts similarity index 100% rename from esign/public/js/web/esign.web.bundle.ts rename to esign/public/js/web/esign_web.ts diff --git a/esign/tests/test_esign.py b/esign/tests/test_esign.py index f719757..c89f93d 100644 --- a/esign/tests/test_esign.py +++ b/esign/tests/test_esign.py @@ -68,8 +68,8 @@ def test_controllers_are_extended_not_overridden(self): def test_assets_are_named_as_their_bundles(self): hooks = frappe.get_hooks(app_name="esign") - # assets.json keys a bundle by its source file name, .ts included. - self.assertEqual(hooks["app_include_js"], ["esign.desk.bundle.ts", "esign.control.bundle.ts"]) + # .js entry names: --using-cached keys assets.json by the built file name, always .js. + self.assertEqual(hooks["app_include_js"], ["esign.desk.bundle.js", "esign.control.bundle.js"]) self.assertEqual(hooks["app_include_css"], ["esign.control.bundle.css", "esign-fonts.bundle.css"]) def test_jinja_and_timeline_hooks(self): diff --git a/esign/tests/test_web_form.py b/esign/tests/test_web_form.py index 1bf9930..b152b06 100644 --- a/esign/tests/test_web_form.py +++ b/esign/tests/test_web_form.py @@ -217,7 +217,7 @@ def test_guest_with_key_gets_the_signing_page(self): self.assertIn(f'key: "{self.key}"', content) self.assertIn('print_format: "Standard"', content) self.assertIn("signer@example.com", content) - self.assertIn("esign.web.bundle.ts", content) + self.assertIn("esign.web.bundle.js", content) self.assertIn("esign-fonts.bundle.css", content) self.assertIn("frappe.utils.print_format.download_pdf", content) self.assertNotIn("esign-completed", content) From 5c0da14dfb00628f6fedfde337d7fbc13ac72265 Mon Sep 17 00:00:00 2001 From: Kevin Shenk Date: Sat, 17 Oct 2026 00:46:07 -0400 Subject: [PATCH 13/14] fix: apply the keyless web form rule to the preview and signed-PDF download The signing page admits a logged-in user without a key when the eSign web form's own rules do (owner, website permission, document permissions), but get_print_html and download_signed_pdf still required print permission, so such an owner got the page with a failed preview and a refused signed-PDF link. Both endpoints now take an optional web_form and go through one helper, validate_esign_access, which the page uses as well: without a key, a logged-in user the named eSign web form (for the same doctype) admits is let in; everyone else falls back to the share-key and print-permission checks as before. Guests still need a key. The page passes its web form name to both. Adds tests calling both endpoints as the keyless owner, as a non-owner, with a missing or non-eSign web form, and as Guest. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SvdEeMiewirZ3ifFoUix5F --- esign/esign/custom/web_form.py | 68 ++++++++++++++++++--- esign/public/js/web/esign_webform_script.ts | 1 + esign/templates/esign.html | 1 + esign/tests/test_web_form.py | 54 ++++++++++++++++ types/esign.augment.d.ts | 1 + 5 files changed, 116 insertions(+), 9 deletions(-) diff --git a/esign/esign/custom/web_form.py b/esign/esign/custom/web_form.py index e78509c..d21104d 100644 --- a/esign/esign/custom/web_form.py +++ b/esign/esign/custom/web_form.py @@ -237,9 +237,9 @@ def get_context(self, context): # only marked permission-free afterwards: frappe's website permission check (part of # the fallback) passes any document that already carries ignore_permissions. Without a # key, the web form's own rules (owner, website permission, document permissions) also - # let a user in, as in frappe's web form and in accept. - if key or not BaseWebForm.has_web_form_permission(self, self.doc_type, cur_doc_name, "read"): - validate_esign_key(key, doc) + # let a user in, as in frappe's web form and in accept; the preview and the signed-PDF + # download apply the same rule through the web form name the page passes them. + validate_esign_access(key, doc, self) doc.flags.ignore_permissions = True # Allow public access to the web form if validation passes @@ -279,6 +279,7 @@ def get_context(self, context): super().add_custom_context_and_script(context) context.key = key # Pass key to frontend for async API calls + context.esign_web_form = self.name # Lets the async calls apply the keyless web form rules context.printview_url = ( "/api/method/frappe.utils.print_format.download_pdf?" f"doctype={self.doc_type}&name={cur_doc_name}&format={print_format}&key={key}" @@ -311,7 +312,14 @@ def get_context(self, context): # Build the download URL through our key-validated endpoint from urllib.parse import urlencode - params = urlencode({"doctype": self.doc_type, "docname": cur_doc_name, "key": key}) + params = urlencode( + { + "doctype": self.doc_type, + "docname": cur_doc_name, + "key": key, + "web_form": self.name, + } + ) context.signed_pdf_url = ( f"/api/method/esign.esign.custom.web_form.download_signed_pdf?{params}" ) @@ -351,6 +359,42 @@ def has_web_form_permission(self, doctype, name, ptype="read"): return super().has_web_form_permission(doctype, name, ptype) +def validate_esign_access( + key: str, doc: Document, web_form: EsignWebForm | str | None = None, require_key: bool = False +) -> bool: + """Validate access to an eSign document for the signing page and its endpoints. + + A key is checked as the share key. Without one, a logged-in user the eSign web form's own + rules admit (owner, website permission, document permissions) is let in, so the page, its + preview and its signed-PDF download agree; anyone else falls back to validate_esign_key. + + Args: + key: The share key, or an empty string + doc: The document being accessed + web_form: The eSign web form (or its name) the request comes from, if any + require_key: Passed on to validate_esign_key when the web form rules do not admit + + Raises: + frappe.PermissionError: If access is not granted + """ + if not key and web_form and frappe.session.user != "Guest": + wf = _get_esign_web_form(web_form, doc.doctype) + if wf and BaseWebForm.has_web_form_permission(wf, doc.doctype, str(doc.name), "read"): + return True + return validate_esign_key(key, doc, require_key=require_key) + + +def _get_esign_web_form(web_form: EsignWebForm | str, doctype: str) -> EsignWebForm | None: + """The eSign-enabled web form for doctype, or None if web_form is not one.""" + if isinstance(web_form, str): + if not frappe.db.exists("Web Form", web_form): + return None + web_form = cast(EsignWebForm, frappe.get_cached_doc("Web Form", web_form)) + if not web_form.get("esign_enabled") or web_form.get("doc_type") != doctype: + return None + return web_form + + _UNSET = object() @@ -801,7 +845,7 @@ def get_esign_link( @frappe.whitelist(allow_guest=True) @rate_limit(key="web_form", limit=30, seconds=60) def get_print_html( - doctype: str, docname: str, print_format: str = "standard", key: str = "" + doctype: str, docname: str, print_format: str = "standard", key: str = "", web_form: str = "" ) -> PrintHtmlResponse: """Get the print HTML and styles for a document. @@ -812,6 +856,9 @@ def get_print_html( doctype: The DocType of the document docname: The name of the document print_format: The print format to use (default: "standard") + key: The document share key, if any + web_form: The eSign web form the page belongs to; without a key, its rules can admit + the user (see validate_esign_access) Returns: PrintHtmlResponse with print_html and print_style @@ -834,7 +881,7 @@ def get_print_html( # before the document or the request is marked permission-free: without a key, the # fallback is the standard print permission check, which ignore_print_permissions turns # off and which passes a document that carries ignore_permissions. - validate_esign_key(key, doc) + validate_esign_access(key, doc, web_form) doc.flags.ignore_permissions = True # Access is granted: skip the renderer's own permission checks, which would only add @@ -874,7 +921,7 @@ def get_print_html( @frappe.whitelist(allow_guest=True) @rate_limit(key="esign_download", limit=20, seconds=60) -def download_signed_pdf(doctype: str, docname: str, key: str = ""): +def download_signed_pdf(doctype: str, docname: str, key: str = "", web_form: str = ""): """Download a signed PDF attachment with key-based access validation. This endpoint allows Guest users to download private signed PDF files @@ -884,6 +931,8 @@ def download_signed_pdf(doctype: str, docname: str, key: str = ""): doctype: The DocType of the document docname: The name of the document key: The document share key for access validation + web_form: The eSign web form the page belongs to; without a key, its rules can admit + a logged-in user (see validate_esign_access) Returns: The PDF file response @@ -895,8 +944,9 @@ def download_signed_pdf(doctype: str, docname: str, key: str = ""): doc = frappe.get_doc(doctype, docname) # Validate access via share key - require key for guest access. Without a key a user - # needs print permission, which a document marked ignore_permissions would skip. - validate_esign_key(key, doc, require_key=(frappe.session.user == "Guest")) + # needs the web form's permission or print permission, which a document marked + # ignore_permissions would skip. + validate_esign_access(key, doc, web_form, require_key=(frappe.session.user == "Guest")) # Find the signed PDF for this document # First try to find an eSign Communication for this document diff --git a/esign/public/js/web/esign_webform_script.ts b/esign/public/js/web/esign_webform_script.ts index e6a12dd..7468a69 100644 --- a/esign/public/js/web/esign_webform_script.ts +++ b/esign/public/js/web/esign_webform_script.ts @@ -107,6 +107,7 @@ function loadPrintHtml(): Promise { docname: ctx.name, print_format: ctx.print_format || "standard", key: ctx.key || "", + web_form: ctx.web_form || "", }, freeze: false, }), diff --git a/esign/templates/esign.html b/esign/templates/esign.html index 354fd07..272d613 100644 --- a/esign/templates/esign.html +++ b/esign/templates/esign.html @@ -18,6 +18,7 @@ doctype: {{ doctype | tojson }}, name: {{ name | tojson }}, key: {{ key | tojson }}, + web_form: {{ esign_web_form | tojson }}, print_format: {{ print_format | tojson }}, signed_pdf_url: {{ signed_pdf_url | default('') | tojson }} }; diff --git a/esign/tests/test_web_form.py b/esign/tests/test_web_form.py index b152b06..bd1af32 100644 --- a/esign/tests/test_web_form.py +++ b/esign/tests/test_web_form.py @@ -215,6 +215,7 @@ def test_guest_with_key_gets_the_signing_page(self): self.assertIn("esign-layout", content) self.assertIn(f'key: "{self.key}"', content) + self.assertIn(f'web_form: "{self.web_form_name}"', content) self.assertIn('print_format: "Standard"', content) self.assertIn("signer@example.com", content) self.assertIn("esign.web.bundle.js", content) @@ -255,6 +256,7 @@ def test_completed_document_links_the_signed_pdf(self): self.assertIn("esign-completed", content) self.assertIn("esign.esign.custom.web_form.download_signed_pdf", content) self.assertIn(f"docname={self.doc.name}", content) + self.assertIn(f"web_form={self.web_form_name}", content) def test_force_mobile_adds_a_qr_code(self): self.web_form.db_set("esign_force_mobile", 1) @@ -332,6 +334,41 @@ def test_user_with_print_permission_needs_no_key(self): result = get_print_html(TEST_DOCTYPE, self.doc.name) self.assertIn("Service agreement", result["print_html"]) + def test_owner_without_key_gets_the_print_html_through_the_web_form(self): + # The keyless owner the signing page admits must also get its preview. + self.doc.db_set("owner", OUTSIDER) + frappe.set_user(OUTSIDER) + result = get_print_html(TEST_DOCTYPE, self.doc.name, web_form=self.web_form_name) + self.assertIn("Service agreement", result["print_html"]) + + # Without the web form, the owner has no print permission. + frappe.flags.ignore_print_permissions = False + with self.assertRaises(frappe.PermissionError): + get_print_html(TEST_DOCTYPE, self.doc.name) + + def test_web_form_does_not_admit_other_users(self): + frappe.set_user(OUTSIDER) + with self.assertRaises(frappe.PermissionError): + get_print_html(TEST_DOCTYPE, self.doc.name, web_form=self.web_form_name) + + def test_only_an_esign_web_form_for_the_doctype_admits(self): + self.doc.db_set("owner", OUTSIDER) + frappe.set_user(OUTSIDER) + with self.assertRaises(frappe.PermissionError): + get_print_html(TEST_DOCTYPE, self.doc.name, web_form="no-such-web-form") + + frappe.set_user("Administrator") + self.web_form.db_set("esign_enabled", 0) + frappe.clear_document_cache("Web Form", self.web_form_name) + frappe.set_user(OUTSIDER) + with self.assertRaises(frappe.PermissionError): + get_print_html(TEST_DOCTYPE, self.doc.name, web_form=self.web_form_name) + + def test_guest_gets_nothing_from_the_web_form(self): + frappe.set_user("Guest") + with self.assertRaises(frappe.PermissionError): + get_print_html(TEST_DOCTYPE, self.doc.name, web_form=self.web_form_name) + class TestDownloadSignedPdf(EsignTestCase): def attach_signed_pdf(self, **kwargs: Any): @@ -364,6 +401,23 @@ def test_user_without_key_needs_print_permission(self): download_signed_pdf(TEST_DOCTYPE, self.doc.name) self.assertEqual(frappe.local.response["type"], "redirect") + def test_owner_without_key_downloads_through_the_web_form(self): + file = self.attach_signed_pdf(is_private=0) + self.doc.db_set("owner", OUTSIDER) + frappe.set_user(OUTSIDER) + with signing_request(method="GET"), self.assertRaises(frappe.PermissionError): + download_signed_pdf(TEST_DOCTYPE, self.doc.name) + with signing_request(method="GET"): + download_signed_pdf(TEST_DOCTYPE, self.doc.name, web_form=self.web_form_name) + self.assertEqual(frappe.local.response["type"], "redirect") + self.assertEqual(frappe.local.response["location"], file.file_url) + + def test_guest_needs_a_key_even_with_the_web_form(self): + self.attach_signed_pdf() + frappe.set_user("Guest") + with signing_request(method="GET"), self.assertRaises(frappe.PermissionError): + download_signed_pdf(TEST_DOCTYPE, self.doc.name, web_form=self.web_form_name) + def test_no_signed_pdf(self): with signing_request(method="GET"), self.assertRaises(frappe.DoesNotExistError): download_signed_pdf(TEST_DOCTYPE, self.doc.name, key=self.key) diff --git a/types/esign.augment.d.ts b/types/esign.augment.d.ts index ccfcd05..9f06cf8 100644 --- a/types/esign.augment.d.ts +++ b/types/esign.augment.d.ts @@ -11,6 +11,7 @@ interface EsignContext { doctype: string; name: string; key: string; + web_form: string; print_format: string; signed_pdf_url: string; } From 623a73afb30b61bd2a0ebc85d9a73297b3a764bf Mon Sep 17 00:00:00 2001 From: Kevin Shenk Date: Sat, 17 Oct 2026 00:53:39 -0400 Subject: [PATCH 14/14] style: define _get_esign_web_form before its first use ssort's order in a clean checkout puts the helper above validate_esign_access, which calls it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SvdEeMiewirZ3ifFoUix5F --- esign/esign/custom/web_form.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/esign/esign/custom/web_form.py b/esign/esign/custom/web_form.py index d21104d..9cf653b 100644 --- a/esign/esign/custom/web_form.py +++ b/esign/esign/custom/web_form.py @@ -359,6 +359,17 @@ def has_web_form_permission(self, doctype, name, ptype="read"): return super().has_web_form_permission(doctype, name, ptype) +def _get_esign_web_form(web_form: EsignWebForm | str, doctype: str) -> EsignWebForm | None: + """The eSign-enabled web form for doctype, or None if web_form is not one.""" + if isinstance(web_form, str): + if not frappe.db.exists("Web Form", web_form): + return None + web_form = cast(EsignWebForm, frappe.get_cached_doc("Web Form", web_form)) + if not web_form.get("esign_enabled") or web_form.get("doc_type") != doctype: + return None + return web_form + + def validate_esign_access( key: str, doc: Document, web_form: EsignWebForm | str | None = None, require_key: bool = False ) -> bool: @@ -384,17 +395,6 @@ def validate_esign_access( return validate_esign_key(key, doc, require_key=require_key) -def _get_esign_web_form(web_form: EsignWebForm | str, doctype: str) -> EsignWebForm | None: - """The eSign-enabled web form for doctype, or None if web_form is not one.""" - if isinstance(web_form, str): - if not frappe.db.exists("Web Form", web_form): - return None - web_form = cast(EsignWebForm, frappe.get_cached_doc("Web Form", web_form)) - if not web_form.get("esign_enabled") or web_form.get("doc_type") != doctype: - return None - return web_form - - _UNSET = object()