fix: status change no longer affects edited status#8662
fix: status change no longer affects edited status#8662VincentHan12 wants to merge 4 commits intomainfrom
Conversation
WalkthroughResolver changed from bulk Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit 7d8aaa7
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apis/api-journeys/src/app/modules/journey/journey.resolver.ts`:
- Around line 1010-1014: Remove the stray console.log in the resolver method
that calls this.prismaService.journey.findMany; specifically delete the line
console.log(results.length) (or replace it with a proper structured debug log
via the resolver's logger, e.g., this.logger.debug(...), if you need telemetry).
Update the method in journey.resolver.ts (the resolver function that queries
this.prismaService.journey.findMany and returns Promise.all) so no console
output remains in the request path.
🧹 Nitpick comments (1)
apis/api-journeys/src/app/modules/journey/journey.resolver.ts (1)
953-975: Consider a transaction to avoid partial status changes.Per-journey
Promise.allupdates can leave a subset updated if one fails. Wrapping the updates in a transaction preserves all-or-nothing behavior (apply similarly to delete/trash/restore).♻️ Suggested transaction wrapper
- return await Promise.all( - results.map( - async (journey) => - await this.prismaService.journey.update({ - where: { id: journey.id }, - data: { - status: JourneyStatus.archived, - archivedAt: new Date(), - updatedAt: journey.updatedAt - } - }) - ) - ) + return await this.prismaService.$transaction( + results.map((journey) => + this.prismaService.journey.update({ + where: { id: journey.id }, + data: { + status: JourneyStatus.archived, + archivedAt: new Date(), + updatedAt: journey.updatedAt + } + }) + ) + )
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apis/api-journeys/src/app/modules/journey/journey.resolver.ts`:
- Around line 960-975: The current flow reads journeys with
prismaService.journey.findMany then writes back journey.updatedAt in
prismaService.journey.update which can regress a newer edit; remove writing the
read snapshot updatedAt and instead perform a safe update that either (a) omits
updatedAt from the update so the DB sets a new timestamp or (b) performs a
conditional update using where: { id: journey.id, updatedAt: journey.updatedAt }
and retry/refresh on failure to avoid clobbering concurrent changes; locate the
affected calls in journey.resolver.ts (the prismaService.journey.findMany and
prismaService.journey.update invocations around the archived status changes) and
implement one of these strategies consistently for the other similar blocks
referenced (lines ~985-1000, ~1010-1025, ~1035-1047).
🧹 Nitpick comments (1)
apis/api-journeys/src/app/modules/journey/journey.resolver.ts (1)
960-975: Optional cleanup: early return + clearer naming + shared timestamp.Consider an early return when
ids.length === 0, renameresultsto something status-specific (e.g.,journeysToArchive), and reuse a singleconst now = new Date()for consistency and less churn.♻️ Example (apply similarly to the other mutations)
- const results = await this.prismaService.journey.findMany({ + if (ids.length === 0) return [] + const now = new Date() + const journeysToArchive = await this.prismaService.journey.findMany({ where: { AND: [accessibleJourneys, { id: { in: ids } }] } }) return await Promise.all( - results.map( + journeysToArchive.map( async (journey) => await this.prismaService.journey.update({ where: { id: journey.id }, data: { status: JourneyStatus.archived, - archivedAt: new Date(), + archivedAt: now, updatedAt: journey.updatedAt } }) ) )As per coding guidelines, "Use early returns whenever possible to make the code more readable. Use descriptive variable and function/const names."
Also applies to: 985-1000, 1010-1025, 1035-1047
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apis/api-journeys/src/app/modules/journey/journey.resolver.ts`:
- Around line 964-979: The batch updates using Promise.all over results.map
calling this.prismaService.journey.update are not atomic and can leave partial
updates on P2025 conflicts; replace the Promise.all(...) with a single
transactional call using this.prismaService.$transaction([...updates]) where you
build an array of the same update calls (one per journey) and pass it to
$transaction so the updates are applied all-or-nothing. Apply this transactional
change to the other similar mutations that update journeys in bulk (the blocks
using this.prismaService.journey.findMany + Promise.all with
this.prismaService.journey.update).
| const results = await this.prismaService.journey.findMany({ | ||
| where: { AND: [accessibleJourneys, { id: { in: ids } }] } | ||
| }) | ||
| return await Promise.all( | ||
| results.map( | ||
| async (journey) => | ||
| await this.prismaService.journey.update({ | ||
| where: { id: journey.id, updatedAt: journey.updatedAt }, | ||
| data: { | ||
| status: JourneyStatus.archived, | ||
| archivedAt: new Date(), | ||
| updatedAt: journey.updatedAt | ||
| } | ||
| }) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Prisma Client update error when no records match where clause NotFoundError
💡 Result:
In Prisma Client, update() (and delete()) throws when no row matches the where clause. In older Prisma versions you might see a NotFoundError; in Prisma ORM v6 that specific error type was removed and you should expect a PrismaClientKnownRequestError with code P2025 instead. [1]
What to do instead (pick one)
- If “0 matched” is OK (don’t throw): use
updateMany()
updateMany()does not throw when nothing matches; it returns{ count: 0 }. [2][3]
- If you want “update if exists else create”: use
upsert()
upsert()updates when found, otherwise creates. [3]
- If you want to keep
update()but handle “not found”: catchP2025
- Wrap the call in
try/catchand treaterror.code === 'P2025'as your “not found” case. [1]
Why it happens: update() is designed to update exactly one existing record; when Prisma can’t find it, it raises the “record not found” error (P2025). [1][3]
Sources: Prisma v6 upgrade guide (NotFoundError removal → use P2025) [1], Prisma CRUD docs (updateMany / upsert) [3], Prisma discussions/answers describing handling P2025 and using updateMany to avoid throwing on zero matches [2].
🌐 Web query:
Prisma Client $transaction rollback atomicity guarantee all or nothing
💡 Result:
Prisma Client’s $transaction is intended to give database-level atomicity (“all or nothing”) for the operations that are actually executed inside the same DB transaction:
- Prisma frames transactions in ACID terms, where Atomic means “either all operations succeed, or the transaction is aborted and rolled back.” (prisma.io)
- For interactive transactions (
await prisma.$transaction(async (tx) => { ... })), if your function throws/returns a rejected promise, Prisma says it automatically rolls back the transaction. (prisma.io) - For the array form (
await prisma.$transaction([q1, q2, ...])), Prisma describes it as a single atomic operation where if any operation fails, Prisma rolls back the entire transaction. (prisma.io)
Important caveat (common source of “partial updates” confusion):
- In an interactive transaction, wrapping queries in
Promise.all()is not true parallelism (one connection can run only one query at a time), so they still execute serially. (prisma.io) - If you observe partial effects, it usually means some work wasn’t actually part of the same transaction (e.g., queries executed on the non-transaction client, started outside the callback, or otherwise escaped the transaction scope). Prisma support has specifically suggested using the
$transaction([])form for “array of operations” use-cases to avoid partial updates in patterns like this. (github.com)
Citations:
- 1: https://www.prisma.io/docs/orm/prisma-client/queries/transactions?utm_source=openai
- 2: https://www.prisma.io/docs/orm/prisma-client/queries/transactions?utm_source=openai
- 3: https://www.prisma.io/docs/orm/prisma-client/queries/transactions?utm_source=openai
- 4: https://www.prisma.io/docs/orm/prisma-client/queries/transactions?utm_source=openai
- 5: Prisma support for transaction rollback and Promise.all prisma/prisma#13564
🌐 Web query:
Prisma Client Promise.all parallel updates vs $transaction batch updates
💡 Result:
What’s the difference?
1) Promise.all([...prisma.model.update(...)]) (parallel outside a transaction)
- Executes concurrently (up to your Prisma/DB connection pool capacity). If you fire many queries at once you can hit connection pool timeouts and end up effectively queued/throttled. [1][3]
- Not atomic: some updates can succeed while others fail (partial completion).
- Usually fastest for independent writes when your pool + DB can handle the concurrency and you don’t need all-or-nothing semantics. [1][3]
2) prisma.$transaction([q1, q2, ...]) (batch transaction with an array)
- Atomic: all queries succeed or the whole transaction rolls back. [2]
- Not parallel: Prisma executes the array sequentially in the transaction. [2]
- Works well when you need all-or-nothing across multiple independent operations, and you can tolerate sequential execution. [2][4]
3) prisma.$transaction(async (tx) => { ... }) (interactive transaction)
- Also atomic, but Prisma must run everything on one connection, so even if you do
await Promise.all([...])inside the transaction, the queries still run serially (one at a time). [2] - Prisma explicitly warns to keep interactive transactions short (don’t do slow work or network calls inside), because long transactions hurt DB performance / can deadlock. [2]
When to choose which?
-
Use
Promise.allwhen:- updates are independent, and
- you can accept partial success, and
- you’ve sized/tuned your connection pool appropriately. [1][3]
-
Use
$transaction([...])when:- updates are independent but you need atomic all-or-nothing, and
- sequential execution is fine (or preferred to reduce concurrency pressure). [2][4]
-
Use interactive
$transaction(async tx => ...)when:- you need conditional logic / read-modify-write in one transaction,
- but keep it short and don’t expect parallelism inside it. [2]
Common gotchas
- “Parallel updates” inside a single transaction aren’t really parallel: one transaction = one connection = one query at a time. [2]
- High concurrency via
Promise.allcan cause pool timeouts unlessconnection_limit/ pool settings are tuned for your workload. [1][3]
Sources: Prisma docs on connection pooling & parallel queries [1][3], Prisma transactions reference (sequential vs interactive, and Promise.all inside transactions being serial) [2], Prisma Client PrismaPromise behavior enabling $transaction batching [4].
Wrap batch status updates in prisma.$transaction(...) to prevent partial state on conflict.
The current Promise.all() pattern is not atomic. If any journey's update() fails with P2025 (due to updatedAt mismatch), some updates may have already succeeded, leaving the batch in a partially updated state. Using prisma.$transaction([...]) instead guarantees all-or-nothing semantics: either all journeys are updated or the entire transaction rolls back on any conflict.
✅ Suggested transactional pattern (apply across all four mutations)
- return await Promise.all(
- results.map(
- async (journey) =>
- await this.prismaService.journey.update({
- where: { id: journey.id, updatedAt: journey.updatedAt },
- data: {
- status: JourneyStatus.archived,
- archivedAt: new Date(),
- updatedAt: journey.updatedAt
- }
- })
- )
- )
+ return await this.prismaService.$transaction(
+ results.map((journey) =>
+ this.prismaService.journey.update({
+ where: { id: journey.id, updatedAt: journey.updatedAt },
+ data: {
+ status: JourneyStatus.archived,
+ archivedAt: new Date(),
+ updatedAt: journey.updatedAt
+ }
+ })
+ )
+ )Also applies to: 989-1004, 1014-1029, 1046-1051
🤖 Prompt for AI Agents
In `@apis/api-journeys/src/app/modules/journey/journey.resolver.ts` around lines
964 - 979, The batch updates using Promise.all over results.map calling
this.prismaService.journey.update are not atomic and can leave partial updates
on P2025 conflicts; replace the Promise.all(...) with a single transactional
call using this.prismaService.$transaction([...updates]) where you build an
array of the same update calls (one per journey) and pass it to $transaction so
the updates are applied all-or-nothing. Apply this transactional change to the
other similar mutations that update journeys in bulk (the blocks using
this.prismaService.journey.findMany + Promise.all with
this.prismaService.journey.update).
1 participant
Summary by CodeRabbit
Tests
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.