Skip to content

Commit

Permalink
chore: Use DateRange everywhere instead of InsightDateRange
Browse files Browse the repository at this point in the history 
These are basically the same, the only difference is the default. Let's use `DateRange` without the default everywhere. If this proves to be a problem (too many broken tests) we can either:?
1. revert this
2. try to use it with the default on the places where we were not using a default before

fix: Remove last references to `InsightDateRange`
  • Loading branch information
@rafaeelaudibert
rafaeelaudibert committed Dec 16, 2024
1 parent 7ac990d commit b4a59fa
Show file tree
Hide file tree
Showing 28 changed files with 209 additions and 312 deletions.
62 changes: 14 additions & 48 deletions frontend/src/queries/schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,7 @@
"description": "Breakdown the chart by a property"
},
"dateRange": {
"$ref": "#/definitions/AssistantInsightDateRange",
"$ref": "#/definitions/DateRange",
"description": "Date range for the query"
},
"filterTestAccounts": {
Expand Down Expand Up @@ -1027,27 +1027,11 @@
}
]
},
"AssistantInsightDateRange": {
"additionalProperties": false,
"properties": {
"date_from": {
"default": "-7d",
"description": "Start date. The value can be:\n- a relative date. Examples of relative dates are: `-1y` for 1 year ago, `-14m` for 14 months ago, `-1w` for 1 week ago, `-14d` for 14 days ago, `-30h` for 30 hours ago.\n- an absolute ISO 8601 date string. a constant `yStart` for the current year start. a constant `mStart` for the current month start. a constant `dStart` for the current day start. Prefer using relative dates.",
"type": ["string", "null"]
},
"date_to": {
"default": null,
"description": "Right boundary of the date range. Use `null` for the current date. You can not use relative dates here.",
"type": ["string", "null"]
}
},
"type": "object"
},
"AssistantInsightsQueryBase": {
"additionalProperties": false,
"properties": {
"dateRange": {
"$ref": "#/definitions/AssistantInsightDateRange",
"$ref": "#/definitions/DateRange",
"description": "Date range for the query"
},
"filterTestAccounts": {
Expand Down Expand Up @@ -1349,7 +1333,7 @@
"description": "Compare to date range"
},
"dateRange": {
"$ref": "#/definitions/AssistantInsightDateRange",
"$ref": "#/definitions/DateRange",
"description": "Date range for the query"
},
"filterTestAccounts": {
Expand Down Expand Up @@ -6860,7 +6844,7 @@
"description": "Breakdown of the events and actions"
},
"dateRange": {
"$ref": "#/definitions/InsightDateRange",
"$ref": "#/definitions/DateRange",
"description": "Date range for the query"
},
"filterTestAccounts": {
Expand Down Expand Up @@ -7773,24 +7757,6 @@
},
"type": "object"
},
"InsightDateRange": {
"additionalProperties": false,
"properties": {
"date_from": {
"default": "-7d",
"type": ["string", "null"]
},
"date_to": {
"type": ["string", "null"]
},
"explicitDate": {
"default": false,
"description": "Whether the date_from and date_to should be used verbatim. Disables rounding to the start and end of period.",
"type": ["boolean", "null"]
}
},
"type": "object"
},
"InsightFilter": {
"anyOf": [
{
Expand Down Expand Up @@ -7953,7 +7919,7 @@
"description": "Groups aggregation"
},
"dateRange": {
"$ref": "#/definitions/InsightDateRange",
"$ref": "#/definitions/DateRange",
"description": "Date range for the query"
},
"filterTestAccounts": {
Expand Down Expand Up @@ -8010,7 +7976,7 @@
"description": "Groups aggregation"
},
"dateRange": {
"$ref": "#/definitions/InsightDateRange",
"$ref": "#/definitions/DateRange",
"description": "Date range for the query"
},
"filterTestAccounts": {
Expand Down Expand Up @@ -8067,7 +8033,7 @@
"description": "Groups aggregation"
},
"dateRange": {
"$ref": "#/definitions/InsightDateRange",
"$ref": "#/definitions/DateRange",
"description": "Date range for the query"
},
"filterTestAccounts": {
Expand Down Expand Up @@ -8124,7 +8090,7 @@
"description": "Groups aggregation"
},
"dateRange": {
"$ref": "#/definitions/InsightDateRange",
"$ref": "#/definitions/DateRange",
"description": "Date range for the query"
},
"filterTestAccounts": {
Expand Down Expand Up @@ -8181,7 +8147,7 @@
"description": "Groups aggregation"
},
"dateRange": {
"$ref": "#/definitions/InsightDateRange",
"$ref": "#/definitions/DateRange",
"description": "Date range for the query"
},
"filterTestAccounts": {
Expand Down Expand Up @@ -8291,7 +8257,7 @@
"description": "Groups aggregation"
},
"dateRange": {
"$ref": "#/definitions/InsightDateRange",
"$ref": "#/definitions/DateRange",
"description": "Date range for the query"
},
"filterTestAccounts": {
Expand Down Expand Up @@ -8700,7 +8666,7 @@
"description": "Groups aggregation"
},
"dateRange": {
"$ref": "#/definitions/InsightDateRange",
"$ref": "#/definitions/DateRange",
"description": "Date range for the query"
},
"filterTestAccounts": {
Expand Down Expand Up @@ -11468,7 +11434,7 @@
"description": "Groups aggregation"
},
"dateRange": {
"$ref": "#/definitions/InsightDateRange",
"$ref": "#/definitions/DateRange",
"description": "Date range for the query"
},
"filterTestAccounts": {
Expand Down Expand Up @@ -12140,7 +12106,7 @@
"description": "Compare to date range"
},
"dateRange": {
"$ref": "#/definitions/InsightDateRange",
"$ref": "#/definitions/DateRange",
"description": "Date range for the query"
},
"filterTestAccounts": {
Expand Down Expand Up @@ -12691,7 +12657,7 @@
"description": "Whether we should be comparing against a specific conversion goal"
},
"dateRange": {
"$ref": "#/definitions/InsightDateRange",
"$ref": "#/definitions/DateRange",
"description": "Date range for the query"
},
"filterTestAccounts": {
Expand Down
35 changes: 2 additions & 33 deletions frontend/src/queries/schema.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,7 @@ interface InsightVizNodeViewProps {
/** Base class for insight query nodes. Should not be used directly. */
export interface InsightsQueryBase<R extends AnalyticsQueryResponseBase<any>> extends Node<R> {
/** Date range for the query */
dateRange?: InsightDateRange
dateRange?: DateRange
/**
* Exclude internal and test users by applying the respective filters
*
Expand Down Expand Up @@ -1004,31 +1004,11 @@ export type AssistantGroupPropertyFilter = AssistantBasePropertyFilter & {

export type AssistantPropertyFilter = AssistantGenericPropertyFilter | AssistantGroupPropertyFilter

export interface AssistantInsightDateRange {
/**
* Start date. The value can be:
* - a relative date. Examples of relative dates are: `-1y` for 1 year ago, `-14m` for 14 months ago, `-1w` for 1 week ago, `-14d` for 14 days ago, `-30h` for 30 hours ago.
* - an absolute ISO 8601 date string.
* a constant `yStart` for the current year start.
* a constant `mStart` for the current month start.
* a constant `dStart` for the current day start.
* Prefer using relative dates.
* @default -7d
*/
date_from?: string | null

/**
* Right boundary of the date range. Use `null` for the current date. You can not use relative dates here.
* @default null
*/
date_to?: string | null
}

export interface AssistantInsightsQueryBase {
/**
* Date range for the query
*/
dateRange?: AssistantInsightDateRange
dateRange?: DateRange

/**
* Exclude internal and test users by applying the respective filters
Expand Down Expand Up @@ -2319,17 +2299,6 @@ export interface DateRange {
explicitDate?: boolean | null
}

export interface InsightDateRange {
/** @default -7d */
date_from?: string | null
date_to?: string | null
/** Whether the date_from and date_to should be used verbatim. Disables
* rounding to the start and end of period.
* @default false
* */
explicitDate?: boolean | null
}

export type MultipleBreakdownType = Extract<BreakdownType, 'person' | 'event' | 'group' | 'session' | 'hogql'>

export interface Breakdown {
Expand Down
4 changes: 2 additions & 2 deletions posthog/hogql/database/schema/test/test_persons.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
TrendsQuery,
ActorsQuery,
EventsNode,
InsightDateRange,
DateRange,
)
from posthog.hogql_queries.actors_query_runner import ActorsQueryRunner
from posthog.hogql.modifiers import create_default_modifiers_for_team
Expand Down Expand Up @@ -102,7 +102,7 @@ def test_joins_are_left_alone_for_now(self):
def test_person_modal_not_optimized_yet(self):
source_query = TrendsQuery(
series=[EventsNode(event="$pageview")],
dateRange=InsightDateRange(date_from="2024-01-01", date_to="2024-01-07"),
dateRange=DateRange(date_from="2024-01-01", date_to="2024-01-07"),
# breakdownFilter=BreakdownFilter(breakdown="$", breakdown_type=BreakdownType.PERSON),
)
insight_actors_query = InsightActorsQuery(
Expand Down
4 changes: 2 additions & 2 deletions posthog/hogql_queries/experiments/experiment_funnels_query_runner.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
FunnelsFilter,
FunnelsQuery,
FunnelsQueryResponse,
InsightDateRange,
DateRange,
BreakdownFilter,
)
from typing import Optional, Any, cast
Expand Down Expand Up @@ -105,7 +105,7 @@ def _prepare_funnel_query(self) -> FunnelsQuery:
start_date = self.experiment.start_date
end_date = self.experiment.end_date

prepared_funnels_query.dateRange = InsightDateRange(
prepared_funnels_query.dateRange = DateRange(
date_from=start_date.isoformat() if start_date else None,
date_to=end_date.isoformat() if end_date else None,
explicitDate=True,
Expand Down
16 changes: 8 additions & 8 deletions posthog/hogql_queries/experiments/experiment_trends_query_runner.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
ExperimentTrendsQuery,
ExperimentTrendsQueryResponse,
ExperimentVariantTrendsBaseStats,
InsightDateRange,
DateRange,
PropertyMathType,
PropertyOperator,
TrendsFilter,
Expand Down Expand Up @@ -80,9 +80,9 @@ def _uses_math_aggregation_by_user_or_property_value(self, query: TrendsQuery):
math_keys.remove("sum")
return any(entity.math in math_keys for entity in query.series)

def _get_insight_date_range(self) -> InsightDateRange:
def _get_date_range(self) -> DateRange:
"""
Returns an InsightDateRange object based on the experiment's start and end dates,
Returns an DateRange object based on the experiment's start and end dates,
adjusted for the team's timezone if applicable.
"""
if self.team.timezone:
Expand All @@ -93,7 +93,7 @@ def _get_insight_date_range(self) -> InsightDateRange:
start_date = self.experiment.start_date
end_date = self.experiment.end_date

return InsightDateRange(
return DateRange(
date_from=start_date.isoformat() if start_date else None,
date_to=end_date.isoformat() if end_date else None,
explicitDate=True,
Expand Down Expand Up @@ -133,7 +133,7 @@ def _prepare_count_query(self) -> TrendsQuery:
prepared_count_query.series[0].math = None

prepared_count_query.trendsFilter = TrendsFilter(display=ChartDisplayType.ACTIONS_LINE_GRAPH_CUMULATIVE)
prepared_count_query.dateRange = self._get_insight_date_range()
prepared_count_query.dateRange = self._get_date_range()
if self._is_data_warehouse_query(prepared_count_query):
prepared_count_query.breakdownFilter = self._get_data_warehouse_breakdown_filter()
prepared_count_query.properties = [
Expand Down Expand Up @@ -182,7 +182,7 @@ def _prepare_exposure_query(self) -> TrendsQuery:

if uses_math_aggregation:
prepared_exposure_query = prepared_count_query
prepared_exposure_query.dateRange = self._get_insight_date_range()
prepared_exposure_query.dateRange = self._get_date_range()
prepared_exposure_query.trendsFilter = TrendsFilter(display=ChartDisplayType.ACTIONS_LINE_GRAPH_CUMULATIVE)

# For a data warehouse query, we can use the unique users for the series
Expand Down Expand Up @@ -229,7 +229,7 @@ def _prepare_exposure_query(self) -> TrendsQuery:
# 2. Otherwise, if an exposure query is provided, we use it as is, adapting the date range and breakdown
elif self.query.exposure_query and not self._is_data_warehouse_query(prepared_count_query):
prepared_exposure_query = TrendsQuery(**self.query.exposure_query.model_dump())
prepared_exposure_query.dateRange = self._get_insight_date_range()
prepared_exposure_query.dateRange = self._get_date_range()
prepared_exposure_query.trendsFilter = TrendsFilter(display=ChartDisplayType.ACTIONS_LINE_GRAPH_CUMULATIVE)
prepared_exposure_query.breakdownFilter = self._get_event_breakdown_filter()
prepared_exposure_query.properties = [
Expand All @@ -243,7 +243,7 @@ def _prepare_exposure_query(self) -> TrendsQuery:
# 3. Otherwise, we construct a default exposure query: unique users for the $feature_flag_called event
else:
prepared_exposure_query = TrendsQuery(
dateRange=self._get_insight_date_range(),
dateRange=self._get_date_range(),
trendsFilter=TrendsFilter(display=ChartDisplayType.ACTIONS_LINE_GRAPH_CUMULATIVE),
breakdownFilter=BreakdownFilter(
breakdown="$feature_flag_response",
Expand Down
Loading

0 comments on commit b4a59fa

Please sign in to comment.