Prevent moving a channel into its descendant

Max Brunsfeld created 2 years ago

Change summary

crates/collab/src/db/queries/channels.rs     |  7 +++++++
crates/collab/src/db/tests/channel_tests.rs  | 14 ++++++++++++++
crates/collab2/src/db/queries/channels.rs    |  7 +++++++
crates/collab2/src/db/tests/channel_tests.rs | 16 ++++++++++++++--
4 files changed, 42 insertions(+), 2 deletions(-)

Detailed changes

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

@@ -1220,6 +1220,13 @@ impl Database {
                 self.check_user_is_channel_admin(&new_parent, admin_id, &*tx)
                     .await?;
 
+                if new_parent
+                    .ancestors_including_self()
+                    .any(|id| id == channel.id)
+                {
+                    Err(anyhow!("cannot move a channel into one of its descendants"))?;
+                }
+
                 new_parent_path = new_parent.path();
                 new_parent_channel = Some(new_parent);
             } else {

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

@@ -450,6 +450,20 @@ async fn test_db_channel_moving_bugs(db: &Arc<Database>) {
             (livestreaming_id, &[projects_id]),
         ],
     );
+
+    // Can't move a channel into its ancestor
+    db.move_channel(projects_id, Some(livestreaming_id), user_id)
+        .await
+        .unwrap_err();
+    let result = db.get_channels_for_user(user_id).await.unwrap();
+    assert_channel_tree(
+        result.channels,
+        &[
+            (zed_id, &[]),
+            (projects_id, &[]),
+            (livestreaming_id, &[projects_id]),
+        ],
+    );
 }
 
 test_both_dbs!(

crates/collab2/src/db/queries/channels.rs

@@ -1220,6 +1220,13 @@ impl Database {
                 self.check_user_is_channel_admin(&new_parent, admin_id, &*tx)
                     .await?;
 
+                if new_parent
+                    .ancestors_including_self()
+                    .any(|id| id == channel.id)
+                {
+                    Err(anyhow!("cannot move a channel into one of its descendants"))?;
+                }
+
                 new_parent_path = new_parent.path();
                 new_parent_channel = Some(new_parent);
             } else {

crates/collab2/src/db/tests/channel_tests.rs

@@ -420,8 +420,6 @@ async fn test_db_channel_moving_bugs(db: &Arc<Database>) {
         .await
         .unwrap();
 
-    // Dag is: zed - projects - livestreaming
-
     // Move to same parent should be a no-op
     assert!(db
         .move_channel(projects_id, Some(zed_id), user_id)
@@ -450,6 +448,20 @@ async fn test_db_channel_moving_bugs(db: &Arc<Database>) {
             (livestreaming_id, &[projects_id]),
         ],
     );
+
+    // Can't move a channel into its ancestor
+    db.move_channel(projects_id, Some(livestreaming_id), user_id)
+        .await
+        .unwrap_err();
+    let result = db.get_channels_for_user(user_id).await.unwrap();
+    assert_channel_tree(
+        result.channels,
+        &[
+            (zed_id, &[]),
+            (projects_id, &[]),
+            (livestreaming_id, &[projects_id]),
+        ],
+    );
 }
 
 test_both_dbs!(