cleanup

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<MigrationAlert, MigrationSnoozeFormData> = MigrationSnoozeForm;
-  // AcceptForm: AlertForm<MigrationAlert, MigrationAcceptFormData> = 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<MigrationDismissFormData>,
   ): Promise<Alert[]> {
@@ -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<AlertSnoozeFormData>,
   ): Promise<Alert[]> {
@@ -139,8 +134,8 @@ export class KubernetesMigrationAlert implements MigrationAlert {
     );
   }
 
-  /* Displays default accept form. */
-  async onAccepted(options: AlertOptions): Promise<Alert[]> {
+  /* Fires when the onSubmit event is raised on an AcceptAlert form. Displays default accept form. */
+  async onAccepted(options: AlertOptions<null>): Promise<Alert[]> {
     const alerts = await this.api.getAlerts(options.group);
     return new Promise(resolve =>
       setTimeout(resolve, 750, [

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: {},
           },

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) => (
+  <Tooltip title={title}>
+    <IconButton
+      onClick={onClick}
+      role="button"
+      aria-hidden={false}
+      {...buttonProps}
+    >
+      <Badge badgeContent={amount}>{icon}</Badge>
+    </IconButton>
+  </Tooltip>
+);
+
 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) => (
-  <Tooltip title={title}>
-    <IconButton
-      onClick={onClick}
-      role="button"
-      aria-hidden={false}
-      {...buttonProps}
-    >
-      <Badge badgeContent={amount}>{icon}</Badge>
-    </IconButton>
-  </Tooltip>
-);

plugins/cost-insights/src/components/AlertInsights/AlertDialog.test.tsx

@@ -75,8 +75,8 @@ describe('<AlertDialog />', () => {
         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('<AlertDialog />', () => {
         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('<AlertDialog />', () => {
         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('<AlertDialog />', () => {
         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('<AlertDialog />', () => {
         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();

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<HTMLInputElement>, value: string) => {