chore(environments): Make team.project_id officially non-null (#25597)

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: 0493_insightvariable_values
+posthog: 0494_team_project_non_null
 sessions: 0001_initial
 social_django: 0010_uid_db_index
 two_factor: 0007_auto_20201201_1019

mypy-baseline.txt

@@ -756,8 +756,6 @@ posthog/admin/inlines/plugin_attachment_inline.py:0: note: Superclass:
 posthog/admin/inlines/plugin_attachment_inline.py:0: note: def has_delete_permission(self, request: HttpRequest, obj: Any | None = ...) -> bool
 posthog/admin/inlines/plugin_attachment_inline.py:0: note: Subclass:
 posthog/admin/inlines/plugin_attachment_inline.py:0: note: def has_delete_permission(self, request: Any, obj: Any) -> Any
-posthog/admin/admins/team_admin.py:0: error: Item "None" of "Project | None" has no attribute "pk"  [union-attr]
-posthog/admin/admins/team_admin.py:0: error: Item "None" of "Project | None" has no attribute "name"  [union-attr]
 posthog/admin/admins/plugin_admin.py:0: error: Item "None" of "Organization | None" has no attribute "pk"  [union-attr]
 posthog/admin/admins/plugin_admin.py:0: error: Item "None" of "Organization | None" has no attribute "name"  [union-attr]
 posthog/temporal/tests/batch_exports/test_run_updates.py:0: error: Unused "type: ignore" comment  [unused-ignore]

posthog/api/routing.py

@@ -242,14 +242,12 @@ def team(self) -> Team:
     @cached_property
     def project_id(self) -> int:
         if team_from_token := self._get_team_from_request():
-            assert team_from_token.project_id is not None
             return team_from_token.project_id
 
         if self.param_derived_from_user_current_team == "project_id":
             user = cast(User, self.request.user)
             team = user.team
             assert team is not None
-            assert team.project_id is not None
             return team.project_id
 
         return self.parents_query_dict["project_id"]

posthog/hogql_queries/insights/funnels/base.py

@@ -304,7 +304,6 @@ def _serialize_step(
                 "Data warehouse tables are not supported in funnels just yet. For now, please try this funnel without the data warehouse-based step."
             )
         else:
-            assert self.context.team.project_id is not None
             action = Action.objects.get(pk=step.id, team__project_id=self.context.team.project_id)
             name = action.name
             action_id = step.id
@@ -677,7 +676,6 @@ def _build_step_query(
 
         if isinstance(entity, ActionsNode) or isinstance(entity, FunnelExclusionActionsNode):
             # action
-            assert self.context.team.project_id is not None
             action = Action.objects.get(pk=int(entity.id), team__project_id=self.context.team.project_id)
             event_expr = action_to_expr(action)
         elif isinstance(entity, DataWarehouseNode):

posthog/hogql_queries/insights/funnels/funnel_correlation_query_runner.py

@@ -830,7 +830,6 @@ def _get_funnel_step_names(self) -> list[str]:
         events: set[str] = set()
         for entity in self.funnels_query.series:
             if isinstance(entity, ActionsNode):
-                assert self.context.team.project_id is not None
                 action = Action.objects.get(pk=int(entity.id), team__project_id=self.context.team.project_id)
                 events.update([x for x in action.get_step_events() if x])
             elif isinstance(entity, EventsNode):

posthog/hogql_queries/insights/funnels/funnel_event_query.py

@@ -144,7 +144,6 @@ def _entity_expr(self, skip_entity_filter: bool) -> ast.Expr | None:
             if isinstance(node, EventsNode) or isinstance(node, FunnelExclusionEventsNode):
                 events.add(node.event)
             elif isinstance(node, ActionsNode) or isinstance(node, FunnelExclusionActionsNode):
-                assert self.context.team.project_id is not None
                 try:
                     action = Action.objects.get(pk=int(node.id), team__project_id=self.context.team.project_id)
                     events.update(action.get_step_events())

posthog/hogql_queries/insights/lifecycle_query_runner.py

@@ -181,7 +181,6 @@ def calculate(self) -> LifecycleQueryResponse:
             action_object = {}
             label = "{} - {}".format("", val[2])
             if isinstance(self.query.series[0], ActionsNode):
-                assert self.team.project_id is not None
                 action = Action.objects.get(pk=int(self.query.series[0].id), team__project_id=self.team.project_id)
                 label = "{} - {}".format(action.name, val[2])
                 action_object = {
@@ -249,7 +248,6 @@ def event_filter(self) -> ast.Expr:
         with self.timings.measure("series_filters"):
             for serie in self.query.series or []:
                 if isinstance(serie, ActionsNode):
-                    assert self.team.project_id is not None
                     action = Action.objects.get(pk=int(serie.id), team__project_id=self.team.project_id)
                     event_filters.append(action_to_expr(action))
                 elif isinstance(serie, EventsNode):

posthog/hogql_queries/insights/retention_query_runner.py

@@ -87,7 +87,6 @@ def filter_timestamp(self) -> ast.Expr:
 
     def _get_events_for_entity(self, entity: RetentionEntity) -> list[str | None]:
         if entity.type == EntityType.ACTIONS and entity.id:
-            assert self.team.project_id is not None
             action = Action.objects.get(pk=int(entity.id), team__project_id=self.team.project_id)
             return action.get_step_events()
         return [entity.id] if isinstance(entity.id, str) else [None]

posthog/hogql_queries/insights/stickiness_query_runner.py

@@ -275,7 +275,6 @@ def where_clause(self, series_with_extra: SeriesWithExtras) -> ast.Expr:
                 )
             )
         elif isinstance(series, ActionsNode):
-            assert self.team.project_id is not None
             try:
                 action = Action.objects.get(pk=int(series.id), team__project_id=self.team.project_id)
                 filters.append(action_to_expr(action))
@@ -331,7 +330,6 @@ def series_event(self, series: EventsNode | ActionsNode | DataWarehouseNode) ->
             return series.table_name
 
         if isinstance(series, ActionsNode):
-            assert self.team.project_id is not None
             # TODO: Can we load the Action in more efficiently?
             action = Action.objects.get(pk=int(series.id), team__project_id=self.team.project_id)
             return action.name

posthog/hogql_queries/insights/trends/trends_actors_query_builder.py

@@ -292,7 +292,6 @@ def _entity_where_expr(self) -> list[ast.Expr]:
     def _event_or_action_where_expr(self) -> ast.Expr | None:
         if isinstance(self.entity, ActionsNode):
             # Actions
-            assert self.team.project_id is not None
             try:
                 action = Action.objects.get(pk=int(self.entity.id), team__project_id=self.team.project_id)
                 return action_to_expr(action)

posthog/hogql_queries/insights/trends/trends_query_builder.py

@@ -738,7 +738,6 @@ def _event_or_action_where_expr(self) -> ast.Expr | None:
 
         # Actions
         if isinstance(self.series, ActionsNode):
-            assert self.team.project_id is not None
             try:
                 action = Action.objects.get(pk=int(self.series.id), team__project_id=self.team.project_id)
                 return action_to_expr(action)

posthog/hogql_queries/insights/trends/trends_query_runner.py

@@ -640,7 +640,6 @@ def series_event(self, series: Union[EventsNode, ActionsNode, DataWarehouseNode]
         if isinstance(series, EventsNode):
             return series.event
         if isinstance(series, ActionsNode):
-            assert self.team.project_id is not None
             # TODO: Can we load the Action in more efficiently?
             action = Action.objects.get(pk=int(series.id), team__project_id=self.team.project_id)
             return action.name

posthog/hogql_queries/web_analytics/web_overview.py

@@ -100,7 +100,6 @@ def session_properties(self) -> ast.Expr:
     @cached_property
     def conversion_goal_expr(self) -> ast.Expr:
         if isinstance(self.query.conversionGoal, ActionConversionGoal):
-            assert self.team.project_id is not None
             action = Action.objects.get(pk=self.query.conversionGoal.actionId, team__project_id=self.team.project_id)
             return action_to_expr(action)
         elif isinstance(self.query.conversionGoal, CustomEventConversionGoal):

posthog/management/commands/test_migrations_are_safe.py

@@ -67,13 +67,18 @@ def validate_migration_sql(sql) -> bool:
                 f"\n\n\033[91mFound a DROP TABLE command. This could lead to unsafe states for the app. Please avoid dropping tables.\nSource: `{operation_sql}`"
             )
             return True
-        if "CONSTRAINT" in operation_sql and (
-            "-- existing-table-constraint-ignore" not in operation_sql
+        if (
+            "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 "-- 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
             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
+                or _get_table("ALTER TABLE", operation_sql) not in new_tables  # Ignore for brand-new tables
+            )
         ):
             print(
                 f"\n\n\033[91mFound a CONSTRAINT command without NOT VALID. This locks tables which causes downtime. "

posthog/migrations/0494_team_project_non_null.py

@@ -0,0 +1,34 @@
+# Generated by Django 4.2.15 on 2024-10-15 13:04
+
+from django.db import migrations, models
+import django.db.models.deletion
+from django.contrib.postgres.operations import ValidateConstraint
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("posthog", "0493_insightvariable_values"),
+    ]
+
+    operations = [
+        migrations.SeparateDatabaseAndState(
+            state_operations=[
+                # The only difference is lack of null=True
+                migrations.AlterField(
+                    model_name="team",
+                    name="project",
+                    field=models.ForeignKey(
+                        on_delete=django.db.models.deletion.CASCADE,
+                        related_name="teams",
+                        related_query_name="team",
+                        to="posthog.project",
+                    ),
+                )
+            ],
+            database_operations=[
+                # Finishing the job started in 0445_require_team_project_id_not_valid
+                # This is safe in both Cloud regions, as project_id is not null for any posthog_team record
+                ValidateConstraint(model_name="team", name="project_id_is_not_null")
+            ],
+        )
+    ]

posthog/models/team/team.py

@@ -200,11 +200,7 @@ class Meta:
         related_query_name="team",
     )
     project = models.ForeignKey(
-        "posthog.Project",
-        on_delete=models.CASCADE,
-        related_name="teams",
-        related_query_name="team",
-        null=True,
+        "posthog.Project", on_delete=models.CASCADE, related_name="teams", related_query_name="team"
     )
     api_token = models.CharField(
         max_length=200,
@@ -286,7 +282,7 @@ class Meta:
     # during feature releases.
     extra_settings = models.JSONField(null=True, blank=True)
 
-    # Project level default HogQL query modifiers
+    # Environment-level default HogQL query modifiers
     modifiers = models.JSONField(null=True, blank=True)
 
     # This is meant to be used as a stopgap until https://github.com/PostHog/meta/pull/39 gets implemented

posthog/user_permissions.py

@@ -80,7 +80,7 @@ def team_ids_visible_for_user(self) -> list[int]:
 
     @cached_property
     def project_ids_visible_for_user(self) -> list[int]:
-        return list({team.project_id for team in self.teams_visible_for_user if team.project_id is not None})
+        return list({team.project_id for team in self.teams_visible_for_user})
 
     # Cached properties/functions for efficient lookups in other classes
 

rust/feature-flags/src/test_utils.rs

@@ -214,12 +214,24 @@ pub async fn insert_new_team_in_pg(
     let uuid = Uuid::now_v7();
 
     let mut conn = client.get_connection().await?;
+
+    let res = sqlx::query(
+        r#"INSERT INTO posthog_project
+        (id, organization_id, name, created_at) VALUES
+        ($1, $2::uuid, $3, '2024-06-17 14:40:51.332036+00:00')"#,
+    )
+    .bind(team.id)
+    .bind(ORG_ID)
+    .bind(&team.name)
+    .execute(&mut *conn)
+    .await?;
+    assert_eq!(res.rows_affected(), 1);
+
     let res = sqlx::query(
         r#"INSERT INTO posthog_team 
         (id, uuid, organization_id, project_id, api_token, name, created_at, updated_at, app_urls, anonymize_ips, completed_snippet_onboarding, ingested_event, session_recording_opt_in, is_demo, access_control, test_account_filters, timezone, data_attributes, plugins_opt_in, opt_out_capture, event_names, event_names_with_usage, event_properties, event_properties_with_usage, event_properties_numerical) VALUES
-        ($1, $5, $2::uuid, 1, $3, $4, '2024-06-17 14:40:51.332036+00:00', '2024-06-17', '{}', false, false, false, false, false, false, '{}', 'UTC', '["data-attr"]', false, false, '[]', '[]', '[]', '[]', '[]')"#
+        ($1, $5, $2::uuid, $1, $3, $4, '2024-06-17 14:40:51.332036+00:00', '2024-06-17', '{}', false, false, false, false, false, false, '{}', 'UTC', '["data-attr"]', false, false, '[]', '[]', '[]', '[]', '[]')"#
     ).bind(team.id).bind(ORG_ID).bind(&team.api_token).bind(&team.name).bind(uuid).execute(&mut *conn).await?;
-
     assert_eq!(res.rows_affected(), 1);
 
     // Insert group type mappings
@@ -243,7 +255,6 @@ pub async fn insert_new_team_in_pg(
         .bind(team.id)
         .execute(&mut *conn)
         .await?;
-
         assert_eq!(res.rows_affected(), 1);
     }
     Ok(team)