Skip to content

feat(new-widget-builder-experience): Add Metrics dataset - Part 1 - [DD-537] #33047

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
priscilawebdev merged 9 commits into from
Apr 5, 2022
Merged

feat(new-widget-builder-experience): Add Metrics dataset - Part 1 - [DD-537] #33047

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2b3315a
feat(new-widget-builder-experience): Add Metrics dataset
priscilawebdev Mar 29, 2022
2bbbb27
add tests
priscilawebdev Apr 3, 2022
f74b349
remove not need file
priscilawebdev Apr 3, 2022
3dd22a3
add extra comment
priscilawebdev Apr 3, 2022
c7c334f
move client mock
priscilawebdev Apr 3, 2022
79a57bc
fix test
priscilawebdev Apr 3, 2022
41cd3aa
remove useProjects
priscilawebdev Apr 3, 2022
d068e51
Merge branch 'master' into feat/new-widget-builder-experience-add-met…
priscilawebdev Apr 4, 2022
f5de69d
fix test
priscilawebdev Apr 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions static/app/actionCreators/tags.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export function fetchOrganizationTags(
method: 'GET',
query,
});

promise.then(tagFetchSuccess, TagActions.loadTagsError);

return promise;
Expand Down
2 changes: 1 addition & 1 deletion static/app/components/dashboards/issueWidgetQueriesForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
getColumnsAndAggregates,
} from 'sentry/utils/discover/fields';
import {DisplayType, WidgetQuery, WidgetType} from 'sentry/views/dashboardsV2/types';
import IssuesSearchBar from 'sentry/views/dashboardsV2/widgetBuilder/buildSteps/filterResultsStep/issuesSearchBar';
import {IssuesSearchBar} from 'sentry/views/dashboardsV2/widgetBuilder/buildSteps/filterResultsStep/issuesSearchBar';
import {generateIssueWidgetOrderOptions} from 'sentry/views/dashboardsV2/widgetBuilder/issueWidget/utils';
import {generateFieldOptions} from 'sentry/views/eventsV2/utils';
import {IssueSortOptions} from 'sentry/views/issueList/utils';
Expand Down
1 change: 1 addition & 0 deletions static/app/components/dashboards/widgetQueriesForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ class WidgetQueriesForm extends React.Component<Props> {
onChange,
widgetType = WidgetType.DISCOVER,
} = this.props;

const isMetrics = widgetType === WidgetType.METRICS;

const hideLegendAlias = ['table', 'world_map', 'big_number'].includes(displayType);
Expand Down
34 changes: 20 additions & 14 deletions static/app/stores/metricsMetaStore.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,24 @@ import {makeSafeRefluxStore} from 'sentry/utils/makeSafeRefluxStore';
import {CommonStoreDefinition} from './types';

type State = {
/**
* This is state for when tags fetched from the API are loaded
*/
loaded: boolean;
metricsMeta: MetricsMetaCollection;
};

type InternalDefinition = {
metricsMeta: MetricsMetaCollection;
};

interface MetricsMetaStoreDefinition
extends InternalDefinition,
CommonStoreDefinition<State> {
interface MetricsMetaStoreDefinition extends CommonStoreDefinition<State> {
onLoadSuccess(data: MetricsMeta[]): void;
reset(): void;
}

const storeConfig: MetricsMetaStoreDefinition = {
unsubscribeListeners: [],
metricsMeta: {},
state: {
metricsMeta: {},
loaded: false,
},

init() {
this.unsubscribeListeners.push(
Expand All @@ -32,13 +33,15 @@ const storeConfig: MetricsMetaStoreDefinition = {
},

reset() {
this.metricsMeta = {};
this.trigger(this.metricsMeta);
this.state = {
metricsMeta: {},
loaded: false,
};
this.trigger(this.state);
},

getState() {
const {metricsMeta} = this;
return {metricsMeta};
return this.state;
},

onLoadSuccess(data) {
Expand All @@ -50,8 +53,11 @@ const storeConfig: MetricsMetaStoreDefinition = {
return acc;
}, {});

this.metricsMeta = {...this.metricsMeta, ...newFields};
this.trigger(this.metricsMeta);
this.state = {
metricsMeta: {...this.state.metricsMeta, ...newFields},
loaded: true,
};
this.trigger(this.state);
},
};

Expand Down
35 changes: 21 additions & 14 deletions static/app/stores/metricsTagStore.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,24 @@ import {makeSafeRefluxStore} from 'sentry/utils/makeSafeRefluxStore';
import {CommonStoreDefinition} from './types';

type State = {
/**
* This is state for when tags fetched from the API are loaded
*/
loaded: boolean;
metricsTags: MetricsTagCollection;
};

type InternalDefinition = {
metricsTags: MetricsTagCollection;
};

interface MetricsTagStoreDefinition
extends InternalDefinition,
CommonStoreDefinition<State> {
interface MetricsTagStoreDefinition extends CommonStoreDefinition<State> {
onLoadSuccess(data: MetricsTag[]): void;
reset(): void;
}

const storeConfig: MetricsTagStoreDefinition = {
unsubscribeListeners: [],
metricsTags: {},
state: {
metricsTags: {},
loaded: false,
},

init() {
this.unsubscribeListeners.push(
Expand All @@ -32,13 +33,15 @@ const storeConfig: MetricsTagStoreDefinition = {
},

reset() {
this.metricsTags = {};
this.trigger(this.metricsTags);
this.state = {
metricsTags: {},
loaded: false,
};
this.trigger(this.state);
},

getState() {
const {metricsTags} = this;
return {metricsTags};
return this.state;
},

onLoadSuccess(data) {
Expand All @@ -50,8 +53,12 @@ const storeConfig: MetricsTagStoreDefinition = {
return acc;
}, {});

this.metricsTags = {...this.metricsTags, ...newTags};
this.trigger(this.metricsTags);
this.state = {
metricsTags: {...this.state.metricsTags, ...newTags},
loaded: true,
};

this.trigger(this.state);
},
};

Expand Down
31 changes: 31 additions & 0 deletions static/app/utils/useMetricMetas.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import {useEffect} from 'react';

import {fetchMetricsFields} from 'sentry/actionCreators/metrics';
import MetricsMetaStore from 'sentry/stores/metricsMetaStore';
import PageFiltersStore from 'sentry/stores/pageFiltersStore';
import {useLegacyStore} from 'sentry/stores/useLegacyStore';
import useApi from 'sentry/utils/useApi';

import useOrganization from './useOrganization';

export function useMetricMetas() {
const api = useApi();
const organization = useOrganization();
Copy link
Copy Markdown
Member

@shruthilayaj shruthilayaj Apr 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I go into a dashboard and try to add from the 'Add Widget' button, I get an error saying useOrganization called but organization is not set

Copy link
Copy Markdown
Member Author

@priscilawebdev priscilawebdev Apr 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this shouldn't happened :/ the OrganizationContainerContext is wrapping the whole app... I will take a look into it...thanks.

const {selection} = useLegacyStore(PageFiltersStore);
const {metricsMeta, loaded} = useLegacyStore(MetricsMetaStore);
const shouldFetchMetricsMeta = !loaded;

useEffect(() => {
let unmounted = false;

if (!unmounted && shouldFetchMetricsMeta) {
fetchMetricsFields(api, organization.slug, selection.projects);
}

return () => {
unmounted = true;
};
}, [selection.projects, organization.slug, shouldFetchMetricsMeta]);

return {metricMetas: metricsMeta};
}
31 changes: 31 additions & 0 deletions static/app/utils/useMetricTags.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import {useEffect} from 'react';

import {fetchMetricsTags} from 'sentry/actionCreators/metrics';
import MetricsTagStore from 'sentry/stores/metricsTagStore';
import PageFiltersStore from 'sentry/stores/pageFiltersStore';
import {useLegacyStore} from 'sentry/stores/useLegacyStore';
import useApi from 'sentry/utils/useApi';

import useOrganization from './useOrganization';

export function useMetricTags() {
const api = useApi();
const organization = useOrganization();
const {selection} = useLegacyStore(PageFiltersStore);
const {metricsTags, loaded} = useLegacyStore(MetricsTagStore);
const shouldFetchMetricTags = !loaded;

useEffect(() => {
let unmounted = false;

if (!unmounted && shouldFetchMetricTags) {
fetchMetricsTags(api, organization.slug, selection.projects);
}

return () => {
unmounted = true;
};
}, [selection.projects, organization.slug, shouldFetchMetricTags]);
Comment on lines +18 to +28
Copy link
Copy Markdown
Contributor

@edwardgou-sentry edwardgou-sentry Apr 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a blocker, but I think it's possible for fetchMetricsTags to be ran multiple times if it is not loaded and useMetricTags is used multiple times on the same page. This is because the loaded variable is only set to true after the request to metrics/tags/ is finished so multiple requests can happen if they are fired at the same time.

Maybe we can investigate reducing this to a single request by having some kind of state for loading in the store (i think useProjects does something similar)? I think this also applies for useMetricMetas. Again, not a blocker.

Copy link
Copy Markdown
Member Author

@priscilawebdev priscilawebdev Apr 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes I've noticed that and was talking about it with @narsaynorath yesterday... that's why we are still keeping the fetching(tags and fields) on the top of the widget builder, but that's something we want to improve and will investigate afterward


return {metricTags: metricsTags};
}
17 changes: 14 additions & 3 deletions static/app/views/dashboardsV2/dashboard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,11 @@ class Dashboard extends Component<Props, State> {
window.addEventListener('resize', this.debouncedHandleResize);
}

if (organization.features.includes('dashboards-metrics')) {
if (
organization.features.includes('dashboards-metrics') &&
!organization.features.includes('new-widget-builder-experience') &&
organization.features.includes('new-widget-builder-experience-modal-access')
) {
fetchMetricsFields(api, organization.slug, selection.projects);
fetchMetricsTags(api, organization.slug, selection.projects);
}
Expand Down Expand Up @@ -178,8 +182,15 @@ class Dashboard extends Component<Props, State> {
}
if (!isEqual(prevProps.selection.projects, selection.projects)) {
this.fetchMemberList();
fetchMetricsFields(api, organization.slug, selection.projects);
fetchMetricsTags(api, organization.slug, selection.projects);

if (
organization.features.includes('dashboards-metrics') &&
!organization.features.includes('new-widget-builder-experience') &&
organization.features.includes('new-widget-builder-experience-modal-access')
) {
fetchMetricsFields(api, organization.slug, selection.projects);
fetchMetricsTags(api, organization.slug, selection.projects);
}
}
}

Expand Down
9 changes: 9 additions & 0 deletions static/app/views/dashboardsV2/widgetBuilder/buildSteps/columnsStep/columnFields.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {Organization} from 'sentry/types';
import {QueryFieldValue} from 'sentry/utils/discover/fields';
import {DisplayType, WidgetType} from 'sentry/views/dashboardsV2/types';
import ColumnEditCollection from 'sentry/views/eventsV2/table/columnEditCollection';
import {FieldValueOption} from 'sentry/views/eventsV2/table/queryField';
import {generateFieldOptions} from 'sentry/views/eventsV2/utils';

interface Props {
Expand All @@ -16,6 +17,8 @@ interface Props {
organization: Organization;
widgetType: WidgetType;
errors?: Record<string, string>[];
filterPrimaryOptions?: (option: FieldValueOption) => boolean;
noFieldsMessage?: string;
}

export function ColumnFields({
Expand All @@ -26,6 +29,8 @@ export function ColumnFields({
organization,
errors,
onChange,
filterPrimaryOptions,
noFieldsMessage,
}: Props) {
return (
<Field
Expand All @@ -44,6 +49,8 @@ export function ColumnFields({
showAliasField={organization.features.includes(
'new-widget-builder-experience-design'
)}
filterPrimaryOptions={filterPrimaryOptions}
noFieldsMessage={noFieldsMessage}
/>
) : (
// The only other display type this component
Expand All @@ -57,6 +64,8 @@ export function ColumnFields({
fieldOptions={fieldOptions}
organization={organization}
source={widgetType}
filterPrimaryOptions={filterPrimaryOptions}
noFieldsMessage={noFieldsMessage}
/>
)}
</Field>
Expand Down
12 changes: 11 additions & 1 deletion static/app/views/dashboardsV2/widgetBuilder/buildSteps/columnsStep/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {DataSet, getAmendedFieldOptions} from '../../utils';
import {BuildStep} from '../buildStep';

import {ColumnFields} from './columnFields';
import {ReleaseColumnFields} from './releaseColumnFields';

interface Props {
dataSet: DataSet;
Expand Down Expand Up @@ -82,7 +83,7 @@ export function ColumnsStep({
/>
)}
</Measurements>
) : (
) : dataSet === DataSet.ISSUES ? (
<ColumnFields
displayType={displayType}
organization={organization}
Expand All @@ -101,6 +102,15 @@ export function ColumnsStep({
onQueryChange(0, newQuery);
}}
/>
) : (
<ReleaseColumnFields
displayType={displayType}
organization={organization}
widgetType={widgetType}
explodedFields={explodedFields}
queryErrors={queryErrors}
onYAxisOrColumnFieldChange={onYAxisOrColumnFieldChange}
/>
)}
</BuildStep>
);
Expand Down
Loading