-
-
Notifications
You must be signed in to change notification settings - Fork 736
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: health trend insight #8335
Changes from 1 commit
c2dae53
003ed57
36d5c21
6e7e490
73fe9dc
547bef1
c8a1954
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ import { styled, Typography } from '@mui/material'; | |
import type { FC } from 'react'; | ||
import { Link } from 'react-router-dom'; | ||
import Lightbulb from '@mui/icons-material/LightbulbOutlined'; | ||
import type { PersonalDashboardProjectDetailsSchemaInsights } from '../../openapi'; | ||
|
||
const TitleContainer = styled('div')(({ theme }) => ({ | ||
display: 'flex', | ||
|
@@ -18,18 +19,17 @@ const ActionBox = styled('article')(({ theme }) => ({ | |
flexDirection: 'column', | ||
})); | ||
|
||
export const ProjectSetupComplete: FC<{ project: string }> = ({ project }) => { | ||
const PercentageScore = styled('span')(({ theme }) => ({ | ||
color: theme.palette.primary.main, | ||
})); | ||
|
||
const ConnectedSdkProject: FC<{ project: string }> = ({ project }) => { | ||
return ( | ||
<ActionBox> | ||
<TitleContainer> | ||
<Lightbulb color='primary' /> | ||
<h3>Project Insight</h3> | ||
</TitleContainer> | ||
<> | ||
<Typography> | ||
This project already has connected SDKs and existing feature | ||
flags. | ||
</Typography> | ||
|
||
<Typography> | ||
<Link to={`/projects/${project}?create=true`}> | ||
Create a new feature flag | ||
|
@@ -40,6 +40,91 @@ export const ProjectSetupComplete: FC<{ project: string }> = ({ project }) => { | |
</Link>{' '} | ||
to work with existing flags | ||
</Typography> | ||
</> | ||
); | ||
}; | ||
|
||
type HeathTrend = 'consistent' | 'improved' | 'declined' | 'unknown'; | ||
|
||
const determineProjectHealthTrend = ( | ||
insights: PersonalDashboardProjectDetailsSchemaInsights, | ||
) => { | ||
let trend: HeathTrend = 'unknown'; | ||
if ( | ||
insights.avgHealthCurrentWindow !== null && | ||
insights.avgHealthPastWindow !== null | ||
) { | ||
if (insights.avgHealthCurrentWindow > insights.avgHealthPastWindow) { | ||
trend = 'improved'; | ||
} else if ( | ||
insights.avgHealthCurrentWindow < insights.avgHealthPastWindow | ||
) { | ||
trend = 'declined'; | ||
} else if ( | ||
insights.avgHealthPastWindow === insights.avgHealthCurrentWindow | ||
) { | ||
trend = 'consistent'; | ||
} | ||
} | ||
return trend; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we could do an early return here by checking if they're both null and just return the string directly. would decrease nesting by one level and remove the mutable variable. Your choice, though There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 100%. Fixing |
||
}; | ||
|
||
export const ProjectSetupComplete: FC<{ | ||
project: string; | ||
insights: PersonalDashboardProjectDetailsSchemaInsights; | ||
}> = ({ project, insights }) => { | ||
const projectHealthTrend = determineProjectHealthTrend(insights); | ||
|
||
return ( | ||
<ActionBox> | ||
<TitleContainer> | ||
<Lightbulb color='primary' /> | ||
<h3>Project Insight</h3> | ||
</TitleContainer> | ||
|
||
{projectHealthTrend === 'unknown' ? ( | ||
<ConnectedSdkProject project={project} /> | ||
) : null} | ||
{projectHealthTrend === 'improved' ? ( | ||
<Typography> | ||
On average, your project health went up from{' '} | ||
<PercentageScore> | ||
{insights.avgHealthPastWindow}% | ||
</PercentageScore>{' '} | ||
to{' '} | ||
<PercentageScore> | ||
{insights.avgHealthCurrentWindow}% | ||
</PercentageScore>{' '} | ||
during the last 4 weeks. | ||
</Typography> | ||
) : null} | ||
{projectHealthTrend === 'declined' ? ( | ||
<Typography> | ||
On average, your project health went down from{' '} | ||
<PercentageScore> | ||
{insights.avgHealthPastWindow}% | ||
</PercentageScore>{' '} | ||
to{' '} | ||
<PercentageScore> | ||
{insights.avgHealthCurrentWindow}% | ||
</PercentageScore>{' '} | ||
during the last 4 weeks. | ||
</Typography> | ||
) : null} | ||
{projectHealthTrend === 'consistent' ? ( | ||
<Typography> | ||
On average, your project health has remained at{' '} | ||
<PercentageScore> | ||
{insights.avgHealthCurrentWindow}% | ||
</PercentageScore>{' '} | ||
during the last 4 weeks. | ||
</Typography> | ||
) : null} | ||
{projectHealthTrend !== 'unknown' ? ( | ||
<Link to={`/projects/${project}/insights`}> | ||
View more insights | ||
</Link> | ||
) : null} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd maybe consider extracting this into a variable above instead of a series of ternaries. Just to avoid any future issues 🤷🏼 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refactored to a separate component with early returns |
||
</ActionBox> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
/** | ||
* Generated by Orval | ||
* Do not edit manually. | ||
* See `gen:api` script in package.json | ||
*/ | ||
|
||
/** | ||
* Insights for the project | ||
*/ | ||
export type PersonalDashboardProjectDetailsSchemaInsights = { | ||
/** | ||
* The average health score in the current window of the last 4 weeks | ||
* @nullable | ||
*/ | ||
avgHealthCurrentWindow: number | null; | ||
/** | ||
* The average health score in the previous 4 weeks before the current window | ||
* @nullable | ||
*/ | ||
avgHealthPastWindow: number | null; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
/** | ||
* Generated by Orval | ||
* Do not edit manually. | ||
* See `gen:api` script in package.json | ||
*/ | ||
import type { SignalQuerySignalSchema } from './signalQuerySignalSchema'; | ||
|
||
/** | ||
* A list of signals that have been registered by the system | ||
*/ | ||
export interface SignalQueryResponseSchema { | ||
/** The list of signals */ | ||
signals: SignalQuerySignalSchema[]; | ||
/** | ||
* The total count of signals | ||
* @minimum 0 | ||
*/ | ||
total: number; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
/** | ||
* Generated by Orval | ||
* Do not edit manually. | ||
* See `gen:api` script in package.json | ||
*/ | ||
import type { SignalQuerySignalSchemaPayload } from './signalQuerySignalSchemaPayload'; | ||
import type { SignalQuerySignalSchemaSource } from './signalQuerySignalSchemaSource'; | ||
|
||
/** | ||
* An object describing a signal enriched with source data. | ||
*/ | ||
export interface SignalQuerySignalSchema { | ||
/** The date and time of when the signal was created. */ | ||
createdAt: string; | ||
/** | ||
* The signal's ID. Signal IDs are incrementing integers. In other words, a more recently created signal will always have a higher ID than an older one. | ||
* @minimum 1 | ||
*/ | ||
id: number; | ||
/** The payload of the signal. */ | ||
payload?: SignalQuerySignalSchemaPayload; | ||
/** The signal source type. Should be used along with `sourceId` to uniquely identify the resource that created this signal. */ | ||
source: SignalQuerySignalSchemaSource; | ||
/** | ||
* A more detailed description of the source that registered this signal. | ||
* @nullable | ||
*/ | ||
sourceDescription?: string | null; | ||
/** The ID of the source that created this signal. Should be used along with `source` to uniquely identify the resource that created this signal. */ | ||
sourceId: number; | ||
/** | ||
* The name of the source that registered this signal. | ||
* @nullable | ||
*/ | ||
sourceName?: string | null; | ||
/** | ||
* The name of the token used to register this signal. | ||
* @nullable | ||
*/ | ||
tokenName?: string | null; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
/** | ||
* Generated by Orval | ||
* Do not edit manually. | ||
* See `gen:api` script in package.json | ||
*/ | ||
|
||
/** | ||
* The payload of the signal. | ||
*/ | ||
export type SignalQuerySignalSchemaPayload = { [key: string]: unknown }; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
/** | ||
* Generated by Orval | ||
* Do not edit manually. | ||
* See `gen:api` script in package.json | ||
*/ | ||
|
||
/** | ||
* The signal source type. Should be used along with `sourceId` to uniquely identify the resource that created this signal. | ||
*/ | ||
export type SignalQuerySignalSchemaSource = | ||
(typeof SignalQuerySignalSchemaSource)[keyof typeof SignalQuerySignalSchemaSource]; | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-redeclare | ||
export const SignalQuerySignalSchemaSource = { | ||
'signal-endpoint': 'signal-endpoint', | ||
} as const; |
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.
question: can you have an active project stage without personaldashboardprojectdetails? is this redundant?
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.
no, but the orval type doesn't allow me to make this state more explicit so I ended up adding two expressions in here