Fix government share double-posting; restrict Close Sales Order to Managers

landms/landms/doctype/plot_contract/plot_contract.py

@@ -606,27 +606,20 @@ def _post_completion_entries(self, settings):
 		if selling_price <= 0:
 			return None
 
-		accounts = [{
-			"account": settings.customer_advance_account,
-			"debit_in_account_currency": selling_price,
-			"cost_center": settings.cost_center,
-			"land_acquisition": self.land_acquisition,
-		}]
-
-		if govt_fee > 0:
-			accounts.append({
-				"account": settings.government_payable_account,
-				"credit_in_account_currency": govt_fee,
+		accounts = [
+			{
+				"account": settings.customer_advance_account,
+				"debit_in_account_currency": net_revenue,
 				"cost_center": settings.cost_center,
 				"land_acquisition": self.land_acquisition,
-			})
-
-		accounts.append({
-			"account": settings.revenue_account,
-			"credit_in_account_currency": net_revenue,
-			"cost_center": settings.cost_center,
-			"land_acquisition": self.land_acquisition,
-		})
+			},
+			{
+				"account": settings.revenue_account,
+				"credit_in_account_currency": net_revenue,
+				"cost_center": settings.cost_center,
+				"land_acquisition": self.land_acquisition,
+			},
+		]
 
 		je = frappe.get_doc({
 			"doctype": "Journal Entry",

landms/landms/doctype/plot_master/plot_master.js

@@ -5,6 +5,15 @@ frappe.ui.form.on('Plot Master', {
         frm.set_query('land_acquisition', () => ({
             filters: { status: ['in', ['Approved', 'Subdivided']] }
         }));
+
+        // Only show plot types that have a selling rate on the current LA
+        frm.set_query('plot_type', () => {
+            if (!frm.doc.land_acquisition) return {};
+            return {
+                query: 'landms.landms.doctype.plot_master.plot_master.get_plot_types_for_la',
+                filters: { land_acquisition: frm.doc.land_acquisition }
+            };
+        });
     },
 
     refresh(frm) {

landms/landms/doctype/plot_master/plot_master.py

@@ -306,3 +306,18 @@ def _build_plot_stock_entry_doc(
 			}
 		],
 	}
+
+
+@frappe.whitelist()
+@frappe.validate_and_sanitize_search_inputs
+def get_plot_types_for_la(doctype, txt, searchfield, start, page_len, filters):
+	la = filters.get("land_acquisition") if isinstance(filters, dict) else None
+	if not la:
+		return []
+	return frappe.db.sql("""
+		SELECT DISTINCT plot_type
+		FROM `tabLand Acquisition Plot Type Rate`
+		WHERE parent = %s AND plot_type LIKE %s
+		ORDER BY plot_type
+		LIMIT %s OFFSET %s
+	""", (la, f"%{txt}%", page_len, start))

landms/landms/report/landms_revenue/landms_revenue.py

@@ -30,51 +30,65 @@ def get_data(filters):
 	if grouping == "Weekly":
 		period_expr = "CONCAT('Week ', LPAD(WEEK(pe.posting_date, 1), 2, '0'), ' — ', YEAR(pe.posting_date))"
 		sort_expr   = "YEARWEEK(pe.posting_date, 1)"
+		je_sort     = "YEARWEEK(je.posting_date, 1)"
 	else:
 		period_expr = "DATE_FORMAT(pe.posting_date, '%%M %%Y')"
 		sort_expr   = "DATE_FORMAT(pe.posting_date, '%%Y%%m')"
+		je_sort     = "DATE_FORMAT(je.posting_date, '%%Y%%m')"
 
-	rows = frappe.db.sql(f"""
+	params = {"from_date": from_date, "to_date": to_date}
+
+	pay_rows = frappe.db.sql(f"""
 		SELECT
-			{period_expr}                                                AS period,
-			{sort_expr}                                                  AS sort_key,
-			COUNT(DISTINCT pe.name)                                      AS payment_count,
-			COUNT(DISTINCT COALESCE(pc.name, so.plot_contract))          AS contract_count,
-			SUM(per.allocated_amount)                                    AS total_collected,
-			SUM(
-				CASE
-					WHEN pc.contract_status = 'Completed'
-					THEN per.allocated_amount * pc.government_share_percent / 100
-					ELSE 0
-				END
-			)                                                            AS govt_fee
+			{period_expr}                                       AS period,
+			{sort_expr}                                         AS sort_key,
+			COUNT(DISTINCT pe.name)                             AS payment_count,
+			COUNT(DISTINCT COALESCE(pc.name, so.plot_contract)) AS contract_count,
+			SUM(per.allocated_amount)                           AS total_collected
 		FROM `tabPayment Entry` pe
 		INNER JOIN `tabPayment Entry Reference` per
-			ON  per.parent             = pe.name
-			AND per.reference_doctype  = 'Sales Invoice'
+			ON  per.parent            = pe.name
+			AND per.reference_doctype = 'Sales Invoice'
 		INNER JOIN `tabSales Invoice` si
 			ON  si.name                 = per.reference_name
 			AND si.docstatus            = 1
 			AND si.is_plot_sale_invoice = 1
 		LEFT JOIN `tabSales Order` so
-			ON  so.plot_sales_invoice   = si.name
-			AND so.docstatus            = 1
+			ON  so.plot_sales_invoice = si.name
+			AND so.docstatus          = 1
 		LEFT JOIN `tabPlot Contract` pc
-			ON  pc.name                 = COALESCE(NULLIF(si.plot_contract, ''), so.plot_contract)
-			AND pc.docstatus           != 2
+			ON  pc.name      = COALESCE(NULLIF(si.plot_contract, ''), so.plot_contract)
+			AND pc.docstatus != 2
 		WHERE pe.party_type   = 'Customer'
 		  AND pe.docstatus    = 1
 		  AND pe.posting_date BETWEEN %(from_date)s AND %(to_date)s
 		GROUP BY sort_key, period
 		ORDER BY sort_key
-	""", {"from_date": from_date, "to_date": to_date}, as_dict=True)
+	""", params, as_dict=True)
+
+	je_rows = frappe.db.sql(f"""
+		SELECT
+			{je_sort}                             AS sort_key,
+			SUM(jea.credit_in_account_currency)   AS govt_fee
+		FROM `tabJournal Entry` je
+		INNER JOIN `tabJournal Entry Account` jea
+			ON  jea.parent                     = je.name
+			AND jea.credit_in_account_currency > 0
+		WHERE je.docstatus          = 1
+		  AND je.lms_payment_entry IS NOT NULL
+		  AND je.lms_payment_entry != ''
+		  AND je.posting_date BETWEEN %(from_date)s AND %(to_date)s
+		GROUP BY sort_key
+	""", params, as_dict=True)
+
+	je_by_key = {str(r.sort_key): flt(r.govt_fee) for r in je_rows}
 
 	data = []
 	grand_collected = grand_govt = grand_payments = grand_contracts = 0.0
 
-	for row in rows:
+	for row in pay_rows:
 		collected = flt(row.total_collected)
-		govt      = flt(row.govt_fee)
+		govt      = je_by_key.get(str(row.sort_key), 0.0)
 		net       = collected - govt
 		grand_collected  += collected
 		grand_govt       += govt

landms/payment_sync.py

@@ -696,20 +696,26 @@ def _post_government_share_je(pe_doc):
 
 	Dr Customer Advances Account  (reducing deferred revenue)
 	Cr Government Payable Account (recording govt share payable)
-	Amount = government_share_percent × paid_amount
+	Amount = government_share_percent × allocated_amount
+
+	allocated_amount (not the PE's total paid_amount) is used because a single
+	payment can bundle in extra cash beyond the plot price — e.g. customers
+	often pay government fees alongside the plot price in the same transaction.
+	Only the portion actually allocated to the plot sale is land-sale revenue
+	subject to the government's share.
 	"""
 	if pe_doc.docstatus != 1:
 		return
 
-	so_name, govt_pct, land_acquisition = _get_govt_share_fields_from_pe(pe_doc)
+	so_name, govt_pct, land_acquisition, allocated_amount = _get_govt_share_fields_from_pe(pe_doc)
 	if not so_name or flt(govt_pct) <= 0:
 		return
 
 	settings = frappe.get_single("LandMS Settings")
 	if not settings.government_payable_account or not settings.customer_advance_account:
 		return
 
-	govt_amount = flt(pe_doc.paid_amount) * flt(govt_pct) / 100.0
+	govt_amount = flt(allocated_amount) * flt(govt_pct) / 100.0
 	if govt_amount <= 0:
 		return
 
@@ -756,8 +762,24 @@ def _cancel_government_share_je(pe_doc):
 
 
 def _get_govt_share_fields_from_pe(pe_doc):
-	"""Return (sales_order_name, government_share_percent, land_acquisition) from PE references."""
+	"""Return (sales_order_name, government_share_percent, land_acquisition, allocated_amount)
+	from PE references.
+
+	allocated_amount is the SUM of allocated amounts across every reference row that
+	resolves to the plot sale — its Sales Order directly, or a Sales Invoice flagged
+	as the plot sale invoice for that Sales Order. Frappe can split one payment's
+	allocation to the same invoice/order across multiple reference rows, so a single
+	matching row is not enough. This is not the Payment Entry's total paid_amount,
+	which can include extra cash the customer bundled into the same transaction (e.g.
+	for government fees) that was never allocated to the plot sale invoice/order."""
+	so_name = None
+	govt_pct = 0
+	land_acquisition = ""
+	allocated_amount = 0.0
+
 	for row in pe_doc.get("references") or []:
+		row_so_name = None
+
 		if row.reference_doctype == "Sales Invoice":
 			si = frappe.db.get_value(
 				"Sales Invoice", row.reference_name,
@@ -766,24 +788,28 @@ def _get_govt_share_fields_from_pe(pe_doc):
 			)
 			if not si or not si.is_plot_sale_invoice:
 				continue
-			so_name = frappe.db.get_value(
+			row_so_name = frappe.db.get_value(
 				"Sales Invoice Item",
 				{"parent": row.reference_name},
 				"sales_order",
 			)
-			if not so_name:
-				continue
+		elif row.reference_doctype == "Sales Order":
+			row_so_name = row.reference_name
+
+		if not row_so_name:
+			continue
+
+		if so_name is None:
+			so_name = row_so_name
 			govt_pct, land_acquisition = frappe.db.get_value(
 				"Sales Order", so_name,
 				["government_share_percent", "land_acquisition"],
 			) or (0, "")
-			return so_name, flt(govt_pct), land_acquisition
 
-		if row.reference_doctype == "Sales Order":
-			govt_pct, land_acquisition = frappe.db.get_value(
-				"Sales Order", row.reference_name,
-				["government_share_percent", "land_acquisition"],
-			) or (0, "")
-			return row.reference_name, flt(govt_pct), land_acquisition
+		if row_so_name == so_name:
+			allocated_amount += flt(row.allocated_amount)
+
+	if not so_name:
+		return None, 0, "", 0
 
-	return None, 0, ""
+	return so_name, flt(govt_pct), land_acquisition, allocated_amount

landms/public/js/sales_order.js

@@ -76,6 +76,8 @@ function _render_so_close_button(frm) {
         frm.remove_custom_button(__('Hold'), __('Status'));
     }, 10);
 
+    if (!frappe.user.has_role('LandMS Sales Manager')) return;
+
     frm.add_custom_button(__('Close Sales Order'), () => {
         frappe.confirm(
             __('Close this Sales Order? The plot will be released and the TCB reference declined. This cannot be undone.'),

landms/sales_order_hooks.py

@@ -181,6 +181,8 @@ def cancel_sales_order(doc, method=None):
 @frappe.whitelist()
 def close_sales_order(sales_order_name):
 	"""Close a plot Sales Order with no payments — releases plot, declines TCB CN, cancels application."""
+	frappe.only_for("LandMS Sales Manager")
+
 	doc = frappe.get_doc("Sales Order", sales_order_name)
 	if not _is_landms_sales_order(doc):
 		frappe.throw("This Sales Order is not a LandMS plot sale.")