Add a role column to the database and start using it

Conrad Irwin created

We cannot yet stop using `admin` because stable will continue writing
it.

Change summary

crates/collab/migrations.sqlite/20221109000000_test_schema.sql |  1 
crates/collab/migrations/20231011214412_add_guest_role.sql     |  4 
crates/collab/src/db/ids.rs                                    | 11 +
crates/collab/src/db/queries/channels.rs                       | 34 ++-
crates/collab/src/db/tables/channel_member.rs                  |  6 
crates/collab/src/db/tests/buffer_tests.rs                     |  4 
crates/collab/src/db/tests/channel_tests.rs                    | 18 +
crates/collab/src/db/tests/message_tests.rs                    |  6 
crates/collab/src/rpc.rs                                       | 18 +
crates/collab/src/tests/random_channel_buffer_tests.rs         |  4 
10 files changed, 76 insertions(+), 30 deletions(-)

Detailed changes

crates/collab/migrations.sqlite/20221109000000_test_schema.sql 🔗

@@ -226,6 +226,7 @@ CREATE TABLE "channel_members" (
     "channel_id" INTEGER NOT NULL REFERENCES channels (id) ON DELETE CASCADE,
     "user_id" INTEGER NOT NULL REFERENCES users (id) ON DELETE CASCADE,
     "admin" BOOLEAN NOT NULL DEFAULT false,
+    "role" VARCHAR,
     "accepted" BOOLEAN NOT NULL DEFAULT false,
     "updated_at" TIMESTAMP NOT NULL DEFAULT now
 );

crates/collab/src/db/ids.rs 🔗

@@ -80,3 +80,14 @@ id_type!(SignupId);
 id_type!(UserId);
 id_type!(ChannelBufferCollaboratorId);
 id_type!(FlagId);
+
+#[derive(Eq, PartialEq, Copy, Clone, Debug, EnumIter, DeriveActiveEnum)]
+#[sea_orm(rs_type = "String", db_type = "String(None)")]
+pub enum ChannelRole {
+    #[sea_orm(string_value = "admin")]
+    Admin,
+    #[sea_orm(string_value = "member")]
+    Member,
+    #[sea_orm(string_value = "guest")]
+    Guest,
+}

crates/collab/src/db/queries/channels.rs 🔗

@@ -74,11 +74,12 @@ impl Database {
             }
 
             channel_member::ActiveModel {
+                id: ActiveValue::NotSet,
                 channel_id: ActiveValue::Set(channel.id),
                 user_id: ActiveValue::Set(creator_id),
                 accepted: ActiveValue::Set(true),
                 admin: ActiveValue::Set(true),
-                ..Default::default()
+                role: ActiveValue::Set(Some(ChannelRole::Admin)),
             }
             .insert(&*tx)
             .await?;
