update drizzle-zod #2819
update drizzle-zod #2819
Conversation
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 212 files out of 294 files are above the max files limit of 75. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the WalkthroughThis pull request introduces a comprehensive update to the validation schemas across multiple Drizzle table definition files. The changes primarily focus on simplifying the schema validation methods by removing explicit field references and applying validation constraints directly to the schema object. This modification affects numerous files in the Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
xoldd
added
the
ignore-sensitive-files-pr
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
package.json(2 hunks)src/drizzle/tables/advertisementAttachments.ts(1 hunks)src/drizzle/tables/advertisements.ts(1 hunks)src/drizzle/tables/agendaFolders.ts(1 hunks)src/drizzle/tables/agendaItems.ts(1 hunks)src/drizzle/tables/chatMessages.ts(1 hunks)src/drizzle/tables/chats.ts(1 hunks)src/drizzle/tables/comments.ts(1 hunks)src/drizzle/tables/eventAttachments.ts(1 hunks)src/drizzle/tables/events.ts(1 hunks)src/drizzle/tables/fundCampaignPledges.ts(1 hunks)src/drizzle/tables/fundCampaigns.ts(1 hunks)src/drizzle/tables/funds.ts(1 hunks)src/drizzle/tables/organizations.ts(1 hunks)src/drizzle/tables/postAttachments.ts(1 hunks)src/drizzle/tables/posts.ts(1 hunks)src/drizzle/tables/tagFolders.ts(1 hunks)src/drizzle/tables/tags.ts(1 hunks)src/drizzle/tables/users.ts(1 hunks)src/drizzle/tables/venueAttachments.ts(1 hunks)src/drizzle/tables/venues.ts(1 hunks)src/graphql/inputs/MutationUpdateEventInput.ts(1 hunks)src/plugins/seedPostgres.ts(1 hunks)
🔇 Additional comments (20)
src/graphql/inputs/MutationUpdateEventInput.ts (1)
8-8: Ensuring consistent optional field usage
Switching from unwrap() to optional() for description helps align the schema fields and ensures the field can be safely omitted. This change is consistent with checked optional fields like endAt, name, and startAt. No immediate issues detected.
src/drizzle/tables/postAttachments.ts (1)
108-108: Validation refactor looks clean
Removing the direct property reference (e.g. schema.name) in favor of schema.min(1) is consistent with the new drizzle-zod approach. No issues found.
src/drizzle/tables/comments.ts (1)
111-111: Streamlined validation
Switching from schema.body.min(1).max(2048) to schema.min(1).max(2048) remains functionally identical while adhering to the updated pattern. Looks good.
src/drizzle/tables/venueAttachments.ts (1)
108-108: Confirm min-length validation coverage for the name field.
This change correctly sets a minimum length requirement of 1, but make sure there are tests or validations that confirm no empty string slips through. Consider adding more constraints (e.g., maximum length) if necessary.
src/drizzle/tables/eventAttachments.ts (1)
108-108: Assess consistency of schema validations.
The updated validation (schema) => schema.min(1) ensures at least one character is provided. Confirm uniform application across related attachments and verify that an empty name is adequately handled or rejected.
src/drizzle/tables/advertisementAttachments.ts (1)
109-109: Validate non-empty attachments.
Requiring name to have at least one character helps prevent storing malformed entries. Confirm that consistent validations are in place for all relevant fields.
src/drizzle/tables/agendaItems.ts (1)
127-128: Expand on test coverage for validation
These validations ensure that description and name meet the required length constraints. However, there are no accompanying tests to confirm that short or excessively long values for these fields properly fail validation. Consider adding test coverage to validate both valid and out-of-bound inputs.
src/drizzle/tables/tags.ts (1)
132-132: Consider upstream references to field-based validation
Similar to other files, this change removes the explicit reference to the name field from schema.name. Confirm that no other part of the code relies on or expects that schema function signature. If such references exist, they might require updates to reflect these new direct schema calls.
src/drizzle/tables/posts.ts (1)
133-133: Ensure that the field is typed as a non-empty string.
Applying min(1).max(2048) directly on the schema enforces a non-empty string with a maximum length. This approach aligns with your intended validation constraints and is consistent with the new refactoring.
src/drizzle/tables/chats.ts (1)
131-133: Adopt consistent optional/required semantics.
All three fields (avatarName, description, name) now use min(1) to guard against empty strings, which is consistent with the pattern in other files. Verify that these fields are truly required and non-null in the database schema (or optional if that’s intended). If the columns are not marked NOT NULL, consider aligning validation rules accordingly to avoid conflicts.
src/drizzle/tables/fundCampaignPledges.ts (1)
137-138: Double-check column defaults vs. non-empty constraints.
Line 137 ensures amount is at least 1, which is appropriate. However, line 138 sets note to min(1), yet the database column for note doesn’t appear to be NOT NULL. If note is optional, consider removing this constraint or allowing empty strings to prevent possible insertion conflicts for rows lacking a note.
src/drizzle/tables/tagFolders.ts (1)
144-144: Good transition to direct schema validation.
Applying min(1).max(256) directly to the schema enforces that name cannot be empty or exceed 256 characters. This is a straightforward way to ensure valid data for the name field.
src/drizzle/tables/fundCampaigns.ts (1)
152-152: Confirm scope of validation changes.
Changing the validator from schema.name.min(1).max(256) to schema.min(1).max(256) may be stylistically consistent with the rest of the code, but ensure that it doesn't inadvertently remove any field-specific checks or naming clarity. If there's any field-specific logic for the name field, confirm that it's handled elsewhere.
src/drizzle/tables/advertisements.ts (1)
154-155: Validate against existing data.
Applying more restrictive validations (min length of 1, max length of 2048 or 256) could break existing data if some fields were allowed to be empty or longer in the past. Confirm there is no existing production data that violates these constraints.
src/drizzle/tables/agendaFolders.ts (1)
152-152: Maintain consistency with other schemas.
This change mirrors the same validation approach seen across other tables (using schema.min(...) instead of schema.name.min(...)). Verify that the calling code and related unit tests (if any) do not rely on the old field-based approach. If there are no tests, consider adding some to confirm correct validation behavior.
src/drizzle/tables/events.ts (1)
152-153: 🧹 Nitpick (assertive)
Enhance verification and test coverage.
The new validations enforce minimum and maximum lengths for description and name. Ensure that any data migration or seeding scripts align with these constraints. It’s also advisable to add or update tests to confirm that these validation constraints hold true in different edge cases.
Would you like me to provide sample test scaffolding to validate these constraints?
src/drizzle/tables/organizations.ts (1)
214-220: Consider optional vs. required fields for better data modeling.
Currently, each field imposes a minimum length of 1, which means they are strictly required to be non-empty. If your data model permits empty or null values for any of these fields, you might need to make them optional (.optional()) or remove the min constraint. Otherwise, the schema will reject empty strings or missing values that might be valid in certain business scenarios.
If these fields should indeed be required, ensure all consuming code and tests account for non-empty constraints. If they can be empty, add .optional() or remove the .min(1) constraint as appropriate.
src/drizzle/tables/users.ts (1)
646-652: Reconfirm requiredness of fields in user schema.
Similar to the organizations table changes, each field is enforced to be non-empty. Verify whether fields like address and description are truly required or can be omitted. If they can be optional, use .optional() or omit min(1).
Additionally, consider creating or updating unit tests to ensure these validations work as expected.
package.json (2)
40-40: Confirm the @types/node update.
While this is a minor version bump, verify that local and CI environments are aligned so that type definitions remain consistent.
[approve]
19-19: Verify stability of the updated drizzle-zod version.
The jump from "0.5.1" to "0.6.1" may introduce breaking changes. Ensure the updated library is properly tested to confirm that none of its new constraints break existing workflows.
You can run a test script to confirm the updated dependency is compatible with all usage sites:
✅ Verification successful
Let me analyze the usage of drizzle-zod in the codebase to verify if the version update might cause any issues.
Let me check the peer dependencies and actual version requirements to ensure compatibility.
The drizzle-zod update from 0.5.1 to 0.6.1 is safe to proceed
The version update is safe because:
- The codebase uses
[email protected]and[email protected], which are within the peer dependency requirements of drizzle-zod 0.6.1 - The usage pattern of
createInsertSchemaacross all files follows a consistent pattern that's compatible with both versions - All schema validations use standard zod methods (min, max) which are stable across these versions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for all drizzle-zod usage sites across the codebase and display context
rg -A 8 'drizzle-zod'
Length of output: 18809
Script:
#!/bin/bash
# Check the changelog or release notes of drizzle-zod for version 0.6.1
gh release view v0.6.1 --repo "drizzle-team/drizzle-zod"
# Check the actual usage pattern of createInsertSchema to verify compatibility
ast-grep --pattern 'createInsertSchema($$$)'
Length of output: 8787
Script:
#!/bin/bash
# Check the package.json for drizzle-orm and zod versions
rg '"drizzle-orm"|"zod"' package.json
# Check the actual version installed in pnpm-lock.yaml
rg 'drizzle-orm@|zod@' pnpm-lock.yaml
Length of output: 378
| address: null, | ||
| avatarName: null, | ||
| city: null, | ||
| creatorId: userId, | ||
| description: null, | ||
| emailAddress: fastify.envConfig.API_ADMINISTRATOR_USER_EMAIL_ADDRESS, | ||
| id: userId, | ||
| isEmailAddressVerified: true, | ||
| name: "name", | ||
| passwordHash: await hash( | ||
| fastify.envConfig.API_ADMINISTRATOR_USER_PASSWORD, | ||
| ), | ||
| postalCode: null, | ||
| role: "administrator", | ||
| state: null, |
There was a problem hiding this comment.
💡 Codebase verification
Schema validation constraints need adjustment
The database schema allows null values for the fields in question (address, avatarName, city, description, postalCode, state), but the usersTableInsertSchema enforces non-null constraints with minimum length requirements. This misalignment needs to be fixed:
src/drizzle/tables/users.ts: The insert schema enforces non-null constraints with:address: min(1).max(1024)avatarName: min(1)city: min(1).max(64)description: min(1).max(2048)postalCode: min(1).max(32)state: min(1).max(64)
These validation rules need to be made optional to match the nullable database columns.
🔗 Analysis chain
Verify nullability alignment with schema
Newly added null fields (address, avatarName, city, description, postalCode, state) may require explicit allowance for null in the usersTableInsertSchema. Ensure these fields accept null without violating validations or constraints. Otherwise, insertion might fail.
Let me generate another script to check the schema definition in the users table.
Let me try a different approach to check the schema definition.
Let me get more lines of the schema definition since it was cut off.
Let me get the remaining part of the schema definition.
Let me get one more piece of the schema to see the remaining fields.
Let me try one final search to get the middle part of the schema.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the schema in files referencing `usersTableInsertSchema` to ensure null is allowed
rg -A 10 "usersTableInsertSchema"
Length of output: 14951
Script:
#!/bin/bash
# Check the schema definition in users table
ast-grep --pattern 'export const usersTable = pgTable("users", {
$$$
});'
Length of output: 77
Script:
#!/bin/bash
# Check the schema definition in users table using ripgrep
rg -A 30 "export const usersTable = pgTable" src/drizzle/tables/users.ts
Length of output: 718
Script:
#!/bin/bash
# Check the complete schema definition in users table using ripgrep with more context
rg -A 100 "export const usersTable = pgTable" src/drizzle/tables/users.ts
Length of output: 2527
Script:
#!/bin/bash
# Get the remaining part of the schema definition
rg -B 50 "}\);" src/drizzle/tables/users.ts
Length of output: 1927
Script:
#!/bin/bash
# Get the middle part of the schema that we missed
rg -B 20 "venueBookingsWhereCreator" src/drizzle/tables/users.ts
Length of output: 790
Script:
#!/bin/bash
# Search for specific fields we need to verify
rg "passwordHash|postalCode|role|state" src/drizzle/tables/users.ts -A 5 -B 5
Length of output: 999
| @@ -119,5 +119,5 @@ export const fundsTableRelations = relations(fundsTable, ({ one, many }) => ({ | |||
| })); | |||
|
|
|||
| export const fundsTableInsertSchema = createInsertSchema(fundsTable, { | |||
| name: (schema) => schema.name.min(1).max(256), | |||
| name: (schema) => schema.min(1).max(256), | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Ensure test coverage for length boundaries.
The name field has min and max length requirements. Confirm you have a test case for edge values (e.g., name of length 1 and 256) to detect any off-by-one errors.
| @@ -120,6 +120,6 @@ export const chatMessagesTableRelations = relations( | |||
| export const chatMessagesTableInsertSchema = createInsertSchema( | |||
| chatMessagesTable, | |||
| { | |||
| body: (schema) => schema.body.min(1).max(2048), | |||
| body: (schema) => schema.min(1).max(2048), | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Validate indirect property usage carefully
By referencing schema.min(1).max(2048) directly, it no longer explicitly ties this validation to the body field, which might cause confusion when reading the schema definitions. If the intention is to preserve clarity that these rules apply specifically to the body field, consider adding a comment or clarifying inline documentation to ensure future maintainers understand which field is being validated.
| description: (schema) => schema.min(1).max(2048), | ||
| name: (schema) => schema.min(1).max(256), |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Revisit user-facing error messages
When description or name fail the .min(1).max(...) checks, the user-facing error message provided by drizzle-zod might be generic. Ensure that any thrown errors or messages clearly communicate which field failed validation and why, improving the user experience.
|
@palisadoes |
|
Thanks. We'll open an issue for that |
|
Only other repo using it is Eslint is very slow both during manual checking and its editor integrations that run in the background because its written in javascript. It also doesn't come with strict sane defaults and manually configuring it is a mess because of plugins. Biome is written in rust and is one of the fastest linters in the javascript ecosystem. It comes with strict sane defaults so no need configuring it. I've mentioned the main reason in this comment: Line 1 in 9a5a8c6 |
updated drizzle zod to the latest version and fixed the breaking changes
xoldd
force-pushed
the
update_drizzle_zod
branch
from
caf12af to
e604d10
Compare
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
|
Can this be merged now? |
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
xoldd
force-pushed
the
update_drizzle_zod
branch
from
a29bb10 to
87d7116
Compare
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
yes |
palisadoes
merged commit 98670f5
into
PalisadoesFoundation:develop-postgres
|
Can we start the migration now? |
What kind of change does this PR introduce?
feature
Issue Number:
Fixes #
Did you add tests for your changes?
No
Snapshots/Videos:
If relevant, did you update the documentation?
Summary
Updated the
drizzle-zodpackage to latest stable release. It introduced breaking changes so had to fix them.Does this PR introduce a breaking change?
Other information
Have you read the contributing guide?
Yes
Summary by CodeRabbit
Release Notes
Dependencies
drizzle-zodfrom 0.5.1 to 0.6.1@types/nodefrom 22.10.2 to 22.10.3Database Schema
User Management
These updates primarily focus on improving internal validation and schema consistency across the application's database models.