From 0a8384ab33bbfb05c6578f7b9faa1aaace2c59ae Mon Sep 17 00:00:00 2001 From: Stefan Date: Thu, 1 Oct 2026 10:07:13 +0200 Subject: [PATCH] fix(invoices): Stornorechnung save failed on negative totals Customer log: /do/inv/dsave failed with "Error converting data type float to numeric" followed by "Cannot insert the value NULL into column 'InvId'". - Bump OCORE: SQL_Float/SQL_Decimal string overloads parsed negatives via the de-DE server culture ("-19.95" -> -1995); float noise overflowed numeric(10,3), so fds__setInvoice never ran. - InvoiceService: the header save batch now stops with a clear THROW when fds__createInvoice/fds__setInvoice yields no @Id, instead of failing again in fds__createInvoice_Details and burying the real error. Batch SQL extracted to BuildRegisterInvoiceSql for testing. - Tests: signed/culture parsing under de-DE, Storno param build against the proc numeric limits, and the batch guard ordering. - hlp: read-only script to find invoice/reminder amounts stored scaled up by the bug. Co-Authored-By: Claude Opus 4.8 --- Fuchs.Tests/InvoiceRegisterSqlTests.cs | 62 ++++++++++ Fuchs.Tests/SqlNumberParameterTests.cs | 107 ++++++++++++++++++ Fuchs/Services/InvoiceService.cs | 42 ++++--- OCORE | 2 +- ...6-10-01 check negative invoice amounts.sql | 29 +++++ 5 files changed, 224 insertions(+), 18 deletions(-) create mode 100644 Fuchs.Tests/InvoiceRegisterSqlTests.cs create mode 100644 Fuchs.Tests/SqlNumberParameterTests.cs create mode 100644 hlp/2026-10-01 check negative invoice amounts.sql diff --git a/Fuchs.Tests/InvoiceRegisterSqlTests.cs b/Fuchs.Tests/InvoiceRegisterSqlTests.cs new file mode 100644 index 0000000..44a5012 --- /dev/null +++ b/Fuchs.Tests/InvoiceRegisterSqlTests.cs @@ -0,0 +1,62 @@ +using System; +using Fuchs.Services; +using Xunit; + +namespace Fuchs.Tests; + +/// +/// The invoice header save batch must stop as soon as fds__createInvoice / fds__setInvoice fails: +/// previously a parameter conversion error left @Id NULL and the batch went on to +/// fds__createInvoice_Details, whose "Cannot insert NULL into InvId" buried the real error. +/// +public class InvoiceRegisterSqlTests +{ + [Theory] + [InlineData(false, "[dbo].[fds__createInvoice] ", "fds__createInvoice did not return")] + [InlineData(true, "[dbo].[fds__setInvoice] ", "fds__setInvoice did not return")] + public void BuildRegisterInvoiceSql_GuardSitsBetweenHeaderProcAndDetails(bool change, string headerProc, string guardText) + { + string sql = InvoiceService.BuildRegisterInvoiceSql(change, hasProvisionLocation: true); + + int header = sql.IndexOf(headerProc, StringComparison.Ordinal); + int guard = sql.IndexOf("IF @Id IS NULL THROW", StringComparison.Ordinal); + int details = sql.IndexOf("[dbo].[fds__createInvoice_Details]", StringComparison.Ordinal); + int update = sql.IndexOf("UPDATE [dbo].[fds__invoices]", StringComparison.Ordinal); + + Assert.True(header >= 0, "header proc missing"); + Assert.True(header < guard && guard < details && details < update, sql); + Assert.Contains(guardText, sql); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void BuildRegisterInvoiceSql_ExactlyOneHeaderProcPerMode(bool change) + { + string sql = InvoiceService.BuildRegisterInvoiceSql(change, hasProvisionLocation: false); + + Assert.Equal(change, sql.Contains("[dbo].[fds__setInvoice] @InvId,")); + Assert.Equal(!change, sql.Contains("[dbo].[fds__createInvoice] @InvoiceType,")); + Assert.Equal(change, sql.Contains("@InvId")); + } + + [Theory] + [InlineData(false, false)] + [InlineData(true, true)] + public void BuildRegisterInvoiceSql_ProvisionLocationUpdateOnlyWhenPresent(bool hasProvisionLocation, bool expected) + { + string sql = InvoiceService.BuildRegisterInvoiceSql(change: true, hasProvisionLocation); + + Assert.Equal(expected, sql.Contains("@ProvisionLocation")); + } + + [Fact] + public void BuildRegisterInvoiceSql_EveryStatementIsSemicolonTerminated() + { + // THROW requires the preceding statement to be terminated with ';'. + string sql = InvoiceService.BuildRegisterInvoiceSql(change: true, hasProvisionLocation: true); + + foreach (var line in sql.Split('\n')) + Assert.EndsWith(";", line.TrimEnd()); + } +} diff --git a/Fuchs.Tests/SqlNumberParameterTests.cs b/Fuchs.Tests/SqlNumberParameterTests.cs new file mode 100644 index 0000000..32576dc --- /dev/null +++ b/Fuchs.Tests/SqlNumberParameterTests.cs @@ -0,0 +1,107 @@ +using System; +using System.Globalization; +using System.Linq; +using Fuchs.intranet; +using Newtonsoft.Json.Linq; +using Xunit; +using static OCORE.SQL.sql; + +namespace Fuchs.Tests; + +/// +/// Regression tests for the OCORE string → number SQL parameter helpers. Saving a Stornorechnung +/// failed in production with "Error converting data type float to numeric": the negative totals +/// did not parse in the first (sign-less) attempt and fell back to the server's de-DE culture, +/// where "." is a group separator — "-19.95" became -1995 and JS float noise +/// ("-1234.5600000000002") overflowed numeric(10,3), so fds__setInvoice never ran. +/// Every test runs under de-DE, the production server culture. +/// +public class SqlNumberParameterTests : IDisposable +{ + private readonly CultureInfo _culture = CultureInfo.CurrentCulture; + + public SqlNumberParameterTests() => CultureInfo.CurrentCulture = new CultureInfo("de-DE"); + + public void Dispose() => CultureInfo.CurrentCulture = _culture; + + [Theory] + [InlineData("-19.95", -19.95)] + [InlineData("-1234.56", -1234.56)] + [InlineData("-1234.5600000000002", -1234.56)] + [InlineData("-0.5", -0.5)] + [InlineData("1234.56", 1234.56)] + [InlineData("1234.5600000000002", 1234.56)] + [InlineData("-1,5", -1.5)] + [InlineData("-1.234,56", -1234.56)] + [InlineData("1.234,56", 1234.56)] + [InlineData("1,234.56", 1234.56)] + [InlineData(" -42 ", -42)] + [InlineData("-1.5E-05", -0.000015)] + [InlineData("0", 0)] + public void SQL_Float_StringValue_ParsesSignedAndCultureFormattedNumbers(string input, double expected) + { + var p = SQL_Float("@v", stringvalue: input); + + var value = Assert.IsType(p.Value); + Assert.Equal(expected, value, 0.01); + } + + [Theory] + [InlineData("-19.95", -19.95)] + [InlineData("-1234.56", -1234.56)] + [InlineData("-1.234,56", -1234.56)] + [InlineData("1.234,56", 1234.56)] + [InlineData("-1,5", -1.5)] + public void SQL_Decimal_StringValue_ParsesSignedAndCultureFormattedNumbers(string input, double expected) + { + var p = SQL_Decimal("@v", input, precision: 10, scale: 3); + + var value = Assert.IsType(p.Value); + Assert.Equal((decimal)expected, value); + } + + [Theory] + [InlineData("")] + [InlineData("NaN")] + [InlineData("abc")] + [InlineData("--5")] + public void SQL_Float_StringValue_InvalidOrEmpty_IsDbNull(string input) + { + var p = SQL_Float("@v", stringvalue: input); + + Assert.Equal(DBNull.Value, p.Value); + } + + [Theory] + [InlineData("")] + [InlineData("NaN")] + [InlineData("abc")] + public void SQL_Decimal_StringValue_InvalidOrEmpty_IsDbNull(string input) + { + var p = SQL_Decimal("@v", input); + + 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/Services/InvoiceService.cs b/Fuchs/Services/InvoiceService.cs index 1bd8da3..a67c111 100644 --- a/Fuchs/Services/InvoiceService.cs +++ b/Fuchs/Services/InvoiceService.cs @@ -151,6 +151,27 @@ public class InvoiceService : IInvoiceService return blocks; } + /// + /// The header save batch for . If the create/set proc fails + /// (e.g. a parameter conversion error) the batch would otherwise carry on with a NULL + /// @Id and fail again in fds__createInvoice_Details ("Cannot insert NULL into + /// InvId"), burying the real error; the guard stops the batch right after the first failure. + /// + internal static string BuildRegisterInvoiceSql(bool change, bool hasProvisionLocation) + { + var sqlParts = new List { "DECLARE @Id varchar(10);" }; + sqlParts.Add(change + ? "EXECUTE [dbo].[fds__setInvoice] @InvId, @InvoiceType, @InvoiceTitle, @InvoiceBalance, @InvoiceBalance_net, @InvoiceVAT_net1, @InvoiceVAT_1, @PaymentTerm, @CustomerId, @SendToAddress, @SendToEmail, @SendToEmailCc, @ProvisionPeriod, @CustomValues, @authuser, @Id OUTPUT, @SendToAddressJson;" + : "EXECUTE [dbo].[fds__createInvoice] @InvoiceType, @InvoiceTitle, @InvoiceBalance, @InvoiceBalance_net, @InvoiceVAT_net1, @InvoiceVAT_1, @PaymentTerm, @CustomerId, @SendToAddress, @SendToEmail, @SendToEmailCc, @ProvisionPeriod, @CustomValues, @authuser, @Id OUTPUT, @SendToAddressJson;"); + sqlParts.Add(change + ? "IF @Id IS NULL THROW 50001, N'fds__setInvoice did not return an invoice id - invoice header not saved.', 1;" + : "IF @Id IS NULL THROW 50001, N'fds__createInvoice did not return an invoice id - invoice header not saved.', 1;"); + sqlParts.Add("EXECUTE [dbo].[fds__createInvoice_Details] @Id, @InvoiceService_net, @InvoiceService_VAT, @InvoiceOptions, @authuser;"); + if (hasProvisionLocation) + sqlParts.Add("UPDATE [dbo].[fds__invoices] SET [ProvisionLocation] = LEFT(@ProvisionLocation,1000) WHERE [Id] = @Id AND [isFinal] = 0;"); + return string.Join("\n", sqlParts); + } + public async Task RegisterInvoiceAsync(FdsInvoiceData invoice, bool change, string invId, string userAccountId, DatabaseSecurity dbSec) { @@ -160,26 +181,13 @@ public class InvoiceService : IInvoiceService var pl = new List { SQL_VarChar("@authuser", userAccountId) }; pl.AddRange(invoice.BuildInvoiceParams(change, invId)); - var sqlParts = new List { "DECLARE @Id varchar(10);" }; - if (!change) - { - sqlParts.Add("EXECUTE [dbo].[fds__createInvoice] @InvoiceType, @InvoiceTitle, @InvoiceBalance, @InvoiceBalance_net, @InvoiceVAT_net1, @InvoiceVAT_1, @PaymentTerm, @CustomerId, @SendToAddress, @SendToEmail, @SendToEmailCc, @ProvisionPeriod, @CustomValues, @authuser, @Id OUTPUT, @SendToAddressJson;"); - sqlParts.Add("EXECUTE [dbo].[fds__createInvoice_Details] @Id, @InvoiceService_net, @InvoiceService_VAT, @InvoiceOptions, @authuser;"); - } - else - { - pl.Add(SQL_VarChar("@InvId", invId)); - sqlParts.Add("EXECUTE [dbo].[fds__setInvoice] @InvId, @InvoiceType, @InvoiceTitle, @InvoiceBalance, @InvoiceBalance_net, @InvoiceVAT_net1, @InvoiceVAT_1, @PaymentTerm, @CustomerId, @SendToAddress, @SendToEmail, @SendToEmailCc, @ProvisionPeriod, @CustomValues, @authuser, @Id OUTPUT, @SendToAddressJson;"); - sqlParts.Add("EXECUTE [dbo].[fds__createInvoice_Details] @Id, @InvoiceService_net, @InvoiceService_VAT, @InvoiceOptions, @authuser;"); - } - if (invoice.RawProvisionLocation.Length > 0) - { + if (change) pl.Add(SQL_VarChar("@InvId", invId)); + bool hasProvisionLocation = invoice.RawProvisionLocation.Length > 0; + if (hasProvisionLocation) pl.Add(SQL_NVarChar("@ProvisionLocation", string.Join("\n", invoice.RawProvisionLocation).LeftToFirst("