@@ -761,20 +761,41 @@ impl Database {
.await
}
- async fn link_channel(
+ // Insert an edge from the given channel to the given other channel.
+ pub async fn link_channel(
&self,
- from: ChannelId,
+ user: UserId,
+ channel: ChannelId,
+ to: ChannelId,
+ ) -> Result<Vec<Channel>> {
+ self.transaction(|tx| async move {
+ // Note that even with these maxed permissions, this linking operation
+ // is still insecure because you can't remove someone's permissions to a
+ // channel if they've linked the channel to one where they're an admin.
+ self.check_user_is_channel_admin(channel, user, &*tx)
+ .await?;
+
+ self.link_channel_internal(user, channel, to, &*tx).await
+ })
+ .await
+ }
+
+ pub async fn link_channel_internal(
+ &self,
+ user: UserId,
+ channel: ChannelId,
to: ChannelId,
tx: &DatabaseTransaction,
- ) -> Result<ChannelDescendants> {
+ ) -> Result<Vec<Channel>> {
+ self.check_user_is_channel_admin(to, user, &*tx).await?;
+
let to_ancestors = self.get_channel_ancestors(to, &*tx).await?;
- let from_descendants = self.get_channel_descendants([from], &*tx).await?;
+ let mut from_descendants = self.get_channel_descendants([channel], &*tx).await?;
for ancestor in to_ancestors {
if from_descendants.contains_key(&ancestor) {
return Err(anyhow!("Cannot create a channel cycle").into());
}
}
-
let sql = r#"
INSERT INTO channel_paths
(id_path, channel_id)
@@ -790,14 +811,13 @@ impl Database {
self.pool.get_database_backend(),
sql,
[
- from.to_proto().into(),
- from.to_proto().into(),
+ channel.to_proto().into(),
+ channel.to_proto().into(),
to.to_proto().into(),
],
);
tx.execute(channel_paths_stmt).await?;
-
- for (from_id, to_ids) in from_descendants.iter().filter(|(id, _)| id != &&from) {
+ for (from_id, to_ids) in from_descendants.iter().filter(|(id, _)| id != &&channel) {
for to_id in to_ids {
let channel_paths_stmt = Statement::from_sql_and_values(
self.pool.get_database_backend(),
@@ -812,94 +832,116 @@ impl Database {
}
}
- Ok(from_descendants)
+ if let Some(channel) = from_descendants.get_mut(&channel) {
+ // Remove the other parents
+ channel.clear();
+ channel.insert(to);
+ }
+
+ let channels = self.get_all_channels(from_descendants, &*tx).await?;
+
+ Ok(channels)
+ }
+
+ /// Unlink a channel from a given parent. This will add in a root edge if
+ /// the channel has no other parents after this operation.
+ pub async fn unlink_channel(
+ &self,
+ user: UserId,
+ channel: ChannelId,
+ from: Option<ChannelId>,
+ ) -> Result<()> {
+ self.transaction(|tx| async move {
+ // Note that even with these maxed permissions, this linking operation
+ // is still insecure because you can't remove someone's permissions to a
+ // channel if they've linked the channel to one where they're an admin.
+ self.check_user_is_channel_admin(channel, user, &*tx)
+ .await?;
+
+ self.unlink_channel_internal(user, channel, from, &*tx)
+ .await?;
+
+ Ok(())
+ })
+ .await
}
- async fn remove_channel_from_parent(
+ pub async fn unlink_channel_internal(
&self,
- from: ChannelId,
- parent: ChannelId,
+ user: UserId,
+ channel: ChannelId,
+ from: Option<ChannelId>,
tx: &DatabaseTransaction,
) -> Result<()> {
- let sql = r#"
+ if let Some(from) = from {
+ self.check_user_is_channel_admin(from, user, &*tx).await?;
+
+ let sql = r#"
DELETE FROM channel_paths
WHERE
id_path LIKE '%' || $1 || '/' || $2 || '%'
"#;
+ let channel_paths_stmt = Statement::from_sql_and_values(
+ self.pool.get_database_backend(),
+ sql,
+ [from.to_proto().into(), channel.to_proto().into()],
+ );
+ tx.execute(channel_paths_stmt).await?;
+ } else {
+ let sql = r#"
+ DELETE FROM channel_paths
+ WHERE
+ id_path = '/' || $1 || '/'
+ "#;
+ let channel_paths_stmt = Statement::from_sql_and_values(
+ self.pool.get_database_backend(),
+ sql,
+ [channel.to_proto().into()],
+ );
+ tx.execute(channel_paths_stmt).await?;
+ }
+
+ // Make sure that there is always at least one path to the channel
+ let sql = r#"
+ INSERT INTO channel_paths
+ (id_path, channel_id)
+ SELECT
+ '/' || $1 || '/', $2
+ WHERE NOT EXISTS
+ (SELECT *
+ FROM channel_paths
+ WHERE channel_id = $2)
+ "#;
+
let channel_paths_stmt = Statement::from_sql_and_values(
self.pool.get_database_backend(),
sql,
- [parent.to_proto().into(), from.to_proto().into()],
+ [channel.to_proto().into(), channel.to_proto().into()],
);
tx.execute(channel_paths_stmt).await?;
Ok(())
}
- /// Move a channel from one parent to another.
- /// Note that this requires a valid parent_id in the 'from_parent' field.
- /// As channels are a DAG, we need to know which parent to remove the channel from.
- /// Here's a list of the parameters to this function and their behavior:
- ///
- /// - (`None`, `None`) No op
- /// - (`None`, `Some(id)`) Link the channel without removing it from any of it's parents
- /// - (`Some(id)`, `None`) Remove a channel from a given parent, and leave other parents
- /// - (`Some(id)`, `Some(id)`) Move channel from one parent to another, leaving other parents
- ///
- /// Returns the channel that was moved + it's sub channels for use
- /// by the members for `to`
+ /// Move a channel from one parent to another, returns the
+ /// Channels that were moved for notifying clients
pub async fn move_channel(
&self,
user: UserId,
- from: ChannelId,
- from_parent: Option<ChannelId>,
- to: Option<ChannelId>,
+ channel: ChannelId,
+ from: Option<ChannelId>,
+ to: ChannelId,
) -> Result<Vec<Channel>> {
self.transaction(|tx| async move {
- // Note that even with these maxed permissions, this linking operation
- // is still insecure because you can't remove someone's permissions to a
- // channel if they've linked the channel to one where they're an admin.
- self.check_user_is_channel_admin(from, user, &*tx).await?;
-
- let mut channel_descendants = None;
-
- // Note that we have to do the linking before the removal, so that we
- // can leave the channel_path table in a consistent state.
- if let Some(to) = to {
- self.check_user_is_channel_admin(to, user, &*tx).await?;
-
- channel_descendants = Some(self.link_channel(from, to, &*tx).await?);
- }
-
- let mut channel_descendants = match channel_descendants {
- Some(channel_descendants) => channel_descendants,
- None => self.get_channel_descendants([from], &*tx).await?,
- };
-
- if let Some(from_parent) = from_parent {
- self.check_user_is_channel_admin(from_parent, user, &*tx)
- .await?;
-
- self.remove_channel_from_parent(from, from_parent, &*tx)
- .await?;
- }
+ self.check_user_is_channel_admin(channel, user, &*tx)
+ .await?;
- let channels;
- if let Some(to) = to {
- if let Some(channel) = channel_descendants.get_mut(&from) {
- // Remove the other parents
- channel.clear();
- channel.insert(to);
- }
+ let moved_channels = self.link_channel_internal(user, channel, to, &*tx).await?;
- channels = self
- .get_all_channels(channel_descendants, &*tx)
- .await?;
- } else {
- channels = vec![];
- }
+ self.unlink_channel_internal(user, channel, from, &*tx)
+ .await?;
- Ok(channels)
+ Ok(moved_channels)
})
.await
}
@@ -1,7 +1,7 @@
use rpc::{proto, ConnectionId};
use crate::{
- db::{Channel, Database, NewUserParams},
+ db::{Channel, ChannelId, Database, NewUserParams},
test_both_dbs,
};
use std::sync::Arc;
@@ -501,50 +501,32 @@ async fn test_channels_moving(db: &Arc<Database>) {
.await
.unwrap();
+ // ========================================================================
// sanity check
// Initial DAG:
// /- gpui2
// zed -- crdb - livestreaming - livestreaming_dag
let result = db.get_channels_for_user(a_id).await.unwrap();
- pretty_assertions::assert_eq!(
+ assert_dag(
result.channels,
- vec![
- Channel {
- id: zed_id,
- name: "zed".to_string(),
- parent_id: None,
- },
- Channel {
- id: crdb_id,
- name: "crdb".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: gpui2_id,
- name: "gpui2".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: livestreaming_id,
- name: "livestreaming".to_string(),
- parent_id: Some(crdb_id),
- },
- Channel {
- id: livestreaming_dag_id,
- name: "livestreaming_dag".to_string(),
- parent_id: Some(livestreaming_id),
- },
- ]
+ &[
+ (zed_id, None),
+ (crdb_id, Some(zed_id)),
+ (gpui2_id, Some(zed_id)),
+ (livestreaming_id, Some(crdb_id)),
+ (livestreaming_dag_id, Some(livestreaming_id)),
+ ],
);
// Attempt to make a cycle
assert!(db
- .move_channel(a_id, zed_id, None, Some(livestreaming_id))
+ .link_channel(a_id, zed_id, livestreaming_id)
.await
.is_err());
+ // ========================================================================
// Make a link
- db.move_channel(a_id, livestreaming_id, None, Some(zed_id))
+ db.link_channel(a_id, livestreaming_id, zed_id)
.await
.unwrap();
@@ -553,42 +535,16 @@ async fn test_channels_moving(db: &Arc<Database>) {
// zed -- crdb - livestreaming - livestreaming_dag
// \---------/
let result = db.get_channels_for_user(a_id).await.unwrap();
- pretty_assertions::assert_eq!(
- result.channels,
- vec![
- Channel {
- id: zed_id,
- name: "zed".to_string(),
- parent_id: None,
- },
- Channel {
- id: crdb_id,
- name: "crdb".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: gpui2_id,
- name: "gpui2".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: livestreaming_id,
- name: "livestreaming".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: livestreaming_id,
- name: "livestreaming".to_string(),
- parent_id: Some(crdb_id),
- },
- Channel {
- id: livestreaming_dag_id,
- name: "livestreaming_dag".to_string(),
- parent_id: Some(livestreaming_id),
- },
- ]
- );
-
+ assert_dag(result.channels, &[
+ (zed_id, None),
+ (crdb_id, Some(zed_id)),
+ (gpui2_id, Some(zed_id)),
+ (livestreaming_id, Some(zed_id)),
+ (livestreaming_id, Some(crdb_id)),
+ (livestreaming_dag_id, Some(livestreaming_id)),
+ ]);
+
+ // ========================================================================
// Create a new channel below a channel with multiple parents
let livestreaming_dag_sub_id = db
.create_channel(
@@ -605,50 +561,20 @@ async fn test_channels_moving(db: &Arc<Database>) {
// zed -- crdb - livestreaming - livestreaming_dag - livestreaming_dag_sub_id
// \---------/
let result = db.get_channels_for_user(a_id).await.unwrap();
- pretty_assertions::assert_eq!(
- result.channels,
- vec![
- Channel {
- id: zed_id,
- name: "zed".to_string(),
- parent_id: None,
- },
- Channel {
- id: crdb_id,
- name: "crdb".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: gpui2_id,
- name: "gpui2".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: livestreaming_id,
- name: "livestreaming".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: livestreaming_id,
- name: "livestreaming".to_string(),
- parent_id: Some(crdb_id),
- },
- Channel {
- id: livestreaming_dag_id,
- name: "livestreaming_dag".to_string(),
- parent_id: Some(livestreaming_id),
- },
- Channel {
- id: livestreaming_dag_sub_id,
- name: "livestreaming_dag_sub".to_string(),
- parent_id: Some(livestreaming_dag_id),
- },
- ]
- );
+ assert_dag(result.channels, &[
+ (zed_id, None),
+ (crdb_id, Some(zed_id)),
+ (gpui2_id, Some(zed_id)),
+ (livestreaming_id, Some(zed_id)),
+ (livestreaming_id, Some(crdb_id)),
+ (livestreaming_dag_id, Some(livestreaming_id)),
+ (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
+ ]);
- // Make a link
- let channels = db
- .move_channel(a_id, livestreaming_dag_sub_id, None, Some(livestreaming_id))
+ // ========================================================================
+ // Test a complex DAG by making another link
+ let returned_channels = db
+ .link_channel(a_id, livestreaming_dag_sub_id, livestreaming_id)
.await
.unwrap();
@@ -658,66 +584,32 @@ async fn test_channels_moving(db: &Arc<Database>) {
// \--------/
// make sure we're getting just the new link
+ // Not using the assert_dag helper because we want to make sure we're returning the full data
pretty_assertions::assert_eq!(
- channels,
- vec![
- Channel {
- id: livestreaming_dag_sub_id,
- name: "livestreaming_dag_sub".to_string(),
- parent_id: Some(livestreaming_id),
- }
- ]
+ returned_channels,
+ vec![Channel {
+ id: livestreaming_dag_sub_id,
+ name: "livestreaming_dag_sub".to_string(),
+ parent_id: Some(livestreaming_id),
+ }]
);
let result = db.get_channels_for_user(a_id).await.unwrap();
- pretty_assertions::assert_eq!(
- result.channels,
- vec![
- Channel {
- id: zed_id,
- name: "zed".to_string(),
- parent_id: None,
- },
- Channel {
- id: crdb_id,
- name: "crdb".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: gpui2_id,
- name: "gpui2".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: livestreaming_id,
- name: "livestreaming".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: livestreaming_id,
- name: "livestreaming".to_string(),
- parent_id: Some(crdb_id),
- },
- Channel {
- id: livestreaming_dag_id,
- name: "livestreaming_dag".to_string(),
- parent_id: Some(livestreaming_id),
- },
- Channel {
- id: livestreaming_dag_sub_id,
- name: "livestreaming_dag_sub".to_string(),
- parent_id: Some(livestreaming_id),
- },
- Channel {
- id: livestreaming_dag_sub_id,
- name: "livestreaming_dag_sub".to_string(),
- parent_id: Some(livestreaming_dag_id),
- },
- ]
- );
+ assert_dag(result.channels, &[
+ (zed_id, None),
+ (crdb_id, Some(zed_id)),
+ (gpui2_id, Some(zed_id)),
+ (livestreaming_id, Some(zed_id)),
+ (livestreaming_id, Some(crdb_id)),
+ (livestreaming_dag_id, Some(livestreaming_id)),
+ (livestreaming_dag_sub_id, Some(livestreaming_id)),
+ (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
+ ]);
- // Make another link
- let channels = db.move_channel(a_id, livestreaming_id, None, Some(gpui2_id))
+ // ========================================================================
+ // Test a complex DAG by making another link
+ let returned_channels = db
+ .link_channel(a_id, livestreaming_id, gpui2_id)
.await
.unwrap();
@@ -727,62 +619,14 @@ async fn test_channels_moving(db: &Arc<Database>) {
// \---------/
// Make sure that we're correctly getting the full sub-dag
- pretty_assertions::assert_eq!(channels,
- vec![Channel {
- id: livestreaming_id,
- name: "livestreaming".to_string(),
- parent_id: Some(gpui2_id),
- },
- Channel {
- id: livestreaming_dag_id,
- name: "livestreaming_dag".to_string(),
- parent_id: Some(livestreaming_id),
- },
- Channel {
- id: livestreaming_dag_sub_id,
- name: "livestreaming_dag_sub".to_string(),
- parent_id: Some(livestreaming_id),
- },
- Channel {
- id: livestreaming_dag_sub_id,
- name: "livestreaming_dag_sub".to_string(),
- parent_id: Some(livestreaming_dag_id),
- }]);
-
- let result = db.get_channels_for_user(a_id).await.unwrap();
pretty_assertions::assert_eq!(
- result.channels,
+ returned_channels,
vec![
- Channel {
- id: zed_id,
- name: "zed".to_string(),
- parent_id: None,
- },
- Channel {
- id: crdb_id,
- name: "crdb".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: gpui2_id,
- name: "gpui2".to_string(),
- parent_id: Some(zed_id),
- },
Channel {
id: livestreaming_id,
name: "livestreaming".to_string(),
parent_id: Some(gpui2_id),
},
- Channel {
- id: livestreaming_id,
- name: "livestreaming".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: livestreaming_id,
- name: "livestreaming".to_string(),
- parent_id: Some(crdb_id),
- },
Channel {
id: livestreaming_dag_id,
name: "livestreaming_dag".to_string(),
@@ -797,12 +641,31 @@ async fn test_channels_moving(db: &Arc<Database>) {
id: livestreaming_dag_sub_id,
name: "livestreaming_dag_sub".to_string(),
parent_id: Some(livestreaming_dag_id),
- },
+ }
]
);
- // Remove that inner link
- let channels = db.move_channel(a_id, livestreaming_dag_sub_id, Some(livestreaming_id), None)
+ let result = db.get_channels_for_user(a_id).await.unwrap();
+ assert_dag(result.channels, &[
+ (zed_id, None),
+ (crdb_id, Some(zed_id)),
+ (gpui2_id, Some(zed_id)),
+ (livestreaming_id, Some(gpui2_id)),
+ (livestreaming_id, Some(zed_id)),
+ (livestreaming_id, Some(crdb_id)),
+ (livestreaming_dag_id, Some(livestreaming_id)),
+ (livestreaming_dag_sub_id, Some(livestreaming_id)),
+ (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
+ ]);
+
+ // ========================================================================
+ // Test unlinking in a complex DAG by removing the inner link
+ db
+ .unlink_channel(
+ a_id,
+ livestreaming_dag_sub_id,
+ Some(livestreaming_id),
+ )
.await
.unwrap();
@@ -811,62 +674,21 @@ async fn test_channels_moving(db: &Arc<Database>) {
// zed - crdb -- livestreaming - livestreaming_dag - livestreaming_dag_sub
// \---------/
- // Since we're not moving it to anywhere, there's nothing to notify anyone about
- pretty_assertions::assert_eq!(
- channels,
- vec![]
- );
-
-
let result = db.get_channels_for_user(a_id).await.unwrap();
- pretty_assertions::assert_eq!(
- result.channels,
- vec![
- Channel {
- id: zed_id,
- name: "zed".to_string(),
- parent_id: None,
- },
- Channel {
- id: crdb_id,
- name: "crdb".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: gpui2_id,
- name: "gpui2".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: livestreaming_id,
- name: "livestreaming".to_string(),
- parent_id: Some(gpui2_id),
- },
- Channel {
- id: livestreaming_id,
- name: "livestreaming".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: livestreaming_id,
- name: "livestreaming".to_string(),
- parent_id: Some(crdb_id),
- },
- Channel {
- id: livestreaming_dag_id,
- name: "livestreaming_dag".to_string(),
- parent_id: Some(livestreaming_id),
- },
- Channel {
- id: livestreaming_dag_sub_id,
- name: "livestreaming_dag_sub".to_string(),
- parent_id: Some(livestreaming_dag_id),
- },
- ]
- );
+ assert_dag(result.channels, &[
+ (zed_id, None),
+ (crdb_id, Some(zed_id)),
+ (gpui2_id, Some(zed_id)),
+ (livestreaming_id, Some(gpui2_id)),
+ (livestreaming_id, Some(zed_id)),
+ (livestreaming_id, Some(crdb_id)),
+ (livestreaming_dag_id, Some(livestreaming_id)),
+ (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
+ ]);
- // Remove that outer link
- db.move_channel(a_id, livestreaming_id, Some(gpui2_id), None)
+ // ========================================================================
+ // Test unlinking in a complex DAG by removing the inner link
+ db.unlink_channel(a_id, livestreaming_id, Some(gpui2_id))
.await
.unwrap();
@@ -875,49 +697,19 @@ async fn test_channels_moving(db: &Arc<Database>) {
// zed - crdb -- livestreaming - livestreaming_dag - livestreaming_dag_sub
// \---------/
let result = db.get_channels_for_user(a_id).await.unwrap();
- pretty_assertions::assert_eq!(
- result.channels,
- vec![
- Channel {
- id: zed_id,
- name: "zed".to_string(),
- parent_id: None,
- },
- Channel {
- id: crdb_id,
- name: "crdb".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: gpui2_id,
- name: "gpui2".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: livestreaming_id,
- name: "livestreaming".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: livestreaming_id,
- name: "livestreaming".to_string(),
- parent_id: Some(crdb_id),
- },
- Channel {
- id: livestreaming_dag_id,
- name: "livestreaming_dag".to_string(),
- parent_id: Some(livestreaming_id),
- },
- Channel {
- id: livestreaming_dag_sub_id,
- name: "livestreaming_dag_sub".to_string(),
- parent_id: Some(livestreaming_dag_id),
- },
- ]
- );
+ assert_dag(result.channels, &[
+ (zed_id, None),
+ (crdb_id, Some(zed_id)),
+ (gpui2_id, Some(zed_id)),
+ (livestreaming_id, Some(zed_id)),
+ (livestreaming_id, Some(crdb_id)),
+ (livestreaming_dag_id, Some(livestreaming_id)),
+ (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
+ ]);
- // Move livestreaming to be below gpui2
- db.move_channel(a_id, livestreaming_id, Some(crdb_id), Some(gpui2_id))
+ // ========================================================================
+ // Test moving DAG nodes by moving livestreaming to be below gpui2
+ db.move_channel(a_id, livestreaming_id, Some(crdb_id), gpui2_id)
.await
.unwrap();
@@ -926,47 +718,17 @@ async fn test_channels_moving(db: &Arc<Database>) {
// zed - crdb /
// \---------/
let result = db.get_channels_for_user(a_id).await.unwrap();
- pretty_assertions::assert_eq!(
- result.channels,
- vec![
- Channel {
- id: zed_id,
- name: "zed".to_string(),
- parent_id: None,
- },
- Channel {
- id: crdb_id,
- name: "crdb".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: gpui2_id,
- name: "gpui2".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: livestreaming_id,
- name: "livestreaming".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: livestreaming_id,
- name: "livestreaming".to_string(),
- parent_id: Some(gpui2_id),
- },
- Channel {
- id: livestreaming_dag_id,
- name: "livestreaming_dag".to_string(),
- parent_id: Some(livestreaming_id),
- },
- Channel {
- id: livestreaming_dag_sub_id,
- name: "livestreaming_dag_sub".to_string(),
- parent_id: Some(livestreaming_dag_id),
- },
- ]
- );
-
+ assert_dag(result.channels, &[
+ (zed_id, None),
+ (crdb_id, Some(zed_id)),
+ (gpui2_id, Some(zed_id)),
+ (livestreaming_id, Some(zed_id)),
+ (livestreaming_id, Some(gpui2_id)),
+ (livestreaming_dag_id, Some(livestreaming_id)),
+ (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
+ ]);
+
+ // ========================================================================
// Deleting a channel should not delete children that still have other parents
db.delete_channel(gpui2_id, a_id).await.unwrap();
@@ -974,46 +736,109 @@ async fn test_channels_moving(db: &Arc<Database>) {
// zed - crdb
// \- livestreaming - livestreaming_dag - livestreaming_dag_sub
let result = db.get_channels_for_user(a_id).await.unwrap();
- pretty_assertions::assert_eq!(
- result.channels,
- vec![
- Channel {
- id: zed_id,
- name: "zed".to_string(),
- parent_id: None,
- },
- Channel {
- id: crdb_id,
- name: "crdb".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: livestreaming_id,
- name: "livestreaming".to_string(),
- parent_id: Some(zed_id),
- },
- Channel {
- id: livestreaming_dag_id,
- name: "livestreaming_dag".to_string(),
- parent_id: Some(livestreaming_id),
- },
- Channel {
- id: livestreaming_dag_sub_id,
- name: "livestreaming_dag_sub".to_string(),
- parent_id: Some(livestreaming_dag_id),
- },
- ]
- );
+ assert_dag(result.channels, &[
+ (zed_id, None),
+ (crdb_id, Some(zed_id)),
+ (livestreaming_id, Some(zed_id)),
+ (livestreaming_dag_id, Some(livestreaming_id)),
+ (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
+ ]);
+
+ // ========================================================================
+ // Unlinking a channel from it's parent should automatically promote it to a root channel
+ db.unlink_channel(a_id, crdb_id, Some(zed_id))
+ .await
+ .unwrap();
+
+ // DAG is now:
+ // crdb
+ // zed
+ // \- livestreaming - livestreaming_dag - livestreaming_dag_sub
+
+ let result = db.get_channels_for_user(a_id).await.unwrap();
+ assert_dag(result.channels, &[
+ (zed_id, None),
+ (crdb_id, None),
+ (livestreaming_id, Some(zed_id)),
+ (livestreaming_dag_id, Some(livestreaming_id)),
+ (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
+ ]);
- // But deleting a parent of a DAG should delete the whole DAG:
- db.move_channel(a_id, livestreaming_id, None, Some(crdb_id))
+ // ========================================================================
+ // Unlinking a root channel should not have any effect
+ db.unlink_channel(a_id, crdb_id, None)
.await
.unwrap();
+
+ // DAG is now:
+ // crdb
+ // zed
+ // \- livestreaming - livestreaming_dag - livestreaming_dag_sub
+ //
+ let result = db.get_channels_for_user(a_id).await.unwrap();
+ assert_dag(result.channels, &[
+ (zed_id, None),
+ (crdb_id, None),
+ (livestreaming_id, Some(zed_id)),
+ (livestreaming_dag_id, Some(livestreaming_id)),
+ (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
+ ]);
+
+ // ========================================================================
+ // You should be able to move a root channel into a non-root channel
+ db.move_channel(a_id, crdb_id, None, zed_id)
+ .await
+ .unwrap();
+
+ // DAG is now:
+ // zed - crdb
+ // \- livestreaming - livestreaming_dag - livestreaming_dag_sub
+
+ let result = db.get_channels_for_user(a_id).await.unwrap();
+ assert_dag(result.channels, &[
+ (zed_id, None),
+ (crdb_id, Some(zed_id)),
+ (livestreaming_id, Some(zed_id)),
+ (livestreaming_dag_id, Some(livestreaming_id)),
+ (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
+ ]);
+
+
+ // ========================================================================
+ // Moving a non-root channel without a parent id should be the equivalent of a link operation
+ db.move_channel(a_id, livestreaming_id, None, crdb_id)
+ .await
+ .unwrap();
+
// DAG is now:
// zed - crdb - livestreaming - livestreaming_dag - livestreaming_dag_sub
// \--------/
+ let result = db.get_channels_for_user(a_id).await.unwrap();
+ assert_dag(result.channels, &[
+ (zed_id, None),
+ (crdb_id, Some(zed_id)),
+ (livestreaming_id, Some(zed_id)),
+ (livestreaming_id, Some(crdb_id)),
+ (livestreaming_dag_id, Some(livestreaming_id)),
+ (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
+ ]);
+
+ // ========================================================================
+ // Deleting a parent of a DAG should delete the whole DAG:
db.delete_channel(zed_id, a_id).await.unwrap();
let result = db.get_channels_for_user(a_id).await.unwrap();
- assert!(result.channels.is_empty())
+ assert!(
+ result.channels.is_empty()
+ )
+}
+
+#[track_caller]
+fn assert_dag(actual: Vec<Channel>, expected: &[(ChannelId, Option<ChannelId>)]) {
+ let actual = actual
+ .iter()
+ .map(|channel| (channel.id, channel.parent_id))
+ .collect::<Vec<_>>();
+
+ pretty_assertions::assert_eq!(actual, expected)
}
@@ -114,7 +114,7 @@ struct PutChannel {
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
struct UnlinkChannel {
channel_id: ChannelId,
- parent_id: ChannelId,
+ parent_id: Option<ChannelId>,
}
actions!(
@@ -199,13 +199,13 @@ pub fn init(cx: &mut AppContext) {
cx.add_action(
|panel: &mut CollabPanel, action: &LinkChannel, _: &mut ViewContext<CollabPanel>| {
- panel.copy = Some(ChannelCopy::Link(action.channel_id));
+ panel.link_or_move = Some(ChannelCopy::Link(action.channel_id));
},
);
cx.add_action(
|panel: &mut CollabPanel, action: &MoveChannel, _: &mut ViewContext<CollabPanel>| {
- panel.copy = Some(ChannelCopy::Move {
+ panel.link_or_move = Some(ChannelCopy::Move {
channel_id: action.channel_id,
parent_id: action.parent_id,
});
@@ -214,20 +214,20 @@ pub fn init(cx: &mut AppContext) {
cx.add_action(
|panel: &mut CollabPanel, action: &PutChannel, cx: &mut ViewContext<CollabPanel>| {
- if let Some(copy) = panel.copy.take() {
+ if let Some(copy) = panel.link_or_move.take() {
match copy {
ChannelCopy::Move {
channel_id,
parent_id,
} => panel.channel_store.update(cx, |channel_store, cx| {
channel_store
- .move_channel(channel_id, parent_id, Some(action.to), cx)
+ .move_channel(channel_id, parent_id, action.to, cx)
.detach_and_log_err(cx)
}),
ChannelCopy::Link(channel) => {
panel.channel_store.update(cx, |channel_store, cx| {
channel_store
- .move_channel(channel, None, Some(action.to), cx)
+ .link_channel(channel, action.to, cx)
.detach_and_log_err(cx)
})
}
@@ -240,7 +240,7 @@ pub fn init(cx: &mut AppContext) {
|panel: &mut CollabPanel, action: &UnlinkChannel, cx: &mut ViewContext<CollabPanel>| {
panel.channel_store.update(cx, |channel_store, cx| {
channel_store
- .move_channel(action.channel_id, Some(action.parent_id), None, cx)
+ .unlink_channel(action.channel_id, action.parent_id, cx)
.detach_and_log_err(cx)
})
},
@@ -284,13 +284,20 @@ impl ChannelCopy {
ChannelCopy::Link(channel_id) => *channel_id,
}
}
+
+ fn is_move(&self) -> bool {
+ match self {
+ ChannelCopy::Move { .. } => true,
+ ChannelCopy::Link(_) => false,
+ }
+ }
}
pub struct CollabPanel {
width: Option<f32>,
fs: Arc<dyn Fs>,
has_focus: bool,
- copy: Option<ChannelCopy>,
+ link_or_move: Option<ChannelCopy>,
pending_serialization: Task<Option<()>>,
context_menu: ViewHandle<ContextMenu>,
filter_editor: ViewHandle<Editor>,
@@ -553,7 +560,7 @@ impl CollabPanel {
let mut this = Self {
width: None,
has_focus: false,
- copy: None,
+ link_or_move: None,
fs: workspace.app_state().fs.clone(),
pending_serialization: Task::ready(None),
context_menu: cx.add_view(|cx| ContextMenu::new(view_id, cx)),
@@ -2062,15 +2069,14 @@ impl CollabPanel {
) {
self.context_menu_on_selected = position.is_none();
- let copy_channel = self
- .copy
- .as_ref()
- .and_then(|copy| {
- self.channel_store
- .read(cx)
- .channel_for_id(copy.channel_id())
- })
- .map(|channel| channel.name.clone());
+ let operation_details = self.link_or_move.as_ref().and_then(|link_or_move| {
+ let channel_name = self
+ .channel_store
+ .read(cx)
+ .channel_for_id(link_or_move.channel_id())
+ .map(|channel| channel.name.clone())?;
+ Some((channel_name, link_or_move.is_move()))
+ });
self.context_menu.update(cx, |context_menu, cx| {
context_menu.set_position_mode(if self.context_menu_on_selected {
@@ -2079,13 +2085,29 @@ impl CollabPanel {
OverlayPositionMode::Window
});
+ let mut items = Vec::new();
+
+ if let Some((channel_name, is_move)) = operation_details {
+ items.push(ContextMenuItem::action(
+ format!(
+ "{} '#{}' here",
+ if is_move { "Move" } else { "Link" },
+ channel_name
+ ),
+ PutChannel {
+ to: location.channel,
+ },
+ ));
+ items.push(ContextMenuItem::Separator)
+ }
+
let expand_action_name = if self.is_channel_collapsed(&location) {
"Expand Subchannels"
} else {
"Collapse Subchannels"
};
- let mut items = vec![
+ items.extend([
ContextMenuItem::action(
expand_action_name,
ToggleCollapse {
@@ -2098,7 +2120,7 @@ impl CollabPanel {
channel_id: location.channel,
},
),
- ];
+ ]);
if self.channel_store.read(cx).is_user_admin(location.channel) {
let parent_id = location.path.parent_id();
@@ -2120,25 +2142,27 @@ impl CollabPanel {
ContextMenuItem::Separator,
]);
- if let Some(parent) = parent_id {
- items.push(ContextMenuItem::action(
- "Unlink from parent",
- UnlinkChannel {
- channel_id: location.channel,
- parent_id: parent,
- },
- ))
- }
+ items.push(ContextMenuItem::action(
+ if parent_id.is_some() {
+ "Unlink from parent"
+ } else {
+ "Unlink from root"
+ },
+ UnlinkChannel {
+ channel_id: location.channel,
+ parent_id,
+ },
+ ));
items.extend([
ContextMenuItem::action(
- "Link to new parent",
+ "Link this channel",
LinkChannel {
channel_id: location.channel,
},
),
ContextMenuItem::action(
- "Move",
+ "Move this channel",
MoveChannel {
channel_id: location.channel,
parent_id,
@@ -2146,15 +2170,6 @@ impl CollabPanel {
),
]);
- if let Some(copy_channel) = copy_channel {
- items.push(ContextMenuItem::action(
- format!("Put '#{}'", copy_channel),
- PutChannel {
- to: location.channel,
- },
- ));
- }
-
items.extend([
ContextMenuItem::Separator,
ContextMenuItem::action(