feat: move taxonomy loading to server route - #185
Merged
Conversation
Contributor
WalkthroughThis PR centralizes taxonomy option fetching into a new client hook ChangesTaxonomy Data Consolidation
Sequence Diagram(s)sequenceDiagram
participant Parent as RequirementForm / SpecificationLocalRequirementForm
participant Hook as useTaxonomyOptions
participant API as /api/* endpoints
participant Fields as RequirementFormFields
Parent->>Hook: call useTaxonomyOptions(typeId)
Hook->>API: parallel fetch static taxonomy endpoints
API-->>Hook: respond arrays
Hook->>Parent: return taxonomyOptions
Parent->>Fields: pass taxonomyOptions prop
Fields->>Fields: populate selects/checkboxes with arrays
Parent->>Hook: (on typeId change) request quality-characteristics
Hook->>API: fetch /api/quality-characteristics?typeId=...
API-->>Hook: return qc (or abort/error)
Hook-->>Parent: updated qualityCharacteristics
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unit/use-taxonomy-options.test.tsx (1)
53-200: ⚡ Quick winAdd regression tests for stale-response and rejected QC fetch paths.
Current tests don’t cover the two failure modes in
typeIdupdates: out-of-order responses and thrown fetch errors for/api/quality-characteristics. Adding those would lock in the hook’s expected behavior.🤖 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 `@tests/unit/use-taxonomy-options.test.tsx` around lines 53 - 200, Add two tests to tests/unit/use-taxonomy-options.test.tsx covering stale-response and rejected QC fetch for useTaxonomyOptions: (1) "ignores stale quality-characteristics responses when typeId changes" — mock fetch so the first QC request returns a promise you resolve after the second QC request completes (use controlled Promises), rerender from typeId '1' to '2', resolve promises out-of-order and assert only the latest QC (for typeId '2') is applied to result.current.qualityCharacteristics; (2) "handles thrown fetch error for quality-characteristics" — mock fetch to throw/reject when URL includes '/api/quality-characteristics' during a typeId change and assert qualityCharacteristics becomes [] and no unhandled promise errors occur. Reference useTaxonomyOptions, the '/api/quality-characteristics' URL, renderHook/rerender/act helpers and fetchMock.mockImplementation to locate where to implement the mocks.
🤖 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 `@hooks/useTaxonomyOptions.ts`:
- Around line 133-155: fetchQualityCharacteristics can produce stale state and
uncaught rejections; update it to use an AbortController (or a requestId ref)
and wrap the fetch/parse in try/catch, abort previous requests in the effect
cleanup, and only call setQualityCharacteristics when the response matches the
latest request and the controller is not aborted. Concretely: in the useCallback
fetchQualityCharacteristics(tid) create an AbortController, pass signal to
fetch, use try { if (!res.ok) handle error; const body = await res.json(); if
(!signal.aborted and tid matches current typeId or requestId ===
latestRef.current) setQualityCharacteristics(body.qualityCharacteristics ?? [])
} catch (err) { if (err.name !== 'AbortError') handle/log error and optionally
setQualityCharacteristics([]) } and in the useEffect that calls
fetchQualityCharacteristics(typeId) abort the previous controller (or increment
latestRef) in its cleanup to prevent out-of-order updates; keep
setQualityCharacteristics as the state setter to update only for the latest
successful response.
---
Nitpick comments:
In `@tests/unit/use-taxonomy-options.test.tsx`:
- Around line 53-200: Add two tests to tests/unit/use-taxonomy-options.test.tsx
covering stale-response and rejected QC fetch for useTaxonomyOptions: (1)
"ignores stale quality-characteristics responses when typeId changes" — mock
fetch so the first QC request returns a promise you resolve after the second QC
request completes (use controlled Promises), rerender from typeId '1' to '2',
resolve promises out-of-order and assert only the latest QC (for typeId '2') is
applied to result.current.qualityCharacteristics; (2) "handles thrown fetch
error for quality-characteristics" — mock fetch to throw/reject when URL
includes '/api/quality-characteristics' during a typeId change and assert
qualityCharacteristics becomes [] and no unhandled promise errors occur.
Reference useTaxonomyOptions, the '/api/quality-characteristics' URL,
renderHook/rerender/act helpers and fetchMock.mockImplementation to locate where
to implement the mocks.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a7d672e3-6a1c-4fdc-af9e-640288f09346
📒 Files selected for processing (5)
components/RequirementForm.tsxcomponents/RequirementFormFields.tsxcomponents/SpecificationLocalRequirementForm.tsxhooks/useTaxonomyOptions.tstests/unit/use-taxonomy-options.test.tsx
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #185 +/- ##
==========================================
- Coverage 60.45% 60.38% -0.07%
==========================================
Files 293 293
Lines 17333 17294 -39
Branches 6665 6538 -127
==========================================
- Hits 10478 10443 -35
+ Misses 6714 6710 -4
Partials 141 141
🚀 New features to boost your workflow:
|
- upgraded lucide-react from ^1.11.0 to ^1.14.0 - upgraded mssql from ^12.5.0 to ^12.5.2 - upgraded next from ^16.2.4 to ^16.2.5 - upgraded next-intl from ^4.9.1 to ^4.11.0 - upgraded react and react-dom from ^19.2.5 to ^19.2.6 - upgraded zod from ^4.3.6 to ^4.4.3 - upgraded @biomejs/biome from ^2.4.13 to ^2.4.14 - upgraded jsdom from ^29.1.0 to ^29.1.1 - upgraded hono from 4.12.15 to 4.12.16 - added ip-address at version 10.2.0 - added icu-minify at version 4.11.0
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Also:
chore: update dependencies to latest versions
Screenshots (if applicable)
Related Issues
Type of Change
Testing
npm run checkpasses locallyChecklist
Checklist
This change is