diff --git a/latest_migrations.manifest b/latest_migrations.manifest index 9f1eb8998d4..dd3901be254 100644 --- a/latest_migrations.manifest +++ b/latest_migrations.manifest @@ -5,7 +5,7 @@ contenttypes: 0002_remove_content_type_name ee: 0016_rolemembership_organization_member otp_static: 0002_throttling otp_totp: 0002_auto_20190420_0723 -posthog: 0504_add_dead_clicks_setting +posthog: 0505_grouptypemapping_project sessions: 0001_initial social_django: 0010_uid_db_index two_factor: 0007_auto_20201201_1019 diff --git a/plugin-server/functional_tests/api.ts b/plugin-server/functional_tests/api.ts index 2999170b38c..4df5eff0701 100644 --- a/plugin-server/functional_tests/api.ts +++ b/plugin-server/functional_tests/api.ts @@ -311,14 +311,14 @@ export const fetchGroups = async (teamId: number) => { return queryResult.data.map((group) => ({ ...group, group_properties: JSON.parse(group.group_properties) })) } -export const createGroupType = async (teamId: number, index: number, groupType: string) => { +export const createGroupType = async (teamId: number, projectId: number, index: number, groupType: string) => { await postgres.query( PostgresUse.COMMON_WRITE, ` - INSERT INTO posthog_grouptypemapping (team_id, group_type, group_type_index) - VALUES ($1, $2, $3) + INSERT INTO posthog_grouptypemapping (team_id, project_id, group_type, group_type_index) + VALUES ($1, $2, $3, $4) `, - [teamId, groupType, index], + [teamId, projectId, groupType, index], 'insertGroupType' ) } @@ -455,7 +455,7 @@ export const createOrganizationRaw = async (organizationProperties = {}) => { await postgres.query( PostgresUse.COMMON_WRITE, - `INSERT into posthog_organization + `INSERT into posthog_organization (${keys}) VALUES (${values}) `, diff --git a/plugin-server/functional_tests/webhooks.test.ts b/plugin-server/functional_tests/webhooks.test.ts index 4e26055c8de..7acd5a8448b 100644 --- a/plugin-server/functional_tests/webhooks.test.ts +++ b/plugin-server/functional_tests/webhooks.test.ts @@ -45,7 +45,7 @@ test.concurrent(`webhooks: fires slack webhook`, async () => { }) const teamId = await createTeam(organizationId, `http://localhost:${server.address()?.port}`) const user = await createUser(teamId, new UUIDT().toString()) - await createGroupType(teamId, 0, 'organization') + await createGroupType(teamId, teamId, 0, 'organization') await createGroup(teamId, 0, 'TestWebhookOrg', { name: 'test-webhooks' }) const action = await createAction({ team_id: teamId, diff --git a/plugin-server/src/types.ts b/plugin-server/src/types.ts index 9ae7756af0f..42d31b00ade 100644 --- a/plugin-server/src/types.ts +++ b/plugin-server/src/types.ts @@ -620,6 +620,7 @@ export interface RawOrganization { /** Usable Team model. */ export interface Team { id: number + project_id: number uuid: string organization_id: string name: string diff --git a/plugin-server/src/utils/db/db.ts b/plugin-server/src/utils/db/db.ts index 84e8a98675a..04fad93de51 100644 --- a/plugin-server/src/utils/db/db.ts +++ b/plugin-server/src/utils/db/db.ts @@ -1342,14 +1342,18 @@ export class DB { } public async getTeamsInOrganizationsWithRootPluginAccess(): Promise { - return ( - await this.postgres.query( - PostgresUse.COMMON_READ, - 'SELECT * from posthog_team WHERE organization_id = (SELECT id from posthog_organization WHERE plugins_access_level = $1)', - [OrganizationPluginsAccessLevel.ROOT], - 'getTeamsInOrganizationsWithRootPluginAccess' - ) - ).rows as Team[] + const selectResult = await this.postgres.query( + PostgresUse.COMMON_READ, + 'SELECT * from posthog_team WHERE organization_id = (SELECT id from posthog_organization WHERE plugins_access_level = $1)', + [OrganizationPluginsAccessLevel.ROOT], + 'getTeamsInOrganizationsWithRootPluginAccess' + ) + for (const row of selectResult.rows) { + // pg returns int8 as a string, since it can be larger than JS's max safe integer, + // but this is not a problem for project_id, which is a long long way from that limit. + row.project_id = parseInt(row.project_id as unknown as string) + } + return selectResult.rows } public async addOrUpdatePublicJob( diff --git a/plugin-server/src/worker/ingestion/group-type-manager.ts b/plugin-server/src/worker/ingestion/group-type-manager.ts index 7263a2429cc..32a07e493a1 100644 --- a/plugin-server/src/worker/ingestion/group-type-manager.ts +++ b/plugin-server/src/worker/ingestion/group-type-manager.ts @@ -46,7 +46,11 @@ export class GroupTypeManager { } } - public async fetchGroupTypeIndex(teamId: TeamId, groupType: string): Promise { + public async fetchGroupTypeIndex( + teamId: TeamId, + projectId: TeamId, + groupType: string + ): Promise { const groupTypes = await this.fetchGroupTypes(teamId) if (groupType in groupTypes) { @@ -54,6 +58,7 @@ export class GroupTypeManager { } else { const [groupTypeIndex, isInsert] = await this.insertGroupType( teamId, + projectId, groupType, Object.keys(groupTypes).length ) @@ -70,6 +75,7 @@ export class GroupTypeManager { public async insertGroupType( teamId: TeamId, + projectId: TeamId, groupType: string, index: number ): Promise<[GroupTypeIndex | null, boolean]> { @@ -81,21 +87,21 @@ export class GroupTypeManager { PostgresUse.COMMON_WRITE, ` WITH insert_result AS ( - INSERT INTO posthog_grouptypemapping (team_id, group_type, group_type_index) - VALUES ($1, $2, $3) + INSERT INTO posthog_grouptypemapping (team_id, project_id, group_type, group_type_index) + VALUES ($1, $2, $3, $4) ON CONFLICT DO NOTHING RETURNING group_type_index ) - SELECT group_type_index, 1 AS is_insert FROM insert_result + SELECT group_type_index, 1 AS is_insert FROM insert_result UNION - SELECT group_type_index, 0 AS is_insert FROM posthog_grouptypemapping WHERE team_id = $1 AND group_type = $2; + SELECT group_type_index, 0 AS is_insert FROM posthog_grouptypemapping WHERE team_id = $1 AND group_type = $3; `, - [teamId, groupType, index], + [teamId, projectId, groupType, index], 'insertGroupType' ) if (insertGroupTypeResult.rows.length == 0) { - return await this.insertGroupType(teamId, groupType, index + 1) + return await this.insertGroupType(teamId, projectId, groupType, index + 1) } const { group_type_index, is_insert } = insertGroupTypeResult.rows[0] diff --git a/plugin-server/src/worker/ingestion/groups.ts b/plugin-server/src/worker/ingestion/groups.ts index d65dd2ecabf..0df055e7c8e 100644 --- a/plugin-server/src/worker/ingestion/groups.ts +++ b/plugin-server/src/worker/ingestion/groups.ts @@ -5,11 +5,12 @@ import { GroupTypeManager } from './group-type-manager' export async function addGroupProperties( teamId: TeamId, + projectId: TeamId, properties: Properties, groupTypeManager: GroupTypeManager ): Promise { for (const [groupType, groupIdentifier] of Object.entries(properties.$groups || {})) { - const columnIndex = await groupTypeManager.fetchGroupTypeIndex(teamId, groupType) + const columnIndex = await groupTypeManager.fetchGroupTypeIndex(teamId, projectId, groupType) if (columnIndex !== null) { // :TODO: Update event column instead properties[`$group_${columnIndex}`] = groupIdentifier diff --git a/plugin-server/src/worker/ingestion/process-event.ts b/plugin-server/src/worker/ingestion/process-event.ts index f03ca9d85fb..fc53331c98e 100644 --- a/plugin-server/src/worker/ingestion/process-event.ts +++ b/plugin-server/src/worker/ingestion/process-event.ts @@ -155,7 +155,12 @@ export class EventsProcessor { if (this.pluginsServer.SKIP_UPDATE_EVENT_AND_PROPERTIES_STEP === false) { try { - await this.groupAndFirstEventManager.updateGroupsAndFirstEvent(team.id, event, properties) + await this.groupAndFirstEventManager.updateGroupsAndFirstEvent( + team.id, + team.project_id, + event, + properties + ) } catch (err) { Sentry.captureException(err, { tags: { team_id: team.id } }) status.warn('⚠️', 'Failed to update property definitions for an event', { @@ -168,10 +173,10 @@ export class EventsProcessor { if (processPerson) { // Adds group_0 etc values to properties - properties = await addGroupProperties(team.id, properties, this.groupTypeManager) + properties = await addGroupProperties(team.id, team.project_id, properties, this.groupTypeManager) if (event === '$groupidentify') { - await this.upsertGroup(team.id, properties, timestamp) + await this.upsertGroup(team.id, team.project_id, properties, timestamp) } } @@ -278,13 +283,18 @@ export class EventsProcessor { return [rawEvent, ack] } - private async upsertGroup(teamId: number, properties: Properties, timestamp: DateTime): Promise { + private async upsertGroup( + teamId: number, + projectId: number, + properties: Properties, + timestamp: DateTime + ): Promise { if (!properties['$group_type'] || !properties['$group_key']) { return } const { $group_type: groupType, $group_key: groupKey, $group_set: groupPropertiesToSet } = properties - const groupTypeIndex = await this.groupTypeManager.fetchGroupTypeIndex(teamId, groupType) + const groupTypeIndex = await this.groupTypeManager.fetchGroupTypeIndex(teamId, projectId, groupType) if (groupTypeIndex !== null) { await upsertGroup( diff --git a/plugin-server/src/worker/ingestion/property-definitions-manager.ts b/plugin-server/src/worker/ingestion/property-definitions-manager.ts index 98920ca7dab..7c3d4e8e67e 100644 --- a/plugin-server/src/worker/ingestion/property-definitions-manager.ts +++ b/plugin-server/src/worker/ingestion/property-definitions-manager.ts @@ -27,7 +27,12 @@ export class GroupAndFirstEventManager { this.groupTypeManager = groupTypeManager } - public async updateGroupsAndFirstEvent(teamId: number, event: string, properties: Properties): Promise { + public async updateGroupsAndFirstEvent( + teamId: number, + projectId: number, + event: string, + properties: Properties + ): Promise { if (EVENTS_WITHOUT_EVENT_DEFINITION.includes(event)) { return } @@ -56,7 +61,9 @@ export class GroupAndFirstEventManager { const { $group_type: groupType, $group_set: groupPropertiesToSet } = properties if (groupType != null && groupPropertiesToSet != null) { // This "fetch" is side-effecty, it inserts a group-type and assigns an index if one isn't found - const groupPromise = this.groupTypeManager.fetchGroupTypeIndex(teamId, groupType).then(() => {}) + const groupPromise = this.groupTypeManager + .fetchGroupTypeIndex(teamId, projectId, groupType) + .then(() => {}) promises.push(groupPromise) } } diff --git a/plugin-server/src/worker/ingestion/team-manager.ts b/plugin-server/src/worker/ingestion/team-manager.ts index c050387f658..9846071c094 100644 --- a/plugin-server/src/worker/ingestion/team-manager.ts +++ b/plugin-server/src/worker/ingestion/team-manager.ts @@ -154,6 +154,7 @@ export async function fetchTeam(client: PostgresRouter, teamId: Team['id']): Pro ` SELECT id, + project_id, uuid, organization_id, name, @@ -172,7 +173,13 @@ export async function fetchTeam(client: PostgresRouter, teamId: Team['id']): Pro [teamId], 'fetchTeam' ) - return selectResult.rows[0] ?? null + if (selectResult.rows.length === 0) { + return null + } + // pg returns int8 as a string, since it can be larger than JS's max safe integer, + // but this is not a problem for project_id, which is a long long way from that limit. + selectResult.rows[0].project_id = parseInt(selectResult.rows[0].project_id as unknown as string) + return selectResult.rows[0] } export async function fetchTeamByToken(client: PostgresRouter, token: string): Promise { @@ -181,6 +188,7 @@ export async function fetchTeamByToken(client: PostgresRouter, token: string): P ` SELECT id, + project_id, uuid, organization_id, name, @@ -199,7 +207,13 @@ export async function fetchTeamByToken(client: PostgresRouter, token: string): P [token], 'fetchTeamByToken' ) - return selectResult.rows[0] ?? null + if (selectResult.rows.length === 0) { + return null + } + // pg returns int8 as a string, since it can be larger than JS's max safe integer, + // but this is not a problem for project_id, which is a long long way from that limit. + selectResult.rows[0].project_id = parseInt(selectResult.rows[0].project_id as unknown as string) + return selectResult.rows[0] } export async function fetchTeamTokensWithRecordings(client: PostgresRouter): Promise> { diff --git a/plugin-server/tests/helpers/sql.ts b/plugin-server/tests/helpers/sql.ts index 5935cedb9c5..e815d008654 100644 --- a/plugin-server/tests/helpers/sql.ts +++ b/plugin-server/tests/helpers/sql.ts @@ -262,14 +262,16 @@ export async function createUserTeamAndOrganization( } export async function getTeams(hub: Hub): Promise { - return ( - await hub.db.postgres.query( - PostgresUse.COMMON_READ, - 'SELECT * FROM posthog_team ORDER BY id', - undefined, - 'fetchAllTeams' - ) - ).rows + const selectResult = await hub.db.postgres.query( + PostgresUse.COMMON_READ, + 'SELECT * FROM posthog_team ORDER BY id', + undefined, + 'fetchAllTeams' + ) + for (const row of selectResult.rows) { + row.project_id = parseInt(row.project_id as unknown as string) + } + return selectResult.rows } export async function getFirstTeam(hub: Hub): Promise { diff --git a/plugin-server/tests/main/db.test.ts b/plugin-server/tests/main/db.test.ts index fe4d090090a..10e514d8323 100644 --- a/plugin-server/tests/main/db.test.ts +++ b/plugin-server/tests/main/db.test.ts @@ -855,6 +855,7 @@ describe('DB', () => { anonymize_ips: false, api_token: 'token1', id: teamId, + project_id: teamId, ingested_event: true, name: 'TEST PROJECT', organization_id: organizationId, @@ -884,6 +885,7 @@ describe('DB', () => { anonymize_ips: false, api_token: 'token2', id: teamId, + project_id: teamId, ingested_event: true, name: 'TEST PROJECT', organization_id: organizationId, diff --git a/plugin-server/tests/worker/ingestion/group-type-manager.test.ts b/plugin-server/tests/worker/ingestion/group-type-manager.test.ts index 5ee484a996d..efbc630365b 100644 --- a/plugin-server/tests/worker/ingestion/group-type-manager.test.ts +++ b/plugin-server/tests/worker/ingestion/group-type-manager.test.ts @@ -33,8 +33,8 @@ describe('GroupTypeManager()', () => { expect(groupTypes).toEqual({}) jest.spyOn(global.Date, 'now').mockImplementation(() => new Date('2020-02-27 11:00:25').getTime()) - await groupTypeManager.insertGroupType(2, 'foo', 0) - await groupTypeManager.insertGroupType(2, 'bar', 1) + await groupTypeManager.insertGroupType(2, 2, 'foo', 0) + await groupTypeManager.insertGroupType(2, 2, 'bar', 1) jest.mocked(hub.db.postgres.query).mockClear() @@ -56,30 +56,30 @@ describe('GroupTypeManager()', () => { it('fetches group types that have been inserted', async () => { expect(await groupTypeManager.fetchGroupTypes(2)).toEqual({}) - expect(await groupTypeManager.insertGroupType(2, 'g0', 0)).toEqual([0, true]) - expect(await groupTypeManager.insertGroupType(2, 'g1', 1)).toEqual([1, true]) + expect(await groupTypeManager.insertGroupType(2, 2, 'g0', 0)).toEqual([0, true]) + expect(await groupTypeManager.insertGroupType(2, 2, 'g1', 1)).toEqual([1, true]) groupTypeManager['groupTypesCache'].clear() // Clear cache expect(await groupTypeManager.fetchGroupTypes(2)).toEqual({ g0: 0, g1: 1 }) }) it('handles conflicting by index when inserting and limits', async () => { - expect(await groupTypeManager.insertGroupType(2, 'g0', 0)).toEqual([0, true]) - expect(await groupTypeManager.insertGroupType(2, 'g1', 0)).toEqual([1, true]) - expect(await groupTypeManager.insertGroupType(2, 'g2', 0)).toEqual([2, true]) - expect(await groupTypeManager.insertGroupType(2, 'g3', 1)).toEqual([3, true]) - expect(await groupTypeManager.insertGroupType(2, 'g4', 0)).toEqual([4, true]) - expect(await groupTypeManager.insertGroupType(2, 'g5', 0)).toEqual([null, false]) - expect(await groupTypeManager.insertGroupType(2, 'g6', 0)).toEqual([null, false]) + expect(await groupTypeManager.insertGroupType(2, 2, 'g0', 0)).toEqual([0, true]) + expect(await groupTypeManager.insertGroupType(2, 2, 'g1', 0)).toEqual([1, true]) + expect(await groupTypeManager.insertGroupType(2, 2, 'g2', 0)).toEqual([2, true]) + expect(await groupTypeManager.insertGroupType(2, 2, 'g3', 1)).toEqual([3, true]) + expect(await groupTypeManager.insertGroupType(2, 2, 'g4', 0)).toEqual([4, true]) + expect(await groupTypeManager.insertGroupType(2, 2, 'g5', 0)).toEqual([null, false]) + expect(await groupTypeManager.insertGroupType(2, 2, 'g6', 0)).toEqual([null, false]) expect(await groupTypeManager.fetchGroupTypes(2)).toEqual({ g0: 0, g1: 1, g2: 2, g3: 3, g4: 4 }) }) it('handles conflict by name when inserting', async () => { - expect(await groupTypeManager.insertGroupType(2, 'group_name', 0)).toEqual([0, true]) - expect(await groupTypeManager.insertGroupType(2, 'group_name', 0)).toEqual([0, false]) - expect(await groupTypeManager.insertGroupType(2, 'group_name', 0)).toEqual([0, false]) - expect(await groupTypeManager.insertGroupType(2, 'foo', 0)).toEqual([1, true]) - expect(await groupTypeManager.insertGroupType(2, 'foo', 0)).toEqual([1, false]) + expect(await groupTypeManager.insertGroupType(2, 2, 'group_name', 0)).toEqual([0, true]) + expect(await groupTypeManager.insertGroupType(2, 2, 'group_name', 0)).toEqual([0, false]) + expect(await groupTypeManager.insertGroupType(2, 2, 'group_name', 0)).toEqual([0, false]) + expect(await groupTypeManager.insertGroupType(2, 2, 'foo', 0)).toEqual([1, true]) + expect(await groupTypeManager.insertGroupType(2, 2, 'foo', 0)).toEqual([1, false]) expect(await groupTypeManager.fetchGroupTypes(2)).toEqual({ group_name: 0, foo: 1 }) }) @@ -87,14 +87,14 @@ describe('GroupTypeManager()', () => { describe('fetchGroupTypeIndex()', () => { it('fetches an already existing value', async () => { - await groupTypeManager.insertGroupType(2, 'foo', 0) - await groupTypeManager.insertGroupType(2, 'bar', 1) + await groupTypeManager.insertGroupType(2, 2, 'foo', 0) + await groupTypeManager.insertGroupType(2, 2, 'bar', 1) jest.mocked(hub.db.postgres.query).mockClear() jest.mocked(groupTypeManager.insertGroupType).mockClear() - expect(await groupTypeManager.fetchGroupTypeIndex(2, 'foo')).toEqual(0) - expect(await groupTypeManager.fetchGroupTypeIndex(2, 'bar')).toEqual(1) + expect(await groupTypeManager.fetchGroupTypeIndex(2, 2, 'foo')).toEqual(0) + expect(await groupTypeManager.fetchGroupTypeIndex(2, 2, 'bar')).toEqual(1) expect(hub.db.postgres.query).toHaveBeenCalledTimes(1) expect(groupTypeManager.insertGroupType).toHaveBeenCalledTimes(0) @@ -102,12 +102,12 @@ describe('GroupTypeManager()', () => { }) it('inserts value if it does not exist yet at next index, resets cache', async () => { - await groupTypeManager.insertGroupType(2, 'foo', 0) + await groupTypeManager.insertGroupType(2, 2, 'foo', 0) jest.mocked(groupTypeManager.insertGroupType).mockClear() jest.mocked(hub.db.postgres.query).mockClear() - expect(await groupTypeManager.fetchGroupTypeIndex(2, 'second')).toEqual(1) + expect(await groupTypeManager.fetchGroupTypeIndex(2, 2, 'second')).toEqual(1) expect(groupTypeManager.insertGroupType).toHaveBeenCalledTimes(1) expect(hub.db.postgres.query).toHaveBeenCalledTimes(3) // FETCH + INSERT + Team lookup @@ -118,7 +118,7 @@ describe('GroupTypeManager()', () => { groupTypeIndex: 1, }) - expect(await groupTypeManager.fetchGroupTypeIndex(2, 'third')).toEqual(2) + expect(await groupTypeManager.fetchGroupTypeIndex(2, 2, 'third')).toEqual(2) jest.mocked(hub.db.postgres.query).mockClear() expect(await groupTypeManager.fetchGroupTypes(2)).toEqual({ @@ -126,8 +126,8 @@ describe('GroupTypeManager()', () => { second: 1, third: 2, }) - expect(await groupTypeManager.fetchGroupTypeIndex(2, 'second')).toEqual(1) - expect(await groupTypeManager.fetchGroupTypeIndex(2, 'third')).toEqual(2) + expect(await groupTypeManager.fetchGroupTypeIndex(2, 2, 'second')).toEqual(1) + expect(await groupTypeManager.fetchGroupTypeIndex(2, 2, 'third')).toEqual(2) expect(hub.db.postgres.query).toHaveBeenCalledTimes(1) }) @@ -135,8 +135,8 @@ describe('GroupTypeManager()', () => { it('handles raciness for inserting a new group', async () => { expect(await groupTypeManager.fetchGroupTypes(2)).toEqual({}) - await groupTypeManager.insertGroupType(2, 'foo', 0) // Emulate another thread inserting foo - expect(await groupTypeManager.fetchGroupTypeIndex(2, 'second')).toEqual(1) + await groupTypeManager.insertGroupType(2, 2, 'foo', 0) // Emulate another thread inserting foo + expect(await groupTypeManager.fetchGroupTypeIndex(2, 2, 'second')).toEqual(1) expect(await groupTypeManager.fetchGroupTypes(2)).toEqual({ foo: 0, second: 1, @@ -147,10 +147,10 @@ describe('GroupTypeManager()', () => { expect(await groupTypeManager.fetchGroupTypes(2)).toEqual({}) // Emulate another thread inserting group types - await groupTypeManager.insertGroupType(2, 'foo', 0) - await groupTypeManager.insertGroupType(2, 'bar', 0) + await groupTypeManager.insertGroupType(2, 2, 'foo', 0) + await groupTypeManager.insertGroupType(2, 2, 'bar', 0) - expect(await groupTypeManager.fetchGroupTypeIndex(2, 'bar')).toEqual(1) + expect(await groupTypeManager.fetchGroupTypeIndex(2, 2, 'bar')).toEqual(1) expect(await groupTypeManager.fetchGroupTypes(2)).toEqual({ foo: 0, bar: 1, @@ -158,13 +158,13 @@ describe('GroupTypeManager()', () => { }) it('returns null once limit is met', async () => { - await groupTypeManager.insertGroupType(2, 'g0', 0) - await groupTypeManager.insertGroupType(2, 'g1', 1) - await groupTypeManager.insertGroupType(2, 'g2', 2) - await groupTypeManager.insertGroupType(2, 'g3', 3) - await groupTypeManager.insertGroupType(2, 'g4', 4) + await groupTypeManager.insertGroupType(2, 2, 'g0', 0) + await groupTypeManager.insertGroupType(2, 2, 'g1', 1) + await groupTypeManager.insertGroupType(2, 2, 'g2', 2) + await groupTypeManager.insertGroupType(2, 2, 'g3', 3) + await groupTypeManager.insertGroupType(2, 2, 'g4', 4) - expect(await groupTypeManager.fetchGroupTypeIndex(2, 'new')).toEqual(null) + expect(await groupTypeManager.fetchGroupTypeIndex(2, 2, 'new')).toEqual(null) expect(await groupTypeManager.fetchGroupTypes(2)).toEqual({ g0: 0, g1: 1, diff --git a/plugin-server/tests/worker/ingestion/groups.test.ts b/plugin-server/tests/worker/ingestion/groups.test.ts index c9aad702ab7..263d92eab4f 100644 --- a/plugin-server/tests/worker/ingestion/groups.test.ts +++ b/plugin-server/tests/worker/ingestion/groups.test.ts @@ -10,12 +10,12 @@ describe('addGroupProperties()', () => { foobar: null, } mockGroupTypeManager = { - fetchGroupTypeIndex: jest.fn().mockImplementation((teamId, key) => lookup[key]), + fetchGroupTypeIndex: jest.fn().mockImplementation((teamId, projectId, key) => lookup[key]), } }) it('does nothing if no group properties', async () => { - expect(await addGroupProperties(2, { foo: 'bar' }, mockGroupTypeManager)).toEqual({ foo: 'bar' }) + expect(await addGroupProperties(2, 2, { foo: 'bar' }, mockGroupTypeManager)).toEqual({ foo: 'bar' }) expect(mockGroupTypeManager.fetchGroupTypeIndex).not.toHaveBeenCalled() }) @@ -30,7 +30,7 @@ describe('addGroupProperties()', () => { }, } - expect(await addGroupProperties(2, properties, mockGroupTypeManager)).toEqual({ + expect(await addGroupProperties(2, 2, properties, mockGroupTypeManager)).toEqual({ foo: 'bar', $groups: { organization: 'PostHog', @@ -41,8 +41,8 @@ describe('addGroupProperties()', () => { $group_1: 'web', }) - expect(mockGroupTypeManager.fetchGroupTypeIndex).toHaveBeenCalledWith(2, 'organization') - expect(mockGroupTypeManager.fetchGroupTypeIndex).toHaveBeenCalledWith(2, 'project') - expect(mockGroupTypeManager.fetchGroupTypeIndex).toHaveBeenCalledWith(2, 'foobar') + expect(mockGroupTypeManager.fetchGroupTypeIndex).toHaveBeenCalledWith(2, 2, 'organization') + expect(mockGroupTypeManager.fetchGroupTypeIndex).toHaveBeenCalledWith(2, 2, 'project') + expect(mockGroupTypeManager.fetchGroupTypeIndex).toHaveBeenCalledWith(2, 2, 'foobar') }) }) diff --git a/posthog/api/test/__snapshots__/test_api_docs.ambr b/posthog/api/test/__snapshots__/test_api_docs.ambr index 1558efeef94..5ffc635eb9d 100644 --- a/posthog/api/test/__snapshots__/test_api_docs.ambr +++ b/posthog/api/test/__snapshots__/test_api_docs.ambr @@ -13,7 +13,6 @@ '/home/runner/work/posthog/posthog/ee/clickhouse/views/experiment_holdouts.py: Warning [ExperimentHoldoutViewSet]: could not derive type of path parameter "project_id" because model "posthog.models.experiment.ExperimentHoldout" contained no such field. Consider annotating parameter with @extend_schema. Defaulting to "string".', '/home/runner/work/posthog/posthog/ee/clickhouse/views/experiment_saved_metrics.py: Warning [ExperimentSavedMetricViewSet]: could not derive type of path parameter "project_id" because model "posthog.models.experiment.ExperimentSavedMetric" contained no such field. Consider annotating parameter with @extend_schema. Defaulting to "string".', '/home/runner/work/posthog/posthog/ee/clickhouse/views/experiments.py: Warning [EnterpriseExperimentsViewSet]: could not derive type of path parameter "project_id" because model "posthog.models.experiment.Experiment" contained no such field. Consider annotating parameter with @extend_schema. Defaulting to "string".', - '/home/runner/work/posthog/posthog/ee/clickhouse/views/groups.py: Warning [GroupsTypesViewSet]: could not derive type of path parameter "project_id" because model "posthog.models.group_type_mapping.GroupTypeMapping" contained no such field. Consider annotating parameter with @extend_schema. Defaulting to "string".', '/home/runner/work/posthog/posthog/ee/clickhouse/views/groups.py: Warning [GroupsViewSet]: could not derive type of path parameter "project_id" because model "posthog.models.group.group.Group" contained no such field. Consider annotating parameter with @extend_schema. Defaulting to "string".', '/home/runner/work/posthog/posthog/ee/clickhouse/views/insights.py: Warning [EnterpriseInsightsViewSet]: could not derive type of path parameter "project_id" because model "posthog.models.insight.Insight" contained no such field. Consider annotating parameter with @extend_schema. Defaulting to "string".', '/home/runner/work/posthog/posthog/ee/clickhouse/views/person.py: Warning [EnterprisePersonViewSet]: could not derive type of path parameter "project_id" because model "posthog.models.person.person.Person" contained no such field. Consider annotating parameter with @extend_schema. Defaulting to "string".', diff --git a/posthog/api/test/__snapshots__/test_feature_flag.ambr b/posthog/api/test/__snapshots__/test_feature_flag.ambr index 0ba9846a238..451d10a0057 100644 --- a/posthog/api/test/__snapshots__/test_feature_flag.ambr +++ b/posthog/api/test/__snapshots__/test_feature_flag.ambr @@ -2248,6 +2248,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -2260,6 +2261,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -2272,6 +2274,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", diff --git a/posthog/management/commands/test_migrations_are_safe.py b/posthog/management/commands/test_migrations_are_safe.py index 6c7d832e971..d7c593d1194 100644 --- a/posthog/management/commands/test_migrations_are_safe.py +++ b/posthog/management/commands/test_migrations_are_safe.py @@ -68,13 +68,13 @@ def validate_migration_sql(sql) -> bool: ) return True if ( - "CONSTRAINT" in operation_sql + " CONSTRAINT " in operation_sql # Ignore for new foreign key columns that are nullable, as their foreign key constraint does not lock - and not re.match(r"ADD COLUMN .+ NULL CONSTRAINT", operation_sql) + and not re.search(r"ADD COLUMN .+ NULL CONSTRAINT", operation_sql) and "-- existing-table-constraint-ignore" not in operation_sql and " NOT VALID" not in operation_sql - and " VALIDATE CONSTRAINT " - not in operation_sql # VALIDATE CONSTRAINT is a different, non-locking operation + # VALIDATE CONSTRAINT is a different, non-locking operation + and " VALIDATE CONSTRAINT " not in operation_sql and ( table_being_altered not in tables_created_so_far or _get_table("ALTER TABLE", operation_sql) not in new_tables # Ignore for brand-new tables @@ -85,7 +85,7 @@ def validate_migration_sql(sql) -> bool: "If adding a foreign key field, see `0415_pluginconfig_match_action` for an example of how to do this safely. " "If adding the constraint by itself, please use `AddConstraintNotValid()` of `django.contrib.postgres.operations` instead. " "See https://docs.djangoproject.com/en/4.2/ref/contrib/postgres/operations/#adding-constraints-without-enforcing-validation.\n" - "Source: `{operation_sql}`" + f"Source: `{operation_sql}`" ) return True if ( @@ -98,7 +98,7 @@ def validate_migration_sql(sql) -> bool: "If adding a foreign key field, see `0415_pluginconfig_match_action` for an example of how to do this safely. " "If adding the index by itself, please use `AddIndexConcurrently()` of `django.contrib.postgres.operations` instead. " "See https://docs.djangoproject.com/en/4.2/ref/contrib/postgres/operations/#concurrent-index-operations.\n" - "Source: `{operation_sql}`" + f"Source: `{operation_sql}`" ) return True diff --git a/posthog/migrations/0505_grouptypemapping_project.py b/posthog/migrations/0505_grouptypemapping_project.py new file mode 100644 index 00000000000..0f0d4573dd9 --- /dev/null +++ b/posthog/migrations/0505_grouptypemapping_project.py @@ -0,0 +1,41 @@ +# Generated by Django 4.2.15 on 2024-10-15 13:32 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + atomic = False # Added to support concurrent index creation + dependencies = [ + ("posthog", "0504_add_dead_clicks_setting"), + ] + + operations = [ + migrations.SeparateDatabaseAndState( + state_operations=[ + migrations.AddField( + model_name="grouptypemapping", + name="project", + field=models.ForeignKey( + null=True, on_delete=django.db.models.deletion.CASCADE, to="posthog.project" + ), + ), + ], + database_operations=[ + migrations.RunSQL( + """ + ALTER TABLE "posthog_grouptypemapping" ADD COLUMN "project_id" bigint NULL CONSTRAINT "posthog_grouptypemap_project_id_239c0515_fk_posthog_p" REFERENCES "posthog_project"("id") DEFERRABLE INITIALLY DEFERRED; + SET CONSTRAINTS "posthog_grouptypemap_project_id_239c0515_fk_posthog_p" IMMEDIATE;""", + reverse_sql=""" + ALTER TABLE "posthog_grouptypemapping" DROP COLUMN IF EXISTS "project_id";""", + ), + # We add CONCURRENTLY to the create command + migrations.RunSQL( + """ + CREATE INDEX CONCURRENTLY "posthog_grouptypemapping_project_id_239c0515" ON "posthog_grouptypemapping" ("project_id");""", + reverse_sql=""" + DROP INDEX IF EXISTS "posthog_grouptypemapping_project_id_239c0515";""", + ), + ], + ), + ] diff --git a/posthog/models/group_type_mapping.py b/posthog/models/group_type_mapping.py index 8dcb9cd74f1..38cfe4e6e10 100644 --- a/posthog/models/group_type_mapping.py +++ b/posthog/models/group_type_mapping.py @@ -5,6 +5,7 @@ from django.db import models # to add group keys class GroupTypeMapping(models.Model): team = models.ForeignKey("Team", on_delete=models.CASCADE) + project = models.ForeignKey("Project", on_delete=models.CASCADE, null=True) group_type = models.CharField(max_length=400, null=False, blank=False) group_type_index = models.IntegerField(null=False, blank=False) # Used to display in UI diff --git a/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr b/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr index fc4dd0038fe..eed189b5adf 100644 --- a/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr +++ b/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr @@ -645,6 +645,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -770,6 +771,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -1949,6 +1951,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -2074,6 +2077,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -2534,6 +2538,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -2728,6 +2733,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -3201,6 +3207,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -3345,6 +3352,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -3833,6 +3841,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -3958,6 +3967,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -4510,6 +4520,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -4635,6 +4646,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -4934,6 +4946,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -5059,6 +5072,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -5527,6 +5541,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -5560,6 +5575,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -5664,6 +5680,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -6129,6 +6146,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -6327,6 +6345,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -6792,6 +6811,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -6917,6 +6937,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -6950,6 +6971,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -7398,6 +7420,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -7523,6 +7546,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", diff --git a/posthog/test/__snapshots__/test_feature_flag.ambr b/posthog/test/__snapshots__/test_feature_flag.ambr index 36ed6e1cce5..fe95ff9b2ce 100644 --- a/posthog/test/__snapshots__/test_feature_flag.ambr +++ b/posthog/test/__snapshots__/test_feature_flag.ambr @@ -418,6 +418,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -470,6 +471,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular", @@ -570,6 +572,7 @@ ''' SELECT "posthog_grouptypemapping"."id", "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."project_id", "posthog_grouptypemapping"."group_type", "posthog_grouptypemapping"."group_type_index", "posthog_grouptypemapping"."name_singular",