Open
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a ProjectActions audit model and migration; records project-level action entries across server handlers; surfaces actions in UI with new components and modal; migrates legacy product transitions into project actions; updates enums, icons, locales, SSE enqueueing, and related job/migration steps. Changes
Sequence DiagramsequenceDiagram
actor User
participant Client as Browser
participant Server as AppServer
participant DB as Database (Prisma)
participant Queue as BullMQ (SvelteSSE)
participant SSE as SSE Broadcaster
User->>Client: submit project change (edit/owner/claim/archive/access)
Client->>Server: POST action/request
Server->>DB: update project / create ProjectActions
DB-->>Server: result / created action(s)
Server->>Queue: enqueue SvelteSSE_UpdateProject for projectId
Queue-->>SSE: trigger project update broadcast
SSE->>Client: stream updated project details (incl. ProjectActions)
Client->>Client: render action history/modal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/lib/projects/server.ts (1)
123-184:⚠️ Potential issue | 🔴 CriticalKeep project state, audit rows, and task side effects on the same confirmed success path.
DatabaseWrites.projects.update()can returnfalse, but the archive/reactivate branches still create aProjectActionsrow and enqueue task mutations even when that happens.DatabaseWrites.projectActions.create()can also returnfalse, and that result is ignored in all three branches. That makes it possible for project state, audit history, and user-task churn to drift out of sync.🧩 Minimum guard to add immediately
- await DatabaseWrites.projects.update(project.Id, { + const updated = await DatabaseWrites.projects.update(project.Id, { DateArchived: timestamp }); - await DatabaseWrites.projectActions.create({ + if (!updated) return; + + const action = await DatabaseWrites.projectActions.create({ ProjectId: project.Id, UserId: security.userId, DateAction: timestamp, ActionType: ProjectActionType.Archival, Action: ProjectActionString.Archive }); + if (!action) return;Please apply the same guard to
reactivateandclaim, and ideally move the project update + audit insert into one transactional helper.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/projects/server.ts` around lines 123 - 184, DatabaseWrites.projects.update(...) can return false but the code proceeds to create ProjectActions and enqueue UserTasks regardless, and DatabaseWrites.projectActions.create(...) is also unchecked; ensure archive, reactivate and claim only perform the audit insert and enqueue the UserTasks after a successful DatabaseWrites.projects.update(project.Id, ...) (check the returned success boolean), and also verify the projectActions create result before enqueuing; preferably move the update + audit-create + queue-add into a transactional helper (e.g., a new helper that wraps DatabaseWrites.projects.update, DatabaseWrites.projectActions.create and getQueues().UserTasks.add) so that the project state, audit rows, and task side effects are applied only on confirmed success.src/routes/(authenticated)/organizations/[id=idNumber]/settings/stores/transfer/+page.server.ts (1)
114-154:⚠️ Potential issue | 🟠 MajorMake the store move and transition write atomic.
All
products.update()calls complete before anyproductTransitions.createMany()runs. If the second phase fails, the products stay moved but the transfer history is missing. This should be a single transactional write path, or at least one dedicated helper that performs both steps together.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`(authenticated)/organizations/[id=idNumber]/settings/stores/transfer/+page.server.ts around lines 114 - 154, The current flow updates products with DatabaseWrites.products.update first, then separately creates ProductTransition rows with DatabaseWrites.productTransitions.createMany, which can leave products moved without transfer history if the second step fails; wrap the update+transition for each project in a single transactional operation (or add a dedicated helper like transferProducts or DatabaseWrites.transferProducts/transferProjectProducts) that within one transaction updates the Product.StoreId(s) (formerly using products.update) and inserts the corresponding productTransitions records (formerly createMany) using the same transferDate and event.locals.security.userId, and ensure the transaction rolls back on any error so both steps are atomic; locate logic around the successes/grouped variables and replace the separate Promise.all calls with the new transactional helper per grouped project.
🧹 Nitpick comments (5)
src/routes/(authenticated)/projects/[id=idNumber]/+page.svelte (1)
252-256: Hoist the project-action filter out of the product loop.Every card recomputes the same scan over
projectData.project.ProjectActions. Deriving that array once will keep render cost flat as history grows and avoid reallocating identicalprojectActionsarrays per product.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`(authenticated)/projects/[id=idNumber]/+page.svelte around lines 252 - 256, The code repeatedly filters projectData.project.ProjectActions inside the product/card rendering, causing redundant recomputation; compute a single derived array once (e.g., const filteredProjectActions = projectData.project.ProjectActions.filter(pa => pa.ActionType === ProjectActionType.Access || pa.ActionType === ProjectActionType.Archival)) outside the product loop or card component and pass that filteredProjectActions into each card via the projectActions prop instead of calling .filter(...) per product.src/lib/projects/sse.ts (1)
325-377: Precompute unique action lookup IDs once and fetchactionParamsin parallel.
project.ProjectActionsis filtered/mapped three times here, and eachfindMany()waits for the previous one. A single pre-pass withSets plusPromise.all()will trim work on every SSE refresh. Based on learnings: FyreByrd prefers to optimize by creating Sets at the caller level for better performance and separation of concerns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/projects/sse.ts` around lines 325 - 377, project.ProjectActions is being filtered and mapped three times and the three DatabaseReads.findMany calls inside actionParams are executed sequentially; refactor by precomputing three Sets of unique ExternalId values (e.g., authorIds, ownerGroupIds, productDefIds) from project.ProjectActions once, then call DatabaseReads.users.findMany, DatabaseReads.groups.findMany, and DatabaseReads.productDefinitions.findMany in parallel using Promise.all and the precomputed Sets in the respective where.in clauses so actionParams is built from the parallel results.src/lib/server/database/ProjectActions.ts (1)
15-17: Silent exception swallowing hinders debugging.Both
createandcreateManycatch exceptions and returnfalsewithout logging. When callers don't check the return value (as seen insrc/lib/projects/server.ts:125-145), failures go completely unnoticed.Consider logging the error before returning:
♻️ Proposed fix
} catch { + console.error('Failed to create projectAction(s)'); return false; }Also applies to: 33-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/server/database/ProjectActions.ts` around lines 15 - 17, The catch blocks in ProjectActions.ts (in the create and createMany functions) silently swallow exceptions and return false; change those catches to accept the error (catch (err)) and log the error before returning so failures are visible; use the module's established logger (or console.error if none exists) to emit a contextual message including the function name (e.g., "ProjectActions.create" / "ProjectActions.createMany") and the caught error, then return false as before.src/lib/prisma/schema.prisma (1)
301-313: Consider adding indexes on foreign key columns.The
ProjectActionsmodel lacks indexes onProjectIdandUserId. Given that queries will frequently filter by project (e.g., loading action history for a project page), performance could degrade as the table grows.🗃️ Proposed schema addition
User Users `@relation`(fields: [UserId], references: [Id], onDelete: Cascade, onUpdate: NoAction, map: "FK_ProjectActions_Users_UserId") + + @@index([ProjectId], map: "IX_ProjectActions_ProjectId") + @@index([UserId], map: "IX_ProjectActions_UserId") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/prisma/schema.prisma` around lines 301 - 313, Add indexes on the foreign key columns in the ProjectActions model: add @@index([ProjectId]) and @@index([UserId]) to the ProjectActions model so queries filtering by ProjectId or UserId use indexes; update the Prisma schema (model ProjectActions) accordingly and run prisma migrate to generate/apply the migration.src/routes/(authenticated)/projects/[id=idNumber]/+page.server.ts (1)
214-226: Consider moving the store lookup before the queue job.The
BuildEnginePublisherIdlookup (lines 219-224) occurs after the product creation job is queued. If the store doesn't exist (unlikely given earlier validation),Valuewould beundefined. Consider fetching this data earlier alongside the repository check, or add a fallback value.♻️ Optional: Consolidate store lookup with earlier validation
const checkRepository = await DatabaseReads.projects.findUnique({ where: { Id: parseInt(event.params.id) }, select: { RepositoryUrl: true } }); if (!checkRepository?.RepositoryUrl) { return error(400, 'Project Repository not Yet Initialized'); } + const store = await DatabaseReads.stores.findFirst({ + where: { Id: form.data.storeId }, + select: { BuildEnginePublisherId: true } + }); await getQueues().Products.add(`Create Product for Project #${event.params.id}`, { type: BullMQ.JobType.Product_CreateLocal, projectId, productDefinitionId: form.data.productDefinitionId, storeId: form.data.storeId }); await DatabaseWrites.projectActions.create({ ProjectId: projectId, UserId: event.locals.security.userId, ActionType: ProjectActionType.Product, Action: ProjectActionString.AddProduct, - Value: ( - await DatabaseReads.stores.findFirst({ - where: { Id: form.data.storeId }, - select: { BuildEnginePublisherId: true } - }) - )?.BuildEnginePublisherId, + Value: store?.BuildEnginePublisherId, ExternalId: form.data.productDefinitionId });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`(authenticated)/projects/[id=idNumber]/+page.server.ts around lines 214 - 226, Move the BuildEnginePublisherId lookup out of the DatabaseWrites.projectActions.create call so it is fetched before queueing the job (e.g. alongside the earlier repository/store validation) using DatabaseReads.stores.findFirst to assign a local variable (BuildEnginePublisherId) and pass that variable into DatabaseWrites.projectActions.create; alternatively, if you prefer minimal change, perform the findFirst just before calling DatabaseWrites.projectActions.create and provide a sensible fallback (empty string or null) for Value when BuildEnginePublisherId is undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/locales/fr-FR.json`:
- Line 235: Update the French locale JSON to replace the English values with
proper French translations for the affected keys: change the value for
"products_details" from "History" to "Historique", replace the entry currently
containing "Transfer Store" with a French equivalent (e.g., "Transfert de
magasin"), replace the entry currently containing "Update Properties" with a
French equivalent (e.g., "Mettre à jour les propriétés"), and replace the entry
containing "Transfer Group" with a French equivalent (e.g., "Transférer le
groupe"); locate these by the existing JSON keys/strings in
src/lib/locales/fr-FR.json and update only the string values, preserving JSON
structure and commas.
In `@src/lib/prisma/migrations/35_project_actions/migration.sql`:
- Around line 2-19: The migration creates the ProjectActions table with foreign
keys ProjectId and UserId but doesn't add indexes on those referencing columns,
which will cause full-table scans as the audit log grows; update the migration
to create indexes on "ProjectActions"("ProjectId") and
"ProjectActions"("UserId") (e.g., names like IX_ProjectActions_ProjectId and
IX_ProjectActions_UserId) so reads filtered by project or user use the indexes,
ensuring the index creation statements are added alongside the table and FK
ALTERs in the migration.sql.
In `@src/lib/products/components/ProductDetails.svelte`:
- Around line 270-271: The ProjectActionEntry instances in ProductDetails.svelte
are missing the optional prodDefs, users, and groups props so action details
(product definition names, user/group names) render incomplete; update
ProductDetails.svelte's export props (the component Props) to accept prodDefs,
users, and groups (matching the types used by ProjectActionEntry) and forward
them into every ProjectActionEntry usage (e.g., <ProjectActionEntry
act={transition} compact={false} prodDefs={prodDefs} users={users}
groups={groups} /> and the other occurrence) so the child can resolve names
correctly.
In `@src/lib/projects/components/ProjectActionEntry.svelte`:
- Around line 114-123: The current render calls m[act.Value as ValidI13nKey]
when useI18n is true, which can throw if act.Value is not a valid i18n key;
update the extraBox branch (and the compact variant) to defensively check that
act.Value exists as a key and that m[act.Value] is a callable translation before
invoking it, and otherwise pass a safe fallback (e.g., act.Value or a default
translation string) into TaskComment; refer to symbols extraBox, useI18n, act,
m, TaskComment, and ValidI13nKey when locating the code to change.
- Around line 91-92: The template can render "User `#null`" when act.ExternalId is
null; update the fallback in ProjectActionEntry.svelte (the expression using act
and users) to check for a non-null ExternalId before interpolating it: if
users.find(...) fails and act.ExternalId is not null/undefined show `User
#<id>`, otherwise show a safe label like "Unknown user" (or just omit the
"#<id>") so that act.ExternalId is never stringified as "null"; modify the
expression that references act.ExternalId and users?.find(...) accordingly.
- Around line 52-53: ProjectActionType.Access currently renders raw action
strings; add corresponding keys to the ProjectActionString enum for the Access
values (e.g., Upload, Download, use) and update ProjectActionEntry.svelte so the
Access branch uses the i18n lookup like the others: replace the raw {act.Action}
rendering with m[act.Action as ValidI13nKey]({} as never). Ensure enum keys
match the act.Action values and that ValidI13nKey covers them.
In `@src/lib/server/job-executors/system.ts`:
- Around line 733-746: The current mapping uses a non-null assertion on
pt.UserId and writes raw workflow names into ProjectAction.Action; instead,
update the mapping in the p.Products[0].ProductTransitions -> ProjectAction
creation so that (1) you do not use pt.UserId! — either skip creating the
ProjectAction when pt.UserId is null or explicitly set UserId to null/undefined
only if your storage accepts that (preferably filter out transitions with
pt.UserId == null before mapping), and (2) replace the inline pt.InitialState
use for ProjectAccess transitions with a translation to a valid
ProjectActionString: add a small mapping (e.g., map InitialState values like
"Product Build"/"Product Publish" to concrete ProjectActionString constants or
fall back to a generic ProjectActionString.Access) and use that mapped value
when pt.TransitionType === ProductTransitionType.ProjectAccess; keep the
existing logic for Archival/Reactive branches unchanged and ensure DateAction
uses pt.DateTransition as before.
- Around line 730-756: The createMany and deleteMany calls inside the
projects.map block (DatabaseWrites.projectActions.createMany and
DatabaseWrites.productTransitions.deleteMany) must be executed atomically to
avoid partial migration state; wrap both operations for each project in a single
transaction (use the existing transaction helper used elsewhere, e.g.,
prismaInternal.$transaction or the DatabaseWrites transaction API) so that if
deleteMany fails the createMany is rolled back, or alternatively check the
result of createMany and abort/rollback before calling deleteMany; ensure the
transaction encloses both the creation payload (built from
p.Products[0].ProductTransitions) and the delete filter (transitionFilter with
Product: { ProjectId: p.Id }) for each project.
In
`@src/routes/`(authenticated)/organizations/[id=idNumber]/settings/stores/transfer/+page.server.ts:
- Around line 88-90: The POST action currently queries Products with where: {
Id: { in: form.data.products }, Project: { OrganizationId: orgId } } but does
not filter out archived projects; update the Products where predicate to mirror
the load() filter by adding Project: { OrganizationId: orgId, DateArchived: null
} (i.e., include DateArchived: null alongside OrganizationId) so the action only
operates on products belonging to active projects and cannot transfer products
from archived projects.
---
Outside diff comments:
In `@src/lib/projects/server.ts`:
- Around line 123-184: DatabaseWrites.projects.update(...) can return false but
the code proceeds to create ProjectActions and enqueue UserTasks regardless, and
DatabaseWrites.projectActions.create(...) is also unchecked; ensure archive,
reactivate and claim only perform the audit insert and enqueue the UserTasks
after a successful DatabaseWrites.projects.update(project.Id, ...) (check the
returned success boolean), and also verify the projectActions create result
before enqueuing; preferably move the update + audit-create + queue-add into a
transactional helper (e.g., a new helper that wraps
DatabaseWrites.projects.update, DatabaseWrites.projectActions.create and
getQueues().UserTasks.add) so that the project state, audit rows, and task side
effects are applied only on confirmed success.
In
`@src/routes/`(authenticated)/organizations/[id=idNumber]/settings/stores/transfer/+page.server.ts:
- Around line 114-154: The current flow updates products with
DatabaseWrites.products.update first, then separately creates ProductTransition
rows with DatabaseWrites.productTransitions.createMany, which can leave products
moved without transfer history if the second step fails; wrap the
update+transition for each project in a single transactional operation (or add a
dedicated helper like transferProducts or
DatabaseWrites.transferProducts/transferProjectProducts) that within one
transaction updates the Product.StoreId(s) (formerly using products.update) and
inserts the corresponding productTransitions records (formerly createMany) using
the same transferDate and event.locals.security.userId, and ensure the
transaction rolls back on any error so both steps are atomic; locate logic
around the successes/grouped variables and replace the separate Promise.all
calls with the new transactional helper per grouped project.
---
Nitpick comments:
In `@src/lib/prisma/schema.prisma`:
- Around line 301-313: Add indexes on the foreign key columns in the
ProjectActions model: add @@index([ProjectId]) and @@index([UserId]) to the
ProjectActions model so queries filtering by ProjectId or UserId use indexes;
update the Prisma schema (model ProjectActions) accordingly and run prisma
migrate to generate/apply the migration.
In `@src/lib/projects/sse.ts`:
- Around line 325-377: project.ProjectActions is being filtered and mapped three
times and the three DatabaseReads.findMany calls inside actionParams are
executed sequentially; refactor by precomputing three Sets of unique ExternalId
values (e.g., authorIds, ownerGroupIds, productDefIds) from
project.ProjectActions once, then call DatabaseReads.users.findMany,
DatabaseReads.groups.findMany, and DatabaseReads.productDefinitions.findMany in
parallel using Promise.all and the precomputed Sets in the respective where.in
clauses so actionParams is built from the parallel results.
In `@src/lib/server/database/ProjectActions.ts`:
- Around line 15-17: The catch blocks in ProjectActions.ts (in the create and
createMany functions) silently swallow exceptions and return false; change those
catches to accept the error (catch (err)) and log the error before returning so
failures are visible; use the module's established logger (or console.error if
none exists) to emit a contextual message including the function name (e.g.,
"ProjectActions.create" / "ProjectActions.createMany") and the caught error,
then return false as before.
In `@src/routes/`(authenticated)/projects/[id=idNumber]/+page.server.ts:
- Around line 214-226: Move the BuildEnginePublisherId lookup out of the
DatabaseWrites.projectActions.create call so it is fetched before queueing the
job (e.g. alongside the earlier repository/store validation) using
DatabaseReads.stores.findFirst to assign a local variable
(BuildEnginePublisherId) and pass that variable into
DatabaseWrites.projectActions.create; alternatively, if you prefer minimal
change, perform the findFirst just before calling
DatabaseWrites.projectActions.create and provide a sensible fallback (empty
string or null) for Value when BuildEnginePublisherId is undefined.
In `@src/routes/`(authenticated)/projects/[id=idNumber]/+page.svelte:
- Around line 252-256: The code repeatedly filters
projectData.project.ProjectActions inside the product/card rendering, causing
redundant recomputation; compute a single derived array once (e.g., const
filteredProjectActions = projectData.project.ProjectActions.filter(pa =>
pa.ActionType === ProjectActionType.Access || pa.ActionType ===
ProjectActionType.Archival)) outside the product loop or card component and pass
that filteredProjectActions into each card via the projectActions prop instead
of calling .filter(...) per product.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5bab7056-3efd-402b-bdb0-60e35575e6a4
📒 Files selected for processing (28)
src/lib/icons/index.tssrc/lib/locales/en-US.jsonsrc/lib/locales/es-419.jsonsrc/lib/locales/fr-FR.jsonsrc/lib/prisma.tssrc/lib/prisma/migrations/35_project_actions/migration.sqlsrc/lib/prisma/schema.prismasrc/lib/products/components/ProductDetails.sveltesrc/lib/projects/components/ProjectActionEntry.sveltesrc/lib/projects/components/ProjectActionMenu.sveltesrc/lib/projects/components/ProjectDetails.sveltesrc/lib/projects/server.tssrc/lib/projects/sse.tssrc/lib/server/bullmq/BullWorker.tssrc/lib/server/database/Products.tssrc/lib/server/database/ProjectActions.tssrc/lib/server/database/index.tssrc/lib/server/job-executors/build.tssrc/lib/server/job-executors/system.tssrc/lib/server/job-executors/userTasks.tssrc/routes/(authenticated)/organizations/[id=idNumber]/settings/stores/transfer/+page.server.tssrc/routes/(authenticated)/projects/[id=idNumber]/+page.server.tssrc/routes/(authenticated)/projects/[id=idNumber]/+page.sveltesrc/routes/(authenticated)/projects/[id=idNumber]/ProductCard.sveltesrc/routes/(authenticated)/projects/[id=idNumber]/edit/+page.server.tssrc/routes/(authenticated)/projects/[id=idNumber]/edit/+page.sveltesrc/routes/(authenticated)/workflow-instances/[product_id]/+page.server.tssrc/routes/(unauthenticated)/api/projects/[id=idNumber]/token/+server.ts
src/routes/(authenticated)/organizations/[id=idNumber]/settings/stores/transfer/+page.server.ts
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/server/job-executors/system.ts`:
- Around line 703-752: The code currently only reads a single product
(Products.take:1 ordered by DateCreated) and maps ProductTransitions from
p.Products[0], which can miss transitions and then the subsequent
tx.productTransitions.deleteMany deletes all matching transitions for the
project; change DatabaseReads.projects.findMany so Products is filtered by the
same transitionFilter (Products: { where: transitionFilter, select: {
ProductTransitions: { where: transitionFilter, ... } } }), remove the
orderBy/take:1 so you fetch all products with matching transitions, then in the
prismaInternal.$transaction use p.Products.flatMap(p => p.ProductTransitions) to
build the tx.projectActions.createMany payload so you migrate every matching
ProductTransition before calling tx.productTransitions.deleteMany with the same
transitionFilter and Product: { ProjectId: p.Id } to ensure only migrated rows
are deleted (referencing DatabaseReads.projects.findMany, transitionFilter,
p.Products, prismaInternal.$transaction, tx.projectActions.createMany,
tx.productTransitions.deleteMany).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 68fc4fcf-2abb-44a1-ab91-35111d9bc2b0
📒 Files selected for processing (5)
src/lib/prisma/migrations/35_project_actions/migration.sqlsrc/lib/prisma/schema.prismasrc/lib/projects/components/ProjectActionEntry.sveltesrc/lib/server/job-executors/system.tssrc/routes/(authenticated)/organizations/[id=idNumber]/settings/stores/transfer/+page.server.ts
✅ Files skipped from review due to trivial changes (2)
- src/lib/prisma/migrations/35_project_actions/migration.sql
- src/lib/prisma/schema.prisma
🚧 Files skipped from review as they are similar to previous changes (2)
- src/routes/(authenticated)/organizations/[id=idNumber]/settings/stores/transfer/+page.server.ts
- src/lib/projects/components/ProjectActionEntry.svelte
Member
|
How is this sorted? I did these series of actions:
It looks like the migration of the product transitions came in-between two of my actions, but the dates don't line up in the display. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/projects/sse.ts (1)
328-380: Consider parallelizing independent database queries.The three queries for
users,groups, andprodDefsare independent and could be executed concurrently usingPromise.all()to reduce overall latency.♻️ Proposed refactor to parallelize queries
actionParams: { - users: await DatabaseReads.users.findMany({ + ...(await (async () => { + const userIds = project.ProjectActions.filter( + (pa) => + pa.ExternalId && + (pa.ActionType === ProjectActionType.Author || + (pa.ActionType === ProjectActionType.OwnerGroup && + pa.Action !== ProjectActionString.AssignGroup)) + ).map((pa) => pa.ExternalId!); + + const groupIds = project.ProjectActions.filter( + (pa) => + pa.ExternalId && + pa.ActionType === ProjectActionType.OwnerGroup && + pa.Action === ProjectActionString.AssignGroup + ).map((pa) => pa.ExternalId!); + + const prodDefIds = project.ProjectActions.filter( + (pa) => pa.ExternalId && pa.ActionType === ProjectActionType.Product + ).map((pa) => pa.ExternalId!); + + const [users, groups, prodDefs] = await Promise.all([ + DatabaseReads.users.findMany({ + where: { Id: { in: userIds } }, + select: { Id: true, Name: true } + }), + DatabaseReads.groups.findMany({ + where: { Id: { in: groupIds } }, + select: { Id: true, Name: true } + }), + DatabaseReads.productDefinitions.findMany({ + where: { Id: { in: prodDefIds } }, + select: { + Id: true, + Name: true, + Workflow: { select: { ProductType: true } } + } + }) + ]); + + return { users, groups, prodDefs }; + })()) - where: { - Id: { - in: project.ProjectActions.filter( - (pa) => - pa.ExternalId && - (pa.ActionType === ProjectActionType.Author || - (pa.ActionType === ProjectActionType.OwnerGroup && - pa.Action !== ProjectActionString.AssignGroup)) - ).map((pa) => pa.ExternalId!) - } - }, - select: { - Id: true, - Name: true - } - }), - groups: await DatabaseReads.groups.findMany({ - where: { - Id: { - in: project.ProjectActions.filter( - (pa) => - pa.ExternalId && - pa.ActionType === ProjectActionType.OwnerGroup && - pa.Action === ProjectActionString.AssignGroup - ).map((pa) => pa.ExternalId!) - } - }, - select: { - Id: true, - Name: true - } - }), - prodDefs: await DatabaseReads.productDefinitions.findMany({ - where: { - Id: { - in: project.ProjectActions.filter( - (pa) => pa.ExternalId && pa.ActionType === ProjectActionType.Product - ).map((pa) => pa.ExternalId!) - } - }, - select: { - Id: true, - Name: true, - Workflow: { - select: { - ProductType: true - } - } - } - }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/projects/sse.ts` around lines 328 - 380, The three independent DB calls that populate actionParams.users, actionParams.groups and actionParams.prodDefs should be executed in parallel to reduce latency: create the three DatabaseReads.findMany promises (the users, groups and productDefinitions queries referencing project.ProjectActions and ProjectActionType/ProductActionString) and run them with Promise.all(), then assign the resolved results back into actionParams.users, actionParams.groups and actionParams.prodDefs; ensure the select shapes remain the same and preserve the filtering logic when extracting ExternalId from project.ProjectActions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/lib/projects/sse.ts`:
- Around line 328-380: The three independent DB calls that populate
actionParams.users, actionParams.groups and actionParams.prodDefs should be
executed in parallel to reduce latency: create the three DatabaseReads.findMany
promises (the users, groups and productDefinitions queries referencing
project.ProjectActions and ProjectActionType/ProductActionString) and run them
with Promise.all(), then assign the resolved results back into
actionParams.users, actionParams.groups and actionParams.prodDefs; ensure the
select shapes remain the same and preserve the filtering logic when extracting
ExternalId from project.ProjectActions.
Contributor
Author
This has been fixed. I forgot to order by date in the db query. |
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
2 participants
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.
Changes:
DetailstoHistoryfor ProductTransitions and now ProjectActionsScreenshots:
Project page
ProjectActions
ProductTransitions
Summary by CodeRabbit
New Features
Improvements
Localization