Don't expose channel admin actions in UI if user isn't admin

Max Brunsfeld created

Change summary

crates/client/src/channel_store.rs       |   5 
crates/collab/src/rpc.rs                 |  29 ++++--
crates/collab/src/tests/channel_tests.rs | 105 +++++++++++++++++++++----
crates/collab_ui/src/collab_panel.rs     |  28 +++--
4 files changed, 123 insertions(+), 44 deletions(-)

Detailed changes

crates/client/src/channel_store.rs 🔗

@@ -263,13 +263,14 @@ impl ChannelStore {
 
             if let Some(parent_id) = channel.parent_id {
                 if let Some(ix) = self.channels.iter().position(|c| c.id == parent_id) {
-                    let depth = self.channels[ix].depth + 1;
+                    let parent_channel = &self.channels[ix];
+                    let depth = parent_channel.depth + 1;
                     self.channels.insert(
                         ix + 1,
                         Arc::new(Channel {
                             id: channel.id,
                             name: channel.name,
-                            user_is_admin: channel.user_is_admin,
+                            user_is_admin: channel.user_is_admin || parent_channel.user_is_admin,
                             parent_id: Some(parent_id),
                             depth,
                         }),

crates/collab/src/rpc.rs 🔗

@@ -2209,7 +2209,7 @@ async fn invite_channel_member(
         .await?
         .ok_or_else(|| anyhow!("channel not found"))?;
     let invitee_id = UserId::from_proto(request.user_id);
-    db.invite_channel_member(channel_id, invitee_id, session.user_id, false)
+    db.invite_channel_member(channel_id, invitee_id, session.user_id, request.admin)
         .await?;
 
     let mut update = proto::UpdateChannels::default();
@@ -2268,22 +2268,29 @@ async fn respond_to_channel_invite(
     let channel_id = ChannelId::from_proto(request.channel_id);
     db.respond_to_channel_invite(channel_id, session.user_id, request.accept)
         .await?;
-    let channel = db
-        .get_channel(channel_id, session.user_id)
-        .await?
-        .ok_or_else(|| anyhow!("no such channel"))?;
 
     let mut update = proto::UpdateChannels::default();
     update
         .remove_channel_invitations
         .push(channel_id.to_proto());
     if request.accept {
-        update.channels.push(proto::Channel {
-            id: channel.id.to_proto(),
-            name: channel.name,
-            user_is_admin: channel.user_is_admin,
-            parent_id: None,
-        });
+        let (channels, participants) = db.get_channels_for_user(session.user_id).await?;
+        update
+            .channels
+            .extend(channels.into_iter().map(|channel| proto::Channel {
+                id: channel.id.to_proto(),
+                name: channel.name,
+                user_is_admin: channel.user_is_admin,
+                parent_id: channel.parent_id.map(ChannelId::to_proto),
+            }));
+        update
+            .channel_participants
+            .extend(participants.into_iter().map(|(channel_id, user_ids)| {
+                proto::ChannelParticipants {
+                    channel_id: channel_id.to_proto(),
+                    participant_user_ids: user_ids.into_iter().map(UserId::to_proto).collect(),
+                }
+            }));
     }
     session.peer.send(session.connection_id, update)?;
     response.send(proto::Ack {})?;

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

@@ -23,18 +23,34 @@ async fn test_basic_channels(
         })
         .await
         .unwrap();
+    let channel_b_id = client_a
+        .channel_store()
+        .update(cx_a, |channel_store, _| {
+            channel_store.create_channel("channel-b", Some(channel_a_id))
+        })
+        .await
+        .unwrap();
 
     deterministic.run_until_parked();
     client_a.channel_store().read_with(cx_a, |channels, _| {
         assert_eq!(
             channels.channels(),
-            &[Arc::new(Channel {
-                id: channel_a_id,
-                name: "channel-a".to_string(),
-                parent_id: None,
-                user_is_admin: true,
-                depth: 0,
-            })]
+            &[
+                Arc::new(Channel {
+                    id: channel_a_id,
+                    name: "channel-a".to_string(),
+                    parent_id: None,
+                    user_is_admin: true,
+                    depth: 0,
+                }),
+                Arc::new(Channel {
+                    id: channel_b_id,
+                    name: "channel-b".to_string(),
+                    parent_id: Some(channel_a_id),
+                    user_is_admin: true,
+                    depth: 1,
+                })
+            ]
         )
     });
 
@@ -48,7 +64,7 @@ async fn test_basic_channels(
         .update(cx_a, |store, cx| {
             assert!(!store.has_pending_channel_invite(channel_a_id, client_b.user_id().unwrap()));
 
-            let invite = store.invite_member(channel_a_id, client_b.user_id().unwrap(), false, cx);
+            let invite = store.invite_member(channel_a_id, client_b.user_id().unwrap(), true, cx);
 
             // Make sure we're synchronously storing the pending invite
             assert!(store.has_pending_channel_invite(channel_a_id, client_b.user_id().unwrap()));
@@ -57,9 +73,8 @@ async fn test_basic_channels(
         .await
         .unwrap();
 
-    // Wait for client b to see the invitation
+    // Client A sees that B has been invited.
     deterministic.run_until_parked();
-
     client_b.channel_store().read_with(cx_b, |channels, _| {
         assert_eq!(
             channels.channel_invitations(),
@@ -69,7 +84,7 @@ async fn test_basic_channels(
                 parent_id: None,
                 user_is_admin: false,
                 depth: 0,
-            })]
+            }),]
         )
     });
     let members = client_a
@@ -94,7 +109,7 @@ async fn test_basic_channels(
         ],
     );
 
-    // Client B now sees that they are a member channel A.
+    // Client B accepts the invitation.
     client_b
         .channel_store()
         .update(cx_b, |channels, _| {
@@ -102,17 +117,69 @@ async fn test_basic_channels(
         })
         .await
         .unwrap();
+
+    // Client B now sees that they are a member of channel A and its existing
+    // subchannels. Their admin priveleges extend to subchannels of channel A.
+    deterministic.run_until_parked();
     client_b.channel_store().read_with(cx_b, |channels, _| {
         assert_eq!(channels.channel_invitations(), &[]);
         assert_eq!(
             channels.channels(),
-            &[Arc::new(Channel {
-                id: channel_a_id,
-                name: "channel-a".to_string(),
-                parent_id: None,
-                user_is_admin: false,
-                depth: 0,
-            })]
+            &[
+                Arc::new(Channel {
+                    id: channel_a_id,
+                    name: "channel-a".to_string(),
+                    parent_id: None,
+                    user_is_admin: true,
+                    depth: 0,
+                }),
+                Arc::new(Channel {
+                    id: channel_b_id,
+                    name: "channel-b".to_string(),
+                    parent_id: Some(channel_a_id),
+                    user_is_admin: true,
+                    depth: 1,
+                })
+            ]
+        )
+    });
+
+    let channel_c_id = client_a
+        .channel_store()
+        .update(cx_a, |channel_store, _| {
+            channel_store.create_channel("channel-c", Some(channel_a_id))
+        })
+        .await
+        .unwrap();
+
+    // TODO - ensure sibling channels are sorted in a stable way
+    deterministic.run_until_parked();
+    client_b.channel_store().read_with(cx_b, |channels, _| {
+        assert_eq!(
+            channels.channels(),
+            &[
+                Arc::new(Channel {
+                    id: channel_a_id,
+                    name: "channel-a".to_string(),
+                    parent_id: None,
+                    user_is_admin: true,
+                    depth: 0,
+                }),
+                Arc::new(Channel {
+                    id: channel_c_id,
+                    name: "channel-c".to_string(),
+                    parent_id: Some(channel_a_id),
+                    user_is_admin: true,
+                    depth: 1,
+                }),
+                Arc::new(Channel {
+                    id: channel_b_id,
+                    name: "channel-b".to_string(),
+                    parent_id: Some(channel_a_id),
+                    user_is_admin: true,
+                    depth: 1,
+                }),
+            ]
         )
     });
 

crates/collab_ui/src/collab_panel.rs 🔗

@@ -1509,18 +1509,22 @@ impl CollabPanel {
         channel_id: u64,
         cx: &mut ViewContext<Self>,
     ) {
-        self.context_menu.update(cx, |context_menu, cx| {
-            context_menu.show(
-                position,
-                gpui::elements::AnchorCorner::BottomLeft,
-                vec![
-                    ContextMenuItem::action("New Channel", NewChannel { channel_id }),
-                    ContextMenuItem::action("Remove Channel", RemoveChannel { channel_id }),
-                    ContextMenuItem::action("Add member", AddMember { channel_id }),
-                ],
-                cx,
-            );
-        });
+        if let Some(channel) = self.channel_store.read(cx).channel_for_id(channel_id) {
+            if channel.user_is_admin {
+                self.context_menu.update(cx, |context_menu, cx| {
+                    context_menu.show(
+                        position,
+                        gpui::elements::AnchorCorner::BottomLeft,
+                        vec![
+                            ContextMenuItem::action("New Channel", NewChannel { channel_id }),
+                            ContextMenuItem::action("Remove Channel", RemoveChannel { channel_id }),
+                            ContextMenuItem::action("Add member", AddMember { channel_id }),
+                        ],
+                        cx,
+                    );
+                });
+            }
+        }
     }
 
     fn cancel(&mut self, _: &Cancel, cx: &mut ViewContext<Self>) {