diff --git a/frontend/rust-lib/flowy-grid/src/services/grid_view_editor.rs b/frontend/rust-lib/flowy-grid/src/services/grid_view_editor.rs index 49f662169f..4b52f928d9 100644 --- a/frontend/rust-lib/flowy-grid/src/services/grid_view_editor.rs +++ b/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(¶ms.field_id).await { let _ = self .modify(|pad| { diff --git a/frontend/rust-lib/flowy-grid/src/services/grid_view_manager.rs b/frontend/rust-lib/flowy-grid/src/services/grid_view_manager.rs index 1e69caf81d..0aa503ce83 100644 --- a/frontend/rust-lib/flowy-grid/src/services/grid_view_manager.rs +++ b/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<()> { diff --git a/frontend/rust-lib/flowy-grid/src/services/group/configuration.rs b/frontend/rust-lib/flowy-grid/src/services/group/configuration.rs index 8492808820..6fd261fa8b 100644 --- a/frontend/rust-lib/flowy-grid/src/services/group/configuration.rs +++ b/frontend/rust-lib/flowy-grid/src/services/group/configuration.rs @@ -185,11 +185,20 @@ where }); let mut old_groups = self.configuration.groups.clone(); - if !old_groups.iter().any(|group| group.id == self.field_rev.id) { - old_groups.push(make_no_status_group(&self.field_rev)); + // clear all the groups if grouping by a new field + if self.configuration.field_id != self.field_rev.id { + old_groups.clear(); } - // The `all_group_revs` represents as the combination of the new groups and old groups + // 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.insert(0, make_no_status_group(&self.field_rev)); + } + + // 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::>(); - // 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, new_groups: Vec) -> 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, new_groups: Vec) } // 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 } diff --git a/frontend/rust-lib/flowy-grid/src/services/group/group_util.rs b/frontend/rust-lib/flowy-grid/src/services/group/group_util.rs index 683bce705f..5c6180c859 100644 --- a/frontend/rust-lib/flowy-grid/src/services/group/group_util.rs +++ b/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 } diff --git a/frontend/rust-lib/flowy-grid/tests/grid/group_test/script.rs b/frontend/rust-lib/flowy-grid/tests/grid/group_test/script.rs index 93daabbb69..bd117cd30b 100644 --- a/frontend/rust-lib/flowy-grid/tests/grid/group_test/script.rs +++ b/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, }, 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); diff --git a/frontend/rust-lib/flowy-grid/tests/grid/group_test/test.rs b/frontend/rust-lib/flowy-grid/tests/grid/group_test/test.rs index 2040379c4c..237f7f1d28 100644 --- a/frontend/rust-lib/flowy-grid/tests/grid/group_test/test.rs +++ b/frontend/rust-lib/flowy-grid/tests/grid/group_test/test.rs @@ -8,20 +8,20 @@ async fn group_init_test() { let mut test = GridGroupTest::new().await; let scripts = vec![ AssertGroupCount(4), - AssertGroupRowCount { - group_index: 0, - row_count: 2, - }, AssertGroupRowCount { group_index: 1, row_count: 2, }, AssertGroupRowCount { group_index: 2, - row_count: 1, + row_count: 2, }, AssertGroupRowCount { group_index: 3, + row_count: 1, + }, + AssertGroupRowCount { + 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 }, - AssertGroupRowCount { - group_index: 0, - row_count: 3, - }, - CreateRow { group_index: 1 }, CreateRow { group_index: 1 }, AssertGroupRowCount { group_index: 1, + row_count: 3, + }, + CreateRow { group_index: 2 }, + CreateRow { group_index: 2 }, + AssertGroupRowCount { + 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, - }, - AssertGroupRowCount { - group_index: 0, - row_count: 1, + to_group_index: 2, }, AssertGroupRowCount { group_index: 1, + row_count: 1, + }, + AssertGroupRowCount { + 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, - }, - AssertGroupRowCount { - group_index: 0, - row_count: 1, + to_group_index: 2, }, AssertGroupRowCount { group_index: 1, + row_count: 1, + }, + AssertGroupRowCount { + 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, - }, - AssertGroup { - group_index: 0, - expected_group: group_1, + from_group_index: 1, + to_group_index: 2, }, AssertGroup { group_index: 1, - expected_group: group_0, + expected_group: group_2, + }, + AssertGroup { + 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; }