Skip to content

Commit

Permalink
Merge branch 'master' into projects-filtering
Browse files Browse the repository at this point in the history
  • Loading branch information
Twixes committed Jul 11, 2024
2 parents 87f4c12 + 26f0438 commit dd8fdee
Show file tree
Hide file tree
Showing 174 changed files with 7,958 additions and 6,682 deletions.
20 changes: 9 additions & 11 deletions .github/workflows/ci-plugin-server.yml
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,11 @@ jobs:
if: needs.changes.outputs.plugin-server == 'true'
run: cd plugin-server && pnpm i

- name: Wait for Clickhouse & Kafka
- name: Wait for Clickhouse, Redis & Kafka
if: needs.changes.outputs.plugin-server == 'true'
run: bin/check_kafka_clickhouse_up
run: |
docker compose -f docker-compose.dev.yml up kafka redis clickhouse -d --wait
bin/check_kafka_clickhouse_up
- name: Set up databases
if: needs.changes.outputs.plugin-server == 'true'
Expand All @@ -173,24 +175,18 @@ jobs:
run: cd plugin-server && pnpm test -- --runInBand --forceExit tests/ --shard=${{matrix.shard}}

functional-tests:
name: Functional tests (POE=${{matrix.POE_EMBRACE_JOIN_FOR_TEAMS}},RDK=${{matrix.KAFKA_CONSUMPTION_USE_RDKAFKA}})
name: Functional tests
needs: changes
if: needs.changes.outputs.plugin-server == 'true'
runs-on: ubuntu-latest

strategy:
fail-fast: false
matrix:
POE_EMBRACE_JOIN_FOR_TEAMS: ['', '*']

env:
REDIS_URL: 'redis://localhost'
CLICKHOUSE_HOST: 'localhost'
CLICKHOUSE_DATABASE: 'posthog_test'
KAFKA_HOSTS: 'kafka:9092'
DATABASE_URL: 'postgres://posthog:posthog@localhost:5432/posthog'
RELOAD_PLUGIN_JITTER_MAX_MS: 0
POE_EMBRACE_JOIN_FOR_TEAMS: ${{matrix.POE_EMBRACE_JOIN_FOR_TEAMS}}

steps:
- name: Code check out
Expand Down Expand Up @@ -241,8 +237,10 @@ jobs:
pnpm install --frozen-lockfile
pnpm build
- name: Wait for Clickhouse & Kafka
run: bin/check_kafka_clickhouse_up
- name: Wait for Clickhouse, Redis & Kafka
run: |
docker compose -f docker-compose.dev.yml up kafka redis clickhouse -d --wait
bin/check_kafka_clickhouse_up
- name: Set up databases
env:
Expand Down
152 changes: 151 additions & 1 deletion cypress/e2e/surveys.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ describe('Surveys', () => {
cy.get('tbody').should('not.exist')
})

