From f09cd01b7598c8e5fec1b9bb5b649274b88ecd35 Mon Sep 17 00:00:00 2001 From: HenningWendtland <156231187+HenningWendtland@users.noreply.github.com> Date: Mon, 7 Sep 2026 14:20:36 +0200 Subject: [PATCH 1/7] feat(Simple Subscription): optionally add additional customer info --- simple_subscription/locale/de.po | 65 ++++++++++++-- simple_subscription/locale/main.pot | 65 ++++++++++++-- .../simple_subscription.js | 45 ++++++++++ .../simple_subscription.json | 60 ++++++++++++- .../simple_subscription.py | 54 ++++++++++++ .../test_simple_subscription.py | 86 +++++++++++++++++++ 6 files changed, 358 insertions(+), 17 deletions(-) diff --git a/simple_subscription/locale/de.po b/simple_subscription/locale/de.po index 5872677..6a7ca67 100644 --- a/simple_subscription/locale/de.po +++ b/simple_subscription/locale/de.po @@ -28,6 +28,12 @@ msgstr "" msgid "Accounts User" msgstr "" +#. Label of the additional_customer_data_section (Section Break) field in +#. DocType 'Simple Subscription' +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json +msgid "Additional Customer Data" +msgstr "Zusätzliche Kundendaten" + #. Label of the amended_from (Link) field in DocType 'Simple Subscription' #: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json msgid "Amended From" @@ -38,6 +44,23 @@ msgstr "" msgid "Biennial" msgstr "Zweijährlich" +#. Label of the billing_address_display (Text Editor) field in DocType +#. 'Simple Subscription' +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json +msgid "Billing Address" +msgstr "" + +#. Label of the customer_address (Link) field in DocType 'Simple +#. Subscription' +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json +msgid "Billing Address Name" +msgstr "" + +#. Label of the contact_person (Link) field in DocType 'Simple Subscription' +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json +msgid "Billing Contact Name" +msgstr "Name des Rechnungskontakts" + #. Label of the period_type (Select) field in DocType 'Simple Subscription' #: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json msgid "Billing Period is based on" @@ -53,7 +76,13 @@ msgstr "Rechnungszeit" msgid "Company" msgstr "Unternehmen" -#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.js:28 +#. Label of the contact_display (Small Text) field in DocType 'Simple +#. Subscription' +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json +msgid "Contact" +msgstr "" + +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.js:73 msgid "Create current {0} invoice" msgstr "Aktuelle {0} Rechnung erstellen" @@ -89,7 +118,7 @@ msgstr "" msgid "Disabled" msgstr "" -#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:135 +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:195 msgid "Failed to create subscription invoice" msgstr "Fehler beim Erstellen der Abo-Rechnung" @@ -129,15 +158,21 @@ msgstr "" msgid "Monthly" msgstr "Monatlich" -#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:57 +#. Description of the 'Additional Customer Data' (Section Break) field in +#. DocType 'Simple Subscription' +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json +msgid "Optional. If empty, defaults are set on invoice generation" +msgstr "Optional. Wenn leer, werden bei der Rechnungserstellung die Standardwerte gesetzt." + +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:107 msgid "Please amend Subscription {0} before creating a Sales Invoice." msgstr "Bitte ändern Sie das Abo {0} bevor Sie eine Rechnung erstellen." -#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:51 +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:101 msgid "Please enable Subscription {0} before creating a Sales Invoice." msgstr "Bitte aktivieren Sie das Abo {0} bevor Sie eine Rechnung erstellen." -#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:54 +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:104 msgid "Please submit Subscription {0} before creating a Sales Invoice." msgstr "Bitte buchen Sie das Abo {0} bevor Sie eine Rechnung erstellen." @@ -151,7 +186,7 @@ msgstr "" msgid "Quarterly" msgstr "Vierteljährlich" -#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:124 +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:184 msgid "Sales Invoice already exists for this period: {}" msgstr "Rechnung für diese Periode existiert bereits: {}" @@ -160,6 +195,18 @@ msgstr "Rechnung für diese Periode existiert bereits: {}" msgid "Selling" msgstr "" +#. Label of the shipping_address_display (Text Editor) field in DocType +#. 'Simple Subscription' +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json +msgid "Shipping Address" +msgstr "" + +#. Label of the shipping_address_name (Link) field in DocType 'Simple +#. Subscription' +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json +msgid "Shipping Address Name" +msgstr "" + #. Name of a DocType #. Label of a Link in the Simple Subscription Selling Workspace #. Label of a shortcut in the Simple Subscription Selling Workspace @@ -184,8 +231,7 @@ msgstr "Vertrieb" msgid "Start Date" msgstr "Startdatum" -#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:209 -#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:221 +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:271 msgid "Start Date is required for frequency {0}." msgstr "Startdatum ist für die Frequenz {0} erforderlich." @@ -233,3 +279,6 @@ msgstr "Kalendermonate" msgid "start date" msgstr "Startdatum" +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:90 +msgid "{0} {1} does not belong to Customer {2}." +msgstr "{0} {1} gehört nicht zu Kunde {2}." diff --git a/simple_subscription/locale/main.pot b/simple_subscription/locale/main.pot index a8c2ac1..3d6e406 100644 --- a/simple_subscription/locale/main.pot +++ b/simple_subscription/locale/main.pot @@ -26,6 +26,12 @@ msgstr "" msgid "Accounts User" msgstr "" +#. Label of the additional_customer_data_section (Section Break) field in +#. DocType 'Simple Subscription' +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json +msgid "Additional Customer Data" +msgstr "" + #. Label of the amended_from (Link) field in DocType 'Simple Subscription' #: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json msgid "Amended From" @@ -36,6 +42,23 @@ msgstr "" msgid "Biennial" msgstr "" +#. Label of the billing_address_display (Text Editor) field in DocType +#. 'Simple Subscription' +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json +msgid "Billing Address" +msgstr "" + +#. Label of the customer_address (Link) field in DocType 'Simple +#. Subscription' +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json +msgid "Billing Address Name" +msgstr "" + +#. Label of the contact_person (Link) field in DocType 'Simple Subscription' +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json +msgid "Billing Contact Name" +msgstr "" + #. Label of the period_type (Select) field in DocType 'Simple Subscription' #: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json msgid "Billing Period is based on" @@ -51,7 +74,13 @@ msgstr "" msgid "Company" msgstr "" -#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.js:28 +#. Label of the contact_display (Small Text) field in DocType 'Simple +#. Subscription' +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json +msgid "Contact" +msgstr "" + +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.js:73 msgid "Create current {0} invoice" msgstr "" @@ -87,7 +116,7 @@ msgstr "" msgid "Disabled" msgstr "" -#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:135 +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:195 msgid "Failed to create subscription invoice" msgstr "" @@ -127,15 +156,21 @@ msgstr "" msgid "Monthly" msgstr "" -#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:57 +#. Description of the 'Additional Customer Data' (Section Break) field in +#. DocType 'Simple Subscription' +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json +msgid "Optional. If empty, defaults are set on invoice generation" +msgstr "" + +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:107 msgid "Please amend Subscription {0} before creating a Sales Invoice." msgstr "" -#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:51 +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:101 msgid "Please enable Subscription {0} before creating a Sales Invoice." msgstr "" -#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:54 +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:104 msgid "Please submit Subscription {0} before creating a Sales Invoice." msgstr "" @@ -149,7 +184,7 @@ msgstr "" msgid "Quarterly" msgstr "" -#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:124 +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:184 msgid "Sales Invoice already exists for this period: {}" msgstr "" @@ -158,6 +193,18 @@ msgstr "" msgid "Selling" msgstr "" +#. Label of the shipping_address_display (Text Editor) field in DocType +#. 'Simple Subscription' +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json +msgid "Shipping Address" +msgstr "" + +#. Label of the shipping_address_name (Link) field in DocType 'Simple +#. Subscription' +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json +msgid "Shipping Address Name" +msgstr "" + #. Name of a DocType #. Label of a Link in the Simple Subscription Selling Workspace #. Label of a shortcut in the Simple Subscription Selling Workspace @@ -182,8 +229,7 @@ msgstr "" msgid "Start Date" msgstr "" -#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:209 -#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:221 +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:271 msgid "Start Date is required for frequency {0}." msgstr "" @@ -231,3 +277,6 @@ msgstr "" msgid "start date" msgstr "" +#: simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py:90 +msgid "{0} {1} does not belong to Customer {2}." +msgstr "" diff --git a/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.js b/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.js index 9f89807..d76df45 100644 --- a/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.js +++ b/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.js @@ -1,8 +1,25 @@ // Copyright (c) 2022, ALYF GmbH and contributors // For license information, please see license.txt +function customer_link_query(frm, query) { + return { + query: query, + filters: { link_doctype: "Customer", link_name: frm.doc.customer }, + }; +} + frappe.ui.form.on("Simple Subscription", { setup: function (frm) { + for (const field of ["customer_address", "shipping_address_name"]) { + frm.set_query(field, () => + customer_link_query(frm, "frappe.contacts.doctype.address.address.address_query") + ); + } + + frm.set_query("contact_person", () => + customer_link_query(frm, "frappe.contacts.doctype.contact.contact.contact_query") + ); + frm.set_query("item", "items", function () { return { filters: { @@ -21,6 +38,34 @@ frappe.ui.form.on("Simple Subscription", { }); }, + customer: function (frm) { + // address and contact are customer specific, don't keep the previous customer's + for (const field of ["customer_address", "shipping_address_name", "contact_person"]) { + frm.set_value(field, null); + } + }, + + customer_address: function (frm) { + erpnext.utils.get_address_display(frm, "customer_address", "billing_address_display"); + }, + + shipping_address_name: function (frm) { + erpnext.utils.get_address_display(frm, "shipping_address_name", "shipping_address_display"); + }, + + contact_person: function (frm) { + if (!frm.doc.contact_person) { + frm.set_value("contact_display", ""); + return; + } + + frappe.call({ + method: "frappe.contacts.doctype.contact.contact.get_contact_details", + args: { contact: frm.doc.contact_person }, + callback: (r) => r.message && frm.set_value("contact_display", r.message.contact_display), + }); + }, + refresh: function (frm) { if (frm.doc.docstatus !== 1 || frm.doc.disabled === 1) return; diff --git a/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json b/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json index 4c0d406..8c06744 100644 --- a/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json +++ b/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json @@ -17,6 +17,14 @@ "billing_time", "frequency", "disabled", + "additional_customer_data_section", + "customer_address", + "billing_address_display", + "shipping_address_name", + "shipping_address_display", + "column_break_beul", + "contact_person", + "contact_display", "section_break_5", "items", "section_break_7", @@ -125,6 +133,56 @@ "label": "Currency", "options": "Currency", "reqd": 1 + }, + { + "collapsible": 1, + "description": "Optional. If empty, defaults are set on invoice generation", + "fieldname": "additional_customer_data_section", + "fieldtype": "Section Break", + "label": "Additional Customer Data" + }, + { + "fieldname": "customer_address", + "fieldtype": "Link", + "label": "Billing Address Name", + "options": "Address" + }, + { + "fieldname": "billing_address_display", + "fieldtype": "Text Editor", + "is_virtual": 1, + "label": "Billing Address", + "read_only": 1 + }, + { + "fieldname": "shipping_address_name", + "fieldtype": "Link", + "label": "Shipping Address Name", + "options": "Address" + }, + { + "fieldname": "shipping_address_display", + "fieldtype": "Text Editor", + "is_virtual": 1, + "label": "Shipping Address", + "read_only": 1 + }, + { + "fieldname": "column_break_beul", + "fieldtype": "Column Break" + }, + { + "fieldname": "contact_person", + "fieldtype": "Link", + "label": "Billing Contact Name", + "options": "Contact" + }, + { + "fieldname": "contact_display", + "fieldtype": "Small Text", + "is_virtual": 1, + "label": "Contact", + "read_only": 1 } ], "is_submittable": 1, @@ -134,7 +192,7 @@ "link_fieldname": "simple_subscription" } ], - "modified": "2026-06-15 11:53:49.870013", + "modified": "2026-09-07 12:41:55.374840", "modified_by": "Administrator", "module": "Simple Subscription", "name": "Simple Subscription", diff --git a/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py b/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py index 8571c7f..acac47d 100644 --- a/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py +++ b/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py @@ -8,6 +8,8 @@ from erpnext.accounts.doctype.sales_invoice.sales_invoice import SalesInvoice from erpnext.accounts.party import get_party_details from frappe import _ +from frappe.contacts.doctype.address.address import get_address_display +from frappe.contacts.doctype.contact.contact import get_contact_details from frappe.model.document import Document from frappe.utils import today @@ -41,9 +43,56 @@ class BillingTime(Enum): AfterEndOfPeriod = "after end of period" +# forwarded to the Sales Invoice, which uses the same fieldnames +CUSTOMER_LINK_FIELDS: dict[str, str] = { + "customer_address": "Address", + "shipping_address_name": "Address", + "contact_person": "Contact", +} + + class SimpleSubscription(Document): + @property + def billing_address_display(self) -> str | None: + return get_address_display(self.customer_address) + + @property + def shipping_address_display(self) -> str | None: + return get_address_display(self.shipping_address_name) + + @property + def contact_display(self) -> str | None: + if not self.contact_person: + return None + return frappe.db.get_value("Contact", self.contact_person, "full_name") + def validate(self): validate_calendar_frequencies(self.period_type, self.frequency, self.start_date) + self.validate_customer_links() + + def validate_customer_links(self) -> None: + """The .js queries filter by customer, but the customer can change afterwards.""" + for fieldname, doctype in CUSTOMER_LINK_FIELDS.items(): + name = self.get(fieldname) + if not name: + continue + + if not frappe.db.exists( + "Dynamic Link", + { + "parenttype": doctype, + "parent": name, + "link_doctype": "Customer", + "link_name": self.customer, + }, + ): + frappe.throw( + _("{0} {1} does not belong to Customer {2}.").format( + _(self.meta.get_label(fieldname)), + frappe.bold(name), + frappe.bold(self.customer), + ) + ) def create_invoice(self, from_date: date, to_date: date) -> SalesInvoice: msg = None @@ -62,6 +111,8 @@ def create_invoice(self, from_date: date, to_date: date) -> SalesInvoice: invoice = frappe.new_doc("Sales Invoice") invoice.company = self.company invoice.customer = self.customer + invoice.customer_address = self.customer_address + invoice.shipping_address_name = self.shipping_address_name invoice.selling_price_list = self.get_price_list() for row in self.items: invoice.append( @@ -76,6 +127,9 @@ def create_invoice(self, from_date: date, to_date: date) -> SalesInvoice: invoice.to_date = to_date invoice.simple_subscription = self.name invoice.set_missing_values() + if self.contact_person: + # set_missing_values() would have filled in the customer's primary contact instead + invoice.update(get_contact_details(self.contact_person)) return invoice.insert() def get_price_list(self) -> str | None: diff --git a/simple_subscription/simple_subscription/doctype/simple_subscription/test_simple_subscription.py b/simple_subscription/simple_subscription/doctype/simple_subscription/test_simple_subscription.py index efa1d60..d3e670a 100644 --- a/simple_subscription/simple_subscription/doctype/simple_subscription/test_simple_subscription.py +++ b/simple_subscription/simple_subscription/doctype/simple_subscription/test_simple_subscription.py @@ -5,6 +5,7 @@ from datetime import date import frappe +from frappe.tests import IntegrationTestCase from .simple_subscription import ( BillingTime, @@ -196,3 +197,88 @@ def test_get_from_and_to_date(self): ) self.assertEqual(from_date, date(2020, 1, 1)) self.assertEqual(to_date, date(2021, 12, 31)) + + +class TestSubscriptionCustomerData(IntegrationTestCase): + """Address/Contact prefill: customer-bound validation and forwarding to the invoice.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.customer = frappe.get_doc(doctype="Customer", customer_name="_Test Sub Customer").insert() + cls.other_customer = frappe.get_doc( + doctype="Customer", customer_name="_Test Sub Other Customer" + ).insert() + cls.address = make_address("_Test Sub Billing", cls.customer.name, "Rechnungsweg 1") + cls.shipping_address = make_address("_Test Sub Shipping", cls.customer.name, "Lieferweg 2") + cls.other_address = make_address("_Test Sub Foreign", cls.other_customer.name, "Fremdweg 3") + cls.contact = make_contact("_Test Sub Contact", cls.customer.name) + + def make_subscription(self, **kwargs): + return frappe.get_doc( + doctype="Simple Subscription", + company="_Test Company", + customer=self.customer.name, + start_date="2024-01-01", + frequency="Yearly", + items=[{"item": "_Test Item", "qty": 1}], + **kwargs, + ) + + def test_rejects_address_of_another_customer(self): + subscription = self.make_subscription(customer_address=self.other_address) + self.assertRaises(frappe.ValidationError, subscription.insert) + + def test_virtual_fields_render_linked_records(self): + subscription = self.make_subscription( + customer_address=self.address, + shipping_address_name=self.shipping_address, + contact_person=self.contact, + ).insert() + + self.assertIn("Rechnungsweg 1", subscription.billing_address_display) + self.assertIn("Lieferweg 2", subscription.shipping_address_display) + self.assertEqual(subscription.contact_display, "_Test Sub Contact") + + def test_forwards_links_to_invoice(self): + subscription = self.make_subscription( + customer_address=self.address, + shipping_address_name=self.shipping_address, + contact_person=self.contact, + ).insert() + subscription.submit() + + invoice = subscription.create_invoice(date(2024, 1, 1), date(2024, 12, 31)) + + self.assertIn(invoice.customer_address, (self.address, self.shipping_address)) + self.assertEqual(invoice.shipping_address_name, self.shipping_address) + self.assertEqual(invoice.contact_person, self.contact) + self.assertEqual(invoice.contact_display, "_Test Sub Contact") + + def test_invoice_falls_back_to_customer_defaults(self): + subscription = self.make_subscription().insert() + subscription.submit() + + invoice = subscription.create_invoice(date(2024, 1, 1), date(2024, 12, 31)) + + self.assertIn(invoice.customer_address, (self.address, self.shipping_address)) + + +def make_address(title: str, customer: str, line1: str) -> str: + return frappe.get_doc( + doctype="Address", + address_title=title, + address_type="Billing", + address_line1=line1, + city="Berlin", + country="Germany", + links=[{"link_doctype": "Customer", "link_name": customer}], + ).insert().name + + +def make_contact(first_name: str, customer: str) -> str: + return frappe.get_doc( + doctype="Contact", + first_name=first_name, + links=[{"link_doctype": "Customer", "link_name": customer}], + ).insert().name From 87fb4b095320762bd2fa1f44c7048e21cc340647 Mon Sep 17 00:00:00 2001 From: HenningWendtland <156231187+HenningWendtland@users.noreply.github.com> Date: Mon, 7 Sep 2026 14:26:00 +0200 Subject: [PATCH 2/7] fix: set allow_on_submit for new link fields --- .../doctype/simple_subscription/simple_subscription.json | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json b/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json index 8c06744..c089041 100644 --- a/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json +++ b/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.json @@ -142,6 +142,7 @@ "label": "Additional Customer Data" }, { + "allow_on_submit": 1, "fieldname": "customer_address", "fieldtype": "Link", "label": "Billing Address Name", @@ -155,6 +156,7 @@ "read_only": 1 }, { + "allow_on_submit": 1, "fieldname": "shipping_address_name", "fieldtype": "Link", "label": "Shipping Address Name", @@ -172,6 +174,7 @@ "fieldtype": "Column Break" }, { + "allow_on_submit": 1, "fieldname": "contact_person", "fieldtype": "Link", "label": "Billing Contact Name", @@ -192,7 +195,7 @@ "link_fieldname": "simple_subscription" } ], - "modified": "2026-09-07 12:41:55.374840", + "modified": "2026-09-07 14:24:50.615780", "modified_by": "Administrator", "module": "Simple Subscription", "name": "Simple Subscription", From b75054ad92147b9c8b4c32add369b7296940b946 Mon Sep 17 00:00:00 2001 From: HenningWendtland <156231187+HenningWendtland@users.noreply.github.com> Date: Mon, 7 Sep 2026 17:25:32 +0200 Subject: [PATCH 3/7] fix: add on_update_after_submit validation --- .../doctype/simple_subscription/simple_subscription.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py b/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py index acac47d..8682130 100644 --- a/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py +++ b/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.py @@ -70,6 +70,9 @@ def validate(self): validate_calendar_frequencies(self.period_type, self.frequency, self.start_date) self.validate_customer_links() + def before_update_after_submit(self): + self.validate_customer_links() + def validate_customer_links(self) -> None: """The .js queries filter by customer, but the customer can change afterwards.""" for fieldname, doctype in CUSTOMER_LINK_FIELDS.items(): From 5494f0a91b5ff9cc1e4a19cc556c2b6f9feb4629 Mon Sep 17 00:00:00 2001 From: HenningWendtland <156231187+HenningWendtland@users.noreply.github.com> Date: Mon, 7 Sep 2026 17:26:07 +0200 Subject: [PATCH 4/7] fix: make tests not depend on erpnext --- .../test_simple_subscription.py | 146 ++++++++++++++---- 1 file changed, 112 insertions(+), 34 deletions(-) diff --git a/simple_subscription/simple_subscription/doctype/simple_subscription/test_simple_subscription.py b/simple_subscription/simple_subscription/doctype/simple_subscription/test_simple_subscription.py index d3e670a..54be686 100644 --- a/simple_subscription/simple_subscription/doctype/simple_subscription/test_simple_subscription.py +++ b/simple_subscription/simple_subscription/doctype/simple_subscription/test_simple_subscription.py @@ -5,7 +5,6 @@ from datetime import date import frappe -from frappe.tests import IntegrationTestCase from .simple_subscription import ( BillingTime, @@ -199,35 +198,63 @@ def test_get_from_and_to_date(self): self.assertEqual(to_date, date(2021, 12, 31)) -class TestSubscriptionCustomerData(IntegrationTestCase): - """Address/Contact prefill: customer-bound validation and forwarding to the invoice.""" +class TestSubscriptionCustomerData(unittest.TestCase): + """Address/Contact prefill: customer-bound validation and forwarding to the invoice. + + Deliberately a plain TestCase: frappe.tests.IntegrationTestCase would resolve the test + record dependencies of Simple Subscription, which pulls in ERPNext's whole object graph + (including every Company test record) and breaks on a fresh site. Everything needed here + is created below instead. + """ @classmethod def setUpClass(cls): super().setUpClass() - cls.customer = frappe.get_doc(doctype="Customer", customer_name="_Test Sub Customer").insert() - cls.other_customer = frappe.get_doc( - doctype="Customer", customer_name="_Test Sub Other Customer" - ).insert() - cls.address = make_address("_Test Sub Billing", cls.customer.name, "Rechnungsweg 1") - cls.shipping_address = make_address("_Test Sub Shipping", cls.customer.name, "Lieferweg 2") - cls.other_address = make_address("_Test Sub Foreign", cls.other_customer.name, "Fremdweg 3") - cls.contact = make_contact("_Test Sub Contact", cls.customer.name) + cls.currency = frappe.defaults.get_global_default("currency") + cls.company = make_company(cls.currency) + cls.item = make_item() + cls.customer = make_customer("_Test Sub Customer") + cls.other_customer = make_customer("_Test Sub Other Customer") + cls.address = make_address("_Test Sub Billing", cls.customer, "Rechnungsweg 1", primary=True) + cls.shipping_address = make_address("_Test Sub Shipping", cls.customer, "Lieferweg 2", shipping=True) + cls.other_address = make_address("_Test Sub Foreign", cls.other_customer, "Fremdweg 3") + cls.contact = make_contact("_Test Sub Contact", cls.customer) + cls.other_contact = make_contact("_Test Sub Foreign Contact", cls.other_customer) + frappe.db.commit() + + def tearDown(self): + frappe.db.rollback() def make_subscription(self, **kwargs): return frappe.get_doc( doctype="Simple Subscription", - company="_Test Company", - customer=self.customer.name, + company=self.company, + customer=self.customer, + currency=self.currency, start_date="2024-01-01", frequency="Yearly", - items=[{"item": "_Test Item", "qty": 1}], + items=[{"item": self.item, "qty": 1}], **kwargs, ) - def test_rejects_address_of_another_customer(self): - subscription = self.make_subscription(customer_address=self.other_address) - self.assertRaises(frappe.ValidationError, subscription.insert) + def test_rejects_links_of_another_customer(self): + for fieldname, value in ( + ("customer_address", self.other_address), + ("shipping_address_name", self.other_address), + ("contact_person", self.other_contact), + ): + with self.subTest(fieldname=fieldname): + subscription = self.make_subscription(**{fieldname: value}) + self.assertRaises(frappe.ValidationError, subscription.insert) + + def test_rejects_links_still_stale_after_submit(self): + """The links are allow_on_submit, so validate() no longer guards them.""" + subscription = self.make_subscription(customer_address=self.address).insert() + subscription.submit() + + subscription.customer_address = self.other_address + # must be the ownership check, not the generic "cannot change after submit" guard + self.assertRaisesRegex(frappe.ValidationError, "does not belong to Customer", subscription.save) def test_virtual_fields_render_linked_records(self): subscription = self.make_subscription( @@ -240,6 +267,13 @@ def test_virtual_fields_render_linked_records(self): self.assertIn("Lieferweg 2", subscription.shipping_address_display) self.assertEqual(subscription.contact_display, "_Test Sub Contact") + def test_virtual_fields_are_empty_without_links(self): + subscription = self.make_subscription().insert() + + self.assertIsNone(subscription.billing_address_display) + self.assertIsNone(subscription.shipping_address_display) + self.assertIsNone(subscription.contact_display) + def test_forwards_links_to_invoice(self): subscription = self.make_subscription( customer_address=self.address, @@ -250,9 +284,10 @@ def test_forwards_links_to_invoice(self): invoice = subscription.create_invoice(date(2024, 1, 1), date(2024, 12, 31)) - self.assertIn(invoice.customer_address, (self.address, self.shipping_address)) + self.assertEqual(invoice.customer_address, self.address) self.assertEqual(invoice.shipping_address_name, self.shipping_address) self.assertEqual(invoice.contact_person, self.contact) + # the whole contact block must come from the chosen contact, not the customer default self.assertEqual(invoice.contact_display, "_Test Sub Contact") def test_invoice_falls_back_to_customer_defaults(self): @@ -261,24 +296,67 @@ def test_invoice_falls_back_to_customer_defaults(self): invoice = subscription.create_invoice(date(2024, 1, 1), date(2024, 12, 31)) - self.assertIn(invoice.customer_address, (self.address, self.shipping_address)) + self.assertEqual(invoice.customer_address, self.address) + self.assertEqual(invoice.shipping_address_name, self.shipping_address) -def make_address(title: str, customer: str, line1: str) -> str: - return frappe.get_doc( - doctype="Address", - address_title=title, - address_type="Billing", - address_line1=line1, - city="Berlin", - country="Germany", - links=[{"link_doctype": "Customer", "link_name": customer}], - ).insert().name +def make_company(currency: str) -> str: + """A company in the site's own currency, so the invoice does not trip the party-account check.""" + name = "_Test Sub Company" + if not frappe.db.exists("Company", name): + frappe.get_doc( + doctype="Company", + company_name=name, + abbr="_TSC", + default_currency=currency, + country="Germany", + ).insert() + return name + + +def make_item() -> str: + name = "_Test Sub Item" + if not frappe.db.exists("Item", name): + frappe.get_doc( + doctype="Item", + item_code=name, + item_group="All Item Groups", + stock_uom="Nos", + is_stock_item=0, + is_sales_item=1, + ).insert() + return name + + +def make_customer(name: str) -> str: + if not frappe.db.exists("Customer", name): + frappe.get_doc(doctype="Customer", customer_name=name).insert() + return name + + +def make_address(title: str, customer: str, line1: str, primary=False, shipping=False) -> str: + name = f"{title}-Billing" + if not frappe.db.exists("Address", name): + frappe.get_doc( + doctype="Address", + address_title=title, + address_type="Billing", + address_line1=line1, + city="Berlin", + country="Germany", + is_primary_address=int(primary), + is_shipping_address=int(shipping), + links=[{"link_doctype": "Customer", "link_name": customer}], + ).insert() + return name def make_contact(first_name: str, customer: str) -> str: - return frappe.get_doc( - doctype="Contact", - first_name=first_name, - links=[{"link_doctype": "Customer", "link_name": customer}], - ).insert().name + name = f"{first_name}-{customer}" + if not frappe.db.exists("Contact", name): + frappe.get_doc( + doctype="Contact", + first_name=first_name, + links=[{"link_doctype": "Customer", "link_name": customer}], + ).insert() + return name From 26ad94e32829c3e33e0b5548cc5655ad915f8ce8 Mon Sep 17 00:00:00 2001 From: HenningWendtland <156231187+HenningWendtland@users.noreply.github.com> Date: Mon, 7 Sep 2026 17:27:44 +0200 Subject: [PATCH 5/7] fix: catch the edge case where the contact changes, while the contact_display is fetched --- .../simple_subscription/simple_subscription.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.js b/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.js index d76df45..e7603db 100644 --- a/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.js +++ b/simple_subscription/simple_subscription/doctype/simple_subscription/simple_subscription.js @@ -54,15 +54,20 @@ frappe.ui.form.on("Simple Subscription", { }, contact_person: function (frm) { - if (!frm.doc.contact_person) { + const contact = frm.doc.contact_person; + if (!contact) { frm.set_value("contact_display", ""); return; } frappe.call({ method: "frappe.contacts.doctype.contact.contact.get_contact_details", - args: { contact: frm.doc.contact_person }, - callback: (r) => r.message && frm.set_value("contact_display", r.message.contact_display), + args: { contact }, + callback: (r) => + // the selection may have moved on while the call was in flight + r.message && + frm.doc.contact_person === contact && + frm.set_value("contact_display", r.message.contact_display), }); }, From 141172db4fcde76c2b522d43b8467587b2398c75 Mon Sep 17 00:00:00 2001 From: HenningWendtland <156231187+HenningWendtland@users.noreply.github.com> Date: Tue, 8 Sep 2026 12:52:24 +0200 Subject: [PATCH 6/7] fix: simplify test cases --- .../test_simple_subscription.py | 153 +++++++----------- 1 file changed, 60 insertions(+), 93 deletions(-) diff --git a/simple_subscription/simple_subscription/doctype/simple_subscription/test_simple_subscription.py b/simple_subscription/simple_subscription/doctype/simple_subscription/test_simple_subscription.py index 54be686..2890c99 100644 --- a/simple_subscription/simple_subscription/doctype/simple_subscription/test_simple_subscription.py +++ b/simple_subscription/simple_subscription/doctype/simple_subscription/test_simple_subscription.py @@ -10,6 +10,7 @@ BillingTime, Frequency, PeriodType, + SimpleSubscription, get_calendar_period, get_date_period, get_from_and_to_date, @@ -199,43 +200,56 @@ def test_get_from_and_to_date(self): class TestSubscriptionCustomerData(unittest.TestCase): - """Address/Contact prefill: customer-bound validation and forwarding to the invoice. + """Customer-bound Address/Contact links: the ownership check and the display properties. - Deliberately a plain TestCase: frappe.tests.IntegrationTestCase would resolve the test - record dependencies of Simple Subscription, which pulls in ERPNext's whole object graph - (including every Company test record) and breaks on a fresh site. Everything needed here - is created below instead. + Kept deliberately light. Anything that inserts a Simple Subscription needs a Company and an + Item, and anything that generates the invoice needs a fully set-up ERPNext site -- fixtures + that only the setup wizard creates and that CI (a bare `install-app erpnext`) does not have. + So these exercise the app's own logic on an in-memory document; that the invoice picks the + links up is ERPNext's `_get_party_details` doing its normal job. """ @classmethod def setUpClass(cls): super().setUpClass() - cls.currency = frappe.defaults.get_global_default("currency") - cls.company = make_company(cls.currency) - cls.item = make_item() cls.customer = make_customer("_Test Sub Customer") cls.other_customer = make_customer("_Test Sub Other Customer") - cls.address = make_address("_Test Sub Billing", cls.customer, "Rechnungsweg 1", primary=True) - cls.shipping_address = make_address("_Test Sub Shipping", cls.customer, "Lieferweg 2", shipping=True) + cls.address = make_address("_Test Sub Billing", cls.customer, "Rechnungsweg 1") cls.other_address = make_address("_Test Sub Foreign", cls.other_customer, "Fremdweg 3") cls.contact = make_contact("_Test Sub Contact", cls.customer) cls.other_contact = make_contact("_Test Sub Foreign Contact", cls.other_customer) frappe.db.commit() - def tearDown(self): - frappe.db.rollback() + @classmethod + def tearDownClass(cls): + for doctype, name in ( + ("Contact", cls.contact), + ("Contact", cls.other_contact), + ("Address", cls.address), + ("Address", cls.other_address), + ("Customer", cls.customer), + ("Customer", cls.other_customer), + ): + frappe.delete_doc(doctype, name, force=True, ignore_missing=True) + frappe.db.commit() + super().tearDownClass() def make_subscription(self, **kwargs): - return frappe.get_doc( - doctype="Simple Subscription", - company=self.company, - customer=self.customer, - currency=self.currency, - start_date="2024-01-01", - frequency="Yearly", - items=[{"item": self.item, "qty": 1}], - **kwargs, + subscription = frappe.new_doc("Simple Subscription") + subscription.customer = self.customer + subscription.update(kwargs) + return subscription + + def test_accepts_links_of_the_same_customer(self): + subscription = self.make_subscription( + customer_address=self.address, + shipping_address_name=self.address, + contact_person=self.contact, ) + subscription.validate_customer_links() # must not raise + + def test_accepts_empty_links(self): + self.make_subscription().validate_customer_links() # must not raise def test_rejects_links_of_another_customer(self): for fieldname, value in ( @@ -245,88 +259,43 @@ def test_rejects_links_of_another_customer(self): ): with self.subTest(fieldname=fieldname): subscription = self.make_subscription(**{fieldname: value}) - self.assertRaises(frappe.ValidationError, subscription.insert) - - def test_rejects_links_still_stale_after_submit(self): - """The links are allow_on_submit, so validate() no longer guards them.""" - subscription = self.make_subscription(customer_address=self.address).insert() - subscription.submit() - - subscription.customer_address = self.other_address - # must be the ownership check, not the generic "cannot change after submit" guard - self.assertRaisesRegex(frappe.ValidationError, "does not belong to Customer", subscription.save) + self.assertRaisesRegex( + frappe.ValidationError, + "does not belong to Customer", + subscription.validate_customer_links, + ) + + def test_guards_the_links_after_submit_too(self): + """The fields are allow_on_submit, and validate() does not run on update after submit.""" + self.assertTrue( + hasattr(SimpleSubscription, "before_update_after_submit"), + "allow_on_submit links need a before_update_after_submit guard", + ) + subscription = self.make_subscription(customer_address=self.other_address) + self.assertRaisesRegex( + frappe.ValidationError, + "does not belong to Customer", + subscription.before_update_after_submit, + ) - def test_virtual_fields_render_linked_records(self): + def test_display_properties_render_linked_records(self): subscription = self.make_subscription( customer_address=self.address, - shipping_address_name=self.shipping_address, + shipping_address_name=self.address, contact_person=self.contact, - ).insert() + ) self.assertIn("Rechnungsweg 1", subscription.billing_address_display) - self.assertIn("Lieferweg 2", subscription.shipping_address_display) + self.assertIn("Rechnungsweg 1", subscription.shipping_address_display) self.assertEqual(subscription.contact_display, "_Test Sub Contact") - def test_virtual_fields_are_empty_without_links(self): - subscription = self.make_subscription().insert() + def test_display_properties_are_empty_without_links(self): + subscription = self.make_subscription() self.assertIsNone(subscription.billing_address_display) self.assertIsNone(subscription.shipping_address_display) self.assertIsNone(subscription.contact_display) - def test_forwards_links_to_invoice(self): - subscription = self.make_subscription( - customer_address=self.address, - shipping_address_name=self.shipping_address, - contact_person=self.contact, - ).insert() - subscription.submit() - - invoice = subscription.create_invoice(date(2024, 1, 1), date(2024, 12, 31)) - - self.assertEqual(invoice.customer_address, self.address) - self.assertEqual(invoice.shipping_address_name, self.shipping_address) - self.assertEqual(invoice.contact_person, self.contact) - # the whole contact block must come from the chosen contact, not the customer default - self.assertEqual(invoice.contact_display, "_Test Sub Contact") - - def test_invoice_falls_back_to_customer_defaults(self): - subscription = self.make_subscription().insert() - subscription.submit() - - invoice = subscription.create_invoice(date(2024, 1, 1), date(2024, 12, 31)) - - self.assertEqual(invoice.customer_address, self.address) - self.assertEqual(invoice.shipping_address_name, self.shipping_address) - - -def make_company(currency: str) -> str: - """A company in the site's own currency, so the invoice does not trip the party-account check.""" - name = "_Test Sub Company" - if not frappe.db.exists("Company", name): - frappe.get_doc( - doctype="Company", - company_name=name, - abbr="_TSC", - default_currency=currency, - country="Germany", - ).insert() - return name - - -def make_item() -> str: - name = "_Test Sub Item" - if not frappe.db.exists("Item", name): - frappe.get_doc( - doctype="Item", - item_code=name, - item_group="All Item Groups", - stock_uom="Nos", - is_stock_item=0, - is_sales_item=1, - ).insert() - return name - def make_customer(name: str) -> str: if not frappe.db.exists("Customer", name): @@ -334,7 +303,7 @@ def make_customer(name: str) -> str: return name -def make_address(title: str, customer: str, line1: str, primary=False, shipping=False) -> str: +def make_address(title: str, customer: str, line1: str) -> str: name = f"{title}-Billing" if not frappe.db.exists("Address", name): frappe.get_doc( @@ -344,8 +313,6 @@ def make_address(title: str, customer: str, line1: str, primary=False, shipping= address_line1=line1, city="Berlin", country="Germany", - is_primary_address=int(primary), - is_shipping_address=int(shipping), links=[{"link_doctype": "Customer", "link_name": customer}], ).insert() return name From a5e0f70da7b1df4736a67e1751e515f6b3318484 Mon Sep 17 00:00:00 2001 From: HenningWendtland <156231187+HenningWendtland@users.noreply.github.com> Date: Tue, 8 Sep 2026 15:14:08 +0200 Subject: [PATCH 7/7] fix: remove commit from tests and add address template creation --- .../test_simple_subscription.py | 59 +++++++++++-------- 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/simple_subscription/simple_subscription/doctype/simple_subscription/test_simple_subscription.py b/simple_subscription/simple_subscription/doctype/simple_subscription/test_simple_subscription.py index 2890c99..e9569ee 100644 --- a/simple_subscription/simple_subscription/doctype/simple_subscription/test_simple_subscription.py +++ b/simple_subscription/simple_subscription/doctype/simple_subscription/test_simple_subscription.py @@ -7,10 +7,10 @@ import frappe from .simple_subscription import ( + CUSTOMER_LINK_FIELDS, BillingTime, Frequency, PeriodType, - SimpleSubscription, get_calendar_period, get_date_period, get_from_and_to_date, @@ -202,37 +202,25 @@ def test_get_from_and_to_date(self): class TestSubscriptionCustomerData(unittest.TestCase): """Customer-bound Address/Contact links: the ownership check and the display properties. - Kept deliberately light. Anything that inserts a Simple Subscription needs a Company and an - Item, and anything that generates the invoice needs a fully set-up ERPNext site -- fixtures - that only the setup wizard creates and that CI (a bare `install-app erpnext`) does not have. - So these exercise the app's own logic on an in-memory document; that the invoice picks the - links up is ERPNext's `_get_party_details` doing its normal job. + Kept deliberately light. Generating the invoice needs a fully set-up ERPNext site -- Company, + Item Group, UOM, Price List, Fiscal Year -- fixtures that only the setup wizard creates and + that CI (a bare `install-app erpnext`) does not have. So these exercise the app's own logic on + an in-memory document, plus a meta check that the fieldnames create_invoice() forwards still + line up with the Sales Invoice. """ @classmethod def setUpClass(cls): super().setUpClass() + # never committed, so the fixtures die with the transaction -- same as IntegrationTestCase + cls.addClassCleanup(frappe.db.rollback) + make_default_address_template() cls.customer = make_customer("_Test Sub Customer") cls.other_customer = make_customer("_Test Sub Other Customer") cls.address = make_address("_Test Sub Billing", cls.customer, "Rechnungsweg 1") cls.other_address = make_address("_Test Sub Foreign", cls.other_customer, "Fremdweg 3") cls.contact = make_contact("_Test Sub Contact", cls.customer) cls.other_contact = make_contact("_Test Sub Foreign Contact", cls.other_customer) - frappe.db.commit() - - @classmethod - def tearDownClass(cls): - for doctype, name in ( - ("Contact", cls.contact), - ("Contact", cls.other_contact), - ("Address", cls.address), - ("Address", cls.other_address), - ("Customer", cls.customer), - ("Customer", cls.other_customer), - ): - frappe.delete_doc(doctype, name, force=True, ignore_missing=True) - frappe.db.commit() - super().tearDownClass() def make_subscription(self, **kwargs): subscription = frappe.new_doc("Simple Subscription") @@ -266,11 +254,11 @@ def test_rejects_links_of_another_customer(self): ) def test_guards_the_links_after_submit_too(self): - """The fields are allow_on_submit, and validate() does not run on update after submit.""" - self.assertTrue( - hasattr(SimpleSubscription, "before_update_after_submit"), - "allow_on_submit links need a before_update_after_submit guard", - ) + """The fields are allow_on_submit, so validate() no longer runs once they change.""" + meta = frappe.get_meta("Simple Subscription") + for fieldname in CUSTOMER_LINK_FIELDS: + self.assertTrue(meta.get_field(fieldname).allow_on_submit, fieldname) + subscription = self.make_subscription(customer_address=self.other_address) self.assertRaisesRegex( frappe.ValidationError, @@ -296,6 +284,25 @@ def test_display_properties_are_empty_without_links(self): self.assertIsNone(subscription.shipping_address_display) self.assertIsNone(subscription.contact_display) + def test_link_fields_match_the_sales_invoice(self): + """create_invoice() forwards these by name, so both doctypes have to spell them the same. + + The forwarding itself needs a set-up site to exercise, so this is what guards it: a + renamed or retyped field on either side breaks the invoice silently, and fails here. + """ + subscription_meta = frappe.get_meta("Simple Subscription") + invoice_meta = frappe.get_meta("Sales Invoice") + for fieldname, doctype in CUSTOMER_LINK_FIELDS.items(): + with self.subTest(fieldname=fieldname): + self.assertEqual(subscription_meta.get_field(fieldname).options, doctype) + self.assertEqual(invoice_meta.get_field(fieldname).options, doctype) + + +def make_default_address_template() -> None: + """Address.validate() renders the address, which needs one. A bare CI site has none.""" + if not frappe.db.exists("Address Template", {"is_default": 1}): + frappe.get_doc(doctype="Address Template", country="Germany", is_default=1).insert() + def make_customer(name: str) -> str: if not frappe.db.exists("Customer", name):