-
Notifications
You must be signed in to change notification settings - Fork 523
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat: Display total no. of entities with heading in Organization #10092
base: develop
Are you sure you want to change the base?
Feat: Display total no. of entities with heading in Organization #10092
Conversation
Warning Rate limit exceeded@AdityaJ2305 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 3 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request introduces a consistent approach to displaying entity counts across various organization-related tabs. The changes involve modifying localization files and updating multiple components to include a Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/pages/Organization/OrganizationView.tsx (2)
54-65
: Maintain consistency with heading and translation keys.The heading shows "organizations" while viewing child organizations, which might be confusing. Consider changing it to "child organizations" or similar for clarity.
- <h2 className="text-lg font-semibold">{t("organizations")}</h2> + <h2 className="text-lg font-semibold">{t("child_organizations")}</h2>
57-58
: Extract common Badge styling to a shared component.The Badge styling is duplicated across all organization pages. Consider creating a shared component to maintain consistency and reduce duplication.
// src/components/Common/EntityCountBadge.tsx export const EntityCountBadge = ({ count, entity }: { count: number; entity: string }) => ( <Badge className="bg-purple-50 text-purple-700 ml-2 text-sm font-medium rounded-xl px-3 m-3" variant="outline" > {t("entity_count", { count, entity })} </Badge> );Also applies to: 63-63
src/pages/Organization/OrganizationFacilities.tsx (1)
67-69
: Standardize translation key usage.Use the common
entity_count
translation key for consistency with other organization pages.- {t("facility_count", { - count: facilities?.count || 0, - })} + {t("entity_count", { + count: facilities?.count || 0, + entity: "Facility" + })}src/pages/Organization/OrganizationPatients.tsx (1)
102-110
: Consider adding ARIA label for screen readers.The Badge component should have an aria-label for better accessibility.
<Badge className="bg-purple-50 text-purple-700 ml-2 text-sm font-medium rounded-xl px-3 m-3" variant="outline" + aria-label={t("entity_count", { + count: patients?.count || 0, + entity: "Patient" + })} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
public/locale/en.json
(2 hunks)src/components/Facility/FacilityUsers.tsx
(1 hunks)src/pages/Organization/OrganizationFacilities.tsx
(2 hunks)src/pages/Organization/OrganizationPatients.tsx
(2 hunks)src/pages/Organization/OrganizationUsers.tsx
(2 hunks)src/pages/Organization/OrganizationView.tsx
(1 hunks)
👮 Files not reviewed due to content moderation or server errors (2)
- src/components/Facility/FacilityUsers.tsx
- public/locale/en.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
public/locale/en.json (1)
987-988
: Consider handling irregular plurals.The current pluralization simply adds 's' to the entity name, which works for regular English plurals but might not handle irregular cases (e.g., "person" → "people", "facility" → "facilities"). Consider using a more robust pluralization mechanism if irregular plurals are needed.
Example enhancement:
- "entity_count_other": "{{count}} {{entity}}s", + "entity_count_other": "{{count}} {{entityPlural}}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
public/locale/en.json
(2 hunks)src/components/Facility/FacilityUsers.tsx
(1 hunks)src/pages/Organization/OrganizationFacilities.tsx
(2 hunks)src/pages/Organization/OrganizationPatients.tsx
(2 hunks)src/pages/Organization/OrganizationUsers.tsx
(2 hunks)src/pages/Organization/OrganizationView.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/pages/Organization/OrganizationFacilities.tsx
- src/pages/Organization/OrganizationUsers.tsx
- src/components/Facility/FacilityUsers.tsx
- src/pages/Organization/OrganizationView.tsx
- src/pages/Organization/OrganizationPatients.tsx
🔇 Additional comments (1)
public/locale/en.json (1)
987-988
: LGTM! Well-designed localization approach.The new generalized entity count entries provide a flexible and maintainable solution for displaying counts across different types of entities. The implementation follows i18n best practices for handling pluralization.
@coderabbitai, how to implement the nitpick comment |
Tip For best results, initiate chat on the files or code changes. @AdityaJ2305 The nitpick comment suggests improving the pluralization handling in
{
"entity_count_one": "{{count}} {{entity}}",
"entity_count_other": "{{count}} {{entityPlural}}"
}
t("entity_count", {
count: facilities?.count || 0,
entity: "Facility",
entityPlural: "Facilities"
}) This approach will handle irregular plurals correctly (like "person"/"people") instead of just adding 's' to the singular form. Would you like me to help you implement this change across all the files? 🔗 Analysis chain🏁 Scripts executedThe following scripts were executed for the analysis: Script:
Length of output: 5733 |
Hey Team, Do we really need to do that ? It just makes it too complex for simple translation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/pages/Organization/OrganizationView.tsx (1)
57-66
: Extract Badge styling to a reusable component.The Badge styling is duplicated across multiple components. Consider extracting it to a reusable component.
Create a new component
EntityCountBadge
:// src/components/Common/EntityCountBadge.tsx interface EntityCountBadgeProps { count: number | undefined; entity: string; } export function EntityCountBadge({ count, entity }: EntityCountBadgeProps) { const { t } = useTranslation(); return ( <Badge className="bg-purple-50 text-purple-700 ml-2 text-sm font-medium rounded-xl px-3 m-3" variant="outline" > {t("entity_count", { count: count ?? 0, entity, defaultValue: "{{count}} {{entity}}" })} </Badge> ); }🧰 Tools
🪛 GitHub Check: cypress-run (1)
[failure] 61-61:
Argument of type '["entity_count", { count: string | number; entity: string; }]' is not assignable to parameter of type '[key: string | string[], options: TOptionsBase & $Dictionary & { defaultValue: string; }] | [key: string | string[], defaultValue: string, options?: (TOptionsBase & $Dictionary) | undefined] | [key: ...]'.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/Facility/FacilityUsers.tsx
(1 hunks)src/pages/Organization/OrganizationFacilities.tsx
(2 hunks)src/pages/Organization/OrganizationPatients.tsx
(2 hunks)src/pages/Organization/OrganizationUsers.tsx
(2 hunks)src/pages/Organization/OrganizationView.tsx
(1 hunks)
🧰 Additional context used
🪛 GitHub Check: cypress-run (1)
src/components/Facility/FacilityUsers.tsx
[failure] 76-76:
Argument of type '["entity_count", { count: string | number; entity: string; }]' is not assignable to parameter of type '[key: string | string[], options: TOptionsBase & $Dictionary & { defaultValue: string; }] | [key: string | string[], defaultValue: string, options?: (TOptionsBase & $Dictionary) | undefined] | [key: ...]'.
src/pages/Organization/OrganizationFacilities.tsx
[failure] 68-68:
Argument of type '["facility_count", { count: string | number; }]' is not assignable to parameter of type '[key: string | string[], options: TOptionsBase & $Dictionary & { defaultValue: string; }] | [key: string | string[], defaultValue: string, options?: (TOptionsBase & $Dictionary) | undefined] | [key: ...]'.
src/pages/Organization/OrganizationPatients.tsx
[failure] 107-107:
Argument of type '["entity_count", { count: string | number; entity: string; }]' is not assignable to parameter of type '[key: string | string[], options: TOptionsBase & $Dictionary & { defaultValue: string; }] | [key: string | string[], defaultValue: string, options?: (TOptionsBase & $Dictionary) | undefined] | [key: ...]'.
src/pages/Organization/OrganizationUsers.tsx
[failure] 67-67:
Argument of type '["entity_count", { count: string | number; entity: string; }]' is not assignable to parameter of type '[key: string | string[], options: TOptionsBase & $Dictionary & { defaultValue: string; }] | [key: string | string[], defaultValue: string, options?: (TOptionsBase & $Dictionary) | undefined] | [key: ...]'.
src/pages/Organization/OrganizationView.tsx
[failure] 61-61:
Argument of type '["entity_count", { count: string | number; entity: string; }]' is not assignable to parameter of type '[key: string | string[], options: TOptionsBase & $Dictionary & { defaultValue: string; }] | [key: string | string[], defaultValue: string, options?: (TOptionsBase & $Dictionary) | undefined] | [key: ...]'.
🪛 GitHub Actions: Cypress Tests
src/components/Facility/FacilityUsers.tsx
[error] 76-76: Type error: Argument of type '['entity_count', { count: string | number; entity: string; }]' is not assignable to parameter of type '[key: string | string[], options: TOptionsBase & $Dictionary & { defaultValue: string; }] | [key: string | string[], defaultValue: string, options?: (TOptionsBase & $Dictionary) | undefined] | [key: ...]'
🔇 Additional comments (2)
src/pages/Organization/OrganizationUsers.tsx (1)
67-71
: Same issues as other components.This component has the same translation type error and Badge styling duplication issues as discussed in previous files.
🧰 Tools
🪛 GitHub Check: cypress-run (1)
[failure] 67-67:
Argument of type '["entity_count", { count: string | number; entity: string; }]' is not assignable to parameter of type '[key: string | string[], options: TOptionsBase & $Dictionary & { defaultValue: string; }] | [key: string | string[], defaultValue: string, options?: (TOptionsBase & $Dictionary) | undefined] | [key: ...]'.src/pages/Organization/OrganizationPatients.tsx (1)
107-111
: Same issues as other components.This component has the same translation type error and Badge styling duplication issues as discussed in previous files.
🧰 Tools
🪛 GitHub Check: cypress-run (1)
[failure] 107-107:
Argument of type '["entity_count", { count: string | number; entity: string; }]' is not assignable to parameter of type '[key: string | string[], options: TOptionsBase & $Dictionary & { defaultValue: string; }] | [key: string | string[], defaultValue: string, options?: (TOptionsBase & $Dictionary) | undefined] | [key: ...]'.
Proposed Changes
Organization
Tabs #10087@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Release Notes
New Features
Improvements
Localization