@@ -160,18 +161,19 @@ impl Database {
         channel_id: ChannelId,
         invitee_id: UserId,
         inviter_id: UserId,
-        is_admin: bool,
+        role: ChannelRole,
     ) -> Result<()> {
         self.transaction(move |tx| async move {
             self.check_user_is_channel_admin(channel_id, inviter_id, &*tx)
                 .await?;
 
             channel_member::ActiveModel {
+                id: ActiveValue::NotSet,
                 channel_id: ActiveValue::Set(channel_id),
                 user_id: ActiveValue::Set(invitee_id),
                 accepted: ActiveValue::Set(false),
-                admin: ActiveValue::Set(is_admin),
-                ..Default::default()
+                admin: ActiveValue::Set(role == ChannelRole::Admin),
+                role: ActiveValue::Set(Some(role)),
             }
             .insert(&*tx)
             .await?;
@@ -417,7 +419,13 @@ impl Database {
 
         let channels_with_admin_privileges = channel_memberships
             .iter()
-            .filter_map(|membership| membership.admin.then_some(membership.channel_id))
+            .filter_map(|membership| {
+                if membership.role == Some(ChannelRole::Admin) || membership.admin {
+                    Some(membership.channel_id)
+                } else {
+                    None
+                }
+            })
             .collect();
 
         let graph = self
@@ -470,12 +478,12 @@ impl Database {
             .await
     }
 
-    pub async fn set_channel_member_admin(
+    pub async fn set_channel_member_role(
         &self,
         channel_id: ChannelId,
         from: UserId,
         for_user: UserId,
-        admin: bool,
+        role: ChannelRole,
     ) -> Result<()> {
         self.transaction(|tx| async move {
             self.check_user_is_channel_admin(channel_id, from, &*tx)
@@ -488,7 +496,8 @@ impl Database {
                         .and(channel_member::Column::UserId.eq(for_user)),
                 )
                 .set(channel_member::ActiveModel {
-                    admin: ActiveValue::set(admin),
+                    admin: ActiveValue::set(role == ChannelRole::Admin),
+                    role: ActiveValue::set(Some(role)),
                     ..Default::default()
                 })
                 .exec(&*tx)
@@ -516,6 +525,7 @@ impl Database {
             enum QueryMemberDetails {
                 UserId,
                 Admin,
+                Role,
                 IsDirectMember,
                 Accepted,
             }
@@ -528,6 +538,7 @@ impl Database {
                 .select_only()
                 .column(channel_member::Column::UserId)
                 .column(channel_member::Column::Admin)
+                .column(channel_member::Column::Role)
                 .column_as(
                     channel_member::Column::ChannelId.eq(channel_id),
                     QueryMemberDetails::IsDirectMember,
@@ -540,9 +551,10 @@ impl Database {
 
             let mut rows = Vec::<proto::ChannelMember>::new();
             while let Some(row) = stream.next().await {
-                let (user_id, is_admin, is_direct_member, is_invite_accepted): (
+                let (user_id, is_admin, channel_role, is_direct_member, is_invite_accepted): (
                     UserId,
                     bool,
+                    Option<ChannelRole>,
                     bool,
                     bool,
                 ) = row?;
@@ -558,7 +570,7 @@ impl Database {
                     if last_row.user_id == user_id {
                         if is_direct_member {
                             last_row.kind = kind;
-                            last_row.admin = is_admin;
+                            last_row.admin = channel_role == Some(ChannelRole::Admin) || is_admin;
                         }
                         continue;
                     }
@@ -566,7 +578,7 @@ impl Database {
                 rows.push(proto::ChannelMember {
                     user_id,
                     kind,
-                    admin: is_admin,
+                    admin: channel_role == Some(ChannelRole::Admin) || is_admin,
                 });
             }
 

crates/collab/src/db/tables/channel_member.rs 🔗

@@ -1,7 +1,7 @@
-use crate::db::{channel_member, ChannelId, ChannelMemberId, UserId};
+use crate::db::{channel_member, ChannelId, ChannelMemberId, ChannelRole, UserId};
 use sea_orm::entity::prelude::*;
 
-#[derive(Clone, Debug, Default, PartialEq, Eq, DeriveEntityModel)]
+#[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)]
 #[sea_orm(table_name = "channel_members")]
 pub struct Model {
     #[sea_orm(primary_key)]
@@ -10,6 +10,8 @@ pub struct Model {
     pub user_id: UserId,
     pub accepted: bool,
     pub admin: bool,
+    // only optional while migrating
+    pub role: Option<ChannelRole>,
 }
 
 impl ActiveModelBehavior for ActiveModel {}

crates/collab/src/db/tests/buffer_tests.rs 🔗

@@ -56,7 +56,7 @@ async fn test_channel_buffers(db: &Arc<Database>) {
 
     let zed_id = db.create_root_channel("zed", a_id).await.unwrap();
 
-    db.invite_channel_member(zed_id, b_id, a_id, false)
+    db.invite_channel_member(zed_id, b_id, a_id, ChannelRole::Member)
         .await
         .unwrap();
 
@@ -211,7 +211,7 @@ async fn test_channel_buffers_last_operations(db: &Database) {
             .await
             .unwrap();
 
-        db.invite_channel_member(channel, observer_id, user_id, false)
+        db.invite_channel_member(channel, observer_id, user_id, ChannelRole::Member)
             .await
             .unwrap();
         db.respond_to_channel_invite(channel, observer_id, true)

crates/collab/src/db/tests/channel_tests.rs 🔗

@@ -8,7 +8,7 @@ use crate::{
     db::{
         queries::channels::ChannelGraph,
         tests::{graph, TEST_RELEASE_CHANNEL},
-        ChannelId, Database, NewUserParams,
+        ChannelId, ChannelRole, Database, NewUserParams,
     },
     test_both_dbs,
 };
@@ -50,7 +50,7 @@ async fn test_channels(db: &Arc<Database>) {
     // Make sure that people cannot read channels they haven't been invited to
     assert!(db.get_channel(zed_id, b_id).await.unwrap().is_none());
 
-    db.invite_channel_member(zed_id, b_id, a_id, false)
+    db.invite_channel_member(zed_id, b_id, a_id, ChannelRole::Member)
         .await
         .unwrap();
 
@@ -125,9 +125,13 @@ async fn test_channels(db: &Arc<Database>) {
     );
 
     // Update member permissions
-    let set_subchannel_admin = db.set_channel_member_admin(crdb_id, a_id, b_id, true).await;
+    let set_subchannel_admin = db
+        .set_channel_member_role(crdb_id, a_id, b_id, ChannelRole::Admin)
+        .await;
     assert!(set_subchannel_admin.is_err());
-    let set_channel_admin = db.set_channel_member_admin(zed_id, a_id, b_id, true).await;
+    let set_channel_admin = db
+        .set_channel_member_role(zed_id, a_id, b_id, ChannelRole::Admin)
+        .await;
     assert!(set_channel_admin.is_ok());
 
     let result = db.get_channels_for_user(b_id).await.unwrap();
@@ -284,13 +288,13 @@ async fn test_channel_invites(db: &Arc<Database>) {
 
     let channel_1_2 = db.create_root_channel("channel_2", user_1).await.unwrap();
 
-    db.invite_channel_member(channel_1_1, user_2, user_1, false)
+    db.invite_channel_member(channel_1_1, user_2, user_1, ChannelRole::Member)
         .await
         .unwrap();
-    db.invite_channel_member(channel_1_2, user_2, user_1, false)
+    db.invite_channel_member(channel_1_2, user_2, user_1, ChannelRole::Member)
         .await
         .unwrap();
-    db.invite_channel_member(channel_1_1, user_3, user_1, true)
+    db.invite_channel_member(channel_1_1, user_3, user_1, ChannelRole::Admin)
         .await
         .unwrap();
 

crates/collab/src/db/tests/message_tests.rs 🔗

@@ -1,5 +1,5 @@
 use crate::{
-    db::{Database, MessageId, NewUserParams},
+    db::{ChannelRole, Database, MessageId, NewUserParams},
     test_both_dbs,
 };
 use std::sync::Arc;
@@ -155,7 +155,7 @@ async fn test_channel_message_new_notification(db: &Arc<Database>) {
 
     let channel_2 = db.create_channel("channel-2", None, user).await.unwrap();
 
-    db.invite_channel_member(channel_1, observer, user, false)
+    db.invite_channel_member(channel_1, observer, user, ChannelRole::Member)
         .await
         .unwrap();
 
@@ -163,7 +163,7 @@ async fn test_channel_message_new_notification(db: &Arc<Database>) {
         .await
         .unwrap();
 
-    db.invite_channel_member(channel_2, observer, user, false)
+    db.invite_channel_member(channel_2, observer, user, ChannelRole::Member)
         .await
         .unwrap();
 

crates/collab/src/rpc.rs 🔗

@@ -3,8 +3,8 @@ mod connection_pool;
 use crate::{
     auth,
     db::{
-        self, BufferId, ChannelId, ChannelsForUser, Database, MessageId, ProjectId, RoomId,
-        ServerId, User, UserId,
+        self, BufferId, ChannelId, ChannelRole, ChannelsForUser, Database, MessageId, ProjectId,
+        RoomId, ServerId, User, UserId,
     },
     executor::Executor,
     AppState, Result,
@@ -2282,7 +2282,12 @@ async fn invite_channel_member(
     let db = session.db().await;
     let channel_id = ChannelId::from_proto(request.channel_id);
     let invitee_id = UserId::from_proto(request.user_id);
-    db.invite_channel_member(channel_id, invitee_id, session.user_id, request.admin)
+    let role = if request.admin {
+        ChannelRole::Admin
+    } else {
+        ChannelRole::Member
+    };
+    db.invite_channel_member(channel_id, invitee_id, session.user_id, role)
         .await?;
 
     let (channel, _) = db
@@ -2342,7 +2347,12 @@ async fn set_channel_member_admin(
     let db = session.db().await;
     let channel_id = ChannelId::from_proto(request.channel_id);
     let member_id = UserId::from_proto(request.user_id);
-    db.set_channel_member_admin(channel_id, session.user_id, member_id, request.admin)
+    let role = if request.admin {
+        ChannelRole::Admin
+    } else {
+        ChannelRole::Member
+    };
+    db.set_channel_member_role(channel_id, session.user_id, member_id, role)
         .await?;
 
     let (channel, has_accepted) = db

crates/collab/src/tests/random_channel_buffer_tests.rs 🔗

@@ -1,3 +1,5 @@
+use crate::db::ChannelRole;
+
 use super::{run_randomized_test, RandomizedTest, TestClient, TestError, TestServer, UserTestPlan};
 use anyhow::Result;
 use async_trait::async_trait;
@@ -50,7 +52,7 @@ impl RandomizedTest for RandomChannelBufferTest {
                 .await
                 .unwrap();
             for user in &users[1..] {
-                db.invite_channel_member(id, user.user_id, users[0].user_id, false)
+                db.invite_channel_member(id, user.user_id, users[0].user_id, ChannelRole::Member)
                     .await
                     .unwrap();
                 db.respond_to_channel_invite(id, user.user_id, true)