feat: checklist sort (#4659)

* refactor: use BoxAny for dynamically-typed cell changesets * fix: rust-lib tests and clippy * feat: enable sorting by checklist type option * test: checklist sort rust-lib tests * chore: update related tests * fix: clippy --------- Co-authored-by: Nathan.fooo <86001920+appflowy@users.noreply.github.com>
2024-08-30 18:12:39 +00:00 · 2024-02-22 08:00:59 +08:00 · 2024-02-22 08:00:59 +08:00 · 6636731487
commit 6636731487
parent a4a2a4088b
13 changed files with 150 additions and 50 deletions
--- a/frontend/appflowy_flutter/lib/plugins/database/application/field/field_info.dart
+++ b/frontend/appflowy_flutter/lib/plugins/database/application/field/field_info.dart
@ -78,6 +78,7 @@ class FieldInfo with _$FieldInfo {
      case FieldType.MultiSelect:
      case FieldType.LastEditedTime:
      case FieldType.CreatedTime:
+      case FieldType.Checklist:
        return true;
      default:
        return false;
--- a/frontend/rust-lib/flowy-database2/src/event_handler.rs
+++ b/frontend/rust-lib/flowy-database2/src/event_handler.rs
@ -566,7 +566,11 @@ pub(crate) async fn update_checklist_cell_handler(
  let params: ChecklistCellDataChangesetParams = data.into_inner().try_into()?;
  let database_editor = manager.get_database_with_view_id(&params.view_id).await?;
  let changeset = ChecklistCellChangeset {
-    insert_options: params.insert_options,
+    insert_options: params
+      .insert_options
+      .into_iter()
+      .map(|name| (name, false))
+      .collect(),
    selected_option_ids: params.selected_option_ids,
    delete_option_ids: params.delete_option_ids,
    update_options: params.update_options,
--- a/frontend/rust-lib/flowy-database2/src/services/cell/cell_operation.rs
+++ b/frontend/rust-lib/flowy-database2/src/services/cell/cell_operation.rs
@ -232,7 +232,7 @@ pub fn insert_select_option_cell(option_ids: Vec<String>, field: &Field) -> Cell
  apply_cell_changeset(BoxAny::new(changeset), None, field, None).unwrap()
 }

-pub fn insert_checklist_cell(insert_options: Vec<String>, field: &Field) -> Cell {
+pub fn insert_checklist_cell(insert_options: Vec<(String, bool)>, field: &Field) -> Cell {
  let changeset = ChecklistCellChangeset {
    insert_options,
    ..Default::default()
@ -391,14 +391,13 @@ impl<'a> CellBuilder<'a> {
      },
    }
  }
-  pub fn insert_checklist_cell(&mut self, field_id: &str, option_names: Vec<String>) {
+  pub fn insert_checklist_cell(&mut self, field_id: &str, options: Vec<(String, bool)>) {
    match self.field_maps.get(&field_id.to_owned()) {
      None => tracing::warn!("Can't find the field with id: {}", field_id),
      Some(field) => {
-        self.cells.insert(
-          field_id.to_owned(),
-          insert_checklist_cell(option_names, field),
-        );
+        self
+          .cells
+          .insert(field_id.to_owned(), insert_checklist_cell(options, field));
      },
    }
  }
--- a/frontend/rust-lib/flowy-database2/src/services/field/type_options/checklist_type_option/checklist.rs
+++ b/frontend/rust-lib/flowy-database2/src/services/field/type_options/checklist_type_option/checklist.rs
@ -2,8 +2,8 @@ use crate::entities::{ChecklistCellDataPB, ChecklistFilterPB, FieldType, SelectO
 use crate::services::cell::{CellDataChangeset, CellDataDecoder};
 use crate::services::field::checklist_type_option::{ChecklistCellChangeset, ChecklistCellData};
 use crate::services::field::{
-  SelectOption, TypeOption, TypeOptionCellDataCompare, TypeOptionCellDataFilter,
-  TypeOptionCellDataSerde, TypeOptionTransform, SELECTION_IDS_SEPARATOR,
+  SelectOption, TypeOption, TypeOptionCellData, TypeOptionCellDataCompare,
+  TypeOptionCellDataFilter, TypeOptionCellDataSerde, TypeOptionTransform, SELECTION_IDS_SEPARATOR,
 };
 use crate::services::sort::SortCondition;
 use collab_database::fields::{Field, TypeOptionData, TypeOptionDataBuilder};
@ -101,8 +101,11 @@ fn update_cell_data_with_changeset(
  changeset
    .insert_options
    .into_iter()
-    .for_each(|option_name| {
+    .for_each(|(option_name, is_selected)| {
      let option = SelectOption::new(&option_name);
+      if is_selected {
+        cell_data.selected_option_ids.push(option.id.clone())
+      }
      cell_data.options.push(option);
    });

@ -153,7 +156,7 @@ impl CellDataDecoder for ChecklistTypeOption {

  fn stringify_cell_data(&self, cell_data: <Self as TypeOption>::CellData) -> String {
    cell_data
-      .selected_options()
+      .options
      .into_iter()
      .map(|option| option.name)
      .collect::<Vec<_>>()
@ -191,16 +194,19 @@ impl TypeOptionCellDataCompare for ChecklistTypeOption {
    &self,
    cell_data: &<Self as TypeOption>::CellData,
    other_cell_data: &<Self as TypeOption>::CellData,
-    _sort_condition: SortCondition,
+    sort_condition: SortCondition,
  ) -> Ordering {
+    match (cell_data.is_cell_empty(), other_cell_data.is_cell_empty()) {
+      (true, true) => Ordering::Equal,
+      (true, false) => Ordering::Greater,
+      (false, true) => Ordering::Less,
+      (false, false) => {
        let left = cell_data.percentage_complete();
        let right = other_cell_data.percentage_complete();
-    if left > right {
-      Ordering::Greater
-    } else if left < right {
-      Ordering::Less
-    } else {
-      Ordering::Equal
+        // safe to unwrap because the two floats won't be NaN
+        let order = left.partial_cmp(&right).unwrap();
+        sort_condition.evaluate_order(order)
+      },
    }
  }
 }
--- a/frontend/rust-lib/flowy-database2/src/services/field/type_options/checklist_type_option/checklist_entities.rs
+++ b/frontend/rust-lib/flowy-database2/src/services/field/type_options/checklist_type_option/checklist_entities.rs
@ -19,7 +19,7 @@ impl ToString for ChecklistCellData {

 impl TypeOptionCellData for ChecklistCellData {
  fn is_cell_empty(&self) -> bool {
-    self.selected_option_ids.is_empty()
+    self.options.is_empty()
  }
 }

@ -43,15 +43,20 @@ impl ChecklistCellData {
    ((selected_options as f64) / (total_options as f64) * 100.0).round() / 100.0
  }

-  pub fn from_options(options: Vec<String>) -> Self {
-    let options = options
+  pub fn from_options(options: Vec<(String, bool)>) -> Self {
+    let (options, selected_ids): (Vec<_>, Vec<_>) = options
      .into_iter()
-      .map(|option_name| SelectOption::new(&option_name))
-      .collect();
+      .map(|(name, is_selected)| {
+        let option = SelectOption::new(&name);
+        let selected_id = is_selected.then(|| option.id.clone());
+        (option, selected_id)
+      })
+      .unzip();
+    let selected_option_ids = selected_ids.into_iter().flatten().collect();

    Self {
      options,
-      ..Default::default()
+      selected_option_ids,
    }
  }
 }
@ -77,7 +82,7 @@ impl From<ChecklistCellData> for Cell {
 #[derive(Debug, Clone, Default)]
 pub struct ChecklistCellChangeset {
  /// List of option names that will be inserted
-  pub insert_options: Vec<String>,
+  pub insert_options: Vec<(String, bool)>,
  pub selected_option_ids: Vec<String>,
  pub delete_option_ids: Vec<String>,
  pub update_options: Vec<SelectOption>,
--- a/frontend/rust-lib/flowy-database2/src/services/field/type_options/type_option.rs
+++ b/frontend/rust-lib/flowy-database2/src/services/field/type_options/type_option.rs
@ -27,9 +27,10 @@ pub trait TypeOption {
  /// `FromCellString` and `Default` trait. If the cell string can not be decoded into the specified
  /// cell data type then the default value will be returned.
  /// For example:
-  ///     FieldType::Checkbox => CheckboxCellData
-  ///     FieldType::Date => DateCellData
-  ///     FieldType::URL => URLCellData
+  ///
+  /// - FieldType::Checkbox => CheckboxCellData
+  /// - FieldType::Date => DateCellData
+  /// - FieldType::URL => URLCellData
  ///
  /// Uses `StrCellData` for any `TypeOption` if their cell data is pure `String`.
  ///
@ -57,7 +58,7 @@ pub trait TypeOption {
  ///
  type CellProtobufType: TryInto<Bytes, Error = ProtobufError> + Debug;

-  /// Represents as the filter configuration for this type option.
+  /// Represents the filter configuration for this type option.
  type CellFilter: FromFilterString + Send + Sync + 'static;
 }
 /// This trait providing serialization and deserialization methods for cell data.
@ -81,12 +82,9 @@ pub trait TypeOptionCellDataSerde: TypeOption {
 }

 /// This trait that provides methods to extend the [TypeOption::CellData] functionalities.
-///
 pub trait TypeOptionCellData {
-  /// Checks if the cell content is considered empty.
-  ///
-  /// Even if a cell is initialized, its content might still be considered empty
-  /// based on certain criteria. e.g. empty text, date, select option, etc.
+  /// Checks if the cell content is considered empty based on certain criteria. e.g. empty text,
+  /// no date selected, no selected options
  fn is_cell_empty(&self) -> bool {
    false
  }
@ -99,8 +97,8 @@ pub trait TypeOptionTransform: TypeOption {
  }

  /// Transform the TypeOption from one field type to another
-  /// For example, when switching from `checkbox` type-option to `single-select`
-  /// type-option, adding the `Yes` option if the `single-select` type-option doesn't contain it.
+  /// For example, when switching from `Checkbox` type option to `Single-Select`
+  /// type option, adding the `Yes` option if the `Single-select` type-option doesn't contain it.
  /// But the cell content is a string, `Yes`, it's need to do the cell content transform.
  /// The `Yes` string will be transformed to the `Yes` option id.
  ///
@ -109,7 +107,6 @@ pub trait TypeOptionTransform: TypeOption {
  /// * `old_type_option_field_type`: the FieldType of the passed-in TypeOption
  /// * `old_type_option_data`: the data that can be parsed into corresponding `TypeOption`.
  ///
-  ///
  fn transform_type_option(
    &mut self,
    _old_type_option_field_type: FieldType,
--- a/frontend/rust-lib/flowy-database2/tests/database/cell_test/test.rs
+++ b/frontend/rust-lib/flowy-database2/tests/database/cell_test/test.rs
@ -47,7 +47,7 @@ async fn grid_cell_update() {
          ))
        },
        FieldType::Checklist => BoxAny::new(ChecklistCellChangeset {
-          insert_options: vec!["new option".to_string()],
+          insert_options: vec![("new option".to_string(), false)],
          ..Default::default()
        }),
        FieldType::Checkbox => BoxAny::new("1".to_string()),
--- a/frontend/rust-lib/flowy-database2/tests/database/database_editor.rs
+++ b/frontend/rust-lib/flowy-database2/tests/database/database_editor.rs
@ -383,11 +383,11 @@ impl<'a> TestRowBuilder<'a> {
    multi_select_field.id.clone()
  }

-  pub fn insert_checklist_cell(&mut self, option_names: Vec<String>) -> String {
+  pub fn insert_checklist_cell(&mut self, options: Vec<(String, bool)>) -> String {
    let checklist_field = self.field_with_type(&FieldType::Checklist);
    self
      .cell_build
-      .insert_checklist_cell(&checklist_field.id, option_names);
+      .insert_checklist_cell(&checklist_field.id, options);
    checklist_field.id.clone()
  }

--- a/frontend/rust-lib/flowy-database2/tests/database/filter_test/checklist_filter_test.rs
+++ b/frontend/rust-lib/flowy-database2/tests/database/filter_test/checklist_filter_test.rs
@ -7,7 +7,7 @@ use crate::database::filter_test::script::{DatabaseFilterTest, FilterRowChanged}
 #[tokio::test]
 async fn grid_filter_checklist_is_incomplete_test() {
  let mut test = DatabaseFilterTest::new().await;
-  let expected = 6;
+  let expected = 5;
  let row_count = test.row_details.len();
  let option_ids = get_checklist_cell_options(&test).await;

@ -31,7 +31,7 @@ async fn grid_filter_checklist_is_incomplete_test() {
 #[tokio::test]
 async fn grid_filter_checklist_is_complete_test() {
  let mut test = DatabaseFilterTest::new().await;
-  let expected = 1;
+  let expected = 2;
  let row_count = test.row_details.len();
  let option_ids = get_checklist_cell_options(&test).await;
  let scripts = vec![
--- a/frontend/rust-lib/flowy-database2/tests/database/group_test/script.rs
+++ b/frontend/rust-lib/flowy-database2/tests/database/group_test/script.rs
@ -239,7 +239,6 @@ impl DatabaseGroupTest {
          .move_group(&self.view_id, &from_group.group_id, &to_group.group_id)
          .await
          .unwrap();
-        //
      },
      GroupScript::AssertGroup {
        group_index,
--- a/frontend/rust-lib/flowy-database2/tests/database/mock_data/grid_mock_data.rs
+++ b/frontend/rust-lib/flowy-database2/tests/database/mock_data/grid_mock_data.rs
@ -151,7 +151,7 @@ pub fn make_test_grid() -> DatabaseData {
              row_builder.insert_url_cell("AppFlowy website - https://www.appflowy.io")
            },
            FieldType::Checklist => {
-              row_builder.insert_checklist_cell(vec!["First thing".to_string()])
+              row_builder.insert_checklist_cell(vec![("First thing".to_string(), false)])
            },
            _ => "".to_owned(),
          };
@ -168,6 +168,13 @@ pub fn make_test_grid() -> DatabaseData {
            FieldType::MultiSelect => row_builder
              .insert_multi_select_cell(|mut options| vec![options.remove(0), options.remove(1)]),
            FieldType::Checkbox => row_builder.insert_checkbox_cell("true"),
+            FieldType::Checklist => row_builder.insert_checklist_cell(vec![
+              ("Have breakfast".to_string(), true),
+              ("Have lunch".to_string(), true),
+              ("Take a nap".to_string(), false),
+              ("Have dinner".to_string(), true),
+              ("Shower and head to bed".to_string(), false),
+            ]),
            _ => "".to_owned(),
          };
        }
@ -203,6 +210,9 @@ pub fn make_test_grid() -> DatabaseData {
              row_builder.insert_single_select_cell(|mut options| options.remove(0))
            },
            FieldType::Checkbox => row_builder.insert_checkbox_cell("false"),
+            FieldType::Checklist => {
+              row_builder.insert_checklist_cell(vec![("Task 1".to_string(), true)])
+            },
            _ => "".to_owned(),
          };
        }
@ -240,6 +250,11 @@ pub fn make_test_grid() -> DatabaseData {
              row_builder.insert_multi_select_cell(|mut options| vec![options.remove(1)])
            },
            FieldType::Checkbox => row_builder.insert_checkbox_cell("true"),
+            FieldType::Checklist => row_builder.insert_checklist_cell(vec![
+              ("Sprint".to_string(), true),
+              ("Sprint some more".to_string(), false),
+              ("Rest".to_string(), true),
+            ]),
            _ => "".to_owned(),
          };
        }
--- a/frontend/rust-lib/flowy-database2/tests/database/share_test/export_test.rs
+++ b/frontend/rust-lib/flowy-database2/tests/database/share_test/export_test.rs
@ -28,12 +28,12 @@ async fn export_csv_test() {
  let database = test.editor.clone();
  let s = database.export_csv(CSVFormat::Original).await.unwrap();
  let expected = r#"Name,Price,Time,Status,Platform,is urgent,link,TODO,Last Modified,Created At
-A,$1,2022/03/14,,"Google,Facebook",Yes,AppFlowy website - https://www.appflowy.io,,,
-,$2,2022/03/14,,"Google,Twitter",Yes,,,,
+A,$1,2022/03/14,,"Google,Facebook",Yes,AppFlowy website - https://www.appflowy.io,First thing,,
+,$2,2022/03/14,,"Google,Twitter",Yes,,"Have breakfast,Have lunch,Take a nap,Have dinner,Shower and head to bed",,
 C,$3,2022/03/14,Completed,"Facebook,Google,Twitter",No,,,,
-DA,$14,2022/11/17,Completed,,No,,,,
+DA,$14,2022/11/17,Completed,,No,,Task 1,,
 AE,,2022/11/13,Planned,"Facebook,Twitter",No,,,,
-AE,$5,2022/12/25,Planned,Facebook,Yes,,,,
+AE,$5,2022/12/25,Planned,Facebook,Yes,,"Sprint,Sprint some more,Rest",,
 CB,,,,,,,,,
 "#;
  println!("{}", s);
--- a/frontend/rust-lib/flowy-database2/tests/database/sort_test/single_sort_test.rs
+++ b/frontend/rust-lib/flowy-database2/tests/database/sort_test/single_sort_test.rs
@ -407,3 +407,77 @@ async fn sort_multi_select_by_descending_test() {
  ];
  test.run_scripts(scripts).await;
 }
+
+#[tokio::test]
+async fn sort_checklist_by_ascending_test() {
+  let mut test = DatabaseSortTest::new().await;
+  let checklist_field = test.get_first_field(FieldType::Checklist);
+  let scripts = vec![
+    AssertCellContentOrder {
+      field_id: checklist_field.id.clone(),
+      orders: vec![
+        "First thing",
+        "Have breakfast,Have lunch,Take a nap,Have dinner,Shower and head to bed",
+        "",
+        "Task 1",
+        "",
+        "Sprint,Sprint some more,Rest",
+        "",
+      ],
+    },
+    InsertSort {
+      field: checklist_field.clone(),
+      condition: SortCondition::Ascending,
+    },
+    AssertCellContentOrder {
+      field_id: checklist_field.id.clone(),
+      orders: vec![
+        "First thing",
+        "Have breakfast,Have lunch,Take a nap,Have dinner,Shower and head to bed",
+        "Sprint,Sprint some more,Rest",
+        "Task 1",
+        "",
+        "",
+        "",
+      ],
+    },
+  ];
+  test.run_scripts(scripts).await;
+}
+
+#[tokio::test]
+async fn sort_checklist_by_descending_test() {
+  let mut test = DatabaseSortTest::new().await;
+  let checklist_field = test.get_first_field(FieldType::Checklist);
+  let scripts = vec![
+    AssertCellContentOrder {
+      field_id: checklist_field.id.clone(),
+      orders: vec![
+        "First thing",
+        "Have breakfast,Have lunch,Take a nap,Have dinner,Shower and head to bed",
+        "",
+        "Task 1",
+        "",
+        "Sprint,Sprint some more,Rest",
+        "",
+      ],
+    },
+    InsertSort {
+      field: checklist_field.clone(),
+      condition: SortCondition::Descending,
+    },
+    AssertCellContentOrder {
+      field_id: checklist_field.id.clone(),
+      orders: vec![
+        "Task 1",
+        "Sprint,Sprint some more,Rest",
+        "Have breakfast,Have lunch,Take a nap,Have dinner,Shower and head to bed",
+        "First thing",
+        "",
+        "",
+        "",
+      ],
+    },
+  ];
+  test.run_scripts(scripts).await;
+}