fix(Italy): setup, validations, optimisations (#25065)

erpnext/regional/italy/sales_invoice.js

@@ -11,15 +11,10 @@ erpnext.setup_e_invoice_button = (doctype) => {
 						callback: function(r) {
 							frm.reload_doc();
 							if(r.message) {
-								var w = window.open(
-									frappe.urllib.get_full_url(
-										"/api/method/erpnext.regional.italy.utils.download_e_invoice_file?"
-										+ "file_name=" + r.message
-									)
-								)
-								if (!w) {
-									frappe.msgprint(__("Please enable pop-ups")); return;
-								}
+								open_url_post(frappe.request.url, {
+									cmd: 'frappe.core.doctype.file.file.download_file',
+									file_url: r.message
+								});
 							}
 						}
 					});

erpnext/regional/italy/setup.py

@@ -128,11 +128,8 @@ def make_custom_fields(update=True):
 				fetch_from="company.vat_collectability"),
 			dict(fieldname='sb_e_invoicing_reference', label='E-Invoicing',
 				fieldtype='Section Break', insert_after='against_income_account', print_hide=1),
-			dict(fieldname='company_tax_id', label='Company Tax ID',
-				fieldtype='Data', insert_after='sb_e_invoicing_reference', print_hide=1, read_only=1,
-				fetch_from="company.tax_id"),
 			dict(fieldname='company_fiscal_code', label='Company Fiscal Code',
-				fieldtype='Data', insert_after='company_tax_id', print_hide=1, read_only=1,
+				fieldtype='Data', insert_after='sb_e_invoicing_reference', print_hide=1, read_only=1,
 				fetch_from="company.fiscal_code"),
 			dict(fieldname='company_fiscal_regime', label='Company Fiscal Regime',
 				fieldtype='Data', insert_after='company_fiscal_code', print_hide=1, read_only=1,
@@ -217,4 +214,4 @@ def add_permissions():
 	update_permission_property(doctype, 'Accounts Manager', 0, 'delete', 1)
 	add_permission(doctype, 'Accounts Manager', 1)
 	update_permission_property(doctype, 'Accounts Manager', 1, 'write', 1)
-	update_permission_property(doctype, 'Accounts Manager', 1, 'create', 1)
+	update_permission_property(doctype, 'Accounts Manager', 1, 'create', 1)

erpnext/regional/italy/utils.py

@@ -1,6 +1,8 @@
 from __future__ import unicode_literals
 
-import frappe, json, os
+import io
+import json
+import frappe
 from frappe.utils import flt, cstr
 from erpnext.controllers.taxes_and_totals import get_itemised_tax
 from frappe import _
@@ -28,20 +30,22 @@ def update_itemised_tax_data(doc):
 
 @frappe.whitelist()
 def export_invoices(filters=None):
-	saved_xmls = []
+	frappe.has_permission('Sales Invoice', throw=True)
 
-	invoices = frappe.get_all("Sales Invoice", filters=get_conditions(filters), fields=["*"])
+	invoices = frappe.get_all(
+		"Sales Invoice",
+		filters=get_conditions(filters),
+		fields=["name", "company_tax_id"]
+	)
 
-	for invoice in invoices:
-		attachments = get_e_invoice_attachments(invoice)
-		saved_xmls += [attachment.file_name for attachment in attachments]
+	attachments = get_e_invoice_attachments(invoices)
 
-	zip_filename = "{0}-einvoices.zip".format(frappe.utils.get_datetime().strftime("%Y%m%d_%H%M%S"))
+	zip_filename = "{0}-einvoices.zip".format(
+		frappe.utils.get_datetime().strftime("%Y%m%d_%H%M%S"))
 
-	download_zip(saved_xmls, zip_filename)
+	download_zip(attachments, zip_filename)
 
 
-@frappe.whitelist()
 def prepare_invoice(invoice, progressive_number):
 	#set company information
 	company = frappe.get_doc("Company", invoice.company)
@@ -98,7 +102,7 @@ def prepare_invoice(invoice, progressive_number):
 def get_conditions(filters):
 	filters = json.loads(filters)
 
-	conditions = {"docstatus": 1}
+	conditions = {"docstatus": 1, "company_tax_id": ("!=", "")}
 
 	if filters.get("company"): conditions["company"] = filters["company"]
 	if filters.get("customer"): conditions["customer"] =  filters["customer"]
@@ -111,23 +115,22 @@ def get_conditions(filters):
 
 	return conditions
 
-#TODO: Use function from frappe once PR #6853 is merged.
+
 def download_zip(files, output_filename):
-	from zipfile import ZipFile
+	import zipfile
 
-	input_files = [frappe.get_site_path('private', 'files', filename) for filename in files]
-	output_path = frappe.get_site_path('private', 'files', output_filename)
+	zip_stream = io.BytesIO()
+	with zipfile.ZipFile(zip_stream, 'w', zipfile.ZIP_DEFLATED) as zip_file:
+		for file in files:
+			file_path = frappe.utils.get_files_path(
+				file.file_name, is_private=file.is_private)
 
-	with ZipFile(output_path, 'w') as output_zip:
-		for input_file in input_files:
-			output_zip.write(input_file, arcname=os.path.basename(input_file))
-
-	with open(output_path, 'rb') as fileobj:
-		filedata = fileobj.read()
+			zip_file.write(file_path, arcname=file.file_name)
 
 	frappe.local.response.filename = output_filename
-	frappe.local.response.filecontent = filedata
+	frappe.local.response.filecontent = zip_stream.getvalue()
 	frappe.local.response.type = "download"
+	zip_stream.close()
 
 def get_invoice_summary(items, taxes):
 	summary_data = frappe._dict()
@@ -307,23 +310,12 @@ def prepare_and_attach_invoice(doc, replace=False):
 @frappe.whitelist()
 def generate_single_invoice(docname):
 	doc = frappe.get_doc("Sales Invoice", docname)
-
+	frappe.has_permission("Sales Invoice", doc=doc, throw=True)
 
 	e_invoice = prepare_and_attach_invoice(doc, True)
+	return e_invoice.file_url
 
-	return e_invoice.file_name
-
-@frappe.whitelist()
-def download_e_invoice_file(file_name):
-	content = None
-	with open(frappe.get_site_path('private', 'files', file_name), "r") as f:
-		content = f.read()
-
-	frappe.local.response.filename = file_name
-	frappe.local.response.filecontent = content
-	frappe.local.response.type = "download"
-
-#Delete e-invoice attachment on cancel.
+# Delete e-invoice attachment on cancel.
 def sales_invoice_on_cancel(doc, method):
 	if get_company_country(doc.company) not in ['Italy',
 		'Italia', 'Italian Republic', 'Repubblica Italiana']:
@@ -335,16 +327,38 @@ def sales_invoice_on_cancel(doc, method):
 def get_company_country(company):
 	return frappe.get_cached_value('Company', company, 'country')
 
-def get_e_invoice_attachments(invoice):
-	if not invoice.company_tax_id:
-		return []
+def get_e_invoice_attachments(invoices):
+	if not isinstance(invoices, list):
+		if not invoices.company_tax_id:
+			return
+
+		invoices = [invoices]
+
+	tax_id_map = {
+		invoice.name: (
+			invoice.company_tax_id
+			if invoice.company_tax_id.startswith("IT")
+			else "IT" + invoice.company_tax_id
+		) for invoice in invoices
+	}
+
+	attachments = frappe.get_all(
+		"File",
+		fields=("name", "file_name", "attached_to_name", "is_private"),
+		filters= {
+			"attached_to_name": ('in', tax_id_map),
+			"attached_to_doctype": 'Sales Invoice'
+		}
+	)
 
 	out = []
-	attachments = get_attachments(invoice.doctype, invoice.name)
-	company_tax_id = invoice.company_tax_id if invoice.company_tax_id.startswith("IT") else "IT" + invoice.company_tax_id
-
 	for attachment in attachments:
-		if attachment.file_name and attachment.file_name.startswith(company_tax_id) and attachment.file_name.endswith(".xml"):
+		if (
+			attachment.file_name
+			and attachment.file_name.endswith(".xml")
+			and attachment.file_name.startswith(
+				tax_id_map.get(attachment.attached_to_name))
+		):
 			out.append(attachment)
 
 	return out