mirror of
https://github.com/PostHog/posthog.git
synced 2024-11-21 21:49:51 +01:00
fix(data-management): Fix event definition ordering (#13362)
* fix(data-management): Fix event definition ordering * fix test
This commit is contained in:
parent
4f2b0b549f
commit
bed31b8dc5
@ -64,7 +64,7 @@ export function EventDefinitionsTable(): JSX.Element {
|
||||
render: function Render(_, definition: EventDefinition) {
|
||||
return <EventDefinitionHeader definition={definition} hideIcon asLink />
|
||||
},
|
||||
sorter: (a, b) => a.name?.localeCompare(b.name ?? '') ?? 0,
|
||||
sorter: true,
|
||||
},
|
||||
...(hasDashboardCollaboration
|
||||
? [
|
||||
@ -90,7 +90,7 @@ export function EventDefinitionsTable(): JSX.Element {
|
||||
<span className="text-muted">—</span>
|
||||
)
|
||||
},
|
||||
sorter: (a, b) => (a?.volume_30_day ?? 0) - (b?.volume_30_day ?? 0),
|
||||
sorter: true,
|
||||
} as LemonTableColumn<EventDefinition, keyof EventDefinition | undefined>,
|
||||
{
|
||||
title: <ThirtyDayQueryCountTitle tooltipPlacement="bottom" />,
|
||||
@ -103,7 +103,7 @@ export function EventDefinitionsTable(): JSX.Element {
|
||||
<span className="text-muted">—</span>
|
||||
)
|
||||
},
|
||||
sorter: (a, b) => (a?.query_usage_30_day ?? 0) - (b?.query_usage_30_day ?? 0),
|
||||
sorter: true,
|
||||
} as LemonTableColumn<EventDefinition, keyof EventDefinition | undefined>,
|
||||
]
|
||||
: []),
|
||||
@ -198,6 +198,13 @@ export function EventDefinitionsTable(): JSX.Element {
|
||||
}
|
||||
: undefined,
|
||||
}}
|
||||
onSort={(newSorting) =>
|
||||
setFilters({
|
||||
ordering: newSorting
|
||||
? `${newSorting.order === -1 ? '-' : ''}${newSorting.columnKey}`
|
||||
: undefined,
|
||||
})
|
||||
}
|
||||
expandable={{
|
||||
expandedRowRender: function RenderPropertiesTable(definition) {
|
||||
return <EventDefinitionProperties definition={definition} />
|
||||
@ -206,6 +213,7 @@ export function EventDefinitionsTable(): JSX.Element {
|
||||
noIndent: true,
|
||||
}}
|
||||
dataSource={eventDefinitions.results}
|
||||
useURLForSorting={false}
|
||||
emptyState="No event definitions"
|
||||
nouns={['event', 'events']}
|
||||
/>
|
||||
|
@ -25,6 +25,7 @@ export interface Filters {
|
||||
event: string
|
||||
properties: AnyPropertyFilter[]
|
||||
event_type: EventDefinitionType
|
||||
ordering?: string
|
||||
}
|
||||
|
||||
function cleanFilters(filter: Partial<Filters>): Filters {
|
||||
@ -32,6 +33,7 @@ function cleanFilters(filter: Partial<Filters>): Filters {
|
||||
event: '',
|
||||
properties: [],
|
||||
event_type: EventDefinitionType.Event,
|
||||
ordering: '-volume_30_day',
|
||||
...filter,
|
||||
}
|
||||
}
|
||||
@ -299,7 +301,7 @@ export const eventDefinitionsTableLogic = kea<eventDefinitionsTableLogicType>([
|
||||
actions.loadEventDefinitions(
|
||||
normalizeEventDefinitionEndpointUrl({
|
||||
url: values.eventDefinitions.current,
|
||||
searchParams: { search: values.filters.event },
|
||||
searchParams: { search: values.filters.event, ordering: values.filters.ordering },
|
||||
full: true,
|
||||
eventTypeFilter: values.filters.event_type,
|
||||
})
|
||||
|
@ -250,7 +250,10 @@
|
||||
"lint-staged": {
|
||||
"*.{js,jsx,mjs,ts,tsx,json,yaml,yml,css,scss}": "prettier --write",
|
||||
"((frontend|cypress)/**).{js,jsx,mjs,ts,tsx}": "eslint -c .eslintrc.js --fix",
|
||||
"(plugin-server/**).{js,jsx,mjs,ts,tsx}": "eslint -c plugin-server/.eslintrc.js --fix",
|
||||
"(plugin-server/**).{js,jsx,mjs,ts,tsx}": [
|
||||
"eslint -c plugin-server/.eslintrc.js --fix",
|
||||
"prettier --write"
|
||||
],
|
||||
"*.{py,pyi}": [
|
||||
"black",
|
||||
"flake8",
|
||||
|
@ -65,7 +65,9 @@ class EventDefinitionViewSet(
|
||||
permission_classes = [permissions.IsAuthenticated, OrganizationMemberPermissions, TeamMemberAccessPermission]
|
||||
lookup_field = "id"
|
||||
filter_backends = [TermSearchFilterBackend]
|
||||
|
||||
search_fields = ["name"]
|
||||
ordering_fields = ["volume_30_day", "query_usage_30_day", "name"]
|
||||
|
||||
def get_queryset(self):
|
||||
# `type` = 'all' | 'event' | 'action_event'
|
||||
@ -76,12 +78,24 @@ class EventDefinitionViewSet(
|
||||
search_query, search_kwargs = term_search_filter_sql(self.search_fields, search)
|
||||
|
||||
params = {"team_id": self.team_id, "is_posthog_event": "$%", **search_kwargs}
|
||||
if self.request.GET.get("ordering") and self.request.GET["ordering"].replace("-", "") in self.ordering_fields:
|
||||
order = self.request.GET["ordering"].replace("-", "")
|
||||
order_direction = "DESC" if "-" in self.request.GET["ordering"] else "ASC"
|
||||
else:
|
||||
order = "volume_30_day"
|
||||
order_direction = "DESC"
|
||||
|
||||
if EE_AVAILABLE and self.request.user.organization.is_feature_available(AvailableFeature.INGESTION_TAXONOMY): # type: ignore
|
||||
from ee.models.event_definition import EnterpriseEventDefinition
|
||||
|
||||
# Prevent fetching deprecated `tags` field. Tags are separately fetched in TaggedItemSerializerMixin
|
||||
sql = create_event_definitions_sql(event_type, is_enterprise=True, conditions=search_query)
|
||||
sql = create_event_definitions_sql(
|
||||
event_type,
|
||||
is_enterprise=True,
|
||||
conditions=search_query,
|
||||
order_UNSAFE=order,
|
||||
order_direction=order_direction,
|
||||
)
|
||||
|
||||
ee_event_definitions = EnterpriseEventDefinition.objects.raw(sql, params=params)
|
||||
ee_event_definitions_list = ee_event_definitions.prefetch_related(
|
||||
@ -90,7 +104,13 @@ class EventDefinitionViewSet(
|
||||
|
||||
return ee_event_definitions_list
|
||||
|
||||
sql = create_event_definitions_sql(event_type, is_enterprise=False, conditions=search_query)
|
||||
sql = create_event_definitions_sql(
|
||||
event_type,
|
||||
is_enterprise=False,
|
||||
conditions=search_query,
|
||||
order_UNSAFE=order,
|
||||
order_direction=order_direction,
|
||||
)
|
||||
event_definitions_list = EventDefinition.objects.raw(sql, params=params)
|
||||
|
||||
return event_definitions_list
|
||||
|
@ -73,6 +73,12 @@ class TestEventDefinitionAPI(APIBaseTest):
|
||||
(dateutil.parser.isoparse(response_item["created_at"]) - timezone.now()).total_seconds(), 0
|
||||
)
|
||||
|
||||
# Test ordering
|
||||
response = self.client.get("/api/projects/@current/event_definitions/?ordering=volume_30_day")
|
||||
|
||||
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
||||
self.assertEqual(response.json()["results"][0]["volume_30_day"], 1)
|
||||
|
||||
def test_pagination_of_event_definitions(self):
|
||||
EventDefinition.objects.bulk_create(
|
||||
[EventDefinition(team=self.demo_team, name=f"z_event_{i}") for i in range(1, 301)]
|
||||
@ -82,8 +88,8 @@ class TestEventDefinitionAPI(APIBaseTest):
|
||||
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
||||
self.assertEqual(response.json()["count"], 306)
|
||||
self.assertEqual(len(response.json()["results"]), 100) # Default page size
|
||||
self.assertEqual(response.json()["results"][0]["name"], "$pageview") # Order by name (ascending)
|
||||
self.assertEqual(response.json()["results"][1]["name"], "entered_free_trial") # Order by name (ascending)
|
||||
self.assertEqual(response.json()["results"][0]["name"], "$pageview") # Order by volume (desc)
|
||||
self.assertEqual(response.json()["results"][1]["name"], "watched_movie") # Order by volume (desc)
|
||||
|
||||
event_checkpoints = [
|
||||
184,
|
||||
|
@ -41,12 +41,22 @@ def get_target_entity(filter: Union[Filter, StickinessFilter]) -> Entity:
|
||||
return possible_entity
|
||||
|
||||
possible_entity = retrieve_entity_from(
|
||||
filter.target_entity_id, filter.target_entity_type, entity_math, filter.events, filter.actions
|
||||
filter.target_entity_id,
|
||||
filter.target_entity_type,
|
||||
entity_math,
|
||||
filter.events,
|
||||
filter.actions,
|
||||
)
|
||||
if possible_entity:
|
||||
return possible_entity
|
||||
elif filter.target_entity_type:
|
||||
return Entity({"id": filter.target_entity_id, "type": filter.target_entity_type, "math": entity_math})
|
||||
return Entity(
|
||||
{
|
||||
"id": filter.target_entity_id,
|
||||
"type": filter.target_entity_type,
|
||||
"math": entity_math,
|
||||
}
|
||||
)
|
||||
else:
|
||||
raise ValidationError("An entity must be provided for target entity to be determined")
|
||||
|
||||
@ -62,7 +72,11 @@ def entity_from_order(order: Optional[str], entities: List[Entity]) -> Optional[
|
||||
|
||||
|
||||
def retrieve_entity_from(
|
||||
entity_id: str, entity_type: Optional[str], entity_math: MathType, events: List[Entity], actions: List[Entity]
|
||||
entity_id: str,
|
||||
entity_type: Optional[str],
|
||||
entity_math: MathType,
|
||||
events: List[Entity],
|
||||
actions: List[Entity],
|
||||
) -> Optional[Entity]:
|
||||
"""
|
||||
Retrieves the entity from the events and actions.
|
||||
@ -158,7 +172,11 @@ def get_data(request):
|
||||
None,
|
||||
cors_response(
|
||||
request,
|
||||
generate_exception_response("capture", f"Malformed request data: {error}", code="invalid_payload"),
|
||||
generate_exception_response(
|
||||
"capture",
|
||||
f"Malformed request data: {error}",
|
||||
code="invalid_payload",
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
@ -222,7 +240,10 @@ def get_event_ingestion_context(
|
||||
error_response = cors_response(
|
||||
request,
|
||||
generate_exception_response(
|
||||
"capture", "Invalid Project ID.", code="invalid_project", attr="project_id"
|
||||
"capture",
|
||||
"Invalid Project ID.",
|
||||
code="invalid_project",
|
||||
attr="project_id",
|
||||
),
|
||||
)
|
||||
return None, db_error, error_response
|
||||
@ -272,7 +293,9 @@ def get_event_ingestion_context(
|
||||
return ingestion_context, db_error, error_response
|
||||
|
||||
|
||||
def get_event_ingestion_context_for_token(token: str) -> Optional[EventIngestionContext]:
|
||||
def get_event_ingestion_context_for_token(
|
||||
token: str,
|
||||
) -> Optional[EventIngestionContext]:
|
||||
"""
|
||||
Based on a token associated with a Team, retrieve the context that is
|
||||
required to ingest events.
|
||||
@ -333,7 +356,11 @@ def safe_clickhouse_string(s: str) -> str:
|
||||
|
||||
|
||||
def create_event_definitions_sql(
|
||||
event_type: EventDefinitionType, is_enterprise: bool = False, conditions: str = ""
|
||||
event_type: EventDefinitionType,
|
||||
is_enterprise: bool = False,
|
||||
conditions: str = "",
|
||||
order_UNSAFE: str = "",
|
||||
order_direction: str = "DESC",
|
||||
) -> str:
|
||||
# Prevent fetching deprecated `tags` field. Tags are separately fetched in TaggedItemSerializerMixin
|
||||
if is_enterprise:
|
||||
@ -364,11 +391,10 @@ def create_event_definitions_sql(
|
||||
|
||||
# Only return event definitions
|
||||
raw_event_definition_fields = ",".join(event_definition_fields)
|
||||
ordering = (
|
||||
"ORDER BY last_seen_at DESC NULLS LAST, query_usage_30_day DESC NULLS LAST, name ASC"
|
||||
if is_enterprise
|
||||
else "ORDER BY name ASC"
|
||||
provided_ordering = (
|
||||
f"{order_UNSAFE} {order_direction} {'NULLS FIRST' if order_direction == 'ASC' else 'NULLS LAST'}"
|
||||
)
|
||||
ordering = f"ORDER BY {provided_ordering}, name ASC"
|
||||
|
||||
if event_type == EventDefinitionType.EVENT_CUSTOM:
|
||||
shared_conditions += " AND posthog_eventdefinition.name NOT LIKE %(is_posthog_event)s"
|
||||
|
Loading…
Reference in New Issue
Block a user