feat(prisma): introduce CreateOrUpdate function #1455
Conversation
WalkthroughThe pull request introduces a new Changes
Assessment against linked issues
Possibly related PRs
Finishing Touches
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
test/features/upsert/upsert_test.go (1)
94-153: Add negative test cases.The test suite should include:
- MongoDB provider rejection test
- Required field validation test
- Error cases (e.g., database errors)
Example test case to add:
{ name: "CreateOrUpdate with MongoDB should return error", run: func(t *testing.T, client *PrismaClient, ctx cx) { // Mock MongoDB provider client.Engine = &mockMongoEngine{} _, err := client.Post.UpsertOne( Post.ID.Equals("upsert"), ).CreateOrUpdate( Post.Title.Set("title"), Post.Views.Set(0), ).Exec(ctx) if err == nil { t.Fatal("expected error for MongoDB provider") } }, }test/features/upsert/schema.prisma (1)
14-17: Add field documentation.Consider adding documentation comments to describe the purpose and constraints of each field, especially the new
descriptionfield.model Post { - id String @id @default(cuid()) @map("_id") - title String - views Int - description String? + /// Unique identifier for the post + id String @id @default(cuid()) @map("_id") + /// Post title + title String + /// Number of post views + views Int + /// Optional post description + description String? }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
generator/templates/_header.gotpl(1 hunks)generator/templates/actions/upsert.gotpl(1 hunks)generator/templates/client.gotpl(1 hunks)test/features/upsert/schema.prisma(1 hunks)test/features/upsert/upsert_test.go(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- generator/templates/_header.gotpl
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: integration-macos
- GitHub Check: integration-windows
- GitHub Check: test
- GitHub Check: lint
🔇 Additional comments (1)
generator/templates/client.gotpl (1)
8-8: Consider handling multiple datasources.The
providerNameconstant assumes a single datasource by accessing index 0. While this works for the current use case, it might need revision if multiple datasources are supported in the future.Let's check if multiple datasources are supported or planned:
✅ Verification successful
Accessing first datasource is consistent with current implementation
The codebase consistently uses only the first datasource (
Datasources[0]) across multiple files, suggesting this is an intentional design choice rather than an oversight. While the data structure supports multiple datasources, the current implementation expects exactly one datasource.
generator/generator.go: Usesr.Datasources[0]in multiple locationsgenerator/templates/client.gotpl: References first datasource🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if multiple datasources are mentioned in docs or tests rg -i "multiple.?datasources?" -g "!{vendor,node_modules}/*"Length of output: 62
Script:
#!/bin/bash # Check for datasources array definition and usage ast-grep --pattern 'type $_ struct { $$$ Datasources $$$ $$$ }' # Look for array operations on datasources rg "\.Datasources\[" -A 2 # Find template files that might use datasources fd "\.gotpl$" --exec grep -l "Datasources"Length of output: 1379
ynden
force-pushed
the
feature/upsert-create-or-update
branch
from
33f4f9d to
39e912b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
generator/templates/actions/upsert.gotpl (2)
121-123:⚠️ Potential issueConsider returning an error instead of panicking.
For better error handling and API design, consider returning an error when MongoDB is detected instead of panicking. This allows API consumers to handle the error gracefully.
112-120:⚠️ Potential issueAdd validation for required fields.
The method should validate that all required fields are provided and not nil before proceeding with the operation.
🧹 Nitpick comments (1)
test/features/upsert/upsert_test.go (1)
94-116: Add negative test cases.While the current test cases cover the happy path, consider adding tests for:
- Invalid/missing required fields
- MongoDB provider to verify error handling
- Concurrent upsert operations
Also applies to: 117-153
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
generator/templates/_header.gotpl(1 hunks)generator/templates/actions/upsert.gotpl(1 hunks)generator/templates/client.gotpl(1 hunks)test/features/upsert/schema.prisma(1 hunks)test/features/upsert/upsert_test.go(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- generator/templates/_header.gotpl
- generator/templates/client.gotpl
- test/features/upsert/schema.prisma
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: integration-macos
- GitHub Check: integration-windows
- GitHub Check: lint
- GitHub Check: test
🔇 Additional comments (3)
generator/templates/actions/upsert.gotpl (1)
138-146: LGTM! Efficient field reuse.The implementation efficiently reuses the same fields for both create and update operations, which aligns well with the PR's objective of reducing verbosity in upsert operations.
test/features/upsert/upsert_test.go (2)
14-18: LGTM! Clear database support definition.The explicit list of supported databases helps users understand which databases are compatible with the feature.
194-225: LGTM! Comprehensive transaction testing.The transaction test case properly validates the CreateOrUpdate functionality within a transaction context.
…redeclaring the same fields
ynden
force-pushed
the
feature/upsert-create-or-update
branch
from
39e912b to
bf2dad7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
test/features/upsert/upsert_test.go (3)
94-116: LGTM! Good test coverage for creation scenario.The test properly validates the creation flow and handles generated IDs correctly.
Consider adding edge cases:
- Empty string values
- Maximum field lengths
- Special characters in fields
117-154: LGTM! Comprehensive test for update scenario.The test thoroughly validates the update flow, including optional fields.
Consider adding test cases for:
- Updating only a subset of fields
- Removing optional fields (setting to null)
195-229: LGTM! Good transaction test for CreateOrUpdate.The test properly validates transaction behavior with CreateOrUpdate.
Consider adding test cases for:
- Transaction rollback scenarios
- Concurrent transactions
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
generator/templates/actions/upsert.gotpl(1 hunks)test/features/upsert/schema.prisma(1 hunks)test/features/upsert/upsert_test.go(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- test/features/upsert/schema.prisma
- generator/templates/actions/upsert.gotpl
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: integration-macos
- GitHub Check: integration-windows
- GitHub Check: lint
- GitHub Check: test
🔇 Additional comments (3)
test/features/upsert/upsert_test.go (3)
192-193: LGTM! Added missing assertion.Good addition of result validation for the transaction test.
236-236: LGTM! Proper test runner configuration.The test runner correctly uses the new database configuration while maintaining proper test isolation.
14-18: LGTM! Database configuration aligns with PR objectives.The database list correctly excludes MongoDB, which aligns with the PR's stated limitation that
CreateOrUpdateshould panic for MongoDB.Let's verify that MongoDB is not accidentally used elsewhere:
✅ Verification successful
Verification successful: MongoDB is properly excluded from upsert operations
The codebase correctly handles MongoDB exclusion:
- The generator template (
upsert.gotpl) explicitly excludes MongoDB through conditional statements- Test configuration properly omits MongoDB from upsert tests
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any MongoDB-related configurations or tests rg -i "mongodb|mongo" .Length of output: 5043
Description
UpsertOneis a convenient function when we want to create a record if it doesn't exist, or update it when it does exist. All without having to handle the logic to check whether, or not the record exists. To use it we call this way:However, sometimes we might have the same fields in
CreateandUpdate, so we need to use:This now becomes verbose, so we need a way to make it less verbose. This should close #489.
Changelog
CreateOrUpdatechain that goes withUpsertOne.CreateOrUpdatewhen record exists, and don't exist.CreateOrUpdatefor mongo)New behaviour
We now can use
UpsertOnethis way:This will now automatically create the record if it doesn't exist, otherwise, it'll update the specific record pointed by the where parameter value.
Important note
@steebchen
The current logic makes it that if we pass in a primary key like an
IDtoUpdate, it'll fail formongodatabase as specified in #1446, hence why we panic formongowhen usingCreateOrUpdateSummary by CodeRabbit
New Features
CreateOrUpdatemethod for database operationsdescriptionfield for Post modelTests
CreateOrUpdatemethod across different database types