diff --git a/Fuchs.Tests/InvoiceAmountParamTests.cs b/Fuchs.Tests/InvoiceAmountParamTests.cs new file mode 100644 index 0000000..1defeb6 --- /dev/null +++ b/Fuchs.Tests/InvoiceAmountParamTests.cs @@ -0,0 +1,165 @@ +using System; +using System.Collections.Generic; +using System.Data; +using System.Globalization; +using System.Linq; +using Fuchs.intranet; +using Microsoft.Data.SqlClient; +using Newtonsoft.Json.Linq; +using Xunit; + +namespace Fuchs.Tests; + +/// +/// Invoice amounts reach the procs as exact, cent-rounded decimals, independent of the server +/// culture. Regression for the Stornorechnung save failure: the VAT amount was formatted +/// invariantly ("-615.3757999999999", unrounded double from a reloaded Storno line) and re-parsed +/// under de-DE as -6153757999999999 → "Error converting data type float to numeric". +/// +public class InvoiceAmountParamTests : IDisposable +{ + private readonly CultureInfo _culture = CultureInfo.CurrentCulture; + public void Dispose() => CultureInfo.CurrentCulture = _culture; + + private static readonly string[] AmountParams = + { "@InvoiceBalance", "@InvoiceBalance_net", "@InvoiceVAT_net1", "@InvoiceService_net", "@InvoiceService_VAT" }; + + /// The exact shape InvoiceDraftEditService.BuildFdsData produces: decimal sums, VAT map by rate. + private static FdsInvoiceData StornoDraft(object vatAmount) => new(new JObject + { + ["admin"] = new JObject { ["type"] = "c", ["customerid"] = "7" }, + ["new"] = new JObject { ["title"] = "Stornorechnung", ["total_net"] = -3238.82m, ["total_gross"] = -3854.20m }, + ["sms"] = new JObject + { + ["tscn"] = -1234.56m, ["tscvat"] = -234.57m, + ["vat"] = new JObject { ["19"] = JToken.FromObject(vatAmount) } + } + }); + + private static decimal Val(List pl, string name) + { + var p = pl.Single(x => x.ParameterName == name); + Assert.Equal(SqlDbType.Decimal, p.SqlDbType); + return Assert.IsType(p.Value); + } + + public static TheoryData CulturesAndVat => new() + { + { "de-DE", -615.38m }, + { "en-US", -615.38m }, + { "de-DE", -615.3757999999999 }, // unrounded double from a reloaded Storno line (net * vat * 0.01) + { "en-US", -615.3757999999999 }, + { "de-DE", -615.3800000000001 }, + { "", -615.38m }, // invariant + }; + + [Theory] + [MemberData(nameof(CulturesAndVat))] + public void BuildInvoiceParams_StornoDraft_AmountsExactInEveryCulture(string culture, object vatAmount) + { + CultureInfo.CurrentCulture = new CultureInfo(culture); + + var pl = StornoDraft(vatAmount).BuildInvoiceParams(change: true, invId: "abc"); + + Assert.Equal(-3854.20m, Val(pl, "@InvoiceBalance")); + Assert.Equal(-3238.82m, Val(pl, "@InvoiceBalance_net")); + Assert.Equal(-615.38m, Val(pl, "@InvoiceVAT_net1")); + Assert.Equal(-1234.56m, Val(pl, "@InvoiceService_net")); + Assert.Equal(-234.57m, Val(pl, "@InvoiceService_VAT")); + } + + [Fact] + public void BuildInvoiceParams_AmountParams_MatchProcPrecisionAndScale() + { + var pl = StornoDraft(-615.38m).BuildInvoiceParams(change: false, invId: ""); + + foreach (var name in AmountParams) + { + var p = pl.Single(x => x.ParameterName == name); + Assert.Equal(10, p.Precision); + // fds__setInvoice/fds__createInvoice: numeric(10,3); fds__createInvoice_Details: numeric(10,2). + Assert.Equal(name.StartsWith("@InvoiceService_") ? 2 : 3, p.Scale); + } + } + + [Theory] + [InlineData(-10_000_000.00)] + [InlineData(10_000_000.00)] + [InlineData(-6153757999999999.0)] + public void BuildInvoiceParams_AmountOutOfProcRange_Throws(double vat) + { + var ex = Assert.Throws(() => + StornoDraft(vat).BuildInvoiceParams(change: true, invId: "abc")); + + Assert.Equal("@InvoiceVAT_net1", ex.Parameter); + } + + [Theory] + [InlineData(9_999_999.99)] + [InlineData(-9_999_999.99)] + [InlineData(0.0)] + public void BuildInvoiceParams_AmountAtProcRangeBoundary_Accepted(double vat) + { + var pl = StornoDraft(vat).BuildInvoiceParams(change: true, invId: "abc"); + + Assert.Equal((decimal)vat, Val(pl, "@InvoiceVAT_net1")); + } + + [Fact] + public void BuildInvoiceParams_MissingAmounts_KeepPreviousNullAndZeroSemantics() + { + var noSms = new FdsInvoiceData(new JObject { ["admin"] = new JObject(), ["new"] = new JObject() }) + .BuildInvoiceParams(change: false, invId: ""); + Assert.Equal(DBNull.Value, noSms.Single(p => p.ParameterName == "@InvoiceBalance").Value); + Assert.Equal(0m, noSms.Single(p => p.ParameterName == "@InvoiceService_net").Value); // no sms block → 0 + Assert.Equal(0m, noSms.Single(p => p.ParameterName == "@InvoiceVAT_net1").Value); // HighestVat → "0" + + var emptySms = new FdsInvoiceData(new JObject { ["new"] = new JObject(), ["sms"] = new JObject() }) + .BuildInvoiceParams(change: false, invId: ""); + Assert.Equal(DBNull.Value, emptySms.Single(p => p.ParameterName == "@InvoiceService_net").Value); // key missing → NULL + + // No "new" block at all: the legacy SQL_Float(nz(...) ?? "0") sent 0, not NULL. + var noNew = new FdsInvoiceData(new JObject { ["admin"] = new JObject(), ["sms"] = new JObject() }) + .BuildInvoiceParams(change: false, invId: ""); + Assert.Equal(0m, noNew.Single(p => p.ParameterName == "@InvoiceBalance").Value); + Assert.Equal(0m, noNew.Single(p => p.ParameterName == "@InvoiceBalance_net").Value); + } + + [Theory] + [InlineData("-19.95", "-19.95")] + [InlineData("-19,95", "-19.95")] + [InlineData("-1234.5600000000002", "-1234.56")] + [InlineData("-615.3757999999999", "-615.38")] + [InlineData("1234,565", "1234.57")] // rounded half away from zero + [InlineData("-1234,565", "-1234.57")] + [InlineData("-1.5E-05", "0")] + [InlineData(" 42 ", "42")] + public void ParseAmount_Strings_ParsedCultureIndependentlyAndRoundedToCents(string raw, string expected) + { + foreach (var c in new[] { "de-DE", "en-US" }) + { + CultureInfo.CurrentCulture = new CultureInfo(c); + Assert.Equal(decimal.Parse(expected, CultureInfo.InvariantCulture), FdsInvoiceData.ParseAmount(raw)); + } + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData("NaN")] + [InlineData("abc")] + [InlineData(double.NaN)] + [InlineData(double.PositiveInfinity)] + public void ParseAmount_EmptyOrInvalid_IsNull(object? raw) => + Assert.Null(FdsInvoiceData.ParseAmount(raw)); + + [Fact] + public void ParseAmount_NumbersAndJValues_TakenAsIs() + { + Assert.Equal(-3238.82m, FdsInvoiceData.ParseAmount(-3238.82m)); + Assert.Equal(-3238.82m, FdsInvoiceData.ParseAmount(-3238.82)); + Assert.Equal(-12m, FdsInvoiceData.ParseAmount(-12)); + Assert.Equal(-3238.82m, FdsInvoiceData.ParseAmount(new JValue(-3238.82m))); + Assert.Null(FdsInvoiceData.ParseAmount(JValue.CreateNull())); + } +} diff --git a/Fuchs.Tests/SqlNumberParameterTests.cs b/Fuchs.Tests/SqlNumberParameterTests.cs index 32576dc..359cdd6 100644 --- a/Fuchs.Tests/SqlNumberParameterTests.cs +++ b/Fuchs.Tests/SqlNumberParameterTests.cs @@ -83,25 +83,4 @@ public class SqlNumberParameterTests : IDisposable Assert.Equal(DBNull.Value, p.Value); } - [Fact] - public void BuildInvoiceParams_StornoNegativeTotals_KeepsTheirValueAndFitTheProcNumerics() - { - // Shape of a Stornorechnung post: negated totals with JS float noise. - var jo = JObject.Parse( - "{ \"admin\": { \"type\": \"S\", \"customerid\": \"7\" }, " + - " \"new\": { \"title\": \"Stornorechnung\", \"total_gross\": \"-1469.1400000000003\", \"total_net\": \"-1234.5600000000002\" }, " + - " \"sms\": { \"vat\": { \"19,0%\": -234.58000000000004 }, \"tscn\": \"-1234.56\", \"tscvat\": \"-234.58\" } }"); - - var pl = new FdsInvoiceData(jo).BuildInvoiceParams(change: true, invId: "abc"); - double Val(string n) => Convert.ToDouble(Assert.IsType(pl.Single(p => p.ParameterName == n).Value)); - - Assert.Equal(-1469.14, Val("@InvoiceBalance"), 0.01); - Assert.Equal(-1234.56, Val("@InvoiceBalance_net"), 0.01); - Assert.Equal(-234.58, Val("@InvoiceVAT_net1"), 0.01); - Assert.Equal(-1234.56, Val("@InvoiceService_net"), 0.01); - Assert.Equal(-234.58, Val("@InvoiceService_VAT"), 0.01); - // numeric(10,3) / numeric(10,2) in fds__setInvoice / fds__createInvoice_Details. - Assert.All(new[] { "@InvoiceBalance", "@InvoiceBalance_net", "@InvoiceVAT_net1", "@InvoiceService_net", "@InvoiceService_VAT" }, - n => Assert.InRange(Math.Abs(Val(n)), 0, 9_999_999.99)); - } } diff --git a/Fuchs/Controllers/IntranetController.Invoices2.cs b/Fuchs/Controllers/IntranetController.Invoices2.cs index 940642e..4353f09 100644 --- a/Fuchs/Controllers/IntranetController.Invoices2.cs +++ b/Fuchs/Controllers/IntranetController.Invoices2.cs @@ -475,7 +475,9 @@ public partial class IntranetController { ["Id"] = d["Id"], ["net_val"] = net, - ["vat_val"] = net * vat * 0.01, + // Cent-rounded in decimal like InvoiceDraftCalculator; the former double product + // (e.g. -615.3757999999999) carried float noise into the draft's VAT sums. + ["vat_val"] = Math.Round((decimal)net * (decimal)vat / 100m, 2, MidpointRounding.AwayFromZero), ["vat"] = vat == 0 ? "" : vat.ToString("0.00", FuchsPdf.DeCulture) + "%", ["svcnet_val"] = d.no("value_service", 0), ["net"] = d.no("value", 0), diff --git a/Fuchs/Fuchs.csproj b/Fuchs/Fuchs.csproj index 0856a22..aa993d3 100644 --- a/Fuchs/Fuchs.csproj +++ b/Fuchs/Fuchs.csproj @@ -54,7 +54,7 @@ - + diff --git a/Fuchs/Services/InvoiceService.cs b/Fuchs/Services/InvoiceService.cs index a67c111..47e0f7f 100644 --- a/Fuchs/Services/InvoiceService.cs +++ b/Fuchs/Services/InvoiceService.cs @@ -70,6 +70,8 @@ public class InvoiceService : IInvoiceService private string Conn => _intranet.Intranet__SQLConnectionString; + private static readonly EventId InvoiceAmountRejected = new(0, "InvoiceAmountRejected"); + public async Task LoadInvoiceAsync(string id, string userAccountId, DatabaseSecurity dbSec) { _logger.LogDebug("LoadInvoiceAsync id={Id} user={User}", id, userAccountId); @@ -179,7 +181,18 @@ public class InvoiceService : IInvoiceService if (invoice.NewValues == null) { _logger.LogWarning("RegisterInvoiceAsync: no form values supplied (user={User})", userAccountId); return invoice; } var pl = new List { SQL_VarChar("@authuser", userAccountId) }; - pl.AddRange(invoice.BuildInvoiceParams(change, invId)); + try + { + pl.AddRange(invoice.BuildInvoiceParams(change, invId)); + } + catch (InvoiceAmountOutOfRangeException ex) + { + // Refuse rather than store a wrong amount; an empty registration makes the caller report the save as failed. + _logger.LogError(InvoiceAmountRejected, ex, + "RegisterInvoiceAsync: refused invoice {InvId} - amount {Parameter} = {Value} is out of range (user={User})", + invId, ex.Parameter, ex.Value, userAccountId); + return invoice; + } if (change) pl.Add(SQL_VarChar("@InvId", invId)); bool hasProvisionLocation = invoice.RawProvisionLocation.Length > 0; diff --git a/Fuchs/code/FdsInvoiceData.cs b/Fuchs/code/FdsInvoiceData.cs index 7a35455..16b0297 100644 --- a/Fuchs/code/FdsInvoiceData.cs +++ b/Fuchs/code/FdsInvoiceData.cs @@ -173,9 +173,9 @@ public class FdsInvoiceData { SQL_VarChar("@InvoiceType", Admin?.nz("type") ?? ""), SQL_NVarChar("@InvoiceTitle", NewValues?.nz("title") ?? ""), - SQL_Float("@InvoiceBalance", stringvalue: NewValues?.nz("total_gross") ?? "0"), - SQL_Float("@InvoiceBalance_net", stringvalue: NewValues?.nz("total_net") ?? "0"), - SQL_Float("@InvoiceVAT_net1", stringvalue: vatNet), + AmountParam("@InvoiceBalance", NewValues is null ? 0m : NewValues.getItem("total_gross"), scale: 3), + AmountParam("@InvoiceBalance_net", NewValues is null ? 0m : NewValues.getItem("total_net"), scale: 3), + AmountParam("@InvoiceVAT_net1", vatNet, scale: 3), SQL_VarChar("@InvoiceVAT_1", vatRate), SQL_VarChar("@PaymentTerm", NewValues?.nz("paymentterm") ?? "", dbNull_IfEmpty: true), SQL_BigInt("@CustomerId", Admin?.nz("customerid") ?? ""), @@ -185,12 +185,63 @@ public class FdsInvoiceData SQL_NVarChar("@ProvisionPeriod", RawProvisionPeriod, dbNull_IfEmpty: true), SQL_NVarChar("@CustomValues", RawCustomValues, dbNull_IfEmpty: true), SQL_NVarChar("@SendToAddressJson", SendToAddressJsonForPersist(), dbNull_IfEmpty: true), - SQL_Float("@InvoiceService_net", stringvalue: Sms?.nz("tscn") ?? "0"), - SQL_Float("@InvoiceService_VAT", stringvalue: Sms?.nz("tscvat") ?? "0"), + AmountParam("@InvoiceService_net", Sms is null ? 0m : Sms.getItem("tscn"), scale: 2), + AmountParam("@InvoiceService_VAT", Sms is null ? 0m : Sms.getItem("tscvat"), scale: 2), SQL_VarChar("@InvoiceOptions", BuildInvoiceOptions(), dbNull_IfEmpty: true) }; } + /// Largest magnitude the numeric(10,x) amount parameters of the invoice procs accept. + internal const decimal MaxAmount = 9_999_999.99m; + + /// + /// Invoice amounts go to SQL as exact decimal parameters rounded to cents — never through + /// a culture-dependent string → float round trip. That round trip once parsed negative amounts + /// with the server's de-DE culture ("-615.3758" → -6153758, with float noise beyond numeric(10,3)), + /// so Stornorechnungen could not be saved. An amount outside the procs' range is refused here + /// () instead of being stored wrongly or failing in SQL. + /// + internal static SqlParameter AmountParam(string name, object? raw, byte scale) + { + decimal? value = ParseAmount(raw); + if (value is { } v && Math.Abs(v) > MaxAmount) + throw new InvoiceAmountOutOfRangeException(name, v); + return new SqlParameter(name, System.Data.SqlDbType.Decimal) + { + Precision = 10, + Scale = scale, + Value = value.HasValue ? value.Value : DBNull.Value + }; + } + + /// + /// Reads an amount the draft session or editor supplied: numbers are taken as-is; strings may be + /// German ("-1234,56") or invariant ("-1234.56", "-1.5E-05") — a comma marks German formatting. + /// The result is rounded to cents. Empty / "NaN" / unparseable → null (stored as NULL). + /// + internal static decimal? ParseAmount(object? raw) + { + if (raw is JValue jv) raw = jv.Value; + decimal d; + switch (raw) + { + case null: return null; + case decimal m: d = m; break; + case double dbl when double.IsFinite(dbl): d = (decimal)dbl; break; + case float flt when float.IsFinite(flt): d = (decimal)flt; break; + case int or long or short or byte: d = Convert.ToDecimal(raw, CultureInfo.InvariantCulture); break; + case double or float: return null; + default: + string s = (Convert.ToString(raw, CultureInfo.InvariantCulture) ?? "").Trim(); + var culture = s.Contains(',') ? GermanCulture : CultureInfo.InvariantCulture; + if (!decimal.TryParse(s, NumberStyles.Float, culture, out d)) return null; + break; + } + return Math.Round(d, 2, MidpointRounding.AwayFromZero); + } + + private static readonly CultureInfo GermanCulture = new("de-DE"); + /// /// Builds the InvoiceOptions CSV from the posted admin flags. /// §13b is the existing reverse-charge flag; setmode:<mode> @@ -253,6 +304,14 @@ public class FdsInvoiceData } } +/// An invoice amount exceeds what the invoice procs' numeric(10,x) parameters can store. +public sealed class InvoiceAmountOutOfRangeException(string parameter, decimal value) + : Exception($"Invoice amount {parameter} = {value.ToString(CultureInfo.InvariantCulture)} is outside ±{FdsInvoiceData.MaxAmount.ToString(CultureInfo.InvariantCulture)}.") +{ + public string Parameter { get; } = parameter; + public decimal Value { get; } = value; +} + /// A service-request group as it renders on the invoice: a heading plus its line items. public sealed class InvoiceBlock { diff --git a/OCORE b/OCORE index 00a7923..728ab39 160000 --- a/OCORE +++ b/OCORE @@ -1 +1 @@ -Subproject commit 00a7923d0532085b2f0a244b446d4fb61e5f211e +Subproject commit 728ab395c7445122361f823a428bcd0a7f226f48 diff --git a/OCORE_web b/OCORE_web index edf50f1..bc4e534 160000 --- a/OCORE_web +++ b/OCORE_web @@ -1 +1 @@ -Subproject commit edf50f1e91ec5e5ddf071e25d650da2d5a11a8f1 +Subproject commit bc4e534e1b1b964d0d0d97e8ec9d8be316eecabe diff --git a/hlp/2026-10-01 check negative invoice amounts.sql b/hlp/2026-10-01 check negative invoice amounts.sql index f92fc82..64d3f05 100644 --- a/hlp/2026-10-01 check negative invoice amounts.sql +++ b/hlp/2026-10-01 check negative invoice amounts.sql @@ -1,29 +1,22 @@ --- Read-only check for amounts mangled by the OCORE SQL_Float sign bug (fixed 2026-10-01). --- Before the fix, a negative amount posted as "-19.95" was parsed with the server's de-DE culture --- ("." = thousands separator) and stored scaled up, e.g. -19.95 -> -1995. Positive amounts were fine. --- Affected: fds__invoices header totals (Stornos / credits) and fds__invoice_details service totals --- written through fds__setInvoice / fds__createInvoice / fds__createInvoice_Details, plus reminder --- amounts written by ReminderService. Line items (fds__invoice_items) were NOT affected. +-- Read-only check for amounts mangled by the SQL_Float culture bug (fixed 2026-10-01). +-- No corrections are to be made from this: issued invoices are not changed retrospectively. -- --- Signal: the header net total disagrees with the sum of the invoice's own line items. - --- 1) Invoices with a negative header total, compared against their persisted line items. +-- What was at risk: only the VAT amount of a negative invoice (Storno / Gutschrift / Korrektur). +-- FdsInvoiceData.HighestVat formatted it invariantly ("-615.38"), and the old OCORE SQL_Float parsed a +-- negative value with the server's de-DE culture ("." = thousands separator) -> -61538. When the VAT +-- carried float noise ("-615.3757999999999") the result overflowed numeric(10,3) and the save failed +-- (the reported error); without noise it would have been stored scaled by 10/100/1000. +-- The header totals (InvoiceBalance / _net) and service totals were formatted and parsed in the same +-- culture and were never affected. Negative amounts themselves are legitimate. +-- +-- Bug signature: InvoiceVAT_net1 = (InvoiceBalance - InvoiceBalance_net) * 10 / 100 / 1000. SELECT i.[Id], i.[InvoiceId], i.[InvoiceType], i.[IsFinal], i.[DateCreated], i.[DateModified], i.[InvoiceBalance], i.[InvoiceBalance_net], i.[InvoiceVAT_net1], - d.[InvoiceService_net], d.[InvoiceService_VAT], - items_net = it.[items_net], - suspicious = CASE WHEN it.[items_net] IS NOT NULL - AND ABS(i.[InvoiceBalance_net] - it.[items_net]) > 0.05 THEN 1 ELSE 0 END + vat_from_totals = i.[InvoiceBalance] - i.[InvoiceBalance_net], + vat_scaled_by_bug = CASE WHEN EXISTS (SELECT 1 FROM (VALUES (10), (100), (1000)) f(n) + WHERE i.[InvoiceBalance] - i.[InvoiceBalance_net] <> 0 + AND ABS(i.[InvoiceVAT_net1] - (i.[InvoiceBalance] - i.[InvoiceBalance_net]) * f.n) <= 0.05 * f.n) + THEN 1 ELSE 0 END FROM [dbo].[fds__invoices] i -LEFT JOIN [dbo].[fds__invoice_details] d ON d.[InvId] = i.[Id] -OUTER APPLY (SELECT [items_net] = SUM(x.[value_total]) - FROM [dbo].[fds__invoice_items] x - WHERE x.[InvId] = i.[Id]) it -WHERE i.[InvoiceBalance] < 0 OR i.[InvoiceBalance_net] < 0 - OR d.[InvoiceService_net] < 0 OR d.[InvoiceService_VAT] < 0 -ORDER BY suspicious DESC, i.[DateModified] DESC; - --- 2) Reminders with a negative amount (same helper). -SELECT * -FROM [dbo].[fds__reminder] -WHERE [amount] < 0 OR [amount_payed] < 0; +WHERE i.[InvoiceBalance] < 0 OR i.[InvoiceBalance_net] < 0 OR i.[InvoiceVAT_net1] < 0 +ORDER BY vat_scaled_by_bug DESC, i.[DateModified] DESC;