feat: template details page#351
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
4e0926b to
53bf652
Compare
ca9b9d0 to
d2d5b04
Compare
17112e2 to
422cd2b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 478a8fe671
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | 'names' | ||
| | 'createdAt' | ||
| | 'updatedAt' | ||
| | 'createdBy' | ||
| | 'lastSpawnedAt' | ||
| | 'spawnCount' | ||
| | 'buildCount' |
There was a problem hiding this comment.
🟣 Pre-existing nit cleanup: createDefaultTemplatesRepository.getDefaultTemplatesCached in src/core/modules/templates/repository.server.ts still assigns createdBy: null on each mapped DefaultTemplate, even though this PR removed createdBy from the Template Pick (models.ts) and from every mock entry (mock-data.ts). It's dead — no compile error, no consumer reads it — but the assignment should be dropped to finish the cleanup the PR started.
Extended reasoning...
What the issue is. This PR removes createdBy from the Pick<> in src/core/modules/templates/models.ts (lines 14–19 in the diff) and removes createdBy: … / createdBy: null entries from every mock template in src/configs/mock-data.ts. However, the matching mapping in createDefaultTemplatesRepository.getDefaultTemplatesCached (around src/core/modules/templates/repository.server.ts:585) was not touched and still does createdBy: null inside its .map(...) callback when building templates: DefaultTemplate[].\n\nWhy this is not a build break. The original report hypothesized a TypeScript excess-property error, and a verifier confirmed via bunx tsc --noEmit that no such error fires. The literal flows through generic inference into the callback's return type U, the resulting U[] is then structurally assignable to DefaultTemplate[] (extra properties are fine for subtyping at the array level, and excess-property freshness doesn't apply once the literal has been inferred through a generic). Vercel's "Ready" deployment status is consistent with that. So the original "build break" framing is incorrect — I'm filing this corrected.\n\nWhat the actual impact is. Each returned DefaultTemplate object carries a createdBy: null property that the type system no longer surfaces and no consumer reads. The OpenAPI DefaultTemplate response shape doesn't include it, and the dashboard renders templates from the typed shape, so there is zero behavioral effect at runtime. This is dead/stale code, not a bug that affects users — which is why I'm marking it pre-existing/nit rather than normal.\n\nStep-by-step proof.\n1. Template in models.ts (post-PR) is Pick<…, 'templateID' | 'buildID' | … | 'updatedAt' | 'lastSpawnedAt' | …> — no createdBy.\n2. DefaultTemplate = Template & { isDefault: true; defaultDescription?: string } — also no createdBy.\n3. In repository.server.ts, the mapped object literal still includes createdBy: null, between updatedAt: t.createdAt, and lastSpawnedAt: t.createdAt,.\n4. TS narrows the callback return to the literal's shape, then unifies into U[] and finally checks U[] assignability against DefaultTemplate[] — that check accepts extra properties (no fresh-literal excess-property check applies post-inference), so the file compiles cleanly.\n5. At runtime, every entry of templates carries an untyped createdBy: null property. Nothing in the dashboard reads it, so it just sits there.\n\nHow to fix it. Drop the createdBy: null, line in the .map callback alongside the matching removals in models.ts and mock-data.ts. That completes the cleanup with no behavior change.
a5b893f to
8c2ca01
Compare
Add template detail page with Overview, Builds, and Tags tabs.
Changes:
Other UI fixes: