Stefan
c741535aec
Playwright Tests / test (push) Canceled after 0s
- Added a new SQL script to address the "String or binary data would be truncated" error in mfr sync by widening the Note fields in service objects and companies to NVARCHAR(MAX). - Updated the table types and associated stored procedures to accommodate the changes, ensuring compatibility with existing data. - Queued a re-sync for previously rejected rows due to truncation issues. - Created application logins for fuchs_fds and fuchs_web with placeholder passwords for initial setup. - Introduced functions for invoice backdating and invoice date retrieval to streamline invoice processing. - Developed a password generation function with customizable parameters for length and character types. - Added a random number generation function to support password creation.
86 lines
5.0 KiB
Markdown
86 lines
5.0 KiB
Markdown
---
|
|
status: Accepted
|
|
date: 2026-09-30
|
|
applyTo:
|
|
- "Fuchs_Database/**"
|
|
supersededBy: ""
|
|
---
|
|
|
|
# 0036 — The database project build is the T-SQL gate
|
|
|
|
## Context
|
|
|
|
A rewrite of `fds__getDatevExports` added a `JOIN @inv as i` next to `fds__invoices as iv`
|
|
and left an unqualified `[file_guid]` in the select list. SQL Server rejects that statement
|
|
with Msg 209 (ambiguous column name) — but only when it runs, and the working-tree SQL runs
|
|
nowhere: the schema lives in the `Fuchs_Database` SSDT project, the dev database only gets
|
|
what is explicitly published to it, and the xUnit suite cannot bind T-SQL. The defect reached
|
|
code review as the only check.
|
|
|
|
The SSDT build would have caught it: it reported the line as `SQL71502` ("unresolved reference
|
|
… or ambiguous"). Nobody saw that because
|
|
|
|
- the project did not build at all — the two `CREATE USER … FOR LOGIN` files referenced logins
|
|
that were not in the model, which SSDT treats as error `SQL71501`;
|
|
- the genuine warning sat among ~160 identical `SQL71502` warnings caused by references to
|
|
`sys.*`, `INFORMATION_SCHEMA`, `sp_executesql`, `sp_getapplock` (no `master` reference) and
|
|
to `ocms_fn_generatePassword`, an OCMS helper the project never contained;
|
|
- a warning does not fail anything.
|
|
|
|
The same sweep found that `ocms_fn_generatePassword` (and its dependency `ocms_fn_randbetween`)
|
|
is missing on the dev database as well, so every id-generating function (`fds__fn_invoice_id`,
|
|
`fds__fn_reminder_id`, `fis_fn_id`, …) fails there with Msg 4121.
|
|
|
|
## Decision
|
|
|
|
1. `FuchsDatabase.sqlproj` references the `master` system dacpac
|
|
(`$(DacPacRootPath)\…\SqlServer\170\SQLSchemas\master.dacpac`), so system objects bind.
|
|
2. Everything the SQL references is in the project. The OCMS helpers `ocms_fn_generatePassword`
|
|
and `ocms_fn_randbetween` are vendored copies of the OCMSsharp `ocms_mt_database` files.
|
|
`Security/Logins.sql` scripts the two application logins with a placeholder,
|
|
`MUST_CHANGE` password so the users bind; real passwords stay in Key Vault and SSDT never
|
|
compares passwords, so existing logins are untouched on publish.
|
|
3. `TreatTSqlWarningsAsErrors` is on. Suppressed: `71501` (dynamic SQL / cross-database
|
|
references) and `71558` (identifier differs only by case — harmless under the `CI` model
|
|
collation). Every other T-SQL warning, in particular `SQL71502`, fails the build.
|
|
4. Building the database project is part of the workflow whenever `Fuchs_Database/**` changes
|
|
(`msbuild Fuchs_Database/FuchsDatabase.sqlproj -p:Configuration=Debug`, from the Visual
|
|
Studio MSBuild — `dotnet build` cannot build the legacy SSDT project format). A red database
|
|
build is a failure like a red C# build.
|
|
5. The C# side of the contract is a test: `Fuchs.Tests/SqlContractTests` scans every
|
|
`[dbo].[…]` named in C# strings (`Fuchs`, `Fuchs_DataService`) and checks it against the
|
|
SSDT files — the object exists, every `EXECUTE` binds (no unknown names, no surplus
|
|
positional arguments, every parameter without a default supplied), every function call
|
|
passes the declared argument count, and every `mfr__updt__*` proc declares the
|
|
`@tblname, @referencetable, @tgtid` signature `FdsMfrClient` calls dynamically.
|
|
|
|
## Consequences
|
|
|
|
- An unqualified column that becomes ambiguous, a typo in a column name, or a call to a
|
|
function that is not in the project is a build error before review, not a Msg 209/4121 at
|
|
a customer.
|
|
- `SELECT *` inside `EXISTS (…)` over joined tables trips `SQL71502` in SSDT even though SQL
|
|
Server accepts it; write `SELECT 1` there (done for `fds__fn_IntermediateIsAllocatedToOther`
|
|
and `fis_admin_testGroupMembership`).
|
|
- New procedures that use `sys.*`/`sp_*` need no suppression any more. A new reference that is
|
|
genuinely outside the model (another database, dynamic SQL) is suppressed per statement or
|
|
by adding the object to the project — never by widening `SuppressTSqlWarnings` for `71502`.
|
|
- A proc signature change that breaks a C# caller fails `dotnet test`, not the first request.
|
|
Only literal SQL is covered: dynamically built names (`FuchsVisualization`'s report procs,
|
|
resolved from `sys.parameters` at runtime) and result-set column names are not.
|
|
- The build proves binding, not behaviour: publishing to the dev database (`SqlPackage
|
|
/Action:Publish` with `ExcludeObjectTypes=Logins;Users;Permissions;RoleMembership`, or the
|
|
schema compare) and exercising the code path remains the runtime check.
|
|
- The dev database needs the two `ocms_*` helpers published before any id-generating path
|
|
works there.
|
|
|
|
## Alternatives considered
|
|
|
|
- **Run the changed procedures against the dev database before review.** Needs a publish of
|
|
every working-tree change first, and does not cover code paths the reviewer does not think
|
|
to exercise; the compile-time check is free and total.
|
|
- **Suppress `SQL71502` and keep the build warning-free that way.** Removes exactly the signal
|
|
that catches this class of bug.
|
|
- **A C#-side test that parses the SQL for unqualified columns.** A parser-level heuristic
|
|
without a schema cannot decide ambiguity; the SSDT model can.
|