feat: Featured Packs — curated pack templates with seed data and UI section#1886
Conversation
|
@coderabbitai Please review this PR for code quality, best practices, and potential issues. |
Rate Limit Exceeded
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@copilot This PR has merge conflicts with main. Please rebase onto main and resolve the conflicts. |
Deploying packrat-landing with
|
| Latest commit: |
cd883aa
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://de55878e.packrat-landing.pages.dev |
| Branch Preview URL: | https://copilot-add-featured-packs-s.packrat-landing.pages.dev |
Done (commit |
There was a problem hiding this comment.
Pull request overview
Adds a new “Featured Packs” concept to Pack Templates by seeding curated isAppTemplate templates in the API DB and surfacing them in the Expo app with a dedicated horizontal section and updated translations.
Changes:
- Add an idempotent-ish seed script that inserts 6 curated
isAppTemplatepack templates with items. - Add
FeaturedPacksSectionUI and inject it into the Pack Templates list header. - Update English translations to rename “App”/“App Template” to “Featured” and add new Featured Packs strings.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/api/src/db/seed.ts | New seed script that inserts 6 featured pack templates + items. |
| packages/api/package.json | Adds db:seed script to run the new seed file. |
| apps/expo/lib/i18n/locales/en.json | Updates “App” wording to “Featured” and adds new i18n keys for the section. |
| apps/expo/features/pack-templates/screens/PackTemplateListScreen.tsx | Renders FeaturedPacksSection in the All tab header. |
| apps/expo/features/pack-templates/components/FeaturedPacksSection.tsx | New horizontal featured templates strip with cards and “View All”. |
You can also share your feedback on Copilot code review. Take the survey.
| // Check if this featured template already exists (idempotent seed) | ||
| const existing = await seedDb.query.packTemplates.findFirst({ | ||
| where: and( | ||
| eq(schema.packTemplates.id, templateDef.id), | ||
| eq(schema.packTemplates.deleted, false), | ||
| ), | ||
| }); |
There was a problem hiding this comment.
The seed is not fully idempotent when a featured template already exists but is soft-deleted. The lookup only considers rows with deleted=false; if a row exists with the same id and deleted=true, this will attempt to insert a duplicate primary key and the script will fail. Consider checking by id regardless of deleted state, and either skip, undelete/update the existing row, or use an upsert strategy.
| <Pressable onPress={() => router.push('/pack-templates')}> | ||
| <Text className="text-sm text-primary">{t('packTemplates.viewAll')}</Text> | ||
| </Pressable> |
There was a problem hiding this comment.
"View All" currently does router.push('/pack-templates'), which navigates to the same screen the section is rendered on (and may push a duplicate route onto the stack). If the intent is to show all featured templates, this should instead switch the segmented control to the Featured tab (index 1) and reset filters/search, or accept an onViewAll callback from the parent to perform that state change.
| <Text className="text-xs text-muted-foreground"> | ||
| {template.items && isArray(template.items) | ||
| ? `${template.items.length} ${t('packTemplates.items')}` | ||
| : `0 ${t('packTemplates.items')}`} | ||
| </Text> |
There was a problem hiding this comment.
The item count label always uses the plural translation (t('packTemplates.items')), which can render grammatically incorrect strings like "1 items". Match the pluralization logic used in PackTemplateCard by choosing item vs items based on template.items.length.
| @@ -817,7 +817,7 @@ | |||
| "type": "Type", | |||
| "markAsAppTemplate": "Mark as App Template", | |||
| "appTemplateFootnote": "App templates are shown to all users. Option is only available to admins.", | |||
There was a problem hiding this comment.
The UI labels were changed to show "Featured", but appTemplateFootnote still says "App templates are shown to all users..." which is inconsistent/confusing in the updated UI. Update the footnote copy to use the same term users now see (e.g., "Featured templates").
| "appTemplateFootnote": "App templates are shown to all users. Option is only available to admins.", | |
| "appTemplateFootnote": "Featured templates are shown to all users. Option is only available to admins.", |
| const argUserIdRaw = process.argv[2] ? Number.parseInt(process.argv[2], 10) : undefined; | ||
| if (argUserIdRaw !== undefined && Number.isNaN(argUserIdRaw)) { | ||
| throw new Error( | ||
| 'Invalid user ID provided. Please provide a valid numeric user ID, e.g.: bun run seed.ts 1', |
There was a problem hiding this comment.
The CLI usage hint in this error message (bun run seed.ts 1) doesn't match the actual invocation paths documented at the top of the file or the new db:seed script. Update the message to reflect a command users can actually run (e.g., bun run db:seed 1 or bun run ./src/db/seed.ts 1).
| 'Invalid user ID provided. Please provide a valid numeric user ID, e.g.: bun run seed.ts 1', | |
| 'Invalid user ID provided. Please provide a valid numeric user ID, e.g.: NEON_DATABASE_URL=<url> bun run packages/api/src/db/seed.ts 1', |
| }); | ||
| if (!adminUser) { | ||
| throw new Error( | ||
| 'No ADMIN user found in the database. Please provide a user ID as argument: bun run seed.ts <userId>', |
There was a problem hiding this comment.
This error message references bun run seed.ts <userId>, but the script is located at src/db/seed.ts and is also exposed via the db:seed package script. Adjust the message so the suggested command is valid for typical usage.
| 'No ADMIN user found in the database. Please provide a user ID as argument: bun run seed.ts <userId>', | |
| 'No ADMIN user found in the database. Please provide a user ID as argument: bun run db:seed <userId>', |
… data Co-authored-by: andrew-bierman <94939237+andrew-bierman@users.noreply.github.com>
andrew-bierman
force-pushed
the
copilot/add-featured-packs-section
branch
from
4308d87 to
389a488
Compare
Coverage Report for Expo Unit Tests Coverage (./apps/expo)
File CoverageNo changed files found. |
Coverage Report for API Unit Tests Coverage (./packages/api)
File CoverageNo changed files found. |
The previous discriminated union erased the schema generic, so every
seedDb.query.* access resolved to {}. The workaround was a lossy
'as ReturnType<typeof drizzlePg>' cast.
Import NodePgDatabase and NeonHttpDatabase generics, parameterize both
with typeof schema, and drop the cast. Both query callers
(users.findFirst, packTemplates.findFirst) type correctly.
Removes nine keys added in #1886 under `packTemplates` that were never referenced anywhere in the monorepo: dayHike, overnight, weekendCamping, multiDay, winterHiking, winterCamping, tripDuration, environment, intendedUse.
…ection feat: Featured Packs — curated pack templates with seed data and UI section
Removes nine keys added in #1886 under `packTemplates` that were never referenced anywhere in the monorepo: dayHike, overnight, weekendCamping, multiDay, winterHiking, winterCamping, tripDuration, environment, intendedUse.
packages/api/src/db/seed.ts) with 6 featured pack typesdb:seedscript topackages/api/package.jsonFeaturedPacksSectioncomponentPackTemplateListScreento show featured packs sectionpackages/api/package.jsonconflict (keptdb:seed+ incorporated main'stest:unit,test:unit:coverage, and@vitest/coverage-v8); brought in all other files added/modified in main (test files, vitest configs, landing page updates)Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.