From 37713b2392c5ad5f5f0161130b52bfa7cf086469 Mon Sep 17 00:00:00 2001 From: Shruthilaya Date: Tue, 28 Sep 2021 13:07:57 -0400 Subject: [PATCH 1/2] feat(dashboards): Add max widget limit Add a limit to the number of widgets that can be added to a dashboard to limit the number of parallel queries. --- .../modals/addDashboardWidgetModal.tsx | 47 +++++++++++++++---- static/app/views/dashboardsV2/dashboard.tsx | 4 +- static/app/views/dashboardsV2/types.tsx | 2 + .../modals/addDashboardWidgetModal.spec.jsx | 2 +- 4 files changed, 43 insertions(+), 12 deletions(-) diff --git a/static/app/components/modals/addDashboardWidgetModal.tsx b/static/app/components/modals/addDashboardWidgetModal.tsx index ca6715795855de..8f65533355c522 100644 --- a/static/app/components/modals/addDashboardWidgetModal.tsx +++ b/static/app/components/modals/addDashboardWidgetModal.tsx @@ -1,5 +1,6 @@ import * as React from 'react'; import {browserHistory} from 'react-router'; +import {components, OptionProps} from 'react-select'; import {css} from '@emotion/react'; import styled from '@emotion/styled'; import cloneDeep from 'lodash/cloneDeep'; @@ -15,7 +16,7 @@ import ButtonBar from 'app/components/buttonBar'; import WidgetQueriesForm from 'app/components/dashboards/widgetQueriesForm'; import SelectControl from 'app/components/forms/selectControl'; import {PanelAlert} from 'app/components/panels'; -import {t} from 'app/locale'; +import {t, tct} from 'app/locale'; import space from 'app/styles/space'; import { DateString, @@ -35,6 +36,7 @@ import { DashboardDetails, DashboardListItem, DisplayType, + MAX_WIDGETS, Widget, WidgetQuery, } from 'app/views/dashboardsV2/types'; @@ -47,6 +49,8 @@ import {generateFieldOptions} from 'app/views/eventsV2/utils'; import Input from 'app/views/settings/components/forms/controls/input'; import Field from 'app/views/settings/components/forms/field'; +import Tooltip from '../tooltip'; + export type DashboardWidgetModalOptions = { organization: Organization; dashboard?: DashboardDetails; @@ -81,7 +85,7 @@ type State = { queries: Widget['queries']; loading: boolean; errors?: Record; - dashboards: SelectValue[]; + dashboards: DashboardListItem[]; selectedDashboard?: SelectValue; }; @@ -179,8 +183,8 @@ class AddDashboardWidgetModal extends React.Component { if ( !selectedDashboard || !( - dashboards.find(({label, value}) => { - return label === selectedDashboard?.label && value === selectedDashboard?.value; + dashboards.find(({title, id}) => { + return title === selectedDashboard?.label && id === selectedDashboard?.value; }) || selectedDashboard.value === 'new' ) ) { @@ -288,10 +292,7 @@ class AddDashboardWidgetModal extends React.Component { ); try { - const response = await promise; - const dashboards = response.map(({id, title}) => { - return {label: title, value: id}; - }); + const dashboards = await promise; this.setState({ dashboards, }); @@ -311,6 +312,13 @@ class AddDashboardWidgetModal extends React.Component { } renderDashboardSelector() { const {errors, loading, dashboards} = this.state; + const dashboardOptions = dashboards.map(d => { + return { + label: d.title, + value: d.id, + isDisabled: d.widgetDisplay.length > MAX_WIDGETS, + }; + }); return (

@@ -329,9 +337,30 @@ class AddDashboardWidgetModal extends React.Component { > ) => this.handleDashboardChange(option)} disabled={loading} + components={{ + Option: ({label, data, ...optionProps}: OptionProps) => ( + + + + ), + }} /> diff --git a/static/app/views/dashboardsV2/dashboard.tsx b/static/app/views/dashboardsV2/dashboard.tsx index a385a95a03c563..8390c9cd4cad61 100644 --- a/static/app/views/dashboardsV2/dashboard.tsx +++ b/static/app/views/dashboardsV2/dashboard.tsx @@ -18,7 +18,7 @@ import withGlobalSelection from 'app/utils/withGlobalSelection'; import {DataSet} from './widget/utils'; import AddWidget, {ADD_WIDGET_BUTTON_DRAG_ID} from './addWidget'; import SortableWidget from './sortableWidget'; -import {DashboardDetails, Widget} from './types'; +import {DashboardDetails, MAX_WIDGETS, Widget} from './types'; type Props = { api: Client; @@ -225,7 +225,7 @@ class Dashboard extends Component { {widgets.map((widget, index) => this.renderWidget(widget, index))} - {isEditing && ( + {isEditing && widgets.length < MAX_WIDGETS && ( AddDashboardWidgetModal', function () { }); MockApiClient.addMockResponse({ url: '/organizations/org-slug/dashboards/', - body: [{id: '1', title: t('Test Dashboard')}], + body: [{id: '1', title: t('Test Dashboard'), widgetDisplay: ['area']}], }); }); From 4fef28376422362bb6c320d26b8050926a958a56 Mon Sep 17 00:00:00 2001 From: Shruthilaya Date: Tue, 28 Sep 2021 17:27:23 -0400 Subject: [PATCH 2/2] add some tests --- .../modals/addDashboardWidgetModal.tsx | 2 +- .../modals/addDashboardWidgetModal.spec.jsx | 29 +++++++++- .../spec/views/dashboardsV2/detail.spec.jsx | 57 ++++++++++++++++++- 3 files changed, 83 insertions(+), 5 deletions(-) diff --git a/static/app/components/modals/addDashboardWidgetModal.tsx b/static/app/components/modals/addDashboardWidgetModal.tsx index 8f65533355c522..4fc83bf0910d03 100644 --- a/static/app/components/modals/addDashboardWidgetModal.tsx +++ b/static/app/components/modals/addDashboardWidgetModal.tsx @@ -316,7 +316,7 @@ class AddDashboardWidgetModal extends React.Component { return { label: d.title, value: d.id, - isDisabled: d.widgetDisplay.length > MAX_WIDGETS, + isDisabled: d.widgetDisplay.length >= MAX_WIDGETS, }; }); return ( diff --git a/tests/js/spec/components/modals/addDashboardWidgetModal.spec.jsx b/tests/js/spec/components/modals/addDashboardWidgetModal.spec.jsx index 8d59ca3896dd92..36a6c85a557993 100644 --- a/tests/js/spec/components/modals/addDashboardWidgetModal.spec.jsx +++ b/tests/js/spec/components/modals/addDashboardWidgetModal.spec.jsx @@ -2,11 +2,12 @@ import {browserHistory} from 'react-router'; import {mountWithTheme} from 'sentry-test/enzyme'; import {initializeOrg} from 'sentry-test/initializeOrg'; -import {getOptionByLabel, selectByLabel} from 'sentry-test/select-new'; +import {getOptionByLabel, openMenu, selectByLabel} from 'sentry-test/select-new'; import AddDashboardWidgetModal from 'app/components/modals/addDashboardWidgetModal'; import {t} from 'app/locale'; import TagStore from 'app/stores/tagStore'; +import * as types from 'app/views/dashboardsV2/types'; const stubEl = props =>

{props.children}
; @@ -97,7 +98,13 @@ describe('Modals -> AddDashboardWidgetModal', function () { }); MockApiClient.addMockResponse({ url: '/organizations/org-slug/dashboards/', - body: [{id: '1', title: t('Test Dashboard'), widgetDisplay: ['area']}], + body: [ + TestStubs.Dashboard([], { + id: '1', + title: 'Test Dashboard', + widgetDisplay: ['area'], + }), + ], }); }); @@ -109,6 +116,7 @@ describe('Modals -> AddDashboardWidgetModal', function () { const wrapper = mountModal({initialData, fromDiscover: true}); // @ts-expect-error await tick(); + await wrapper.update(); selectDashboard(wrapper, {label: t('+ Create New Dashboard'), value: 'new'}); await clickSubmit(wrapper); expect(browserHistory.push).toHaveBeenCalledWith( @@ -123,6 +131,7 @@ describe('Modals -> AddDashboardWidgetModal', function () { const wrapper = mountModal({initialData, fromDiscover: true}); // @ts-expect-error await tick(); + await wrapper.update(); selectDashboard(wrapper, {label: t('Test Dashboard'), value: '1'}); await clickSubmit(wrapper); expect(browserHistory.push).toHaveBeenCalledWith( @@ -133,6 +142,22 @@ describe('Modals -> AddDashboardWidgetModal', function () { wrapper.unmount(); }); + it('disables dashboards with max widgets', async function () { + types.MAX_WIDGETS = 1; + const wrapper = mountModal({initialData, fromDiscover: true}); + // @ts-expect-error + await tick(); + await wrapper.update(); + openMenu(wrapper, {name: 'dashboard', control: true}); + + const input = wrapper.find('SelectControl[name="dashboard"]'); + expect(input.find('Option Option')).toHaveLength(2); + expect(input.find('Option Option').at(0).props().isDisabled).toBe(false); + expect(input.find('Option Option').at(1).props().isDisabled).toBe(true); + + wrapper.unmount(); + }); + it('can update the title', async function () { let widget = undefined; const wrapper = mountModal({ diff --git a/tests/js/spec/views/dashboardsV2/detail.spec.jsx b/tests/js/spec/views/dashboardsV2/detail.spec.jsx index 8e2fe423571316..aaad130425902e 100644 --- a/tests/js/spec/views/dashboardsV2/detail.spec.jsx +++ b/tests/js/spec/views/dashboardsV2/detail.spec.jsx @@ -8,6 +8,7 @@ import {mountGlobalModal} from 'sentry-test/modal'; import ProjectsStore from 'app/stores/projectsStore'; import {DashboardState} from 'app/views/dashboardsV2/types'; +import * as types from 'app/views/dashboardsV2/types'; import ViewEditDashboard from 'app/views/dashboardsV2/view'; describe('Dashboards > Detail', function () { @@ -225,8 +226,16 @@ describe('Dashboards > Detail', function () { MockApiClient.addMockResponse({ url: '/organizations/org-slug/dashboards/', body: [ - TestStubs.Dashboard([], {id: 'default-overview', title: 'Default'}), - TestStubs.Dashboard([], {id: '1', title: 'Custom Errors'}), + TestStubs.Dashboard([], { + id: 'default-overview', + title: 'Default', + widgetDisplay: ['area'], + }), + TestStubs.Dashboard([], { + id: '1', + title: 'Custom Errors', + widgetDisplay: ['area'], + }), ], }); MockApiClient.addMockResponse({ @@ -337,6 +346,50 @@ describe('Dashboards > Detail', function () { expect(modal.find('AddDashboardWidgetModal').props().widget).toEqual(widgets[0]); }); + it('shows add wiget option', async function () { + wrapper = mountWithTheme( + , + initialData.routerContext + ); + await tick(); + wrapper.update(); + + // Enter edit mode. + wrapper.find('Controls Button[data-test-id="dashboard-edit"]').simulate('click'); + wrapper.update(); + expect(wrapper.find('AddWidget').exists()).toBe(true); + + wrapper.unmount(); + }); + + it('hides add widget option', async function () { + types.MAX_WIDGETS = 1; + + wrapper = mountWithTheme( + , + initialData.routerContext + ); + await tick(); + wrapper.update(); + + // Enter edit mode. + wrapper.find('Controls Button[data-test-id="dashboard-edit"]').simulate('click'); + wrapper.update(); + expect(wrapper.find('AddWidget').exists()).toBe(false); + + wrapper.unmount(); + }); + it('hides and shows breadcrumbs based on feature', async function () { const newOrg = initializeOrg({ organization: TestStubs.Organization({