Browse Source

fix(insights): query limit does not work between pages (#80320)

Currently, when the user goes from a non module to a module page it
transfers all the datetime selection. This is fine, unless the user has
the query date range limit feature enabled, in which case we should
reset the date selection so that the query limit is still enforced, but
only if the selection is greater then the limit as to not make the UI
confusing.

We also need to be careful to not reset the project, and environment
filters.

This PR adds a `maxPickableDays` prop to the `PageFiltersContainer`,
this is similar to the `maxPickableDays` prop in the `DatePageFilter`.
When `maxPickableDays` is set, we look to see if the current
pageFilters.datetime is greater then the `maxPickableDays`, if it is we
set the pageFilters to `maxPickableDays`
Dominik Buszowiecki 4 months ago
parent
commit
3ad8de2566

+ 47 - 8
static/app/actionCreators/pageFilters.tsx

@@ -15,6 +15,7 @@ import {
 } from 'sentry/components/organizations/pageFilters/persistence';
 import type {PageFiltersStringified} from 'sentry/components/organizations/pageFilters/types';
 import {getDefaultSelection} from 'sentry/components/organizations/pageFilters/utils';
+import {parseStatsPeriod} from 'sentry/components/timeRangeSelector/utils';
 import {
   ALL_ACCESS_PROJECTS,
   DATE_TIME_KEYS,
@@ -28,6 +29,7 @@ import type {Organization} from 'sentry/types/organization';
 import type {Environment, MinimalProject, Project} from 'sentry/types/project';
 import {defined} from 'sentry/utils';
 import {getUtcDateString} from 'sentry/utils/dates';
+import {DAY} from 'sentry/utils/formatters';
 import {valueIsEqual} from 'sentry/utils/object/valueIsEqual';
 
 type EnvironmentId = Environment['id'];
@@ -121,6 +123,10 @@ export type InitializeUrlStateParams = {
   shouldEnforceSingleProject: boolean;
   defaultSelection?: Partial<PageFilters>;
   forceProject?: MinimalProject | null;
+  /**
+   * When set, the stats period will fallback to the `maxPickableDays` days if the stored selection exceeds the limit.
+   */
+  maxPickableDays?: number;
   shouldForceProject?: boolean;
   /**
    * Whether to save changes to local storage. This setting should be page-specific:
@@ -163,6 +169,7 @@ export function initializeUrlState({
   nonMemberProjects,
   skipLoadLastUsed,
   skipLoadLastUsedEnvironment,
+  maxPickableDays,
   shouldPersist = true,
   shouldForceProject,
   shouldEnforceSingleProject,
@@ -327,6 +334,31 @@ export function initializeUrlState({
     project = newProject;
   }
 
+  let shouldUseMaxPickableDays = false;
+
+  if (maxPickableDays && pageFilters.datetime) {
+    let {start, end} = pageFilters.datetime;
+
+    if (pageFilters.datetime.period) {
+      const parsedPeriod = parseStatsPeriod(pageFilters.datetime.period);
+      start = parsedPeriod.start;
+      end = parsedPeriod.end;
+    }
+
+    if (start && end) {
+      const difference = new Date(end).getTime() - new Date(start).getTime();
+      if (difference > maxPickableDays * DAY) {
+        shouldUseMaxPickableDays = true;
+        pageFilters.datetime = {
+          period: `${maxPickableDays}d`,
+          start: null,
+          end: null,
+          utc: datetime.utc,
+        };
+      }
+    }
+  }
+
   const pinnedFilters = organization.features.includes('new-page-filter')
     ? new Set<PinnedPageFilter>(['projects', 'environments', 'datetime'])
     : storedPageFilters?.pinnedFilters ?? new Set();
@@ -343,14 +375,21 @@ export function initializeUrlState({
     PageFiltersStore.updateDesyncedFilters(new Set());
   }
 
-  const newDatetime = {
-    ...datetime,
-    period:
-      parsed.start || parsed.end || parsed.period || shouldUsePinnedDatetime
-        ? datetime.period
-        : null,
-    utc: parsed.utc || shouldUsePinnedDatetime ? datetime.utc : null,
-  };
+  const newDatetime = shouldUseMaxPickableDays
+    ? {
+        period: `${maxPickableDays}d`,
+        start: null,
+        end: null,
+        utc: datetime.utc,
+      }
+    : {
+        ...datetime,
+        period:
+          parsed.start || parsed.end || parsed.period || shouldUsePinnedDatetime
+            ? datetime.period
+            : null,
+        utc: parsed.utc || shouldUsePinnedDatetime ? datetime.utc : null,
+      };
 
   if (!skipInitializeUrlParams) {
     updateParams({project, environment, ...newDatetime}, router, {

+ 52 - 0
static/app/components/organizations/pageFilters/container.spec.tsx

@@ -652,6 +652,58 @@ describe('PageFiltersContainer', function () {
     });
   });
 
+  describe('maxPickableDays param', function () {
+    it('applies maxPickableDays if the query parms exceed it', async function () {
+      renderComponent(
+        <PageFiltersContainer maxPickableDays={7} />,
+        changeQuery(router, {statsPeriod: '14d'}),
+        organization
+      );
+
+      expect(router.push).not.toHaveBeenCalled();
+
+      await waitFor(() =>
+        expect(PageFiltersStore.getState().selection).toEqual({
+          datetime: {
+            period: '7d',
+            utc: null,
+            start: null,
+            end: null,
+          },
+          environments: [],
+          projects: [],
+        })
+      );
+
+      expect(router.push).not.toHaveBeenCalled();
+    });
+
+    it('does not use maxPickableDays if the query parms do not exceed it', async function () {
+      renderComponent(
+        <PageFiltersContainer maxPickableDays={7} />,
+        changeQuery(router, {statsPeriod: '3d'}),
+        organization
+      );
+
+      expect(router.push).not.toHaveBeenCalled();
+
+      await waitFor(() =>
+        expect(PageFiltersStore.getState().selection).toEqual({
+          datetime: {
+            period: '3d',
+            utc: null,
+            start: null,
+            end: null,
+          },
+          environments: [],
+          projects: [],
+        })
+      );
+
+      expect(router.push).not.toHaveBeenCalled();
+    });
+  });
+
   describe('skipInitializeUrlParams', function () {
     const initialData = initializeOrg({
       organization,

+ 2 - 0
static/app/components/organizations/pageFilters/container.tsx

@@ -57,6 +57,7 @@ interface Props extends InitializeUrlStateProps {
 function PageFiltersContainer({
   skipLoadLastUsed,
   skipLoadLastUsedEnvironment,
+  maxPickableDays,
   children,
   ...props
 }: Props) {
@@ -99,6 +100,7 @@ function PageFiltersContainer({
       router,
       skipLoadLastUsed,
       skipLoadLastUsedEnvironment,
+      maxPickableDays,
       memberProjects,
       nonMemberProjects,
       defaultSelection,

+ 8 - 2
static/app/views/insights/common/components/modulePageProviders.tsx

@@ -12,7 +12,7 @@ import {NoAccess} from 'sentry/views/insights/common/components/noAccess';
 import {useHasDataTrackAnalytics} from 'sentry/views/insights/common/utils/useHasDataTrackAnalytics';
 import {useModuleTitles} from 'sentry/views/insights/common/utils/useModuleTitle';
 import {useDomainViewFilters} from 'sentry/views/insights/pages/useFilters';
-import {INSIGHTS_TITLE} from 'sentry/views/insights/settings';
+import {INSIGHTS_TITLE, QUERY_DATE_RANGE_LIMIT} from 'sentry/views/insights/settings';
 import type {ModuleName} from 'sentry/views/insights/types';
 
 type ModuleNameStrings = `${ModuleName}`;
@@ -37,6 +37,10 @@ export function ModulePageProviders({
   const moduleTitles = useModuleTitles();
   const {isInDomainView} = useDomainViewFilters();
 
+  const hasDateRangeQueryLimit = organization.features.includes(
+    'insights-query-date-range-limit'
+  );
+
   useHasDataTrackAnalytics(moduleName as ModuleName, analyticEventName);
 
   const moduleTitle = moduleTitles[moduleName];
@@ -47,7 +51,9 @@ export function ModulePageProviders({
     .join(' — ');
 
   return (
-    <PageFiltersContainer>
+    <PageFiltersContainer
+      maxPickableDays={hasDateRangeQueryLimit ? QUERY_DATE_RANGE_LIMIT : undefined}
+    >
       <SentryDocumentTitle title={fullPageTitle} orgSlug={organization.slug}>
         {shouldUseUpsellHook && (
           <UpsellPageHook moduleName={moduleName}>