Add ETH and ROLV to Language Lookup Dropdown#1791
Conversation
📝 WalkthroughWalkthroughAdds ETH and ROLV columns to the Language lookup: updates GraphQL fragment to fetch code fields, extends LookupField to accept custom option rendering, implements a Paper-based dropdown with header and fixed-width code columns, and adds Jest/RTL tests covering rendering and search results. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/form/Lookup/Language/LanguageField.tsx`:
- Around line 1-2: The import for MUI's Box in LanguageField.tsx is from
'@mui/system' but should come from '@mui/material'; update the import statement
so Box is imported alongside Paper and PaperProps from '@mui/material' (i.e.,
ensure the module specifier for Box matches Paper/PaperProps), then run the
linter/build to confirm no other import inconsistencies remain.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b32d45da-2af2-4516-82a0-365b333c2e9d
📒 Files selected for processing (4)
src/components/form/Lookup/Language/LanguageField.test.tsxsrc/components/form/Lookup/Language/LanguageField.tsxsrc/components/form/Lookup/Language/LanguageLookup.graphqlsrc/components/form/Lookup/LookupField.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/components/form/Lookup/Language/LanguageField.tsx (1)
83-83: Nullable code values will render as empty — verify this is the intended UX.Both
ethnologue.code.valueandregistryOfLanguageVarietiesCode.valueare typed asSecuredStringNullableper the GraphQL fragment, meaning they can benull. The current implementation silently renders nothing for null values, which creates an empty column cell. If a placeholder (e.g.,—orN/A) is preferred for clarity, consider adding a fallback.Optional: add fallback for null codes
> - {option.ethnologue.code.value} + {option.ethnologue.code.value ?? '—'} </Box>> - {option.registryOfLanguageVarietiesCode.value} + {option.registryOfLanguageVarietiesCode.value ?? '—'} </Box>Also applies to: 94-94
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/form/Lookup/Language/LanguageField.tsx` at line 83, The Ethnologue/registry code fields are SecuredStringNullable and currently render empty when null; update the LanguageField rendering to provide a clear fallback (e.g., "—" or "N/A") instead of an empty cell by checking option.ethnologue.code.value and option.registryOfLanguageVarietiesCode.value before outputting them; modify the JSX in LanguageField (where it references option.ethnologue.code.value and option.registryOfLanguageVarietiesCode.value) to render the value if present or the chosen placeholder when null.src/components/form/Lookup/Language/LanguageField.test.tsx (1)
129-156: Consider adding a test for the "Create" option rendering.The
renderOptioninLanguageField.tsxhandles a string option case withCreate "${option}". This path isn't covered by the current tests. Adding coverage would ensure the create-new-language flow renders correctly.Example test for create option
it('shows create option for unmatched input', async () => { const input = setup([searchMock('Xyz', [])]); // empty results search(input, 'Xyz'); await waitFor(() => { expect(screen.getByText('Create "Xyz"')).toBeInTheDocument(); }); });Note: This may require the
useSessionmock to return appropriate powers (e.g.,['CreateLanguage']) for the create option to appear.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/form/Lookup/Language/LanguageField.test.tsx` around lines 129 - 156, Add a test that covers the string-option "Create" branch in LanguageField's renderOption by simulating no search results and an unmatched input so the UI shows Create "Xyz": update the test (LanguageField.test.tsx) to call setup and search (using existing setup, search and searchMock helpers) with an empty result array for the query (e.g., 'Xyz'), mock useSession to include the CreateLanguage permission/power so the create option is allowed, then waitFor and assert that the text Create "Xyz" is in the document; focus on exercising the renderOption string branch and reuse setup/searchMock to avoid duplicating wiring.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/form/Lookup/Language/LanguageField.test.tsx`:
- Around line 129-156: Add a test that covers the string-option "Create" branch
in LanguageField's renderOption by simulating no search results and an unmatched
input so the UI shows Create "Xyz": update the test (LanguageField.test.tsx) to
call setup and search (using existing setup, search and searchMock helpers) with
an empty result array for the query (e.g., 'Xyz'), mock useSession to include
the CreateLanguage permission/power so the create option is allowed, then
waitFor and assert that the text Create "Xyz" is in the document; focus on
exercising the renderOption string branch and reuse setup/searchMock to avoid
duplicating wiring.
In `@src/components/form/Lookup/Language/LanguageField.tsx`:
- Line 83: The Ethnologue/registry code fields are SecuredStringNullable and
currently render empty when null; update the LanguageField rendering to provide
a clear fallback (e.g., "—" or "N/A") instead of an empty cell by checking
option.ethnologue.code.value and option.registryOfLanguageVarietiesCode.value
before outputting them; modify the JSX in LanguageField (where it references
option.ethnologue.code.value and option.registryOfLanguageVarietiesCode.value)
to render the value if present or the chosen placeholder when null.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 30e2ee96-8366-4731-a52c-b7c8f2be82bc
📒 Files selected for processing (2)
src/components/form/Lookup/Language/LanguageField.test.tsxsrc/components/form/Lookup/Language/LanguageField.tsx
brentkulwicki
left a comment
There was a problem hiding this comment.
I still don't understand how Etherum applies to Bible translations, but LGTM 😹
Addresses #1701