refactor: checkbox field type (#4682)

frontend/rust-lib/flowy-database2/src/entities/type_option_entities/checkbox_entities.rs

@@ -1,24 +1,33 @@
 use crate::services::field::CheckboxTypeOption;
 use flowy_derive::ProtoBuf;
 
+#[derive(Default, Debug, Clone, ProtoBuf)]
+pub struct CheckboxCellDataPB {
+  #[pb(index = 1)]
+  pub is_checked: bool,
+}
+
+impl CheckboxCellDataPB {
+  pub fn new(is_checked: bool) -> Self {
+    Self { is_checked }
+  }
+}
+
 #[derive(Debug, Clone, Default, ProtoBuf)]
 pub struct CheckboxTypeOptionPB {
+  /// unused
   #[pb(index = 1)]
-  pub is_selected: bool,
+  pub dummy_field: bool,
 }
 
 impl From<CheckboxTypeOption> for CheckboxTypeOptionPB {
-  fn from(data: CheckboxTypeOption) -> Self {
-    Self {
-      is_selected: data.is_selected,
-    }
+  fn from(_type_option: CheckboxTypeOption) -> Self {
+    Self { dummy_field: false }
   }
 }
 
 impl From<CheckboxTypeOptionPB> for CheckboxTypeOption {
-  fn from(data: CheckboxTypeOptionPB) -> Self {
-    Self {
-      is_selected: data.is_selected,
-    }
+  fn from(_type_option: CheckboxTypeOptionPB) -> Self {
+    Self()
   }
 }

frontend/rust-lib/flowy-database2/src/services/cell/cell_operation.rs

@@ -6,7 +6,7 @@ use collab_database::rows::{get_field_type_from_cell, Cell, Cells};
 
 use flowy_error::{ErrorCode, FlowyError, FlowyResult};
 
-use crate::entities::FieldType;
+use crate::entities::{CheckboxCellDataPB, FieldType};
 use crate::services::cell::{CellCache, CellProtobufBlob};
 use crate::services::field::checklist_type_option::ChecklistCellChangeset;
 use crate::services::field::*;
@@ -203,8 +203,8 @@ pub fn insert_url_cell(url: String, field: &Field) -> Cell {
   apply_cell_changeset(url, None, field, None).unwrap()
 }
 
-pub fn insert_checkbox_cell(is_check: bool, field: &Field) -> Cell {
-  let s = if is_check {
+pub fn insert_checkbox_cell(is_checked: bool, field: &Field) -> Cell {
+  let s = if is_checked {
     CHECK.to_string()
   } else {
     UNCHECK.to_string()
@@ -343,8 +343,8 @@ impl<'a> CellBuilder<'a> {
             }
           },
           FieldType::Checkbox => {
-            if let Ok(value) = CheckboxCellData::from_cell_str(&cell_str) {
-              cells.insert(field_id, insert_checkbox_cell(value.into_inner(), field));
+            if let Ok(value) = CheckboxCellDataPB::from_cell_str(&cell_str) {
+              cells.insert(field_id, insert_checkbox_cell(value.is_checked, field));
             }
           },
           FieldType::URL => {
@@ -399,13 +399,13 @@ impl<'a> CellBuilder<'a> {
     }
   }
 
-  pub fn insert_checkbox_cell(&mut self, field_id: &str, is_check: bool) {
+  pub fn insert_checkbox_cell(&mut self, field_id: &str, is_checked: bool) {
     match self.field_maps.get(&field_id.to_owned()) {
       None => tracing::warn!("Can't find the checkbox field with id: {}", field_id),
       Some(field) => {
         self
           .cells
-          .insert(field_id.to_owned(), insert_checkbox_cell(is_check, field));
+          .insert(field_id.to_owned(), insert_checkbox_cell(is_checked, field));
       },
     }
   }

frontend/rust-lib/flowy-database2/src/services/field/type_options/checkbox_type_option/checkbox_filter.rs

@@ -1,20 +1,17 @@
-use crate::entities::{CheckboxFilterConditionPB, CheckboxFilterPB};
-use crate::services::field::CheckboxCellData;
+use crate::entities::{CheckboxCellDataPB, CheckboxFilterConditionPB, CheckboxFilterPB};
 
 impl CheckboxFilterPB {
-  pub fn is_visible(&self, cell_data: &CheckboxCellData) -> bool {
-    let is_check = cell_data.is_check();
+  pub fn is_visible(&self, cell_data: &CheckboxCellDataPB) -> bool {
     match self.condition {
-      CheckboxFilterConditionPB::IsChecked => is_check,
-      CheckboxFilterConditionPB::IsUnChecked => !is_check,
+      CheckboxFilterConditionPB::IsChecked => cell_data.is_checked,
+      CheckboxFilterConditionPB::IsUnChecked => !cell_data.is_checked,
     }
   }
 }
 
 #[cfg(test)]
 mod tests {
-  use crate::entities::{CheckboxFilterConditionPB, CheckboxFilterPB};
-  use crate::services::field::CheckboxCellData;
+  use crate::entities::{CheckboxCellDataPB, CheckboxFilterConditionPB, CheckboxFilterPB};
   use std::str::FromStr;
 
   #[test]
@@ -27,8 +24,9 @@ mod tests {
       ("yes", true),
       ("false", false),
       ("no", false),
+      ("", false),
     ] {
-      let data = CheckboxCellData::from_str(value).unwrap();
+      let data = CheckboxCellDataPB::from_str(value).unwrap();
       assert_eq!(checkbox_filter.is_visible(&data), visible);
     }
   }
@@ -43,8 +41,9 @@ mod tests {
       ("no", true),
       ("true", false),
       ("yes", false),
+      ("", true),
     ] {
-      let data = CheckboxCellData::from_str(value).unwrap();
+      let data = CheckboxCellDataPB::from_str(value).unwrap();
       assert_eq!(checkbox_filter.is_visible(&data), visible);
     }
   }

frontend/rust-lib/flowy-database2/src/services/field/type_options/checkbox_type_option/checkbox_tests.rs

@@ -2,6 +2,7 @@
 mod tests {
   use collab_database::fields::Field;
 
+  use crate::entities::CheckboxCellDataPB;
   use crate::entities::FieldType;
   use crate::services::cell::CellDataDecoder;
   use crate::services::cell::FromCellString;
@@ -27,9 +28,9 @@ mod tests {
     assert_checkbox(&type_option, "NO", UNCHECK, &field_type, &field_rev);
     assert_checkbox(&type_option, "0", UNCHECK, &field_type, &field_rev);
 
-    // the checkout value will be empty if the value is letters or empty string
-    assert_checkbox(&type_option, "abc", "", &field_type, &field_rev);
-    assert_checkbox(&type_option, "", "", &field_type, &field_rev);
+    // the checkout value will be uncheck as well if the value is letters or empty string
+    assert_checkbox(&type_option, "abc", UNCHECK, &field_type, &field_rev);
+    assert_checkbox(&type_option, "", UNCHECK, &field_type, &field_rev);
   }
 
   fn assert_checkbox(
@@ -42,7 +43,7 @@ mod tests {
     assert_eq!(
       type_option
         .decode_cell(
-          &CheckboxCellData::from_cell_str(input_str).unwrap().into(),
+          &CheckboxCellDataPB::from_cell_str(input_str).unwrap().into(),
           field_type,
           field
         )

frontend/rust-lib/flowy-database2/src/services/field/type_options/checkbox_type_option/checkbox_type_option.rs

@@ -1,30 +1,27 @@
 use std::cmp::Ordering;
 use std::str::FromStr;
 
-use collab::core::any_map::AnyMapExtension;
 use collab_database::fields::{Field, TypeOptionData, TypeOptionDataBuilder};
 use collab_database::rows::Cell;
 use serde::{Deserialize, Serialize};
 
 use flowy_error::FlowyResult;
 
-use crate::entities::{CheckboxFilterPB, FieldType};
+use crate::entities::{CheckboxCellDataPB, CheckboxFilterPB, FieldType};
 use crate::services::cell::{CellDataChangeset, CellDataDecoder};
 use crate::services::field::{
-  CheckboxCellData, TypeOption, TypeOptionCellDataCompare, TypeOptionCellDataFilter,
-  TypeOptionCellDataSerde, TypeOptionTransform,
+  TypeOption, TypeOptionCellDataCompare, TypeOptionCellDataFilter, TypeOptionCellDataSerde,
+  TypeOptionTransform,
 };
 use crate::services::sort::SortCondition;
 
 #[derive(Debug, Clone, Serialize, Deserialize, Default)]
-pub struct CheckboxTypeOption {
-  pub is_selected: bool,
-}
+pub struct CheckboxTypeOption();
 
 impl TypeOption for CheckboxTypeOption {
-  type CellData = CheckboxCellData;
+  type CellData = CheckboxCellDataPB;
   type CellChangeset = CheckboxCellChangeset;
-  type CellProtobufType = CheckboxCellData;
+  type CellProtobufType = CheckboxCellDataPB;
   type CellFilter = CheckboxFilterPB;
 }
 
@@ -47,7 +44,7 @@ impl TypeOptionTransform for CheckboxTypeOption {
     _field: &Field,
   ) -> Option<<Self as TypeOption>::CellData> {
     if transformed_field_type.is_text() {
-      Some(CheckboxCellData::from(cell))
+      Some(CheckboxCellDataPB::from(cell))
     } else {
       None
     }
@@ -55,17 +52,14 @@ impl TypeOptionTransform for CheckboxTypeOption {
 }
 
 impl From<TypeOptionData> for CheckboxTypeOption {
-  fn from(data: TypeOptionData) -> Self {
-    let is_selected = data.get_bool_value("is_selected").unwrap_or(false);
-    CheckboxTypeOption { is_selected }
+  fn from(_data: TypeOptionData) -> Self {
+    Self()
   }
 }
 
 impl From<CheckboxTypeOption> for TypeOptionData {
-  fn from(data: CheckboxTypeOption) -> Self {
-    TypeOptionDataBuilder::new()
-      .insert_bool_value("is_selected", data.is_selected)
-      .build()
+  fn from(_data: CheckboxTypeOption) -> Self {
+    TypeOptionDataBuilder::new().build()
   }
 }
 
@@ -78,7 +72,7 @@ impl TypeOptionCellDataSerde for CheckboxTypeOption {
   }
 
   fn parse_cell(&self, cell: &Cell) -> FlowyResult<<Self as TypeOption>::CellData> {
-    Ok(CheckboxCellData::from(cell))
+    Ok(CheckboxCellDataPB::from(cell))
   }
 }
 
@@ -105,7 +99,7 @@ impl CellDataDecoder for CheckboxTypeOption {
 
   fn numeric_cell(&self, cell: &Cell) -> Option<f64> {
     let cell_data = self.parse_cell(cell).ok()?;
-    if cell_data.is_check() {
+    if cell_data.is_checked {
       Some(1.0)
     } else {
       Some(0.0)
@@ -121,7 +115,7 @@ impl CellDataChangeset for CheckboxTypeOption {
     changeset: <Self as TypeOption>::CellChangeset,
     _cell: Option<Cell>,
   ) -> FlowyResult<(Cell, <Self as TypeOption>::CellData)> {
-    let checkbox_cell_data = CheckboxCellData::from_str(&changeset)?;
+    let checkbox_cell_data = CheckboxCellDataPB::from_str(&changeset)?;
     Ok((checkbox_cell_data.clone().into(), checkbox_cell_data))
   }
 }
@@ -147,7 +141,7 @@ impl TypeOptionCellDataCompare for CheckboxTypeOption {
     other_cell_data: &<Self as TypeOption>::CellData,
     sort_condition: SortCondition,
   ) -> Ordering {
-    let order = cell_data.is_check().cmp(&other_cell_data.is_check());
+    let order = cell_data.is_checked.cmp(&other_cell_data.is_checked);
     sort_condition.evaluate_order(order)
   }
 
@@ -164,10 +158,10 @@ impl TypeOptionCellDataCompare for CheckboxTypeOption {
     sort_condition: SortCondition,
   ) -> Ordering {
     match (cell_data, other_cell_data) {
-      (None, Some(right_cell_data)) if right_cell_data.is_check() => {
+      (None, Some(right_cell_data)) if right_cell_data.is_checked => {
         sort_condition.evaluate_order(Ordering::Less)
       },
-      (Some(left_cell_data), None) if left_cell_data.is_check() => {
+      (Some(left_cell_data), None) if left_cell_data.is_checked => {
         sort_condition.evaluate_order(Ordering::Greater)
       },
       _ => Ordering::Equal,

frontend/rust-lib/flowy-database2/src/services/field/type_options/checkbox_type_option/checkbox_type_option_entities.rs

@@ -3,93 +3,53 @@ use std::str::FromStr;
 use bytes::Bytes;
 use collab::core::any_map::AnyMapExtension;
 use collab_database::rows::{new_cell_builder, Cell};
-use protobuf::ProtobufError;
 
 use flowy_error::{FlowyError, FlowyResult};
 
-use crate::entities::FieldType;
+use crate::entities::{CheckboxCellDataPB, FieldType};
 use crate::services::cell::{CellProtobufBlobParser, DecodedCellData, FromCellString};
 use crate::services::field::{TypeOptionCellData, CELL_DATA};
 
 pub const CHECK: &str = "Yes";
 pub const UNCHECK: &str = "No";
 
-#[derive(Default, Debug, Clone)]
-pub struct CheckboxCellData(pub String);
-
-impl CheckboxCellData {
-  pub fn into_inner(self) -> bool {
-    self.is_check()
-  }
-
-  pub fn is_check(&self) -> bool {
-    self.0 == CHECK
-  }
-
-  pub fn is_uncheck(&self) -> bool {
-    self.0 == UNCHECK
-  }
-}
-
-impl TypeOptionCellData for CheckboxCellData {
+impl TypeOptionCellData for CheckboxCellDataPB {
   fn is_cell_empty(&self) -> bool {
     false
   }
 }
 
-impl AsRef<[u8]> for CheckboxCellData {
-  fn as_ref(&self) -> &[u8] {
-    self.0.as_ref()
-  }
-}
-
-impl From<&Cell> for CheckboxCellData {
+impl From<&Cell> for CheckboxCellDataPB {
   fn from(cell: &Cell) -> Self {
     let value = cell.get_str_value(CELL_DATA).unwrap_or_default();
-    CheckboxCellData::from_cell_str(&value).unwrap_or_default()
+    CheckboxCellDataPB::from_cell_str(&value).unwrap_or_default()
   }
 }
 
-impl From<CheckboxCellData> for Cell {
-  fn from(data: CheckboxCellData) -> Self {
+impl From<CheckboxCellDataPB> for Cell {
+  fn from(data: CheckboxCellDataPB) -> Self {
     new_cell_builder(FieldType::Checkbox)
       .insert_str_value(CELL_DATA, data.to_string())
       .build()
   }
 }
 
-impl FromStr for CheckboxCellData {
+impl FromStr for CheckboxCellDataPB {
   type Err = FlowyError;
 
   fn from_str(s: &str) -> Result<Self, Self::Err> {
     let lower_case_str: &str = &s.to_lowercase();
-    let val = match lower_case_str {
-      "1" => Some(true),
-      "true" => Some(true),
-      "yes" => Some(true),
-      "0" => Some(false),
-      "false" => Some(false),
-      "no" => Some(false),
-      _ => None,
+    let is_checked = match lower_case_str {
+      "1" | "true" | "yes" => true,
+      "0" | "false" | "no" => false,
+      _ => false,
     };
 
-    match val {
-      Some(true) => Ok(Self(CHECK.to_string())),
-      Some(false) => Ok(Self(UNCHECK.to_string())),
-      None => Ok(Self("".to_string())),
-    }
+    Ok(Self::new(is_checked))
   }
 }
 
-impl std::convert::TryFrom<CheckboxCellData> for Bytes {
-  type Error = ProtobufError;
-
-  fn try_from(value: CheckboxCellData) -> Result<Self, Self::Error> {
-    Ok(Bytes::from(value.0))
-  }
-}
-
-impl FromCellString for CheckboxCellData {
+impl FromCellString for CheckboxCellDataPB {
   fn from_cell_str(s: &str) -> FlowyResult<Self>
   where
     Self: Sized,
@@ -98,27 +58,29 @@ impl FromCellString for CheckboxCellData {
   }
 }
 
-impl ToString for CheckboxCellData {
+impl ToString for CheckboxCellDataPB {
   fn to_string(&self) -> String {
-    self.0.clone()
+    if self.is_checked {
+      CHECK.to_string()
+    } else {
+      UNCHECK.to_string()
+    }
   }
 }
 
-impl DecodedCellData for CheckboxCellData {
-  type Object = CheckboxCellData;
+impl DecodedCellData for CheckboxCellDataPB {
+  type Object = CheckboxCellDataPB;
 
   fn is_empty(&self) -> bool {
-    self.0.is_empty()
+    false
   }
 }
 
 pub struct CheckboxCellDataParser();
 impl CellProtobufBlobParser for CheckboxCellDataParser {
-  type Object = CheckboxCellData;
+  type Object = CheckboxCellDataPB;
+
   fn parser(bytes: &Bytes) -> FlowyResult<Self::Object> {
-    match String::from_utf8(bytes.to_vec()) {
-      Ok(s) => CheckboxCellData::from_cell_str(&s),
-      Err(_) => Ok(CheckboxCellData("".to_string())),
-    }
+    CheckboxCellDataPB::try_from(bytes.as_ref()).or_else(|_| Ok(CheckboxCellDataPB::default()))
   }
 }

frontend/rust-lib/flowy-database2/src/services/field/type_options/selection_type_option/multi_select_type_option.rs

@@ -190,7 +190,7 @@ mod tests {
 
   #[test]
   fn multi_select_transform_with_checkbox_type_option_test() {
-    let checkbox_type_option = CheckboxTypeOption { is_selected: false };
+    let checkbox_type_option = CheckboxTypeOption();
     let mut multi_select = MultiSelectTypeOption::default();
     multi_select.transform_type_option(FieldType::Checkbox, checkbox_type_option.clone().into());
     debug_assert_eq!(multi_select.options.len(), 2);

frontend/rust-lib/flowy-database2/src/services/field/type_options/selection_type_option/select_type_option.rs

@@ -5,15 +5,15 @@ use serde::{Deserialize, Serialize};
 
 use flowy_error::{internal_error, ErrorCode, FlowyResult};
 
-use crate::entities::{FieldType, SelectOptionCellDataPB};
+use crate::entities::{CheckboxCellDataPB, FieldType, SelectOptionCellDataPB};
 use crate::services::cell::{
   CellDataDecoder, CellProtobufBlobParser, DecodedCellData, FromCellChangeset, ToCellChangeset,
 };
 use crate::services::field::selection_type_option::type_option_transform::SelectOptionTypeOptionTransformHelper;
 use crate::services::field::{
-  make_selected_options, CheckboxCellData, MultiSelectTypeOption, SelectOption,
-  SelectOptionCellData, SelectOptionColor, SelectOptionIds, SingleSelectTypeOption, TypeOption,
-  TypeOptionCellDataSerde, TypeOptionTransform, SELECTION_IDS_SEPARATOR,
+  make_selected_options, MultiSelectTypeOption, SelectOption, SelectOptionCellData,
+  SelectOptionColor, SelectOptionIds, SingleSelectTypeOption, TypeOption, TypeOptionCellDataSerde,
+  TypeOptionTransform, SELECTION_IDS_SEPARATOR,
 };
 
 /// Defines the shared actions used by SingleSelect or Multi-Select.
@@ -104,7 +104,7 @@ where
         None
       },
       FieldType::Checkbox => {
-        let cell_content = CheckboxCellData::from(cell).to_string();
+        let cell_content = CheckboxCellDataPB::from(cell).to_string();
         let mut transformed_ids = Vec::new();
         let options = self.options();
         if let Some(option) = options.iter().find(|option| option.name == cell_content) {

frontend/rust-lib/flowy-database2/src/services/group/controller_impls/checkbox_controller.rs

@@ -47,7 +47,7 @@ impl GroupCustomize for CheckboxGroupController {
     content: &str,
     cell_data: &<Self::GroupTypeOption as TypeOption>::CellData,
   ) -> bool {
-    if cell_data.is_check() {
+    if cell_data.is_checked {
       content == CHECK
     } else {
       content == UNCHECK
@@ -64,7 +64,7 @@ impl GroupCustomize for CheckboxGroupController {
       let mut changeset = GroupRowsNotificationPB::new(group.id.clone());
       let is_not_contained = !group.contains_row(&row_detail.row.id);
       if group.id == CHECK {
-        if cell_data.is_uncheck() {
+        if !cell_data.is_checked {
           // Remove the row if the group.id is CHECK but the cell_data is UNCHECK
           changeset
             .deleted_rows
@@ -82,7 +82,7 @@ impl GroupCustomize for CheckboxGroupController {
       }
 
       if group.id == UNCHECK {
-        if cell_data.is_check() {
+        if cell_data.is_checked {
           // Remove the row if the group.id is UNCHECK but the cell_data is CHECK
           changeset
             .deleted_rows
@@ -154,8 +154,8 @@ impl GroupController for CheckboxGroupController {
     match self.context.get_group(group_id) {
       None => tracing::warn!("Can not find the group: {}", group_id),
       Some((_, group)) => {
-        let is_check = group.id == CHECK;
-        let cell = insert_checkbox_cell(is_check, field);
+        let is_checked = group.id == CHECK;
+        let cell = insert_checkbox_cell(is_checked, field);
         cells.insert(field.id.clone(), cell);
       },
     }