feat: connect daily tips to backend#90
Conversation
📝 WalkthroughWalkthroughAdds a ChangesDaily Tip API Integration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 2
🤖 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/hooks/useDailyTip.ts`:
- Around line 9-14: The useEffect hook in the getDailyTip promise chain (setTip,
setError, setIsLoading calls) lacks protection against state updates after
component unmount, which can cause memory leaks and warnings. Add a cleanup
function to the useEffect that prevents these state setters from being called
after unmount by using a mounted ref flag that gets set to false in the cleanup
function, then check this flag before executing any setState calls in the
promise handlers.
In `@src/pages/home/dailyTips/titleAndDescription/TitleAndDescription.tsx`:
- Around line 8-9: The TitleAndDescription component currently returns null when
there is an error or no tip data (the condition on line 8-9), which leaves the
UI blank with no user feedback. Instead of returning null in both the error and
no-data cases, render a lightweight fallback message or default copy that keeps
the UI informative and provides clear feedback to the user about the state of
the content.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5b5a53f3-4582-4758-bcf9-31957594482c
📒 Files selected for processing (3)
src/hooks/useDailyTip.tssrc/pages/home/dailyTips/titleAndDescription/TitleAndDescription.tsxsrc/services/api/tips.service.ts
| useEffect(() => { | ||
| getDailyTip() | ||
| .then(setTip) | ||
| .catch(() => setError('error')) | ||
| .finally(() => setIsLoading(false)) | ||
| }, []) |
There was a problem hiding this comment.
Add unmount/cancel protection for async state updates.
On Line 9-14, the pending request can still call setTip/setError/setIsLoading after unmount. Add a cleanup guard (or request cancellation) to avoid stale updates.
Proposed fix
useEffect(() => {
+ let isActive = true
getDailyTip()
- .then(setTip)
- .catch(() => setError('error'))
- .finally(() => setIsLoading(false))
+ .then((data) => {
+ if (isActive) setTip(data)
+ })
+ .catch(() => {
+ if (isActive) setError('error')
+ })
+ .finally(() => {
+ if (isActive) setIsLoading(false)
+ })
+
+ return () => {
+ isActive = false
+ }
}, [])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| getDailyTip() | |
| .then(setTip) | |
| .catch(() => setError('error')) | |
| .finally(() => setIsLoading(false)) | |
| }, []) | |
| useEffect(() => { | |
| let isActive = true | |
| getDailyTip() | |
| .then((data) => { | |
| if (isActive) setTip(data) | |
| }) | |
| .catch(() => { | |
| if (isActive) setError('error') | |
| }) | |
| .finally(() => { | |
| if (isActive) setIsLoading(false) | |
| }) | |
| return () => { | |
| isActive = false | |
| } | |
| }, []) |
🤖 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/hooks/useDailyTip.ts` around lines 9 - 14, The useEffect hook in the
getDailyTip promise chain (setTip, setError, setIsLoading calls) lacks
protection against state updates after component unmount, which can cause memory
leaks and warnings. Add a cleanup function to the useEffect that prevents these
state setters from being called after unmount by using a mounted ref flag that
gets set to false in the cleanup function, then check this flag before executing
any setState calls in the promise handlers.
| if (isLoading) return null | ||
| if (error || !tip) return null |
There was a problem hiding this comment.
Don’t render a blank section on error/no-data.
On Line 8-9, return null for error or missing tip removes all feedback. Please render a lightweight fallback message (or previous default copy) so the UI stays informative.
Proposed fix
- if (isLoading) return null
- if (error || !tip) return null
+ if (isLoading) return null
+ if (error || !tip) {
+ return (
+ <>
+ <SGLTypography variant="smallTitle" styles={titleAndDescriptionStyles.title}>
+ Daily tip
+ </SGLTypography>
+ <SGLTypography variant="smallText" styles={titleAndDescriptionStyles.description}>
+ Tip is currently unavailable.
+ </SGLTypography>
+ </>
+ )
+ }🤖 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/pages/home/dailyTips/titleAndDescription/TitleAndDescription.tsx` around
lines 8 - 9, The TitleAndDescription component currently returns null when there
is an error or no tip data (the condition on line 8-9), which leaves the UI
blank with no user feedback. Instead of returning null in both the error and
no-data cases, render a lightweight fallback message or default copy that keeps
the UI informative and provides clear feedback to the user about the state of
the content.
| const [error, setError] = useState<string | null>(null) | ||
|
|
||
| useEffect(() => { | ||
| getDailyTip() |
There was a problem hiding this comment.
We are not using useEffect for api calls, we are using tanstack query, check other examples of how we are calling api
2 participants
Description
Please include a summary of the changes and the related issue.
חיברתי את הטיפ היומי לבאקאנד.
יצרתי tips.service.ts עם הקריאה ל-API, hook של useDailyTip שמושך את הדאטה
ועדכנתי את TitleAndDescription שיציג את הדאטה האמיתית.
.
Related Issue(s)
Fixes # (issue number)
Checklist:
Screenshots (if appropriate):
Summary by CodeRabbit