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
79 changes: 56 additions & 23 deletions static/app/components/pageFilters/actions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ export type InitializeUrlStateParams = {
organization: Organization;
defaultSelection?: Partial<PageFilters>;
forceProject?: MinimalProject | null;
/**
* the maximum number of sequential days that can be selected on the date page filter
*/
maxDateRange?: number;
/**
* When set, the stats period will fallback to the `maxPickableDays` days if the stored selection exceeds the limit.
*/
Expand Down Expand Up @@ -157,6 +161,7 @@ export function initializeUrlState({
skipLoadLastUsed,
skipLoadLastUsedEnvironment,
maxPickableDays,
maxDateRange,
shouldPersist = true,
shouldForceProject,
defaultSelection,
Expand Down Expand Up @@ -308,6 +313,7 @@ export function initializeUrlState({
}

let shouldUseMaxPickableDays = false;
let shouldUseMaxDateRange = false;

if (maxPickableDays && pageFilters.datetime) {
let {start, end} = pageFilters.datetime;
Expand All @@ -320,16 +326,33 @@ export function initializeUrlState({

if (start && end) {
const periodStart = new Date(start);
const periodEnd = new Date(end);
const maxPeriod = parseStatsPeriod(`${maxPickableDays}d`);
const maxTimeRange = (maxDateRange ?? maxPickableDays) * 24 * 60 * 60 * 1000;
const maxStart = new Date(maxPeriod.start);
if (periodStart.getTime() < maxStart.getTime()) {
shouldUseMaxPickableDays = true;
pageFilters.datetime = {
period: `${maxPickableDays}d`,
start: null,
end: null,
utc: datetime.utc,
};
if (maxDateRange) {
if (
periodEnd.getTime() - periodStart.getTime() > maxTimeRange ||
periodStart.getTime() < maxStart.getTime()
) {
shouldUseMaxDateRange = true;
pageFilters.datetime = {
period: `${maxDateRange}d`,
start: null,
end: null,
utc: datetime.utc,
};
}
} else {
if (periodStart.getTime() < maxStart.getTime()) {
shouldUseMaxPickableDays = true;
pageFilters.datetime = {
period: `${maxPickableDays}d`,
start: null,
end: null,
utc: datetime.utc,
};
}
}
}
}
Expand All @@ -343,21 +366,31 @@ export function initializeUrlState({
);
}

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,
};
let newDatetime: PageFiltersUpdate;
if (shouldUseMaxDateRange) {
newDatetime = {
period: `${maxDateRange}d`,
start: null,
end: null,
utc: datetime.utc,
};
} else if (shouldUseMaxPickableDays) {
newDatetime = {
period: `${maxPickableDays}d`,
start: null,
end: null,
utc: datetime.utc,
};
} else {
newDatetime = {
...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}, location, navigate, {
Expand Down
124 changes: 124 additions & 0 deletions static/app/components/pageFilters/container.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {PageFiltersStore} from 'sentry/components/pageFilters/store';
import {OrganizationsStore} from 'sentry/stores/organizationsStore';
import {OrganizationStore} from 'sentry/stores/organizationStore';
import {ProjectsStore} from 'sentry/stores/projectsStore';
import {getUtcToLocalDateObject} from 'sentry/utils/dates';
import {localStorageWrapper} from 'sentry/utils/localStorage';

describe('PageFiltersContainer', () => {
Expand Down Expand Up @@ -561,6 +562,129 @@ describe('PageFiltersContainer', () => {
});
});

describe('maxDateRange param', () => {
it('resets period when maxDateRange appears and current selection exceeds it', async () => {
const {rerender} = render(<PageFiltersContainer maxPickableDays={30} />, {
organization,
initialRouterConfig: {
location: {
pathname: '/organizations/org-slug/test/',
query: {statsPeriod: '14d'},
},
route: '/organizations/:orgId/test/',
},
});

await waitFor(() =>
expect(PageFiltersStore.getState().selection.datetime).toEqual({
period: '14d',
utc: null,
start: null,
end: null,
})
);

rerender(<PageFiltersContainer maxPickableDays={30} maxDateRange={7} />);

await waitFor(() =>
expect(PageFiltersStore.getState().selection.datetime).toEqual({
period: '7d',
utc: null,
start: null,
end: null,
})
);
});

it('does not reset period when maxDateRange appears but selection is within it', async () => {
const {rerender} = render(<PageFiltersContainer maxPickableDays={30} />, {
organization,
initialRouterConfig: {
location: {
pathname: '/organizations/org-slug/test/',
query: {statsPeriod: '7d'},
},
route: '/organizations/:orgId/test/',
},
});

await waitFor(() =>
expect(PageFiltersStore.getState().selection.datetime).toEqual({
period: '7d',
utc: null,
start: null,
end: null,
})
);

rerender(<PageFiltersContainer maxPickableDays={30} maxDateRange={14} />);

await waitFor(() =>
expect(PageFiltersStore.getState().selection.datetime).toEqual({
period: '7d',
utc: null,
start: null,
end: null,
})
);
});

it('does not reset period when selection is within maxPickableDays and maxDateRange', async () => {
const start = moment().subtract(14, 'days').format('YYYY-MM-DDTHH:mm:ss');
const end = moment().subtract(8, 'days').format('YYYY-MM-DDTHH:mm:ss');
render(<PageFiltersContainer maxPickableDays={30} maxDateRange={7} />, {
organization,
initialRouterConfig: {
location: {
pathname: '/organizations/org-slug/test/',
query: {start, end},
},
route: '/organizations/:orgId/test/',
},
});

await waitFor(() =>
expect(PageFiltersStore.getState().selection.datetime).toEqual({
period: null,
utc: null,
start: getUtcToLocalDateObject(start),
end: getUtcToLocalDateObject(end),
})
);
});

it('resets absolute range when maxDateRange appears and range exceeds it', async () => {
const start = moment().subtract(10, 'days').format('YYYY-MM-DDTHH:mm:ss');
const end = moment().subtract(1, 'days').format('YYYY-MM-DDTHH:mm:ss');

const {rerender} = render(<PageFiltersContainer maxPickableDays={30} />, {
organization,
initialRouterConfig: {
location: {
pathname: '/organizations/org-slug/test/',
query: {start, end},
},
route: '/organizations/:orgId/test/',
},
});

await waitFor(() =>
expect(PageFiltersStore.getState().selection.datetime.period).toBeNull()
);

rerender(<PageFiltersContainer maxPickableDays={30} maxDateRange={7} />);

await waitFor(() =>
expect(PageFiltersStore.getState().selection.datetime).toEqual({
period: '7d',
utc: null,
start: null,
end: null,
})
);
});
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should a test be added in checking that absolute date ranges don't get reset as well, just a small regression test 👀

describe('skipInitializeUrlParams', () => {
const skipInitProjects = [
ProjectFixture({id: '1', slug: 'staging-project', environments: ['staging']}),
Expand Down
45 changes: 36 additions & 9 deletions static/app/components/pageFilters/container.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {usePageFilters} from 'sentry/components/pageFilters/usePageFilters';
import {parseStatsPeriod} from 'sentry/components/timeRangeSelector/utils';
import {DEFAULT_STATS_PERIOD} from 'sentry/constants';
import {statsPeriodToDays} from 'sentry/utils/duration/statsPeriodToDays';
import {DAY as DAY_IN_MS} from 'sentry/utils/formatters';
import {isActiveSuperuser} from 'sentry/utils/isActiveSuperuser';
import {useLocation} from 'sentry/utils/useLocation';
import {useDefaultMaxPickableDays} from 'sentry/utils/useMaxPickableDays';
Expand Down Expand Up @@ -58,6 +59,7 @@ export function PageFiltersContainer({
skipLoadLastUsed,
skipLoadLastUsedEnvironment,
maxPickableDays,
maxDateRange,
children,
...props
}: Props) {
Expand Down Expand Up @@ -103,6 +105,7 @@ export function PageFiltersContainer({
skipLoadLastUsed,
skipLoadLastUsedEnvironment,
maxPickableDays,
maxDateRange,
memberProjects,
nonMemberProjects,
defaultSelection,
Expand Down Expand Up @@ -132,20 +135,24 @@ export function PageFiltersContainer({
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [projectsLoaded]);

// Handle dynamic maxPickableDays changes (e.g., switching between pages with different limits).
// Handle dynamic maxPickableDays/maxDateRange changes (e.g., switching between pages with different limits).
// When the limit decreases and the current selection exceeds it, reset to the new max.
const previousMaxPickableDays = usePrevious(maxPickableDays);
const previousMaxDateRange = usePrevious(maxDateRange);
const shouldResetDateTime = useMemo(() => {
// Don't act until page filters are initialized - selection.datetime contains
// default values until isReady, not the actual URL state
if (!isReady) {
return false;
}

// Only act when maxPickableDays decreases (increasing the limit never invalidates selection)
const effectiveMaxDays = maxDateRange ?? maxPickableDays;
const previousEffectiveMaxDays = previousMaxDateRange ?? previousMaxPickableDays;

// Only act when the effective limit decreases (increasing the limit never invalidates selection)
if (
previousMaxPickableDays === maxPickableDays ||
previousMaxPickableDays < maxPickableDays
previousEffectiveMaxDays === effectiveMaxDays ||
previousEffectiveMaxDays < effectiveMaxDays
) {
return false;
}
Expand All @@ -154,19 +161,39 @@ export function PageFiltersContainer({

// For relative periods (e.g., "14d"), check if the period exceeds the new max
if (period) {
return statsPeriodToDays(period) > maxPickableDays;
Comment thread
nikkikapadia marked this conversation as resolved.
return statsPeriodToDays(period) > effectiveMaxDays;
}

// For absolute date ranges, check if the start date is before the allowed window.
// Uses same calculation as initialization in pageFilters.tsx
if (start && end) {
const periodStart = new Date(start);
const periodEnd = new Date(end);
const maxPeriod = parseStatsPeriod(`${maxPickableDays}d`);
const maxStart = new Date(maxPeriod.start);
return new Date(start).getTime() < maxStart.getTime();

if (maxDateRange) {
const maxTimeRange = maxDateRange * DAY_IN_MS;
return (
periodEnd.getTime() - periodStart.getTime() > maxTimeRange ||
periodStart.getTime() < maxStart.getTime()
);
}

return periodStart.getTime() < maxStart.getTime();
}

return false;
}, [isReady, maxPickableDays, previousMaxPickableDays, selection.datetime]);
}, [
isReady,
maxDateRange,
maxPickableDays,
previousMaxDateRange,
previousMaxPickableDays,
selection.datetime,
]);

const resetPeriodDays = maxDateRange ?? maxPickableDays;

useLayoutEffect(() => {
if (!shouldResetDateTime) {
Comment thread
sentry[bot] marked this conversation as resolved.
Expand All @@ -175,15 +202,15 @@ export function PageFiltersContainer({

// Reset to a relative period matching the new max (clears any absolute dates)
const newDateState = getDatetimeFromState({
period: `${maxPickableDays}d`,
period: `${resetPeriodDays}d`,
start: null,
end: null,
utc: selection.datetime.utc,
environment: [],
project: [],
});
updateDateTime(newDateState, location, navigate);
}, [maxPickableDays, location, navigate, selection.datetime.utc, shouldResetDateTime]);
}, [location, navigate, resetPeriodDays, selection.datetime.utc, shouldResetDateTime]);

// Update store persistence when `disablePersistence` changes
useEffect(() => updatePersistence(!disablePersistence), [disablePersistence]);
Expand Down
2 changes: 1 addition & 1 deletion static/app/components/timeRangeSelector/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ export const _timeRangeAutoCompleteFilter = function <T extends RelativeUnitsMap
timePeriodIsWithinLimit({
amount: userSuppliedAmount,
unit: matchingUnit,
maxDays,
maxDays: maxDateRange ?? maxDays,
supportedPeriods,
})
) {
Expand Down
Loading
Loading