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 <noreply@anthropic.com>
This commit is contained in:
Stefan
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
c741535aec
commit
0a8384ab33
@@ -0,0 +1,62 @@
|
||||
using System;
|
||||
using Fuchs.Services;
|
||||
using Xunit;
|
||||
|
||||
namespace Fuchs.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
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());
|
||||
}
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
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<float>(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<decimal>(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<float>(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));
|
||||
}
|
||||
}
|
||||
@@ -151,6 +151,27 @@ public class InvoiceService : IInvoiceService
|
||||
return blocks;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The header save batch for <see cref="RegisterInvoiceAsync"/>. If the create/set proc fails
|
||||
/// (e.g. a parameter conversion error) the batch would otherwise carry on with a NULL
|
||||
/// <c>@Id</c> and fail again in <c>fds__createInvoice_Details</c> ("Cannot insert NULL into
|
||||
/// InvId"), burying the real error; the guard stops the batch right after the first failure.
|
||||
/// </summary>
|
||||
internal static string BuildRegisterInvoiceSql(bool change, bool hasProvisionLocation)
|
||||
{
|
||||
var sqlParts = new List<string> { "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<FdsInvoiceData> RegisterInvoiceAsync(FdsInvoiceData invoice, bool change, string invId,
|
||||
string userAccountId, DatabaseSecurity dbSec)
|
||||
{
|
||||
@@ -160,26 +181,13 @@ public class InvoiceService : IInvoiceService
|
||||
var pl = new List<SqlParameter> { SQL_VarChar("@authuser", userAccountId) };
|
||||
pl.AddRange(invoice.BuildInvoiceParams(change, invId));
|
||||
|
||||
var sqlParts = new List<string> { "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("<!--", emptyIfNotFound: false)));
|
||||
sqlParts.Add("UPDATE [dbo].[fds__invoices] SET [ProvisionLocation] = LEFT(@ProvisionLocation,1000) WHERE [Id] = @Id AND [isFinal] = 0;");
|
||||
}
|
||||
|
||||
var invdset = await getSQLDataSet_async(string.Join("\n", sqlParts),
|
||||
var invdset = await getSQLDataSet_async(BuildRegisterInvoiceSql(change, hasProvisionLocation),
|
||||
Conn, pl, tablenames: new[] { "inv", "det" },
|
||||
Security: dbSec, options: new FIS_SQLOptions());
|
||||
if (!string.IsNullOrEmpty(invdset.Exception))
|
||||
|
||||
+1
-1
Submodule OCORE updated: def034e155...00a7923d05
@@ -0,0 +1,29 @@
|
||||
-- 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.
|
||||
--
|
||||
-- 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.
|
||||
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
|
||||
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;
|
||||
Reference in New Issue
Block a user