Просмотр исходного кода

fix: can not keep the noo status groups order. Refactor the default index of no status group is 0

appflowy 2 лет назад
Родитель
Сommit
da20d5c221

+ 3 - 1
frontend/rust-lib/flowy-grid/src/services/grid_view_editor.rs

@@ -228,7 +228,9 @@ impl GridViewRevisionEditor {
         }
     }
 
-    pub(crate) async fn insert_group(&self, params: InsertGroupParams) -> FlowyResult<()> {
+    /// Initialize new group when grouping by a new field
+    ///
+    pub(crate) async fn initialize_new_group(&self, params: InsertGroupParams) -> FlowyResult<()> {
         if let Some(field_rev) = self.field_delegate.get_field_rev(&params.field_id).await {
             let _ = self
                 .modify(|pad| {

+ 1 - 1
frontend/rust-lib/flowy-grid/src/services/grid_view_manager.rs

@@ -135,7 +135,7 @@ impl GridViewManager {
 
     pub(crate) async fn insert_or_update_group(&self, params: InsertGroupParams) -> FlowyResult<()> {
         let view_editor = self.get_default_view_editor().await?;
-        view_editor.insert_group(params).await
+        view_editor.initialize_new_group(params).await
     }
 
     pub(crate) async fn delete_group(&self, params: DeleteGroupParams) -> FlowyResult<()> {

+ 21 - 12
frontend/rust-lib/flowy-grid/src/services/group/configuration.rs

@@ -185,11 +185,20 @@ where
         });
 
         let mut old_groups = self.configuration.groups.clone();
+        // clear all the groups if grouping by a new field
+        if self.configuration.field_id != self.field_rev.id {
+            old_groups.clear();
+        }
+
+        // When grouping by a new field, there is no `No status` group. So it needs
+        // to insert a `No status` group at index 0
+        // The `No status` group index is initialized to 0
+        //
         if !old_groups.iter().any(|group| group.id == self.field_rev.id) {
-            old_groups.push(make_no_status_group(&self.field_rev));
+            old_groups.insert(0, make_no_status_group(&self.field_rev));
         }
 
-        // The `all_group_revs` represents as the combination of the new groups and old groups
+        // The `all_group_revs` is the combination of the new groups and old groups
         let MergeGroupResult {
             mut all_group_revs,
             new_group_revs,
@@ -202,15 +211,17 @@ where
             .map(|group_rev| group_rev.id)
             .collect::<Vec<String>>();
 
-        // Delete/Insert the group in the current configuration
         self.mut_configuration(|configuration| {
-            let mut is_changed = false;
+            let mut is_changed = !deleted_group_ids.is_empty();
+
+            // Remove the groups
             if !deleted_group_ids.is_empty() {
                 configuration
                     .groups
                     .retain(|group| !deleted_group_ids.contains(&group.id));
-                is_changed = true;
             }
+
+            // Update/Insert new groups
             for group_rev in &mut all_group_revs {
                 match configuration
                     .groups
@@ -223,17 +234,14 @@ where
                         is_changed = true;
                     }
                     Some(pos) => {
-                        let mut old_group = configuration.groups.remove(pos);
-
+                        let mut old_group = configuration.groups.get_mut(pos).unwrap();
                         // Take the old group setting
                         group_rev.update_with_other(&old_group);
                         if !is_changed {
                             is_changed = is_group_changed(group_rev, &old_group);
                         }
-
                         // Consider the the name of the `group_rev` as the newest.
                         old_group.name = group_rev.name.clone();
-                        configuration.groups.insert(pos, old_group);
                     }
                 }
             }
@@ -331,6 +339,8 @@ where
     }
 }
 
+/// Merge the new groups into old groups while keeping the order in the old groups
+///
 fn merge_groups(old_groups: Vec<GroupRevision>, new_groups: Vec<GroupRevision>) -> MergeGroupResult {
     let mut merge_result = MergeGroupResult::new();
     // group_map is a helper map is used to filter out the new groups.
@@ -352,11 +362,10 @@ fn merge_groups(old_groups: Vec<GroupRevision>, new_groups: Vec<GroupRevision>)
     }
 
     // Find out the new groups
-    new_group_map.reverse();
     let new_groups = new_group_map.into_values();
     for (_, group) in new_groups.into_iter().enumerate() {
-        merge_result.all_group_revs.insert(0, group.clone());
-        merge_result.new_group_revs.insert(0, group);
+        merge_result.all_group_revs.push(group.clone());
+        merge_result.new_group_revs.push(group);
     }
     merge_result
 }

+ 1 - 7
frontend/rust-lib/flowy-grid/src/services/group/group_util.rs

@@ -133,13 +133,7 @@ pub fn default_group_configuration(field_rev: &FieldRevision) -> GroupConfigurat
     };
 
     // Append the no `status` group
-    let no_status_group_rev = GroupRevision {
-        id: field_rev.id.clone(),
-        name: format!("No {}", field_rev.name),
-        visible: true,
-    };
-
-    group_configuration_rev.groups.push(no_status_group_rev);
+    group_configuration_rev.groups.push(make_no_status_group(field_rev));
     group_configuration_rev
 }
 

+ 2 - 2
frontend/rust-lib/flowy-grid/tests/grid/group_test/script.rs

@@ -46,7 +46,7 @@ pub enum GroupScript {
         from_group_index: usize,
         to_group_index: usize,
     },
-    UpdateSingleSelectOption {
+    UpdateSingleSelectSelectOption {
         inserted_options: Vec<SelectOptionPB>,
     },
     GroupByField {
@@ -187,7 +187,7 @@ impl GridGroupTest {
                 assert_eq!(group.group_id, group_pb.group_id);
                 assert_eq!(group.desc, group_pb.desc);
             }
-            GroupScript::UpdateSingleSelectOption { inserted_options } => {
+            GroupScript::UpdateSingleSelectSelectOption { inserted_options } => {
                 self.edit_single_select_type_option(|type_option| {
                     for inserted_option in inserted_options {
                         type_option.insert_option(inserted_option);

+ 108 - 94
frontend/rust-lib/flowy-grid/tests/grid/group_test/test.rs

@@ -9,19 +9,19 @@ async fn group_init_test() {
     let scripts = vec![
         AssertGroupCount(4),
         AssertGroupRowCount {
-            group_index: 0,
+            group_index: 1,
             row_count: 2,
         },
         AssertGroupRowCount {
-            group_index: 1,
+            group_index: 2,
             row_count: 2,
         },
         AssertGroupRowCount {
-            group_index: 2,
+            group_index: 3,
             row_count: 1,
         },
         AssertGroupRowCount {
-            group_index: 3,
+            group_index: 0,
             row_count: 0,
         },
     ];
@@ -31,21 +31,21 @@ async fn group_init_test() {
 #[tokio::test]
 async fn group_move_row_test() {
     let mut test = GridGroupTest::new().await;
-    let group = test.group_at_index(0).await;
+    let group = test.group_at_index(1).await;
     let scripts = vec![
         // Move the row at 0 in group0 to group1 at 1
         MoveRow {
-            from_group_index: 0,
+            from_group_index: 1,
             from_row_index: 0,
-            to_group_index: 0,
+            to_group_index: 1,
             to_row_index: 1,
         },
         AssertGroupRowCount {
-            group_index: 0,
+            group_index: 1,
             row_count: 2,
         },
         AssertRow {
-            group_index: 0,
+            group_index: 1,
             row_index: 1,
             row: group.rows.get(0).unwrap().clone(),
         },
@@ -56,24 +56,24 @@ async fn group_move_row_test() {
 #[tokio::test]
 async fn group_move_row_to_other_group_test() {
     let mut test = GridGroupTest::new().await;
-    let group = test.group_at_index(0).await;
+    let group = test.group_at_index(1).await;
     let scripts = vec![
         MoveRow {
-            from_group_index: 0,
+            from_group_index: 1,
             from_row_index: 0,
-            to_group_index: 1,
+            to_group_index: 2,
             to_row_index: 1,
         },
         AssertGroupRowCount {
-            group_index: 0,
+            group_index: 1,
             row_count: 1,
         },
         AssertGroupRowCount {
-            group_index: 1,
+            group_index: 2,
             row_count: 3,
         },
         AssertRow {
-            group_index: 1,
+            group_index: 2,
             row_index: 1,
             row: group.rows.get(0).unwrap().clone(),
         },
@@ -84,50 +84,52 @@ async fn group_move_row_to_other_group_test() {
 #[tokio::test]
 async fn group_move_two_row_to_other_group_test() {
     let mut test = GridGroupTest::new().await;
-    let group = test.group_at_index(0).await;
+    let group_1 = test.group_at_index(1).await;
     let scripts = vec![
+        // Move row at index 0 from group 1 to group 2 at index 1
         MoveRow {
-            from_group_index: 0,
+            from_group_index: 1,
             from_row_index: 0,
-            to_group_index: 1,
+            to_group_index: 2,
             to_row_index: 1,
         },
         AssertGroupRowCount {
-            group_index: 0,
+            group_index: 1,
             row_count: 1,
         },
         AssertGroupRowCount {
-            group_index: 1,
+            group_index: 2,
             row_count: 3,
         },
         AssertRow {
-            group_index: 1,
+            group_index: 2,
             row_index: 1,
-            row: group.rows.get(0).unwrap().clone(),
+            row: group_1.rows.get(0).unwrap().clone(),
         },
     ];
     test.run_scripts(scripts).await;
 
-    let group = test.group_at_index(0).await;
+    let group_1 = test.group_at_index(1).await;
+    // Move row at index 0 from group 1 to group 2 at index 1
     let scripts = vec![
         MoveRow {
-            from_group_index: 0,
+            from_group_index: 1,
             from_row_index: 0,
-            to_group_index: 1,
+            to_group_index: 2,
             to_row_index: 1,
         },
         AssertGroupRowCount {
-            group_index: 0,
+            group_index: 1,
             row_count: 0,
         },
         AssertGroupRowCount {
-            group_index: 1,
+            group_index: 2,
             row_count: 4,
         },
         AssertRow {
-            group_index: 1,
+            group_index: 2,
             row_index: 1,
-            row: group.rows.get(0).unwrap().clone(),
+            row: group_1.rows.get(0).unwrap().clone(),
         },
     ];
     test.run_scripts(scripts).await;
@@ -136,34 +138,34 @@ async fn group_move_two_row_to_other_group_test() {
 #[tokio::test]
 async fn group_move_row_to_other_group_and_reorder_from_up_to_down_test() {
     let mut test = GridGroupTest::new().await;
-    let group_0 = test.group_at_index(0).await;
     let group_1 = test.group_at_index(1).await;
+    let group_2 = test.group_at_index(2).await;
     let scripts = vec![
         MoveRow {
-            from_group_index: 0,
+            from_group_index: 1,
             from_row_index: 0,
-            to_group_index: 1,
+            to_group_index: 2,
             to_row_index: 1,
         },
         AssertRow {
-            group_index: 1,
+            group_index: 2,
             row_index: 1,
-            row: group_0.rows.get(0).unwrap().clone(),
+            row: group_1.rows.get(0).unwrap().clone(),
         },
     ];
     test.run_scripts(scripts).await;
 
     let scripts = vec![
         MoveRow {
-            from_group_index: 1,
+            from_group_index: 2,
             from_row_index: 0,
-            to_group_index: 1,
+            to_group_index: 2,
             to_row_index: 2,
         },
         AssertRow {
-            group_index: 1,
+            group_index: 2,
             row_index: 2,
-            row: group_1.rows.get(0).unwrap().clone(),
+            row: group_2.rows.get(0).unwrap().clone(),
         },
     ];
     test.run_scripts(scripts).await;
@@ -173,27 +175,27 @@ async fn group_move_row_to_other_group_and_reorder_from_up_to_down_test() {
 async fn group_move_row_to_other_group_and_reorder_from_bottom_to_up_test() {
     let mut test = GridGroupTest::new().await;
     let scripts = vec![MoveRow {
-        from_group_index: 0,
+        from_group_index: 1,
         from_row_index: 0,
-        to_group_index: 1,
+        to_group_index: 2,
         to_row_index: 1,
     }];
     test.run_scripts(scripts).await;
 
-    let group = test.group_at_index(1).await;
+    let group = test.group_at_index(2).await;
     let scripts = vec![
         AssertGroupRowCount {
-            group_index: 1,
+            group_index: 2,
             row_count: 3,
         },
         MoveRow {
-            from_group_index: 1,
+            from_group_index: 2,
             from_row_index: 2,
-            to_group_index: 1,
+            to_group_index: 2,
             to_row_index: 0,
         },
         AssertRow {
-            group_index: 1,
+            group_index: 2,
             row_index: 0,
             row: group.rows.get(2).unwrap().clone(),
         },
@@ -204,15 +206,15 @@ async fn group_move_row_to_other_group_and_reorder_from_bottom_to_up_test() {
 async fn group_create_row_test() {
     let mut test = GridGroupTest::new().await;
     let scripts = vec![
-        CreateRow { group_index: 0 },
+        CreateRow { group_index: 1 },
         AssertGroupRowCount {
-            group_index: 0,
+            group_index: 1,
             row_count: 3,
         },
-        CreateRow { group_index: 1 },
-        CreateRow { group_index: 1 },
+        CreateRow { group_index: 2 },
+        CreateRow { group_index: 2 },
         AssertGroupRowCount {
-            group_index: 1,
+            group_index: 2,
             row_count: 4,
         },
     ];
@@ -224,11 +226,11 @@ async fn group_delete_row_test() {
     let mut test = GridGroupTest::new().await;
     let scripts = vec![
         DeleteRow {
-            group_index: 0,
+            group_index: 1,
             row_index: 0,
         },
         AssertGroupRowCount {
-            group_index: 0,
+            group_index: 1,
             row_count: 1,
         },
     ];
@@ -240,15 +242,15 @@ async fn group_delete_all_row_test() {
     let mut test = GridGroupTest::new().await;
     let scripts = vec![
         DeleteRow {
-            group_index: 0,
+            group_index: 1,
             row_index: 0,
         },
         DeleteRow {
-            group_index: 0,
+            group_index: 1,
             row_index: 0,
         },
         AssertGroupRowCount {
-            group_index: 0,
+            group_index: 1,
             row_count: 0,
         },
     ];
@@ -261,16 +263,16 @@ async fn group_update_row_test() {
     let scripts = vec![
         // Update the row at 0 in group0 by setting the row's group field data
         UpdateRow {
-            from_group_index: 0,
+            from_group_index: 1,
             row_index: 0,
-            to_group_index: 1,
+            to_group_index: 2,
         },
         AssertGroupRowCount {
-            group_index: 0,
+            group_index: 1,
             row_count: 1,
         },
         AssertGroupRowCount {
-            group_index: 1,
+            group_index: 2,
             row_count: 3,
         },
     ];
@@ -283,16 +285,16 @@ async fn group_reorder_group_test() {
     let scripts = vec![
         // Update the row at 0 in group0 by setting the row's group field data
         UpdateRow {
-            from_group_index: 0,
+            from_group_index: 1,
             row_index: 0,
-            to_group_index: 1,
+            to_group_index: 2,
         },
         AssertGroupRowCount {
-            group_index: 0,
+            group_index: 1,
             row_count: 1,
         },
         AssertGroupRowCount {
-            group_index: 1,
+            group_index: 2,
             row_count: 3,
         },
     ];
@@ -304,16 +306,16 @@ async fn group_move_to_default_group_test() {
     let mut test = GridGroupTest::new().await;
     let scripts = vec![
         UpdateRow {
-            from_group_index: 0,
+            from_group_index: 1,
             row_index: 0,
-            to_group_index: 3,
+            to_group_index: 0,
         },
         AssertGroupRowCount {
-            group_index: 0,
+            group_index: 1,
             row_count: 1,
         },
         AssertGroupRowCount {
-            group_index: 3,
+            group_index: 0,
             row_count: 1,
         },
     ];
@@ -323,25 +325,37 @@ async fn group_move_to_default_group_test() {
 #[tokio::test]
 async fn group_move_from_default_group_test() {
     let mut test = GridGroupTest::new().await;
-    let scripts = vec![UpdateRow {
-        from_group_index: 0,
-        row_index: 0,
-        to_group_index: 3,
-    }];
-    test.run_scripts(scripts).await;
-
+    // Move one row from group 1 to group 0
     let scripts = vec![
         UpdateRow {
-            from_group_index: 3,
+            from_group_index: 1,
             row_index: 0,
             to_group_index: 0,
         },
+        AssertGroupRowCount {
+            group_index: 1,
+            row_count: 1,
+        },
         AssertGroupRowCount {
             group_index: 0,
+            row_count: 1,
+        },
+    ];
+    test.run_scripts(scripts).await;
+
+    // Move one row from group 0 to group 1
+    let scripts = vec![
+        UpdateRow {
+            from_group_index: 0,
+            row_index: 0,
+            to_group_index: 1,
+        },
+        AssertGroupRowCount {
+            group_index: 1,
             row_count: 2,
         },
         AssertGroupRowCount {
-            group_index: 3,
+            group_index: 0,
             row_count: 0,
         },
     ];
@@ -368,7 +382,7 @@ async fn group_move_group_test() {
         },
         AssertGroupRowCount {
             group_index: 1,
-            row_count: 2,
+            row_count: 0,
         },
         AssertGroup {
             group_index: 1,
@@ -381,33 +395,33 @@ async fn group_move_group_test() {
 #[tokio::test]
 async fn group_move_group_row_after_move_group_test() {
     let mut test = GridGroupTest::new().await;
-    let group_0 = test.group_at_index(0).await;
     let group_1 = test.group_at_index(1).await;
+    let group_2 = test.group_at_index(2).await;
     let scripts = vec![
         MoveGroup {
-            from_group_index: 0,
-            to_group_index: 1,
+            from_group_index: 1,
+            to_group_index: 2,
         },
         AssertGroup {
-            group_index: 0,
-            expected_group: group_1,
+            group_index: 1,
+            expected_group: group_2,
         },
         AssertGroup {
-            group_index: 1,
-            expected_group: group_0,
+            group_index: 2,
+            expected_group: group_1,
         },
         MoveRow {
-            from_group_index: 0,
+            from_group_index: 1,
             from_row_index: 0,
-            to_group_index: 1,
+            to_group_index: 2,
             to_row_index: 0,
         },
         AssertGroupRowCount {
-            group_index: 0,
+            group_index: 1,
             row_count: 1,
         },
         AssertGroupRowCount {
-            group_index: 1,
+            group_index: 2,
             row_count: 3,
         },
     ];
@@ -415,7 +429,7 @@ async fn group_move_group_row_after_move_group_test() {
 }
 
 #[tokio::test]
-async fn group_default_move_group_test() {
+async fn group_move_group_to_default_group_pos_test() {
     let mut test = GridGroupTest::new().await;
     let group_0 = test.group_at_index(0).await;
     let group_3 = test.group_at_index(3).await;
@@ -442,13 +456,13 @@ async fn group_insert_single_select_option_test() {
     let new_option_name = "New option";
     let scripts = vec![
         AssertGroupCount(4),
-        UpdateSingleSelectOption {
+        UpdateSingleSelectSelectOption {
             inserted_options: vec![SelectOptionPB::new(new_option_name)],
         },
         AssertGroupCount(5),
     ];
     test.run_scripts(scripts).await;
-    let new_group = test.group_at_index(0).await;
+    let new_group = test.group_at_index(1).await;
     assert_eq!(new_group.desc, new_option_name);
 }
 
@@ -461,14 +475,14 @@ async fn group_group_by_other_field() {
             field_id: multi_select_field.id.clone(),
         },
         AssertGroupRowCount {
-            group_index: 0,
+            group_index: 1,
             row_count: 3,
         },
         AssertGroupRowCount {
-            group_index: 1,
+            group_index: 2,
             row_count: 2,
         },
-        AssertGroupCount(5),
+        AssertGroupCount(4),
     ];
     test.run_scripts(scripts).await;
 }