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
  • Loading branch information
@rafaeelaudibert
rafaeelaudibert committed Dec 16, 2024
1 parent 7ac990d commit a216659
Show file tree
Hide file tree
Showing 26 changed files with 205 additions and 308 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
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 a216659

Please sign in to comment.