From d1db9a00b2fd24de0cce5d3ac902431694c24d74 Mon Sep 17 00:00:00 2001 From: Edward Gou Date: Mon, 1 Nov 2021 15:40:51 -0400 Subject: [PATCH 1/6] Allow functions in top events widget columns --- .../dashboards/widgetQueryFields.tsx | 17 ++-- .../modals/addDashboardWidgetModal.tsx | 23 +---- .../dashboards/widgetQueryFields.spec.tsx | 89 +++++++++++++++++++ 3 files changed, 102 insertions(+), 27 deletions(-) create mode 100644 tests/js/spec/components/dashboards/widgetQueryFields.spec.tsx diff --git a/static/app/components/dashboards/widgetQueryFields.tsx b/static/app/components/dashboards/widgetQueryFields.tsx index 0ed4eb74a6f7a3..610669753e8f4f 100644 --- a/static/app/components/dashboards/widgetQueryFields.tsx +++ b/static/app/components/dashboards/widgetQueryFields.tsx @@ -104,7 +104,7 @@ function WidgetQueryFields({ function handleTopNColumnChange(columns: QueryFieldValue[]) { const aggregateFields = getAggregateFields(); - const newFields = [...columns, ...aggregateFields]; + const newFields = [...columns, aggregateFields[aggregateFields.length - 1]]; onChange(newFields); } @@ -186,9 +186,14 @@ function WidgetQueryFields({ }; if (displayType === 'top_n') { - const aggregateFields = getAggregateFields(); - const otherFields = fields.filter(field => !!!aggregateFields.includes(field)); - const fieldValue = aggregateFields[0]; + const fieldValue = fields + .slice() + .reverse() + .find(field => { + const fieldStr = generateFieldAsString(field); + return isAggregateField(fieldStr) || isAggregateEquation(fieldStr); + }) as QueryFieldValue; + const columns = fields.filter(field => field !== fieldValue); return ( @@ -203,7 +208,7 @@ function WidgetQueryFields({ required > - + { query.fields = [...defaultTableColumns]; }); } else if (displayType === DisplayType.TOP_N) { - // Function columns not valid group bys for TOP_N display - const topNFields = [ - ...defaultTableColumns.filter(column => !parseFunction(column)), - ...defaultWidgetQuery.fields, - ]; normalized.forEach(query => { - query.fields = [...topNFields]; + query.fields = [...defaultTableColumns]; query.orderby = defaultWidgetQuery.orderby; }); } else { @@ -460,14 +454,6 @@ class AddDashboardWidgetModal extends React.Component { measurementKeys, }); - const topNFieldOptions = (measurementKeys: string[]) => - generateFieldOptions({ - organization, - tagKeys: Object.values(tags).map(({key}) => key), - measurementKeys, - aggregations: {} as Record, - }); - const isUpdatingWidget = typeof onUpdateWidget === 'function' && !!previousWidget; return ( @@ -526,12 +512,7 @@ class AddDashboardWidgetModal extends React.Component { {({measurements}) => { const measurementKeys = Object.values(measurements).map(({key}) => key); - let amendedFieldOptions; - if (state.displayType === 'top_n') { - amendedFieldOptions = topNFieldOptions(measurementKeys); - } else { - amendedFieldOptions = fieldOptions(measurementKeys); - } + const amendedFieldOptions = fieldOptions(measurementKeys); return ( { + wrapper = mountWithTheme( + undefined} + />, + routerContext + ); + }); + + it('renders with grey dotted previous period when using only a single series', function () { + const columns = wrapper.find('StyledColumnEditCollection').find('QueryField'); + expect(columns.length).toEqual(2); + expect(columns.at(0).props().fieldValue).toEqual({ + kind: 'field', + field: 'title', + }); + expect(columns.at(1).props().fieldValue).toEqual({ + kind: 'function', + function: ['count', '', undefined, undefined], + }); + expect( + wrapper.find('QueryFieldWrapper').find('QueryField').props().fieldValue + ).toEqual({ + function: ['p95', 'transaction.duration', undefined, undefined], + kind: 'function', + }); + }); +}); From 40ca0bfc97bee019ff00fb28ae9faeb64bf8c408 Mon Sep 17 00:00:00 2001 From: Edward Gou Date: Mon, 1 Nov 2021 17:30:24 -0400 Subject: [PATCH 2/6] fixed handleTopNChangeField --- .../components/dashboards/widgetQueryFields.tsx | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/static/app/components/dashboards/widgetQueryFields.tsx b/static/app/components/dashboards/widgetQueryFields.tsx index 610669753e8f4f..d4bd99033a71fd 100644 --- a/static/app/components/dashboards/widgetQueryFields.tsx +++ b/static/app/components/dashboards/widgetQueryFields.tsx @@ -94,11 +94,16 @@ function WidgetQueryFields({ }); } - function handleTopNChangeField(value: QueryFieldValue, fieldIndex: number) { - const aggregateFields = getAggregateFields(); - const otherFields = fields.filter(field => !!!aggregateFields.includes(field)); - aggregateFields[fieldIndex] = value; - const newFields = [...otherFields, ...aggregateFields]; + function handleTopNChangeField(value: QueryFieldValue) { + const fieldValue = fields + .slice() + .reverse() + .find(field => { + const fieldStr = generateFieldAsString(field); + return isAggregateField(fieldStr) || isAggregateEquation(fieldStr); + }) as QueryFieldValue; + const newFields = [...fields]; + newFields[newFields.findIndex(field => field === fieldValue)] = value; onChange(newFields); } @@ -228,7 +233,7 @@ function WidgetQueryFields({ handleTopNChangeField(value, 0)} + onChange={value => handleTopNChangeField(value)} filterPrimaryOptions={filterPrimaryOptions} filterAggregateParameters={filterAggregateParameters(fieldValue)} /> From ba7660f98f47e017e05f540f8d90ef8afde1080b Mon Sep 17 00:00:00 2001 From: Edward Gou Date: Thu, 4 Nov 2021 12:33:49 -0400 Subject: [PATCH 3/6] simplified and fixed y axis not being added to query fields --- .../dashboards/widgetQueryFields.tsx | 32 +++---------------- .../modals/addDashboardWidgetModal.tsx | 3 +- 2 files changed, 6 insertions(+), 29 deletions(-) diff --git a/static/app/components/dashboards/widgetQueryFields.tsx b/static/app/components/dashboards/widgetQueryFields.tsx index d4bd99033a71fd..708c05fb495999 100644 --- a/static/app/components/dashboards/widgetQueryFields.tsx +++ b/static/app/components/dashboards/widgetQueryFields.tsx @@ -8,9 +8,6 @@ import space from 'app/styles/space'; import {Organization} from 'app/types'; import { aggregateFunctionOutputType, - generateFieldAsString, - isAggregateEquation, - isAggregateField, isLegalYAxisType, QueryFieldValue, } from 'app/utils/discover/fields'; @@ -87,29 +84,14 @@ function WidgetQueryFields({ onChange(newFields); } - function getAggregateFields(): QueryFieldValue[] { - return fields.filter(field => { - const fieldStr = generateFieldAsString(field); - return isAggregateField(fieldStr) || isAggregateEquation(fieldStr); - }); - } - function handleTopNChangeField(value: QueryFieldValue) { - const fieldValue = fields - .slice() - .reverse() - .find(field => { - const fieldStr = generateFieldAsString(field); - return isAggregateField(fieldStr) || isAggregateEquation(fieldStr); - }) as QueryFieldValue; const newFields = [...fields]; - newFields[newFields.findIndex(field => field === fieldValue)] = value; + newFields[fields.length - 1] = value; onChange(newFields); } function handleTopNColumnChange(columns: QueryFieldValue[]) { - const aggregateFields = getAggregateFields(); - const newFields = [...columns, aggregateFields[aggregateFields.length - 1]]; + const newFields = [...columns, fields[fields.length - 1]]; onChange(newFields); } @@ -191,14 +173,8 @@ function WidgetQueryFields({ }; if (displayType === 'top_n') { - const fieldValue = fields - .slice() - .reverse() - .find(field => { - const fieldStr = generateFieldAsString(field); - return isAggregateField(fieldStr) || isAggregateEquation(fieldStr); - }) as QueryFieldValue; - const columns = fields.filter(field => field !== fieldValue); + const fieldValue = fields[fields.length - 1]; + const columns = fields.slice(0, fields.length - 1); return ( diff --git a/static/app/components/modals/addDashboardWidgetModal.tsx b/static/app/components/modals/addDashboardWidgetModal.tsx index ae97f8a6bd7126..b0c1a625e2d26c 100644 --- a/static/app/components/modals/addDashboardWidgetModal.tsx +++ b/static/app/components/modals/addDashboardWidgetModal.tsx @@ -260,7 +260,8 @@ class AddDashboardWidgetModal extends React.Component { }); } else if (displayType === DisplayType.TOP_N) { normalized.forEach(query => { - query.fields = [...defaultTableColumns]; + // Append Y-Axis to query.fields since TOP_N view assumes the last field is the Y-Axis + query.fields = [...defaultTableColumns, defaultWidgetQuery.fields[0]]; query.orderby = defaultWidgetQuery.orderby; }); } else { From 989eab6fa22d8bb60cfec5f5fc6d03c80d24fc5f Mon Sep 17 00:00:00 2001 From: Edward Gou Date: Thu, 4 Nov 2021 15:31:53 -0400 Subject: [PATCH 4/6] updated normalizeQueries for top_n --- .../dashboardsV2/widget/eventWidget/utils.tsx | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/static/app/views/dashboardsV2/widget/eventWidget/utils.tsx b/static/app/views/dashboardsV2/widget/eventWidget/utils.tsx index d24bb41815a0a3..5007932cb72df8 100644 --- a/static/app/views/dashboardsV2/widget/eventWidget/utils.tsx +++ b/static/app/views/dashboardsV2/widget/eventWidget/utils.tsx @@ -2,7 +2,6 @@ import isEqual from 'lodash/isEqual'; import { aggregateOutputType, - getAggregateFields, isAggregateFieldOrEquation, isLegalYAxisType, } from 'app/utils/discover/fields'; @@ -66,22 +65,6 @@ export function normalizeQueries( } if (displayType === DisplayType.TOP_N) { - queries = queries.slice(0, 1); - const aggregateFields = getAggregateFields(queries[0].fields); - - let otherFields = queries[0].fields.filter( - field => !!!aggregateFields.includes(field) - ); - - otherFields = otherFields.length ? otherFields : ['title']; - - const fields: string[] = [ - ...otherFields, - aggregateFields.length ? aggregateFields[0] : 'count()', - ]; - - queries = queries.map(query => ({...query, fields})); - return queries; } From f22c10ad89aaabfca2b51227a9a8bd4d776bd60c Mon Sep 17 00:00:00 2001 From: Edward Gou Date: Fri, 5 Nov 2021 14:51:21 -0400 Subject: [PATCH 5/6] clean up --- static/app/views/dashboardsV2/widget/eventWidget/utils.tsx | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/static/app/views/dashboardsV2/widget/eventWidget/utils.tsx b/static/app/views/dashboardsV2/widget/eventWidget/utils.tsx index 5007932cb72df8..462e22ed9ee725 100644 --- a/static/app/views/dashboardsV2/widget/eventWidget/utils.tsx +++ b/static/app/views/dashboardsV2/widget/eventWidget/utils.tsx @@ -60,11 +60,7 @@ export function normalizeQueries( queries = queries.slice(0, 3); } - if (displayType === DisplayType.TABLE) { - return queries; - } - - if (displayType === DisplayType.TOP_N) { + if ([DisplayType.TABLE, DisplayType.TOP_N].includes(displayType)) { return queries; } From 98cae3d4df137abf8ca43c7b0e97f5468327e5b3 Mon Sep 17 00:00:00 2001 From: Edward Gou Date: Mon, 8 Nov 2021 13:30:37 -0500 Subject: [PATCH 6/6] fixed tests --- .../modals/addDashboardWidgetModal.spec.jsx | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/js/spec/components/modals/addDashboardWidgetModal.spec.jsx b/tests/js/spec/components/modals/addDashboardWidgetModal.spec.jsx index 7a6c2766cd2b95..68f3be8b8dafd8 100644 --- a/tests/js/spec/components/modals/addDashboardWidgetModal.spec.jsx +++ b/tests/js/spec/components/modals/addDashboardWidgetModal.spec.jsx @@ -19,6 +19,7 @@ function mountModal({ fromDiscover, defaultWidgetQuery, displayType, + defaultTableColumns, }) { return mountWithTheme( , initialData.routerContext ); @@ -873,6 +875,13 @@ describe('Modals -> AddDashboardWidgetModal', function () { const wrapper = mountModal({ initialData, onAddWidget: data => (widget = data), + defaultTableColumns: ['title', 'count()', 'count_unique(user)', 'epm()'], + defaultWidgetQuery: { + name: '', + fields: ['count()'], + conditions: 'tag:value', + orderby: '', + }, }); // Select Top n display selectByLabel(wrapper, 'Top Events', {name: 'displayType', at: 0, control: true}); @@ -952,12 +961,13 @@ describe('Modals -> AddDashboardWidgetModal', function () { fromDiscover: true, displayType: types.DisplayType.TOP_N, defaultWidgetQuery: {fields: ['count_unique(user)'], orderby: '-count_unique_user'}, - defaultTableColumns: ['title', 'count()', 'count_unique(user)'], + defaultTableColumns: ['title', 'count()'], }); expect(wrapper.find('SelectPicker').at(1).props().value.value).toEqual('top_n'); expect(wrapper.find('WidgetQueriesForm').props().queries[0].fields).toEqual([ 'title', + 'count()', 'count_unique(user)', ]); expect(wrapper.find('WidgetQueriesForm').props().queries[0].orderby).toEqual(