diff --git a/plugins/cost-insights/src/alerts/KubernetesMigrationAlert.tsx b/plugins/cost-insights/src/alerts/KubernetesMigrationAlert.tsx index 8940322223..d7cc5ac79d 100644 --- a/plugins/cost-insights/src/alerts/KubernetesMigrationAlert.tsx +++ b/plugins/cost-insights/src/alerts/KubernetesMigrationAlert.tsx @@ -42,18 +42,19 @@ export interface MigrationAlert extends Alert { } /** - * The alert below is an example of an Alert implementation using event hooks. + * This is an example of an Alert implementation using optional event hooks. * - * Alerts can be customized to be accepted, dismissed snoozed or any combination - * by defining a corresponding hook on the alert instance. + * Event hooks can be used to enable users to dismiss, snooze, or accept an action item + * - or any combination thereof. Defining a hook will generate default UI - button, dialog and + * form. Cost Insights does not preserve client side alert state - each hook is expected to return a new set of alerts. * - * For example, defining an onDismissed hook will render a dismiss button that, when clicked, will - * generate a dialog prompting the user to provide a reason for dismissing the alert. - * Dismiss form data will be passed to the hook, which must eventually return a new set of alerts. - * Errors thrown within hooks will generate a snackbar, which can be used to display a - * user-friendly error message. + * Snoozed, accepted, etc. alerts should define a corresponding status property. Alerts will be aggregated + * by status in a collapsed view below Alert Insights section and a badge will appear in Action Items + * showing the total alerts of that status. * - * Cost Insights provides default forms for each hook, which can be overriden by providing a custom form component. + * Default forms can be overriden by providing a valid React form component. Form components + * must return valid form elements, and accept a ref and onSubmit event handler. See /forms + * for example implementations. Custom forms must implement a corresponding event hook. */ export class KubernetesMigrationAlert implements MigrationAlert { @@ -63,9 +64,7 @@ export class KubernetesMigrationAlert implements MigrationAlert { subtitle = 'Services running on Kubernetes are estimated to save 50% or more compared to Compute Engine.'; - // Override default dismiss form with custom form component. - // SnoozeForm: AlertForm = MigrationSnoozeForm; - // AcceptForm: AlertForm = MigrationAcceptForm; + // Override default dismiss form with a custom form component. DismissForm: AlertForm< MigrationAlert, MigrationDismissFormData @@ -101,7 +100,7 @@ export class KubernetesMigrationAlert implements MigrationAlert { ); } - /* Displays a custom dismiss form. */ + /* Fires when the onSubmit event is raised on a DismissAlert form. Displays a custom dismiss form. */ async onDismissed( options: AlertOptions, ): Promise { @@ -112,17 +111,13 @@ export class KubernetesMigrationAlert implements MigrationAlert { { title: this.title, subtitle: this.subtitle, - /** - * If a status property is defined, the alert will be filtered from the action items list - * but still appear grouped with other action items of the same status in the Hidden Action Items section. - */ status: AlertStatus.Dismissed, }, ]), ); } - /* Displays default accept form. */ + /* Fires when the onSubmit event is raised on an SnoozeAlert form. Displays default snooze form. */ async onSnoozed( options: AlertOptions, ): Promise { @@ -139,8 +134,8 @@ export class KubernetesMigrationAlert implements MigrationAlert { ); } - /* Displays default accept form. */ - async onAccepted(options: AlertOptions): Promise { + /* Fires when the onSubmit event is raised on an AcceptAlert form. Displays default accept form. */ + async onAccepted(options: AlertOptions): Promise { const alerts = await this.api.getAlerts(options.group); return new Promise(resolve => setTimeout(resolve, 750, [ diff --git a/plugins/cost-insights/src/client.ts b/plugins/cost-insights/src/client.ts index 837cd7a560..dd53d1f942 100644 --- a/plugins/cost-insights/src/client.ts +++ b/plugins/cost-insights/src/client.ts @@ -172,12 +172,13 @@ export class ExampleCostInsightsClient implements CostInsightsApi { ], }; + const today = dayjs(); const alerts: Alert[] = await this.request({ group }, [ new ProjectGrowthAlert(projectGrowthData), new UnlabeledDataflowAlert(unlabeledDataflowData), new KubernetesMigrationAlert(this, { - startDate: '2021-01-24', - endDate: '2020-02-24', + startDate: today.format(DEFAULT_DATE_FORMAT), + endDate: today.add(30, 'day').format(DEFAULT_DATE_FORMAT), change: { ratio: 0, amount: 0, @@ -187,7 +188,7 @@ export class ExampleCostInsightsClient implements CostInsightsApi { id: 'service-a', aggregation: [20_000, 10_000], change: { - ratio: -1, + ratio: -0.5, amount: -10_000, }, entities: {}, @@ -196,8 +197,8 @@ export class ExampleCostInsightsClient implements CostInsightsApi { id: 'service-b', aggregation: [30_000, 15_000], change: { - ratio: -1, - amount: 15_000, + ratio: -0.5, + amount: -15_000, }, entities: {}, }, diff --git a/plugins/cost-insights/src/components/ActionItems/ActionItems.tsx b/plugins/cost-insights/src/components/ActionItems/ActionItems.tsx index 320ebb38aa..4fbe3470b1 100644 --- a/plugins/cost-insights/src/components/ActionItems/ActionItems.tsx +++ b/plugins/cost-insights/src/components/ActionItems/ActionItems.tsx @@ -32,6 +32,32 @@ import { Alert, AlertStatus } from '../../types'; import { useScroll, ScrollType } from '../../hooks'; import { useActionItemCardStyles as useStyles } from '../../utils/styles'; +type AlertStatusButtonProps = { + title: string; + amount: number; + icon: JSX.Element; + onClick: MouseEventHandler; +} & IconButtonProps; + +const AlertStatusButton = ({ + title, + amount, + icon, + onClick, + ...buttonProps +}: AlertStatusButtonProps) => ( + + + {icon} + + +); + type ActionItemsProps = { active: Alert[]; snoozed: Alert[]; @@ -104,29 +130,3 @@ export const ActionItems = ({ ); }; - -type AlertStatusButtonProps = { - title: string; - amount: number; - icon: JSX.Element; - onClick: MouseEventHandler; -} & IconButtonProps; - -const AlertStatusButton = ({ - title, - amount, - icon, - onClick, - ...buttonProps -}: AlertStatusButtonProps) => ( - - - {icon} - - -); diff --git a/plugins/cost-insights/src/components/AlertInsights/AlertDialog.test.tsx b/plugins/cost-insights/src/components/AlertInsights/AlertDialog.test.tsx index 5785592aeb..2e4cb53681 100644 --- a/plugins/cost-insights/src/components/AlertInsights/AlertDialog.test.tsx +++ b/plugins/cost-insights/src/components/AlertInsights/AlertDialog.test.tsx @@ -75,8 +75,8 @@ describe('', () => { snoozed={snoozableAlert} accepted={null} dismissed={null} - onClose={jest.fn} - onSubmit={jest.fn} + onClose={jest.fn()} + onSubmit={jest.fn()} />, ); expect(getByText('For how long?')).toBeInTheDocument(); @@ -97,8 +97,8 @@ describe('', () => { snoozed={customSnoozeAlert} accepted={null} dismissed={null} - onClose={jest.fn} - onSubmit={jest.fn} + onClose={jest.fn()} + onSubmit={jest.fn()} />, ); expect(getByText('You. Complete. Me.')).toBeInTheDocument(); @@ -116,8 +116,8 @@ describe('', () => { snoozed={null} accepted={null} dismissed={dimissableAlert} - onClose={jest.fn} - onSubmit={jest.fn} + onClose={jest.fn()} + onSubmit={jest.fn()} />, ); expect(getByText('Dismiss this action item?')).toBeInTheDocument(); @@ -137,8 +137,8 @@ describe('', () => { snoozed={null} accepted={null} dismissed={customDismissAlert} - onClose={jest.fn} - onSubmit={jest.fn} + onClose={jest.fn()} + onSubmit={jest.fn()} />, ); expect(getByText('Dismiss this action item?')).toBeInTheDocument(); @@ -156,8 +156,8 @@ describe('', () => { snoozed={null} accepted={customAcceptAlert} dismissed={null} - onClose={jest.fn} - onSubmit={jest.fn} + onClose={jest.fn()} + onSubmit={jest.fn()} />, ); expect(getByText('Accept this action item?')).toBeInTheDocument(); diff --git a/plugins/cost-insights/src/forms/AlertDismissForm.tsx b/plugins/cost-insights/src/forms/AlertDismissForm.tsx index 1b3cbdb363..4271785958 100644 --- a/plugins/cost-insights/src/forms/AlertDismissForm.tsx +++ b/plugins/cost-insights/src/forms/AlertDismissForm.tsx @@ -55,18 +55,14 @@ export const AlertDismissForm = forwardRef< ); const onFormSubmit: FormEventHandler = e => { - function submit() { + e.preventDefault(); + if (reason) { onSubmit({ other: other, reason: reason, feedback: feedback, }); } - - e.preventDefault(); - if (reason) { - submit(); - } }; const onReasonChange = (_: ChangeEvent, value: string) => {