Conversation
📝 WalkthroughWalkthroughConsolidates product mutation DTOs into a single product-mutations.dto.ts with type-specific Created/Updated/Deleted variants; adds ProductChannels and subscriptions; new resolvers for mutation links and updatedKeys; several resolvers/services updated to publish typed mutation events and to load products via ProductLoader. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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 unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/product/product.service.ts`:
- Around line 434-445: The previous-value payload is built with
DerivativeScriptureProductUpdate.pickPrevious but mistakenly passes
simpleChanges (which omits produces and scriptureReferencesOverride) in
updateDerivative; make it consistent with updateDirect by passing the full
changes object to pickPrevious so that previous includes prior produces and
scriptureReferencesOverride values, i.e. update the call in updateDerivative to
use changes (not simpleChanges) and verify
DerivativeScriptureProductUpdate.fromInput(changes) and
pickPrevious(currentProduct, changes) align.
🧹 Nitpick comments (2)
src/components/product/product-mutation-subscriptions.resolver.ts (1)
53-109: Consider extracting the repeated payload-mapping logic.All 9 per-type subscription methods share the same destructure-and-remap pattern (
project→projectId,engagement→engagementId,product→productId,...rest). A small helper could reduce the boilerplate:♻️ Example helper extraction
private mapPayload<T extends string>( __typename: T, { project, engagement, product, ...rest }: ProductMutationPayload, ) { return { __typename, projectId: project, engagementId: engagement, productId: product, ...rest } as const; }Then each subscription becomes:
- map(({ project, engagement, product, ...rest }): DirectScriptureProductCreated => ({ - __typename: 'DirectScriptureProductCreated', - projectId: project, - engagementId: engagement, - productId: product, - ...rest, - })), + map((p) => this.mapPayload('DirectScriptureProductCreated', p)),Also applies to: 120-176
src/components/product/product.channels.ts (1)
62-92: The conditional type on thepayloadparameter is impressive but dense.The
ReturnType<ProductChannels[Method]> extends Channel<infer T extends ProductMutationPayload> ? Omit<T, 'by'> : neverconstruct ensures compile-time correctness of the payload shape per action/type combination, which is great. The trade-off is readability — a brief doc comment explaining the intent of this type gymnastics would help future readers.
No description provided.