Verify the ERPNext integration live and fix the defects it found (Phase F4)
Ran the push flow against ERPNext 15.121.6 on a plain site and on a site with India Compliance 15.32.0 (podman, scripts/erpnext-e2e). 15 ignored live tests pass on both. Fixes, each covered by a mock-based test: - load_options no longer fails on a plain site: the India Compliance-only gstin field is dropped on a 417 and retried. - Code-less rows with fractional hours or minutes send stock_uom, since ERPNext defaults it to Nos. - A created document whose total differs from Voiced's (ERPNext's default Banker's Rounding on half-paise ties) is recorded as a conflict, kept as a draft, not submitted and not given a PDF. The message names Commercial Rounding as the fix. - An existing customer address is reused instead of creating a duplicate on every first push. A bare creation order-by is not used with a Dynamic Link filter. - A re-adopted document no longer gets a second copy of the PDF. - Reverse charge: India Compliance rejects is_reverse_charge unless tax rows are negative RCM amounts, so the flag is sent as 0 with a warning that the invoice lands as a normal taxed invoice. - The 16-character name rule is checked locally in mirror mode under India Compliance, and a TDS-only payment is refused locally with the reason. Payment Entry mapping verified: bank_account is the Account name (paid_to), paid and received amounts equal the cash received, allocated_amount is cash plus TDS, and TDS is one positive deductions row. The integration user needs the Accounts User and Sales User roles. Not verified: ERPNext v14 and v16, other India Compliance versions, the Tauri commands and UI against a live site, SEZ and overseas customers, UTGST supplier states, TLS sites, e-invoicing.
This commit is contained in:
@@ -6,6 +6,7 @@
|
||||
|
||||
use super::client::{ErpClient, Upload};
|
||||
use super::config::{self, ErpnextConfig, NamingMode};
|
||||
use super::discovery::ic_number_ok;
|
||||
use super::errors::{ErpError, ErrorKind};
|
||||
use super::mapping::{
|
||||
self, build_address, build_customer, build_sales_invoice, paise_to_decimal, remarks_marker, InvoiceContext, Vendor,
|
||||
@@ -561,8 +562,15 @@ async fn ensure_address(
|
||||
}
|
||||
validate_address(client).map_err(pre)?;
|
||||
let req = build_address(client, customer, l.india_compliance).map_err(pre)?;
|
||||
let resp = http.post(req.path, &req.body, req.idempotent).await?;
|
||||
let name = doc_name(&resp).ok_or_else(|| ErpError::protocol("ERPNext did not return the new address's name."))?;
|
||||
// The link to the address is lost when the local database is (re)built: reuse the customer's matching
|
||||
// address rather than creating a duplicate on every first push.
|
||||
let name = match find_address(http, customer, &req.body).await {
|
||||
Some(existing) => existing,
|
||||
None => {
|
||||
let resp = http.post(req.path, &req.body, req.idempotent).await?;
|
||||
doc_name(&resp).ok_or_else(|| ErpError::protocol("ERPNext did not return the new address's name."))?
|
||||
}
|
||||
};
|
||||
with_db(db, |c| {
|
||||
c.execute("UPDATE clients SET erpnext_address = ?1 WHERE id = ?2", params![name, client_id])
|
||||
.map(|_| ())
|
||||
@@ -571,10 +579,52 @@ async fn ensure_address(
|
||||
Ok((Some(name), None))
|
||||
}
|
||||
|
||||
/// An enabled address linked to `customer` with the same first line and PIN code as the one about to be created.
|
||||
/// Any lookup problem counts as "none": the create that follows reports the real error. (A bare `creation` in
|
||||
/// the order-by is ambiguous once a Dynamic Link filter joins the child table: HTTP 500.)
|
||||
async fn find_address(http: &ErpClient, customer: &str, body: &Value) -> Option<String> {
|
||||
let line1 = body.get("address_line1")?.as_str()?;
|
||||
let mut filters = vec![
|
||||
json!(["Dynamic Link", "link_doctype", "=", "Customer"]),
|
||||
json!(["Dynamic Link", "link_name", "=", customer]),
|
||||
json!(["address_line1", "=", line1]),
|
||||
json!(["disabled", "=", 0]),
|
||||
];
|
||||
if let Some(pin) = body.get("pincode").and_then(Value::as_str) {
|
||||
filters.push(json!(["pincode", "=", pin]));
|
||||
}
|
||||
let rows = http.list_resource("Address", &["name"], Value::Array(filters), "`tabAddress`.creation asc").await.ok()?;
|
||||
rows.first()?.get("name")?.as_str().map(str::to_string)
|
||||
}
|
||||
|
||||
struct RemoteDoc {
|
||||
name: String,
|
||||
docstatus: i64,
|
||||
created: bool,
|
||||
/// The grand total ERPNext computed, when the response carried it.
|
||||
total_paise: Option<i64>,
|
||||
}
|
||||
|
||||
/// ERPNext's default Rounding Method is Banker's Rounding (half to even); Voiced rounds half up. They only
|
||||
/// differ on half-paise ties (e.g. 9% of 10.50), by one paise per tie.
|
||||
fn rounding_hint(theirs: i64, ours: i64) -> &'static str {
|
||||
if (theirs - ours).abs() <= 5 {
|
||||
" The usual cause is ERPNext's rounding of half-paise amounts: set System Settings > Rounding Method to \"Commercial Rounding\" in ERPNext."
|
||||
} else {
|
||||
""
|
||||
}
|
||||
}
|
||||
|
||||
fn total_conflict(name: &str, number: &str, theirs: i64, ours: i64) -> ErpError {
|
||||
ErpError::new(
|
||||
ErrorKind::Conflict,
|
||||
format!(
|
||||
"ERPNext has {name} for invoice {number}, but its total is {} and Voiced's is {}. Voiced does not overwrite it and does not submit it: fix or delete the ERPNext document, then push again.{}",
|
||||
paise_to_decimal(theirs),
|
||||
paise_to_decimal(ours),
|
||||
rounding_hint(theirs, ours)
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
/// An existing remote document is only accepted when its total equals Voiced's; anything else is a
|
||||
@@ -589,16 +639,10 @@ fn accept_existing(name: &str, doc: &Value, inv: &Invoice) -> Result<RemoteDoc,
|
||||
}
|
||||
let ours = gst::rupees_to_paise(inv.total);
|
||||
match doc_total_paise(doc) {
|
||||
Some(theirs) if theirs == ours => Ok(RemoteDoc { name: name.to_string(), docstatus, created: false }),
|
||||
Some(theirs) => Err(ErpError::new(
|
||||
ErrorKind::Conflict,
|
||||
format!(
|
||||
"ERPNext already has {name} for invoice {}, but its total is {} and Voiced's is {}. Voiced does not overwrite it: fix or delete the ERPNext document, then push again.",
|
||||
inv.number,
|
||||
paise_to_decimal(theirs),
|
||||
paise_to_decimal(ours)
|
||||
),
|
||||
)),
|
||||
Some(theirs) if theirs == ours => {
|
||||
Ok(RemoteDoc { name: name.to_string(), docstatus, created: false, total_paise: Some(theirs) })
|
||||
}
|
||||
Some(theirs) => Err(total_conflict(name, &inv.number, theirs, ours)),
|
||||
None => Err(ErpError::new(
|
||||
ErrorKind::Conflict,
|
||||
format!("ERPNext already has {name} for invoice {}, but its total could not be read to compare.", inv.number),
|
||||
@@ -647,7 +691,7 @@ async fn create_or_find(http: &ErpClient, l: &Loaded, body: &mapping::BuiltReque
|
||||
return Err(ErpError::protocol("ERPNext did not return the new Sales Invoice's name."))
|
||||
}
|
||||
};
|
||||
Ok(RemoteDoc { name, docstatus: doc_docstatus(&doc), created: true })
|
||||
Ok(RemoteDoc { name, docstatus: doc_docstatus(&doc), created: true, total_paise: doc_total_paise(&doc) })
|
||||
}
|
||||
Err(e) if e.kind == ErrorKind::Duplicate && l.cfg.naming_mode == NamingMode::Mirror => {
|
||||
// The mirrored name is taken: either a repeat of an earlier push or someone else's document.
|
||||
@@ -696,8 +740,30 @@ fn attachment_file_name(number: &str) -> String {
|
||||
format!("{}.pdf", if cleaned.is_empty() { "invoice" } else { cleaned })
|
||||
}
|
||||
|
||||
async fn attach_pdf(http: &ErpClient, l: &Loaded, remote_name: &str, pdf: &Pdf) -> Result<(), ErpError> {
|
||||
/// True when the document already carries a file of this name. Used when the local row lost its attachment
|
||||
/// hash (a re-adopted document), so the same PDF is not attached twice.
|
||||
async fn has_attachment(http: &ErpClient, remote_name: &str, file_name: &str) -> Result<bool, ErpError> {
|
||||
let rows = http
|
||||
.list_resource(
|
||||
"File",
|
||||
&["name"],
|
||||
json!([
|
||||
["attached_to_doctype", "=", DOCTYPE_INVOICE],
|
||||
["attached_to_name", "=", remote_name],
|
||||
["file_name", "=", file_name]
|
||||
]),
|
||||
"creation asc",
|
||||
)
|
||||
.await?;
|
||||
Ok(!rows.is_empty())
|
||||
}
|
||||
|
||||
/// `adopted`: the document was found, not created by this push, and nothing is recorded as attached.
|
||||
async fn attach_pdf(http: &ErpClient, l: &Loaded, remote_name: &str, pdf: &Pdf, adopted: bool) -> Result<(), ErpError> {
|
||||
let file_name = attachment_file_name(&l.invoice.number);
|
||||
if adopted && has_attachment(http, remote_name, &file_name).await.unwrap_or(false) {
|
||||
return Ok(());
|
||||
}
|
||||
let fields = [
|
||||
("doctype", DOCTYPE_INVOICE.to_string()),
|
||||
("docname", remote_name.to_string()),
|
||||
@@ -754,6 +820,9 @@ fn persist(db: &Db, invoice_id: i64, prev: Option<&SyncRow>, st: &Progress, stat
|
||||
fn failure_text(step: &str, e: &ErpError) -> String {
|
||||
if step.is_empty() || matches!(e.kind, ErrorKind::Config | ErrorKind::Precondition | ErrorKind::Conflict) {
|
||||
e.to_string()
|
||||
} else if e.kind == ErrorKind::Validation && e.message.contains("cannot be a fraction") {
|
||||
// ERPNext names the row and the UOM already; say what to change in Voiced's settings.
|
||||
format!("Could not {step}: {e} Map this unit to a UOM that allows fractions (ERPNext settings, unit mapping), or use a whole quantity.")
|
||||
} else {
|
||||
format!("Could not {step}: {e}")
|
||||
}
|
||||
@@ -780,6 +849,12 @@ async fn run_push(db: &Db, http: &ErpClient, l: &Loaded, want_submit: bool, st:
|
||||
st.warnings.extend(address_warning);
|
||||
|
||||
st.step = "";
|
||||
if inv.reverse_charge && l.india_compliance && l.vendor.registered {
|
||||
st.warnings.push(
|
||||
"This invoice is marked reverse charge in Voiced. It was sent as a normal taxed invoice: India Compliance books reverse-charge sales on separate RCM tax accounts with negative tax rows, which would not match Voiced's total. Check its GST treatment in ERPNext."
|
||||
.into(),
|
||||
);
|
||||
}
|
||||
let ctx = InvoiceContext {
|
||||
invoice: inv,
|
||||
config: cfg,
|
||||
@@ -807,11 +882,25 @@ async fn run_push(db: &Db, http: &ErpClient, l: &Loaded, want_submit: bool, st:
|
||||
}
|
||||
None => {
|
||||
st.payload_hash = hash;
|
||||
if cfg.naming_mode == NamingMode::Mirror && l.india_compliance && !ic_number_ok(&inv.number) {
|
||||
return Err(pre(format!(
|
||||
"Invoice number {} is longer than 16 characters or has characters India Compliance refuses (letters, digits, - and / only), so ERPNext would reject it. Use the ERPNext series naming mode, or start a new Voiced series with a shorter prefix.",
|
||||
inv.number
|
||||
)));
|
||||
}
|
||||
st.step = "create the Sales Invoice";
|
||||
let doc = create_or_find(http, l, &built).await?;
|
||||
st.remote_name = doc.name;
|
||||
st.remote_name = doc.name.clone();
|
||||
st.remote_docstatus = doc.docstatus;
|
||||
st.created = doc.created;
|
||||
// The totals must agree before anything else happens (no PDF, never submitted); the row keeps the
|
||||
// remote name, so the draft is found again after it is fixed or deleted.
|
||||
if let Some(theirs) = doc.total_paise {
|
||||
let ours = gst::rupees_to_paise(inv.total);
|
||||
if theirs != ours {
|
||||
return Err(total_conflict(&doc.name, &inv.number, theirs, ours));
|
||||
}
|
||||
}
|
||||
// Keep the remote name even if the next steps fail.
|
||||
persist(db, inv.id, prev, st, "synced", "")?;
|
||||
}
|
||||
@@ -834,7 +923,8 @@ async fn run_push(db: &Db, http: &ErpClient, l: &Loaded, want_submit: bool, st:
|
||||
}
|
||||
if let Some(pdf) = pdf {
|
||||
st.step = "attach the PDF";
|
||||
match attach_pdf(http, l, &st.remote_name.clone(), pdf).await {
|
||||
let adopted = !st.created && st.attachment_sha256.is_empty();
|
||||
match attach_pdf(http, l, &st.remote_name.clone(), pdf, adopted).await {
|
||||
Ok(()) => st.attachment_sha256 = pdf.sha256.clone(),
|
||||
// The invoice itself is in ERPNext; a failed upload is a warning, retried by the next push.
|
||||
Err(e) => st.warnings.push(format!("The PDF was not attached: {e}")),
|
||||
@@ -924,9 +1014,13 @@ pub struct PaymentEntryInput<'a> {
|
||||
|
||||
/// Turns the unsaved dict from `get_payment_entry` into the Payment Entry to insert and submit.
|
||||
///
|
||||
/// UNVERIFIED against a live ERPNext (check in F4): the deduction row fields (`account`, `cost_center`,
|
||||
/// `amount`), the sign ERPNext expects for a TDS deduction on a receipt, and whether `allocated_amount` must be
|
||||
/// cash plus TDS for the difference amount to come out zero. Everything that depends on those guesses is here.
|
||||
/// Verified against a live ERPNext v15.121.6 (F4; see scripts/erpnext-e2e): `bank_account` in the
|
||||
/// `get_payment_entry` query is the *Account* name and ends up as `paid_to` (the Payment Entry's own
|
||||
/// `bank_account` link, a Bank Account document, stays empty). A Payment Entry that settles an invoice with TDS has
|
||||
/// `paid_amount = received_amount = cash`, the invoice reference's `allocated_amount = cash + TDS`, and one
|
||||
/// `deductions` row `{account: TDS receivable, cost_center, amount: +TDS}` (a positive amount); then
|
||||
/// `difference_amount` is 0, `total_allocated_amount = cash + TDS` and the invoice's outstanding drops by
|
||||
/// cash + TDS. A partial payment without TDS leaves the rest outstanding.
|
||||
pub fn build_payment_entry(draft: &Value, p: &PaymentEntryInput) -> Result<Value, String> {
|
||||
let mut doc: Map<String, Value> = draft.as_object().cloned().ok_or("ERPNext returned an unexpected payment draft.")?;
|
||||
doc.retain(|k, _| !k.starts_with("__"));
|
||||
@@ -1043,6 +1137,12 @@ pub async fn push_payment(db: &Db, http: &ErpClient, payment_id: i64) -> Payment
|
||||
)))
|
||||
}
|
||||
};
|
||||
if row.amount_paise <= 0 {
|
||||
// Live finding: ERPNext answers a zero paid amount with a bare "Paid Amount is mandatory".
|
||||
return fail(pre(
|
||||
"This payment records TDS only, with no cash received. ERPNext's Payment Entry needs a paid amount above zero, so it was not sent: book the TDS in ERPNext (for example as a Journal Entry), or send the payment once the cash is recorded.",
|
||||
));
|
||||
}
|
||||
if cfg.payment_bank_account.trim().is_empty() {
|
||||
return fail(pre("Set the payment bank account in the ERPNext settings first."));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user