Browse Source

feat(new-widget-builder-experience): Port handleDefaultFields function

Priscila Oliveira 3 years ago
parent
commit
60ef4dd63d

+ 2 - 6
static/app/views/dashboardsV2/widgetBuilder/columnFields.tsx

@@ -31,7 +31,7 @@ export function ColumnFields({
   onChange,
 }: Props) {
   return (
-    <ColumnCollectionField
+    <Field
       inline={false}
       error={errors?.find(error => error?.fields)?.fields}
       flexibleControlStateSize
@@ -57,14 +57,10 @@ export function ColumnFields({
           source={widgetType}
         />
       )}
-    </ColumnCollectionField>
+    </Field>
   );
 }
 
 const ColumnCollectionEdit = styled(ColumnEditCollection)`
   margin-top: ${space(1)};
 `;
-
-const ColumnCollectionField = styled(Field)`
-  padding: ${space(1)} 0;
-`;

+ 6 - 0
static/app/views/dashboardsV2/widgetBuilder/header.tsx

@@ -69,6 +69,12 @@ function Header({
           >
             {t('Give Feedback')}
           </Button>
+          <Button
+            external
+            href="https://docs.sentry.io/product/dashboards/custom-dashboards/#widget-builder"
+          >
+            {t('Read the docs')}
+          </Button>
           <Button to={goBackLocation}>{t('Cancel')}</Button>
           {isEditing && onDelete && (
             <Confirm

+ 157 - 93
static/app/views/dashboardsV2/widgetBuilder/widgetBuilder.tsx

@@ -1,4 +1,4 @@
-import {useState} from 'react';
+import {useEffect, useState} from 'react';
 import {RouteComponentProps} from 'react-router';
 import styled from '@emotion/styled';
 import cloneDeep from 'lodash/cloneDeep';
@@ -117,6 +117,7 @@ type Props = RouteComponentProps<RouteParams, {}> & {
   organization: Organization;
   selection: PageFilters;
   tags: TagCollection;
+  defaultTableColumns?: readonly string[];
   defaultTitle?: string;
   defaultWidgetQuery?: WidgetQuery;
   displayType?: DisplayType;
@@ -152,6 +153,7 @@ function WidgetBuilder({
   defaultWidgetQuery,
   displayType,
   defaultTitle,
+  defaultTableColumns,
   tags,
 }: Props) {
   const {widgetId, orgId, dashboardId} = params;
@@ -209,8 +211,63 @@ function WidgetBuilder({
         : DataSet.EVENTS,
     };
   });
+
   const [blurTimeout, setBlurTimeout] = useState<null | number>(null);
 
+  useEffect(() => {
+    defaultFields();
+  }, [state.displayType]);
+
+  function defaultFields() {
+    setState(prevState => {
+      const newState = cloneDeep(prevState);
+      const normalized = normalizeQueries(prevState.displayType, prevState.queries);
+
+      if (prevState.displayType === DisplayType.TOP_N) {
+        // TOP N display should only allow a single query
+        normalized.splice(1);
+      }
+
+      if (!prevState.userHasModified) {
+        // If the Widget is an issue widget,
+        if (
+          prevState.displayType === DisplayType.TABLE &&
+          widget?.widgetType &&
+          WIDGET_TYPE_TO_DATA_SET[widget.widgetType] === DataSet.ISSUES
+        ) {
+          set(newState, 'queries', widget.queries);
+          set(newState, 'dataSet', DataSet.ISSUES);
+          return {...newState, errors: undefined};
+        }
+
+        // Default widget provided by Add to Dashboard from Discover
+        if (defaultWidgetQuery && defaultTableColumns) {
+          // If switching to Table visualization, use saved query fields for Y-Axis if user has not made query changes
+          // This is so the widget can reflect the same columns as the table in Discover without requiring additional user input
+          if (prevState.displayType === DisplayType.TABLE) {
+            normalized.forEach(query => {
+              query.fields = [...defaultTableColumns];
+            });
+          } else if (prevState.displayType === displayType) {
+            // When switching back to original display type, default fields back to the fields provided from the discover query
+            normalized.forEach(query => {
+              query.fields = [...defaultWidgetQuery.fields];
+              query.orderby = defaultWidgetQuery.orderby;
+            });
+          }
+        }
+      }
+
+      if (prevState.dataSet === DataSet.ISSUES) {
+        set(newState, 'dataSet', DataSet.EVENTS);
+      }
+
+      set(newState, 'queries', normalized);
+
+      return {...newState, errors: undefined};
+    });
+  }
+
   function handleDataSetChange(newDataSet: string) {
     setState(prevState => {
       const newState = cloneDeep(prevState);
@@ -260,37 +317,25 @@ function WidgetBuilder({
     });
   }
 
-  if (
-    isEditing &&
-    (!defined(widgetId) ||
-      !dashboard.widgets.find(dashboardWidget => dashboardWidget.id === String(widgetId)))
-  ) {
-    return (
-      <SentryDocumentTitle title={dashboard.title} orgSlug={orgSlug}>
-        <PageContent>
-          <LoadingError message={t('Widget not found.')} />
-        </PageContent>
-      </SentryDocumentTitle>
-    );
-  }
-
-  function handleChangeField(newFields: QueryFieldValue[]) {
+  function handleChangeYAxisOrColumnField(newFields: QueryFieldValue[]) {
     const fieldStrings = newFields.map(generateFieldAsString);
     const aggregateAliasFieldStrings = fieldStrings.map(getAggregateAlias);
 
     for (const index in state.queries) {
       const queryIndex = Number(index);
       const query = state.queries[queryIndex];
+
       const descending = query.orderby.startsWith('-');
       const orderbyAggregateAliasField = query.orderby.replace('-', '');
-      const prevAggregateAliasFieldStrings = query.fields.map(getAggregateAlias);
+      const prevAggregateAliasFieldStrings = query.fields.map(field =>
+        getAggregateAlias(field)
+      );
       const newQuery = cloneDeep(query);
-
       newQuery.fields = fieldStrings;
-
-      if (!aggregateAliasFieldStrings.includes(orderbyAggregateAliasField)) {
-        newQuery.orderby = '';
-
+      if (
+        !aggregateAliasFieldStrings.includes(orderbyAggregateAliasField) &&
+        query.orderby !== ''
+      ) {
         if (prevAggregateAliasFieldStrings.length === newFields.length) {
           // The Field that was used in orderby has changed. Get the new field.
           newQuery.orderby = `${descending && '-'}${
@@ -298,6 +343,8 @@ function WidgetBuilder({
               prevAggregateAliasFieldStrings.indexOf(orderbyAggregateAliasField)
             ]
           }`;
+        } else {
+          newQuery.orderby = '';
         }
       }
 
@@ -305,6 +352,36 @@ function WidgetBuilder({
     }
   }
 
+  function getAmendedFieldOptions(measurements: MeasurementCollection) {
+    return generateFieldOptions({
+      organization,
+      tagKeys: Object.values(tags).map(({key}) => key),
+      measurementKeys: Object.values(measurements).map(({key}) => key),
+      spanOperationBreakdownKeys: SPAN_OP_BREAKDOWN_FIELDS,
+    });
+  }
+
+  if (
+    isEditing &&
+    (!defined(widgetId) ||
+      !dashboard.widgets.find(dashboardWidget => dashboardWidget.id === String(widgetId)))
+  ) {
+    return (
+      <SentryDocumentTitle title={dashboard.title} orgSlug={orgSlug}>
+        <PageContent>
+          <LoadingError message={t('Widget not found.')} />
+        </PageContent>
+      </SentryDocumentTitle>
+    );
+  }
+
+  const widgetType =
+    state.dataSet === DataSet.EVENTS
+      ? WidgetType.DISCOVER
+      : state.dataSet === DataSet.ISSUES
+      ? WidgetType.ISSUE
+      : WidgetType.METRICS;
+
   const canAddSearchConditions =
     [
       DisplayType.LINE,
@@ -319,22 +396,7 @@ function WidgetBuilder({
     DisplayType.BIG_NUMBER,
   ].includes(state.displayType);
 
-  const widgetType =
-    state.dataSet === DataSet.EVENTS
-      ? WidgetType.DISCOVER
-      : state.dataSet === DataSet.ISSUES
-      ? WidgetType.ISSUE
-      : WidgetType.METRICS;
-
   const explodedFields = state.queries[0].fields.map(field => explodeField({field}));
-  function getAmendedFieldOptions(measurements: MeasurementCollection) {
-    return generateFieldOptions({
-      organization,
-      tagKeys: Object.values(tags).map(({key}) => key),
-      measurementKeys: Object.values(measurements).map(({key}) => key),
-      spanOperationBreakdownKeys: SPAN_OP_BREAKDOWN_FIELDS,
-    });
-  }
 
   return (
     <SentryDocumentTitle title={dashboard.title} orgSlug={orgSlug}>
@@ -360,35 +422,37 @@ function WidgetBuilder({
                   'This is a preview of how your widget will appear in the dashboard.'
                 )}
               >
-                <DisplayTypeOptions
-                  name="displayType"
-                  options={DISPLAY_TYPES_OPTIONS}
-                  value={state.displayType}
-                  onChange={(option: {label: string; value: DisplayType}) => {
-                    setState({...state, displayType: option.value});
-                  }}
-                />
-                <WidgetCard
-                  organization={organization}
-                  selection={pageFilters}
-                  widget={{
-                    title: state.title,
-                    displayType: state.displayType,
-                    interval: state.interval,
-                    queries: state.queries,
-                    widgetType,
-                  }}
-                  isEditing={false}
-                  widgetLimitReached={false}
-                  renderErrorMessage={errorMessage =>
-                    typeof errorMessage === 'string' && (
-                      <PanelAlert type="error">{errorMessage}</PanelAlert>
-                    )
-                  }
-                  isSorting={false}
-                  currentWidgetDragging={false}
-                  noLazyLoad
-                />
+                <VisualizationWrapper>
+                  <DisplayTypeOptions
+                    name="displayType"
+                    options={DISPLAY_TYPES_OPTIONS}
+                    value={state.displayType}
+                    onChange={(option: {label: string; value: DisplayType}) => {
+                      setState({...state, displayType: option.value});
+                    }}
+                  />
+                  <WidgetCard
+                    organization={organization}
+                    selection={pageFilters}
+                    widget={{
+                      title: state.title,
+                      displayType: state.displayType,
+                      interval: state.interval,
+                      queries: state.queries,
+                      widgetType,
+                    }}
+                    isEditing={false}
+                    widgetLimitReached={false}
+                    renderErrorMessage={errorMessage =>
+                      typeof errorMessage === 'string' && (
+                        <PanelAlert type="error">{errorMessage}</PanelAlert>
+                      )
+                    }
+                    isSorting={false}
+                    currentWidgetDragging={false}
+                    noLazyLoad
+                  />
+                </VisualizationWrapper>
               </BuildStep>
               <BuildStep
                 title={t('Choose your data set')}
@@ -414,20 +478,17 @@ function WidgetBuilder({
                 >
                   {state.dataSet === DataSet.EVENTS ? (
                     <Measurements>
-                      {({measurements}) => {
-                        const amendedFieldOptions = getAmendedFieldOptions(measurements);
-                        return (
-                          <ColumnFields
-                            displayType={state.displayType}
-                            organization={organization}
-                            widgetType={widgetType}
-                            columns={explodedFields}
-                            errors={state.errors?.queries}
-                            fieldOptions={amendedFieldOptions}
-                            onChange={handleChangeField}
-                          />
-                        );
-                      }}
+                      {({measurements}) => (
+                        <ColumnFields
+                          displayType={state.displayType}
+                          organization={organization}
+                          widgetType={widgetType}
+                          columns={explodedFields}
+                          errors={state.errors?.queries}
+                          fieldOptions={getAmendedFieldOptions(measurements)}
+                          onChange={handleChangeYAxisOrColumnField}
+                        />
+                      )}
                     </Measurements>
                   ) : (
                     <ColumnFields
@@ -459,19 +520,16 @@ function WidgetBuilder({
                   description="Description of what this means"
                 >
                   <Measurements>
-                    {({measurements}) => {
-                      const amendedFieldOptions = getAmendedFieldOptions(measurements);
-                      return (
-                        <YAxisSelector
-                          widgetType={widgetType}
-                          displayType={state.displayType}
-                          fields={explodedFields}
-                          fieldOptions={amendedFieldOptions}
-                          onChange={handleChangeField}
-                          // TODO: errors={getFirstQueryError('fields')}
-                        />
-                      );
-                    }}
+                    {({measurements}) => (
+                      <YAxisSelector
+                        widgetType={widgetType}
+                        displayType={state.displayType}
+                        fields={explodedFields}
+                        fieldOptions={getAmendedFieldOptions(measurements)}
+                        onChange={handleChangeYAxisOrColumnField}
+                        errors={state.errors?.queries}
+                      />
+                    )}
                   </Measurements>
                 </BuildStep>
               )}
@@ -624,6 +682,12 @@ const PageContentWithoutPadding = styled(PageContent)`
   padding: 0;
 `;
 
+const VisualizationWrapper = styled('div')`
+  display: flex;
+  flex-direction: column;
+  margin-right: ${space(2)};
+`;
+
 const DataSetChoices = styled(RadioGroup)`
   @media (min-width: ${p => p.theme.breakpoints[2]}) {
     grid-auto-flow: column;

+ 98 - 113
static/app/views/dashboardsV2/widgetBuilder/yAxisSelector.tsx

@@ -2,17 +2,19 @@ import React from 'react';
 import styled from '@emotion/styled';
 
 import Button from 'sentry/components/button';
+import ButtonBar from 'sentry/components/buttonBar';
 import Field from 'sentry/components/forms/field';
 import {IconAdd, IconDelete} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import space from 'sentry/styles/space';
 import {
   aggregateFunctionOutputType,
+  ColumnType,
   isLegalYAxisType,
   QueryFieldValue,
 } from 'sentry/utils/discover/fields';
 import useOrganization from 'sentry/utils/useOrganization';
-import {QueryField} from 'sentry/views/eventsV2/table/queryField';
+import {FieldValueOption, QueryField} from 'sentry/views/eventsV2/table/queryField';
 import {FieldValueKind} from 'sentry/views/eventsV2/table/types';
 import {generateFieldOptions} from 'sentry/views/eventsV2/utils';
 
@@ -29,24 +31,16 @@ type Props = {
 
   // TODO: For checking against METRICS widget type
   widgetType: Widget['widgetType'];
-  errors?: Record<string, any>;
-  style?: React.CSSProperties;
+  errors?: Record<string, string>[];
 };
 
-function DeleteButton({onDelete}) {
-  return (
-    <Button
-      size="zero"
-      borderless
-      onClick={onDelete}
-      icon={<IconDelete />}
-      title={t('Remove this Y-Axis')}
-      aria-label={t('Remove this Y-Axis')}
-    />
-  );
-}
-
-function AddButton({title, onAdd}) {
+function AddButton({
+  title,
+  onAdd,
+}: {
+  onAdd: (event: React.MouseEvent) => void;
+  title: string;
+}) {
   return (
     <Button size="small" aria-label={title} onClick={onAdd} icon={<IconAdd isCircled />}>
       {title}
@@ -54,14 +48,7 @@ function AddButton({title, onAdd}) {
   );
 }
 
-function YAxisSelector({
-  displayType,
-  fields,
-  style,
-  fieldOptions,
-  onChange,
-  errors,
-}: Props) {
+function YAxisSelector({displayType, fields, fieldOptions, onChange, errors}: Props) {
   const organization = useOrganization();
   // const isMetricWidget = widgetType === WidgetType.METRICS;
 
@@ -109,9 +96,9 @@ function YAxisSelector({
   // data source is from an endpoint that is not timeseries-based.
   // The function/field choice for World Map widget will need to be numeric-like.
   // Column builder for Table widget is already handled above.
-  const doNotValidateYAxis = displayType === 'big_number';
+  const doNotValidateYAxis = displayType === DisplayType.BIG_NUMBER;
 
-  function filterPrimaryOptions(option) {
+  function filterPrimaryOptions(option: FieldValueOption) {
     // Only validate function names for timeseries widgets and
     // world map widgets.
     if (!doNotValidateYAxis && option.value.kind === FieldValueKind.FUNCTION) {
@@ -138,37 +125,58 @@ function YAxisSelector({
     return option.value.kind === FieldValueKind.FUNCTION;
   }
 
-  const filterAggregateParameters = fieldValue => option => {
-    // Only validate function parameters for timeseries widgets and
-    // world map widgets.
-    if (doNotValidateYAxis) {
-      return true;
-    }
+  function filterAggregateParameters(fieldValue: QueryFieldValue) {
+    return (option: FieldValueOption) => {
+      // Only validate function parameters for timeseries widgets and
+      // world map widgets.
+      if (doNotValidateYAxis) {
+        return true;
+      }
 
-    if (fieldValue.kind !== 'function') {
-      return true;
-    }
+      if (fieldValue.kind !== 'function') {
+        return true;
+      }
 
-    // if (isMetricWidget) {
-    //   return true;
-    // }
+      // if (isMetricWidget) {
+      //   return true;
+      // }
 
-    const functionName = fieldValue.function[0];
-    const primaryOutput = aggregateFunctionOutputType(
-      functionName as string,
-      option.value.meta.name
-    );
-    if (primaryOutput) {
-      return isLegalYAxisType(primaryOutput);
-    }
+      const functionName = fieldValue.function[0];
+      const primaryOutput = aggregateFunctionOutputType(
+        functionName as string,
+        option.value.meta.name
+      );
+      if (primaryOutput) {
+        return isLegalYAxisType(primaryOutput);
+      }
 
-    if (option.value.kind === FieldValueKind.FUNCTION) {
-      // Functions are not legal options as an aggregate/function parameter.
-      return false;
-    }
+      if (option.value.kind === FieldValueKind.FUNCTION) {
+        // Functions are not legal options as an aggregate/function parameter.
+        return false;
+      }
+
+      return isLegalYAxisType(option.value.meta.dataType as ColumnType);
+    };
+  }
 
-    return isLegalYAxisType(option.value.meta.dataType);
-  };
+  const fieldError = errors?.find(error => error?.fields)?.fields;
+
+  if (displayType === DisplayType.TOP_N) {
+    const fieldValue = fields[fields.length - 1];
+    return (
+      <Field inline={false} flexibleControlStateSize error={fieldError} required stacked>
+        <QueryFieldWrapper>
+          <QueryField
+            fieldValue={fieldValue}
+            fieldOptions={generateFieldOptions({organization})}
+            onChange={handleTopNChangeField}
+            filterPrimaryOptions={filterPrimaryOptions}
+            filterAggregateParameters={filterAggregateParameters(fieldValue)}
+          />
+        </QueryFieldWrapper>
+      </Field>
+    );
+  }
 
   const canDelete = fields.length > 1;
 
@@ -183,79 +191,56 @@ function YAxisSelector({
     ].includes(displayType) &&
       fields.length === 3);
 
-  let fieldContents: React.ReactElement;
-  if (displayType === DisplayType.TOP_N) {
-    const fieldValue = fields[fields.length - 1];
-    fieldContents = (
-      <QueryFieldWrapper key={`${fieldValue}:0`}>
-        <QueryField
-          fieldValue={fieldValue}
-          fieldOptions={generateFieldOptions({organization})}
-          onChange={value => handleTopNChangeField(value)}
-          filterPrimaryOptions={filterPrimaryOptions}
-          filterAggregateParameters={filterAggregateParameters(fieldValue)}
-        />
-      </QueryFieldWrapper>
-    );
-  } else {
-    fieldContents = (
-      <React.Fragment>
-        {fields.map((fieldValue, i) => (
-          <QueryFieldWrapper key={`${fieldValue}:${i}`}>
-            <QueryField
-              fieldValue={fieldValue}
-              fieldOptions={fieldOptions}
-              onChange={value => handleChangeField(value, i)}
-              filterPrimaryOptions={filterPrimaryOptions}
-              filterAggregateParameters={filterAggregateParameters(fieldValue)}
-              otherColumns={fields}
-            />
-            {(canDelete || fieldValue.kind === 'equation') && (
-              <DeleteButton onDelete={event => handleRemove(event, i)} />
-            )}
-          </QueryFieldWrapper>
-        ))}
-        {!hideAddYAxisButtons && (
-          <Actions>
-            <AddButton title={t('Add Overlay')} onAdd={handleAdd} />
-            <AddButton title={t('Add an Equation')} onAdd={handleAddEquation} />
-          </Actions>
-        )}
-      </React.Fragment>
-    );
-  }
-
   return (
-    <Field
-      data-test-id="y-axis"
-      label={t('Y-Axis')}
-      inline={false}
-      style={{padding: `${space(2)} 0 24px 0`, ...(style ?? {})}}
-      flexibleControlStateSize
-      error={errors?.fields}
-      required
-      stacked
-    >
-      {fieldContents}
+    <Field inline={false} flexibleControlStateSize error={fieldError} required stacked>
+      {fields.map((fieldValue, i) => (
+        <QueryFieldWrapper key={`${fieldValue}:${i}`}>
+          <QueryField
+            fieldValue={fieldValue}
+            fieldOptions={fieldOptions}
+            onChange={value => handleChangeField(value, i)}
+            filterPrimaryOptions={filterPrimaryOptions}
+            filterAggregateParameters={filterAggregateParameters(fieldValue)}
+            otherColumns={fields}
+          />
+          {(canDelete || fieldValue.kind === 'equation') && (
+            <Button
+              size="zero"
+              borderless
+              onClick={event => handleRemove(event, i)}
+              icon={<IconDelete />}
+              title={t('Remove this Y-Axis')}
+              aria-label={t('Remove this Y-Axis')}
+            />
+          )}
+        </QueryFieldWrapper>
+      ))}
+      {!hideAddYAxisButtons && (
+        <Actions gap={1}>
+          <AddButton title={t('Add Overlay')} onAdd={handleAdd} />
+          <AddButton title={t('Add an Equation')} onAdd={handleAddEquation} />
+        </Actions>
+      )}
     </Field>
   );
 }
 
 export default YAxisSelector;
 
-const Actions = styled('div')`
-  & button {
-    margin-right: ${space(1)};
-  }
-`;
-
 const QueryFieldWrapper = styled('div')`
   display: flex;
   align-items: center;
   justify-content: space-between;
-  margin-bottom: ${space(1)};
+
+  :not(:last-child) {
+    margin-bottom: ${space(1)};
+  }
 
   > * + * {
     margin-left: ${space(1)};
   }
 `;
+
+const Actions = styled(ButtonBar)`
+  justify-content: flex-start;
+`;

+ 1 - 0
static/app/views/dashboardsV2/widgetCard/index.tsx

@@ -223,6 +223,7 @@ const StyledPanel = styled(Panel, {
   min-height: 96px;
   display: flex;
   flex-direction: column;
+  overflow: hidden;
 `;
 
 const ToolbarPanel = styled('div')`

+ 1 - 1
static/app/views/eventsV2/table/columnEditCollection.tsx

@@ -549,7 +549,7 @@ class ColumnEditCollection extends React.Component<Props, State> {
 
 const RowContainer = styled('div')`
   display: grid;
-  grid-template-columns: ${space(3)} 1fr ${space(3)};
+  grid-template-columns: ${space(3)} 1fr 40px;
   justify-content: center;
   align-items: center;
   width: 100%;

+ 2 - 1
tests/js/sentry-test/reactTestingLibrary.tsx

@@ -1,11 +1,12 @@
 import {Component, Fragment} from 'react';
 import {cache} from '@emotion/css';
 import {CacheProvider, ThemeProvider} from '@emotion/react';
+// eslint-disable-next-line no-restricted-imports
 import {
   fireEvent as reactRtlFireEvent,
   render,
   RenderOptions,
-} from '@testing-library/react'; // eslint-disable-line no-restricted-imports
+} from '@testing-library/react';
 import * as reactHooks from '@testing-library/react-hooks'; // eslint-disable-line no-restricted-imports
 import userEvent from '@testing-library/user-event'; // eslint-disable-line no-restricted-imports