fix(schema): require custCompanyName / custFName / custLName to match DB NOT NULL#277
Merged
Merged
Conversation
… DB NOT NULL setup/TimeTracker.sql defines all three columns as text NOT NULL, but customer.schema.js marked them \`.optional()\`. Same class of drift as #265's custState fix: callers omitting any of these fields passed zod and surfaced as a 500 at the postgres INSERT layer ("null value in column 'custFName' violates not-null constraint") instead of a clean 400 with a field-level message. Removed \`.optional()\` from the three required fields. The whitelist message and the surrounding doc comment are updated to call out the DB-NOT-NULL contract explicitly so future readers don't undo it. Test impact: four api-tests (customer-create / customer-bulk / validation / idempotency) sent partial bodies expecting the controller's authKey-missing 403 to short-circuit. With required fields now enforced by zod earlier in the chain, those tests now hit a 400 before reaching auth. Each test was updated to send a complete required-field body — preserving the original intent (testing auth-fail behavior) while accommodating the new validation contract. 760 tests still pass (was 760, no count change — same paths just with non-partial bodies). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CryptoJones
added a commit
that referenced
this pull request
May 19, 2026
… NULL (#279) Same drift class as #265 (custState) and #277 (custCompanyName et al.), but with a twist: the single-create controller already filled in the missing-invPaid case with \`if (payload.invPaid === undefined) payload.invPaid = false;\`, so the bug only manifested in the bulk path, which routes through \`makeBulkCreateIndirect\` and doesn't have entity-specific defaults. A bulk POST like { invoices: [{ invCustId: 5, invDate: "2026-01-01", invDueDate: "2026-02-01" }] } passed the schema (invPaid was \`.optional()\`), and then tripped "null value in column invPaid violates not-null constraint" at the postgres INSERT — surfacing as a 500 instead of a clean accepted row. Switching the schema to \`z.boolean().default(false)\` fills the value at the validator boundary, so both paths now get \`invPaid: false\` for free. The single-create controller's explicit default becomes dead defense but stays harmless. 760 tests still pass. Co-authored-by: Aaron K. Clark <akclark@thenetwerk.net> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CryptoJones
added a commit
that referenced
this pull request
May 19, 2026
…#285) The Customer model was the outlier in the models/ directory: every string field declared as Sequelize.STRING (varchar 255 default) with no allowNull, even though setup/TimeTracker.sql has the corresponding columns as text NOT NULL (or text NULL, varchar(2), varchar(32)) for specific fields. Every other model (Worker, BillingType, InventoryItem, Job, Invoice, ...) already used TEXT / STRING(N) accurately with explicit allowNull declarations. Aligned each field with the DDL: - custCompanyName / custFName / custLName: TEXT, allowNull: false - custAddress1 / 2 / custCity / custZip / custEmail: TEXT - custState: STRING(2) - custPhone: STRING(32) - custArch: BOOLEAN, allowNull: false, defaultValue: false - custCompId: INTEGER, allowNull: false No behavior change for typical callers: - Zod validation (#265, #277) already enforces the NOT NULL fields + custState length at the request boundary, so this is purely defense in depth. - Removing the STRING-255 implicit cap relaxes the model to match the actual DB TEXT (unbounded), but zod's max(255) still caps inputs at the API layer. 760 tests still pass. Co-authored-by: Aaron K. Clark <akclark@thenetwerk.net> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
setup/TimeTracker.sqldefines all three columns astext NOT NULL, butcustomer.schema.jsmarked them.optional(). Same class of drift as #265'scustStatefix: callers omitting any of these fields passed zod and surfaced as a 500 at the postgres INSERT layer ("null value in column 'custFName' violates not-null constraint") instead of a clean 400 with a field-level message.What changed
.optional()fromcustCompanyName,custFName,custLNameincreateCustomerBody.Test impact
Four api-tests (
customer-create,customer-bulk,validation,idempotency) used to send partial bodies expecting the controller's authKey-missing 403 to short-circuit. With required fields now enforced by zod earlier in the middleware chain, those tests would have hit a 400 before reaching auth. Each was updated to send a complete required-field body — same intent (testing auth-fail behavior), accommodating the new validation contract.Impact for callers
Test plan
npm run lint && npm test— 760 passing.Proudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/