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

Merged
merged 4 commits into from
Jan 9, 2025
Merged

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

src/theme.ts Outdated
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 80
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
@dr-bizz dr-bizz added the Preview Environment Add this label to create an Amplify Preview label Jan 6, 2025
Copy link
Contributor

github-actions bot commented Jan 6, 2025

Preview branch generated at https://8479-hi-report-page.d3dytjb8adxkk5.amplifyapp.com

@@ -51,7 +53,8 @@ export const HealthIndicatorWidget: React.FC<HealthIndicatorWidgetProps> = ({
const { data, loading } = useHealthIndicatorWidgetQuery({
variables: {
accountListId,
month: DateTime.now().startOf('month').toISODate(),
month: '2024-12-01',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove this and just grab the last item using .at(-1)

const { data, loading } = useHealthIndicatorFormulaQuery({
variables: {
accountListId,
month: '2024-12-01',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove month and just grab the last item using .at(-1)

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.

Sorry I missed that you were waiting on a review. Looks good!

@@ -65,6 +66,7 @@ const MonthlyGoal = ({
pledged = 0,
totalGiftsNotStarted,
currencyCode = 'USD',
onDashboard = true,
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not a big deal, but I would expect this prop to default to false

@dr-bizz dr-bizz force-pushed the 8479-hi-report-page branch from bfc9116 to 2dd1f76 Compare January 9, 2025 22:23
@dr-bizz dr-bizz force-pushed the 8479-hi-report-page branch from 2dd1f76 to 091a9c9 Compare January 9, 2025 22:42
@dr-bizz dr-bizz merged commit 2ce0474 into health-indicator Jan 9, 2025
5 of 16 checks passed
@dr-bizz dr-bizz deleted the 8479-hi-report-page branch January 9, 2025 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Preview Environment Add this label to create an Amplify Preview
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants