Browse Source

feat(trends): Set default trend parameter based on project type (#31513)

* feat(trends): If no trend parameter in location, set default trend parameter based on project

When no trend parameter is set in query, get appropriate trend parameter based on the project type.

Fixes VIS-1275

* fix(trends): Set duration as default trend param for frontend_other project performance type

* ref(trends): Set default trend parameter inside render

* feat(trends): Determine current trend parameter based on a project

* fix(trends): Set trend parameter to duration for project performance type any

* fix(trends): Fix typings

* fix(trends): Use useProjects hook instead of withProjects HOC

* fix(trends): Revert to using the HOC to get projects

* style(lint): Auto commit lint changes

* test(trends): Test default trend parameter utils

* test(trends): Fix typo in utils tests tests

Co-authored-by: Alberto Leal <mail4alberto@gmail.com>

* test setting default trend parameter

* fix typo

Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
Co-authored-by: Alberto Leal <mail4alberto@gmail.com>
Dameli Ushbayeva 3 years ago
parent
commit
762afa3b82

+ 11 - 4
static/app/utils/performance/trends/trendsDiscoverQuery.tsx

@@ -1,10 +1,12 @@
 import * as React from 'react';
 
+import {Project} from 'sentry/types';
 import GenericDiscoverQuery, {
   DiscoverQueryProps,
   GenericChildrenProps,
 } from 'sentry/utils/discover/genericDiscoverQuery';
 import withApi from 'sentry/utils/withApi';
+import withProjects from 'sentry/utils/withProjects';
 import {
   TrendChangeType,
   TrendFunctionField,
@@ -21,6 +23,7 @@ import {
 
 export type TrendsRequest = {
   eventView: Partial<TrendView>;
+  projects: Project[];
   trendChangeType?: TrendChangeType;
   trendFunctionField?: TrendFunctionField;
 };
@@ -47,10 +50,14 @@ type EventProps = RequestProps & {
 };
 
 export function getTrendsRequestPayload(props: RequestProps) {
-  const {eventView} = props;
+  const {eventView, projects} = props;
   const apiPayload: TrendsQuery = eventView?.getEventsAPIPayload(props.location);
   const trendFunction = getCurrentTrendFunction(props.location, props.trendFunctionField);
-  const trendParameter = getCurrentTrendParameter(props.location);
+  const trendParameter = getCurrentTrendParameter(
+    props.location,
+    projects,
+    eventView.project
+  );
   apiPayload.trendFunction = generateTrendFunctionAsString(
     trendFunction.field,
     trendParameter.column
@@ -89,6 +96,6 @@ function EventsDiscoverQuery(props: EventProps) {
   );
 }
 
-export const TrendsEventsDiscoverQuery = withApi(EventsDiscoverQuery);
+export const TrendsEventsDiscoverQuery = withApi(withProjects(EventsDiscoverQuery));
 
-export default withApi(TrendsDiscoverQuery);
+export default withApi(withProjects(TrendsDiscoverQuery));

+ 3 - 1
static/app/views/performance/data.tsx

@@ -459,7 +459,9 @@ function generateGenericPerformanceEventView(
   }
 
   if (query.trendParameter) {
-    const trendParameter = getCurrentTrendParameter(location);
+    // projects and projectIds are not necessary here since trendParameter will always
+    // be present in location and will not be determined based on the project type
+    const trendParameter = getCurrentTrendParameter(location, [], []);
     if (Boolean(WEB_VITAL_DETAILS[trendParameter.column])) {
       eventView.additionalConditions.addFilterValues('has', [trendParameter.column]);
     }

+ 21 - 6
static/app/views/performance/trends/changedTransactions.tsx

@@ -170,8 +170,14 @@ function handleFilterTransaction(location: Location, transaction: string) {
   });
 }
 
-function handleFilterDuration(location: Location, value: number, symbol: FilterSymbols) {
-  const durationTag = getCurrentTrendParameter(location).column;
+function handleFilterDuration(
+  location: Location,
+  value: number,
+  symbol: FilterSymbols,
+  projects: Project[],
+  projectIds: Readonly<number[]>
+) {
+  const durationTag = getCurrentTrendParameter(location, projects, projectIds).column;
   const queryString = decodeScalar(location.query.query);
   const conditions = new MutableSearch(queryString ?? '');
 
@@ -213,7 +219,7 @@ function ChangedTransactions(props: Props) {
 
   const trendView = props.trendView.clone();
   const chartTitle = getChartTitle(trendChangeType);
-  modifyTrendView(trendView, location, trendChangeType);
+  modifyTrendView(trendView, location, trendChangeType, projects);
 
   const onCursor = makeTrendsCursorHandler(trendChangeType);
   const cursor = decodeScalar(location.query[trendCursorNames[trendChangeType]]);
@@ -230,7 +236,11 @@ function ChangedTransactions(props: Props) {
     >
       {({isLoading, trendsData, pageLinks}) => {
         const trendFunction = getCurrentTrendFunction(location);
-        const trendParameter = getCurrentTrendParameter(location);
+        const trendParameter = getCurrentTrendParameter(
+          location,
+          projects,
+          trendView.project
+        );
         const events = normalizeTrends(
           (trendsData && trendsData.events && trendsData.events.data) || []
         );
@@ -352,6 +362,7 @@ function TrendsListItem(props: TrendsListItemProps) {
     location,
     projects,
     handleSelectTransaction,
+    trendView,
   } = props;
   const color = trendToColor[trendChangeType].default;
 
@@ -449,7 +460,9 @@ function TrendsListItem(props: TrendsListItemProps) {
             handleFilterDuration(
               location,
               longestPeriodValue,
-              FilterSymbols.LESS_THAN_EQUALS
+              FilterSymbols.LESS_THAN_EQUALS,
+              projects,
+              trendView.project
             )
           }
         >
@@ -460,7 +473,9 @@ function TrendsListItem(props: TrendsListItemProps) {
             handleFilterDuration(
               location,
               longestPeriodValue,
-              FilterSymbols.GREATER_THAN_EQUALS
+              FilterSymbols.GREATER_THAN_EQUALS,
+              projects,
+              trendView.project
             )
           }
         >

+ 3 - 1
static/app/views/performance/trends/chart.tsx

@@ -233,6 +233,8 @@ export function Chart({
   disableLegend,
   grid,
   height,
+  projects,
+  project,
 }: Props) {
   const theme = useTheme();
 
@@ -263,7 +265,7 @@ export function Chart({
   const data = events?.data ?? [];
 
   const trendFunction = getCurrentTrendFunction(location, trendFunctionField);
-  const trendParameter = getCurrentTrendParameter(location);
+  const trendParameter = getCurrentTrendParameter(location, projects, project);
   const chartLabel = generateTrendFunctionAsString(
     trendFunction.field,
     trendParameter.column

+ 16 - 6
static/app/views/performance/trends/content.tsx

@@ -13,7 +13,7 @@ import {MAX_QUERY_LENGTH} from 'sentry/constants';
 import {IconFlag} from 'sentry/icons/iconFlag';
 import {t} from 'sentry/locale';
 import space from 'sentry/styles/space';
-import {Organization, PageFilters} from 'sentry/types';
+import {Organization, PageFilters, Project} from 'sentry/types';
 import {trackAnalyticsEvent} from 'sentry/utils/analytics';
 import EventView from 'sentry/utils/discover/eventView';
 import {generateAggregateFields} from 'sentry/utils/discover/fields';
@@ -41,6 +41,7 @@ type Props = {
   eventView: EventView;
   location: Location;
   organization: Organization;
+  projects: Project[];
   selection: PageFilters;
 };
 
@@ -161,7 +162,7 @@ class TrendsContent extends React.Component<Props, State> {
   }
 
   render() {
-    const {organization, eventView, location} = this.props;
+    const {organization, eventView, location, projects} = this.props;
     const {previousTrendFunction} = this.state;
 
     const trendView = eventView.clone() as TrendView;
@@ -192,7 +193,11 @@ class TrendsContent extends React.Component<Props, State> {
       ['epm()', 'eps()']
     );
     const currentTrendFunction = getCurrentTrendFunction(location);
-    const currentTrendParameter = getCurrentTrendParameter(location);
+    const currentTrendParameter = getCurrentTrendParameter(
+      location,
+      projects,
+      eventView.project
+    );
     const query = getTransactionSearchQuery(location);
 
     return (
@@ -224,7 +229,7 @@ class TrendsContent extends React.Component<Props, State> {
         </Layout.Header>
         <Layout.Body>
           <Layout.Main fullWidth>
-            <DefaultTrends location={location} eventView={eventView}>
+            <DefaultTrends location={location} eventView={eventView} projects={projects}>
               <StyledSearchContainer>
                 <StyledSearchBar
                   searchSource="trends"
@@ -300,16 +305,21 @@ type DefaultTrendsProps = {
   children: React.ReactNode[];
   eventView: EventView;
   location: Location;
+  projects: Project[];
 };
 
 class DefaultTrends extends React.Component<DefaultTrendsProps> {
   hasPushedDefaults = false;
 
   render() {
-    const {children, location, eventView} = this.props;
+    const {children, location, eventView, projects} = this.props;
 
     const queryString = decodeScalar(location.query.query);
-    const trendParameter = getCurrentTrendParameter(location);
+    const trendParameter = getCurrentTrendParameter(
+      location,
+      projects,
+      eventView.project
+    );
     const conditions = new MutableSearch(queryString || '');
 
     if (queryString || this.hasPushedDefaults) {

+ 2 - 1
static/app/views/performance/trends/index.tsx

@@ -66,13 +66,14 @@ class TrendsSummary extends Component<Props, State> {
   };
 
   renderContent() {
-    const {organization, location} = this.props;
+    const {organization, location, projects} = this.props;
     const {eventView} = this.state;
     return (
       <TrendsContent
         organization={organization}
         location={location}
         eventView={eventView}
+        projects={projects}
       />
     );
   }

+ 2 - 1
static/app/views/performance/trends/types.tsx

@@ -1,7 +1,7 @@
 import moment from 'moment';
 
 import {EventQuery} from 'sentry/actionCreators/events';
-import {EventsStatsData} from 'sentry/types';
+import {EventsStatsData, Project} from 'sentry/types';
 import EventView, {LocationQuery} from 'sentry/utils/discover/eventView';
 
 export type TrendView = EventView & {
@@ -86,6 +86,7 @@ export type TrendsDataEvents = {
 
 export type TrendsData = {
   events: TrendsDataEvents;
+  projects: Project[];
   stats: TrendsStats;
 };
 

+ 46 - 3
static/app/views/performance/trends/utils.tsx

@@ -18,6 +18,8 @@ import {decodeScalar} from 'sentry/utils/queryString';
 import theme from 'sentry/utils/theme';
 import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 
+import {platformToPerformanceType, PROJECT_PERFORMANCE_TYPE} from '../utils';
+
 import {
   NormalizedTrendsTransaction,
   TrendChangeType,
@@ -154,12 +156,52 @@ export function getCurrentTrendFunction(
   return trendFunction || TRENDS_FUNCTIONS[0];
 }
 
-export function getCurrentTrendParameter(location: Location): TrendParameter {
+function getDefaultTrendParameter(
+  projects: Project[],
+  projectIds: Readonly<number[]>
+): TrendParameter {
+  const performanceType = platformToPerformanceType(projects, projectIds);
+  const trendParameter = performanceTypeToTrendParameterLabel(performanceType);
+
+  return trendParameter;
+}
+
+export function getCurrentTrendParameter(
+  location: Location,
+  projects: Project[],
+  projectIds: Readonly<number[]>
+): TrendParameter {
   const trendParameterLabel = decodeScalar(location?.query?.trendParameter);
   const trendParameter = TRENDS_PARAMETERS.find(
     ({label}) => label === trendParameterLabel
   );
-  return trendParameter || TRENDS_PARAMETERS[0];
+
+  if (trendParameter) {
+    return trendParameter;
+  }
+
+  const defaultTrendParameter = getDefaultTrendParameter(projects, projectIds);
+  return defaultTrendParameter;
+}
+
+export function performanceTypeToTrendParameterLabel(
+  performanceType: PROJECT_PERFORMANCE_TYPE
+): TrendParameter {
+  switch (performanceType) {
+    case PROJECT_PERFORMANCE_TYPE.FRONTEND:
+      return {
+        label: 'LCP',
+        column: TrendColumnField.LCP,
+      };
+    case PROJECT_PERFORMANCE_TYPE.ANY:
+    case PROJECT_PERFORMANCE_TYPE.BACKEND:
+    case PROJECT_PERFORMANCE_TYPE.FRONTEND_OTHER:
+    default:
+      return {
+        label: 'Duration',
+        column: TrendColumnField.DURATION,
+      };
+  }
 }
 
 export function generateTrendFunctionAsString(
@@ -196,10 +238,11 @@ export function modifyTrendView(
   trendView: TrendView,
   location: Location,
   trendsType: TrendChangeType,
+  projects: Project[],
   isProjectOnly?: boolean
 ) {
   const trendFunction = getCurrentTrendFunction(location);
-  const trendParameter = getCurrentTrendParameter(location);
+  const trendParameter = getCurrentTrendParameter(location, projects, trendView.project);
 
   const transactionField = isProjectOnly ? [] : ['transaction'];
   const fields = [...transactionField, 'project'].map(field => ({

+ 48 - 0
tests/js/spec/views/performance/trends/index.spec.jsx

@@ -386,6 +386,54 @@ describe('Performance > Trends', function () {
     }
   }, 10000);
 
+  it('sets LCP as a default trend parameter for frontend project if query does not specify trend parameter', async function () {
+    const projects = [TestStubs.Project({id: 1, platform: 'javascript'})];
+    const data = initializeTrendsData(projects, {project: [1]});
+    wrapper = mountWithTheme(
+      <TrendsIndex organization={data.organization} location={data.router.location} />,
+      data.routerContext
+    );
+
+    await tick();
+    wrapper.update();
+
+    const menu = wrapper.find('TrendsDropdown DropdownMenu').find('DropdownButton').at(1);
+    const label = menu.find('DropdownButton');
+    expect(label.text()).toContain('LCP');
+  });
+
+  it('sets duration as a default trend parameter for backend project if query does not specify trend parameter', async function () {
+    const projects = [TestStubs.Project({id: 1, platform: 'python'})];
+    const data = initializeTrendsData(projects, {project: [1]});
+    wrapper = mountWithTheme(
+      <TrendsIndex organization={data.organization} location={data.router.location} />,
+      data.routerContext
+    );
+
+    await tick();
+    wrapper.update();
+
+    const menu = wrapper.find('TrendsDropdown DropdownMenu').find('DropdownButton').at(1);
+    const label = menu.find('DropdownButton');
+    expect(label.text()).toContain('Duration');
+  });
+
+  it('sets trend parameter from query and ignores default trend parameter', async function () {
+    const projects = [TestStubs.Project({id: 1, platform: 'javascript'})];
+    const data = initializeTrendsData(projects, {project: [1], trendParameter: 'FCP'});
+    wrapper = mountWithTheme(
+      <TrendsIndex organization={data.organization} location={data.router.location} />,
+      data.routerContext
+    );
+
+    await tick();
+    wrapper.update();
+
+    const menu = wrapper.find('TrendsDropdown DropdownMenu').find('DropdownButton').at(1);
+    const label = menu.find('DropdownButton');
+    expect(label.text()).toContain('FCP');
+  });
+
   it('choosing a parameter changes location', async function () {
     const projects = [TestStubs.Project()];
     const data = initializeTrendsData(projects, {project: ['-1']});

+ 75 - 0
tests/js/spec/views/performance/trends/utils.spec.tsx

@@ -0,0 +1,75 @@
+import {TrendColumnField} from 'sentry/views/performance/trends/types';
+import {
+  getCurrentTrendParameter,
+  performanceTypeToTrendParameterLabel,
+} from 'sentry/views/performance/trends/utils';
+import {PROJECT_PERFORMANCE_TYPE} from 'sentry/views/performance/utils';
+
+describe('Trend parameter utils', function () {
+  describe('performanceTypeToTrendParameterLabel', function () {
+    it('returns correct trend parameter label based on performance type', function () {
+      const lcp = {
+        label: 'LCP',
+        column: TrendColumnField.LCP,
+      };
+
+      const duration = {
+        label: 'Duration',
+        column: TrendColumnField.DURATION,
+      };
+
+      const frontendProjectOutput = performanceTypeToTrendParameterLabel(
+        PROJECT_PERFORMANCE_TYPE.FRONTEND
+      );
+      expect(frontendProjectOutput).toEqual(lcp);
+
+      const anyProjectOutput = performanceTypeToTrendParameterLabel(
+        PROJECT_PERFORMANCE_TYPE.ANY
+      );
+      expect(anyProjectOutput).toEqual(duration);
+
+      const backendProjectOutput = performanceTypeToTrendParameterLabel(
+        PROJECT_PERFORMANCE_TYPE.BACKEND
+      );
+      expect(backendProjectOutput).toEqual(duration);
+
+      const frontendOtherProjectOutput = performanceTypeToTrendParameterLabel(
+        PROJECT_PERFORMANCE_TYPE.FRONTEND_OTHER
+      );
+      expect(frontendOtherProjectOutput).toEqual(duration);
+
+      const mobileProjectOutput = performanceTypeToTrendParameterLabel(
+        PROJECT_PERFORMANCE_TYPE.MOBILE
+      );
+      expect(mobileProjectOutput).toEqual(duration);
+    });
+  });
+
+  describe('getCurrentTrendParameter', function () {
+    it('returns trend parameter from location', () => {
+      const location = TestStubs.location({query: {trendParameter: 'FCP'}});
+      const expectedTrendParameter = {
+        label: 'FCP',
+        column: TrendColumnField.FCP,
+      };
+      // project with performance type 'any'
+      const projects = [TestStubs.Project({id: 1, platform: null})];
+
+      const output = getCurrentTrendParameter(location, projects, [1]);
+      expect(output).toEqual(expectedTrendParameter);
+    });
+
+    it('returns default trend parameter based on project type if no trend parameter set in location', function () {
+      const location = TestStubs.location();
+      const expectedTrendParameter = {
+        label: 'Duration',
+        column: TrendColumnField.DURATION,
+      };
+      // project with performance type 'any'
+      const projects = [TestStubs.Project({id: 1, platform: null})];
+
+      const output = getCurrentTrendParameter(location, projects, [1]);
+      expect(output).toEqual(expectedTrendParameter);
+    });
+  });
+});