Skip to content
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

MPDX-8479 HI Report Page & key #1228

Open
wants to merge 24 commits into
base: health-indicator
Choose a base branch
from
Open

Conversation

dr-bizz
Copy link
Contributor

@dr-bizz dr-bizz commented Dec 16, 2024

Description

In this PR, I add the MPD HI report page. This uses many of the same charts or graphs of the other PRs, so it doesn't look completely done, but it's not far off.

If you have no data, it shows a message stating there is no data.

The monthly goal graph will also show the HI widget

Checklist:

  • I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)"
  • I have applied the appropriate labels. (Add the label "On Staging" to get the branch automatically merged into staging.)
  • I have requested a review from another person on the project

Copy link

@codescene-delta-analysis codescene-delta-analysis bot left a 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: FAILED

Change in average Code Health of affected files: +0.19 (9.31 -> 9.50)

  • Declining Code Health: 3 findings(s) 🚩
  • Improving Code Health: 3 findings(s) ✅

View detailed results in CodeScene

@@ -166,8 +166,7 @@ export const AccountSummary: React.FC<AccountSummaryProps> = ({

// Periods
const startDateFormatted = monthYearFormat(
DateTime.fromISO(item?.startDate ?? '').month,
DateTime.fromISO(item?.startDate ?? '').year,
DateTime.fromISO(item.startDate),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Getting better: Complex Method
AccountSummary:React.FC<AccountSummaryProps> decreases in cyclomatic complexity from 27 to 25, threshold = 10

@@ -47,15 +47,18 @@ export const dayMonthFormat = (
}).format(DateTime.local().set({ month, day }).toJSDate());

export const monthYearFormat = (
month: number,
year: number,
date: DateTime | null,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ New issue: Primitive Obsession
In this module, 50.0% of all function arguments are primitive types, threshold = 30.0%

Suppress

@@ -5,10 +5,6 @@ import userEvent from '@testing-library/user-event';
import { DateTime } from 'luxon';
import TestRouter from '__tests__/util/TestRouter';
import { GqlMockedProvider } from '__tests__/util/graphqlMocking';
import {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No longer an issue: Code Duplication
The module no longer contains too many functions with similar structure

@@ -2,10 +2,6 @@ import React from 'react';
import { renderHook } from '@testing-library/react-hooks';
import { GqlMockedProvider } from '__tests__/util/graphqlMocking';
import { render } from '__tests__/util/testingLibraryReactMock';
import {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No longer an issue: Code Duplication
The module no longer contains too many functions with similar structure

Comment on lines +145 to +156
graphBlue1: {
main: graphColors.blue1,
},
graphBlue2: {
main: graphColors.blue2,
},
graphBlue3: {
main: graphColors.blue3,
},
graphTeal: {
main: graphColors.teal,
},

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Getting worse: Large Method
theme increases from 147 to 159 lines of code, threshold = 70

Suppress

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can exclude this file if we decide we want to from the CodeScene config.

Comment on lines 21 to 75
export const HealthIndicatorFormula: React.FC<HealthIndicatorFormulaProps> = ({
accountListId,
setNoHealthIndicatorData,
}) => {
const { t } = useTranslation();

const { data, loading } = useHealthIndicatorFormulaQuery({
variables: {
accountListId,
month: DateTime.now().startOf('month').toISODate(),
},
});

const latestMpdHealthData = data?.healthIndicatorData[0];

if (!data?.healthIndicatorData?.length && !loading) {
setNoHealthIndicatorData(true);
return null;
}

return (
<Card sx={{ padding: 3 }}>
<Typography variant="h6" mb={2}>
{t('MPD Health')} = [({t('Ownership')} * 3) + ({t('Success')} * 2) + (
{t('Consistency')} * 1) + ({t('Depth')} * 1)] / 7
</Typography>
<Box pl={2}>
<FormulaItem
name={t('Ownership')}
explanation={t('% of Self-raised Funds over Total Funds')}
value={latestMpdHealthData?.ownershipHi ?? 0}
isLoading={loading}
/>
<FormulaItem
name={t('Success')}
explanation={t('% of Self-raised Funds over Support Goal')}
value={latestMpdHealthData?.successHi ?? 0}
isLoading={loading}
/>
<FormulaItem
name={t('Consistency')}
explanation={t('% of months with positive account balance')}
value={latestMpdHealthData?.consistencyHi ?? 0}
isLoading={loading}
/>
<FormulaItem
name={t('Depth')}
explanation={t('Trend of local partners')}
value={latestMpdHealthData?.depthHi ?? 0}
isLoading={loading}
/>
</Box>
</Card>
);
};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ New issue: Complex Method
HealthIndicatorFormula:React.FC<HealthIndicatorFormulaProps> has a cyclomatic complexity of 10, threshold = 10

Suppress

Copy link

@codescene-delta-analysis codescene-delta-analysis bot left a 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: FAILED

Change in average Code Health of affected files: +0.19 (9.31 -> 9.50)

  • Declining Code Health: 3 findings(s) 🚩
  • Improving Code Health: 3 findings(s) ✅

View detailed results in CodeScene

@wrandall22
Copy link
Contributor

Looks like it passed the CodeScene check after that last commit.

@dr-bizz dr-bizz requested a review from canac December 19, 2024 18:38
@canac
Copy link
Contributor

canac commented Dec 19, 2024

@dr-bizz Either here or in different PR can you adjust the formula in the HealthIndicatorWidget tooltip to use x instead of * and change the case of the tooltip explanations the match this PR?

@canac
Copy link
Contributor

canac commented Dec 19, 2024

@dr-bizz On the report page, the widget has a View Details button that links to the report page you're on. Clicking it appears to do nothing. Could we remove it so that it doesn't confuse users?

@dr-bizz
Copy link
Contributor Author

dr-bizz commented Dec 19, 2024

@dr-bizz Either here or in different PR can you adjust the formula in the HealthIndicatorWidget tooltip to use x instead of * and change the case of the tooltip explanations the match this PR?

Sorry, I thought I did. I reset my local branch as I was showing Caleb A how to do certain Git commands. I will re-add it.

Copy link
Contributor

@canac canac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last thought, but this all looks really good!

@dr-bizz dr-bizz requested a review from canac December 19, 2024 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants