-
-
Notifications
You must be signed in to change notification settings - Fork 656
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
fix: total flags and flags per user when all projects #6787
Conversation
Signed-off-by: andreas-unleash <andreas@getunleash.ai>
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
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.
✅ Code Health Quality Gates: OK
- Declining Code Health: 1 findings(s) 🚩
@@ -66,6 +71,7 @@ const ChartWidget = styled(Widget)(({ theme }) => ({ | |||
|
|||
export const InsightsCharts: VFC<IChartsProps> = ({ | |||
projects, | |||
flags, |
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.
❌ Getting worse: Large Method
InsightsCharts:VFC<IChartsProps> increases from 137 to 147 lines of code, threshold = 120
flags: InstanceInsightsSchemaFlags, | ||
users: InstanceInsightsSchemaUsers, | ||
) { | ||
const flagsPerUserCalculation = flags.total / users.total; |
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.
This function can receive more primitive type, and thus be simplified. I would pass flags.total
and users.total
instead of entire objects. Also formatFlagsPerUser
would be more descriptive to me.
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.
Not sure I understand what you mean here
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.
This function doesn't need to have complex arguments. It can probably work with ? number | null
arguments instead, and be called as formatFlagsPerUser(flags.total, users.total)
Fixes the flag stats flagsPerUser calculation and display value.
Previously the calculation depended on project data which might not be there with the permission changes
Closes # 1-2255