From fa0a485c85c3c2da565dfbbd6aeda1d3c3024f7e Mon Sep 17 00:00:00 2001 From: appflowy Date: Sat, 24 Sep 2022 15:57:40 +0800 Subject: [PATCH] fix: move checkbox card fail --- .../flowy-grid/src/dart_notification.rs | 4 -- .../src/services/cell/any_cell_data.rs | 5 +- .../src/services/cell/cell_operation.rs | 35 +++++++++----- .../flowy-grid/src/services/group/action.rs | 1 + .../src/services/group/controller.rs | 22 +++++---- .../controller_impls/checkbox_controller.rs | 2 +- .../single_select_controller.rs | 2 +- .../select_option_controller/util.rs | 48 +++++++++++++++---- 8 files changed, 81 insertions(+), 38 deletions(-) diff --git a/frontend/rust-lib/flowy-grid/src/dart_notification.rs b/frontend/rust-lib/flowy-grid/src/dart_notification.rs index 8d7ec2ba36..7489682619 100644 --- a/frontend/rust-lib/flowy-grid/src/dart_notification.rs +++ b/frontend/rust-lib/flowy-grid/src/dart_notification.rs @@ -34,7 +34,3 @@ pub fn send_dart_notification(id: &str, ty: GridNotification) -> DartNotifyBuild DartNotifyBuilder::new(id, ty, OBSERVABLE_CATEGORY) } -#[tracing::instrument(level = "trace")] -pub fn send_anonymous_dart_notification(ty: GridNotification) -> DartNotifyBuilder { - DartNotifyBuilder::new("", ty, OBSERVABLE_CATEGORY) -} diff --git a/frontend/rust-lib/flowy-grid/src/services/cell/any_cell_data.rs b/frontend/rust-lib/flowy-grid/src/services/cell/any_cell_data.rs index 8f76ea62c5..f7ef205435 100644 --- a/frontend/rust-lib/flowy-grid/src/services/cell/any_cell_data.rs +++ b/frontend/rust-lib/flowy-grid/src/services/cell/any_cell_data.rs @@ -19,7 +19,10 @@ impl std::str::FromStr for AnyCellData { type Err = FlowyError; fn from_str(s: &str) -> Result { - let type_option_cell_data: AnyCellData = serde_json::from_str(s)?; + let type_option_cell_data: AnyCellData = serde_json::from_str(s).map_err(|err| { + let msg = format!("Deserialize {} to any cell data failed. Serde error: {}", s, err); + FlowyError::internal().context(msg) + })?; Ok(type_option_cell_data) } } diff --git a/frontend/rust-lib/flowy-grid/src/services/cell/cell_operation.rs b/frontend/rust-lib/flowy-grid/src/services/cell/cell_operation.rs index 20dd09a5ef..569a5f2f7d 100644 --- a/frontend/rust-lib/flowy-grid/src/services/cell/cell_operation.rs +++ b/frontend/rust-lib/flowy-grid/src/services/cell/cell_operation.rs @@ -1,6 +1,7 @@ use crate::entities::FieldType; use crate::services::cell::{AnyCellData, CellBytes}; use crate::services::field::*; +use std::fmt::Debug; use flowy_error::{ErrorCode, FlowyError, FlowyResult}; use flowy_grid_data_model::revision::{CellRevision, FieldRevision, FieldTypeRevision}; @@ -73,20 +74,30 @@ pub fn apply_cell_data_changeset>( Ok(AnyCellData::new(s, field_type).json()) } -pub fn decode_any_cell_data>(data: T, field_rev: &FieldRevision) -> CellBytes { - if let Ok(any_cell_data) = data.try_into() { - let AnyCellData { data, field_type } = any_cell_data; - let to_field_type = field_rev.ty.into(); - match try_decode_cell_data(data.into(), field_rev, &field_type, &to_field_type) { - Ok(cell_bytes) => cell_bytes, - Err(e) => { - tracing::error!("Decode cell data failed, {:?}", e); - CellBytes::default() +pub fn decode_any_cell_data + Debug>( + data: T, + field_rev: &FieldRevision, +) -> CellBytes { + match data.try_into() { + Ok(any_cell_data) => { + let AnyCellData { data, field_type } = any_cell_data; + let to_field_type = field_rev.ty.into(); + match try_decode_cell_data(data.into(), field_rev, &field_type, &to_field_type) { + Ok(cell_bytes) => cell_bytes, + Err(e) => { + tracing::error!("Decode cell data failed, {:?}", e); + CellBytes::default() + } } } - } else { - tracing::error!("Decode type option data failed"); - CellBytes::default() + Err(err) => { + tracing::error!( + "Decode type option data to type: {} failed: {:?}", + std::any::type_name::(), + err, + ); + CellBytes::default() + } } } diff --git a/frontend/rust-lib/flowy-grid/src/services/group/action.rs b/frontend/rust-lib/flowy-grid/src/services/group/action.rs index fc92b68c46..fd9adcb8b5 100644 --- a/frontend/rust-lib/flowy-grid/src/services/group/action.rs +++ b/frontend/rust-lib/flowy-grid/src/services/group/action.rs @@ -14,5 +14,6 @@ pub trait GroupAction: Send + Sync { fn can_group(&self, content: &str, cell_data: &Self::CellDataType) -> bool; fn add_row_if_match(&mut self, row_rev: &RowRevision, cell_data: &Self::CellDataType) -> Vec; fn remove_row_if_match(&mut self, row_rev: &RowRevision, cell_data: &Self::CellDataType) -> Vec; + // Move row from one group to another fn move_row(&mut self, cell_data: &Self::CellDataType, context: MoveGroupRowContext) -> Vec; } diff --git a/frontend/rust-lib/flowy-grid/src/services/group/controller.rs b/frontend/rust-lib/flowy-grid/src/services/group/controller.rs index 5ea8639c92..7a1d7dbe62 100644 --- a/frontend/rust-lib/flowy-grid/src/services/group/controller.rs +++ b/frontend/rust-lib/flowy-grid/src/services/group/controller.rs @@ -1,12 +1,10 @@ -use crate::entities::{GroupChangesetPB, GroupViewChangesetPB, InsertedRowPB, RowPB}; -use crate::services::cell::{decode_any_cell_data, CellBytesParser}; +use crate::entities::{ GroupChangesetPB, GroupViewChangesetPB, InsertedRowPB, RowPB}; +use crate::services::cell::{decode_any_cell_data, CellBytesParser,}; use crate::services::group::action::GroupAction; use crate::services::group::configuration::GroupContext; use crate::services::group::entities::Group; use flowy_error::FlowyResult; -use flowy_grid_data_model::revision::{ - FieldRevision, GroupConfigurationContentSerde, GroupRevision, RowChangeset, RowRevision, TypeOptionDataDeserializer, -}; +use flowy_grid_data_model::revision::{ FieldRevision, GroupConfigurationContentSerde, GroupRevision, RowChangeset, RowRevision, TypeOptionDataDeserializer}; use std::marker::PhantomData; use std::sync::Arc; @@ -86,8 +84,7 @@ where G: GroupGenerator, TypeOptionType = T>, { pub async fn new(field_rev: &Arc, mut configuration: GroupContext) -> FlowyResult { - let field_type_rev = field_rev.ty; - let type_option = field_rev.get_type_option::(field_type_rev); + let type_option = field_rev.get_type_option::(field_rev.ty); let groups = G::generate_groups(&field_rev.id, &configuration, &type_option); let _ = configuration.init_groups(groups, true)?; @@ -278,12 +275,19 @@ where } } + #[tracing::instrument(level = "trace", skip_all, err)] fn move_group_row(&mut self, context: MoveGroupRowContext) -> FlowyResult> { - if let Some(cell_rev) = context.row_rev.cells.get(&self.field_id) { - let cell_bytes = decode_any_cell_data(cell_rev.data.clone(), context.field_rev); + let cell_rev = match context.row_rev.cells.get(&self.field_id) { + Some(cell_rev) => Some(cell_rev.clone()), + None => self.default_cell_rev(), + }; + + if let Some(cell_rev) = cell_rev { + let cell_bytes = decode_any_cell_data(cell_rev.data, context.field_rev); let cell_data = cell_bytes.parser::

