fix(bank-integration): validate required Excel columns per Amount Mode
Importing into Mode 3 with a preset that had no Amount mapping silently dropped every row and showed a confusing "No transactions found" message. Two fixes: - Preset.validate() now requires the Standard Fields the parser needs for the chosen mode (Mode 1: Date+Amount; Mode 2: Date+Debit+Credit; Mode 3: Date+Amount+Direction). Clear error on save lists exactly what's missing. - When all rows drop due to unreadable amount, the import dialog now opens a helper modal pointing at the preset, instead of the generic empty message. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
bd5d773daa
commit
44c3281cb9
|
|
@ -6,11 +6,40 @@ from frappe import _
|
|||
from frappe.model.document import Document
|
||||
|
||||
|
||||
# Standard fields the parser needs depending on the chosen amount_mode.
|
||||
# Date is always required.
|
||||
_REQUIRED_BY_MODE = {
|
||||
"Single column with sign": ("Date", "Amount"),
|
||||
"Separate debit/credit columns": ("Date", "Debit", "Credit"),
|
||||
"Single column + direction column": ("Date", "Amount", "Direction"),
|
||||
}
|
||||
|
||||
|
||||
class BankIntegrationExcelPreset(Document):
|
||||
def validate(self):
|
||||
if self.amount_mode == "Single column + direction column":
|
||||
if not (self.direction_debit_values or "").strip() and not (self.direction_credit_values or "").strip():
|
||||
frappe.throw(_(
|
||||
"Mode 'Single column + direction column' requires at least one of "
|
||||
"'Debit Values' or 'Credit Values' to be filled (comma-separated)."
|
||||
))
|
||||
self._validate_required_mappings()
|
||||
self._validate_direction_values()
|
||||
|
||||
def _validate_required_mappings(self):
|
||||
mode = self.amount_mode or "Separate debit/credit columns"
|
||||
required = _REQUIRED_BY_MODE.get(mode, ())
|
||||
mapped = {
|
||||
(row.standard_field or "").strip()
|
||||
for row in (self.column_mappings or [])
|
||||
if row.excel_column and row.standard_field
|
||||
}
|
||||
missing = [f for f in required if f not in mapped]
|
||||
if missing:
|
||||
frappe.throw(_(
|
||||
"Amount Mode '{0}' requires these Standard Fields to be mapped in the Column Mappings table: {1}. "
|
||||
"Add a row for each missing field."
|
||||
).format(mode, ", ".join(missing)))
|
||||
|
||||
def _validate_direction_values(self):
|
||||
if self.amount_mode != "Single column + direction column":
|
||||
return
|
||||
if not (self.direction_debit_values or "").strip() and not (self.direction_credit_values or "").strip():
|
||||
frappe.throw(_(
|
||||
"Mode 'Single column + direction column' requires at least one of "
|
||||
"'Debit Values' or 'Credit Values' to be filled (comma-separated)."
|
||||
))
|
||||
|
|
|
|||
|
|
@ -128,13 +128,17 @@ const BIExcelImport = {
|
|||
BIExcelImport._showDirectionHelp(droppedDirection, unknownDirections, values.preset);
|
||||
return;
|
||||
}
|
||||
if (droppedAmount > 0 && !txns.length) {
|
||||
BIExcelImport._showAmountHelp(droppedAmount, values.preset);
|
||||
return;
|
||||
}
|
||||
if (!txns.length) {
|
||||
frappe.msgprint({
|
||||
title: __('Nothing to Import'),
|
||||
indicator: 'blue',
|
||||
message: skipped > 0
|
||||
? __('All {0} parsed rows are duplicates.', [skipped])
|
||||
: __('No transactions found in the file.'),
|
||||
: __('No transactions found in the file. Check that the Header Row on the preset points at the actual header line.'),
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
|
@ -316,6 +320,29 @@ const BIExcelImport = {
|
|||
d.show();
|
||||
},
|
||||
|
||||
_showAmountHelp(droppedCount, presetName) {
|
||||
const openPreset = function () {
|
||||
frappe.set_route('Form', 'Bank Integration Excel Preset', presetName);
|
||||
};
|
||||
const d = new frappe.ui.Dialog({
|
||||
title: __('Amount Column Missing or Empty'),
|
||||
fields: [{
|
||||
fieldtype: 'HTML',
|
||||
options:
|
||||
'<div class="alert alert-danger">' +
|
||||
__('All {0} row(s) were dropped because the amount could not be read.', [droppedCount]) +
|
||||
'</div>' +
|
||||
'<p>' + __('Most likely the <b>Amount</b> column (or <b>Debit</b>/<b>Credit</b>, depending on Amount Mode) is not mapped to an Excel header in the preset.') + '</p>' +
|
||||
'<p>' + __('Open the preset and check the Column Mappings table — Standard Field values must include all columns required by the chosen Amount Mode.') + '</p>',
|
||||
}],
|
||||
primary_action_label: __('Open Preset'),
|
||||
primary_action() { d.hide(); openPreset(); },
|
||||
secondary_action_label: __('Close'),
|
||||
secondary_action() { d.hide(); },
|
||||
});
|
||||
d.show();
|
||||
},
|
||||
|
||||
_showDirectionHelp(droppedCount, unknownDirections, presetName) {
|
||||
const openPreset = function () {
|
||||
frappe.set_route('Form', 'Bank Integration Excel Preset', presetName);
|
||||
|
|
|
|||
Loading…
Reference in New Issue