feat: add customizable field to journey, and journey customizable service#8813
feat: add customizable field to journey, and journey customizable service#8813
Conversation
…ustomizable-field-to-journey-schema
…ustomizable-field-to-journey-schema
…ustomizable-field-to-journey-schema
…logic - Introduced JourneyCustomizableService to handle the recalculation of the `customizable` field for journeys based on various conditions. - Updated JourneyModule to include JourneyCustomizableService as a provider and export it for use in other modules. - Added unit tests for JourneyCustomizableService to ensure correct functionality of the customization logic.
…ney recalculation - Added JourneyCustomizableService to BlockResolver and BlockService to handle journey recalculations after block duplication, restoration, and updates. - Updated unit tests to verify that recalculation is triggered appropriately in various scenarios. - Enhanced Block module imports to include JourneyModule for better integration.
…omizableService integration - Integrated JourneyCustomizableService into JourneyResolver to trigger recalculations based on journey updates, specifically when the website field is modified. - Updated JourneyCustomizationFieldResolver to call recalculate after publisher updates. - Added unit tests to ensure the correct invocation of recalculate in various scenarios, enhancing the overall functionality and reliability of journey customization features.
- Introduced new documentation for adding customizable fields to block and action types, outlining necessary updates to the JourneyCustomizableService and BlockService. - Created a customizable-blocks.md file detailing the paths affected and the required logic updates for recalculation. - Added a corresponding customizable-blocks.mdc file to serve as a guardrail for developers, ensuring compliance with the new customization requirements.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a Journey.customizable flag (Prisma + migration + GraphQL), implements JourneyCustomizableService.recalculate(journeyId) to derive and persist the flag from journey fields, blocks, actions and media, and wires recalculation calls across journey, block, and customization-field mutation paths with tests and guardrail docs. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Resolver as Resolver
participant Service as DomainService
participant JourneyCustomizable as JourneyCustomizableService
participant Prisma as PrismaService
participant DB as Database
Client->>Resolver: mutate (updateBlock / updateJourney / duplicate / persistCustomizationField)
Resolver->>Service: call domain method
Service->>Prisma: perform DB transaction (create/update/delete)
Prisma->>DB: persist changes
DB-->>Prisma: ack
Prisma-->>Service: transaction result
Service->>JourneyCustomizable: recalculate(journeyId)
JourneyCustomizable->>Prisma: fetch journey, blocks, actions, logo info
Prisma->>DB: queries
DB-->>Prisma: data
JourneyCustomizable->>JourneyCustomizable: compute newCustomizable
alt value differs
JourneyCustomizable->>Prisma: update journey.customizable
Prisma->>DB: update
DB-->>Prisma: ack
end
JourneyCustomizable-->>Service: done
Service-->>Resolver: return original result
Resolver-->>Client: mutation response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
…-backend-auto-detect-journeycustomizable-via-recalculation
|
View your CI Pipeline Execution ↗ for commit 3eef1f6
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apis/api-gateway/schema.graphql (1)
4518-4550:⚠️ Potential issue | 🟠 MajorKeep
customizableout ofJourneyUpdateInput.This flag is described by the PR as derived from journey content via recalculation. Making it client-writable allows callers to persist a value that no longer matches the actual blocks/actions/media until some later recalculation happens. Keep it queryable on
Journey, but remove it from the public update input.Suggested change
input JourneyUpdateInput `@join__type`(graph: API_JOURNEYS) `@join__type`(graph: API_JOURNEYS_MODERN) { title: String languageId: String themeMode: ThemeMode themeName: ThemeName description: String creatorDescription: String creatorImageBlockId: ID primaryImageBlockId: ID slug: String seoTitle: String seoDescription: String hostId: String strategySlug: String tagIds: [ID!] website: Boolean showShareButton: Boolean showLikeButton: Boolean showDislikeButton: Boolean displayTitle: String showHosts: Boolean showChatButtons: Boolean showReactionButtons: Boolean showLogo: Boolean showMenu: Boolean showDisplayTitle: Boolean menuButtonIcon: JourneyMenuButtonIcon menuStepBlockId: ID logoImageBlockId: ID socialNodeX: Int socialNodeY: Int - customizable: Boolean }Based on learnings: the
showAssistantfield was intentionally excluded fromJourneyUpdateInputbecause it is controlled through direct database operations rather than GraphQL mutations, while remaining queryable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apis/api-gateway/schema.graphql` around lines 4518 - 4550, Remove the client-writable customizable field from the JourneyUpdateInput input type so clients cannot set it; edit the JourneyUpdateInput definition (remove the customizable: Boolean line), keep customizable as a read-only field on the Journey type for queries, and regenerate any GraphQL/typegen artifacts and update any input validators or resolver tests that assumed the field existed to ensure clients cannot persist derived values via update mutations.
🧹 Nitpick comments (2)
apis/api-journeys/src/app/modules/journey/journey.resolver.spec.ts (1)
2475-2499: Add coverage for the other derived-field transitions.These cases only guard the
websiteandtemplate=truepaths. A couple of assertions forjourneyCustomizationDescription/logoImageBlockIdupdates andtemplate=falseclearingcustomizablewould catch the stale-state paths injourneyUpdate()andjourneyTemplate().Also applies to: 2841-2849
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apis/api-journeys/src/app/modules/journey/journey.resolver.spec.ts` around lines 2475 - 2499, Add tests to cover other derived-field transitions by asserting recalculate is called or not for updates to journeyCustomizationDescription and logoImageBlockId and for template toggles: extend tests around resolver.journeyUpdate to include cases where input contains journeyCustomizationDescription or logoImageBlockId (mock prismaService.journey.findUnique and update as in existing tests) and expect journeyCustomizableService.recalculate('journeyId') to be called; also add tests for resolver.journeyTemplate where template=true should set customizable and trigger recalculate and template=false should clear customizable and ensure journeyCustomizableService.recalculate is not called (or is called as appropriate), using the same mocked journey fixtures and prismaService methods as in current tests.apis/api-journeys/src/app/modules/journey/journeyCustomizable.service.ts (1)
39-42: Select only the columns used by the recalculation.This runs after block update/remove/restore flows, so
include: { action: true }pulls every block and action column on a hot path. The checks below only readid,typename,customizable,action.customizable, andaction.blockId.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apis/api-journeys/src/app/modules/journey/journeyCustomizable.service.ts` around lines 39 - 42, The query in this.prismaService.block.findMany pulls entire block and action records; limit the selected columns to only those used by the recalculation: select block.id, block.typename, block.customizable and for the relation use action: select { customizable, blockId } (keep the same where filter { journeyId, deletedAt: null }). Update the findMany call to use select instead of include so only these fields are fetched.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apis/api-journeys/src/app/modules/journey/journey.resolver.ts`:
- Around line 1114-1119: When updating a journey, turning input.template to
false can leave customizable true and the resolver currently returns the
pre-recalc row; fix by ensuring customizable is cleared and the returned row is
the final persisted state: if input.template === false, set input.customizable =
false (so prismaService.journey.update(...) persists customizable:false), still
call this.journeyCustomizableService.recalculate(id) as before, then reload the
journey (e.g. await this.prismaService.journey.findUnique({ where: { id } }))
and return that fresh record instead of the original updatedJourney; reference
prismaService.journey.update, this.journeyCustomizableService.recalculate,
updatedJourney, input.template and id to locate changes.
- Around line 891-894: The returned journey can be stale because recalculate()
is only called when input.website changes and the function returns result before
the recalculation is persisted; update the logic in journey.resolver.ts to call
this.journeyCustomizableService.recalculate(id) whenever any of the fields that
affect customizable state change (website, journeyCustomizationDescription,
logoImageBlockId), await the recalculate() call before returning, and ensure you
return the post-recalculation journey (either by applying the recalculated
values to result or reloading the journey after recalculate) so
journey.customizable is accurate.
In
`@libs/prisma/journeys/db/migrations/20260302193727_20260302193725/migration.sql`:
- Around line 1-2: The migration adds Journey.customizable with DEFAULT false
but leaves historical rows incorrect; add a companion backfill that recomputes
and sets Journey.customizable for all existing journeys (either as an immediate
SQL update in the same migration or as a guaranteed post-deploy job) by scanning
the related entities that determine customizability (blocks, actions, media, or
whatever domain tables/functions your app uses) and setting Journey.customizable
= true where those checks indicate customization; ensure the backfill targets
every Journey row and is idempotent and logged so the derived column reflects
historical state before relying on it.
---
Outside diff comments:
In `@apis/api-gateway/schema.graphql`:
- Around line 4518-4550: Remove the client-writable customizable field from the
JourneyUpdateInput input type so clients cannot set it; edit the
JourneyUpdateInput definition (remove the customizable: Boolean line), keep
customizable as a read-only field on the Journey type for queries, and
regenerate any GraphQL/typegen artifacts and update any input validators or
resolver tests that assumed the field existed to ensure clients cannot persist
derived values via update mutations.
---
Nitpick comments:
In `@apis/api-journeys/src/app/modules/journey/journey.resolver.spec.ts`:
- Around line 2475-2499: Add tests to cover other derived-field transitions by
asserting recalculate is called or not for updates to
journeyCustomizationDescription and logoImageBlockId and for template toggles:
extend tests around resolver.journeyUpdate to include cases where input contains
journeyCustomizationDescription or logoImageBlockId (mock
prismaService.journey.findUnique and update as in existing tests) and expect
journeyCustomizableService.recalculate('journeyId') to be called; also add tests
for resolver.journeyTemplate where template=true should set customizable and
trigger recalculate and template=false should clear customizable and ensure
journeyCustomizableService.recalculate is not called (or is called as
appropriate), using the same mocked journey fixtures and prismaService methods
as in current tests.
In `@apis/api-journeys/src/app/modules/journey/journeyCustomizable.service.ts`:
- Around line 39-42: The query in this.prismaService.block.findMany pulls entire
block and action records; limit the selected columns to only those used by the
recalculation: select block.id, block.typename, block.customizable and for the
relation use action: select { customizable, blockId } (keep the same where
filter { journeyId, deletedAt: null }). Update the findMany call to use select
instead of include so only these fields are fetched.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fa707768-01dd-427d-ac4d-1d1ab49c527b
⛔ Files ignored due to path filters (4)
apis/api-journeys/src/__generated__/graphql.tsis excluded by!**/__generated__/**apis/api-journeys/src/app/__generated__/graphql.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/globalTypes.tsis excluded by!**/__generated__/**libs/shared/gql/src/__generated__/graphql-env.d.tsis excluded by!**/__generated__/**
📒 Files selected for processing (29)
.claude/rules/backend/customizable-blocks.md.cursor/rules/customizable-blocks.mdcapis/api-gateway/schema.graphqlapis/api-journeys-modern/schema.graphqlapis/api-journeys-modern/src/schema/journey/inputs/journeyUpdateInput.tsapis/api-journeys-modern/src/schema/journey/journey.tsapis/api-journeys-modern/src/workers/emailEvents/service/fetchEmailDetails.spec.tsapis/api-journeys-modern/src/workers/emailEvents/service/processUserIds.spec.tsapis/api-journeys-modern/src/workers/emailEvents/service/service.spec.tsapis/api-journeys/schema.graphqlapis/api-journeys/src/app/modules/block/block.module.tsapis/api-journeys/src/app/modules/block/block.resolver.spec.tsapis/api-journeys/src/app/modules/block/block.resolver.tsapis/api-journeys/src/app/modules/block/block.service.spec.tsapis/api-journeys/src/app/modules/block/block.service.tsapis/api-journeys/src/app/modules/block/videoTrigger/videoTrigger.resolver.spec.tsapis/api-journeys/src/app/modules/host/host.resolver.spec.tsapis/api-journeys/src/app/modules/journey/journey.graphqlapis/api-journeys/src/app/modules/journey/journey.module.tsapis/api-journeys/src/app/modules/journey/journey.resolver.spec.tsapis/api-journeys/src/app/modules/journey/journey.resolver.tsapis/api-journeys/src/app/modules/journey/journeyCustomizable.service.spec.tsapis/api-journeys/src/app/modules/journey/journeyCustomizable.service.tsapis/api-journeys/src/app/modules/journeyCustomizationField/journeyCustomizationField.module.tsapis/api-journeys/src/app/modules/journeyCustomizationField/journeyCustomizationField.resolver.spec.tsapis/api-journeys/src/app/modules/journeyCustomizationField/journeyCustomizationField.resolver.tsapis/api-journeys/src/app/modules/journeyTheme/journeyTheme.resolver.spec.tslibs/prisma/journeys/db/migrations/20260302193727_20260302193725/migration.sqllibs/prisma/journeys/db/schema.prisma
libs/prisma/journeys/db/migrations/20260302193727_20260302193725/migration.sql
Show resolved
Hide resolved
|
The latest updates on your projects.
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apis/api-journeys/src/app/modules/journey/journey.resolver.ts (2)
1120-1125:⚠️ Potential issue | 🟠 MajorTurning a template off can leave
customizablestuck on.When
input.templateisfalse,recalculate()exits immediately for non-templates, so an existingcustomizable: truevalue is never cleared. This path also returns the pre-recalculation row. Clearcustomizableduring the update when de-templating, then reload the final journey before returning it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apis/api-journeys/src/app/modules/journey/journey.resolver.ts` around lines 1120 - 1125, When de-templating (input.template === false) ensure customizable is cleared during the update and return the final DB row after recalculation: in the block that calls prismaService.journey.update(...) and journeyCustomizableService.recalculate(id), include customizable: false in the update data when input.template is explicitly false (to avoid leaving customizable stuck on), call recalculate(id) afterward, then reload the journey via prismaService.journey.findUnique({ where: { id } }) (or equivalent) and return that reloaded row instead of the pre-recalculation updatedJourney.
897-900:⚠️ Potential issue | 🟠 MajorThe recalculation trigger is still too narrow, and the returned journey can be stale.
recalculate()only runs wheninput.websiteis present, even though the derived flag also depends on other persisted fields such aslogoImageBlockId. And becauseresultis returned without a reload,journeyUpdate()can respond with the oldcustomizablevalue even when recalculation does run.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apis/api-journeys/src/app/modules/journey/journey.resolver.ts` around lines 897 - 900, The update only calls journeyCustomizableService.recalculate(id) when input.website is set and returns the pre-recalc result, causing stale customizable flags; modify the logic in the journeyUpdate resolver so you trigger recalculate whenever any customizable-dependent fields change (e.g., website, logoImageBlockId, and other persisted customizable fields) or simply always call journeyCustomizableService.recalculate(id) after successful update, and then reload the journey (refetch the updated record) before returning so the response reflects the recalculated customizable values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apis/api-journeys/schema.graphql`:
- Line 2297: The field "customizable" is read-only and must not be writable via
the public API: remove "customizable" from the JourneyUpdateInput type while
keeping it on the Journey type so queries can still read it; locate the GraphQL
schema definitions for Journey and JourneyUpdateInput and delete the
"customizable: Boolean" entry from JourneyUpdateInput (leave the
Journey.customizable field unchanged) so the server continues to compute it but
clients cannot submit it in updates.
In `@apis/api-journeys/src/app/modules/journey/journey.resolver.ts`:
- Around line 631-634: The duplicate seeds customizable from
journey.customizable too early (using template: duplicateAsTemplate and
customizable: duplicateAsTemplate ? (journey.customizable ?? false) : false);
instead, after the clone's blocks/actions/media/fields are copied and the cloned
journey is fully saved, reload the cloned journey from the DB and
recalculate/set its customizable based on the actual copied content, then return
that freshly loaded journey; update the code paths that create the duplicate
(look for duplicateAsTemplate, the customizable assignment, and the post-clone
save/return logic) so the final returned object reflects the recomputed
customizable value rather than the original source flag.
---
Duplicate comments:
In `@apis/api-journeys/src/app/modules/journey/journey.resolver.ts`:
- Around line 1120-1125: When de-templating (input.template === false) ensure
customizable is cleared during the update and return the final DB row after
recalculation: in the block that calls prismaService.journey.update(...) and
journeyCustomizableService.recalculate(id), include customizable: false in the
update data when input.template is explicitly false (to avoid leaving
customizable stuck on), call recalculate(id) afterward, then reload the journey
via prismaService.journey.findUnique({ where: { id } }) (or equivalent) and
return that reloaded row instead of the pre-recalculation updatedJourney.
- Around line 897-900: The update only calls
journeyCustomizableService.recalculate(id) when input.website is set and returns
the pre-recalc result, causing stale customizable flags; modify the logic in the
journeyUpdate resolver so you trigger recalculate whenever any
customizable-dependent fields change (e.g., website, logoImageBlockId, and other
persisted customizable fields) or simply always call
journeyCustomizableService.recalculate(id) after successful update, and then
reload the journey (refetch the updated record) before returning so the response
reflects the recalculated customizable values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c6734871-a554-43b1-b508-561dd6a5ef5a
⛔ Files ignored due to path filters (2)
apis/api-journeys/src/__generated__/graphql.tsis excluded by!**/__generated__/**apis/api-journeys/src/app/__generated__/graphql.tsis excluded by!**/__generated__/**
📒 Files selected for processing (6)
apis/api-gateway/schema.graphqlapis/api-journeys/schema.graphqlapis/api-journeys/src/app/modules/block/videoTrigger/videoTrigger.resolver.spec.tsapis/api-journeys/src/app/modules/journey/journey.graphqlapis/api-journeys/src/app/modules/journey/journey.resolver.spec.tsapis/api-journeys/src/app/modules/journey/journey.resolver.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apis/api-journeys/src/app/modules/block/videoTrigger/videoTrigger.resolver.spec.ts
- apis/api-journeys/src/app/modules/journey/journey.graphql
|
The latest updates on your projects.
|
…urneycustomizable-via-recalculation
…ions - Consolidated instructions for adding a `customizable` field to new block and action types by removing redundant steps and clarifying the update logic in `JourneyCustomizableService.recalculate()`. This enhances clarity and ensures developers understand the necessary adjustments for the new field.
| customizable: true, | ||
| journeyCustomizationDescription: true, | ||
| logoImageBlockId: true, | ||
| website: true |
There was a problem hiding this comment.
NIT: We can add the count for number of customizable fields by doing this. It can save not having to call a second query.
| website: true | |
| website: true, | |
| _count: { select: { journeyCustomizationField: true } } |
| await this.prismaService.journeyCustomizationField.count({ | ||
| where: { journeyId } | ||
| }) | ||
|
|
There was a problem hiding this comment.
If you do the above you can do this instead:
| const fieldsCount = journey._count.journeyCustomizationField |
…ustomizationFields - Modified the service to include _count in the journey query, allowing for more efficient counting of journeyCustomizationFields. - Adjusted tests to reflect the new structure, ensuring accurate validation of customizable logic based on the presence of editable text fields.
|
I see you added the "on stage" label, I'll get this merged to the stage branch! |
|
Merge conflict attempting to merge this into stage. Please fix manually. |
Have merged onto stage. No merge conflicts when I did it |
…urneycustomizable-via-recalculation
|
Merge conflict attempting to merge this into stage. Please fix manually. |
Summary by CodeRabbit
New Features
Behavior
Database
customizablecolumn to journeys.Tests
Documentation