Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions static/app/components/organizations/pageFilters/container.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import LoadingIndicator from 'sentry/components/loadingIndicator';
import {DEFAULT_STATS_PERIOD} from 'sentry/constants';
import {isActiveSuperuser} from 'sentry/utils/isActiveSuperuser';
import {useLocation} from 'sentry/utils/useLocation';
import {useDefaultMaxPickableDays} from 'sentry/utils/useMaxPickableDays';
import useOrganization from 'sentry/utils/useOrganization';
import usePageFilters from 'sentry/utils/usePageFilters';
import useProjects from 'sentry/utils/useProjects';
Expand Down Expand Up @@ -87,6 +88,9 @@ function PageFiltersContainer({
? []
: specifiedProjects.filter(project => !project.isMember);

const defaultMaxPickableDays = useDefaultMaxPickableDays();
maxPickableDays = maxPickableDays ?? defaultMaxPickableDays;

Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: Stale closure captures wrong maxPickableDays value

The doInitialization function captures maxPickableDays computed from useDefaultMaxPickableDays() hook, but the useLayoutEffect at lines 120-129 doesn't include maxPickableDays or defaultMaxPickableDays in its dependency array. When subscription data loads asynchronously after initial mount, the component re-renders with updated defaultMaxPickableDays, creating a new doInitialization with the correct value, but the effect doesn't re-run. This causes page filters to initialize with stale maxPickableDays (typically 90 instead of the subscription's retention days like 30), allowing users to select date ranges beyond their plan limits.

Fix in Cursor Fix in Web

const doInitialization = () => {
initializeUrlState({
organization,
Expand Down
6 changes: 5 additions & 1 deletion static/app/components/timeRangeSelector/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
} from 'sentry/utils/dates';
import {parsePeriodToHours} from 'sentry/utils/duration/parsePeriodToHours';
import getRouteStringFromRoutes from 'sentry/utils/getRouteStringFromRoutes';
import {useDefaultMaxPickableDays} from 'sentry/utils/useMaxPickableDays';
import useOrganization from 'sentry/utils/useOrganization';
import useRouter from 'sentry/utils/useRouter';

Expand Down Expand Up @@ -153,7 +154,7 @@ export function TimeRangeSelector({
showRelative = true,
defaultAbsolute,
defaultPeriod = DEFAULT_STATS_PERIOD,
maxPickableDays = 90,
maxPickableDays,
maxDateRange,
disallowArbitraryRelativeRanges = false,
trigger,
Expand All @@ -167,6 +168,9 @@ export function TimeRangeSelector({
const router = useRouter();
const organization = useOrganization({allowNull: true});

const defaultMaxPickableDays = useDefaultMaxPickableDays();
maxPickableDays = maxPickableDays ?? defaultMaxPickableDays;

const [search, setSearch] = useState('');
const [hasChanges, setHasChanges] = useState(false);
const [hasDateRangeErrors, setHasDateRangeErrors] = useState(false);
Expand Down
6 changes: 5 additions & 1 deletion static/app/types/hooks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ import type {ProductSelectionProps} from 'sentry/components/onboarding/productSe
import type DateRange from 'sentry/components/timeRangeSelector/dateRange';
import type SelectorItems from 'sentry/components/timeRangeSelector/selectorItems';
import type {SentryRouteObject} from 'sentry/router/types';
import type {useMaxPickableDays} from 'sentry/utils/useMaxPickableDays';
import type {
useDefaultMaxPickableDays,
useMaxPickableDays,
} from 'sentry/utils/useMaxPickableDays';
import type {WidgetType} from 'sentry/views/dashboards/types';
import type {OrganizationStatsProps} from 'sentry/views/organizationStats';
import type {RouteAnalyticsContext} from 'sentry/views/routeAnalyticsContextProvider';
Expand Down Expand Up @@ -331,6 +334,7 @@ type ReactHooks = {
'react-hook:use-dashboard-dataset-retention-limit': (props: {
dataset: WidgetType;
}) => number;
'react-hook:use-default-max-pickable-days': typeof useDefaultMaxPickableDays;
'react-hook:use-get-max-retention-days': () => number | undefined;
'react-hook:use-max-pickable-days': typeof useMaxPickableDays;
'react-hook:use-metric-detector-limit': () => {
Expand Down
17 changes: 17 additions & 0 deletions static/app/utils/useMaxPickableDays.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,29 @@ import {useMemo, type ReactNode} from 'react';

import HookOrDefault from 'sentry/components/hookOrDefault';
import type {DatePageFilterProps} from 'sentry/components/organizations/datePageFilter';
import {MAX_PICKABLE_DAYS} from 'sentry/constants';
import {t} from 'sentry/locale';
import HookStore from 'sentry/stores/hookStore';
import {DataCategory} from 'sentry/types/core';
import type {Organization} from 'sentry/types/organization';
import useOrganization from 'sentry/utils/useOrganization';

/**
* This returns the default max pickable days for the current organization.
*
* Use this as the default when there is not known data category.
*/
export function useDefaultMaxPickableDays(): number {
const useDefaultMaxPickableDaysHook =
HookStore.get('react-hook:use-default-max-pickable-days')[0] ??
useDefaultMaxPickableDaysImpl;
return useDefaultMaxPickableDaysHook();
}

function useDefaultMaxPickableDaysImpl() {
return MAX_PICKABLE_DAYS;
}

export interface MaxPickableDaysOptions {
/**
* The maximum number of days the user is allowed to pick on the date page filter
Expand Down
6 changes: 6 additions & 0 deletions static/gsApp/hooks/useMaxPickableDays.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {useMemo} from 'react';
import moment from 'moment-timezone';

import {MAX_PICKABLE_DAYS} from 'sentry/constants';
import {DataCategory} from 'sentry/types/core';
import {defined} from 'sentry/utils';
import {
Expand All @@ -16,6 +17,11 @@ import type {Subscription} from 'getsentry/types';

import useSubscription from './useSubscription';

export function useDefaultMaxPickableDays() {
const subscription = useSubscription();
return subscription?.planDetails?.retentionDays ?? MAX_PICKABLE_DAYS;
}

export function useMaxPickableDays({
dataCategories,
}: UseMaxPickableDaysProps): MaxPickableDaysOptions {
Expand Down
3 changes: 2 additions & 1 deletion static/gsApp/registerHooks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ import ReplayOnboardingAlert from './components/replayOnboardingAlert';
import ReplaySettingsAlert from './components/replaySettingsAlert';
import useButtonTracking from './hooks/useButtonTracking';
import useGetMaxRetentionDays from './hooks/useGetMaxRetentionDays';
import {useMaxPickableDays} from './hooks/useMaxPickableDays';
import {useDefaultMaxPickableDays, useMaxPickableDays} from './hooks/useMaxPickableDays';
import useRouteActivatedHook from './hooks/useRouteActivatedHook';

const PartnershipAgreement = lazy(() => import('getsentry/views/partnershipAgreement'));
Expand Down Expand Up @@ -233,6 +233,7 @@ const GETSENTRY_HOOKS: Partial<Hooks> = {
'component:crons-list-page-header': () => CronsBillingBanner,
'react-hook:route-activated': useRouteActivatedHook,
'react-hook:use-button-tracking': useButtonTracking,
'react-hook:use-default-max-pickable-days': useDefaultMaxPickableDays,
'react-hook:use-max-pickable-days': useMaxPickableDays,
'react-hook:use-get-max-retention-days': useGetMaxRetentionDays,
'react-hook:use-metric-detector-limit': useMetricDetectorLimit,
Expand Down
Loading