From d65b34409aae8579631f1c12ac362afba59b3496 Mon Sep 17 00:00:00 2001 From: Ryan Vazquez Date: Tue, 12 Jan 2021 16:44:30 -0500 Subject: [PATCH 01/10] add alert hooks --- .../src/alerts/KubernetesMigrationAlert.tsx | 156 +++++++++++++ .../src/alerts/ProjectGrowthAlert.tsx | 45 ++++ .../src/alerts/UnlabeledDataflowAlert.tsx | 43 ++++ .../index.ts} | 22 +- plugins/cost-insights/src/client.ts | 31 ++- .../ActionItemCard.test.tsx} | 8 +- .../components/ActionItems/ActionItemCard.tsx | 62 +++++ .../ActionItems/ActionItems.test.tsx | 86 +++++++ .../components/ActionItems/ActionItems.tsx | 132 +++++++++++ .../src/components/ActionItems/index.ts | 18 ++ .../AlertActionCardList/AlertActionCard.tsx | 45 ---- .../AlertInsights/AlertDialog.test.tsx | 169 ++++++++++++++ .../components/AlertInsights/AlertDialog.tsx | 210 +++++++++++++++++ .../AlertInsights/AlertInsights.test.tsx | 89 ++++++++ .../AlertInsights/AlertInsights.tsx | 212 ++++++++++++++++-- .../AlertInsights/AlertInsightsHeader.tsx | 5 +- .../AlertInsightsSection.test.tsx | 104 ++++++++- .../AlertInsights/AlertInsightsSection.tsx | 68 +++++- .../AlertInsightsSectionHeader.tsx | 39 ++-- .../AlertInsights/AlertStatusSummary.test.tsx | 58 +++++ .../AlertInsights/AlertStatusSummary.tsx | 116 ++++++++++ .../AlertStatusSummaryButton.tsx | 54 +++++ .../CostInsightsNavigation.tsx | 5 +- .../CostInsightsPage/CostInsightsPage.tsx | 84 ++++--- .../CostInsightsPage/CostInsightsPageRoot.tsx | 5 +- .../CostOverviewCard/CostOverviewCard.tsx | 4 +- .../MigrationAlertCard/MigrationAlertCard.tsx | 60 +++++ .../MigrationAlertCard/MigrationBarChart.tsx | 54 +++++ .../MigrationBarChartLegend.tsx | 58 +++++ .../index.ts | 2 +- .../ProductInsightsCard.test.tsx | 11 - .../ProductInsightsCard.tsx | 6 +- .../ProjectGrowthInstructionsPage.tsx | 2 +- .../src/forms/AlertAcceptForm.tsx | 57 +++++ .../src/forms/AlertDismissForm.tsx | 156 +++++++++++++ .../src/forms/AlertSnoozeForm.tsx | 115 ++++++++++ .../src/forms/MigrationDismissForm.tsx | 108 +++++++++ plugins/cost-insights/src/forms/index.ts | 21 ++ plugins/cost-insights/src/hooks/index.ts | 1 + plugins/cost-insights/src/hooks/useAlerts.tsx | 76 +++++++ plugins/cost-insights/src/hooks/useScroll.tsx | 63 +++--- plugins/cost-insights/src/index.ts | 2 +- plugins/cost-insights/src/types/Alert.ts | 128 ++++++++++- plugins/cost-insights/src/types/Duration.ts | 1 + plugins/cost-insights/src/utils/alerts.tsx | 58 ++--- plugins/cost-insights/src/utils/duration.ts | 13 +- plugins/cost-insights/src/utils/loading.ts | 1 + plugins/cost-insights/src/utils/styles.ts | 41 +++- plugins/cost-insights/src/utils/tests.tsx | 31 ++- 49 files changed, 2669 insertions(+), 266 deletions(-) create mode 100644 plugins/cost-insights/src/alerts/KubernetesMigrationAlert.tsx create mode 100644 plugins/cost-insights/src/alerts/ProjectGrowthAlert.tsx create mode 100644 plugins/cost-insights/src/alerts/UnlabeledDataflowAlert.tsx rename plugins/cost-insights/src/{components/AlertActionCardList/AlertActionCardList.tsx => alerts/index.ts} (50%) rename plugins/cost-insights/src/components/{AlertActionCardList/AlertActionCard.test.tsx => ActionItems/ActionItemCard.test.tsx} (87%) create mode 100644 plugins/cost-insights/src/components/ActionItems/ActionItemCard.tsx create mode 100644 plugins/cost-insights/src/components/ActionItems/ActionItems.test.tsx create mode 100644 plugins/cost-insights/src/components/ActionItems/ActionItems.tsx create mode 100644 plugins/cost-insights/src/components/ActionItems/index.ts delete mode 100644 plugins/cost-insights/src/components/AlertActionCardList/AlertActionCard.tsx create mode 100644 plugins/cost-insights/src/components/AlertInsights/AlertDialog.test.tsx create mode 100644 plugins/cost-insights/src/components/AlertInsights/AlertDialog.tsx create mode 100644 plugins/cost-insights/src/components/AlertInsights/AlertInsights.test.tsx create mode 100644 plugins/cost-insights/src/components/AlertInsights/AlertStatusSummary.test.tsx create mode 100644 plugins/cost-insights/src/components/AlertInsights/AlertStatusSummary.tsx create mode 100644 plugins/cost-insights/src/components/AlertInsights/AlertStatusSummaryButton.tsx create mode 100644 plugins/cost-insights/src/components/MigrationAlertCard/MigrationAlertCard.tsx create mode 100644 plugins/cost-insights/src/components/MigrationAlertCard/MigrationBarChart.tsx create mode 100644 plugins/cost-insights/src/components/MigrationAlertCard/MigrationBarChartLegend.tsx rename plugins/cost-insights/src/components/{AlertActionCardList => MigrationAlertCard}/index.ts (90%) create mode 100644 plugins/cost-insights/src/forms/AlertAcceptForm.tsx create mode 100644 plugins/cost-insights/src/forms/AlertDismissForm.tsx create mode 100644 plugins/cost-insights/src/forms/AlertSnoozeForm.tsx create mode 100644 plugins/cost-insights/src/forms/MigrationDismissForm.tsx create mode 100644 plugins/cost-insights/src/forms/index.ts create mode 100644 plugins/cost-insights/src/hooks/useAlerts.tsx diff --git a/plugins/cost-insights/src/alerts/KubernetesMigrationAlert.tsx b/plugins/cost-insights/src/alerts/KubernetesMigrationAlert.tsx new file mode 100644 index 0000000000..8940322223 --- /dev/null +++ b/plugins/cost-insights/src/alerts/KubernetesMigrationAlert.tsx @@ -0,0 +1,156 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React from 'react'; +import pluralize from 'pluralize'; +import { MigrationAlertCard } from '../components/MigrationAlertCard'; +import { CostInsightsApi } from '../api'; +import { + Alert, + AlertForm, + AlertOptions, + AlertStatus, + AlertSnoozeFormData, + ChangeStatistic, + Entity, +} from '../types'; +import { MigrationDismissForm, MigrationDismissFormData } from '../forms'; + +export interface MigrationData { + startDate: string; + endDate: string; + change: ChangeStatistic; + services: Array; +} + +export interface MigrationAlert extends Alert { + api: CostInsightsApi; + data: MigrationData; +} + +/** + * The alert below is an example of an Alert implementation using event hooks. + * + * Alerts can be customized to be accepted, dismissed snoozed or any combination + * by defining a corresponding hook on the alert instance. + * + * 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. + * + * Cost Insights provides default forms for each hook, which can be overriden by providing a custom form component. + */ + +export class KubernetesMigrationAlert implements MigrationAlert { + api: CostInsightsApi; + data: MigrationData; + + 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; + DismissForm: AlertForm< + MigrationAlert, + MigrationDismissFormData + > = MigrationDismissForm; + + constructor(api: CostInsightsApi, data: MigrationData) { + this.api = api; + this.data = data; + } + + get title() { + return `Consider migrating ${pluralize( + 'service', + this.data.services.length, + true, + )} to Kubernetes.`; + } + + get element() { + const subheader = `${pluralize( + 'Compute Engine role', + this.data.services.length, + true, + )}, sorted by cost`; + return ( + + ); + } + + /* Displays a custom dismiss form. */ + async onDismissed( + options: AlertOptions, + ): Promise { + const alerts = await this.api.getAlerts(options.group); + return new Promise(resolve => + setTimeout(resolve, 750, [ + ...alerts.slice(0, 2), + { + 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. */ + async onSnoozed( + options: AlertOptions, + ): Promise { + const alerts = await this.api.getAlerts(options.group); + return new Promise(resolve => + setTimeout(resolve, 750, [ + ...alerts.slice(0, 2), + { + title: this.title, + subtitle: this.subtitle, + status: AlertStatus.Snoozed, + }, + ]), + ); + } + + /* Displays default accept form. */ + async onAccepted(options: AlertOptions): Promise { + const alerts = await this.api.getAlerts(options.group); + return new Promise(resolve => + setTimeout(resolve, 750, [ + ...alerts.slice(0, 2), + { + title: this.title, + subtitle: this.subtitle, + status: AlertStatus.Accepted, + }, + ]), + ); + } +} diff --git a/plugins/cost-insights/src/alerts/ProjectGrowthAlert.tsx b/plugins/cost-insights/src/alerts/ProjectGrowthAlert.tsx new file mode 100644 index 0000000000..67eb1b5071 --- /dev/null +++ b/plugins/cost-insights/src/alerts/ProjectGrowthAlert.tsx @@ -0,0 +1,45 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React from 'react'; +import { ProjectGrowthAlertCard } from '../components/ProjectGrowthAlertCard'; +import { Alert, ProjectGrowthData } from '../types'; + +/** + * The alert below is an example of an Alert implementation; the CostInsightsApi permits returning + * any implementation of the Alert type, so adopters can create their own. The CostInsightsApi + * fetches alert data from the backend, then creates Alert classes with the data. + */ + +export class ProjectGrowthAlert implements Alert { + data: ProjectGrowthData; + + url = '/cost-insights/investigating-growth'; + subtitle = + 'Cost growth outpacing business growth is unsustainable long-term.'; + + constructor(data: ProjectGrowthData) { + this.data = data; + } + + get title() { + return `Investigate cost growth in project ${this.data.project}`; + } + + get element() { + return ; + } +} diff --git a/plugins/cost-insights/src/alerts/UnlabeledDataflowAlert.tsx b/plugins/cost-insights/src/alerts/UnlabeledDataflowAlert.tsx new file mode 100644 index 0000000000..e889e0e3d4 --- /dev/null +++ b/plugins/cost-insights/src/alerts/UnlabeledDataflowAlert.tsx @@ -0,0 +1,43 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React from 'react'; +import { UnlabeledDataflowAlertCard } from '../components/UnlabeledDataflowAlertCard'; +import { Alert, AlertStatus, UnlabeledDataflowData } from '../types'; + +/** + * The alert below is an example of an Alert implementation; the CostInsightsApi permits returning + * any implementation of the Alert type, so adopters can create their own. The CostInsightsApi + * fetches alert data from the backend, then creates Alert classes with the data. + */ + +export class UnlabeledDataflowAlert implements Alert { + data: UnlabeledDataflowData; + status?: AlertStatus; + + url = '/cost-insights/labeling-jobs'; + title = 'Add labels to workflows'; + subtitle = + 'Labels show in billing data, enabling cost insights for each workflow.'; + + constructor(data: UnlabeledDataflowData) { + this.data = data; + } + + get element() { + return ; + } +} diff --git a/plugins/cost-insights/src/components/AlertActionCardList/AlertActionCardList.tsx b/plugins/cost-insights/src/alerts/index.ts similarity index 50% rename from plugins/cost-insights/src/components/AlertActionCardList/AlertActionCardList.tsx rename to plugins/cost-insights/src/alerts/index.ts index aaadbed09d..eb4a079e90 100644 --- a/plugins/cost-insights/src/components/AlertActionCardList/AlertActionCardList.tsx +++ b/plugins/cost-insights/src/alerts/index.ts @@ -13,22 +13,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import React, { Fragment } from 'react'; -import { Paper, Divider } from '@material-ui/core'; -import { AlertActionCard } from './AlertActionCard'; -import { Alert } from '../../types'; -type AlertActionCardList = { - alerts: Array; -}; - -export const AlertActionCardList = ({ alerts }: AlertActionCardList) => ( - - {alerts.map((alert, index) => ( - - - {index < alerts.length - 1 && } - - ))} - -); +export { ProjectGrowthAlert } from './ProjectGrowthAlert'; +export { UnlabeledDataflowAlert } from './UnlabeledDataflowAlert'; +export { KubernetesMigrationAlert } from './KubernetesMigrationAlert'; +export type { MigrationAlert } from './KubernetesMigrationAlert'; diff --git a/plugins/cost-insights/src/client.ts b/plugins/cost-insights/src/client.ts index 0ba57f76fa..837cd7a560 100644 --- a/plugins/cost-insights/src/client.ts +++ b/plugins/cost-insights/src/client.ts @@ -31,7 +31,8 @@ import { import { ProjectGrowthAlert, UnlabeledDataflowAlert, -} from '../src/utils/alerts'; + KubernetesMigrationAlert, +} from '../src/alerts'; import { trendlineOf, changeOf, @@ -174,6 +175,34 @@ export class ExampleCostInsightsClient implements CostInsightsApi { const alerts: Alert[] = await this.request({ group }, [ new ProjectGrowthAlert(projectGrowthData), new UnlabeledDataflowAlert(unlabeledDataflowData), + new KubernetesMigrationAlert(this, { + startDate: '2021-01-24', + endDate: '2020-02-24', + change: { + ratio: 0, + amount: 0, + }, + services: [ + { + id: 'service-a', + aggregation: [20_000, 10_000], + change: { + ratio: -1, + amount: -10_000, + }, + entities: {}, + }, + { + id: 'service-b', + aggregation: [30_000, 15_000], + change: { + ratio: -1, + amount: 15_000, + }, + entities: {}, + }, + ], + }), ]); return alerts; diff --git a/plugins/cost-insights/src/components/AlertActionCardList/AlertActionCard.test.tsx b/plugins/cost-insights/src/components/ActionItems/ActionItemCard.test.tsx similarity index 87% rename from plugins/cost-insights/src/components/AlertActionCardList/AlertActionCard.test.tsx rename to plugins/cost-insights/src/components/ActionItems/ActionItemCard.test.tsx index d896bf0d47..b72f050e5b 100644 --- a/plugins/cost-insights/src/components/AlertActionCardList/AlertActionCard.test.tsx +++ b/plugins/cost-insights/src/components/ActionItems/ActionItemCard.test.tsx @@ -16,9 +16,9 @@ import React from 'react'; import { renderInTestApp } from '@backstage/test-utils'; -import { AlertActionCard } from './AlertActionCard'; +import { ActionItemCard } from './ActionItemCard'; import { MockScrollProvider } from '../../utils/tests'; -import { ProjectGrowthAlert } from '../../utils/alerts'; +import { ProjectGrowthAlert } from '../../alerts'; import { ProjectGrowthData } from '../../types'; const data: ProjectGrowthData = { @@ -31,11 +31,11 @@ const data: ProjectGrowthData = { }; const alert = new ProjectGrowthAlert(data); -describe('', () => { +describe('', () => { it('Renders an alert', async () => { const rendered = await renderInTestApp( - , + 1} /> , ); diff --git a/plugins/cost-insights/src/components/ActionItems/ActionItemCard.tsx b/plugins/cost-insights/src/components/ActionItems/ActionItemCard.tsx new file mode 100644 index 0000000000..3c95fdaa2f --- /dev/null +++ b/plugins/cost-insights/src/components/ActionItems/ActionItemCard.tsx @@ -0,0 +1,62 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import React, { MouseEventHandler } from 'react'; +import classnames from 'classnames'; +import { Card, CardHeader } from '@material-ui/core'; +import { useScroll } from '../../hooks'; +import { Alert } from '../../types'; +import { useActionItemCardStyles as useStyles } from '../../utils/styles'; + +type ActionItemCardProps = { + alert: Alert; + number?: number; + avatar?: JSX.Element; + disableScroll?: boolean; +}; + +export const ActionItemCard = ({ + alert, + avatar, + number, + disableScroll = false, +}: ActionItemCardProps) => { + const classes = useStyles(); + const rootClasses = classnames(classes.root, { + [classes.activeRoot]: !disableScroll, + }); + const [, setScroll] = useScroll(); + + const onActionItemClick: MouseEventHandler = () => { + if (!disableScroll && number) { + setScroll(`alert-${number}`); + } + }; + + return ( + + + + ); +}; diff --git a/plugins/cost-insights/src/components/ActionItems/ActionItems.test.tsx b/plugins/cost-insights/src/components/ActionItems/ActionItems.test.tsx new file mode 100644 index 0000000000..8c96e47f1a --- /dev/null +++ b/plugins/cost-insights/src/components/ActionItems/ActionItems.test.tsx @@ -0,0 +1,86 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React from 'react'; +import { render } from '@testing-library/react'; +import { ActionItems } from './ActionItems'; +import { MockScrollProvider } from '../../utils/tests'; + +function renderInContext(children: JSX.Element) { + return render({children}); +} + +describe('', () => { + it('should not display status buttons if there no active alerts', () => { + const { queryByRole } = renderInContext( + , + ); + expect(queryByRole('button', { name: 'snoozed' })).not.toBeInTheDocument(); + expect(queryByRole('button', { name: 'accepted' })).not.toBeInTheDocument(); + expect( + queryByRole('button', { name: 'dismissed' }), + ).not.toBeInTheDocument(); + }); + + it('should display status buttons with correct badge number', () => { + const { getByText, getByRole, getAllByText } = renderInContext( + , + ); + expect(getAllByText('1')).toHaveLength(2); // should be a badge of 1 and action item number of 1 + expect(getByText('2')).toBeInTheDocument(); + expect(getByText('3')).toBeInTheDocument(); + expect(getByRole('button', { name: 'snoozed' })).toBeInTheDocument(); + expect(getByRole('button', { name: 'accepted' })).toBeInTheDocument(); + expect(getByRole('button', { name: 'dismissed' })).toBeInTheDocument(); + }); +}); diff --git a/plugins/cost-insights/src/components/ActionItems/ActionItems.tsx b/plugins/cost-insights/src/components/ActionItems/ActionItems.tsx new file mode 100644 index 0000000000..320ebb38aa --- /dev/null +++ b/plugins/cost-insights/src/components/ActionItems/ActionItems.tsx @@ -0,0 +1,132 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import React, { Fragment, MouseEventHandler } from 'react'; +import { + Avatar, + Badge, + Box, + IconButtonProps, + IconButton, + Paper, + Divider, + Tooltip, +} from '@material-ui/core'; +import { default as SnoozeIcon } from '@material-ui/icons/AccessTime'; +import { default as AcceptIcon } from '@material-ui/icons/Check'; +import { default as DismissIcon } from '@material-ui/icons/Delete'; +import { ActionItemCard } from './ActionItemCard'; +import { Alert, AlertStatus } from '../../types'; +import { useScroll, ScrollType } from '../../hooks'; +import { useActionItemCardStyles as useStyles } from '../../utils/styles'; + +type ActionItemsProps = { + active: Alert[]; + snoozed: Alert[]; + accepted: Alert[]; + dismissed: Alert[]; +}; + +export const ActionItems = ({ + active, + snoozed, + accepted, + dismissed, +}: ActionItemsProps) => { + const classes = useStyles(); + const [, setScroll] = useScroll(); + + const isSnoozedButtonDisplayed = !!snoozed.length; + const isAcceptedButtonDisplayed = !!accepted.length; + const isDismissedButtonDisplayed = !!dismissed.length; + const isStatusButtonGroupDisplayed = !!active.length; + + const onStatusButtonClick: MouseEventHandler = () => + setScroll(ScrollType.AlertSummary); + + return ( + <> + + {active.map((alert, index) => ( + + {index + 1}} + /> + {index < active.length - 1 && } + + ))} + + {isStatusButtonGroupDisplayed && ( + + {isAcceptedButtonDisplayed && ( + } + amount={accepted.length} + onClick={onStatusButtonClick} + /> + )} + {isSnoozedButtonDisplayed && ( + } + onClick={onStatusButtonClick} + /> + )} + {isDismissedButtonDisplayed && ( + } + amount={dismissed.length} + onClick={onStatusButtonClick} + /> + )} + + )} + + ); +}; + +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/ActionItems/index.ts b/plugins/cost-insights/src/components/ActionItems/index.ts new file mode 100644 index 0000000000..b63e34b0af --- /dev/null +++ b/plugins/cost-insights/src/components/ActionItems/index.ts @@ -0,0 +1,18 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export { ActionItems } from './ActionItems'; +export { ActionItemCard } from './ActionItemCard'; diff --git a/plugins/cost-insights/src/components/AlertActionCardList/AlertActionCard.tsx b/plugins/cost-insights/src/components/AlertActionCardList/AlertActionCard.tsx deleted file mode 100644 index 78feade952..0000000000 --- a/plugins/cost-insights/src/components/AlertActionCardList/AlertActionCard.tsx +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -import React from 'react'; -import { Avatar, Card, CardHeader } from '@material-ui/core'; -import { useScroll } from '../../hooks'; -import { Alert } from '../../types'; -import { - useAlertActionCardHeader as useHeaderStyles, - useAlertActionCardStyles as useStyles, -} from '../../utils/styles'; - -type AlertActionCardProps = { - alert: Alert; - number: number; -}; - -export const AlertActionCard = ({ alert, number }: AlertActionCardProps) => { - const { scrollIntoView } = useScroll(`alert-${number}`); - const headerClasses = useHeaderStyles(); - const classes = useStyles(); - - return ( - - {number}} - title={alert.title} - subheader={alert.subtitle} - /> - - ); -}; diff --git a/plugins/cost-insights/src/components/AlertInsights/AlertDialog.test.tsx b/plugins/cost-insights/src/components/AlertInsights/AlertDialog.test.tsx new file mode 100644 index 0000000000..5785592aeb --- /dev/null +++ b/plugins/cost-insights/src/components/AlertInsights/AlertDialog.test.tsx @@ -0,0 +1,169 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import React from 'react'; +import { AlertDialog } from './AlertDialog'; +import { render } from '@testing-library/react'; +import { + Alert, + AlertFormProps, + AlertSnoozeOptions, + AlertDismissOptions, +} from '../../types'; + +type MockFormDataProps = AlertFormProps; + +const MockForm = React.forwardRef( + (props, ref) => ( +
+ You. Complete. Me. +
+ ), +); + +describe('', () => { + const snoozableAlert: Alert = { + title: 'title', + subtitle: 'test-subtitle', + onSnoozed: jest.fn(), + }; + + const dimissableAlert: Alert = { + title: 'title', + subtitle: 'subtitle', + onDismissed: jest.fn(), + }; + + const customSnoozeAlert: Alert = { + title: 'title', + subtitle: 'subtitle', + onSnoozed: jest.fn(), + SnoozeForm: MockForm, + }; + + const customDismissAlert: Alert = { + title: 'title', + subtitle: 'subtitle', + onDismissed: jest.fn(), + DismissForm: MockForm, + }; + + const customAcceptAlert: Alert = { + title: 'title', + subtitle: 'test-subtitle', + onAccepted: jest.fn(), + AcceptForm: MockForm, + }; + + it('Displays a default snooze form', () => { + const { getByText } = render( + , + ); + expect(getByText('For how long?')).toBeInTheDocument(); + expect(getByText('Snooze this action item?')).toBeInTheDocument(); + expect( + getByText('This action item will be snoozed for all of Ramones.'), + ).toBeInTheDocument(); + AlertSnoozeOptions.forEach(a => + expect(getByText(a.label)).toBeInTheDocument(), + ); + }); + + it('Displays a custom snooze form', () => { + const { getByText } = render( + , + ); + expect(getByText('You. Complete. Me.')).toBeInTheDocument(); + expect(getByText('Snooze this action item?')).toBeInTheDocument(); + expect( + getByText('This action item will be snoozed for all of Ramones.'), + ).toBeInTheDocument(); + }); + + it('Displays a default dismiss form', () => { + const { getByText } = render( + , + ); + expect(getByText('Dismiss this action item?')).toBeInTheDocument(); + expect( + getByText('This action item will be dismissed for all of Ramones.'), + ).toBeInTheDocument(); + AlertDismissOptions.forEach(a => + expect(getByText(a.label)).toBeInTheDocument(), + ); + }); + + it('Displays a custom dismiss form', () => { + const { getByText } = render( + , + ); + expect(getByText('Dismiss this action item?')).toBeInTheDocument(); + expect(getByText('You. Complete. Me.')).toBeInTheDocument(); + expect( + getByText('This action item will be dismissed for all of Ramones.'), + ).toBeInTheDocument(); + }); + + it('Displays a custom accept form', () => { + const { getByText } = render( + , + ); + expect(getByText('Accept this action item?')).toBeInTheDocument(); + expect(getByText('You. Complete. Me.')).toBeInTheDocument(); + expect( + getByText('This action item will be accepted for all of Ramones.'), + ).toBeInTheDocument(); + }); +}); diff --git a/plugins/cost-insights/src/components/AlertInsights/AlertDialog.tsx b/plugins/cost-insights/src/components/AlertInsights/AlertDialog.tsx new file mode 100644 index 0000000000..aa5b8c59fb --- /dev/null +++ b/plugins/cost-insights/src/components/AlertInsights/AlertDialog.tsx @@ -0,0 +1,210 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React, { useEffect, useRef, useState } from 'react'; +import { default as CloseIcon } from '@material-ui/icons/Close'; +import { + Box, + Button, + Divider, + Dialog, + DialogActions, + IconButton, + DialogContent, + Typography, +} from '@material-ui/core'; +import { + AlertAcceptForm, + AlertDismissForm, + AlertSnoozeForm, +} from '../../forms'; +import { useAlertDialogStyles as useStyles } from '../../utils/styles'; +import { choose } from '../../utils/alerts'; +import { Alert, Maybe } from '../../types'; + +const DEFAULT_FORM_ID = 'alert-form'; + +type AlertDialogProps = { + open: boolean; + group: string; + snoozed: Maybe; + accepted: Maybe; + dismissed: Maybe; + onClose: () => void; + onSubmit: (data: any) => void; +}; + +export const AlertDialog = ({ + open, + group, + snoozed, + accepted, + dismissed, + onClose, + onSubmit, +}: AlertDialogProps) => { + const classes = useStyles(); + const [isButtonDisabled, setDisabled] = useState(true); + const acceptRef = useRef>(null); + const snoozeRef = useRef>(null); + const dismissRef = useRef>(null); + + useEffect(() => { + if (open) { + setDisabled(true); + } else { + setDisabled(false); + } + }, [open]); + + function disableSubmit(isDisabled: boolean) { + setDisabled(isDisabled); + } + + function onDialogClose() { + onClose(); + setDisabled(true); + } + + const SnoozeForm = snoozed?.SnoozeForm ?? AlertSnoozeForm; + const AcceptForm = accepted?.AcceptForm ?? AlertAcceptForm; + const DismissForm = dismissed?.DismissForm ?? AlertDismissForm; + + const isSnoozeFormDisplayed = !!snoozed?.onSnoozed; + const isAcceptFormDisplayed = !!accepted?.onAccepted; + const isDismissFormDisplayed = !!dismissed?.onDismissed; + + const status = [ + isAcceptFormDisplayed, + isSnoozeFormDisplayed, + isDismissFormDisplayed, + ] as const; + + const TransitionProps = { + mountOnEnter: true, + unmountOnExit: true, + // Wait for child component to mount; avoid recycling refs. + onEntered() { + if (acceptRef.current) { + acceptRef.current.id = DEFAULT_FORM_ID; + } + if (snoozeRef.current) { + snoozeRef.current.id = DEFAULT_FORM_ID; + } + if (dismissRef.current) { + dismissRef.current.id = DEFAULT_FORM_ID; + } + }, + }; + + return ( + + + + + + + + + + + {choose(status, ['Accept', 'Snooze', 'Dismiss'])} this action + item? + + + + + This action item will be{' '} + {choose(status, ['accepted', 'snoozed', 'dismissed'])} for all of{' '} + {group}. + + + + + + + {choose(status, [ + accepted?.title, + snoozed?.title, + dismissed?.title, + ])} + + + + {choose(status, [ + accepted?.subtitle, + snoozed?.subtitle, + dismissed?.subtitle, + ])} + + + {isSnoozeFormDisplayed && ( + + )} + {isDismissFormDisplayed && ( + + )} + {isAcceptFormDisplayed && ( + + )} + + + + + + + ); +}; diff --git a/plugins/cost-insights/src/components/AlertInsights/AlertInsights.test.tsx b/plugins/cost-insights/src/components/AlertInsights/AlertInsights.test.tsx new file mode 100644 index 0000000000..cd8444cb1f --- /dev/null +++ b/plugins/cost-insights/src/components/AlertInsights/AlertInsights.test.tsx @@ -0,0 +1,89 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React from 'react'; +import { render, fireEvent, waitFor } from '@testing-library/react'; +import { AlertInsights } from './AlertInsights'; +import { + MockScrollProvider, + MockAlertsProvider, + MockLoadingProvider, +} from '../../utils/tests'; + +function renderInContext(children: JSX.Element) { + return render( + + + {children} + + , + ); +} + +describe('', () => { + it('should display the correct header if there are active action items', () => { + const { getByText, queryByText } = renderInContext( + , + ); + expect( + getByText( + 'This section outlines suggested action items your team can address to improve cloud costs.', + ), + ).toBeInTheDocument(); + expect(queryByText('Hidden Action Item')).not.toBeInTheDocument(); + }); + + it('should display alert summary if there are hidden action items', async () => { + const { getByText, getByRole } = renderInContext( + , + ); + + expect( + getByText( + "All of your team's action items are hidden. Maybe it's time to give them another look?", + ), + ).toBeInTheDocument(); + expect(getByText('Hidden Action Item')).toBeInTheDocument(); + + fireEvent.click(getByRole('button', { name: 'expand' })); + await waitFor(() => getByRole('img', { name: 'snoozed' })); + + expect(getByText('Vol. 4')).toBeInTheDocument(); + expect(getByText('Sabotage')).toBeInTheDocument(); + }); +}); diff --git a/plugins/cost-insights/src/components/AlertInsights/AlertInsights.tsx b/plugins/cost-insights/src/components/AlertInsights/AlertInsights.tsx index 4606906f04..1bcb684052 100644 --- a/plugins/cost-insights/src/components/AlertInsights/AlertInsights.tsx +++ b/plugins/cost-insights/src/components/AlertInsights/AlertInsights.tsx @@ -14,31 +14,203 @@ * limitations under the License. */ -import React from 'react'; -import { Grid } from '@material-ui/core'; -import { AlertInsightsSection } from './AlertInsightsSection'; +import React, { useCallback, useEffect, useState } from 'react'; +import pluralize from 'pluralize'; +import { Box, Grid, Snackbar } from '@material-ui/core'; +import { default as MuiAlert } from '@material-ui/lab/Alert'; +import { AlertDialog } from './AlertDialog'; +import { AlertStatusSummary } from './AlertStatusSummary'; +import { AlertStatusSummaryButton } from './AlertStatusSummaryButton'; import { AlertInsightsHeader } from './AlertInsightsHeader'; -import { Alert } from '../../types'; +import { AlertInsightsSection } from './AlertInsightsSection'; +import { + useAlerts, + useScroll, + useLoading, + ScrollType, + MapLoadingToProps, +} from '../../hooks'; +import { DefaultLoadingAction } from '../../utils/loading'; +import { Alert, AlertOptions, Maybe } from '../../types'; +import { sumOfAllAlerts } from '../../utils/alerts'; -const title = "Your team's action items"; -const subtitle = - 'This section outlines suggested action items your team can address to improve cloud costs.'; +type MapLoadingtoAlerts = (isLoading: boolean) => void; + +const mapLoadingToAlerts: MapLoadingToProps = ({ + dispatch, +}) => (isLoading: boolean) => + dispatch({ [DefaultLoadingAction.CostInsightsAlerts]: isLoading }); type AlertInsightsProps = { - alerts: Array; + group: string; + active: Alert[]; + snoozed: Alert[]; + accepted: Alert[]; + dismissed: Alert[]; }; -export const AlertInsights = ({ alerts }: AlertInsightsProps) => ( - - - - - - {alerts.map((alert, index) => ( - - +export const AlertInsights = ({ + group, + active, + snoozed, + accepted, + dismissed, +}: AlertInsightsProps) => { + const [alerts, setAlerts] = useAlerts(); + const [scroll, , ScrollAnchor] = useScroll(); + const dispatchLoadingAlerts = useLoading(mapLoadingToAlerts); + // Allow users to pass null values for data. + const [data, setData] = useState>(undefined); + const [error, setError] = useState>(null); + const [isDialogOpen, setDialogOpen] = useState(false); + const [isSummaryOpen, setSummaryOpen] = useState(false); + const [isSnackbarOpen, setSnackbarOpen] = useState(false); + + const closeDialog = useCallback(() => { + setData(undefined); + setDialogOpen(false); + setAlerts({ dismissed: null, snoozed: null, accepted: null }); + }, [setAlerts]); + + useEffect(() => { + async function callHandler( + options: AlertOptions, + callback: (options: AlertOptions) => Promise, + ) { + closeDialog(); + dispatchLoadingAlerts(true); + try { + const a: Alert[] = await callback(options); + setAlerts({ alerts: a }); + } catch (e) { + setError(e); + } finally { + dispatchLoadingAlerts(false); + } + } + + const options: AlertOptions = { data, group }; + const onSnoozed = alerts.snoozed?.onSnoozed?.bind(alerts.snoozed) ?? null; + const onAccepted = + alerts.accepted?.onAccepted?.bind(alerts.accepted) ?? null; + const onDismissed = + alerts.dismissed?.onDismissed?.bind(alerts.dismissed) ?? null; + + if (data !== undefined) { + if (onSnoozed) { + callHandler(options, onSnoozed); + } else if (onAccepted) { + callHandler(options, onAccepted); + } else if (onDismissed) { + callHandler(options, onDismissed); + } + } + }, [group, data, alerts, setAlerts, closeDialog, dispatchLoadingAlerts]); + + useEffect(() => { + if (scroll === ScrollType.AlertSummary) { + setSummaryOpen(true); + } + }, [scroll]); + + useEffect(() => { + if (error) { + setSnackbarOpen(true); + } else { + setSnackbarOpen(false); + } + }, [error]); + + useEffect(() => { + function toggleDialogOnStatusChange() { + const isAlertSnoozed = !!alerts.snoozed; + const isAlertAccepted = !!alerts.accepted; + const isAlertDismissed = !!alerts.dismissed; + + if (isAlertSnoozed || isAlertDismissed || isAlertAccepted) { + setDialogOpen(true); + } else { + setDialogOpen(false); + } + } + + toggleDialogOnStatusChange(); + }, [alerts.snoozed, alerts.dismissed, alerts.accepted]); + + function onSnackbarClose() { + setError(null); + } + + function onDialogSubmit(data: any) { + setData(data); + } + + function onSummaryButtonClick() { + setSummaryOpen(prevOpen => !prevOpen); + } + + const total = [accepted, snoozed, dismissed].reduce(sumOfAllAlerts, 0); + + const isAlertStatusSummaryDisplayed = !!total; + const isAlertInsightSectionDisplayed = !!active.length; + // AlertInsights will not display if there aren't any active or hidden items. + + return ( + + + + + {isAlertInsightSectionDisplayed && ( + + {active.map((alert, index) => ( + + + + ))} - ))} + )} + {isAlertStatusSummaryDisplayed && ( + + + + + {pluralize('Hidden Action Item', total)} + + + + + )} + + + + {error?.message} + + - -); + ); +}; diff --git a/plugins/cost-insights/src/components/AlertInsights/AlertInsightsHeader.tsx b/plugins/cost-insights/src/components/AlertInsights/AlertInsightsHeader.tsx index b58f6973ae..a8006b81d0 100644 --- a/plugins/cost-insights/src/components/AlertInsights/AlertInsightsHeader.tsx +++ b/plugins/cost-insights/src/components/AlertInsights/AlertInsightsHeader.tsx @@ -30,10 +30,11 @@ export const AlertInsightsHeader = ({ subtitle, }: AlertInsightsHeaderProps) => { const classes = useStyles(); - const { ScrollAnchor } = useScroll(DefaultNavigation.AlertInsightsHeader); + const [, , ScrollAnchor] = useScroll(); + return ( - + {title}{' '} diff --git a/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSection.test.tsx b/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSection.test.tsx index 54d350298c..f04e1d8b01 100644 --- a/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSection.test.tsx +++ b/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSection.test.tsx @@ -17,10 +17,10 @@ import React from 'react'; import { AlertInsightsSection } from './AlertInsightsSection'; import { render } from '@testing-library/react'; import { Alert } from '../../types'; -import { MockScrollProvider } from '../..'; +import { AlertState } from '../../hooks'; +import { MockScrollProvider, MockAlertsProvider } from '../../utils/tests'; const mockAlert: Alert = { - element:
, subtitle: 'Wherefore was I to this keen mockery born? When at your hands did I deserve this scorn?', title: 'Mock alert', @@ -29,26 +29,110 @@ const mockAlert: Alert = { describe('', () => { it('Renders alert without exploding', () => { - const { getByText } = render( - - - , + const { getByText, queryByText } = render( + + + + + , ); expect(getByText(mockAlert.title)).toBeInTheDocument(); expect(getByText(mockAlert.subtitle)).toBeInTheDocument(); expect(getByText('View Instructions')).toBeInTheDocument(); + expect(queryByText('Snooze')).not.toBeInTheDocument(); + expect(queryByText('Accept')).not.toBeInTheDocument(); + expect(queryByText('Dismiss')).not.toBeInTheDocument(); }); it('Hides instructions button if url is not provided', () => { - const alert = { + const alert: Alert = { ...mockAlert, url: undefined, }; const { queryByText } = render( - - - , + + + + + , ); expect(queryByText('View Instructions')).not.toBeInTheDocument(); }); + + it('Displays a snooze button if a hook is provided', () => { + const alert: Alert = { + ...mockAlert, + onSnoozed: jest.fn(), + }; + + const context: AlertState = { + alerts: [], + snoozed: alert, + dismissed: null, + accepted: null, + }; + + const { queryByText, getByText } = render( + + + + + , + ); + + expect(getByText('Snooze')).toBeInTheDocument(); + expect(queryByText('Accept')).not.toBeInTheDocument(); + expect(queryByText('Dismiss')).not.toBeInTheDocument(); + }); + + it('Displays a dismiss button if a hook is provided', () => { + const alert: Alert = { + ...mockAlert, + onDismissed: jest.fn(), + }; + const context: AlertState = { + alerts: [], + snoozed: null, + dismissed: alert, + accepted: null, + }; + + const { queryByText, getByText } = render( + + + + + , + ); + + expect(getByText('Dismiss')).toBeInTheDocument(); + expect(queryByText('Accept')).not.toBeInTheDocument(); + expect(queryByText('Snooze')).not.toBeInTheDocument(); + }); + + it('Displays an accept button if a hook is provided', () => { + const alert: Alert = { + ...mockAlert, + onAccepted: jest.fn(), + }; + + const context: AlertState = { + alerts: [], + snoozed: null, + dismissed: null, + accepted: alert, + }; + + const { queryByText, getByText } = render( + + + + + , + ); + + expect(getByText('Accept')).toBeInTheDocument(); + expect(queryByText('Snooze')).not.toBeInTheDocument(); + expect(queryByText('Dismiss')).not.toBeInTheDocument(); + }); }); diff --git a/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSection.tsx b/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSection.tsx index 7e371c8001..dda2d66f3f 100644 --- a/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSection.tsx +++ b/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSection.tsx @@ -15,7 +15,11 @@ */ import React from 'react'; import { Box, Button } from '@material-ui/core'; +import { default as SnoozeIcon } from '@material-ui/icons/AccessTime'; +import { default as AcceptIcon } from '@material-ui/icons/Check'; +import { default as DismissIcon } from '@material-ui/icons/Delete'; import { AlertInsightsSectionHeader } from './AlertInsightsSectionHeader'; +import { useAlerts } from '../../hooks'; import { Alert } from '../../types'; type AlertInsightsSectionProps = { @@ -27,18 +31,60 @@ export const AlertInsightsSection = ({ alert, number, }: AlertInsightsSectionProps) => { + const [, setAlerts] = useAlerts(); + + const isSnoozeButtonDisplayed = !!alert.onSnoozed; + const isAcceptButtonDisplayed = !!alert.onAccepted; + const isDismissButtonDisplayed = !!alert.onDismissed; + const isButtonGroupDisplayed = + isSnoozeButtonDisplayed || + isAcceptButtonDisplayed || + isDismissButtonDisplayed; + return ( - - - {alert.url && ( - - + + + {isButtonGroupDisplayed && ( + + {isAcceptButtonDisplayed && ( + + + + )} + {isSnoozeButtonDisplayed && ( + + + + )} + {isDismissButtonDisplayed && ( + + )} )} {alert.element} diff --git a/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSectionHeader.tsx b/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSectionHeader.tsx index 3613e3adc8..c463a05609 100644 --- a/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSectionHeader.tsx +++ b/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSectionHeader.tsx @@ -15,34 +15,47 @@ */ import React from 'react'; -import { Avatar, Box, Grid, Typography } from '@material-ui/core'; +import { Avatar, Box, Button, Grid, Typography } from '@material-ui/core'; import { useAlertInsightsSectionStyles as useStyles } from '../../utils/styles'; import { useScroll } from '../../hooks'; +import { Alert } from '../../types'; type AlertInsightsSectionHeaderProps = { + alert: Alert; number: number; - title: string; - subtitle: string; }; export const AlertInsightsSectionHeader = ({ + alert, number, - title, - subtitle, }: AlertInsightsSectionHeaderProps) => { - const { ScrollAnchor } = useScroll(`alert-${number}`); + const [, , ScrollAnchor] = useScroll(); const classes = useStyles(); + + const isViewInstructionsButtonDisplayed = !!alert.url; + return ( - - + + - {number} - - - {title} - {subtitle} + + + {number} + + + {alert.title} + {alert.subtitle} + + + {isViewInstructionsButtonDisplayed && ( + + + + )} ); diff --git a/plugins/cost-insights/src/components/AlertInsights/AlertStatusSummary.test.tsx b/plugins/cost-insights/src/components/AlertInsights/AlertStatusSummary.test.tsx new file mode 100644 index 0000000000..830db19fcd --- /dev/null +++ b/plugins/cost-insights/src/components/AlertInsights/AlertStatusSummary.test.tsx @@ -0,0 +1,58 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import React from 'react'; +import { AlertStatusSummary } from './AlertStatusSummary'; +import { render } from '@testing-library/react'; +import { Alert, AlertStatus } from '../../types'; +import { MockScrollProvider } from '../../utils/tests'; + +const mockSnoozed: Alert = { + title: 'snoozed-title', + subtitle: 'snoozed-subtitle', + status: AlertStatus.Snoozed, +}; + +const mockAccepted: Alert = { + title: 'accepted-title', + subtitle: 'accepted-subtitle', + status: AlertStatus.Accepted, +}; + +const mockDismissed: Alert = { + title: 'dismissed-title', + subtitle: 'dismissed-subtitle', + status: AlertStatus.Dismissed, +}; + +describe('', () => { + it('should display alerts', () => { + const { getByText, getByRole } = render( + + + , + ); + [mockSnoozed, mockAccepted, mockDismissed].forEach(a => { + expect(getByText(a.title)).toBeInTheDocument(); + expect(getByText(a.subtitle)).toBeInTheDocument(); + expect(getByRole('img', { name: a.status })).toBeInTheDocument(); + }); + }); +}); diff --git a/plugins/cost-insights/src/components/AlertInsights/AlertStatusSummary.tsx b/plugins/cost-insights/src/components/AlertInsights/AlertStatusSummary.tsx new file mode 100644 index 0000000000..cb0a149511 --- /dev/null +++ b/plugins/cost-insights/src/components/AlertInsights/AlertStatusSummary.tsx @@ -0,0 +1,116 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React, { Fragment } from 'react'; +import { Avatar, Box, Collapse, Divider } from '@material-ui/core'; +import { default as AcceptIcon } from '@material-ui/icons/Check'; +import { default as DismissIcon } from '@material-ui/icons/Delete'; +import { default as SnoozeIcon } from '@material-ui/icons/AccessTime'; +import { ActionItemCard } from '../ActionItems'; +import { Alert, AlertStatus } from '../../types'; +import { useActionItemCardStyles as useStyles } from '../../utils/styles'; + +type AlertStatusSummaryProps = { + open: boolean; + snoozed: Alert[]; + accepted: Alert[]; + dismissed: Alert[]; +}; + +export const AlertStatusSummary = ({ + open, + snoozed, + accepted, + dismissed, +}: AlertStatusSummaryProps) => { + const classes = useStyles(); + + const isSnoozedListDisplayed = !!snoozed.length; + const isAcceptedListDisplayed = !!accepted.length; + const isDismissedListDisplayed = !!dismissed.length; + + return ( + + {isAcceptedListDisplayed && ( + + {accepted.map((alert, index) => ( + + + {/* Icons indicate alert status. Do not hide from accesibility tree */} + + + } + /> + {index < accepted.length - 1 && } + + ))} + + )} + {isSnoozedListDisplayed && ( + + {snoozed.map((alert, index) => ( + + + + + } + /> + {index < snoozed.length - 1 && } + + ))} + + )} + {isDismissedListDisplayed && ( + + {dismissed.map((alert, index) => ( + + + + + } + /> + {index < dismissed.length - 1 && } + + ))} + + )} + + ); +}; diff --git a/plugins/cost-insights/src/components/AlertInsights/AlertStatusSummaryButton.tsx b/plugins/cost-insights/src/components/AlertInsights/AlertStatusSummaryButton.tsx new file mode 100644 index 0000000000..ed8c7fabbb --- /dev/null +++ b/plugins/cost-insights/src/components/AlertInsights/AlertStatusSummaryButton.tsx @@ -0,0 +1,54 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React, { useState, MouseEventHandler, PropsWithChildren } from 'react'; +import classnames from 'classnames'; +import { Button } from '@material-ui/core'; +import { default as ExpandMoreIcon } from '@material-ui/icons/ExpandMore'; +import { useAlertStatusSummaryButtonStyles as useStyles } from '../../utils/styles'; + +type AlertStatusSummaryButtonProps = { + onClick: MouseEventHandler; +}; + +export const AlertStatusSummaryButton = ({ + children, + onClick, +}: PropsWithChildren) => { + const classes = useStyles(); + const [clicked, setClicked] = useState(false); + const iconClassName = classnames(classes.icon, { + [classes.clicked]: clicked, + }); + + const handleOnClick: MouseEventHandler = e => { + setClicked(prevClicked => !prevClicked); + onClick(e); + }; + + return ( + + ); +}; diff --git a/plugins/cost-insights/src/components/CostInsightsNavigation/CostInsightsNavigation.tsx b/plugins/cost-insights/src/components/CostInsightsNavigation/CostInsightsNavigation.tsx index a30b6c5a90..dd1652af46 100644 --- a/plugins/cost-insights/src/components/CostInsightsNavigation/CostInsightsNavigation.tsx +++ b/plugins/cost-insights/src/components/CostInsightsNavigation/CostInsightsNavigation.tsx @@ -105,13 +105,14 @@ export const CostInsightsNavigation = React.memo( const NavigationMenuItem = ({ navigation, icon, title }: NavigationItem) => { const classes = useStyles(); - const { scrollIntoView } = useScroll(navigation); + const [, setScroll] = useScroll(); + return ( setScroll(navigation)} > {icon} { const classes = useSubtleTypographyStyles(); @@ -54,16 +68,24 @@ export const CostInsightsPage = () => { const config = useConfig(); const groups = useGroups(); const lastCompleteBillingDate = useLastCompleteBillingDate(); + const [alerts, setAlerts] = useAlerts(); const [currency, setCurrency] = useCurrency(); const [projects, setProjects] = useState>(null); const [products, setProducts] = useState>(null); const [dailyCost, setDailyCost] = useState>(null); const [metricData, setMetricData] = useState>(null); - const [alerts, setAlerts] = useState>(null); const [error, setError] = useState>(null); const { pageFilters, setPageFilters } = useFilters(p => p); + const snoozed = useMemo(() => alerts.alerts.filter(isSnoozed), [alerts]); + const accepted = useMemo(() => alerts.alerts.filter(isAccepted), [alerts]); + const dismissed = useMemo(() => alerts.alerts.filter(isDismissed), [alerts]); + const activeAlerts = useMemo(() => alerts.alerts.filter(isActive), [alerts]); + + const isActionItemsDisplayed = !!activeAlerts.length; + const isAlertInsightsDisplayed = !!alerts.alerts.length; + const { loadingActions, loadingGroups, @@ -120,7 +142,7 @@ export const CostInsightsPage = () => { : client.getGroupDailyCost(pageFilters.group, intervals), ]); setProjects(fetchedProjects); - setAlerts(fetchedAlerts); + setAlerts({ alerts: fetchedAlerts }); setMetricData(fetchedMetricData); setDailyCost(fetchedDailyCost); } else { @@ -145,6 +167,7 @@ export const CostInsightsPage = () => { loadingActions, loadingGroups, loadingBillingDate, + setAlerts, dispatchLoadingInsights, dispatchLoadingInitial, dispatchLoadingNone, @@ -177,8 +200,8 @@ export const CostInsightsPage = () => { ); } - // These should be defined, alerts can be an empty array but that's truthy - if (!dailyCost || !alerts) { + + if (!dailyCost) { return ( {`Error: Could not fetch cost insights data for team ${pageFilters.group}`} ); @@ -228,7 +251,7 @@ export const CostInsightsPage = () => { @@ -249,19 +272,22 @@ export const CostInsightsPage = () => { owner={pageFilters.group} groups={groups} hasCostData={!!dailyCost.aggregation.length} - alerts={alerts.length} + alerts={activeAlerts.length} /> - {!!alerts.length && ( - <> - - - - - - - - )} + + + + + + + + @@ -276,14 +302,20 @@ export const CostInsightsPage = () => { - - {!!alerts?.length && ( + + - + - )} - - {!alerts.length && } + + + {!isAlertInsightsDisplayed && } ( - + + + diff --git a/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewCard.tsx b/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewCard.tsx index 39adc4bbf1..1539e431fd 100644 --- a/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewCard.tsx +++ b/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewCard.tsx @@ -49,7 +49,7 @@ export const CostOverviewCard = ({ const config = useConfig(); const [tabIndex, setTabIndex] = useState(0); - const { ScrollAnchor } = useScroll(DefaultNavigation.CostOverviewCard); + const [, , ScrollAnchor] = useScroll(); const { setDuration, setProject, setMetric, ...filters } = useFilters( mapFiltersToProps, ); @@ -95,7 +95,7 @@ export const CostOverviewCard = ({ return ( - + {dailyCostData.groupedCosts && } diff --git a/plugins/cost-insights/src/components/MigrationAlertCard/MigrationAlertCard.tsx b/plugins/cost-insights/src/components/MigrationAlertCard/MigrationAlertCard.tsx new file mode 100644 index 0000000000..f4d174d521 --- /dev/null +++ b/plugins/cost-insights/src/components/MigrationAlertCard/MigrationAlertCard.tsx @@ -0,0 +1,60 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React from 'react'; +import { Box } from '@material-ui/core'; +import { InfoCard } from '@backstage/core'; +import { MigrationBarChartLegend } from './MigrationBarChartLegend'; +import { MigrationBarChart } from './MigrationBarChart'; +import { MigrationData } from '../../types'; + +type MigrationAlertProps = { + data: MigrationData; + title: string; + subheader: string; + currentProduct: string; + comparedProduct: string; +}; + +export const MigrationAlertCard = ({ + data, + title, + subheader, + currentProduct, + comparedProduct, +}: MigrationAlertProps) => { + return ( + + + + + + + + + + + ); +}; diff --git a/plugins/cost-insights/src/components/MigrationAlertCard/MigrationBarChart.tsx b/plugins/cost-insights/src/components/MigrationAlertCard/MigrationBarChart.tsx new file mode 100644 index 0000000000..60aa49b2bc --- /dev/null +++ b/plugins/cost-insights/src/components/MigrationAlertCard/MigrationBarChart.tsx @@ -0,0 +1,54 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React from 'react'; +import { BarChart } from '../BarChart'; +import { + BarChartOptions, + CostInsightsTheme, + Entity, + ResourceData, +} from '../../types'; +import { useTheme } from '@material-ui/core'; + +type MigrationBarChartProps = { + currentProduct: string; + comparedProduct: string; + services: Array; +}; + +export const MigrationBarChart = ({ + currentProduct, + comparedProduct, + services, +}: MigrationBarChartProps) => { + const theme = useTheme(); + + const options: BarChartOptions = { + previousFill: theme.palette.magenta, + currentFill: theme.palette.yellow, + previousName: comparedProduct, + currentName: currentProduct, + }; + + const resources: ResourceData[] = services.map(service => ({ + name: service.id, + previous: service.aggregation[0], + current: service.aggregation[1], + })); + + return ; +}; diff --git a/plugins/cost-insights/src/components/MigrationAlertCard/MigrationBarChartLegend.tsx b/plugins/cost-insights/src/components/MigrationAlertCard/MigrationBarChartLegend.tsx new file mode 100644 index 0000000000..a84e865e28 --- /dev/null +++ b/plugins/cost-insights/src/components/MigrationAlertCard/MigrationBarChartLegend.tsx @@ -0,0 +1,58 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React from 'react'; +import { Box, useTheme } from '@material-ui/core'; +import { CostGrowth } from '../CostGrowth'; +import { LegendItem } from '../LegendItem'; +import { ChangeStatistic, CostInsightsTheme, Duration } from '../../types'; +import { monthOf } from '../../utils/formatters'; + +export type MigrationBarChartLegendProps = { + change: ChangeStatistic; + startDate: string; + currentProduct: string; + comparedProduct: string; +}; + +export const MigrationBarChartLegend = ({ + currentProduct, + comparedProduct, + change, + startDate, +}: MigrationBarChartLegendProps) => { + const theme = useTheme(); + return ( + + + + {currentProduct} + + + + + {comparedProduct} + + + + + + + ); +}; diff --git a/plugins/cost-insights/src/components/AlertActionCardList/index.ts b/plugins/cost-insights/src/components/MigrationAlertCard/index.ts similarity index 90% rename from plugins/cost-insights/src/components/AlertActionCardList/index.ts rename to plugins/cost-insights/src/components/MigrationAlertCard/index.ts index a0ce118482..4feb749c27 100644 --- a/plugins/cost-insights/src/components/AlertActionCardList/index.ts +++ b/plugins/cost-insights/src/components/MigrationAlertCard/index.ts @@ -14,4 +14,4 @@ * limitations under the License. */ -export { AlertActionCardList } from './AlertActionCardList'; +export { MigrationAlertCard } from './MigrationAlertCard'; diff --git a/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsCard.test.tsx b/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsCard.test.tsx index fc37bc5dd6..406affcd15 100644 --- a/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsCard.test.tsx +++ b/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsCard.test.tsx @@ -77,17 +77,6 @@ const renderProductInsightsCardInTestApp = async ( ); describe('', () => { - it('Renders the scroll anchors', async () => { - const rendered = await renderProductInsightsCardInTestApp( - mockProductCost, - MockComputeEngine, - Duration.P30D, - ); - expect( - rendered.queryByTestId(`scroll-test-compute-engine`), - ).toBeInTheDocument(); - }); - it('Should render the right subheader for products with cost data', async () => { const entity = { ...mockProductCost, diff --git a/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsCard.tsx b/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsCard.tsx index 534ef490c3..828d229c94 100644 --- a/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsCard.tsx +++ b/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsCard.tsx @@ -60,7 +60,7 @@ export const ProductInsightsCard = ({ }: PropsWithChildren) => { const classes = useStyles(); const mountedRef = useRef(false); - const { ScrollAnchor } = useScroll(product.kind); + const [, , ScrollAnchor] = useScroll(); const [error, setError] = useState>(null); const dispatchLoading = useLoading(mapLoadingToProps); const lastCompleteBillingDate = useLastCompleteBillingDate(); @@ -107,7 +107,7 @@ export const ProductInsightsCard = ({ if (error || !entity) { return ( - + {error ? error.message @@ -123,7 +123,7 @@ export const ProductInsightsCard = ({ subheader={subheader} headerProps={headerProps} > - + {entities.length ? ( ; + +export const AlertAcceptForm = forwardRef< + HTMLFormElement, + AlertAcceptFormProps +>(({ onSubmit, disableSubmit }, ref) => { + const [checked, setChecked] = useState(false); + + const onFormSubmit: FormEventHandler = e => { + e.preventDefault(); + onSubmit(null); + }; + + const onChecked = (_: ChangeEvent, isChecked: boolean) => { + setChecked(isChecked); + disableSubmit(!isChecked); + }; + + return ( +
+ + + } + /> + +
+ ); +}); diff --git a/plugins/cost-insights/src/forms/AlertDismissForm.tsx b/plugins/cost-insights/src/forms/AlertDismissForm.tsx new file mode 100644 index 0000000000..1b3cbdb363 --- /dev/null +++ b/plugins/cost-insights/src/forms/AlertDismissForm.tsx @@ -0,0 +1,156 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React, { + ChangeEvent, + useEffect, + useState, + forwardRef, + FormEventHandler, +} from 'react'; +import { + Box, + Collapse, + FormControl, + FormControlLabel, + TextField, + Typography, + Radio, + RadioGroup, +} from '@material-ui/core'; +import { + Alert, + AlertFormProps, + AlertDismissReason, + AlertDismissOptions, + AlertDismissFormData, + Maybe, +} from '../types'; +import { useAlertDialogStyles as useStyles } from '../utils/styles'; + +export type AlertDismissFormProps = AlertFormProps; + +export const AlertDismissForm = forwardRef< + HTMLFormElement, + AlertDismissFormProps +>(({ onSubmit, disableSubmit }, ref) => { + const classes = useStyles(); + const [other, setOther] = useState>(null); + const [feedback, setFeedback] = useState>(null); + const [reason, setReason] = useState( + AlertDismissReason.Resolved, + ); + + const onFormSubmit: FormEventHandler = e => { + function submit() { + onSubmit({ + other: other, + reason: reason, + feedback: feedback, + }); + } + + e.preventDefault(); + if (reason) { + submit(); + } + }; + + const onReasonChange = (_: ChangeEvent, value: string) => { + setReason(value as AlertDismissReason); + }; + + const onOtherChange = (e: ChangeEvent) => { + return e.target.value + ? setOther(e.target.value as AlertDismissReason) + : setOther(null); + }; + + const onFeedbackChange = (e: ChangeEvent) => { + return e.target.value + ? setFeedback(e.target.value as AlertDismissReason) + : setFeedback(null); + }; + + useEffect(() => { + function validateDismissForm() { + if (reason === AlertDismissReason.Other) { + if (other) { + disableSubmit(false); + } else { + disableSubmit(true); + } + } else if (reason) { + disableSubmit(false); + } else { + disableSubmit(true); + } + } + + validateDismissForm(); + }, [reason, other, disableSubmit]); + + return ( +
+ + + Reason for dismissing? + + + + {AlertDismissOptions.map(option => ( + } + /> + ))} + + + + + + + + + Any other feedback you can provide? + + + +
+ ); +}); diff --git a/plugins/cost-insights/src/forms/AlertSnoozeForm.tsx b/plugins/cost-insights/src/forms/AlertSnoozeForm.tsx new file mode 100644 index 0000000000..51218fe844 --- /dev/null +++ b/plugins/cost-insights/src/forms/AlertSnoozeForm.tsx @@ -0,0 +1,115 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React, { + ChangeEvent, + useEffect, + useState, + forwardRef, + FormEventHandler, +} from 'react'; +import dayjs from 'dayjs'; +import { + Box, + Collapse, + FormControl, + FormControlLabel, + RadioGroup, + Radio, + Typography, +} from '@material-ui/core'; +import { + Alert, + AlertFormProps, + Duration, + DEFAULT_DATE_FORMAT, + Maybe, + AlertSnoozeFormData, + AlertSnoozeOptions, +} from '../types'; +import { useAlertDialogStyles as useStyles } from '../utils/styles'; +import { intervalsOf } from '../utils/duration'; + +export type AlertSnoozeFormProps = AlertFormProps; + +export const AlertSnoozeForm = forwardRef< + HTMLFormElement, + AlertSnoozeFormProps +>(({ onSubmit, disableSubmit }, ref) => { + const classes = useStyles(); + const [error, setError] = useState>(null); + const [duration, setDuration] = useState>(Duration.P7D); + + const onFormSubmit: FormEventHandler = e => { + e.preventDefault(); + if (duration) { + const repeatInterval = 1; + const inclusiveEndDate = dayjs().format(DEFAULT_DATE_FORMAT); + onSubmit({ + intervals: intervalsOf(duration, inclusiveEndDate, repeatInterval), + }); + } else { + setError(new Error('Please select an option.')); + } + }; + + const onSnoozeDurationChange = ( + _: ChangeEvent, + value: string, + ) => { + setDuration(value as Duration); + }; + + useEffect(() => { + function clearErrorOnFormDataChange() { + disableSubmit(false); + setError(prevError => (prevError ? null : prevError)); + } + + clearErrorOnFormDataChange(); + }, [duration, disableSubmit]); + + const isErrorMessageDisplayed = !!error; + + return ( +
+ + + For how long? + + + {error?.message} + + + + {AlertSnoozeOptions.map(option => ( + } + /> + ))} + + + +
+ ); +}); diff --git a/plugins/cost-insights/src/forms/MigrationDismissForm.tsx b/plugins/cost-insights/src/forms/MigrationDismissForm.tsx new file mode 100644 index 0000000000..ce71af3a37 --- /dev/null +++ b/plugins/cost-insights/src/forms/MigrationDismissForm.tsx @@ -0,0 +1,108 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React, { + useEffect, + useState, + forwardRef, + FormEventHandler, + ChangeEvent, +} from 'react'; +import { + Checkbox, + FormControl, + FormControlLabel, + FormGroup, + Typography, +} from '@material-ui/core'; +import { AlertFormProps, Entity } from '../types'; +import { MigrationAlert } from '../alerts'; +import { findAlways } from '../utils/assert'; + +export type MigrationDismissFormData = { + services: Entity[]; +}; + +export type MigrationDismissFormProps = AlertFormProps< + MigrationAlert, + MigrationDismissFormData +>; + +export const MigrationDismissForm = forwardRef< + HTMLFormElement, + MigrationDismissFormProps +>(({ onSubmit, disableSubmit, alert }, ref) => { + const [services, setServices] = useState(alert.data.services); + + const onFormSubmit: FormEventHandler = e => { + /* Remember to prevent default form behavior */ + e.preventDefault(); + onSubmit({ services: services }); + }; + + const onCheckboxChange = ( + e: ChangeEvent, + checked: boolean, + ) => { + if (checked) { + const service = findAlways( + alert.data.services, + s => s.id === e.target.value, + ); + setServices(prevServices => prevServices.concat(service)); + } else { + setServices(prevServices => + prevServices.filter(p => p.id !== e.target.value), + ); + } + }; + + /* Submit button is disabled by default. Use props.disableSubmit to toggle disabled state. */ + useEffect(() => { + if (services.length) { + disableSubmit(false); + } else { + disableSubmit(true); + } + }, [services, disableSubmit]); + + return ( + /* All custom forms must accept a ref and implement an onSubmit handler. */ +
+ + + Or choose which services to dismiss this alert for. + + + {alert.data.services.map((service, index) => ( + p.id === service.id)} + onChange={onCheckboxChange} + /> + } + /> + ))} + + +
+ ); +}); diff --git a/plugins/cost-insights/src/forms/index.ts b/plugins/cost-insights/src/forms/index.ts new file mode 100644 index 0000000000..5b8384a293 --- /dev/null +++ b/plugins/cost-insights/src/forms/index.ts @@ -0,0 +1,21 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export { AlertAcceptForm } from './AlertAcceptForm'; +export { AlertSnoozeForm } from './AlertSnoozeForm'; +export { AlertDismissForm } from './AlertDismissForm'; +export { MigrationDismissForm } from './MigrationDismissForm'; +export type { MigrationDismissFormData } from './MigrationDismissForm'; diff --git a/plugins/cost-insights/src/hooks/index.ts b/plugins/cost-insights/src/hooks/index.ts index 94c556763c..c1a4b3512c 100644 --- a/plugins/cost-insights/src/hooks/index.ts +++ b/plugins/cost-insights/src/hooks/index.ts @@ -16,6 +16,7 @@ export * from './useConfig'; export * from './useCurrency'; +export * from './useAlerts'; export * from './useFilters'; export * from './useCurrency'; export * from './useGroups'; diff --git a/plugins/cost-insights/src/hooks/useAlerts.tsx b/plugins/cost-insights/src/hooks/useAlerts.tsx new file mode 100644 index 0000000000..6bc311b09b --- /dev/null +++ b/plugins/cost-insights/src/hooks/useAlerts.tsx @@ -0,0 +1,76 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import React, { + useReducer, + Dispatch, + SetStateAction, + createContext, + useContext, + PropsWithChildren, +} from 'react'; +import { Alert, Maybe } from '../types'; + +export type AlertsContextProps = { + alerts: AlertState; + setAlerts: Dispatch>>; +}; + +export const AlertsContext = createContext( + undefined, +); + +export type AlertState = { + alerts: Alert[]; + snoozed: Maybe; + accepted: Maybe; + dismissed: Maybe; +}; + +const initialState: AlertState = { + alerts: [], + snoozed: null, + accepted: null, + dismissed: null, +}; + +const reducer = ( + prevState: AlertState, + action: SetStateAction>, +): AlertState => ({ + ...prevState, + ...action, +}); + +export const AlertsProvider = ({ children }: PropsWithChildren<{}>) => { + const [alerts, setAlerts] = useReducer(reducer, initialState); + + return ( + + {children} + + ); +}; + +export function useAlerts() { + const context = useContext(AlertsContext); + return context + ? ([context.alerts, context.setAlerts] as const) + : assertNever(); +} + +function assertNever(): never { + throw new Error('useAlerts cannot be used outside AlertsContext provider'); +} diff --git a/plugins/cost-insights/src/hooks/useScroll.tsx b/plugins/cost-insights/src/hooks/useScroll.tsx index 0a1abbdad3..7f310d59b0 100644 --- a/plugins/cost-insights/src/hooks/useScroll.tsx +++ b/plugins/cost-insights/src/hooks/useScroll.tsx @@ -15,6 +15,7 @@ */ import React, { Dispatch, + ElementType, SetStateAction, useState, useContext, @@ -22,19 +23,14 @@ import React, { useRef, PropsWithChildren, } from 'react'; -import { CSSProperties } from '@material-ui/styles'; import { Maybe } from '../types'; export type ScrollTo = Maybe; export type ScrollContextProps = { - scrollTo: ScrollTo; - setScrollTo: Dispatch>; -}; - -export type ScrollUtils = { - ScrollAnchor: (props: Omit) => JSX.Element; - scrollIntoView: () => void; + scroll: ScrollTo; + setScroll: Dispatch>; + ScrollAnchor: ElementType; }; export interface ScrollAnchorProps extends ScrollIntoViewOptions { @@ -49,28 +45,14 @@ export const ScrollContext = React.createContext< export const ScrollAnchor = ({ id, - top, - left, - behavior, block, inline, + left = 0, + top = -20, + behavior = 'smooth', }: ScrollAnchorProps) => { const divRef = useRef(null); - const context = useContext(ScrollContext); - - if (!context) { - assertNever(); - } - - const { scrollTo, setScrollTo } = context; - - const styles: CSSProperties = { - position: 'absolute', - height: 0, - width: 0, - top: top || 0, - left: left || 0, - }; + const [scroll, setScroll] = useScroll(); useEffect(() => { function scrollIntoView() { @@ -80,39 +62,46 @@ export const ScrollAnchor = ({ inline: inline || 'nearest', }; - if (divRef.current && scrollTo === id) { + if (divRef.current && scroll === id) { divRef.current.scrollIntoView(options); - setScrollTo(null); + setScroll(null); } } scrollIntoView(); - }, [scrollTo, setScrollTo, id, behavior, block, inline]); + }, [scroll, setScroll, id, behavior, block, inline]); - return
; + return ( +
+ ); }; export const ScrollProvider = ({ children }: PropsWithChildren<{}>) => { - const [scrollTo, setScrollTo] = useState(null); + const [scroll, setScroll] = useState(null); return ( - + {children} ); }; -export function useScroll(id: ScrollTo): ScrollUtils { +export enum ScrollType { + AlertSummary = 'alert-status-summary', +} + +export function useScroll() { const context = useContext(ScrollContext); if (!context) { assertNever(); } - return { - ScrollAnchor: props => , - scrollIntoView: () => context.setScrollTo(id), - }; + return [context.scroll, context.setScroll, context.ScrollAnchor] as const; } function assertNever(): never { diff --git a/plugins/cost-insights/src/index.ts b/plugins/cost-insights/src/index.ts index 7cd0456ad1..13ce929401 100644 --- a/plugins/cost-insights/src/index.ts +++ b/plugins/cost-insights/src/index.ts @@ -17,8 +17,8 @@ export { plugin } from './plugin'; export * from './client'; export * from './api'; +export { ProjectGrowthAlert, UnlabeledDataflowAlert } from './alerts'; export * from './components'; export { useCurrency } from './hooks'; export * from './types'; export * from './utils/tests'; -export * from './utils/alerts'; diff --git a/plugins/cost-insights/src/types/Alert.ts b/plugins/cost-insights/src/types/Alert.ts index 18b86c8cb8..21e1a779a8 100644 --- a/plugins/cost-insights/src/types/Alert.ts +++ b/plugins/cost-insights/src/types/Alert.ts @@ -13,23 +13,147 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - +import { ForwardRefExoticComponent, RefAttributes } from 'react'; import { ChangeStatistic } from './ChangeStatistic'; +import { Duration } from './Duration'; import { Maybe } from './Maybe'; /** * Generic alert type with required fields for display. The `element` field will be rendered in * the Cost Insights "Action Items" section. This should use data fetched in the CostInsightsApi * implementation to render an InfoCard or other visualization. + * + * The alert type exposes hooks which can be used to enable and access various events, + * such as when a user dismisses or snoozes an alert. Default forms and buttons + * will be rendered if a hook is defined. + * + * Each default form can be overridden with a custom component. It must be implemented using + * React.forwardRef. See https://reactjs.org/docs/forwarding-refs + * + * Errors thrown within hooks will generate a snackbar error notification. */ + export type Alert = { title: string; subtitle: string; + element?: JSX.Element; + status?: AlertStatus; url?: string; buttonText?: string; // Default: View Instructions - element: JSX.Element; + SnoozeForm?: AlertForm; + AcceptForm?: AlertForm; + DismissForm?: AlertForm; + onSnoozed?(options: AlertOptions): Promise; + onAccepted?(options: AlertOptions): Promise; + onDismissed?(options: AlertOptions): Promise; }; +export type AlertForm< + A extends Alert = any, + Data = any +> = ForwardRefExoticComponent< + AlertFormProps & RefAttributes +>; + +export interface AlertOptions { + data: T; + group: string; +} + +/** + * Default snooze form intervals are expressed using an ISO 8601 repeating interval string. + * For example, R1/P7D/2020-09-02 for 1 week or R1/P30D/2020-09-02 for 1 month. + * + * For example, if a user dismisses an alert on Monday January 01 for 1 week, + * it can be re-served on Monday, January 08. 7 calendar days from January 02, + * inclusive of the last day. + * + * https://en.wikipedia.org/wiki/ISO_8601#Repeating_intervals + */ +export interface AlertSnoozeFormData { + intervals: string; +} + +export interface AlertDismissFormData { + other: Maybe; + reason: AlertDismissReason; + feedback: Maybe; +} + +// TODO: Convert enum to literal +export enum AlertStatus { + Snoozed = 'snoozed', + Accepted = 'accepted', + Dismissed = 'dismissed', +} + +export type AlertFormProps = { + alert: A; + onSubmit: (data: FormData) => void; + disableSubmit: (isDisabled: boolean) => void; +}; + +export interface AlertDismissOption { + label: string; + reason: string; +} + +export enum AlertDismissReason { + Other = 'other', + Resolved = 'resolved', + Expected = 'expected', + Seasonal = 'seasonal', + Migration = 'migration', + NotApplicable = 'not-applicable', +} + +export const AlertDismissOptions: AlertDismissOption[] = [ + { + reason: AlertDismissReason.Resolved, + label: 'This action item is now resolved.', + }, + { + reason: AlertDismissReason.Seasonal, + label: 'This is an expected increase at this time of year.', + }, + { + reason: AlertDismissReason.Migration, + label: 'This increase is from a migration in process.', + }, + { + reason: AlertDismissReason.Expected, + label: 'This is an expected increase due to our team’s priorities.', + }, + { + reason: AlertDismissReason.NotApplicable, + label: 'This action item doesn’t make sense for my team.', + }, + { + reason: AlertDismissReason.Other, + label: 'Other (please specify)', + }, +]; + +export type AlertSnoozeOption = { + label: string; + duration: Duration; +}; + +export const AlertSnoozeOptions: AlertSnoozeOption[] = [ + { + duration: Duration.P7D, + label: '1 Week', + }, + { + duration: Duration.P30D, + label: '1 Month', + }, + { + duration: Duration.P3M, + label: '1 Quarter', + }, +]; + export interface AlertCost { id: string; aggregation: [number, number]; diff --git a/plugins/cost-insights/src/types/Duration.ts b/plugins/cost-insights/src/types/Duration.ts index c0f03d5c27..e25f38e63d 100644 --- a/plugins/cost-insights/src/types/Duration.ts +++ b/plugins/cost-insights/src/types/Duration.ts @@ -21,6 +21,7 @@ * September 15. */ export enum Duration { + P7D = 'P7D', P30D = 'P30D', P90D = 'P90D', P3M = 'P3M', diff --git a/plugins/cost-insights/src/utils/alerts.tsx b/plugins/cost-insights/src/utils/alerts.tsx index fbb148f38a..b8dfddf79b 100644 --- a/plugins/cost-insights/src/utils/alerts.tsx +++ b/plugins/cost-insights/src/utils/alerts.tsx @@ -14,50 +14,22 @@ * limitations under the License. */ -import React from 'react'; -import { Alert, UnlabeledDataflowData, ProjectGrowthData } from '../types'; -import { UnlabeledDataflowAlertCard } from '../components/UnlabeledDataflowAlertCard'; -import { ProjectGrowthAlertCard } from '../components/ProjectGrowthAlertCard'; +import { Alert, AlertStatus } from '../types'; -/** - * The alerts below are examples of Alert implementation; the CostInsightsApi permits returning - * any implementation of the Alert type, so adopters can create their own. The CostInsightsApi - * fetches alert data from the backend, then creates Alert classes with the data. - */ +const createStatusHandler = (status?: string) => (alert: Alert) => + alert.status === status; +export const isActive = createStatusHandler(); +export const isSnoozed = createStatusHandler(AlertStatus.Snoozed); +export const isAccepted = createStatusHandler(AlertStatus.Accepted); +export const isDismissed = createStatusHandler(AlertStatus.Dismissed); -export class UnlabeledDataflowAlert implements Alert { - data: UnlabeledDataflowData; +export const sumOfAllAlerts = (sum: number, alerts: Alert[]) => + sum + alerts.length; - constructor(data: UnlabeledDataflowData) { - this.data = data; - } - - title = 'Add labels to workflows'; - subtitle = - 'Labels show in billing data, enabling cost insights for each workflow.'; - url = '/cost-insights/labeling-jobs'; - - get element() { - return ; - } -} - -export class ProjectGrowthAlert implements Alert { - data: ProjectGrowthData; - - constructor(data: ProjectGrowthData) { - this.data = data; - } - - get title() { - return `Investigate cost growth in project ${this.data.project}`; - } - - subtitle = - 'Cost growth outpacing business growth is unsustainable long-term.'; - url = '/cost-insights/investigating-growth'; - - get element() { - return ; - } +export function choose( + status: readonly [boolean, boolean, boolean], + values: [T, T, T], +): T | null { + const i = status.indexOf(true); + return i < 0 ? null : values[i]; } diff --git a/plugins/cost-insights/src/utils/duration.ts b/plugins/cost-insights/src/utils/duration.ts index 6eebead1d7..79ea150fc5 100644 --- a/plugins/cost-insights/src/utils/duration.ts +++ b/plugins/cost-insights/src/utils/duration.ts @@ -31,6 +31,7 @@ export function inclusiveStartDateOf( exclusiveEndDate: string, ): string { switch (duration) { + case Duration.P7D: case Duration.P30D: case Duration.P90D: return moment(exclusiveEndDate) @@ -53,6 +54,7 @@ export function exclusiveEndDateOf( inclusiveEndDate: string, ): string { switch (duration) { + case Duration.P7D: case Duration.P30D: case Duration.P90D: return moment(inclusiveEndDate) @@ -80,8 +82,15 @@ export function inclusiveEndDateOf( } // https://en.wikipedia.org/wiki/ISO_8601#Repeating_intervals -export function intervalsOf(duration: Duration, inclusiveEndDate: string) { - return `R2/${duration}/${exclusiveEndDateOf(duration, inclusiveEndDate)}`; +export function intervalsOf( + duration: Duration, + inclusiveEndDate: string, + repeating: number = 2, +) { + return `R${repeating}/${duration}/${exclusiveEndDateOf( + duration, + inclusiveEndDate, + )}`; } export function quarterEndDate(inclusiveEndDate: string): string { diff --git a/plugins/cost-insights/src/utils/loading.ts b/plugins/cost-insights/src/utils/loading.ts index 3328f23cfc..d15101075d 100644 --- a/plugins/cost-insights/src/utils/loading.ts +++ b/plugins/cost-insights/src/utils/loading.ts @@ -29,6 +29,7 @@ export enum DefaultLoadingAction { CostInsightsInitial = 'cost-insights-initial', CostInsightsPage = 'cost-insights-page', CostInsightsProducts = 'cost-insights-products', + CostInsightsAlerts = 'cost-insights-alerts', } export const INITIAL_LOADING_ACTIONS = [ diff --git a/plugins/cost-insights/src/utils/styles.ts b/plugins/cost-insights/src/utils/styles.ts index c04ceba8e3..acb900c490 100644 --- a/plugins/cost-insights/src/utils/styles.ts +++ b/plugins/cost-insights/src/utils/styles.ts @@ -479,8 +479,8 @@ export const useSelectStyles = makeStyles( }), ); -export const useAlertActionCardStyles = makeStyles( - (theme: BackstageTheme) => +export const useActionItemCardStyles = makeStyles( + (theme: CostInsightsTheme) => createStyles({ card: { boxShadow: 'none', @@ -489,15 +489,12 @@ export const useAlertActionCardStyles = makeStyles( backgroundColor: theme.palette.textVerySubtle, color: theme.palette.text.primary, }, - }), -); - -export const useAlertActionCardHeader = makeStyles( - (theme: CostInsightsTheme) => - createStyles({ root: { + minHeight: 80, paddingBottom: theme.spacing(2), borderRadius: theme.shape.borderRadius, + }, + activeRoot: { cursor: 'pointer', transition: theme.transitions.create('background', { duration: theme.transitions.duration.short, @@ -599,3 +596,31 @@ export const useEntityDialogStyles = makeStyles(theme => }, }), ); + +export const useAlertDialogStyles = makeStyles((theme: BackstageTheme) => + createStyles({ + content: { + padding: theme.spacing(0, 5, 2, 5), + }, + actions: { + padding: theme.spacing(2, 5), + }, + radio: { + margin: theme.spacing(-0.5, 0, -0.5, 0), + }, + icon: { + color: theme.palette.primary.dark, + margin: theme.spacing(2.5, 2.5, 0, 0), + padding: 0, + }, + }), +); + +export const useAlertStatusSummaryButtonStyles = makeStyles(() => ({ + icon: { + transform: 'transform 5s', + }, + clicked: { + transform: 'rotate(180deg)', + }, +})); diff --git a/plugins/cost-insights/src/utils/tests.tsx b/plugins/cost-insights/src/utils/tests.tsx index 852330acc3..13b38a49f2 100644 --- a/plugins/cost-insights/src/utils/tests.tsx +++ b/plugins/cost-insights/src/utils/tests.tsx @@ -22,6 +22,7 @@ import { IdentityApi, identityApiRef, } from '@backstage/core'; +import { AlertsContext, AlertsContextProps } from '../hooks/useAlerts'; import { LoadingContext, LoadingContextProps } from '../hooks/useLoading'; import { GroupsContext, GroupsContextProps } from '../hooks/useGroups'; import { FilterContext, FilterContextProps } from '../hooks/useFilters'; @@ -163,8 +164,9 @@ export type MockScrollProviderProps = PropsWithChildren<{}>; export const MockScrollProvider = ({ children }: MockScrollProviderProps) => { const defaultContext: ScrollContextProps = { - scrollTo: null, - setScrollTo: jest.fn(), + scroll: null, + setScroll: jest.fn(), + ScrollAnchor: jest.fn(() =>
), }; return ( @@ -231,3 +233,28 @@ export const MockCostInsightsApiProvider = ({ return {children}; }; + +export type MockAlertsProviderContextProps = PartialPropsWithChildren< + AlertsContextProps +>; + +export const MockAlertsProvider = ({ + children, + ...context +}: MockAlertsProviderContextProps) => { + const defaultContext: AlertsContextProps = { + alerts: { + alerts: [], + snoozed: null, + accepted: null, + dismissed: null, + }, + setAlerts: jest.fn(), + }; + + return ( + + {children} + + ); +}; From 19172f5a9b7e5b94b6c93bbe74bcc645fbdec789 Mon Sep 17 00:00:00 2001 From: Ryan Vazquez Date: Mon, 25 Jan 2021 18:04:31 -0500 Subject: [PATCH 02/10] changeset --- .changeset/cost-insights-tricky-moles-grin.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/cost-insights-tricky-moles-grin.md diff --git a/.changeset/cost-insights-tricky-moles-grin.md b/.changeset/cost-insights-tricky-moles-grin.md new file mode 100644 index 0000000000..c35769e03c --- /dev/null +++ b/.changeset/cost-insights-tricky-moles-grin.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-cost-insights': minor +--- + +add alert hooks From c81687ade11ae5f852a6482fa99d0135bfd93b37 Mon Sep 17 00:00:00 2001 From: Ryan Vazquez Date: Mon, 25 Jan 2021 18:56:10 -0500 Subject: [PATCH 03/10] cleanup --- .../src/alerts/KubernetesMigrationAlert.tsx | 35 ++++++------- plugins/cost-insights/src/client.ts | 11 ++-- .../components/ActionItems/ActionItems.tsx | 52 +++++++++---------- .../AlertInsights/AlertDialog.test.tsx | 20 +++---- .../src/forms/AlertDismissForm.tsx | 8 +-- 5 files changed, 59 insertions(+), 67 deletions(-) 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) => { From 09771bdf4633b2226dfc0bdd0c32f52223af7d13 Mon Sep 17 00:00:00 2001 From: Ryan Vazquez Date: Mon, 25 Jan 2021 20:44:41 -0500 Subject: [PATCH 04/10] fix import; don't make type public --- plugins/cost-insights/src/alerts/index.ts | 1 + .../src/components/MigrationAlertCard/MigrationAlertCard.tsx | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/cost-insights/src/alerts/index.ts b/plugins/cost-insights/src/alerts/index.ts index eb4a079e90..b55c012a60 100644 --- a/plugins/cost-insights/src/alerts/index.ts +++ b/plugins/cost-insights/src/alerts/index.ts @@ -18,3 +18,4 @@ export { ProjectGrowthAlert } from './ProjectGrowthAlert'; export { UnlabeledDataflowAlert } from './UnlabeledDataflowAlert'; export { KubernetesMigrationAlert } from './KubernetesMigrationAlert'; export type { MigrationAlert } from './KubernetesMigrationAlert'; +export type { MigrationData } from './KubernetesMigrationAlert'; diff --git a/plugins/cost-insights/src/components/MigrationAlertCard/MigrationAlertCard.tsx b/plugins/cost-insights/src/components/MigrationAlertCard/MigrationAlertCard.tsx index f4d174d521..56d636d8aa 100644 --- a/plugins/cost-insights/src/components/MigrationAlertCard/MigrationAlertCard.tsx +++ b/plugins/cost-insights/src/components/MigrationAlertCard/MigrationAlertCard.tsx @@ -19,7 +19,7 @@ import { Box } from '@material-ui/core'; import { InfoCard } from '@backstage/core'; import { MigrationBarChartLegend } from './MigrationBarChartLegend'; import { MigrationBarChart } from './MigrationBarChart'; -import { MigrationData } from '../../types'; +import { MigrationData } from '../../alerts'; type MigrationAlertProps = { data: MigrationData; From a79134c7f1f30b47e4ec8c2c082a607db7a82278 Mon Sep 17 00:00:00 2001 From: Ryan Vazquez Date: Mon, 25 Jan 2021 20:45:01 -0500 Subject: [PATCH 05/10] add duration --- plugins/cost-insights/src/utils/currency.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/cost-insights/src/utils/currency.ts b/plugins/cost-insights/src/utils/currency.ts index f1d67a14e4..8adbc4dcaa 100644 --- a/plugins/cost-insights/src/utils/currency.ts +++ b/plugins/cost-insights/src/utils/currency.ts @@ -20,6 +20,7 @@ export const rateOf = (cost: number, duration: Duration) => { switch (duration) { case Duration.P30D: return cost / 12; + case Duration.P7D: case Duration.P90D: case Duration.P3M: return cost / 4; From f19e4a5837fd99c0e832aff8fe5bbacd9fded7a0 Mon Sep 17 00:00:00 2001 From: Ryan Vazquez Date: Mon, 25 Jan 2021 20:59:48 -0500 Subject: [PATCH 06/10] Update plugins/cost-insights/src/alerts/KubernetesMigrationAlert.tsx Co-authored-by: Adam Harvey <33203301+adamdmharvey@users.noreply.github.com> --- plugins/cost-insights/src/alerts/KubernetesMigrationAlert.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/cost-insights/src/alerts/KubernetesMigrationAlert.tsx b/plugins/cost-insights/src/alerts/KubernetesMigrationAlert.tsx index d7cc5ac79d..7b775a4a96 100644 --- a/plugins/cost-insights/src/alerts/KubernetesMigrationAlert.tsx +++ b/plugins/cost-insights/src/alerts/KubernetesMigrationAlert.tsx @@ -52,7 +52,7 @@ export interface MigrationAlert extends Alert { * 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. * - * Default forms can be overriden by providing a valid React form component. Form components + * Default forms can be overridden 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. */ From 027ee1a7cfa9a6bf7b168a9b793eefe0dc240d27 Mon Sep 17 00:00:00 2001 From: Ryan Vazquez Date: Tue, 26 Jan 2021 17:03:14 -0500 Subject: [PATCH 07/10] add support for disabling alert dialog forms --- .../src/alerts/KubernetesMigrationAlert.tsx | 23 +- .../AlertInsights/AlertDialog.test.tsx | 297 +++++++++--------- .../components/AlertInsights/AlertDialog.tsx | 106 ++++--- plugins/cost-insights/src/types/Alert.ts | 6 +- plugins/cost-insights/src/utils/alerts.tsx | 4 +- 5 files changed, 239 insertions(+), 197 deletions(-) diff --git a/plugins/cost-insights/src/alerts/KubernetesMigrationAlert.tsx b/plugins/cost-insights/src/alerts/KubernetesMigrationAlert.tsx index 7b775a4a96..8c59d0c1a5 100644 --- a/plugins/cost-insights/src/alerts/KubernetesMigrationAlert.tsx +++ b/plugins/cost-insights/src/alerts/KubernetesMigrationAlert.tsx @@ -52,9 +52,15 @@ export interface MigrationAlert extends Alert { * 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. * - * Default forms can be overridden 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. + * Customizing Alerts + * Default forms can be overridden in two ways - by setting a form property to null or defining a custom component. + * + * If a form property is set to null, the Dialog will not render a form. This can be useful in scenarios + * where data isn't needed from the user such as when a user accepts an action item's recommendation. + * + * If a form property is set to a React component, the Dialog will render the form component in place of the default form. + * Form components must return valid form elements, and accept a ref and onSubmit event handler. + * Custom forms must implement the corresponding event hook. See /forms for example implementations. */ export class KubernetesMigrationAlert implements MigrationAlert { @@ -64,7 +70,10 @@ 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 a custom form component. + // Dialog will not render a form if form property set to null. + AcceptForm = null; + + // Overrides default Dismiss form with a custom form component. DismissForm: AlertForm< MigrationAlert, MigrationDismissFormData @@ -100,7 +109,7 @@ export class KubernetesMigrationAlert implements MigrationAlert { ); } - /* Fires when the onSubmit event is raised on a DismissAlert form. Displays a custom dismiss form. */ + /* Fires when the onSubmit event is raised on a Dismiss form. Displays custom dismiss form. */ async onDismissed( options: AlertOptions, ): Promise { @@ -117,7 +126,7 @@ export class KubernetesMigrationAlert implements MigrationAlert { ); } - /* Fires when the onSubmit event is raised on an SnoozeAlert form. Displays default snooze form. */ + /* Fires when the onSubmit event is raised on a Snooze form. Displays default snooze form. */ async onSnoozed( options: AlertOptions, ): Promise { @@ -134,7 +143,7 @@ export class KubernetesMigrationAlert implements MigrationAlert { ); } - /* Fires when the onSubmit event is raised on an AcceptAlert form. Displays default accept form. */ + /* Fires when the Accept button is clicked. Dialog does not render a form. See KubernetesMigrationAlert.AcceptForm */ async onAccepted(options: AlertOptions): Promise { const alerts = await this.api.getAlerts(options.group); return new Promise(resolve => diff --git a/plugins/cost-insights/src/components/AlertInsights/AlertDialog.test.tsx b/plugins/cost-insights/src/components/AlertInsights/AlertDialog.test.tsx index 2e4cb53681..72ff757cee 100644 --- a/plugins/cost-insights/src/components/AlertInsights/AlertDialog.test.tsx +++ b/plugins/cost-insights/src/components/AlertInsights/AlertDialog.test.tsx @@ -16,154 +16,169 @@ import React from 'react'; import { AlertDialog } from './AlertDialog'; import { render } from '@testing-library/react'; -import { - Alert, - AlertFormProps, - AlertSnoozeOptions, - AlertDismissOptions, -} from '../../types'; +import { Alert, AlertFormProps } from '../../types'; type MockFormDataProps = AlertFormProps; -const MockForm = React.forwardRef( - (props, ref) => ( +function createForm(title: string) { + return React.forwardRef((props, ref) => (
- You. Complete. Me. + You. {title}. Me.
- ), -); + )); +} +const snoozableAlert: Alert = { + title: 'title', + subtitle: 'test-subtitle', + onSnoozed: jest.fn(), +}; + +const dimissableAlert: Alert = { + title: 'title', + subtitle: 'subtitle', + onDismissed: jest.fn(), +}; + +const acceptAlert: Alert = { + title: 'title', + subtitle: 'subtitle', + onAccepted: jest.fn(), +}; + +const customSnoozeAlert: Alert = { + title: 'title', + subtitle: 'subtitle', + onSnoozed: jest.fn(), + SnoozeForm: createForm('Snooze'), +}; + +const customDismissAlert: Alert = { + title: 'title', + subtitle: 'subtitle', + onDismissed: jest.fn(), + DismissForm: createForm('Dismiss'), +}; + +const customAcceptAlert: Alert = { + title: 'title', + subtitle: 'test-subtitle', + onAccepted: jest.fn(), + AcceptForm: createForm('Accept'), +}; + +const nullAcceptAlert: Alert = { + title: 'title', + subtitle: 'test-subtitle', + onAccepted: jest.fn(), + AcceptForm: null, +}; + +const nullDismissAlert: Alert = { + title: 'title', + subtitle: 'test-subtitle', + onDismissed: jest.fn(), + DismissForm: null, +}; + +const nullSnoozeAlert: Alert = { + title: 'title', + subtitle: 'test-subtitle', + onSnoozed: jest.fn(), + SnoozeForm: null, +}; describe('', () => { - const snoozableAlert: Alert = { - title: 'title', - subtitle: 'test-subtitle', - onSnoozed: jest.fn(), - }; + describe.each` + accepted | dismissed | snoozed | action | text + ${acceptAlert} | ${null} | ${null} | ${['Accept', 'accepted']} | ${'My team can commit to making this change soon, or has already.'} + ${null} | ${dimissableAlert} | ${null} | ${['Dismiss', 'dismissed']} | ${'Reason for dismissing?'} + ${null} | ${null} | ${snoozableAlert} | ${['Snooze', 'snoozed']} | ${'For how long?'} + `( + 'Default forms', + ({ accepted, dismissed, snoozed, action: [action, actioned], text }) => { + it(`Displays a default ${action} form`, () => { + const { getByText } = render( + , + ); + expect(getByText(text)).toBeInTheDocument(); + expect(getByText(`${action} this action item?`)).toBeInTheDocument(); + expect( + getByText(`This action item will be ${actioned} for all of Ramones.`), + ).toBeInTheDocument(); + }); + }, + ); - const dimissableAlert: Alert = { - title: 'title', - subtitle: 'subtitle', - onDismissed: jest.fn(), - }; + describe.each` + accepted | dismissed | snoozed | action + ${customAcceptAlert} | ${null} | ${null} | ${['Accept', 'accepted']} + ${null} | ${customDismissAlert} | ${null} | ${['Dismiss', 'dismissed']} + ${null} | ${null} | ${customSnoozeAlert} | ${['Snooze', 'snoozed']} + `( + 'Custom forms', + ({ accepted, dismissed, snoozed, action: [Action, actioned] }) => { + it(`Displays a custom ${Action} form`, () => { + const { getByText } = render( + , + ); + expect(getByText(`You. ${Action}. Me.`)).toBeInTheDocument(); + expect(getByText(`${Action} this action item?`)).toBeInTheDocument(); + expect( + getByText(`This action item will be ${actioned} for all of Ramones.`), + ).toBeInTheDocument(); + }); + }, + ); - const customSnoozeAlert: Alert = { - title: 'title', - subtitle: 'subtitle', - onSnoozed: jest.fn(), - SnoozeForm: MockForm, - }; - - const customDismissAlert: Alert = { - title: 'title', - subtitle: 'subtitle', - onDismissed: jest.fn(), - DismissForm: MockForm, - }; - - const customAcceptAlert: Alert = { - title: 'title', - subtitle: 'test-subtitle', - onAccepted: jest.fn(), - AcceptForm: MockForm, - }; - - it('Displays a default snooze form', () => { - const { getByText } = render( - , - ); - expect(getByText('For how long?')).toBeInTheDocument(); - expect(getByText('Snooze this action item?')).toBeInTheDocument(); - expect( - getByText('This action item will be snoozed for all of Ramones.'), - ).toBeInTheDocument(); - AlertSnoozeOptions.forEach(a => - expect(getByText(a.label)).toBeInTheDocument(), - ); - }); - - it('Displays a custom snooze form', () => { - const { getByText } = render( - , - ); - expect(getByText('You. Complete. Me.')).toBeInTheDocument(); - expect(getByText('Snooze this action item?')).toBeInTheDocument(); - expect( - getByText('This action item will be snoozed for all of Ramones.'), - ).toBeInTheDocument(); - }); - - it('Displays a default dismiss form', () => { - const { getByText } = render( - , - ); - expect(getByText('Dismiss this action item?')).toBeInTheDocument(); - expect( - getByText('This action item will be dismissed for all of Ramones.'), - ).toBeInTheDocument(); - AlertDismissOptions.forEach(a => - expect(getByText(a.label)).toBeInTheDocument(), - ); - }); - - it('Displays a custom dismiss form', () => { - const { getByText } = render( - , - ); - expect(getByText('Dismiss this action item?')).toBeInTheDocument(); - expect(getByText('You. Complete. Me.')).toBeInTheDocument(); - expect( - getByText('This action item will be dismissed for all of Ramones.'), - ).toBeInTheDocument(); - }); - - it('Displays a custom accept form', () => { - const { getByText } = render( - , - ); - expect(getByText('Accept this action item?')).toBeInTheDocument(); - expect(getByText('You. Complete. Me.')).toBeInTheDocument(); - expect( - getByText('This action item will be accepted for all of Ramones.'), - ).toBeInTheDocument(); - }); + describe.each` + accepted | dismissed | snoozed | action | text + ${nullAcceptAlert} | ${null} | ${null} | ${['Accept', 'accept', 'accepted']} | ${'My team can commit to making this change soon, or has already.'} + ${null} | ${nullDismissAlert} | ${null} | ${['Dismiss', 'dismiss', 'dismissed']} | ${'Reason for dismissing?'} + ${null} | ${null} | ${nullSnoozeAlert} | ${['Snooze', 'snooze', 'snoozed']} | ${'For how long?'} + `( + 'Null forms', + ({ + accepted, + dismissed, + snoozed, + action: [Action, action, actioned], + text, + }) => { + it(`Does NOT display a ${Action} form`, () => { + const { getByText, getByRole, queryByText } = render( + , + ); + expect(queryByText(text)).not.toBeInTheDocument(); + expect(getByRole('button', { name: action })).toBeInTheDocument(); + expect(getByText(`${Action} this action item?`)).toBeInTheDocument(); + expect( + getByText(`This action item will be ${actioned} for all of Ramones.`), + ).toBeInTheDocument(); + }); + }, + ); }); diff --git a/plugins/cost-insights/src/components/AlertInsights/AlertDialog.tsx b/plugins/cost-insights/src/components/AlertInsights/AlertDialog.tsx index aa5b8c59fb..f7154602b9 100644 --- a/plugins/cost-insights/src/components/AlertInsights/AlertDialog.tsx +++ b/plugins/cost-insights/src/components/AlertInsights/AlertDialog.tsx @@ -33,7 +33,7 @@ import { } from '../../forms'; import { useAlertDialogStyles as useStyles } from '../../utils/styles'; import { choose } from '../../utils/alerts'; -import { Alert, Maybe } from '../../types'; +import { Alert, AlertForm, Maybe } from '../../types'; const DEFAULT_FORM_ID = 'alert-form'; @@ -79,20 +79,41 @@ export const AlertDialog = ({ setDisabled(true); } - const SnoozeForm = snoozed?.SnoozeForm ?? AlertSnoozeForm; - const AcceptForm = accepted?.AcceptForm ?? AlertAcceptForm; - const DismissForm = dismissed?.DismissForm ?? AlertDismissForm; + const SnoozeForm: Maybe = snoozed?.SnoozeForm ?? AlertSnoozeForm; + const AcceptForm: Maybe = accepted?.AcceptForm ?? AlertAcceptForm; + const DismissForm: Maybe = + dismissed?.DismissForm ?? AlertDismissForm; - const isSnoozeFormDisplayed = !!snoozed?.onSnoozed; - const isAcceptFormDisplayed = !!accepted?.onAccepted; - const isDismissFormDisplayed = !!dismissed?.onDismissed; + const isSnoozingEnabled = !!snoozed?.onSnoozed; + const isAcceptingEnabled = !!accepted?.onAccepted; + const isDismissingEnabled = !!dismissed?.onDismissed; + + const isSnoozeFormDisabled = snoozed?.SnoozeForm === null; + const isAcceptFormDisabled = accepted?.AcceptForm === null; + const isDismissFormDisabled = dismissed?.DismissForm === null; + const isFormDisabled = + isSnoozeFormDisabled || isAcceptFormDisabled || isDismissFormDisabled; const status = [ - isAcceptFormDisplayed, - isSnoozeFormDisplayed, - isDismissFormDisplayed, + isSnoozingEnabled, + isAcceptingEnabled, + isDismissingEnabled, ] as const; + const [Action, action, actioned] = + choose(status, [ + ['Snooze', 'snooze', 'snoozed'], + ['Accept', 'accept', 'accepted'], + ['Dismiss', 'dismiss', 'dismissed'], + ]) ?? []; + + const [title, subtitle] = + choose(status, [ + [snoozed?.title, snoozed?.subtitle], + [accepted?.title, accepted?.subtitle], + [dismissed?.title, dismissed?.subtitle], + ]) ?? []; + const TransitionProps = { mountOnEnter: true, unmountOnExit: true, @@ -122,24 +143,20 @@ export const AlertDialog = ({ - + - - {choose(status, ['Accept', 'Snooze', 'Dismiss'])} this action - item? - + {Action} this action item? - This action item will be{' '} - {choose(status, ['accepted', 'snoozed', 'dismissed'])} for all of{' '} - {group}. + This action item will be {actioned} for all of {group}. @@ -152,23 +169,11 @@ export const AlertDialog = ({ borderRadius={4} > - - {choose(status, [ - accepted?.title, - snoozed?.title, - dismissed?.title, - ])} - - - - {choose(status, [ - accepted?.subtitle, - snoozed?.subtitle, - dismissed?.subtitle, - ])} + {title} + {subtitle} - {isSnoozeFormDisplayed && ( + {isSnoozingEnabled && !isSnoozeFormDisabled && ( )} - {isDismissFormDisplayed && ( + {isDismissingEnabled && !isDismissFormDisabled && ( )} - {isAcceptFormDisplayed && ( + {isAcceptingEnabled && !isAcceptFormDisabled && ( - + {isFormDisabled ? ( + + ) : ( + + )} ); diff --git a/plugins/cost-insights/src/types/Alert.ts b/plugins/cost-insights/src/types/Alert.ts index 21e1a779a8..eb8919b71a 100644 --- a/plugins/cost-insights/src/types/Alert.ts +++ b/plugins/cost-insights/src/types/Alert.ts @@ -40,9 +40,9 @@ export type Alert = { status?: AlertStatus; url?: string; buttonText?: string; // Default: View Instructions - SnoozeForm?: AlertForm; - AcceptForm?: AlertForm; - DismissForm?: AlertForm; + SnoozeForm?: Maybe; + AcceptForm?: Maybe; + DismissForm?: Maybe; onSnoozed?(options: AlertOptions): Promise; onAccepted?(options: AlertOptions): Promise; onDismissed?(options: AlertOptions): Promise; diff --git a/plugins/cost-insights/src/utils/alerts.tsx b/plugins/cost-insights/src/utils/alerts.tsx index b8dfddf79b..e10740fbfe 100644 --- a/plugins/cost-insights/src/utils/alerts.tsx +++ b/plugins/cost-insights/src/utils/alerts.tsx @@ -29,7 +29,7 @@ export const sumOfAllAlerts = (sum: number, alerts: Alert[]) => export function choose( status: readonly [boolean, boolean, boolean], values: [T, T, T], -): T | null { +): T | undefined { const i = status.indexOf(true); - return i < 0 ? null : values[i]; + return values[i]; } From d275c3eb6e9ce671cb49ec8da7c97d6b0a1b9c9d Mon Sep 17 00:00:00 2001 From: Ryan Vazquez Date: Tue, 26 Jan 2021 19:05:35 -0500 Subject: [PATCH 08/10] Update ActionItems.test.tsx --- .../src/components/ActionItems/ActionItems.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/cost-insights/src/components/ActionItems/ActionItems.test.tsx b/plugins/cost-insights/src/components/ActionItems/ActionItems.test.tsx index 8c96e47f1a..677ba99c6a 100644 --- a/plugins/cost-insights/src/components/ActionItems/ActionItems.test.tsx +++ b/plugins/cost-insights/src/components/ActionItems/ActionItems.test.tsx @@ -24,7 +24,7 @@ function renderInContext(children: JSX.Element) { } describe('', () => { - it('should not display status buttons if there no active alerts', () => { + it('should not display status buttons if there are no active alerts', () => { const { queryByRole } = renderInContext( , ); From 8180fb76d8ea9b294bdfe595d45a4bedc6a23397 Mon Sep 17 00:00:00 2001 From: Ryan Vazquez Date: Wed, 27 Jan 2021 15:32:29 -0500 Subject: [PATCH 09/10] memoize alerts, not context --- .../CostInsightsPage/CostInsightsPage.tsx | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/plugins/cost-insights/src/components/CostInsightsPage/CostInsightsPage.tsx b/plugins/cost-insights/src/components/CostInsightsPage/CostInsightsPage.tsx index e7377eca10..f974fa0ddd 100644 --- a/plugins/cost-insights/src/components/CostInsightsPage/CostInsightsPage.tsx +++ b/plugins/cost-insights/src/components/CostInsightsPage/CostInsightsPage.tsx @@ -78,10 +78,18 @@ export const CostInsightsPage = () => { const { pageFilters, setPageFilters } = useFilters(p => p); - const snoozed = useMemo(() => alerts.alerts.filter(isSnoozed), [alerts]); - const accepted = useMemo(() => alerts.alerts.filter(isAccepted), [alerts]); - const dismissed = useMemo(() => alerts.alerts.filter(isDismissed), [alerts]); - const activeAlerts = useMemo(() => alerts.alerts.filter(isActive), [alerts]); + const snoozed = useMemo(() => alerts.alerts.filter(isSnoozed), [ + alerts.alerts, + ]); + const accepted = useMemo(() => alerts.alerts.filter(isAccepted), [ + alerts.alerts, + ]); + const dismissed = useMemo(() => alerts.alerts.filter(isDismissed), [ + alerts.alerts, + ]); + const activeAlerts = useMemo(() => alerts.alerts.filter(isActive), [ + alerts.alerts, + ]); const isActionItemsDisplayed = !!activeAlerts.length; const isAlertInsightsDisplayed = !!alerts.alerts.length; From 70906eed4cda0582aea7c4493494333c3afa841d Mon Sep 17 00:00:00 2001 From: Ryan Vazquez Date: Thu, 28 Jan 2021 18:55:36 -0500 Subject: [PATCH 10/10] refactor --- .../src/alerts/KubernetesMigrationAlert.tsx | 9 +- plugins/cost-insights/src/client.ts | 4 +- .../AlertInsights/AlertDialog.test.tsx | 170 +++++++++--------- .../components/AlertInsights/AlertDialog.tsx | 152 +++++----------- .../AlertInsights/AlertInsights.test.tsx | 12 +- .../AlertInsights/AlertInsights.tsx | 119 ++++++------ .../AlertInsights/AlertInsightsHeader.tsx | 3 +- .../AlertInsightsSection.test.tsx | 97 +++++----- .../AlertInsights/AlertInsightsSection.tsx | 26 ++- .../AlertInsightsSectionHeader.tsx | 3 +- .../AlertInsights/AlertStatusSummary.tsx | 130 +++++++------- .../CostInsightsPage/CostInsightsPage.tsx | 45 ++--- .../CostInsightsPage/CostInsightsPageRoot.tsx | 5 +- .../CostOverviewCard/CostOverviewCard.tsx | 4 +- .../ProductInsightsCard.tsx | 3 +- .../src/forms/AlertSnoozeForm.tsx | 26 +-- plugins/cost-insights/src/hooks/index.ts | 1 - plugins/cost-insights/src/hooks/useAlerts.tsx | 76 -------- plugins/cost-insights/src/hooks/useScroll.tsx | 56 +----- .../cost-insights/src/utils/alerts.test.tsx | 109 +++++++++++ plugins/cost-insights/src/utils/alerts.tsx | 124 +++++++++++-- plugins/cost-insights/src/utils/scroll.tsx | 65 +++++++ plugins/cost-insights/src/utils/tests.tsx | 27 --- 23 files changed, 646 insertions(+), 620 deletions(-) delete mode 100644 plugins/cost-insights/src/hooks/useAlerts.tsx create mode 100644 plugins/cost-insights/src/utils/alerts.test.tsx create mode 100644 plugins/cost-insights/src/utils/scroll.tsx diff --git a/plugins/cost-insights/src/alerts/KubernetesMigrationAlert.tsx b/plugins/cost-insights/src/alerts/KubernetesMigrationAlert.tsx index 8c59d0c1a5..31a4d7e0e9 100644 --- a/plugins/cost-insights/src/alerts/KubernetesMigrationAlert.tsx +++ b/plugins/cost-insights/src/alerts/KubernetesMigrationAlert.tsx @@ -72,7 +72,6 @@ export class KubernetesMigrationAlert implements MigrationAlert { // Dialog will not render a form if form property set to null. AcceptForm = null; - // Overrides default Dismiss form with a custom form component. DismissForm: AlertForm< MigrationAlert, @@ -94,7 +93,7 @@ export class KubernetesMigrationAlert implements MigrationAlert { get element() { const subheader = `${pluralize( - 'Compute Engine role', + 'Service', this.data.services.length, true, )}, sorted by cost`; @@ -116,7 +115,7 @@ export class KubernetesMigrationAlert implements MigrationAlert { const alerts = await this.api.getAlerts(options.group); return new Promise(resolve => setTimeout(resolve, 750, [ - ...alerts.slice(0, 2), + ...alerts.filter(a => a.title !== this.title), { title: this.title, subtitle: this.subtitle, @@ -133,7 +132,7 @@ export class KubernetesMigrationAlert implements MigrationAlert { const alerts = await this.api.getAlerts(options.group); return new Promise(resolve => setTimeout(resolve, 750, [ - ...alerts.slice(0, 2), + ...alerts.filter(a => a.title !== this.title), { title: this.title, subtitle: this.subtitle, @@ -148,7 +147,7 @@ export class KubernetesMigrationAlert implements MigrationAlert { const alerts = await this.api.getAlerts(options.group); return new Promise(resolve => setTimeout(resolve, 750, [ - ...alerts.slice(0, 2), + ...alerts.filter(a => a.title !== this.title), { title: this.title, subtitle: this.subtitle, diff --git a/plugins/cost-insights/src/client.ts b/plugins/cost-insights/src/client.ts index 7849214c03..2e70170fc1 100644 --- a/plugins/cost-insights/src/client.ts +++ b/plugins/cost-insights/src/client.ts @@ -183,8 +183,8 @@ export class ExampleCostInsightsClient implements CostInsightsApi { new ProjectGrowthAlert(projectGrowthData), new UnlabeledDataflowAlert(unlabeledDataflowData), new KubernetesMigrationAlert(this, { - startDate: today.format(DEFAULT_DATE_FORMAT), - endDate: today.add(30, 'day').format(DEFAULT_DATE_FORMAT), + startDate: today.subtract(30, 'day').format(DEFAULT_DATE_FORMAT), + endDate: today.format(DEFAULT_DATE_FORMAT), change: { ratio: 0, amount: 0, diff --git a/plugins/cost-insights/src/components/AlertInsights/AlertDialog.test.tsx b/plugins/cost-insights/src/components/AlertInsights/AlertDialog.test.tsx index 72ff757cee..7061f0736b 100644 --- a/plugins/cost-insights/src/components/AlertInsights/AlertDialog.test.tsx +++ b/plugins/cost-insights/src/components/AlertInsights/AlertDialog.test.tsx @@ -14,9 +14,10 @@ * limitations under the License. */ import React from 'react'; +import { capitalize } from '@material-ui/core'; import { AlertDialog } from './AlertDialog'; import { render } from '@testing-library/react'; -import { Alert, AlertFormProps } from '../../types'; +import { Alert, AlertFormProps, AlertStatus } from '../../types'; type MockFormDataProps = AlertFormProps; @@ -40,7 +41,7 @@ const dimissableAlert: Alert = { onDismissed: jest.fn(), }; -const acceptAlert: Alert = { +const acceptableAlert: Alert = { title: 'title', subtitle: 'subtitle', onAccepted: jest.fn(), @@ -87,98 +88,87 @@ const nullSnoozeAlert: Alert = { onSnoozed: jest.fn(), SnoozeForm: null, }; + describe('', () => { describe.each` - accepted | dismissed | snoozed | action | text - ${acceptAlert} | ${null} | ${null} | ${['Accept', 'accepted']} | ${'My team can commit to making this change soon, or has already.'} - ${null} | ${dimissableAlert} | ${null} | ${['Dismiss', 'dismissed']} | ${'Reason for dismissing?'} - ${null} | ${null} | ${snoozableAlert} | ${['Snooze', 'snoozed']} | ${'For how long?'} - `( - 'Default forms', - ({ accepted, dismissed, snoozed, action: [action, actioned], text }) => { - it(`Displays a default ${action} form`, () => { - const { getByText } = render( - , - ); - expect(getByText(text)).toBeInTheDocument(); - expect(getByText(`${action} this action item?`)).toBeInTheDocument(); - expect( - getByText(`This action item will be ${actioned} for all of Ramones.`), - ).toBeInTheDocument(); - }); - }, - ); + alert | status | action | text + ${acceptableAlert} | ${AlertStatus.Accepted} | ${['accept', 'accepted']} | ${'My team can commit to making this change soon, or has already.'} + ${dimissableAlert} | ${AlertStatus.Dismissed} | ${['dismiss', 'dismissed']} | ${'Reason for dismissing?'} + ${snoozableAlert} | ${AlertStatus.Snoozed} | ${['snooze', 'snoozed']} | ${'For how long?'} + `('Default forms', ({ alert, status, action: [action, actioned], text }) => { + it(`Displays a default ${action} form`, () => { + const { getByText } = render( + , + ); + expect(getByText(text)).toBeInTheDocument(); + expect( + getByText(`${capitalize(action)} this action item?`), + ).toBeInTheDocument(); + expect( + getByText(`This action item will be ${actioned} for all of Ramones.`), + ).toBeInTheDocument(); + }); + }); describe.each` - accepted | dismissed | snoozed | action - ${customAcceptAlert} | ${null} | ${null} | ${['Accept', 'accepted']} - ${null} | ${customDismissAlert} | ${null} | ${['Dismiss', 'dismissed']} - ${null} | ${null} | ${customSnoozeAlert} | ${['Snooze', 'snoozed']} - `( - 'Custom forms', - ({ accepted, dismissed, snoozed, action: [Action, actioned] }) => { - it(`Displays a custom ${Action} form`, () => { - const { getByText } = render( - , - ); - expect(getByText(`You. ${Action}. Me.`)).toBeInTheDocument(); - expect(getByText(`${Action} this action item?`)).toBeInTheDocument(); - expect( - getByText(`This action item will be ${actioned} for all of Ramones.`), - ).toBeInTheDocument(); - }); - }, - ); + alert | status | action | text + ${customAcceptAlert} | ${AlertStatus.Accepted} | ${['accept', 'accepted']} | ${'My team can commit to making this change soon, or has already.'} + ${customDismissAlert} | ${AlertStatus.Dismissed} | ${['dismiss', 'dismissed']} | ${'Reason for dismissing?'} + ${customSnoozeAlert} | ${AlertStatus.Snoozed} | ${['snooze', 'snoozed']} | ${'For how long?'} + `('Custom forms', ({ alert, status, action: [action, actioned] }) => { + it(`Displays a custom ${capitalize(action)} form`, () => { + const { getByText } = render( + , + ); + expect(getByText(`You. ${capitalize(action)}. Me.`)).toBeInTheDocument(); + expect( + getByText(`${capitalize(action)} this action item?`), + ).toBeInTheDocument(); + expect( + getByText(`This action item will be ${actioned} for all of Ramones.`), + ).toBeInTheDocument(); + }); + }); describe.each` - accepted | dismissed | snoozed | action | text - ${nullAcceptAlert} | ${null} | ${null} | ${['Accept', 'accept', 'accepted']} | ${'My team can commit to making this change soon, or has already.'} - ${null} | ${nullDismissAlert} | ${null} | ${['Dismiss', 'dismiss', 'dismissed']} | ${'Reason for dismissing?'} - ${null} | ${null} | ${nullSnoozeAlert} | ${['Snooze', 'snooze', 'snoozed']} | ${'For how long?'} - `( - 'Null forms', - ({ - accepted, - dismissed, - snoozed, - action: [Action, action, actioned], - text, - }) => { - it(`Does NOT display a ${Action} form`, () => { - const { getByText, getByRole, queryByText } = render( - , - ); - expect(queryByText(text)).not.toBeInTheDocument(); - expect(getByRole('button', { name: action })).toBeInTheDocument(); - expect(getByText(`${Action} this action item?`)).toBeInTheDocument(); - expect( - getByText(`This action item will be ${actioned} for all of Ramones.`), - ).toBeInTheDocument(); - }); - }, - ); + alert | status | action | text + ${nullAcceptAlert} | ${AlertStatus.Accepted} | ${['accept', 'accepted']} | ${'My team can commit to making this change soon, or has already.'} + ${nullDismissAlert} | ${AlertStatus.Dismissed} | ${['dismiss', 'dismissed']} | ${'Reason for dismissing?'} + ${nullSnoozeAlert} | ${AlertStatus.Snoozed} | ${['snooze', 'snoozed']} | ${'For how long?'} + `('Null forms', ({ alert, status, action: [action, actioned], text }) => { + it(`Does NOT display a ${capitalize(action)} form`, () => { + const { getByText, getByRole, queryByText } = render( + , + ); + expect(queryByText(text)).not.toBeInTheDocument(); + expect(getByRole('button', { name: action })).toBeInTheDocument(); + expect( + getByText(`${capitalize(action)} this action item?`), + ).toBeInTheDocument(); + expect( + getByText(`This action item will be ${actioned} for all of Ramones.`), + ).toBeInTheDocument(); + }); + }); }); diff --git a/plugins/cost-insights/src/components/AlertInsights/AlertDialog.tsx b/plugins/cost-insights/src/components/AlertInsights/AlertDialog.tsx index f7154602b9..ab6f9fb9f0 100644 --- a/plugins/cost-insights/src/components/AlertInsights/AlertDialog.tsx +++ b/plugins/cost-insights/src/components/AlertInsights/AlertDialog.tsx @@ -15,8 +15,8 @@ */ import React, { useEffect, useRef, useState } from 'react'; -import { default as CloseIcon } from '@material-ui/icons/Close'; import { + capitalize, Box, Button, Divider, @@ -26,23 +26,18 @@ import { DialogContent, Typography, } from '@material-ui/core'; -import { - AlertAcceptForm, - AlertDismissForm, - AlertSnoozeForm, -} from '../../forms'; +import { default as CloseIcon } from '@material-ui/icons/Close'; import { useAlertDialogStyles as useStyles } from '../../utils/styles'; -import { choose } from '../../utils/alerts'; -import { Alert, AlertForm, Maybe } from '../../types'; +import { Alert, AlertStatus, Maybe } from '../../types'; +import { choose, formOf } from '../../utils/alerts'; const DEFAULT_FORM_ID = 'alert-form'; type AlertDialogProps = { open: boolean; group: string; - snoozed: Maybe; - accepted: Maybe; - dismissed: Maybe; + alert: Maybe; + status: Maybe; onClose: () => void; onSubmit: (data: any) => void; }; @@ -50,87 +45,50 @@ type AlertDialogProps = { export const AlertDialog = ({ open, group, - snoozed, - accepted, - dismissed, + alert, + status, onClose, onSubmit, }: AlertDialogProps) => { const classes = useStyles(); - const [isButtonDisabled, setDisabled] = useState(true); - const acceptRef = useRef>(null); - const snoozeRef = useRef>(null); - const dismissRef = useRef>(null); + const [isSubmitDisabled, setSubmitDisabled] = useState(true); + const formRef = useRef>(null); useEffect(() => { - if (open) { - setDisabled(true); - } else { - setDisabled(false); - } + setSubmitDisabled(open); }, [open]); function disableSubmit(isDisabled: boolean) { - setDisabled(isDisabled); + setSubmitDisabled(isDisabled); } function onDialogClose() { onClose(); - setDisabled(true); + setSubmitDisabled(true); } - const SnoozeForm: Maybe = snoozed?.SnoozeForm ?? AlertSnoozeForm; - const AcceptForm: Maybe = accepted?.AcceptForm ?? AlertAcceptForm; - const DismissForm: Maybe = - dismissed?.DismissForm ?? AlertDismissForm; - - const isSnoozingEnabled = !!snoozed?.onSnoozed; - const isAcceptingEnabled = !!accepted?.onAccepted; - const isDismissingEnabled = !!dismissed?.onDismissed; - - const isSnoozeFormDisabled = snoozed?.SnoozeForm === null; - const isAcceptFormDisabled = accepted?.AcceptForm === null; - const isDismissFormDisabled = dismissed?.DismissForm === null; - const isFormDisabled = - isSnoozeFormDisabled || isAcceptFormDisabled || isDismissFormDisabled; - - const status = [ - isSnoozingEnabled, - isAcceptingEnabled, - isDismissingEnabled, - ] as const; - - const [Action, action, actioned] = - choose(status, [ - ['Snooze', 'snooze', 'snoozed'], - ['Accept', 'accept', 'accepted'], - ['Dismiss', 'dismiss', 'dismissed'], - ]) ?? []; - - const [title, subtitle] = - choose(status, [ - [snoozed?.title, snoozed?.subtitle], - [accepted?.title, accepted?.subtitle], - [dismissed?.title, dismissed?.subtitle], - ]) ?? []; + const [action, actioned] = choose( + status, + [ + ['snooze', 'snoozed'], + ['accept', 'accepted'], + ['dismiss', 'dismissed'], + ], + ['', ''], + ); const TransitionProps = { mountOnEnter: true, unmountOnExit: true, - // Wait for child component to mount; avoid recycling refs. onEntered() { - if (acceptRef.current) { - acceptRef.current.id = DEFAULT_FORM_ID; - } - if (snoozeRef.current) { - snoozeRef.current.id = DEFAULT_FORM_ID; - } - if (dismissRef.current) { - dismissRef.current.id = DEFAULT_FORM_ID; + if (formRef.current) { + formRef.current.id = DEFAULT_FORM_ID; } }, }; + const Form = formOf(alert, status); + return ( @@ -152,7 +110,7 @@ export const AlertDialog = ({ - {Action} this action item? + {capitalize(action)} this action item? @@ -169,30 +127,14 @@ export const AlertDialog = ({ borderRadius={4} > - {title} + {alert?.title} - {subtitle} + {alert?.subtitle} - {isSnoozingEnabled && !isSnoozeFormDisabled && ( - - )} - {isDismissingEnabled && !isDismissFormDisabled && ( - - )} - {isAcceptingEnabled && !isAcceptFormDisabled && ( - @@ -200,7 +142,18 @@ export const AlertDialog = ({ - {isFormDisabled ? ( + {Form ? ( + + ) : ( - ) : ( - )} diff --git a/plugins/cost-insights/src/components/AlertInsights/AlertInsights.test.tsx b/plugins/cost-insights/src/components/AlertInsights/AlertInsights.test.tsx index cd8444cb1f..98f75b1d41 100644 --- a/plugins/cost-insights/src/components/AlertInsights/AlertInsights.test.tsx +++ b/plugins/cost-insights/src/components/AlertInsights/AlertInsights.test.tsx @@ -17,18 +17,12 @@ import React from 'react'; import { render, fireEvent, waitFor } from '@testing-library/react'; import { AlertInsights } from './AlertInsights'; -import { - MockScrollProvider, - MockAlertsProvider, - MockLoadingProvider, -} from '../../utils/tests'; +import { MockScrollProvider, MockLoadingProvider } from '../../utils/tests'; function renderInContext(children: JSX.Element) { return render( - - {children} - + {children} , ); } @@ -47,6 +41,7 @@ describe('', () => { snoozed={[]} accepted={[]} dismissed={[]} + onChange={jest.fn()} />, ); expect( @@ -70,6 +65,7 @@ describe('', () => { ]} accepted={[]} dismissed={[]} + onChange={jest.fn()} />, ); diff --git a/plugins/cost-insights/src/components/AlertInsights/AlertInsights.tsx b/plugins/cost-insights/src/components/AlertInsights/AlertInsights.tsx index 1bcb684052..59b2dc9eec 100644 --- a/plugins/cost-insights/src/components/AlertInsights/AlertInsights.tsx +++ b/plugins/cost-insights/src/components/AlertInsights/AlertInsights.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { useCallback, useEffect, useState } from 'react'; +import React, { useEffect, useState } from 'react'; import pluralize from 'pluralize'; import { Box, Grid, Snackbar } from '@material-ui/core'; import { default as MuiAlert } from '@material-ui/lab/Alert'; @@ -24,15 +24,20 @@ import { AlertStatusSummaryButton } from './AlertStatusSummaryButton'; import { AlertInsightsHeader } from './AlertInsightsHeader'; import { AlertInsightsSection } from './AlertInsightsSection'; import { - useAlerts, useScroll, useLoading, ScrollType, MapLoadingToProps, } from '../../hooks'; import { DefaultLoadingAction } from '../../utils/loading'; -import { Alert, AlertOptions, Maybe } from '../../types'; -import { sumOfAllAlerts } from '../../utils/alerts'; +import { Alert, AlertOptions, AlertStatus, Maybe } from '../../types'; +import { + isStatusSnoozed, + isStatusAccepted, + isStatusDismissed, + sumOfAllAlerts, +} from '../../utils/alerts'; +import { ScrollAnchor } from '../../utils/scroll'; type MapLoadingtoAlerts = (isLoading: boolean) => void; @@ -47,6 +52,7 @@ type AlertInsightsProps = { snoozed: Alert[]; accepted: Alert[]; dismissed: Alert[]; + onChange: (alerts: Alert[]) => void; }; export const AlertInsights = ({ @@ -55,10 +61,12 @@ export const AlertInsights = ({ snoozed, accepted, dismissed, + onChange, }: AlertInsightsProps) => { - const [alerts, setAlerts] = useAlerts(); - const [scroll, , ScrollAnchor] = useScroll(); + const [scroll] = useScroll(); + const [alert, setAlert] = useState>(null); const dispatchLoadingAlerts = useLoading(mapLoadingToAlerts); + const [status, setStatus] = useState>(null); // Allow users to pass null values for data. const [data, setData] = useState>(undefined); const [error, setError] = useState>(null); @@ -66,22 +74,19 @@ export const AlertInsights = ({ const [isSummaryOpen, setSummaryOpen] = useState(false); const [isSnackbarOpen, setSnackbarOpen] = useState(false); - const closeDialog = useCallback(() => { - setData(undefined); - setDialogOpen(false); - setAlerts({ dismissed: null, snoozed: null, accepted: null }); - }, [setAlerts]); - useEffect(() => { - async function callHandler( + async function callAlertHook( options: AlertOptions, callback: (options: AlertOptions) => Promise, ) { - closeDialog(); + setAlert(null); + setStatus(null); + setData(undefined); + setDialogOpen(false); dispatchLoadingAlerts(true); try { - const a: Alert[] = await callback(options); - setAlerts({ alerts: a }); + const alerts: Alert[] = await callback(options); + onChange(alerts); } catch (e) { setError(e); } finally { @@ -90,22 +95,20 @@ export const AlertInsights = ({ } const options: AlertOptions = { data, group }; - const onSnoozed = alerts.snoozed?.onSnoozed?.bind(alerts.snoozed) ?? null; - const onAccepted = - alerts.accepted?.onAccepted?.bind(alerts.accepted) ?? null; - const onDismissed = - alerts.dismissed?.onDismissed?.bind(alerts.dismissed) ?? null; + const onSnoozed = alert?.onSnoozed?.bind(alert); + const onAccepted = alert?.onAccepted?.bind(alert); + const onDismissed = alert?.onDismissed?.bind(alert); if (data !== undefined) { - if (onSnoozed) { - callHandler(options, onSnoozed); - } else if (onAccepted) { - callHandler(options, onAccepted); - } else if (onDismissed) { - callHandler(options, onDismissed); + if (isStatusSnoozed(status) && onSnoozed) { + callAlertHook(options, onSnoozed); + } else if (isStatusAccepted(status) && onAccepted) { + callAlertHook(options, onAccepted); + } else if (isStatusDismissed(status) && onDismissed) { + callAlertHook(options, onDismissed); } } - }, [group, data, alerts, setAlerts, closeDialog, dispatchLoadingAlerts]); + }, [group, data, alert, status, onChange, dispatchLoadingAlerts]); useEffect(() => { if (scroll === ScrollType.AlertSummary) { @@ -114,34 +117,38 @@ export const AlertInsights = ({ }, [scroll]); useEffect(() => { - if (error) { - setSnackbarOpen(true); - } else { - setSnackbarOpen(false); - } - }, [error]); + setDialogOpen(!!status); + }, [status]); useEffect(() => { - function toggleDialogOnStatusChange() { - const isAlertSnoozed = !!alerts.snoozed; - const isAlertAccepted = !!alerts.accepted; - const isAlertDismissed = !!alerts.dismissed; + setSnackbarOpen(!!error); + }, [error]); - if (isAlertSnoozed || isAlertDismissed || isAlertAccepted) { - setDialogOpen(true); - } else { - setDialogOpen(false); - } - } + function onSnooze(alert: Alert) { + setAlert(alert); + setStatus(AlertStatus.Snoozed); + } - toggleDialogOnStatusChange(); - }, [alerts.snoozed, alerts.dismissed, alerts.accepted]); + function onAccept(alert: Alert) { + setAlert(alert); + setStatus(AlertStatus.Accepted); + } + + function onDismiss(alert: Alert) { + setAlert(alert); + setStatus(AlertStatus.Dismissed); + } function onSnackbarClose() { setError(null); } - function onDialogSubmit(data: any) { + function onDialogClose() { + setAlert(null); + setStatus(null); + } + + function onDialogFormSubmit(data: any) { setData(data); } @@ -153,7 +160,6 @@ export const AlertInsights = ({ const isAlertStatusSummaryDisplayed = !!total; const isAlertInsightSectionDisplayed = !!active.length; - // AlertInsights will not display if there aren't any active or hidden items. return ( @@ -171,7 +177,13 @@ export const AlertInsights = ({ {active.map((alert, index) => ( - + ))} @@ -195,11 +207,10 @@ export const AlertInsights = ({ { const classes = useStyles(); - const [, , ScrollAnchor] = useScroll(); return ( diff --git a/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSection.test.tsx b/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSection.test.tsx index f04e1d8b01..937e099356 100644 --- a/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSection.test.tsx +++ b/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSection.test.tsx @@ -17,8 +17,7 @@ import React from 'react'; import { AlertInsightsSection } from './AlertInsightsSection'; import { render } from '@testing-library/react'; import { Alert } from '../../types'; -import { AlertState } from '../../hooks'; -import { MockScrollProvider, MockAlertsProvider } from '../../utils/tests'; +import { MockScrollProvider } from '../../utils/tests'; const mockAlert: Alert = { subtitle: @@ -27,14 +26,20 @@ const mockAlert: Alert = { url: '/cost-insights/test', }; +function renderInContext(children: JSX.Element) { + return render({children}); +} + describe('', () => { it('Renders alert without exploding', () => { - const { getByText, queryByText } = render( - - - - - , + const { getByText, queryByText } = renderInContext( + , ); expect(getByText(mockAlert.title)).toBeInTheDocument(); expect(getByText(mockAlert.subtitle)).toBeInTheDocument(); @@ -49,12 +54,14 @@ describe('', () => { ...mockAlert, url: undefined, }; - const { queryByText } = render( - - - - - , + const { queryByText } = renderInContext( + , ); expect(queryByText('View Instructions')).not.toBeInTheDocument(); }); @@ -65,19 +72,14 @@ describe('', () => { onSnoozed: jest.fn(), }; - const context: AlertState = { - alerts: [], - snoozed: alert, - dismissed: null, - accepted: null, - }; - - const { queryByText, getByText } = render( - - - - - , + const { queryByText, getByText } = renderInContext( + , ); expect(getByText('Snooze')).toBeInTheDocument(); @@ -90,19 +92,15 @@ describe('', () => { ...mockAlert, onDismissed: jest.fn(), }; - const context: AlertState = { - alerts: [], - snoozed: null, - dismissed: alert, - accepted: null, - }; - const { queryByText, getByText } = render( - - - - - , + const { queryByText, getByText } = renderInContext( + , ); expect(getByText('Dismiss')).toBeInTheDocument(); @@ -116,19 +114,14 @@ describe('', () => { onAccepted: jest.fn(), }; - const context: AlertState = { - alerts: [], - snoozed: null, - dismissed: null, - accepted: alert, - }; - - const { queryByText, getByText } = render( - - - - - , + const { queryByText, getByText } = renderInContext( + , ); expect(getByText('Accept')).toBeInTheDocument(); diff --git a/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSection.tsx b/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSection.tsx index dda2d66f3f..d52687ae19 100644 --- a/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSection.tsx +++ b/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSection.tsx @@ -19,23 +19,31 @@ import { default as SnoozeIcon } from '@material-ui/icons/AccessTime'; import { default as AcceptIcon } from '@material-ui/icons/Check'; import { default as DismissIcon } from '@material-ui/icons/Delete'; import { AlertInsightsSectionHeader } from './AlertInsightsSectionHeader'; -import { useAlerts } from '../../hooks'; import { Alert } from '../../types'; +import { + isSnoozeEnabled, + isAcceptEnabled, + isDismissEnabled, +} from '../../utils/alerts'; type AlertInsightsSectionProps = { alert: Alert; number: number; + onSnooze: (alert: Alert) => void; + onAccept: (alert: Alert) => void; + onDismiss: (alert: Alert) => void; }; export const AlertInsightsSection = ({ alert, number, + onSnooze, + onAccept, + onDismiss, }: AlertInsightsSectionProps) => { - const [, setAlerts] = useAlerts(); - - const isSnoozeButtonDisplayed = !!alert.onSnoozed; - const isAcceptButtonDisplayed = !!alert.onAccepted; - const isDismissButtonDisplayed = !!alert.onDismissed; + const isSnoozeButtonDisplayed = isSnoozeEnabled(alert); + const isAcceptButtonDisplayed = isAcceptEnabled(alert); + const isDismissButtonDisplayed = isDismissEnabled(alert); const isButtonGroupDisplayed = isSnoozeButtonDisplayed || isAcceptButtonDisplayed || @@ -52,7 +60,7 @@ export const AlertInsightsSection = ({ color="primary" variant="contained" aria-label="accept" - onClick={() => setAlerts({ accepted: alert })} + onClick={() => onAccept(alert)} startIcon={} > Accept @@ -66,7 +74,7 @@ export const AlertInsightsSection = ({ variant="outlined" aria-label="snooze" disableElevation - onClick={() => setAlerts({ snoozed: alert })} + onClick={() => onSnooze(alert)} startIcon={} > Snooze @@ -79,7 +87,7 @@ export const AlertInsightsSection = ({ variant="outlined" aria-label="dismiss" disableElevation - onClick={() => setAlerts({ dismissed: alert })} + onClick={() => onDismiss(alert)} startIcon={} > Dismiss diff --git a/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSectionHeader.tsx b/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSectionHeader.tsx index c463a05609..30b8985934 100644 --- a/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSectionHeader.tsx +++ b/plugins/cost-insights/src/components/AlertInsights/AlertInsightsSectionHeader.tsx @@ -17,7 +17,7 @@ import React from 'react'; import { Avatar, Box, Button, Grid, Typography } from '@material-ui/core'; import { useAlertInsightsSectionStyles as useStyles } from '../../utils/styles'; -import { useScroll } from '../../hooks'; +import { ScrollAnchor } from '../../utils/scroll'; import { Alert } from '../../types'; type AlertInsightsSectionHeaderProps = { @@ -29,7 +29,6 @@ export const AlertInsightsSectionHeader = ({ alert, number, }: AlertInsightsSectionHeaderProps) => { - const [, , ScrollAnchor] = useScroll(); const classes = useStyles(); const isViewInstructionsButtonDisplayed = !!alert.url; diff --git a/plugins/cost-insights/src/components/AlertInsights/AlertStatusSummary.tsx b/plugins/cost-insights/src/components/AlertInsights/AlertStatusSummary.tsx index cb0a149511..caee99dc5e 100644 --- a/plugins/cost-insights/src/components/AlertInsights/AlertStatusSummary.tsx +++ b/plugins/cost-insights/src/components/AlertInsights/AlertStatusSummary.tsx @@ -15,7 +15,7 @@ */ import React, { Fragment } from 'react'; -import { Avatar, Box, Collapse, Divider } from '@material-ui/core'; +import { Avatar, Box, Collapse, Divider, Tooltip } from '@material-ui/core'; import { default as AcceptIcon } from '@material-ui/icons/Check'; import { default as DismissIcon } from '@material-ui/icons/Delete'; import { default as SnoozeIcon } from '@material-ui/icons/AccessTime'; @@ -23,6 +23,35 @@ import { ActionItemCard } from '../ActionItems'; import { Alert, AlertStatus } from '../../types'; import { useActionItemCardStyles as useStyles } from '../../utils/styles'; +type AlertGroupProps = { + alerts: Alert[]; + status: AlertStatus; + title: string; + icon: JSX.Element; +}; + +const AlertGroup = ({ alerts, status, title, icon }: AlertGroupProps) => { + const classes = useStyles(); + return ( + + {alerts.map((alert, index) => ( + + + {icon} + + } + /> + {index < alerts.length - 1 && } + + ))} + + ); +}; + type AlertStatusSummaryProps = { open: boolean; snoozed: Alert[]; @@ -36,8 +65,6 @@ export const AlertStatusSummary = ({ accepted, dismissed, }: AlertStatusSummaryProps) => { - const classes = useStyles(); - const isSnoozedListDisplayed = !!snoozed.length; const isAcceptedListDisplayed = !!accepted.length; const isDismissedListDisplayed = !!dismissed.length; @@ -45,71 +72,46 @@ export const AlertStatusSummary = ({ return ( {isAcceptedListDisplayed && ( - - {accepted.map((alert, index) => ( - - - {/* Icons indicate alert status. Do not hide from accesibility tree */} - - - } - /> - {index < accepted.length - 1 && } - - ))} - + + } + /> )} {isSnoozedListDisplayed && ( - - {snoozed.map((alert, index) => ( - - - - - } - /> - {index < snoozed.length - 1 && } - - ))} - + + } + /> )} {isDismissedListDisplayed && ( - - {dismissed.map((alert, index) => ( - - - - - } - /> - {index < dismissed.length - 1 && } - - ))} - + + } + /> )} ); diff --git a/plugins/cost-insights/src/components/CostInsightsPage/CostInsightsPage.tsx b/plugins/cost-insights/src/components/CostInsightsPage/CostInsightsPage.tsx index f974fa0ddd..a8ba454d11 100644 --- a/plugins/cost-insights/src/components/CostInsightsPage/CostInsightsPage.tsx +++ b/plugins/cost-insights/src/components/CostInsightsPage/CostInsightsPage.tsx @@ -44,22 +44,21 @@ import { ProductInsights } from '../ProductInsights'; import { useConfig, useCurrency, - useAlerts, useFilters, useGroups, useLastCompleteBillingDate, useLoading, } from '../../hooks'; -import { Cost, Maybe, MetricData, Product, Project } from '../../types'; +import { Alert, Cost, Maybe, MetricData, Product, Project } from '../../types'; import { mapLoadingToProps } from './selector'; import { ProjectSelect } from '../ProjectSelect'; import { intervalsOf } from '../../utils/duration'; import { useSubtleTypographyStyles } from '../../utils/styles'; import { - isActive, - isAccepted, - isDismissed, - isSnoozed, + isAlertActive, + isAlertAccepted, + isAlertDismissed, + isAlertSnoozed, } from '../../utils/alerts'; export const CostInsightsPage = () => { @@ -68,7 +67,7 @@ export const CostInsightsPage = () => { const config = useConfig(); const groups = useGroups(); const lastCompleteBillingDate = useLastCompleteBillingDate(); - const [alerts, setAlerts] = useAlerts(); + const [alerts, setAlerts] = useState([]); const [currency, setCurrency] = useCurrency(); const [projects, setProjects] = useState>(null); const [products, setProducts] = useState>(null); @@ -78,21 +77,13 @@ export const CostInsightsPage = () => { const { pageFilters, setPageFilters } = useFilters(p => p); - const snoozed = useMemo(() => alerts.alerts.filter(isSnoozed), [ - alerts.alerts, - ]); - const accepted = useMemo(() => alerts.alerts.filter(isAccepted), [ - alerts.alerts, - ]); - const dismissed = useMemo(() => alerts.alerts.filter(isDismissed), [ - alerts.alerts, - ]); - const activeAlerts = useMemo(() => alerts.alerts.filter(isActive), [ - alerts.alerts, - ]); + const active = useMemo(() => alerts.filter(isAlertActive), [alerts]); + const snoozed = useMemo(() => alerts.filter(isAlertSnoozed), [alerts]); + const accepted = useMemo(() => alerts.filter(isAlertAccepted), [alerts]); + const dismissed = useMemo(() => alerts.filter(isAlertDismissed), [alerts]); - const isActionItemsDisplayed = !!activeAlerts.length; - const isAlertInsightsDisplayed = !!alerts.alerts.length; + const isActionItemsDisplayed = !!active.length; + const isAlertInsightsDisplayed = !!alerts.length; const { loadingActions, @@ -150,7 +141,7 @@ export const CostInsightsPage = () => { : client.getGroupDailyCost(pageFilters.group, intervals), ]); setProjects(fetchedProjects); - setAlerts({ alerts: fetchedAlerts }); + setAlerts(fetchedAlerts); setMetricData(fetchedMetricData); setDailyCost(fetchedDailyCost); } else { @@ -175,7 +166,6 @@ export const CostInsightsPage = () => { loadingActions, loadingGroups, loadingBillingDate, - setAlerts, dispatchLoadingInsights, dispatchLoadingInitial, dispatchLoadingNone, @@ -259,7 +249,7 @@ export const CostInsightsPage = () => { @@ -280,14 +270,14 @@ export const CostInsightsPage = () => { owner={pageFilters.group} groups={groups} hasCostData={!!dailyCost.aggregation.length} - alerts={activeAlerts.length} + alerts={active.length} /> { diff --git a/plugins/cost-insights/src/components/CostInsightsPage/CostInsightsPageRoot.tsx b/plugins/cost-insights/src/components/CostInsightsPage/CostInsightsPageRoot.tsx index 1947ca185e..4f506b4076 100644 --- a/plugins/cost-insights/src/components/CostInsightsPage/CostInsightsPageRoot.tsx +++ b/plugins/cost-insights/src/components/CostInsightsPage/CostInsightsPageRoot.tsx @@ -20,7 +20,6 @@ import { FilterProvider } from '../../hooks/useFilters'; import { LoadingProvider } from '../../hooks/useLoading'; import { GroupsProvider } from '../../hooks/useGroups'; import { CurrencyProvider } from '../../hooks/useCurrency'; -import { AlertsProvider } from '../../hooks/useAlerts'; import { ScrollProvider } from '../../hooks/useScroll'; import { ConfigProvider } from '../../hooks/useConfig'; import { BillingDateProvider } from '../../hooks/useLastCompleteBillingDate'; @@ -35,9 +34,7 @@ export const CostInsightsPageRoot = () => ( - - - + diff --git a/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewCard.tsx b/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewCard.tsx index aedec33457..85a9a9db74 100644 --- a/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewCard.tsx +++ b/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewCard.tsx @@ -30,12 +30,13 @@ import { CostOverviewBreakdownChart } from './CostOverviewBreakdownChart'; import { CostOverviewHeader } from './CostOverviewHeader'; import { MetricSelect } from '../MetricSelect'; import { PeriodSelect } from '../PeriodSelect'; -import { useConfig, useFilters, useScroll } from '../../hooks'; +import { useConfig, useFilters } from '../../hooks'; import { mapFiltersToProps } from './selector'; import { DefaultNavigation } from '../../utils/navigation'; import { findAlways } from '../../utils/assert'; import { Cost, CostInsightsTheme, Maybe, MetricData } from '../../types'; import { useOverviewTabsStyles } from '../../utils/styles'; +import { ScrollAnchor } from '../../utils/scroll'; export type CostOverviewCardProps = { dailyCostData: Cost; @@ -49,7 +50,6 @@ export const CostOverviewCard = ({ const theme = useTheme(); const styles = useOverviewTabsStyles(theme); const config = useConfig(); - const [, , ScrollAnchor] = useScroll(); const [tabIndex, setTabIndex] = useState(0); const { setDuration, setProject, setMetric, ...filters } = useFilters( mapFiltersToProps, diff --git a/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsCard.tsx b/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsCard.tsx index 4626954bac..d46737cafb 100644 --- a/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsCard.tsx +++ b/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsCard.tsx @@ -34,9 +34,9 @@ import { MapLoadingToProps, useLastCompleteBillingDate, useLoading, - useScroll, } from '../../hooks'; import { findAnyKey } from '../../utils/assert'; +import { ScrollAnchor } from '../../utils/scroll'; type LoadingProps = (isLoading: boolean) => void; @@ -60,7 +60,6 @@ export const ProductInsightsCard = ({ }: PropsWithChildren) => { const classes = useStyles(); const mountedRef = useRef(false); - const [, , ScrollAnchor] = useScroll(); const [error, setError] = useState>(null); const dispatchLoading = useLoading(mapLoadingToProps); const lastCompleteBillingDate = useLastCompleteBillingDate(); diff --git a/plugins/cost-insights/src/forms/AlertSnoozeForm.tsx b/plugins/cost-insights/src/forms/AlertSnoozeForm.tsx index 51218fe844..743b5ddac1 100644 --- a/plugins/cost-insights/src/forms/AlertSnoozeForm.tsx +++ b/plugins/cost-insights/src/forms/AlertSnoozeForm.tsx @@ -24,7 +24,6 @@ import React, { import dayjs from 'dayjs'; import { Box, - Collapse, FormControl, FormControlLabel, RadioGroup, @@ -50,19 +49,18 @@ export const AlertSnoozeForm = forwardRef< AlertSnoozeFormProps >(({ onSubmit, disableSubmit }, ref) => { const classes = useStyles(); - const [error, setError] = useState>(null); const [duration, setDuration] = useState>(Duration.P7D); + useEffect(() => disableSubmit(false), [disableSubmit]); + const onFormSubmit: FormEventHandler = e => { e.preventDefault(); if (duration) { const repeatInterval = 1; - const inclusiveEndDate = dayjs().format(DEFAULT_DATE_FORMAT); + const today = dayjs().format(DEFAULT_DATE_FORMAT); onSubmit({ - intervals: intervalsOf(duration, inclusiveEndDate, repeatInterval), + intervals: intervalsOf(duration, today, repeatInterval), }); - } else { - setError(new Error('Please select an option.')); } }; @@ -73,26 +71,12 @@ export const AlertSnoozeForm = forwardRef< setDuration(value as Duration); }; - useEffect(() => { - function clearErrorOnFormDataChange() { - disableSubmit(false); - setError(prevError => (prevError ? null : prevError)); - } - - clearErrorOnFormDataChange(); - }, [duration, disableSubmit]); - - const isErrorMessageDisplayed = !!error; - return (
- + For how long? - - {error?.message} - >>; -}; - -export const AlertsContext = createContext( - undefined, -); - -export type AlertState = { - alerts: Alert[]; - snoozed: Maybe; - accepted: Maybe; - dismissed: Maybe; -}; - -const initialState: AlertState = { - alerts: [], - snoozed: null, - accepted: null, - dismissed: null, -}; - -const reducer = ( - prevState: AlertState, - action: SetStateAction>, -): AlertState => ({ - ...prevState, - ...action, -}); - -export const AlertsProvider = ({ children }: PropsWithChildren<{}>) => { - const [alerts, setAlerts] = useReducer(reducer, initialState); - - return ( - - {children} - - ); -}; - -export function useAlerts() { - const context = useContext(AlertsContext); - return context - ? ([context.alerts, context.setAlerts] as const) - : assertNever(); -} - -function assertNever(): never { - throw new Error('useAlerts cannot be used outside AlertsContext provider'); -} diff --git a/plugins/cost-insights/src/hooks/useScroll.tsx b/plugins/cost-insights/src/hooks/useScroll.tsx index 7f310d59b0..137762cc2d 100644 --- a/plugins/cost-insights/src/hooks/useScroll.tsx +++ b/plugins/cost-insights/src/hooks/useScroll.tsx @@ -15,12 +15,9 @@ */ import React, { Dispatch, - ElementType, SetStateAction, useState, useContext, - useEffect, - useRef, PropsWithChildren, } from 'react'; import { Maybe } from '../types'; @@ -30,61 +27,16 @@ export type ScrollTo = Maybe; export type ScrollContextProps = { scroll: ScrollTo; setScroll: Dispatch>; - ScrollAnchor: ElementType; }; -export interface ScrollAnchorProps extends ScrollIntoViewOptions { - id: ScrollTo; - top?: number; - left?: number; -} - export const ScrollContext = React.createContext< ScrollContextProps | undefined >(undefined); -export const ScrollAnchor = ({ - id, - block, - inline, - left = 0, - top = -20, - behavior = 'smooth', -}: ScrollAnchorProps) => { - const divRef = useRef(null); - const [scroll, setScroll] = useScroll(); - - useEffect(() => { - function scrollIntoView() { - const options = { - behavior: behavior || 'auto', - block: block || 'start', - inline: inline || 'nearest', - }; - - if (divRef.current && scroll === id) { - divRef.current.scrollIntoView(options); - setScroll(null); - } - } - - scrollIntoView(); - }, [scroll, setScroll, id, behavior, block, inline]); - - return ( -
- ); -}; - export const ScrollProvider = ({ children }: PropsWithChildren<{}>) => { const [scroll, setScroll] = useState(null); - return ( - + {children} ); @@ -101,11 +53,9 @@ export function useScroll() { assertNever(); } - return [context.scroll, context.setScroll, context.ScrollAnchor] as const; + return [context.scroll, context.setScroll] as const; } function assertNever(): never { - throw new Error( - `Cannot use useScroll or ScrollAnchor outside ScrollProvider`, - ); + throw new Error(`Cannot use useScroll outside ScrollProvider`); } diff --git a/plugins/cost-insights/src/utils/alerts.test.tsx b/plugins/cost-insights/src/utils/alerts.test.tsx new file mode 100644 index 0000000000..d821fd9f1f --- /dev/null +++ b/plugins/cost-insights/src/utils/alerts.test.tsx @@ -0,0 +1,109 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import React, { ReactNode } from 'react'; +import { formOf } from './alerts'; +import { AlertAcceptForm, AlertDismissForm, AlertSnoozeForm } from '../forms'; +import { Alert, AlertStatus, AlertFormProps } from '../types'; + +type Props = AlertFormProps; + +const createMockForm = (children: ReactNode) => + React.forwardRef((props, ref) => ( + + {children} + + )); + +const snoozeDefault: Alert = { + title: 'title', + subtitle: 'subtitle', + onSnoozed: jest.fn(), +}; + +const snoozeCustom: Alert = { + title: 'title', + subtitle: 'subtitle', + onSnoozed: jest.fn(), + SnoozeForm: createMockForm('Snooze'), +}; + +const snoozeNull: Alert = { + title: 'title', + subtitle: 'subtitle', + onSnoozed: jest.fn(), + SnoozeForm: null, +}; + +const acceptDefault: Alert = { + title: 'title', + subtitle: 'subtitle', + onAccepted: jest.fn(), +}; + +const acceptCustom: Alert = { + title: 'title', + subtitle: 'subtitle', + onAccepted: jest.fn(), + AcceptForm: createMockForm('Accept'), +}; + +const acceptNull: Alert = { + title: 'title', + subtitle: 'subtitle', + onAccepted: jest.fn(), + AcceptForm: null, +}; + +const dismissDefault: Alert = { + title: 'title', + subtitle: 'subtitle', + onDismissed: jest.fn(), +}; + +const dismissCustom: Alert = { + title: 'title', + subtitle: 'subtitle', + onDismissed: jest.fn(), + DismissForm: createMockForm('Dismiss'), +}; + +const dismissNull: Alert = { + title: 'title', + subtitle: 'subtitle', + onDismissed: jest.fn(), + DismissForm: null, +}; + +describe('formOf', () => { + describe.each` + msg | alert | status | expected + ${'default snooze form'} | ${snoozeDefault} | ${AlertStatus.Snoozed} | ${AlertSnoozeForm} + ${'custom snooze form'} | ${snoozeCustom} | ${AlertStatus.Snoozed} | ${snoozeCustom.SnoozeForm} + ${'null snooze form'} | ${snoozeNull} | ${AlertStatus.Snoozed} | ${null} + ${'default accept form'} | ${acceptDefault} | ${AlertStatus.Accepted} | ${AlertAcceptForm} + ${'custom accept form'} | ${acceptCustom} | ${AlertStatus.Accepted} | ${acceptCustom.AcceptForm} + ${'null accept form'} | ${acceptNull} | ${AlertStatus.Accepted} | ${null} + ${'default dismiss form'} | ${dismissDefault} | ${AlertStatus.Dismissed} | ${AlertDismissForm} + ${'custom dismiss form'} | ${dismissCustom} | ${AlertStatus.Dismissed} | ${dismissCustom.DismissForm} + ${'null dismiss form'} | ${dismissNull} | ${AlertStatus.Dismissed} | ${null} + ${'no form or status'} | ${null} | ${null} | ${null} + `('Should render the correct form', ({ msg, alert, status, expected }) => { + it(`for ${msg}`, () => { + const result = formOf(alert, status); + expect(result).toBe(expected); + }); + }); +}); diff --git a/plugins/cost-insights/src/utils/alerts.tsx b/plugins/cost-insights/src/utils/alerts.tsx index e10740fbfe..56403a664c 100644 --- a/plugins/cost-insights/src/utils/alerts.tsx +++ b/plugins/cost-insights/src/utils/alerts.tsx @@ -14,22 +14,118 @@ * limitations under the License. */ -import { Alert, AlertStatus } from '../types'; +import { Alert, AlertForm, AlertStatus, Maybe } from '../types'; +import { AlertAcceptForm, AlertDismissForm, AlertSnoozeForm } from '../forms'; -const createStatusHandler = (status?: string) => (alert: Alert) => +const createAlertHandler = (status?: AlertStatus) => (alert: Alert) => alert.status === status; -export const isActive = createStatusHandler(); -export const isSnoozed = createStatusHandler(AlertStatus.Snoozed); -export const isAccepted = createStatusHandler(AlertStatus.Accepted); -export const isDismissed = createStatusHandler(AlertStatus.Dismissed); +export const isAlertActive = (alert: Alert) => !hasProperty(alert, 'status'); +export const isAlertSnoozed = createAlertHandler(AlertStatus.Snoozed); +export const isAlertAccepted = createAlertHandler(AlertStatus.Accepted); +export const isAlertDismissed = createAlertHandler(AlertStatus.Dismissed); + +const createStatusHandler = (status: AlertStatus) => (s: Maybe) => + s === status; +export const isStatusSnoozed = createStatusHandler(AlertStatus.Snoozed); +export const isStatusAccepted = createStatusHandler(AlertStatus.Accepted); +export const isStatusDismissed = createStatusHandler(AlertStatus.Dismissed); + +const createAlertEventHandler = ( + onEvent: 'onSnoozed' | 'onAccepted' | 'onDismissed', +) => (alert: Maybe): boolean => hasProperty(alert, onEvent); +export const isSnoozeEnabled = createAlertEventHandler('onSnoozed'); +export const isAcceptEnabled = createAlertEventHandler('onAccepted'); +export const isDismissEnabled = createAlertEventHandler('onDismissed'); + +const createFormEnabledHandler = ( + Form: 'SnoozeForm' | 'AcceptForm' | 'DismissForm', +) => (alert: Maybe): boolean => { + if (!alert) return false; + if (alert[Form] === null) return false; + switch (Form) { + case 'SnoozeForm': + return isSnoozeEnabled(alert); + case 'AcceptForm': + return isAcceptEnabled(alert); + case 'DismissForm': + return isDismissEnabled(alert); + default: + return false; + } +}; +export const isSnoozeFormEnabled = createFormEnabledHandler('SnoozeForm'); +export const isAcceptFormEnabled = createFormEnabledHandler('AcceptForm'); +export const isDismissFormEnabled = createFormEnabledHandler('DismissForm'); + +/** + * Utility for determining if a form is disabled. + * When a form is disabled, the dialog button's type should convert from submit to button. + * @param alert + * @param status + */ +export const isFormDisabled = ( + alert: Maybe, + status: Maybe, +): boolean => { + switch (status) { + case AlertStatus.Snoozed: + return alert?.SnoozeForm === null; + case AlertStatus.Accepted: + return alert?.AcceptForm === null; + case AlertStatus.Dismissed: + return alert?.DismissForm === null; + default: + return false; + } +}; + +export function formOf( + alert: Maybe, + status: Maybe, +): Maybe { + switch (status) { + case AlertStatus.Snoozed: { + const SnoozeForm = alert?.SnoozeForm ?? AlertSnoozeForm; + return isSnoozeFormEnabled(alert) ? SnoozeForm : null; + } + case AlertStatus.Accepted: { + const AcceptForm = alert?.AcceptForm ?? AlertAcceptForm; + return isAcceptFormEnabled(alert) ? AcceptForm : null; + } + case AlertStatus.Dismissed: { + const DismissForm = alert?.DismissForm ?? AlertDismissForm; + return isDismissFormEnabled(alert) ? DismissForm : null; + } + default: + return null; + } +} + +/** + * Utility for choosing from a fixed set of values for a given alert status. + * @param status + * @param values + */ +export function choose( + status: Maybe, + values: [T, T, T], + none: T, +): T { + switch (status) { + case AlertStatus.Snoozed: + return values[0]; + case AlertStatus.Accepted: + return values[1]; + case AlertStatus.Dismissed: + return values[2]; + default: + return none; + } +} + +export function hasProperty(alert: Maybe, prop: keyof Alert): boolean { + return prop in (alert ?? {}); +} export const sumOfAllAlerts = (sum: number, alerts: Alert[]) => sum + alerts.length; - -export function choose( - status: readonly [boolean, boolean, boolean], - values: [T, T, T], -): T | undefined { - const i = status.indexOf(true); - return values[i]; -} diff --git a/plugins/cost-insights/src/utils/scroll.tsx b/plugins/cost-insights/src/utils/scroll.tsx new file mode 100644 index 0000000000..06f01775df --- /dev/null +++ b/plugins/cost-insights/src/utils/scroll.tsx @@ -0,0 +1,65 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import React, { useEffect, useRef } from 'react'; +import { ScrollTo, useScroll } from '../hooks/useScroll'; + +/* + Utility component use in conjuction with useScroll that allows scrollable components to control behavior and offset. + 1. ScrollAnchor must be a direct child of a scrollable component. + 2. ScrollAnchor's parent position must be relative. + 3. ScrollAnchor's id must be unique. +*/ + +export interface ScrollAnchorProps extends ScrollIntoViewOptions { + id: ScrollTo; + top?: number; + left?: number; +} + +export const ScrollAnchor = ({ + id, + left = 0, + top = -20, + block = 'start', + inline = 'nearest', + behavior = 'smooth', +}: ScrollAnchorProps) => { + const divRef = useRef(null); + const [scroll, setScroll] = useScroll(); + + useEffect(() => { + function scrollIntoView() { + if (divRef.current && scroll === id) { + divRef.current.scrollIntoView({ + block, + inline, + behavior, + }); + setScroll(null); + } + } + + scrollIntoView(); + }, [scroll, setScroll, id, behavior, block, inline]); + + return ( +
+ ); +}; diff --git a/plugins/cost-insights/src/utils/tests.tsx b/plugins/cost-insights/src/utils/tests.tsx index 13b38a49f2..aae2800573 100644 --- a/plugins/cost-insights/src/utils/tests.tsx +++ b/plugins/cost-insights/src/utils/tests.tsx @@ -22,7 +22,6 @@ import { IdentityApi, identityApiRef, } from '@backstage/core'; -import { AlertsContext, AlertsContextProps } from '../hooks/useAlerts'; import { LoadingContext, LoadingContextProps } from '../hooks/useLoading'; import { GroupsContext, GroupsContextProps } from '../hooks/useGroups'; import { FilterContext, FilterContextProps } from '../hooks/useFilters'; @@ -166,7 +165,6 @@ export const MockScrollProvider = ({ children }: MockScrollProviderProps) => { const defaultContext: ScrollContextProps = { scroll: null, setScroll: jest.fn(), - ScrollAnchor: jest.fn(() =>
), }; return ( @@ -233,28 +231,3 @@ export const MockCostInsightsApiProvider = ({ return {children}; }; - -export type MockAlertsProviderContextProps = PartialPropsWithChildren< - AlertsContextProps ->; - -export const MockAlertsProvider = ({ - children, - ...context -}: MockAlertsProviderContextProps) => { - const defaultContext: AlertsContextProps = { - alerts: { - alerts: [], - snoozed: null, - accepted: null, - dismissed: null, - }, - setAlerts: jest.fn(), - }; - - return ( - - {children} - - ); -};