Add optional US regional gear names#369
Conversation
Add US as a distinct gear naming region and allow editors to store complete regional aliases without forcing the canonical brand prefix. Update locale resolution, search suggestions, API validation, translations, documentation, and regression coverage while preserving canonical fallback behavior.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds ChangesRegional Gear Naming
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/gear/rename-gear-dialog.tsx (2)
151-158: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFix broken navigation logic when the user opts to navigate.
The current condition prevents navigation if the user chooses to "Go to item page after renaming" but only changes a regional alias. In that scenario,
shouldNavigateistrue, butresultSlug !== currentSlugisfalse, resulting in a fallback torouter.refresh()that leaves the user stranded on the admin table.Since the
shouldNavigateboolean already incorporates both the default behavior (whenshowNavigateOptionis false) and the checkbox override (whenshowNavigateOptionis true), the extra slug comparison is redundant and breaks the opt-in navigation flow.💡 Proposed fix
const shouldNavigate = showNavigateOption ? navigateAfterRename : resultSlug !== currentSlug; - if (shouldNavigate && resultSlug !== currentSlug) { + if (shouldNavigate) { router.push(`/gear/${resultSlug}`); } else { router.refresh();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/gear/rename-gear-dialog.tsx` around lines 151 - 158, Update the navigation condition in the rename handler to use shouldNavigate directly, removing the redundant resultSlug !== currentSlug check. Preserve router.push for every opted-in or default navigation case, and retain router.refresh only when shouldNavigate is false.
213-218: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAssociate labels with their respective inputs for accessibility.
The dynamically generated
LabelandInputelements for regional aliases lackhtmlForandidattributes. This prevents screen readers from correctly associating the region name label with the input field.♿ Proposed fix
- <Label className="text-muted-foreground text-xs"> + <Label htmlFor={`alias-${region}`} className="text-muted-foreground text-xs"> {region} </Label> <Input + id={`alias-${region}`} placeholder={t("aliasPlaceholder", { region })} value={aliases[region] ?? ""}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/gear/rename-gear-dialog.tsx` around lines 213 - 218, Associate each regional alias Label and Input in the rename dialog by assigning a matching htmlFor and id derived from the region value, ensuring every dynamically generated label targets its corresponding input.
🧹 Nitpick comments (1)
src/lib/specs/registry.tsx (1)
578-582: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider using a consistent label format for regional names.
For consistency with
"EU Name"and"Japan Name"used in the subsequent conditionals, consider labeling the US alias as"US Name".💡 Proposed refactor
const usAlias = findAlias("US"); if (usAlias && viewer !== "US") { - entries.push({ label: "US", name: usAlias }); + entries.push({ label: "US Name", name: usAlias }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/specs/registry.tsx` around lines 578 - 582, Update the US alias entry in the surrounding regional-name logic to use the consistent “US Name” label, matching the existing “EU Name” and “Japan Name” labels while leaving the alias lookup and viewer condition unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@drizzle/0025_ordinary_valeria_richards.sql`:
- Line 1: Remove the generated migration artifacts: delete
drizzle/0025_ordinary_valeria_richards.sql lines 1-1,
drizzle/meta/0025_snapshot.json lines 1-5, and the corresponding journal entry
in drizzle/meta/_journal.json lines 180-185. No replacement migration files are
needed; retain only the schema change.
---
Outside diff comments:
In `@src/components/gear/rename-gear-dialog.tsx`:
- Around line 151-158: Update the navigation condition in the rename handler to
use shouldNavigate directly, removing the redundant resultSlug !== currentSlug
check. Preserve router.push for every opted-in or default navigation case, and
retain router.refresh only when shouldNavigate is false.
- Around line 213-218: Associate each regional alias Label and Input in the
rename dialog by assigning a matching htmlFor and id derived from the region
value, ensuring every dynamically generated label targets its corresponding
input.
---
Nitpick comments:
In `@src/lib/specs/registry.tsx`:
- Around line 578-582: Update the US alias entry in the surrounding
regional-name logic to use the consistent “US Name” label, matching the existing
“EU Name” and “Japan Name” labels while leaving the alias lookup and viewer
condition unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b1f69ef7-b63c-4239-ba74-98056ea74083
📒 Files selected for processing (34)
docs/camera-specs/regional-naming-system.mddocs/developer-api.mddocs/gear-specification-system.mddocs/mapping-system.mddocs/search-system.mddrizzle/0025_ordinary_valeria_richards.sqldrizzle/meta/0025_snapshot.jsondrizzle/meta/_journal.jsonmessages/de.jsonmessages/en.jsonmessages/es.jsonmessages/fr.jsonmessages/it.jsonmessages/ja.jsonmessages/ms.jsonmessages/zh.jsonsrc/app/[locale]/(admin)/admin/gear/columns.tsxsrc/app/[locale]/(pages)/gear/[slug]/page.tsxsrc/app/[locale]/(pages)/u/_components/lists/list-sortable-items.tsxsrc/app/[locale]/(pages)/u/_components/lists/types.tssrc/components/gear/rename-gear-button.tsxsrc/components/gear/rename-gear-dialog-utils.tssrc/components/gear/rename-gear-dialog.tsxsrc/lib/gear/region.tssrc/lib/locale/locales.tssrc/lib/specs/registry.tsxsrc/server/db/schema.tssrc/server/developer-api/schemas.tssrc/server/user-lists/service.tstests/unit/developer-api-schemas-serializers.test.tstests/unit/gear-naming.test.tstests/unit/locale-market-detection.test.tstests/unit/rename-gear-dialog.test.tstests/unit/search-service.test.ts
💤 Files with no reviewable changes (3)
- src/components/gear/rename-gear-button.tsx
- src/app/[locale]/(pages)/gear/[slug]/page.tsx
- src/app/[locale]/(admin)/admin/gear/columns.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: lint
🧰 Additional context used
📓 Path-based instructions (6)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Update all relevant documentation in
/docswhen code changes could cause documentation drift.
Files:
tests/unit/locale-market-detection.test.tssrc/server/db/schema.tssrc/app/[locale]/(pages)/u/_components/lists/types.tstests/unit/search-service.test.tsmessages/it.jsonsrc/components/gear/rename-gear-dialog-utils.tssrc/app/[locale]/(pages)/u/_components/lists/list-sortable-items.tsxmessages/ms.jsonmessages/es.jsonmessages/fr.jsondrizzle/meta/_journal.jsondocs/search-system.mddocs/developer-api.mddocs/mapping-system.mddrizzle/0025_ordinary_valeria_richards.sqlmessages/zh.jsonsrc/server/developer-api/schemas.tstests/unit/gear-naming.test.tssrc/server/user-lists/service.tsmessages/de.jsonmessages/ja.jsonsrc/lib/locale/locales.tsmessages/en.jsonsrc/lib/gear/region.tstests/unit/developer-api-schemas-serializers.test.tsdocs/gear-specification-system.mdsrc/lib/specs/registry.tsxtests/unit/rename-gear-dialog.test.tsdrizzle/meta/0025_snapshot.jsondocs/camera-specs/regional-naming-system.mdsrc/components/gear/rename-gear-dialog.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Replace new hardcoded user-facing strings with translation keys for all locales.
Files:
tests/unit/locale-market-detection.test.tssrc/server/db/schema.tssrc/app/[locale]/(pages)/u/_components/lists/types.tstests/unit/search-service.test.tssrc/components/gear/rename-gear-dialog-utils.tssrc/app/[locale]/(pages)/u/_components/lists/list-sortable-items.tsxsrc/server/developer-api/schemas.tstests/unit/gear-naming.test.tssrc/server/user-lists/service.tssrc/lib/locale/locales.tssrc/lib/gear/region.tstests/unit/developer-api-schemas-serializers.test.tssrc/lib/specs/registry.tsxtests/unit/rename-gear-dialog.test.tssrc/components/gear/rename-gear-dialog.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Do not introduce direct database access in UI or genericlibmodules; pages, API routes, and server components should prefer importing fromservice/.
Server actions must not be used for fetching; use them only for mutations or other client-to-server actions.
Use TypeScript type checking withnpm run typecheck; for large changes, run the combinednpm run checkvalidation pass.
Files:
tests/unit/locale-market-detection.test.tssrc/server/db/schema.tssrc/app/[locale]/(pages)/u/_components/lists/types.tstests/unit/search-service.test.tssrc/components/gear/rename-gear-dialog-utils.tssrc/app/[locale]/(pages)/u/_components/lists/list-sortable-items.tsxsrc/server/developer-api/schemas.tstests/unit/gear-naming.test.tssrc/server/user-lists/service.tssrc/lib/locale/locales.tssrc/lib/gear/region.tstests/unit/developer-api-schemas-serializers.test.tssrc/lib/specs/registry.tsxtests/unit/rename-gear-dialog.test.tssrc/components/gear/rename-gear-dialog.tsx
src/server/db/schema.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/server/db/schema.ts: Make all database schema changes insrc/server/db/schema.tsusing Drizzle's type-safe schema definitions.
Keep schema changes backwards compatible by default, preferring deprecations over destructive deletions.
Update relevant documentation whenever the database schema changes, includinggear-specification-system.mdfor gear-related schemas andmapping-system.mdfor mapping or relationship logic changes.
Files:
src/server/db/schema.ts
messages/*.json
📄 CodeRabbit inference engine (AGENTS.md)
Maintain translation-key parity across all locale files whenever keys are added, removed, or renamed in
messages/en.json.
Files:
messages/it.jsonmessages/ms.jsonmessages/es.jsonmessages/fr.jsonmessages/zh.jsonmessages/de.jsonmessages/ja.jsonmessages/en.json
drizzle/**
📄 CodeRabbit inference engine (AGENTS.md)
Do not write or modify migration files in
drizzle/; do not run migration-generation or database-push commands as part of schema changes.
Files:
drizzle/meta/_journal.jsondrizzle/0025_ordinary_valeria_richards.sqldrizzle/meta/0025_snapshot.json
🪛 GitHub Actions: Protect migration files / 0_check-migration-files.txt
drizzle/0025_ordinary_valeria_richards.sql
[error] 1-1: CI failed because migration files were detected in the PR. Migration files should not be committed by contributors; only schema changes in src/server/db/schema.ts should be committed, and local testing should use npm run db:push.
🪛 GitHub Actions: Protect migration files / check-migration-files
drizzle/0025_ordinary_valeria_richards.sql
[error] 1-1: CI workflow failed: migration files detected in PR. Migration files should not be committed by contributors. Workflow expects schema changes only in src/server/db/schema.ts and local testing via 'npm run db:push'.
🔇 Additional comments (26)
docs/camera-specs/regional-naming-system.md (4)
3-9: LGTM!
13-15: LGTM!
19-21: LGTM!
25-26: LGTM!docs/developer-api.md (1)
36-38: LGTM!docs/gear-specification-system.md (1)
60-65: LGTM!docs/mapping-system.md (1)
157-158: LGTM!docs/search-system.md (1)
59-59: LGTM!src/server/db/schema.ts (1)
55-60: LGTM!src/lib/gear/region.ts (1)
1-1: LGTM!Also applies to: 42-42
src/lib/locale/locales.ts (1)
48-48: LGTM!Also applies to: 255-258
src/app/[locale]/(pages)/u/_components/lists/list-sortable-items.tsx (1)
33-33: LGTM!src/app/[locale]/(pages)/u/_components/lists/types.ts (1)
18-18: LGTM!src/server/user-lists/service.ts (1)
46-46: LGTM!src/server/developer-api/schemas.ts (1)
34-40: LGTM!tests/unit/developer-api-schemas-serializers.test.ts (1)
30-31: LGTM!Also applies to: 37-41
tests/unit/locale-market-detection.test.ts (1)
9-14: LGTM!src/components/gear/rename-gear-dialog-utils.ts (1)
8-37: LGTM!tests/unit/search-service.test.ts (1)
182-212: LGTM!messages/it.json (1)
161-180: LGTM!messages/ja.json (1)
161-180: LGTM!messages/ms.json (1)
161-180: LGTM!messages/zh.json (1)
161-180: LGTM!tests/unit/rename-gear-dialog.test.ts (1)
13-23: LGTM!Also applies to: 82-87, 121-129, 147-147, 162-181
tests/unit/gear-naming.test.ts (2)
1-54: LGTM!
56-66: 🎯 Functional CorrectnessNo issue here.
buildGearSearchNamealready lowercases and strips punctuation vianormalizeGearSearchText, so these expectations match the implementation.> Likely an incorrect or invalid review comment.
1 participant
Summary
USas a distinct gear naming region, including the generated Drizzle enum migrationWhy
Some products use a different brand in the United States, such as Samyang products sold as Rokinon. A dedicated optional US alias supports that market-specific name without changing the canonical/global catalog identity.
Validation
npm run test— 165 files, 729 tests passedgit diff --check— passedKnown baseline issue
Repository-wide TypeScript checking still reports the existing untouched
tests/unit/spec-registry-i18n.test.ts:257mismatch where the test suppliesstring | numberto numericweightGrams.