it('Delete survey', () => {
it('deletes a survey', () => {
cy.get('h1').should('contain', 'Surveys')
cy.get('[data-attr=new-survey]').click()
cy.get('[data-attr=new-blank-survey]').click()
Expand Down Expand Up @@ -278,4 +278,154 @@ describe('Surveys', () => {
cy.reload()
cy.contains('The survey will be stopped once 228 responses are received.').should('be.visible')
})

it('creates a new survey with branching logic', () => {
cy.window().then((win) => {
win.POSTHOG_APP_CONTEXT.current_user.organization.available_product_features = [
{
key: 'surveys_multiple_questions',
name: 'Multiple questions',
description: 'Ask up to 10 questions in a single survey.',
unit: null,
limit: null,
note: null,
},
]

cy.get('h1').should('contain', 'Surveys')
cy.title().should('equal', 'Surveys • PostHog')

cy.get('[data-attr="new-survey"]').click()
cy.get('[data-attr="new-blank-survey"]').click()
cy.get('[data-attr="survey-name"]').type(name)

// Prepare questions
cy.get('[data-attr=survey-question-label-0]')
.focus()
.clear()
.type('How happy are you?', { delay: 0 })
.should('have.value', 'How happy are you?')
cy.get('[data-attr=survey-question-type-0]').click()
cy.get('[data-attr=survey-question-type-0-rating]').click()
cy.get('[data-attr="add-question"]').click()

cy.get('[data-attr=survey-question-label-1]')
.focus()
.clear()
.type('Sorry to hear that. Please tell us more!', { delay: 0 })
.should('have.value', 'Sorry to hear that. Please tell us more!')
cy.get('[data-attr="add-question"]').click()

cy.get('[data-attr=survey-question-label-2]')
.focus()
.clear()
.type('Seems you are not completely happy. Please tell us more!', { delay: 0 })
.should('have.value', 'Seems you are not completely happy. Please tell us more!')
cy.get('[data-attr="add-question"]').click()

cy.get('[data-attr=survey-question-label-3]')
.focus()
.clear()
.type('Glad to hear that! Please tell us more', { delay: 0 })
.should('have.value', 'Glad to hear that! Please tell us more')
cy.get('[data-attr="add-question"]').click()

cy.get('[data-attr=survey-question-label-4]')
.focus()
.clear()
.type('Would you like to leave us a review?', { delay: 0 })
.should('have.value', 'Would you like to leave us a review?')
cy.get('[data-attr=survey-question-type-4]').click()
cy.get('[data-attr=survey-question-type-4-single_choice]').click()
cy.get('[data-attr="add-question"]').click()

cy.get('[data-attr=survey-question-label-5]')
.focus()
.clear()
.type('Please write your review here', { delay: 0 })
.should('have.value', 'Please write your review here')

// Set branching
// Question 1 - How happy are you?
cy.get('[data-attr=survey-question-panel-0]').click()
// Default branching is always "Next question"
cy.get('button[data-attr="survey-question-0-branching-select"]').find('span').contains('Next question')
cy.get('[data-attr=survey-question-0-branching-select]').click()
cy.get('.Popover__box button').contains('span', 'Specific question based on answer').click()
cy.get('button[data-attr="survey-question-0-branching-select"]')
.find('span')
.contains('Specific question based on answer')

cy.get('[data-attr=survey-question-0-branching-response_based-select-0]').click()
cy.get('.Popover__box button').contains('span', '2.').click()
cy.get('button[data-attr="survey-question-0-branching-response_based-select-0"]')
.find('span')
.contains('2.')

cy.get('[data-attr=survey-question-0-branching-response_based-select-1]').click()
cy.get('.Popover__box button').contains('span', '3.').click()
cy.get('button[data-attr="survey-question-0-branching-response_based-select-1"]')
.find('span')
.contains('3.')

cy.get('[data-attr=survey-question-0-branching-response_based-select-2]').click()
cy.get('.Popover__box button').contains('span', '4.').click()
cy.get('button[data-attr="survey-question-0-branching-response_based-select-2"]')
.find('span')
.contains('4.')

// Question 2 - Sorry to hear that...
cy.get('[data-attr=survey-question-panel-1]').click()
cy.get('[data-attr=survey-question-1-branching-select]').click()
cy.get('.Popover__box button').contains('span', 'Confirmation message').click()
cy.get('button[data-attr="survey-question-1-branching-select"]')
.find('span')
.contains('Confirmation message')

// Question 3 - Seems you are not completely happy...
cy.get('[data-attr=survey-question-panel-2]').click()
cy.get('[data-attr=survey-question-2-branching-select]').click()
cy.get('.Popover__box button').contains('span', 'Confirmation message').click()
cy.get('button[data-attr="survey-question-2-branching-select"]')
.find('span')
.contains('Confirmation message')

// Question 4 - Glad to hear that...
cy.get('[data-attr=survey-question-panel-3]').click()
cy.get('[data-attr=survey-question-3-branching-select]').click()
// "Would you like to leave us a review?"
cy.get('.Popover__box button').contains('span', '5.').click()
cy.get('button[data-attr="survey-question-3-branching-select"]').find('span').contains('5.')

// Question 5 - Would you like to leave us a review?
cy.get('[data-attr=survey-question-panel-4]').click()
cy.get('[data-attr=survey-question-4-branching-select]').click()
cy.get('.Popover__box button').contains('span', 'Specific question based on answer').click()
cy.get('button[data-attr="survey-question-4-branching-select"]')
.find('span')
.contains('Specific question based on answer')

cy.get('[data-attr=survey-question-4-branching-response_based-select-0]').click()
cy.get('.Popover__box button').contains('span', 'Next question').click()
cy.get('button[data-attr="survey-question-4-branching-response_based-select-0"]')
.find('span')
.contains('Next question')

cy.get('[data-attr=survey-question-4-branching-response_based-select-1]').click()
cy.get('.Popover__box button').contains('span', 'Confirmation message').click()
cy.get('button[data-attr="survey-question-4-branching-response_based-select-1"]')
.find('span')
.contains('Confirmation message')

// Question 6 - Please write your review here
cy.get('[data-attr=survey-question-panel-5]').click()
cy.get('button[data-attr="survey-question-5-branching-select"]')
.find('span')
.contains('Confirmation message')

// Save
cy.get('[data-attr="save-survey"]').eq(0).click()
cy.get('[data-attr=success-toast]').contains('created').should('exist')
})
})
})
10 changes: 10 additions & 0 deletions docker-compose.base.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ services:
image: redis:6.2.7-alpine
restart: on-failure
command: redis-server --maxmemory-policy allkeys-lru --maxmemory 200mb
healthcheck:
test: ['CMD', 'redis-cli', 'ping']
interval: 3s
timeout: 10s
retries: 10

clickhouse:
#
Expand All @@ -42,6 +47,11 @@ services:
KAFKA_CFG_ADVERTISED_LISTENERS: PLAINTEXT://kafka:9092
KAFKA_CFG_ZOOKEEPER_CONNECT: zookeeper:2181
ALLOW_PLAINTEXT_LISTENER: 'true'
healthcheck:
test: kafka-cluster.sh cluster-id --bootstrap-server localhost:9092 || exit 1
interval: 3s
timeout: 10s
retries: 10

kafka_ui:
image: provectuslabs/kafka-ui:latest
Expand Down
22 changes: 0 additions & 22 deletions ee/clickhouse/materialized_columns/analyze.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,6 @@ def event_properties(self, team_id: str) -> set[str]:
def person_on_events_properties(self, team_id: str) -> set[str]:
return self._get_properties(GET_EVENT_PROPERTIES_COUNT.format(column_name="person_properties"), team_id)

@instance_memoize
def group_on_events_properties(self, group_type_index: int, team_id: str) -> set[str]:
return self._get_properties(
GET_EVENT_PROPERTIES_COUNT.format(column_name=f"group{group_type_index}_properties"),
team_id,
)

def _get_properties(self, query, team_id) -> set[str]:
rows = sync_execute(query, {"team_id": team_id})
return {name for name, _ in rows}
Expand Down Expand Up @@ -99,11 +92,6 @@ def properties(
person_props = team_manager.person_properties(self.team_id)
event_props = team_manager.event_properties(self.team_id)
person_on_events_props = team_manager.person_on_events_properties(self.team_id)
group0_props = team_manager.group_on_events_properties(0, self.team_id)
group1_props = team_manager.group_on_events_properties(1, self.team_id)
group2_props = team_manager.group_on_events_properties(2, self.team_id)
group3_props = team_manager.group_on_events_properties(3, self.team_id)
group4_props = team_manager.group_on_events_properties(4, self.team_id)

for table_column, property in self._all_properties:
if property in event_props:
Expand All @@ -113,16 +101,6 @@ def properties(

if property in person_on_events_props and "person_properties" in table_column:
yield "events", "person_properties", property
if property in group0_props and "group0_properties" in table_column:
yield "events", "group0_properties", property
if property in group1_props and "group1_properties" in table_column:
yield "events", "group1_properties", property
if property in group2_props and "group2_properties" in table_column:
yield "events", "group2_properties", property
if property in group3_props and "group3_properties" in table_column:
yield "events", "group3_properties", property
if property in group4_props and "group4_properties" in table_column:
yield "events", "group4_properties", property


def _analyze(since_hours_ago: int, min_query_time: int, team_id: Optional[int] = None) -> list[Suggestion]:
Expand Down
2 changes: 1 addition & 1 deletion ee/clickhouse/materialized_columns/columns.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ def _materialized_column_name(
) -> str:
"Returns a sanitized and unique column name to use for materialized column"

prefix = "mat_" if table == "events" or table == "groups" else "pmat_"
prefix = "pmat_" if table == "person" else "mat_"

if table_column != DEFAULT_TABLE_COLUMN:
prefix += f"{SHORT_TABLE_COLUMN_NAME[table_column]}_"
Expand Down
19 changes: 0 additions & 19 deletions ee/clickhouse/queries/column_optimizer.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from collections import Counter as TCounter
from typing import cast

from posthog.clickhouse.materialized_columns.column import ColumnName
from posthog.constants import TREND_FILTER_TYPE_ACTIONS, FunnelCorrelationType
from posthog.models.action.util import get_action_tables_and_properties
from posthog.models.filters.mixins.utils import cached_property
Expand All @@ -24,24 +23,6 @@ def group_types_to_query(self) -> set[GroupTypeIndex]:
used_properties = self.used_properties_with_type("group")
return {cast(GroupTypeIndex, group_type_index) for _, _, group_type_index in used_properties}

@cached_property
def group_on_event_columns_to_query(self) -> set[ColumnName]:
"Returns a list of event table group columns containing materialized properties that this query needs"
used_properties = self.used_properties_with_type("group")

columns_to_query: set[ColumnName] = set()

for group_type_index in range(5):
columns_to_query = columns_to_query.union(
self.columns_to_query(
"events",
{property for property in used_properties if property[2] == group_type_index},
f"group{group_type_index}_properties",
)
)

return columns_to_query

@cached_property
def properties_used_in_filter(self) -> TCounter[PropertyIdentifier]:
"Returns collection of properties + types that this query would use"
Expand Down
5 changes: 0 additions & 5 deletions ee/clickhouse/queries/funnels/test/breakdown_cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
)
from posthog.test.base import (
APIBaseTest,
also_test_with_materialized_columns,
snapshot_clickhouse_queries,
also_test_with_person_on_events_v2,
)
Expand Down Expand Up @@ -307,10 +306,6 @@ def test_funnel_aggregate_by_groups_breakdown_group(self):
],
)

@also_test_with_materialized_columns(
group_properties=[(0, "industry")],
materialize_only_with_person_on_events=True,
)
@also_test_with_person_on_events_v2
@snapshot_clickhouse_queries
def test_funnel_aggregate_by_groups_breakdown_group_person_on_events(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,6 @@ def test_funnel_correlation_with_properties_and_groups(self):
@also_test_with_materialized_columns(
event_properties=[],
person_properties=["$browser"],
group_properties=[(0, "industry")],
verify_no_jsonextract=False,
)
@also_test_with_person_on_events_v2
Expand Down
Loading

0 comments on commit dd8fdee

Please sign in to comment.