From ca91e3470148083744901bb11adecdca54df5cce Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Thu, 4 Jun 2026 11:43:03 -0400 Subject: [PATCH] collab: Remove writes to `github_user_id` and `github_login` (#58474) This PR removes writes to the `github_user_id` and `github_login` columns on the `users` table. These writes were only being done in tests, as the production codepaths that referenced these columns have already been removed. Depends on https://github.com/zed-industries/cloud/pull/2743 to make the `github_user_id` column nullable. The corresponding database schema migration has been created in the Cloud repo and applied to the production database. Closes CLO-831. Release Notes: - N/A --- .../20221109000000_test_schema.sql | 2 +- .../migrations/20251208000000_test_schema.sql | 2 +- crates/collab/src/db.rs | 7 -- crates/collab/src/db/queries/users.rs | 9 +- crates/collab/src/db/tables/user.rs | 4 - crates/collab/tests/integration/db_tests.rs | 16 +-- .../integration/db_tests/buffer_tests.rs | 60 +----------- .../integration/db_tests/channel_tests.rs | 98 ++++--------------- .../tests/integration/db_tests/db_tests.rs | 45 ++------- .../integration/randomized_test_helpers.rs | 10 +- 10 files changed, 39 insertions(+), 214 deletions(-) diff --git a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql index 977d012690..0ff6aa34f8 100644 --- a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql +++ b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql @@ -10,7 +10,7 @@ CREATE TABLE "users" ( "connected_once" BOOLEAN NOT NULL DEFAULT false, "created_at" TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, "metrics_id" TEXT, - "github_user_id" INTEGER NOT NULL, + "github_user_id" INTEGER, "accepted_tos_at" TIMESTAMP WITHOUT TIME ZONE, "github_user_created_at" TIMESTAMP WITHOUT TIME ZONE, "custom_llm_monthly_allowance_in_cents" INTEGER diff --git a/crates/collab/migrations/20251208000000_test_schema.sql b/crates/collab/migrations/20251208000000_test_schema.sql index 27cf684cc8..44c9959d9c 100644 --- a/crates/collab/migrations/20251208000000_test_schema.sql +++ b/crates/collab/migrations/20251208000000_test_schema.sql @@ -426,7 +426,7 @@ CREATE TABLE public.users ( email_address character varying(255) DEFAULT NULL::character varying, connected_once boolean DEFAULT false NOT NULL, created_at timestamp without time zone DEFAULT now() NOT NULL, - github_user_id integer NOT NULL, + github_user_id integer, metrics_id uuid DEFAULT gen_random_uuid() NOT NULL, accepted_tos_at timestamp without time zone, github_user_created_at timestamp without time zone, diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 10c4f7c961..60e49fc39a 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -365,13 +365,6 @@ pub struct WaitlistSummary { pub unknown_count: i64, } -/// The parameters to create a new user. -#[derive(Debug, Serialize, Deserialize)] -pub struct NewUserParams { - pub github_login: String, - pub github_user_id: i32, -} - /// The result of creating a new user. #[derive(Debug)] pub struct NewUserResult { diff --git a/crates/collab/src/db/queries/users.rs b/crates/collab/src/db/queries/users.rs index 6f84ae8ab5..8ec6ba8368 100644 --- a/crates/collab/src/db/queries/users.rs +++ b/crates/collab/src/db/queries/users.rs @@ -3,20 +3,13 @@ use super::*; impl Database { /// Creates a new user. #[cfg(feature = "test-support")] - pub async fn create_user(&self, admin: bool, params: NewUserParams) -> Result { + pub async fn create_user(&self, admin: bool) -> Result { self.transaction(|tx| async { let tx = tx; let user = user::Entity::insert(user::ActiveModel { - github_login: ActiveValue::set(params.github_login.clone()), - github_user_id: ActiveValue::set(params.github_user_id), admin: ActiveValue::set(admin), ..Default::default() }) - .on_conflict( - OnConflict::column(user::Column::GithubUserId) - .update_columns([user::Column::Admin, user::Column::GithubLogin]) - .to_owned(), - ) .exec_with_returning(&*tx) .await?; diff --git a/crates/collab/src/db/tables/user.rs b/crates/collab/src/db/tables/user.rs index 3a0b7ad745..e8bb135c1b 100644 --- a/crates/collab/src/db/tables/user.rs +++ b/crates/collab/src/db/tables/user.rs @@ -8,10 +8,6 @@ use serde::Serialize; pub struct Model { #[sea_orm(primary_key)] pub id: UserId, - #[cfg(feature = "test-support")] - pub github_login: String, - #[cfg(feature = "test-support")] - pub github_user_id: i32, pub admin: bool, pub connected_once: bool, } diff --git a/crates/collab/tests/integration/db_tests.rs b/crates/collab/tests/integration/db_tests.rs index 4142a4575b..bc2c99d2a2 100644 --- a/crates/collab/tests/integration/db_tests.rs +++ b/crates/collab/tests/integration/db_tests.rs @@ -5,7 +5,6 @@ mod extension_tests; mod migrations; use std::sync::Arc; -use std::sync::atomic::{AtomicI32, Ordering::SeqCst}; use std::time::Duration; use collections::HashSet; @@ -205,17 +204,6 @@ fn channel_tree(channels: &[(ChannelId, &[ChannelId], &'static str)]) -> Vec, email: &str) -> UserId { - db.create_user( - false, - NewUserParams { - github_login: email[0..email.find('@').unwrap()].to_string(), - github_user_id: GITHUB_USER_ID.fetch_add(1, SeqCst), - }, - ) - .await - .unwrap() - .user_id +async fn new_test_user(db: &Arc) -> UserId { + db.create_user(false).await.unwrap().user_id } diff --git a/crates/collab/tests/integration/db_tests/buffer_tests.rs b/crates/collab/tests/integration/db_tests/buffer_tests.rs index 35ce57cbf4..0a82fea800 100644 --- a/crates/collab/tests/integration/db_tests/buffer_tests.rs +++ b/crates/collab/tests/integration/db_tests/buffer_tests.rs @@ -11,41 +11,11 @@ test_both_dbs!( ); async fn test_channel_buffers(db: &Arc) { - let a_id = db - .create_user( - false, - NewUserParams { - github_login: "user_a".into(), - github_user_id: 101, - }, - ) - .await - .unwrap() - .user_id; - let b_id = db - .create_user( - false, - NewUserParams { - github_login: "user_b".into(), - github_user_id: 102, - }, - ) - .await - .unwrap() - .user_id; + let a_id = db.create_user(false).await.unwrap().user_id; + let b_id = db.create_user(false).await.unwrap().user_id; // This user will not be a part of the channel - let c_id = db - .create_user( - false, - NewUserParams { - github_login: "user_c".into(), - github_user_id: 103, - }, - ) - .await - .unwrap() - .user_id; + let c_id = db.create_user(false).await.unwrap().user_id; let owner_id = db.create_server("production").await.unwrap().0 as u32; @@ -180,28 +150,8 @@ test_both_dbs!( ); async fn test_channel_buffers_last_operations(db: &Database) { - let user_id = db - .create_user( - false, - NewUserParams { - github_login: "user_a".into(), - github_user_id: 101, - }, - ) - .await - .unwrap() - .user_id; - let observer_id = db - .create_user( - false, - NewUserParams { - github_login: "user_b".into(), - github_user_id: 102, - }, - ) - .await - .unwrap() - .user_id; + let user_id = db.create_user(false).await.unwrap().user_id; + let observer_id = db.create_user(false).await.unwrap().user_id; let owner_id = db.create_server("production").await.unwrap().0 as u32; let connection_id = ConnectionId { owner_id, diff --git a/crates/collab/tests/integration/db_tests/channel_tests.rs b/crates/collab/tests/integration/db_tests/channel_tests.rs index e7752c2795..2fd2217f3b 100644 --- a/crates/collab/tests/integration/db_tests/channel_tests.rs +++ b/crates/collab/tests/integration/db_tests/channel_tests.rs @@ -1,6 +1,6 @@ use super::{assert_channel_tree_matches, channel_tree, new_test_user}; use crate::test_both_dbs; -use collab::db::{Channel, ChannelId, ChannelRole, Database, NewUserParams, RoomId}; +use collab::db::{Channel, ChannelId, ChannelRole, Database, RoomId}; use rpc::{ ConnectionId, proto::{self, reorder_channel}, @@ -10,8 +10,8 @@ use std::{collections::HashSet, sync::Arc}; test_both_dbs!(test_channels, test_channels_postgres, test_channels_sqlite); async fn test_channels(db: &Arc) { - let a_id = new_test_user(db, "user1@example.com").await; - let b_id = new_test_user(db, "user2@example.com").await; + let a_id = new_test_user(db).await; + let b_id = new_test_user(db).await; let zed_id = db.create_root_channel("zed", a_id).await.unwrap(); @@ -118,8 +118,8 @@ test_both_dbs!( async fn test_joining_channels(db: &Arc) { let owner_id = db.create_server("test").await.unwrap().0 as u32; - let user_1 = new_test_user(db, "user1@example.com").await; - let user_2 = new_test_user(db, "user2@example.com").await; + let user_1 = new_test_user(db).await; + let user_2 = new_test_user(db).await; let channel_1 = db.create_root_channel("channel_1", user_1).await.unwrap(); @@ -149,9 +149,9 @@ test_both_dbs!( async fn test_channel_invites(db: &Arc) { db.create_server("test").await.unwrap(); - let user_1 = new_test_user(db, "user1@example.com").await; - let user_2 = new_test_user(db, "user2@example.com").await; - let user_3 = new_test_user(db, "user3@example.com").await; + let user_1 = new_test_user(db).await; + let user_2 = new_test_user(db).await; + let user_3 = new_test_user(db).await; let channel_1_1_id = db.create_root_channel("channel_1", user_1).await.unwrap(); @@ -261,29 +261,9 @@ test_both_dbs!( async fn test_channel_renames(db: &Arc) { db.create_server("test").await.unwrap(); - let user_1 = db - .create_user( - false, - NewUserParams { - github_login: "user1".into(), - github_user_id: 5, - }, - ) - .await - .unwrap() - .user_id; + let user_1 = db.create_user(false).await.unwrap().user_id; - let user_2 = db - .create_user( - false, - NewUserParams { - github_login: "user2".into(), - github_user_id: 6, - }, - ) - .await - .unwrap() - .user_id; + let user_2 = db.create_user(false).await.unwrap().user_id; let zed_id = db.create_root_channel("zed", user_1).await.unwrap(); @@ -308,17 +288,7 @@ test_both_dbs!( ); async fn test_db_channel_moving(db: &Arc) { - let a_id = db - .create_user( - false, - NewUserParams { - github_login: "user1".into(), - github_user_id: 5, - }, - ) - .await - .unwrap() - .user_id; + let a_id = db.create_user(false).await.unwrap().user_id; let zed_id = db.create_root_channel("zed", a_id).await.unwrap(); @@ -396,29 +366,9 @@ test_both_dbs!( ); async fn test_channel_reordering(db: &Arc) { - let admin_id = db - .create_user( - false, - NewUserParams { - github_login: "admin".into(), - github_user_id: 1, - }, - ) - .await - .unwrap() - .user_id; + let admin_id = db.create_user(false).await.unwrap().user_id; - let user_id = db - .create_user( - false, - NewUserParams { - github_login: "user".into(), - github_user_id: 2, - }, - ) - .await - .unwrap() - .user_id; + let user_id = db.create_user(false).await.unwrap().user_id; // Create a root channel with some sub-channels let root_id = db.create_root_channel("root", admin_id).await.unwrap(); @@ -587,17 +537,7 @@ test_both_dbs!( ); async fn test_db_channel_moving_bugs(db: &Arc) { - let user_id = db - .create_user( - false, - NewUserParams { - github_login: "user1".into(), - github_user_id: 5, - }, - ) - .await - .unwrap() - .user_id; + let user_id = db.create_user(false).await.unwrap().user_id; let zed_id = db.create_root_channel("zed", user_id).await.unwrap(); @@ -657,9 +597,9 @@ test_both_dbs!( ); async fn test_user_is_channel_participant(db: &Arc) { - let admin = new_test_user(db, "admin@example.com").await; - let member = new_test_user(db, "member@example.com").await; - let guest = new_test_user(db, "guest@example.com").await; + let admin = new_test_user(db).await; + let member = new_test_user(db).await; + let guest = new_test_user(db).await; let zed_channel = db.create_root_channel("zed", admin).await.unwrap(); let internal_channel_id = db @@ -972,8 +912,8 @@ test_both_dbs!( async fn test_delete_channel_with_active_call(db: &Arc) { let owner_id = db.create_server("test").await.unwrap().0 as u32; - let user_1 = new_test_user(db, "user1@example.com").await; - let user_2 = new_test_user(db, "user2@example.com").await; + let user_1 = new_test_user(db).await; + let user_2 = new_test_user(db).await; let parent_channel_id = db .create_root_channel("parent_channel", user_1) diff --git a/crates/collab/tests/integration/db_tests/db_tests.rs b/crates/collab/tests/integration/db_tests/db_tests.rs index dc3faae5cd..15a90fcfed 100644 --- a/crates/collab/tests/integration/db_tests/db_tests.rs +++ b/crates/collab/tests/integration/db_tests/db_tests.rs @@ -15,19 +15,8 @@ test_both_dbs!( async fn test_add_contacts(db: &Arc) { let mut user_ids = Vec::new(); - for i in 0..3 { - user_ids.push( - db.create_user( - false, - NewUserParams { - github_login: format!("user{i}"), - github_user_id: i, - }, - ) - .await - .unwrap() - .user_id, - ); + for _ in 0..3 { + user_ids.push(db.create_user(false).await.unwrap().user_id); } let user_1 = user_ids[0]; @@ -174,26 +163,8 @@ test_both_dbs!( async fn test_project_count(db: &Arc) { let owner_id = db.create_server("test").await.unwrap().0 as u32; - let user1 = db - .create_user( - true, - NewUserParams { - github_login: "admin".into(), - github_user_id: 0, - }, - ) - .await - .unwrap(); - let user2 = db - .create_user( - false, - NewUserParams { - github_login: "user".into(), - github_user_id: 1, - }, - ) - .await - .unwrap(); + let user1 = db.create_user(true).await.unwrap(); + let user2 = db.create_user(false).await.unwrap(); let room_id = RoomId::from_proto( db.create_room(user1.user_id, ConnectionId { owner_id, id: 0 }, "") @@ -268,7 +239,7 @@ async fn test_upsert_shared_thread(db: &Arc) { use collab::db::SharedThreadId; use uuid::Uuid; - let user_id = new_test_user(db, "user1@example.com").await; + let user_id = new_test_user(db).await; let thread_id = SharedThreadId(Uuid::new_v4()); let title = "My Test Thread"; @@ -298,7 +269,7 @@ async fn test_upsert_shared_thread_updates_existing(db: &Arc) { use collab::db::SharedThreadId; use uuid::Uuid; - let user_id = new_test_user(db, "user1@example.com").await; + let user_id = new_test_user(db).await; let thread_id = SharedThreadId(Uuid::new_v4()); @@ -339,8 +310,8 @@ async fn test_cannot_update_another_users_shared_thread(db: &Arc) { use collab::db::SharedThreadId; use uuid::Uuid; - let user1_id = new_test_user(db, "user1@example.com").await; - let user2_id = new_test_user(db, "user2@example.com").await; + let user1_id = new_test_user(db).await; + let user2_id = new_test_user(db).await; let thread_id = SharedThreadId(Uuid::new_v4()); diff --git a/crates/collab/tests/integration/randomized_test_helpers.rs b/crates/collab/tests/integration/randomized_test_helpers.rs index 98e64ced41..7e1aa2677b 100644 --- a/crates/collab/tests/integration/randomized_test_helpers.rs +++ b/crates/collab/tests/integration/randomized_test_helpers.rs @@ -1,7 +1,7 @@ use crate::{TestClient, TestServer}; use async_trait::async_trait; use collab::{ - db::{self, NewUserParams, UserId}, + db::{self, UserId}, rpc::{CLEANUP_TIMEOUT, RECONNECT_TIMEOUT}, }; use futures::StreamExt; @@ -224,13 +224,7 @@ impl TestPlan { let user_id = server .app_state .db - .create_user( - false, - NewUserParams { - github_login: username.clone(), - github_user_id: ix as i32, - }, - ) + .create_user(false) .await .unwrap() .user_id;