()?; Ok(self.move_row(&cell_data, context)) } else { + tracing::warn!("Unexpected moving group row, changes should not be empty"); Ok(vec![]) } } diff --git a/frontend/rust-lib/flowy-grid/src/services/group/controller_impls/checkbox_controller.rs b/frontend/rust-lib/flowy-grid/src/services/group/controller_impls/checkbox_controller.rs index a15b0cc62b..aa0a0bec96 100644 --- a/frontend/rust-lib/flowy-grid/src/services/group/controller_impls/checkbox_controller.rs +++ b/frontend/rust-lib/flowy-grid/src/services/group/controller_impls/checkbox_controller.rs @@ -80,7 +80,7 @@ impl GroupAction for CheckboxGroupController { fn move_row(&mut self, _cell_data: &Self::CellDataType, mut context: MoveGroupRowContext) -> Vec { let mut group_changeset = vec![]; self.group_ctx.iter_mut_all_groups(|group| { - if let Some(changeset) = move_group_row(group, &mut context) { + if let Some(changeset) = move_group_row(group, &mut context) { group_changeset.push(changeset); } }); diff --git a/frontend/rust-lib/flowy-grid/src/services/group/controller_impls/select_option_controller/single_select_controller.rs b/frontend/rust-lib/flowy-grid/src/services/group/controller_impls/select_option_controller/single_select_controller.rs index 0c86166520..8ba632cc81 100644 --- a/frontend/rust-lib/flowy-grid/src/services/group/controller_impls/select_option_controller/single_select_controller.rs +++ b/frontend/rust-lib/flowy-grid/src/services/group/controller_impls/select_option_controller/single_select_controller.rs @@ -49,7 +49,7 @@ impl GroupAction for SingleSelectGroupController { fn move_row(&mut self, _cell_data: &Self::CellDataType, mut context: MoveGroupRowContext) -> Vec { let mut group_changeset = vec![]; self.group_ctx.iter_mut_all_groups(|group| { - if let Some(changeset) = move_group_row(group, &mut context) { + if let Some(changeset) = move_group_row(group, &mut context) { group_changeset.push(changeset); } }); diff --git a/frontend/rust-lib/flowy-grid/src/services/group/controller_impls/select_option_controller/util.rs b/frontend/rust-lib/flowy-grid/src/services/group/controller_impls/select_option_controller/util.rs index a1d81b0d5c..9c3e71bc3e 100644 --- a/frontend/rust-lib/flowy-grid/src/services/group/controller_impls/select_option_controller/util.rs +++ b/frontend/rust-lib/flowy-grid/src/services/group/controller_impls/select_option_controller/util.rs @@ -1,11 +1,12 @@ -use crate::entities::{GroupChangesetPB, InsertedRowPB, RowPB}; -use crate::services::cell::insert_select_option_cell; -use crate::services::field::{SelectOptionCellDataPB, SelectOptionPB}; +use crate::entities::{FieldType, GroupChangesetPB, InsertedRowPB, RowPB}; +use crate::services::cell::{insert_checkbox_cell, insert_select_option_cell}; +use crate::services::field::{SelectOptionCellDataPB, SelectOptionPB, CHECK}; use crate::services::group::configuration::GroupContext; -use crate::services::group::{GeneratedGroup, Group}; - use crate::services::group::controller::MoveGroupRowContext; -use flowy_grid_data_model::revision::{GroupRevision, RowRevision, SelectOptionGroupConfigurationRevision}; +use crate::services::group::{GeneratedGroup, Group}; +use flowy_grid_data_model::revision::{ + CellRevision, FieldRevision, GroupRevision, RowRevision, SelectOptionGroupConfigurationRevision, +}; pub type SelectOptionGroupContext = GroupContext; @@ -109,10 +110,16 @@ pub fn move_group_row(group: &mut Group, context: &mut MoveGroupRowContext) -> O // Update the corresponding row's cell content. if from_index.is_none() { - tracing::debug!("Mark row:{} belong to group:{}", row_rev.id, group.id); - let cell_rev = insert_select_option_cell(group.id.clone(), field_rev); - row_changeset.cell_by_field_id.insert(field_rev.id.clone(), cell_rev); - changeset.updated_rows.push(RowPB::from(*row_rev)); + let cell_rev = make_inserted_cell_rev(&group.id, field_rev); + if let Some(cell_rev) = cell_rev { + tracing::debug!( + "Update content of the cell in the row:{} to group:{}", + row_rev.id, + group.id + ); + row_changeset.cell_by_field_id.insert(field_rev.id.clone(), cell_rev); + changeset.updated_rows.push(RowPB::from(*row_rev)); + } } } if changeset.is_empty() { @@ -122,6 +129,27 @@ pub fn move_group_row(group: &mut Group, context: &mut MoveGroupRowContext) -> O } } +pub fn make_inserted_cell_rev(group_id: &str, field_rev: &FieldRevision) -> Option { + let field_type: FieldType = field_rev.ty.into(); + match field_type { + FieldType::SingleSelect => { + let cell_rev = insert_select_option_cell(group_id.to_owned(), field_rev); + Some(cell_rev) + } + FieldType::MultiSelect => { + let cell_rev = insert_select_option_cell(group_id.to_owned(), field_rev); + Some(cell_rev) + } + FieldType::Checkbox => { + let cell_rev = insert_checkbox_cell(group_id == CHECK, field_rev); + Some(cell_rev) + } + _ => { + tracing::warn!("Unknown field type: {:?}", field_type); + None + } + } +} pub fn generate_select_option_groups( _field_id: &str, _group_ctx: &SelectOptionGroupContext,