Minor adjustments

Mikayla created

Change summary

.cargo/config.toml                                |  2 
crates/channel/src/channel_store.rs               |  1 
crates/channel/src/channel_store/channel_index.rs |  5 
crates/collab/src/db/ids.rs                       |  9 ++
crates/collab/src/db/queries/channels.rs          | 61 ++++++++--------
5 files changed, 42 insertions(+), 36 deletions(-)

Detailed changes

.cargo/config.toml 🔗

@@ -3,4 +3,4 @@ xtask = "run --package xtask --"
 
 [build]
 # v0 mangling scheme provides more detailed backtraces around closures
-rustflags = ["-C", "symbol-mangling-version=v0", "-C", "link-arg=-fuse-ld=/opt/homebrew/Cellar/llvm/16.0.6/bin/ld64.lld"]
+rustflags = ["-C", "symbol-mangling-version=v0"]

crates/channel/src/channel_store.rs 🔗

@@ -972,7 +972,6 @@ impl ChannelStore {
 
         let mut all_user_ids = Vec::new();
         let channel_participants = payload.channel_participants;
-        dbg!(&channel_participants);
         for entry in &channel_participants {
             for user_id in entry.participant_user_ids.iter() {
                 if let Err(ix) = all_user_ids.binary_search(user_id) {

crates/channel/src/channel_store/channel_index.rs 🔗

@@ -123,8 +123,9 @@ impl<'a> ChannelPathsInsertGuard<'a> {
 
     pub fn insert(&mut self, channel_proto: proto::Channel) {
         if let Some(existing_channel) = self.channels_by_id.get_mut(&channel_proto.id) {
-            Arc::make_mut(existing_channel).visibility = channel_proto.visibility();
-            Arc::make_mut(existing_channel).name = channel_proto.name;
+            let existing_channel = Arc::make_mut(existing_channel);
+            existing_channel.visibility = channel_proto.visibility();
+            existing_channel.name = channel_proto.name;
         } else {
             self.channels_by_id.insert(
                 channel_proto.id,

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

@@ -106,6 +106,15 @@ impl ChannelRole {
             Guest => false,
         }
     }
+
+    pub fn max(&self, other: Self) -> Self {
+        match (self, other) {
+            (ChannelRole::Admin, _) | (_, ChannelRole::Admin) => ChannelRole::Admin,
+            (ChannelRole::Member, _) | (_, ChannelRole::Member) => ChannelRole::Member,
+            (ChannelRole::Banned, _) | (_, ChannelRole::Banned) => ChannelRole::Banned,
+            (ChannelRole::Guest, _) | (_, ChannelRole::Guest) => ChannelRole::Guest,
+        }
+    }
 }
 
 impl From<proto::ChannelRole> for ChannelRole {

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

@@ -209,7 +209,7 @@ impl Database {
             let mut channels_to_remove: HashSet<ChannelId> = HashSet::default();
             channels_to_remove.insert(channel_id);
 
-            let graph = self.get_channel_descendants_2([channel_id], &*tx).await?;
+            let graph = self.get_channel_descendants([channel_id], &*tx).await?;
             for edge in graph.iter() {
                 channels_to_remove.insert(ChannelId::from_proto(edge.channel_id));
             }
@@ -218,7 +218,7 @@ impl Database {
                 let mut channels_to_keep = channel_path::Entity::find()
                     .filter(
                         channel_path::Column::ChannelId
-                            .is_in(channels_to_remove.clone())
+                            .is_in(channels_to_remove.iter().copied())
                             .and(
                                 channel_path::Column::IdPath
                                     .not_like(&format!("%/{}/%", channel_id)),
@@ -243,7 +243,7 @@ impl Database {
                 .await?;
 
             channel::Entity::delete_many()
-                .filter(channel::Column::Id.is_in(channels_to_remove.clone()))
+                .filter(channel::Column::Id.is_in(channels_to_remove.iter().copied()))
                 .exec(&*tx)
                 .await?;
 
@@ -484,7 +484,7 @@ impl Database {
         tx: &DatabaseTransaction,
     ) -> Result<ChannelsForUser> {
         let mut edges = self
-            .get_channel_descendants_2(channel_memberships.iter().map(|m| m.channel_id), &*tx)
+            .get_channel_descendants(channel_memberships.iter().map(|m| m.channel_id), &*tx)
             .await?;
 
         let mut role_for_channel: HashMap<ChannelId, ChannelRole> = HashMap::default();
@@ -515,7 +515,7 @@ impl Database {
         let mut channels_to_remove: HashSet<u64> = HashSet::default();
 
         let mut rows = channel::Entity::find()
-            .filter(channel::Column::Id.is_in(role_for_channel.keys().cloned()))
+            .filter(channel::Column::Id.is_in(role_for_channel.keys().copied()))
             .stream(&*tx)
             .await?;
 
@@ -877,7 +877,7 @@ impl Database {
         let channel_ids = self.get_channel_ancestors(channel_id, tx).await?;
 
         let rows = channel::Entity::find()
-            .filter(channel::Column::Id.is_in(channel_ids.clone()))
+            .filter(channel::Column::Id.is_in(channel_ids.iter().copied()))
             .filter(channel::Column::Visibility.eq(ChannelVisibility::Public))
             .all(&*tx)
             .await?;
@@ -928,40 +928,39 @@ impl Database {
             .stream(&*tx)
             .await?;
 
-        let mut is_admin = false;
-        let mut is_member = false;
+        let mut user_role: Option<ChannelRole> = None;
+        let max_role = |role| {
+            user_role
+                .map(|user_role| user_role.max(role))
+                .get_or_insert(role);
+        };
+
         let mut is_participant = false;
-        let mut is_banned = false;
         let mut current_channel_visibility = None;
 
         // note these channels are not iterated in any particular order,
         // our current logic takes the highest permission available.
         while let Some(row) = rows.next().await {
-            let (ch_id, role, visibility): (ChannelId, ChannelRole, ChannelVisibility) = row?;
+            let (membership_channel, role, visibility): (
+                ChannelId,
+                ChannelRole,
+                ChannelVisibility,
+            ) = row?;
             match role {
-                ChannelRole::Admin => is_admin = true,
-                ChannelRole::Member => is_member = true,
-                ChannelRole::Guest => {
-                    if visibility == ChannelVisibility::Public {
-                        is_participant = true
-                    }
+                ChannelRole::Admin | ChannelRole::Member | ChannelRole::Banned => max_role(role),
+                ChannelRole::Guest if visibility == ChannelVisibility::Public => {
+                    is_participant = true
                 }
-                ChannelRole::Banned => is_banned = true,
+                ChannelRole::Guest => {}
             }
-            if channel_id == ch_id {
+            if channel_id == membership_channel {
                 current_channel_visibility = Some(visibility);
             }
         }
         // free up database connection
         drop(rows);
 
-        Ok(if is_admin {
-            Some(ChannelRole::Admin)
-        } else if is_member {
-            Some(ChannelRole::Member)
-        } else if is_banned {
-            Some(ChannelRole::Banned)
-        } else if is_participant {
+        if is_participant && user_role.is_none() {
             if current_channel_visibility.is_none() {
                 current_channel_visibility = channel::Entity::find()
                     .filter(channel::Column::Id.eq(channel_id))
@@ -970,13 +969,11 @@ impl Database {
                     .map(|channel| channel.visibility);
             }
             if current_channel_visibility == Some(ChannelVisibility::Public) {
-                Some(ChannelRole::Guest)
-            } else {
-                None
+                user_role = Some(ChannelRole::Guest);
             }
-        } else {
-            None
-        })
+        }
+
+        Ok(user_role)
     }
 
     /// Returns the channel ancestors in arbitrary order
@@ -1007,7 +1004,7 @@ impl Database {
     // Returns the channel desendants as a sorted list of edges for further processing.
     // The edges are sorted such that you will see unknown channel ids as children
     // before you see them as parents.
-    async fn get_channel_descendants_2(
+    async fn get_channel_descendants(
         &self,
         channel_ids: impl IntoIterator<Item = ChannelId>,
         tx: &DatabaseTransaction,