feat(qa): add localized git questions and category data#78
Conversation
✅ Deploy Preview for develop-devlovers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughReplaces simple category names with structured, localized categoryData; updates TabsSection to use categoryData and locale-aware labels; makes Accordion list rendering accept heterogeneous ListEntry items with runtime guards; converts seed scripts to data-driven flows and adds quiz upsert/cleanup logic; removes exported slugify and trims seed log emoji/formatting. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 2
🧹 Nitpick comments (2)
frontend/components/q&a/AccordionList.tsx (1)
124-142: Consider extracting shared guard logic.The null/invalid item guards and heterogeneous item handling are duplicated between
renderBulletListandrenderNumberedList. Extracting a shared helper would reduce repetition:🔎 Proposed refactor to reduce duplication
function renderListItem(item: unknown, i: number): ReactNode | null { if (!item || typeof item !== 'object') { return null; } if ('type' in item && item.type === 'listItem') { return ( <li key={i} className="leading-relaxed"> {renderListItemChildren(item.children)} </li> ); } return ( <li key={i} className="leading-relaxed"> {renderListItemChildren([item as ListItemChild])} </li> ); } function renderBulletList(block: BulletListBlock, index: number): ReactNode { return ( <ul key={index} className="list-disc list-outside ml-6 space-y-1 my-2"> {block.children.map(renderListItem)} </ul> ); } function renderNumberedList(block: NumberedListBlock, index: number): ReactNode { return ( <ol key={index} className="list-decimal list-outside ml-6 space-y-1 my-2"> {block.children.map(renderListItem)} </ol> ); }Also applies to: 153-171
frontend/data/category.ts (1)
1-9: Consider adding explicit return type annotation.Adding a return type improves type safety and self-documentation.
🔎 Proposed improvement
+interface CategoryItem { + slug: string; + displayOrder: number; + translations: { + uk: string; + en: string; + pl: string; + }; +} + -const createCategory = (slug: string, title: string, displayOrder: number) => ({ +const createCategory = (slug: string, title: string, displayOrder: number): CategoryItem => ({ slug, displayOrder, translations: { uk: title, en: title, pl: title, }, }); + +export type { CategoryItem };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
frontend/components/q&a/AccordionList.tsxfrontend/components/q&a/TabsSection.tsxfrontend/data/category.tsfrontend/db/seed-categories.tsfrontend/db/seed-demo-leaderboard.tsfrontend/db/seed-questions.tsfrontend/db/seed-quiz-from-json.tsfrontend/db/seed-quiz-javascript.tsfrontend/db/seed-quiz-react.tsfrontend/db/seed-quiz-types.tsfrontend/db/seed-quiz-verify.tsfrontend/drizzle/schema.tsfrontend/parse/git.jsonfrontend/parse/questions.jsonfrontend/utils/slugify.tsjson/01-git.json
💤 Files with no reviewable changes (2)
- frontend/drizzle/schema.ts
- frontend/utils/slugify.ts
🧰 Additional context used
🧬 Code graph analysis (3)
frontend/db/seed-demo-leaderboard.ts (1)
frontend/db/seed-users.ts (1)
main(12-58)
frontend/components/q&a/AccordionList.tsx (2)
frontend/db/seed-quiz-types.ts (1)
ol(64-70)frontend/parse/parse.ts (3)
parseBulletListItem(274-330)mergeConsecutiveLists(502-546)parseBulletList(332-362)
frontend/db/seed-quiz-javascript.ts (2)
frontend/drizzle/schema.ts (1)
questions(30-45)frontend/db/schema/questions.ts (1)
questions(14-27)
🔇 Additional comments (14)
frontend/db/seed-demo-leaderboard.ts (1)
6-6: LGTM! Cosmetic logging cleanup.The removal of emoji prefixes from console messages maintains consistency with other seed scripts in this PR.
Also applies to: 62-62, 67-67
frontend/db/seed-quiz-verify.ts (1)
13-13: LGTM! Cosmetic logging improvements.The log message updates improve consistency across seed scripts without altering verification logic.
Also applies to: 15-15, 47-47, 66-66, 96-96, 103-103, 122-122, 126-126
frontend/db/seed-questions.ts (1)
23-23: LGTM! Minor formatting cleanup.Punctuation normalization and trailing newline addition are appropriate housekeeping changes.
Also applies to: 65-65
frontend/db/seed-quiz-types.ts (1)
22-28: LGTM! Formatting improvements.The formatting changes improve readability while preserving the structural shape and behavior of all exported types and helpers.
Also applies to: 42-48, 58-61, 66-69
frontend/db/seed-quiz-javascript.ts (1)
187-187: LGTM! Consistent logging cleanup.The console message updates align with the broader cleanup effort across seed scripts.
Also applies to: 194-194, 200-200, 209-209, 217-217, 224-224, 230-230
frontend/components/q&a/AccordionList.tsx (1)
176-186: Defensive guards look solid.The updated signature accepting
undefinedand the early-return guards prevent crashes when encountering malformed list data. This aligns with the PR objective to harden Q&A rendering.frontend/db/seed-categories.ts (2)
4-4: LGTM! Solid migration to data-driven category seeding.The shift from
categoryNamestocategoryDataenables structured metadata (slug, displayOrder) and localized translations. The seeding logic correctly handles:
- Per-item slug and display order
- Locale-specific translations from
item.translations- Conflict handling via
onConflictDoNothingAlso applies to: 9-35
40-43: LGTM! Proper error handling.The
.catchblock withprocess.exit(1)ensures seed failures are logged and signal non-zero exit status.frontend/db/seed-quiz-react.ts (2)
895-907: LGTM!The multi-line formatting improves readability. No functional changes.
977-984: LGTM!Consistent formatting for console output and removal of emoji for cleaner logs.
Also applies to: 1001-1001
frontend/db/seed-quiz-from-json.ts (3)
69-79: LGTM!Good addition of category lookup with clear error messaging directing users to run
seed:categoriesfirst.
86-94: Good safety check for existing quiz attempts.This prevents accidental data loss when re-seeding a quiz that users have already interacted with.
96-101: The code is correct and does not require changes. All child tables in the schema haveonDelete: 'cascade'constraints:
quizQuestionContent.quizQuestionId→quizQuestions.idwithonDelete: 'cascade'quizAnswers.quizQuestionId→quizQuestions.idwithonDelete: 'cascade'quizAnswerTranslations.quizAnswerId→quizAnswers.idwithonDelete: 'cascade'When deleting
quizQuestions, the database automatically cascades these deletions to all child records, so the deletion order at lines 96-101 is safe and will not violate foreign key constraints.frontend/data/category.ts (1)
11-22: LGTM!The data structure is well-organized with sequential
displayOrdervalues. Using the same title across locales is appropriate for technology names that typically remain untranslated.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/components/q&a/TabsSection.tsx (1)
12-16: Data-driven category handling looks good.The migration to
categoryDatawith derivedCATEGORY_SLUGSis clean. However, the fallback value'html'on line 15 appears inconsistent—ifcategoryDatais empty, falling back to'html'assumes it exists, but if it's non-empty,CATEGORY_SLUGS[0]would be'git'(per the current data). Consider using a consistent fallback:-const DEFAULT_CATEGORY = CATEGORY_SLUGS[0] || 'html'; +const DEFAULT_CATEGORY = CATEGORY_SLUGS[0] ?? 'git';Or simply trust that
categoryDatais never empty if that's guaranteed by design.frontend/components/q&a/AccordionList.tsx (1)
205-227: Nested list rendering lacks the same defensive handling as top-level lists.In
renderBulletListandrenderNumberedList, you useisListItemBlockto handle bothListItemBlockand raw entries. However, the nested list rendering here (lines 208-212, 220-224) assumes all items inchild.childrenareListItemBlockwith a.childrenproperty:{child.children.map((item, j) => ( <li key={j} className="leading-relaxed"> {renderListItemChildren(item.children)} // assumes item.children exists </li> ))}Since
child.childrenis nowListEntry[], anitemcould be a rawListItemChildwithout a.childrenproperty, causingitem.childrento beundefined. WhilerenderListItemChildrenhandlesundefined, this creates inconsistency and may produce empty<li>elements for non-ListItemBlockentries.Consider applying the same pattern used in the top-level list functions.
🔎 Proposed fix
if ('type' in child && child.type === 'bulletList') { return ( <ul key={i} className="list-disc list-outside ml-6 space-y-1 mt-1"> - {child.children.map((item, j) => ( - <li key={j} className="leading-relaxed"> - {renderListItemChildren(item.children)} - </li> - ))} + {child.children.map((item, j) => { + if (!item || typeof item !== 'object') return null; + if (isListItemBlock(item)) { + return ( + <li key={j} className="leading-relaxed"> + {renderListItemChildren(item.children)} + </li> + ); + } + return ( + <li key={j} className="leading-relaxed"> + {renderListItemChildren([item])} + </li> + ); + })} </ul> ); } if ('type' in child && child.type === 'numberedList') { return ( <ol key={i} className="list-decimal list-outside ml-6 space-y-1 mt-1"> - {child.children.map((item, j) => ( - <li key={j} className="leading-relaxed"> - {renderListItemChildren(item.children)} - </li> - ))} + {child.children.map((item, j) => { + if (!item || typeof item !== 'object') return null; + if (isListItemBlock(item)) { + return ( + <li key={j} className="leading-relaxed"> + {renderListItemChildren(item.children)} + </li> + ); + } + return ( + <li key={j} className="leading-relaxed"> + {renderListItemChildren([item])} + </li> + ); + })} </ol> ); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/components/q&a/AccordionList.tsxfrontend/components/q&a/TabsSection.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/components/q&a/TabsSection.tsx (2)
frontend/data/category.ts (1)
categoryData(11-22)frontend/components/ui/tabs.tsx (3)
TabsList(66-66)TabsTrigger(66-66)TabsContent(66-66)
frontend/components/q&a/AccordionList.tsx (1)
frontend/db/seed-quiz-types.ts (1)
ol(64-70)
🔇 Additional comments (7)
frontend/components/q&a/TabsSection.tsx (3)
37-41: LGTM!The category validation against
CATEGORY_SLUGSwith fallback toDEFAULT_CATEGORYis correct and handles invalid URL parameters gracefully.
159-168: Grid column fix applied; verify TypeScript typing for dynamic locale access.The
grid-cols-10now correctly matches the 10 categories. The locale fallback chain (locale → en → slug) is sensible for internationalization.Note: The dynamic property access
category.translations[locale]may produce a TypeScript error if thetranslationstype doesn't include an index signature. Verify that the type definition incategory.tsallows arbitrary string keys or explicitly includes supported locales.
170-186: LGTM!The data-driven
TabsContentrendering is clean and maintains the existing loading/content/empty-state logic correctly.frontend/components/q&a/AccordionList.tsx (4)
27-44: Type definitions correctly model heterogeneous list structures.The
ListEntryunion type and updatedchildrentypes for list blocks properly accommodate both wrapped (ListItemBlock) and unwrapped (ListItemChild) entries, enabling flexible data handling.
80-87: LGTM!The type guard is well-implemented with proper null/object checks before property access.
132-156: LGTM!The defensive checks and branching logic correctly handle both
ListItemBlockand rawListItemChildentries, preventing runtime errors from malformed data.
158-185: LGTM!Consistent implementation with
renderBulletList, maintaining the same defensive patterns.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/components/q&a/AccordionList.tsx (1)
80-109: LGTM with a note on the fallback wrapping behavior.The type guard
isListItemBlockis correctly implemented with proper null and type checks. TherenderListEntriesfunction handles heterogeneous list entries defensively.One observation: lines 103-107 wrap non-
ListItemBlockentries in a single-element array before passing torenderListItemChildren. This effectively treats bareTextNode/CodeBlock/nested lists as if they were wrapped in alistItem. While this prevents crashes on malformed data, it may silently paper over data structure issues. Consider logging a warning when this fallback is triggered during development to help identify data problems.Optional: Add development warning for fallback wrapping
if (isListItemBlock(item)) { return ( <li key={i} className="leading-relaxed"> {renderListItemChildren(item.children)} </li> ); } + if (process.env.NODE_ENV === 'development') { + console.warn('Encountered non-ListItemBlock entry in list:', item); + } + return ( <li key={i} className="leading-relaxed"> {renderListItemChildren([item])} </li> );
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/components/q&a/AccordionList.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/components/q&a/AccordionList.tsx (1)
frontend/db/seed-quiz-types.ts (2)
ol(64-70)ul(56-62)
🔇 Additional comments (3)
frontend/components/q&a/AccordionList.tsx (3)
154-171: LGTM! Clean delegation to the new rendering helper.The updates to
renderBulletListandrenderNumberedListcorrectly delegate torenderListEntries, maintaining consistency with the new type structure. The separation of concerns makes the code more maintainable.
173-215: Excellent defensive hardening of list rendering.The updates to
renderListItemChildrenadd comprehensive guards against malformed data:
- Early return for undefined or non-array children (lines 176-178)
- Skip null or non-object child items (lines 181-183)
- Validate nested list children before recursion (lines 192-194, 203-205)
These changes directly support the PR objective to "harden Q&A list rendering to avoid crashes on malformed list data." The recursive structure correctly handles arbitrary nesting depth via
renderListEntries.
27-44: The type changes align correctly with actual data structure. The code handles both well-formed and malformed list items found in questions.json. While seed helpers consistently createlistItemwrappers, the actual data contains entries with missingtypefields (children withouttype: 'listItem'). TheListEntryunion permittingListItemChilddirectly is necessary to accommodate this malformed data, making the type modification justified and correct.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/components/q&a/TabsSection.tsx (1)
12-15: LGTM—Clean data-driven refactoring.The migration to
categoryDatawith derivedCATEGORY_SLUGSandDEFAULT_CATEGORYis well-structured and improves maintainability. The fallback to'html'inDEFAULT_CATEGORYis defensive, though since the first category incategoryDatais'git', you might optionally align the fallback for consistency (e.g.,|| 'git'), but this is purely cosmetic since the array will always have entries.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/components/q&a/TabsSection.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/components/q&a/TabsSection.tsx (1)
frontend/data/category.ts (1)
categoryData(11-22)
🔇 Additional comments (4)
frontend/components/q&a/TabsSection.tsx (4)
32-34: LGTM—Defensive locale validation.The runtime check constraining
localeKeyto'uk' | 'en' | 'pl'with an'en'fallback is sound. The type assertion is safe given the.includes()guard and ensures type-safe access tocategory.translations[localeKey]downstream.
40-44: LGTM—Correct active state initialization.The validation of
categoryFromUrlagainstCATEGORY_SLUGSwith a fallback toDEFAULT_CATEGORYcorrectly prevents invalid categories from URL parameters and aligns with the data-driven category model.
163-170: LGTM—Well-structured locale-aware tab rendering.The dynamic mapping over
categoryDatais clean, and the translation fallback chain (category.translations[localeKey] ?? category.translations.en ?? category.slug) ensures labels display correctly across all supported locales. The grid column count correctly matches the number of categories.
173-189: LGTM—Comprehensive TabsContent rendering.The data-driven
TabsContentgeneration correctly handles all states (loading, populated, empty) with appropriate conditional messaging for search vs. no-search scenarios. The integration withAccordionListand translation keys is clean.
feat(qa): add localized git questions and category data
1 participant
Git Q&A localization + category updates
Summary
Changes
categoryData(slug + translations) in Q&A tabs.categoryData.frontend/parse/questions.jsonwith Git Q&A in three locales.frontend/parse/git.jsonand unusedfrontend/utils/slugify.ts.Data / Seeding
npm run seed:categoriesnpm run seed:questions(Git Q&A uk/en/pl)Summary by CodeRabbit
Bug Fixes
New Features
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.