Changes in translation questions resource#303
Merged
Conversation
📝 WalkthroughWalkthroughAdds TipTap runtime handling and optional ChangesUWTranslationQuestions Resource Support
Sequence Diagram(s)sequenceDiagram
participant ResourcePanel
participant AquiferAPI as fetchGuideContent
participant TextResourceAccordion
participant TipTapRenderer
ResourcePanel->>ResourcePanel: Detect selectedResource.id === "UWTranslationQuestions"
ResourcePanel->>AquiferAPI: fetchGuideContent(missing localizeRefName ids)
AquiferAPI-->>ResourcePanel: guideContents (TipTap docs)
ResourcePanel->>TextResourceAccordion: pass resourceId and guideContents
TextResourceAccordion->>TextResourceAccordion: extract first child node as question
TextResourceAccordion->>TextResourceAccordion: remove first node to form answer tiptapContent
TextResourceAccordion->>TipTapRenderer: render answer tiptapContent
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. Comment |
Joel-Joseph-George
changed the title
Joel-Joseph-George
changed the title
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/resources/components/ResourcePanel.tsx (1)
248-251:⚠️ Potential issue | 🟠 Major | ⚡ Quick winImplement the required TQ-specific empty-state message.
When TQ has no entries, the UI still shows the generic text. Per the PR objective, this branch should show: "No Translation Questions available for this passage."
Suggested fix
) : ( <div className='flex h-full items-center justify-center'> - <p className='text-sm'>No resources available</p> + <p className='text-sm'> + {selectedResource.id === 'UWTranslationQuestions' + ? 'No Translation Questions available for this passage.' + : 'No resources available'} + </p> </div> )}🤖 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/features/resources/components/ResourcePanel.tsx` around lines 248 - 251, The empty-state message in ResourcePanel is using a generic string; change the render branch in the ResourcePanel component that currently returns "<p className='text-sm'>No resources available</p>" so that when the active resource type/context is Translation Questions (TQ) it instead displays "No Translation Questions available for this passage." Locate the conditional branch in ResourcePanel's JSX (the same block shown in the diff) and replace the generic message with the TQ-specific sentence, ensuring the message is shown only when the component's TQ indicator (e.g., props.activeTab, props.resourceType, or whatever prop/state denotes TQ) is present.
🤖 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 `@src/features/resources/components/ResourcePanel.tsx`:
- Around line 178-196: The effect can re-trigger while fetches are in-flight
causing duplicate fetchGuideContent calls for the same id; fix by tracking
in-progress prefetches and skipping IDs already pending or present in
guideContents. Implement a local ref/set (e.g. pendingPrefetchIds via
useRef<Set<string>>()) inside ResourcePanel, check both (item.id in
guideContents) and pendingPrefetchIds.current.has(item.id) before calling
fetchGuideContent, add the id to pendingPrefetchIds.current immediately before
invoking fetchGuideContent, and remove it when the fetch resolves/rejects (wrap
fetchGuideContent(item.id) with .then/.catch to clear the id). This keeps
symbols used in the diff (useEffect, selectedResource.id, localizeRefName,
shouldFetchResources, guideContents, fetchGuideContent) and prevents duplicated
pending resolvers/timeouts.
In `@src/features/resources/components/TextResourceAccordion.tsx`:
- Around line 91-95: The conditional currently shows "Loading question..."
whenever tqQuestionNodes.length === 0 even after fetch finished; update the
render logic in TextResourceAccordion so that it checks loadingGuides[sv.id] (or
a similar loading flag) first and only shows the animated "Loading question..."
when loadingGuides[sv.id] is true, otherwise render a non-loading fallback such
as "No question available" or an empty state when tqQuestionNodes.length === 0;
modify the ternary around tqQuestionNodes.length to incorporate
loadingGuides[sv.id] (and consider adding an error/fetched flag) to avoid a
permanent loading UI for empty or malformed entries.
---
Outside diff comments:
In `@src/features/resources/components/ResourcePanel.tsx`:
- Around line 248-251: The empty-state message in ResourcePanel is using a
generic string; change the render branch in the ResourcePanel component that
currently returns "<p className='text-sm'>No resources available</p>" so that
when the active resource type/context is Translation Questions (TQ) it instead
displays "No Translation Questions available for this passage." Locate the
conditional branch in ResourcePanel's JSX (the same block shown in the diff) and
replace the generic message with the TQ-specific sentence, ensuring the message
is shown only when the component's TQ indicator (e.g., props.activeTab,
props.resourceType, or whatever prop/state denotes TQ) is present.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: d7868429-04ab-4ca7-a478-399fe77d08cf
📒 Files selected for processing (2)
src/features/resources/components/ResourcePanel.tsxsrc/features/resources/components/TextResourceAccordion.tsx
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/resources/components/ResourcePanel.tsx (1)
245-260:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRender the TQ-specific empty-state message.
When
selectedResource.id === 'UWTranslationQuestions'andlocalizeRefNameis empty, this branch still shows the generic"No resources available"copy. That misses the issue requirement to show "No Translation Questions available for this passage." for the no-results case.Suggested fix
- ) : ( + ) : selectedResource.id === 'UWTranslationQuestions' ? ( + <div className='flex h-full items-center justify-center'> + <p className='text-sm'>No Translation Questions available for this passage.</p> + </div> + ) : ( <div className='flex h-full items-center justify-center'> <p className='text-sm'>No resources available</p> </div> )}🤖 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/features/resources/components/ResourcePanel.tsx` around lines 245 - 260, The empty-state branch currently renders a generic "No resources available" when localizeRefName is empty; update the render logic in ResourcePanel (around the TextResourceAccordion block) to check if selectedResource.id === 'UWTranslationQuestions' and localizeRefName.length === 0 and, in that case, render the specific string "No Translation Questions available for this passage." instead of the generic message; otherwise keep the existing generic message for other resource types. Ensure this conditional sits where the current fallback div is returned so TextResourceAccordion remains unchanged.
♻️ Duplicate comments (1)
src/features/resources/components/ResourcePanel.tsx (1)
187-195:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlso skip IDs that are already loading.
pendingPrefetchIdsonly tracks requests started by this effect. The first auto-open path at Lines 169-173, and later manual opens throughhandleAccordionChange(), can already havefetchGuideContent(item.id)in flight before this loop runs.useGuideContent()only dedupes the queue entry, not the pending resolver/timeout bookkeeping, so this still stacks extra waiters for the same id.Suggested fix
localizeRefName.forEach(item => { - if (!(item.id in guideContents) && !pendingPrefetchIds.current.has(item.id)) { + if ( + !(item.id in guideContents) && + !loadingGuides[item.id] && + !pendingPrefetchIds.current.has(item.id) + ) { pendingPrefetchIds.current.add(item.id); fetchGuideContent(item.id) .then(() => { pendingPrefetchIds.current.delete(item.id); }) @@ }, [ selectedResource.id, localizeRefName, shouldFetchResources, guideContents, + loadingGuides, fetchGuideContent, ]);🤖 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/features/resources/components/ResourcePanel.tsx` around lines 187 - 195, The loop is starting prefetches even when a fetch for the same id is already in-flight elsewhere (first auto-open path or handleAccordionChange), causing duplicate waiters; update the prefetch guard to also consult the shared in-flight bookkeeping used by useGuideContent (or extract a shared inFlight set) so you skip ids already loading. Concretely: in ResourcePanel's effect that uses pendingPrefetchIds and fetchGuideContent, add a check against the same in-flight set/resolver registry that useGuideContent updates (or move pendingPrefetchIds into a shared ref exported/used by useGuideContent and handleAccordionChange) and only call fetchGuideContent(item.id) when the id is not in either pendingPrefetchIds.current and the shared in-flight set so duplicate fetches/waiters are prevented.
🤖 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.
Outside diff comments:
In `@src/features/resources/components/ResourcePanel.tsx`:
- Around line 245-260: The empty-state branch currently renders a generic "No
resources available" when localizeRefName is empty; update the render logic in
ResourcePanel (around the TextResourceAccordion block) to check if
selectedResource.id === 'UWTranslationQuestions' and localizeRefName.length ===
0 and, in that case, render the specific string "No Translation Questions
available for this passage." instead of the generic message; otherwise keep the
existing generic message for other resource types. Ensure this conditional sits
where the current fallback div is returned so TextResourceAccordion remains
unchanged.
---
Duplicate comments:
In `@src/features/resources/components/ResourcePanel.tsx`:
- Around line 187-195: The loop is starting prefetches even when a fetch for the
same id is already in-flight elsewhere (first auto-open path or
handleAccordionChange), causing duplicate waiters; update the prefetch guard to
also consult the shared in-flight bookkeeping used by useGuideContent (or
extract a shared inFlight set) so you skip ids already loading. Concretely: in
ResourcePanel's effect that uses pendingPrefetchIds and fetchGuideContent, add a
check against the same in-flight set/resolver registry that useGuideContent
updates (or move pendingPrefetchIds into a shared ref exported/used by
useGuideContent and handleAccordionChange) and only call
fetchGuideContent(item.id) when the id is not in either
pendingPrefetchIds.current and the shared in-flight set so duplicate
fetches/waiters are prevented.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 08d90ca7-dfd6-49d3-b5b9-d5b79e16349b
📒 Files selected for processing (2)
src/features/resources/components/ResourcePanel.tsxsrc/features/resources/components/TextResourceAccordion.tsx
vipinpaul
approved these changes
Jun 11, 2026
Joel-Joseph-George
temporarily deployed
to
development
GitHub Actions
Inactive
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
2 participants
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.
closes #256
Summary by CodeRabbit