Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
"collect_registration_fee",
"registration_item",
"registration_fee",
"registration_validity",
"show_payment_popup",
"enable_free_follow_ups",
"max_visits",
Expand Down Expand Up @@ -104,6 +105,15 @@
"non_negative": 1,
"options": "Currency"
},
{
"default": "0",
"depends_on": "eval:doc.collect_registration_fee == 1",
"description": "Number of days a Patient Registration stays valid. Set to 0 for registrations that never expire.",
"fieldname": "registration_validity",
"fieldtype": "Int",
"label": "Registration Validity (Days)",
"non_negative": 1
},
Comment thread
akurungadam marked this conversation as resolved.
{
"depends_on": "eval:doc.enable_free_follow_ups == 1",
"description": "Time period (Valid number of days) for free consultations",
Expand Down Expand Up @@ -513,7 +523,7 @@
],
"issingle": 1,
"links": [],
"modified": "2025-12-25 09:36:33.073374",
"modified": "2026-06-21 16:41:08.414871",
"modified_by": "Administrator",
"module": "Healthcare",
"name": "Healthcare Settings",
Expand Down
26 changes: 22 additions & 4 deletions healthcare/healthcare/doctype/patient/patient.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,16 @@ frappe.ui.form.on('Patient', {
erpnext.toggle_naming_series();
}

if (frappe.defaults.get_default('collect_registration_fee') && frm.doc.status == 'Disabled') {
frm.add_custom_button(__('Invoice Patient Registration'), function () {
invoice_registration(frm);
});
if (frm.doc.status == 'Disabled') {
if (frappe.defaults.get_default('collect_registration_fee')) {
frm.add_custom_button(__('Invoice Patient Registration'), function () {
invoice_registration(frm);
});
} else {
frm.add_custom_button(__('Renew Registration'), function () {
renew_registration(frm);
});
}
}

if (frm.doc.patient_name && frappe.user.has_role('Physician')) {
Expand Down Expand Up @@ -138,6 +144,18 @@ let create_encounter = function (frm) {
frappe.new_doc('Patient Encounter');
};

let renew_registration = function (frm) {
frappe.call({
doc: frm.doc,
method: 'renew_registration',
callback: function (data) {
if (!data.exc) {
frm.reload_doc();
}
}
});
};

let invoice_registration = function (frm) {
frappe.call({
doc: frm.doc,
Expand Down
11 changes: 11 additions & 0 deletions healthcare/healthcare/doctype/patient/patient.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@
get_receivable_account,
send_registration_sms,
)
from healthcare.healthcare.doctype.patient_registration.patient_registration import (
create_patient_registration,
)
Comment on lines +25 to +27

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Enforce renewal eligibility in the whitelisted method.

renew_registration is callable directly, so the client-only fee/no-fee split can be bypassed. When collect_registration_fee is enabled, this can create a free registration and reactivate the patient; also avoid re-enabling a disabled patient that already has a current active registration.

Suggested server-side guard
 from healthcare.healthcare.doctype.patient_registration.patient_registration import (
 	create_patient_registration,
+	has_active_patient_registration,
 )
 	`@frappe.whitelist`()
 	def renew_registration(self):
 		"""Create a fresh registration and re-enable the patient (when no fee is collected)."""
-		if not frappe.db.exists("Patient Registration", {"patient": self.name, "status": "Active"}):
-			create_patient_registration(self.name)
+		if frappe.db.get_single_value("Healthcare Settings", "collect_registration_fee"):
+			frappe.throw(_("Invoice the patient registration fee before renewing this registration."))
+		if has_active_patient_registration(self.name):
+			frappe.throw(_("Patient already has an active registration."))
+		create_patient_registration(self.name)
 		self.db_set("status", "Active")

Also applies to: 186-191

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@healthcare/healthcare/doctype/patient/patient.py` around lines 25 - 27, The
`renew_registration` method in the Patient doctype needs server-side validation
guards to prevent bypassing fee/no-fee split logic and to enforce renewal
eligibility. Add checks within the `renew_registration` whitelisted method to:
verify that when `collect_registration_fee` is enabled, the appropriate fee
collection flow is enforced (preventing free registrations), and validate that a
disabled patient does not get re-enabled if they already have a current active
registration. These guards should run before any patient reactivation logic to
ensure renewal eligibility is properly enforced at the server level.



class Patient(Document):
Expand All @@ -42,6 +45,7 @@ def after_insert(self):
if frappe.db.get_single_value("Healthcare Settings", "collect_registration_fee"):
frappe.db.set_value("Patient", self.name, "status", "Disabled")
else:
create_patient_registration(self.name)
send_registration_sms(self)
self.reload()

Expand Down Expand Up @@ -179,6 +183,13 @@ def get_age(self):
age_str = f"{age.years!s} {_('Year(s)')} {age.months!s} {_('Month(s)')} {age.days!s} {_('Day(s)')}"
return age_str

@frappe.whitelist()
def renew_registration(self):
"""Create a fresh registration and re-enable the patient (when no fee is collected)."""
if not frappe.db.exists("Patient Registration", {"patient": self.name, "status": "Active"}):
create_patient_registration(self.name)
self.db_set("status", "Active")

@frappe.whitelist()
def invoice_patient_registration(self):
registration_fee = frappe.db.get_single_value("Healthcare Settings", "registration_fee")
Expand Down
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// Copyright (c) 2026, ESS LLP and contributors
// For license information, please see license.txt

frappe.ui.form.on("Patient Registration", {});
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
{
"actions": [],
"allow_import": 1,
"autoname": "naming_series:",
"creation": "2026-06-21 12:00:00",
"doctype": "DocType",
"document_type": "Document",
"editable_grid": 1,
"engine": "InnoDB",
"field_order": [
"naming_series",
"patient",
"patient_name",
"company",
"column_break_status",
"status",
"sales_invoice",
"section_break_validity",
"start_date",
"column_break_validity",
"valid_till"
],
"fields": [
{
"fieldname": "naming_series",
"fieldtype": "Select",
"label": "Series",
"options": "HLC-PR-.YYYY.-",
"print_hide": 1,
"report_hide": 1,
"set_only_once": 1
},
{
"fieldname": "patient",
"fieldtype": "Link",
"in_list_view": 1,
"in_standard_filter": 1,
"label": "Patient",
"options": "Patient",
"reqd": 1,
"search_index": 1
},
{
"fetch_from": "patient.patient_name",
"fieldname": "patient_name",
"fieldtype": "Data",
"in_list_view": 1,
"label": "Patient Name",
"read_only": 1
},
{
"fieldname": "company",
"fieldtype": "Link",
"label": "Company",
"options": "Company"
},
{
"fieldname": "column_break_status",
"fieldtype": "Column Break"
},
{
"default": "Active",
"fieldname": "status",
"fieldtype": "Select",
"in_list_view": 1,
"in_standard_filter": 1,
"label": "Status",
"options": "Active\nExpired\nCancelled",
"read_only": 1
},
{
"fieldname": "sales_invoice",
"fieldtype": "Link",
"label": "Registration Invoice",
"options": "Sales Invoice",
"read_only": 1
},
{
"fieldname": "section_break_validity",
"fieldtype": "Section Break",
"label": "Validity"
},
{
"fieldname": "start_date",
"fieldtype": "Date",
"label": "Registration Date",
"read_only": 1
},
{
"fieldname": "column_break_validity",
"fieldtype": "Column Break"
},
{
"description": "Empty when the registration never expires.",
"fieldname": "valid_till",
"fieldtype": "Date",
"in_list_view": 1,
"label": "Valid Till",
"read_only": 1
}
],
"grid_page_length": 50,
"links": [],
"modified": "2026-06-22 16:50:14.056868",
"modified_by": "Administrator",
"module": "Healthcare",
"name": "Patient Registration",
"naming_rule": "By \"Naming Series\" field",
"owner": "Administrator",
"permissions": [
{
"create": 1,
"delete": 1,
"email": 1,
"export": 1,
"print": 1,
"read": 1,
"report": 1,
"role": "Healthcare Administrator",
"share": 1,
"write": 1
},
{
"create": 1,
"email": 1,
"export": 1,
"print": 1,
"read": 1,
"report": 1,
"role": "Physician",
"share": 1
}
],
"restrict_to_domain": "Healthcare",
"row_format": "Dynamic",
"search_fields": "patient, patient_name",
"sort_field": "creation",
"sort_order": "DESC",
"states": [],
"title_field": "patient_name",
"track_changes": 1
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
# Copyright (c) 2026, ESS LLP and contributors
# For license information, please see license.txt

import datetime

import frappe
from frappe.model.document import Document
from frappe.utils import getdate


class PatientRegistration(Document):
def validate(self):
self.update_status()

def update_status(self):
if self.status == "Cancelled":
return
if self.valid_till and getdate(self.valid_till) < getdate():
self.status = "Expired"
else:
self.status = "Active"


def create_patient_registration(patient, sales_invoice=None, start_date=None, company=None):
start_date = getdate(start_date) if start_date else getdate()

registration = frappe.new_doc("Patient Registration")
registration.patient = patient
registration.company = company
registration.sales_invoice = sales_invoice
registration.start_date = start_date
registration.valid_till = get_valid_till(start_date)
registration.save(ignore_permissions=True)

return registration


def get_valid_till(start_date):
validity = frappe.db.get_single_value("Healthcare Settings", "registration_validity")
if not validity:
return None
return start_date + datetime.timedelta(days=int(validity))


def expire_registrations():
"""Daily scheduler: expire lapsed registrations and disable patients without an active one."""
lapsed = frappe.get_all(
"Patient Registration",
filters={"status": "Active", "valid_till": ["<", getdate()]},
pluck="name",
)

patients = set()
for name in lapsed:
registration = frappe.get_doc("Patient Registration", name)
registration.status = "Expired"
registration.save(ignore_permissions=True)
patients.add(registration.patient)

for patient in patients:
disable_patient_without_active_registration(patient)


def disable_patient_without_active_registration(patient):
if frappe.db.exists("Patient Registration", {"patient": patient, "status": "Active"}):
return
if frappe.db.get_value("Patient", patient, "status") == "Disabled":
return
frappe.db.set_value("Patient", patient, "status", "Disabled")
Comment thread
akurungadam marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# Copyright (c) 2026, ESS LLP and Contributors
# See license.txt

import frappe
from frappe.utils import add_days, getdate

from healthcare.healthcare.doctype.patient_registration.patient_registration import (
create_patient_registration,
expire_registrations,
)
from healthcare.tests.utils import HealthcareTestSuite


class TestPatientRegistration(HealthcareTestSuite):
def setUp(self):
super().setUp()
frappe.db.sql("""delete from `tabPatient Registration`""")

def get_patient(self):
return frappe.get_list("Patient", pluck="name")[0]
Comment thread
akurungadam marked this conversation as resolved.

def test_valid_till_from_settings(self):
frappe.db.set_single_value("Healthcare Settings", "registration_validity", 30)

registration = create_patient_registration(self.get_patient())

expected = add_days(getdate(), 30)
self.assertEqual(getdate(registration.valid_till), getdate(expected))
self.assertEqual(registration.status, "Active")

def test_zero_validity_never_expires(self):
frappe.db.set_single_value("Healthcare Settings", "registration_validity", 0)

registration = create_patient_registration(self.get_patient())

self.assertIsNone(registration.valid_till)
self.assertEqual(registration.status, "Active")

def test_past_validity_marks_expired(self):
registration = create_patient_registration(self.get_patient())
registration.valid_till = add_days(getdate(), -1)
registration.save(ignore_permissions=True)

self.assertEqual(registration.status, "Expired")

def test_scheduler_expires_and_disables_patient(self):
patient = self.get_patient()
frappe.db.set_value("Patient", patient, "status", "Active")
registration = create_patient_registration(patient)
registration.db_set("valid_till", add_days(getdate(), -1))

expire_registrations()

self.assertEqual(frappe.db.get_value("Patient Registration", registration.name, "status"), "Expired")
self.assertEqual(frappe.db.get_value("Patient", patient, "status"), "Disabled")

def test_scheduler_keeps_patient_with_active_registration(self):
frappe.db.set_single_value("Healthcare Settings", "registration_validity", 0)
patient = self.get_patient()
frappe.db.set_value("Patient", patient, "status", "Active")
lapsed = create_patient_registration(patient)
lapsed.db_set("valid_till", add_days(getdate(), -1))
create_patient_registration(patient, start_date=getdate()) # never-expiring registration

expire_registrations()

self.assertEqual(frappe.db.get_value("Patient", patient, "status"), "Active")
Loading
Loading