Website: small cleanups #1046
Website: small cleanups #1046
Conversation
mkue
requested review from
andrashee,
ssandino and
socialincome-dev
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThe pull request removes the Changes
Sequence Diagram(s)sequenceDiagram
participant Page
participant NgoList
participant StatsChecker
participant Translator
Page->>NgoList: Render component
NgoList->>NgoList: Initialize hardcoded NGO list
loop For each NGO
NgoList->>StatsChecker: Retrieve recipient stats
alt Stats exist
StatsChecker-->>NgoList: Return stats
NgoList->>Translator: Translate NGO info
Translator-->>NgoList: Return translated info
else No stats
NgoList->>NgoList: Skip processing
end
end
NgoList->>Page: Render NGO list
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
website/src/app/[lang]/[region]/(website)/partners/(sections)/ngolist.tsx (1)
39-40: Consider making NGO slugs configurable.While hardcoding simplifies the code, consider moving the NGO slugs to a configuration file for better maintainability.
-const ngos = ['aurora', 'jamil', 'reachout', 'equal-rights', 'united-polio', 'slaes']; +import { NGO_SLUGS } from '@/config/ngo-config'; +const ngos = NGO_SLUGS;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
shared/src/types/recipient.ts(0 hunks)shared/src/utils/stats/RecipientStatsCalculator.ts(1 hunks)website/src/app/[lang]/[region]/(website)/partners/(sections)/ngolist.tsx(3 hunks)website/src/components/navbar/navbar-client.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- shared/src/types/recipient.ts
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Test functions
- GitHub Check: Test admin
- GitHub Check: Test shared code
- GitHub Check: Test website
- GitHub Check: Prettify
- GitHub Check: Security checks (typescript)
🔇 Additional comments (3)
shared/src/utils/stats/RecipientStatsCalculator.ts (1)
14-14: LGTM! Type definition improvement.The change to
Partial<Record>makes the type more flexible while maintaining the requiredtotalproperty.website/src/app/[lang]/[region]/(website)/partners/(sections)/ngolist.tsx (1)
49-49: LGTM! Good error handling.The null check prevents runtime errors when organization stats are missing.
website/src/components/navbar/navbar-client.tsx (1)
292-301: LGTM! Navigation links reordered.The reordering of navigation links improves the UI flow.
|
Visit the preview URL for this PR (updated for commit 3bb7a89): https://si-admin-staging--pr1046-michael-cleanups-xvv87d8v.web.app (expires Thu, 20 Feb 2025 09:24:12 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: b7b0969384059dce6ea8fad1ee1d1737e54e6676 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
website/src/app/[lang]/[region]/(website)/transparency/finances/page.tsx (1)
7-12: Consider handling loading and error states.The component could benefit from explicit loading and error state handling to improve user experience.
export default function Page() { const { currency } = useI18n(); + const [isLoading, setIsLoading] = useState(true); useEffect(() => { + setIsLoading(true); if (currency) redirect('./finances/' + currency.toLowerCase()); + setIsLoading(false); }, [currency]); + + if (isLoading) return <div>Loading...</div>; + if (!currency) return <div>Error: Currency not available</div>; + + return null; }shared/src/utils/stats/RecipientStatsCalculator.test.ts (2)
148-148: Consider using optional chaining instead of non-null assertions.The non-null assertions (
!) are used to bypass TypeScript's type checking. While this works, it's not the safest approach as it could lead to runtime errors if the properties are actually undefined.Consider using optional chaining for safer property access:
-expect(stats.recipientsCountByOrganisationAndStatus[org1Ref.id]!['total']).toEqual(1); +expect(stats.recipientsCountByOrganisationAndStatus[org1Ref.id]?.['total']).toEqual(1);Also applies to: 151-151, 156-162, 167-168
135-169: Add test coverage for edge cases.The test covers the happy path but lacks coverage for edge cases such as:
- Organizations with no recipients
- Invalid organization IDs
- All possible recipient statuses
Would you like me to generate additional test cases to cover these scenarios?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
shared/src/utils/stats/RecipientStatsCalculator.test.ts(1 hunks)website/src/app/[lang]/[region]/(website)/transparency/finances/[currency]/section-1.tsx(1 hunks)website/src/app/[lang]/[region]/(website)/transparency/finances/page.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Test admin
- GitHub Check: Test website
- GitHub Check: Test functions
- GitHub Check: Test shared code
- GitHub Check: Prettify
- GitHub Check: Security checks (typescript)
🔇 Additional comments (2)
website/src/app/[lang]/[region]/(website)/transparency/finances/page.tsx (1)
11-11: Good addition of the null check!The conditional check prevents unnecessary redirects when currency is not available.
website/src/app/[lang]/[region]/(website)/transparency/finances/[currency]/section-1.tsx (1)
47-47: LGTM! UI refinement improves clarity.The Badge styling changes improve the component's purpose as an informative label by removing unnecessary interactivity and ensuring consistent sizing.
Summary by CodeRabbit