fix: change type for startat and endat #8475
Conversation
WalkthroughThis change updates the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run journeys-admin-e2e:e2e |
❌ Failed | 3m 43s | View ↗ |
nx run watch-e2e:e2e |
✅ Succeeded | 12s | View ↗ |
nx run resources-e2e:e2e |
✅ Succeeded | 12s | View ↗ |
nx run journeys-e2e:e2e |
✅ Succeeded | 20s | View ↗ |
nx run videos-admin-e2e:e2e |
✅ Succeeded | 4s | View ↗ |
nx run short-links-e2e:e2e |
✅ Succeeded | 3s | View ↗ |
nx run-many --target=vercel-alias --projects=re... |
✅ Succeeded | 2s | View ↗ |
nx run-many --target=upload-sourcemaps --projec... |
✅ Succeeded | 8s | View ↗ |
Additional runs (16) |
✅ Succeeded | ... | View ↗ |
☁️ Nx Cloud last updated this comment at 2025-12-08 04:46:36 UTC
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apis/api-journeys-modern/schema.graphql (1)
2095-2096: Output fields updated to Float, but inputs still Int—consider aligning for true fractional support
VideoBlock.startAt/endAtbeingFloatis consistent with the DB and modern resolver, butVideoBlockCreateInput/VideoBlockUpdateInputin this schema still useIntfor these fields. That means clients can only submit whole seconds even though the system can represent fractions.If the goal is full fractional-second support end-to-end, consider changing the input fields to
Floatas well and re-composing the gateway schema, keeping api-journeys and api-journeys-modern in sync as per prior learnings.apis/api-gateway/schema.graphql (1)
1209-1215: Gateway VideoBlock now uses Float, but mutation inputs remain Int-only
VideoBlock.startAtandendAtbeingFloathere is correct and consistent with the services, but the federatedVideoBlockCreateInput/VideoBlockUpdateInputtypes still expose these fields asInt. That prevents callers from sending fractional seconds via the gateway even though the backing services and DB support them.Recommend updating the corresponding input types in the api-journeys-modern (and any legacy) schemas to
Float, then re-running schema composition so this gateway schema reflects the new input types.apis/api-journeys/src/app/modules/block/video/video.graphql (1)
44-50: Type change to Float is consistent; consider clarifying units in docsUsing
FloatforstartAt/endAthere is consistent with the rest of the stack and enables fractional seconds. As a small improvement, consider updating the descriptions to explicitly say “seconds (supports fractions)” so API consumers understand the units and precision.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
apis/api-gateway/schema.graphql(1 hunks)apis/api-journeys-modern/schema.graphql(1 hunks)apis/api-journeys-modern/src/schema/block/video/video.ts(1 hunks)apis/api-journeys/schema.graphql(1 hunks)apis/api-journeys/src/app/modules/block/video/video.graphql(1 hunks)libs/prisma/journeys/db/migrations/20251208033407_20251208033406/migration.sql(1 hunks)libs/prisma/journeys/db/schema.prisma(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
**/*.{ts,tsx,js,jsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names.
Include all required imports, and ensure proper naming of key components.
Files:
apis/api-journeys-modern/src/schema/block/video/video.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
Define a type if possible.
Files:
apis/api-journeys-modern/src/schema/block/video/video.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: mikeallisonJS
Repo: JesusFilm/core PR: 7486
File: apis/api-journeys-modern/schema.graphql:1068-1074
Timestamp: 2025-08-20T21:51:25.797Z
Learning: During the refactor to migrate types to modern, maintain consistency with existing api-journeys schema types, including PlausibleStatsAggregateValue.change as Int to match the original implementation.
📚 Learning: 2025-08-20T21:51:25.797Z
Learnt from: mikeallisonJS
Repo: JesusFilm/core PR: 7486
File: apis/api-journeys-modern/schema.graphql:1068-1074
Timestamp: 2025-08-20T21:51:25.797Z
Learning: During the refactor to migrate types to modern, maintain consistency with existing api-journeys schema types, including PlausibleStatsAggregateValue.change as Int to match the original implementation.
Applied to files:
apis/api-journeys/schema.graphqlapis/api-journeys-modern/schema.graphqlapis/api-journeys/src/app/modules/block/video/video.graphqlapis/api-journeys-modern/src/schema/block/video/video.tsapis/api-gateway/schema.graphqllibs/prisma/journeys/db/schema.prisma
📚 Learning: 2025-08-20T21:51:25.797Z
Learnt from: mikeallisonJS
Repo: JesusFilm/core PR: 7486
File: apis/api-journeys-modern/schema.graphql:1068-1074
Timestamp: 2025-08-20T21:51:25.797Z
Learning: Before suggesting type changes during API migrations, always verify the existing implementation to ensure consistency is maintained. The migration to api-journeys-modern should preserve the same field types as the original api-journeys schema.
Applied to files:
apis/api-journeys/schema.graphqlapis/api-journeys-modern/schema.graphql
📚 Learning: 2025-08-20T23:32:18.117Z
Learnt from: mikeallisonJS
Repo: JesusFilm/core PR: 7486
File: apis/api-journeys-modern/schema.graphql:199-205
Timestamp: 2025-08-20T23:32:18.117Z
Learning: The ChatButton.link field in api-journeys is actually nullable (String), not non-nullable (String!) as previously learned. When migrating GraphQL types from api-journeys to api-journeys-modern, the ChatButton.link field should remain nullable to match the existing api-journeys schema.
Applied to files:
apis/api-journeys-modern/schema.graphql
📚 Learning: 2025-12-03T15:21:24.876Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-03T15:21:24.876Z
Learning: Applies to apps/resources/src/components/**/*Video*.{ts,tsx} : Components like `VideoContentHero` depend on the full provider stack (VideoProvider, WatchProvider, PlayerProvider) plus video.js and mux metadata; preserve existing contracts for autoplay, subtitles, and analytics
Applied to files:
apis/api-journeys/src/app/modules/block/video/video.graphqlapis/api-journeys-modern/src/schema/block/video/video.ts
📚 Learning: 2025-02-04T16:36:58.743Z
Learnt from: Kneesal
Repo: JesusFilm/core PR: 5100
File: apps/api-journeys-modern/src/schema/blocks/card/card.zod.ts:0-0
Timestamp: 2025-02-04T16:36:58.743Z
Learning: In the CardBlockSchema (apps/api-journeys-modern/src/schema/blocks/card/card.zod.ts), the parentBlockId field should remain nullable, despite the description indicating it can only be a child of a step block.
Applied to files:
libs/prisma/journeys/db/schema.prisma
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Deploy Preview (watch, 8475/merge, pull_request, 22)
- GitHub Check: Deploy Preview (short-links, 8475/merge, pull_request, 22)
- GitHub Check: Deploy Preview (resources, 8475/merge, pull_request, 22)
- GitHub Check: Deploy Preview (journeys-admin, 8475/merge, pull_request, 22)
- GitHub Check: Deploy Preview (videos-admin, 8475/merge, pull_request, 22)
- GitHub Check: Deploy Preview (journeys, 8475/merge, pull_request, 22)
- GitHub Check: test (22, 3/3)
- GitHub Check: lint (22)
🔇 Additional comments (4)
libs/prisma/journeys/db/migrations/20251208033407_20251208033406/migration.sql (1)
1-3: DB type change to DOUBLE PRECISION looks correctAltering
Block.startAtandBlock.endAttoDOUBLE PRECISIONis consistent with the PrismaFloat?change and should safely upcast existing integer data.libs/prisma/journeys/db/schema.prisma (1)
563-564: Prisma Block.startAt/endAt Float? matches the new timing semanticsChanging
Block.startAtandBlock.endAttoFloat?aligns with the DB migration and GraphQL schema updates, enabling fractional-second offsets while preserving nullability.apis/api-journeys-modern/src/schema/block/video/video.ts (1)
27-32: Switch to exposeFloat matches the new DB/GraphQL typesExposing
startAtandendAtasFloatwith the same nullability correctly reflects the updated Prisma model and schema, and will surface fractional seconds to clients.It’s worth double-checking that any corresponding create/update mutation schemas and validations (e.g., Zod/Pothos input types) are also updated to accept floats so values round-trip cleanly.
apis/api-journeys/schema.graphql (1)
1231-1318: Consistency across federated schemas is verified; VideoTriggerBlock.triggerStart discrepancy is intentional.The
VideoBlocktype changesstartAtandendAtfromInttoFloatacross all federated schemas (api-journeys,api-journeys-modern, andapi-gateway). The type change is consistently applied and supports fractional second precision for video playback timing.The
VideoTriggerBlock.triggerStartfield remainsInt!by design—it serves a different purpose (trigger timing within a journey flow) compared to video playback timing, so the type difference is appropriate and intentional.
|
Found 1 test failure on Blacksmith runners: Failure
|
| align String? | ||
| startAt Int? | ||
| endAt Int? | ||
| startAt Float? |
There was a problem hiding this comment.
I would use Decimal here
|
This pull request has been marked stale due to 28 days without activity. It will be closed in 14 days unless updated. |
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.