Skip to content

Commit 419471b

Browse files
narsaynorathandrewshie-sentry
authored andcommitted
feat(widget-builder): Clean up feature flag (#90851)
Since this feature is GA'd, I'm removing all frontend references to clean up the feature flag. There were also some `onAddWidget` functions that I had to rename since I was passing both the old one and new one. I made these two functions separate because at the time they were long functions that I thought would be harder to parse if I added feature flag logic in the same one. Also fixed the tooltip on the add widget button so it only shows the tooltip in one place and prioritizes the message for permission checks over the max widget limit message. I had to update a number of tests. I dropped any references to `widgetAsQueryParams` because that's not used by the new widget builder and I'll delete all of the old widget builder code soon. And aggregates don't show up in `fields` anymore. Only columns do to fill the group bys.
1 parent 23b37bf commit 419471b

26 files changed

+166
-3117
lines changed

static/app/components/modals/widgetBuilder/addToDashboardModal.spec.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ describe('add to dashboard modal', () => {
315315

316316
await userEvent.click(screen.getByText('Open in Widget Builder'));
317317
expect(initialData.router.push).toHaveBeenCalledWith({
318-
pathname: '/organizations/org-slug/dashboard/1/widget/new/',
318+
pathname: '/organizations/org-slug/dashboard/1/widget-builder/widget/new/',
319319
query: mockWidgetAsQueryParams,
320320
});
321321
});
@@ -344,7 +344,7 @@ describe('add to dashboard modal', () => {
344344

345345
await userEvent.click(screen.getByText('Open in Widget Builder'));
346346
expect(initialData.router.push).toHaveBeenCalledWith({
347-
pathname: '/organizations/org-slug/dashboard/1/widget/new/',
347+
pathname: '/organizations/org-slug/dashboard/1/widget-builder/widget/new/',
348348
query: expect.objectContaining({
349349
defaultTableColumns: ['field1', 'field2'],
350350
defaultTitle: 'Default title',

static/app/components/modals/widgetBuilder/addToDashboardModal.tsx

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -151,11 +151,7 @@ function AddToDashboardModal({
151151
? `/organizations/${organization.slug}/dashboards/new/`
152152
: `/organizations/${organization.slug}/dashboard/${selectedDashboardId}/`;
153153

154-
const builderSuffix = organization.features.includes(
155-
'dashboards-widget-builder-redesign'
156-
)
157-
? 'widget-builder/widget/new/'
158-
: 'widget/new/';
154+
const builderSuffix = 'widget-builder/widget/new/';
159155

160156
const pathname =
161157
page === 'builder' ? `${dashboardsPath}${builderSuffix}` : dashboardsPath;

static/app/views/dashboards/addWidget.tsx

Lines changed: 17 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import {useSortable} from '@dnd-kit/sortable';
22
import styled from '@emotion/styled';
33

44
import Feature from 'sentry/components/acl/feature';
5-
import {Button} from 'sentry/components/core/button';
65
import type {MenuItemProps} from 'sentry/components/dropdownMenu';
76
import {DropdownMenu} from 'sentry/components/dropdownMenu';
87
import {IconAdd} from 'sentry/icons';
@@ -23,14 +22,10 @@ const initialStyles = {
2322
};
2423

2524
type Props = {
26-
onAddWidget: (dataset: DataSet) => void;
27-
onAddWidgetFromNewWidgetBuilder?: (
28-
dataset: DataSet,
29-
openWidgetTemplates?: boolean
30-
) => void;
25+
onAddWidget?: (dataset: DataSet, openWidgetTemplates?: boolean) => void;
3126
};
3227

33-
function AddWidget({onAddWidget, onAddWidgetFromNewWidgetBuilder}: Props) {
28+
function AddWidget({onAddWidget}: Props) {
3429
const {setNodeRef, transform} = useSortable({
3530
disabled: true,
3631
id: ADD_WIDGET_BUTTON_DRAG_ID,
@@ -49,12 +44,12 @@ function AddWidget({onAddWidget, onAddWidgetFromNewWidgetBuilder}: Props) {
4944
{
5045
key: 'create-custom-widget',
5146
label: t('Create Custom Widget'),
52-
onAction: () => onAddWidgetFromNewWidgetBuilder?.(defaultDataset, false),
47+
onAction: () => onAddWidget?.(defaultDataset, false),
5348
},
5449
{
5550
key: 'from-widget-library',
5651
label: t('From Widget Library'),
57-
onAction: () => onAddWidgetFromNewWidgetBuilder?.(defaultDataset, true),
52+
onAction: () => onAddWidget?.(defaultDataset, true),
5853
},
5954
];
6055

@@ -80,45 +75,24 @@ function AddWidget({onAddWidget, onAddWidgetFromNewWidgetBuilder}: Props) {
8075
duration: 0.25,
8176
}}
8277
>
83-
{organization.features.includes('dashboards-widget-builder-redesign') ? (
84-
<InnerWrapper onClick={() => onAddWidgetFromNewWidgetBuilder?.(defaultDataset)}>
85-
<DropdownMenu
86-
items={addWidgetDropdownItems}
87-
data-test-id="widget-add"
88-
triggerProps={{
89-
'aria-label': t('Add Widget'),
90-
size: 'md',
91-
showChevron: false,
92-
icon: <IconAdd isCircled size="lg" color="subText" />,
93-
borderless: true,
94-
}}
95-
/>
96-
</InnerWrapper>
97-
) : (
98-
<InnerWrapper onClick={() => onAddWidget(defaultDataset)}>
99-
<AddButton
100-
data-test-id="widget-add"
101-
icon={<IconAdd size="lg" isCircled color="subText" />}
102-
aria-label={t('Add widget')}
103-
/>
104-
</InnerWrapper>
105-
)}
78+
<InnerWrapper onClick={() => onAddWidget?.(defaultDataset)}>
79+
<DropdownMenu
80+
items={addWidgetDropdownItems}
81+
data-test-id="widget-add"
82+
triggerProps={{
83+
'aria-label': t('Add Widget'),
84+
size: 'md',
85+
showChevron: false,
86+
icon: <IconAdd isCircled size="lg" color="subText" />,
87+
borderless: true,
88+
}}
89+
/>
90+
</InnerWrapper>
10691
</WidgetWrapper>
10792
</Feature>
10893
);
10994
}
11095

111-
const AddButton = styled(Button)`
112-
border: none;
113-
&,
114-
&:focus,
115-
&:active,
116-
&:hover {
117-
background: transparent;
118-
box-shadow: none;
119-
}
120-
`;
121-
12296
export default AddWidget;
12397

12498
const InnerWrapper = styled('div')<{onClick?: () => void}>`

static/app/views/dashboards/controls.tsx

Lines changed: 25 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,7 @@ type Props = {
3333
dashboard: DashboardDetails;
3434
dashboardState: DashboardState;
3535
dashboards: DashboardListItem[];
36-
onAddWidget: (dataset: DataSet) => void;
37-
onAddWidgetFromNewWidgetBuilder: (
38-
dataset: DataSet,
39-
openWidgetTemplates: boolean
40-
) => void;
36+
onAddWidget: (dataset: DataSet, openWidgetTemplates: boolean) => void;
4137
onCancel: () => void;
4238
onCommit: () => void;
4339
onDelete: () => void;
@@ -60,7 +56,6 @@ function Controls({
6056
onDelete,
6157
onCancel,
6258
onAddWidget,
63-
onAddWidgetFromNewWidgetBuilder,
6459
}: Props) {
6560
const [isFavorited, setIsFavorited] = useState(dashboard.isFavorited);
6661
const queryClient = useQueryClient();
@@ -168,15 +163,22 @@ function Controls({
168163
{
169164
key: 'create-custom-widget',
170165
label: t('Create Custom Widget'),
171-
onAction: () => onAddWidgetFromNewWidgetBuilder(defaultDataset, false),
166+
onAction: () => onAddWidget(defaultDataset, false),
172167
},
173168
{
174169
key: 'from-widget-library',
175170
label: t('From Widget Library'),
176-
onAction: () => onAddWidgetFromNewWidgetBuilder(defaultDataset, true),
171+
onAction: () => onAddWidget(defaultDataset, true),
177172
},
178173
];
179174

175+
const tooltipMessage = hasEditAccess
176+
? widgetLimitReached
177+
? tct('Max widgets ([maxWidgets]) per dashboard reached.', {
178+
maxWidgets: MAX_WIDGETS,
179+
})
180+
: null
181+
: t('You do not have permission to edit this dashboard');
180182
return (
181183
<StyledButtonBar gap={1} key="controls">
182184
<FeedbackWidgetButton />
@@ -259,49 +261,22 @@ function Controls({
259261
</Button>
260262
{hasFeature ? (
261263
<Tooltip
262-
title={tct('Max widgets ([maxWidgets]) per dashboard reached.', {
263-
maxWidgets: MAX_WIDGETS,
264-
})}
265-
disabled={!widgetLimitReached}
264+
title={tooltipMessage}
265+
disabled={!widgetLimitReached && hasEditAccess}
266266
>
267-
{organization.features.includes('dashboards-widget-builder-redesign') ? (
268-
<DropdownMenu
269-
items={addWidgetDropdownItems}
270-
isDisabled={widgetLimitReached || !hasEditAccess}
271-
triggerLabel={t('Add Widget')}
272-
triggerProps={{
273-
'aria-label': t('Add Widget'),
274-
size: 'sm',
275-
showChevron: true,
276-
icon: <IconAdd isCircled size="sm" />,
277-
priority: 'primary',
278-
title:
279-
!hasEditAccess &&
280-
t('You do not have permission to edit this dashboard'),
281-
}}
282-
position="bottom-end"
283-
/>
284-
) : (
285-
<Button
286-
data-test-id="add-widget-library"
287-
priority="primary"
288-
size="sm"
289-
disabled={widgetLimitReached || !hasEditAccess}
290-
icon={<IconAdd isCircled />}
291-
onClick={() => {
292-
trackAnalytics('dashboards_views.widget_library.opened', {
293-
organization,
294-
});
295-
onAddWidget(defaultDataset);
296-
}}
297-
title={
298-
!hasEditAccess &&
299-
t('You do not have permission to edit this dashboard')
300-
}
301-
>
302-
{t('Add Widget')}
303-
</Button>
304-
)}
267+
<DropdownMenu
268+
items={addWidgetDropdownItems}
269+
isDisabled={widgetLimitReached || !hasEditAccess}
270+
triggerLabel={t('Add Widget')}
271+
triggerProps={{
272+
'aria-label': t('Add Widget'),
273+
size: 'sm',
274+
showChevron: true,
275+
icon: <IconAdd isCircled size="sm" />,
276+
priority: 'primary',
277+
}}
278+
position="bottom-end"
279+
/>
305280
</Tooltip>
306281
) : null}
307282
</Fragment>

static/app/views/dashboards/dashboard.spec.tsx

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,7 @@ describe('Dashboards > Dashboard', () => {
436436
router = initialData.router,
437437
location = initialData.router.location,
438438
isPreview = false,
439+
onEditWidget = jest.fn(),
439440
}: any) => {
440441
const getDashboardComponent = () => (
441442
<OrganizationContext value={initialData.organization}>
@@ -455,6 +456,7 @@ describe('Dashboards > Dashboard', () => {
455456
location={location}
456457
widgetLimitReached={false}
457458
isPreview={isPreview}
459+
onEditWidget={onEditWidget}
458460
widgetLegendState={widgetLegendState}
459461
/>
460462
</MEPSettingProvider>
@@ -487,7 +489,7 @@ describe('Dashboards > Dashboard', () => {
487489
});
488490
});
489491

490-
it('opens the widget builder when editing with the modal access flag', async function () {
492+
it('triggers the edit widget callback', async function () {
491493
const testData = initializeOrg({
492494
organization: {
493495
features: ['dashboards-basic', 'dashboards-edit'],
@@ -497,21 +499,21 @@ describe('Dashboards > Dashboard', () => {
497499
...mockDashboard,
498500
widgets: [newWidget],
499501
};
502+
const mockOnEditWidget = jest.fn();
500503

501504
mount({
502505
dashboard: dashboardWithOneWidget,
503506
org: testData.organization,
504507
router: testData.router,
505508
location: testData.router.location,
509+
onEditWidget: mockOnEditWidget,
506510
});
507511

508512
await userEvent.click(await screen.findByLabelText('Edit Widget'));
509513

510-
expect(testData.router.push).toHaveBeenCalledWith(
511-
expect.objectContaining({
512-
pathname: '/organizations/org-slug/dashboard/1/widget/0/edit/',
513-
})
514-
);
514+
await waitFor(() => {
515+
expect(mockOnEditWidget).toHaveBeenCalled();
516+
});
515517
});
516518

517519
it('does not show the add widget button if dashboard is in preview mode', async function () {

0 commit comments

Comments
 (0)