-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(taxonomy): Add project field to taxonomy models #26521
Conversation
bd3b0ed
to
4e3bf0b
Compare
plugin-server/src/types.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only types need to be updated in the plugin server, since these records are SELECTed with *
c32a39a
to
b54cb8e
Compare
bbb6a2d
to
7f9a755
Compare
7f9a755
to
38a1242
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine except for the integer type thing, but approving so once you fix you can merge
rust/property-defs-rs/src/types.rs
Outdated
@@ -84,6 +84,7 @@ impl GroupType { | |||
#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] | |||
pub struct PropertyDefinition { | |||
pub team_id: i32, | |||
pub project_id: i32, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bigint's are i64's, I think? 8 bytes wide.
@@ -9,6 +9,7 @@ CREATE TABLE IF NOT EXISTS posthog_eventdefinition ( | |||
volume_30_day INTEGER, | |||
query_usage_30_day INTEGER, | |||
team_id INTEGER NOT NULL, | |||
project_id INTEGER NULL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to keep these in sync and make them a bigint
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, now i64!
3804d6a
to
6cca91d
Compare
)" This reverts commit caaf6a0.
Problem
Step 9 of the environments implementation plan. (Same as #25600, just for taxonomy models.)
Changes
Added nullable field
project
to theEventDefinition
,PropertyDefinition
, andEventProperty
models. We now insert it in ingestion.Backfill for existing event definitions, property definitions, and event properties will come in a separate PR.
How did you test this code?
All existing tests should continue to pass.