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
This commit is contained in:
1 parent
93b08ef699
commit
ca91e34701
10 files changed
+39
-214
No files matched your search
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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<NewUserResult> {
|
||||
pub async fn create_user(&self, admin: bool) -> Result<NewUserResult> {
|
||||
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?;
|
||||
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
|
||||
@@ -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<Cha
|
||||
result
|
||||
}
|
||||
|
||||
static GITHUB_USER_ID: AtomicI32 = AtomicI32::new(5);
|
||||
|
||||
async fn new_test_user(db: &Arc<Database>, 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<Database>) -> UserId {
|
||||
db.create_user(false).await.unwrap().user_id
|
||||
}
|
||||
@@ -11,41 +11,11 @@ test_both_dbs!(
|
||||
);
|
||||
|
||||
async fn test_channel_buffers(db: &Arc<Database>) {
|
||||
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,
|
||||
|
||||
@@ -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<Database>) {
|
||||
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<Database>) {
|
||||
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<Database>) {
|
||||
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<Database>) {
|
||||
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<Database>) {
|
||||
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<Database>) {
|
||||
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<Database>) {
|
||||
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<Database>) {
|
||||
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<Database>) {
|
||||
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)
|
||||
|
||||
@@ -15,19 +15,8 @@ test_both_dbs!(
|
||||
|
||||
async fn test_add_contacts(db: &Arc<Database>) {
|
||||
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<Database>) {
|
||||
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<Database>) {
|
||||
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<Database>) {
|
||||
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<Database>) {
|
||||
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());
|
||||
|
||||
|
||||
@@ -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<T: RandomizedTest> TestPlan<T> {
|
||||
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;
|
||||
|
||||
Reference in new issue
Block a user