From dfb7ed10536a403027b05f40ba856c1a659d0115 Mon Sep 17 00:00:00 2001 From: nathan Date: Sat, 24 Sep 2022 21:48:48 +0800 Subject: [PATCH] fix: switch field type in board --- .../rust-lib/flowy-grid/src/event_handler.rs | 2 +- frontend/rust-lib/flowy-grid/src/macros.rs | 7 ++++++- .../src/services/cell/cell_operation.rs | 10 ++++------ .../flowy-grid/src/services/grid_editor.rs | 17 ++++++++++------- .../src/services/grid_view_manager.rs | 10 ++++++++-- .../flowy-grid/tests/grid/group_test/test.rs | 7 ++----- 6 files changed, 31 insertions(+), 22 deletions(-) diff --git a/frontend/rust-lib/flowy-grid/src/event_handler.rs b/frontend/rust-lib/flowy-grid/src/event_handler.rs index c9b6cf4662..1de9c2ed30 100644 --- a/frontend/rust-lib/flowy-grid/src/event_handler.rs +++ b/frontend/rust-lib/flowy-grid/src/event_handler.rs @@ -44,7 +44,7 @@ pub(crate) async fn update_grid_setting_handler( let editor = manager.get_grid_editor(¶ms.grid_id)?; if let Some(insert_params) = params.insert_group { - let _ = editor.create_group(insert_params).await?; + let _ = editor.insert_group(insert_params).await?; } if let Some(delete_params) = params.delete_group { diff --git a/frontend/rust-lib/flowy-grid/src/macros.rs b/frontend/rust-lib/flowy-grid/src/macros.rs index bee2e95e21..6ca3de52e5 100644 --- a/frontend/rust-lib/flowy-grid/src/macros.rs +++ b/frontend/rust-lib/flowy-grid/src/macros.rs @@ -73,7 +73,12 @@ macro_rules! impl_type_option { match serde_json::from_str(s) { Ok(obj) => obj, Err(err) => { - tracing::error!("{} convert from any data failed, {:?}", stringify!($target), err); + tracing::error!( + "{} type option deserialize from {} failed, {:?}", + stringify!($target), + s, + err + ); $target::default() } } 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 569a5f2f7d..3d6f951dfd 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 @@ -90,12 +90,10 @@ pub fn decode_any_cell_data + Debug> } } } - Err(err) => { - tracing::error!( - "Decode type option data to type: {} failed: {:?}", - std::any::type_name::(), - err, - ); + Err(_err) => { + // It's okay to ignore this error, because it's okay that the current cell can't + // display the existing cell data. For example, the UI of the text cell will be blank if + // the type of the data of cell is Number. CellBytes::default() } } diff --git a/frontend/rust-lib/flowy-grid/src/services/grid_editor.rs b/frontend/rust-lib/flowy-grid/src/services/grid_editor.rs index b2fe28665e..ccad53bf5c 100644 --- a/frontend/rust-lib/flowy-grid/src/services/grid_editor.rs +++ b/frontend/rust-lib/flowy-grid/src/services/grid_editor.rs @@ -179,23 +179,20 @@ impl GridRevisionEditor { None => Err(ErrorCode::FieldDoesNotExist.into()), Some(field_type) => { let _ = self.update_field_rev(params, field_type).await?; - match self.view_manager.did_update_field(&field_id).await { - Ok(_) => {} - Err(e) => tracing::error!("View manager update field failed: {:?}", e), - } let _ = self.notify_did_update_grid_field(&field_id).await?; Ok(()) } } } + // Replaces the field revision with new field revision. pub async fn replace_field(&self, field_rev: Arc) -> FlowyResult<()> { let field_id = field_rev.id.clone(); let _ = self .modify(|grid_pad| Ok(grid_pad.replace_field_rev(field_rev.clone())?)) .await?; - match self.view_manager.did_update_field(&field_rev.id).await { + match self.view_manager.did_update_field(&field_rev.id, false).await { Ok(_) => {} Err(e) => tracing::error!("View manager update field failed: {:?}", e), } @@ -279,6 +276,7 @@ impl GridRevisionEditor { } async fn update_field_rev(&self, params: FieldChangesetParams, field_type: FieldType) -> FlowyResult<()> { + let mut is_type_option_changed = false; let _ = self .modify(|grid| { let deserializer = TypeOptionJsonDeserializer(field_type); @@ -319,6 +317,7 @@ impl GridRevisionEditor { Ok(json_str) => { let field_type = field.ty; field.insert_type_option_str(&field_type, json_str); + is_type_option_changed = true; is_changed = Some(()) } Err(err) => { @@ -333,7 +332,11 @@ impl GridRevisionEditor { }) .await?; - match self.view_manager.did_update_field(¶ms.field_id).await { + match self + .view_manager + .did_update_field(¶ms.field_id, is_type_option_changed) + .await + { Ok(_) => {} Err(e) => tracing::error!("View manager update field failed: {:?}", e), } @@ -537,7 +540,7 @@ impl GridRevisionEditor { self.view_manager.get_filters().await } - pub async fn create_group(&self, params: InsertGroupParams) -> FlowyResult<()> { + pub async fn insert_group(&self, params: InsertGroupParams) -> FlowyResult<()> { self.view_manager.insert_or_update_group(params).await } 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 db7aa53ee2..d548cbc9cb 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 @@ -175,9 +175,15 @@ impl GridViewManager { Ok(()) } - pub(crate) async fn did_update_field(&self, field_id: &str) -> FlowyResult<()> { + #[tracing::instrument(level = "trace", skip(self), err)] + pub(crate) async fn did_update_field(&self, field_id: &str, is_type_option_changed: bool) -> FlowyResult<()> { let view_editor = self.get_default_view_editor().await?; - let _ = view_editor.did_update_field(field_id).await?; + if is_type_option_changed { + let _ = view_editor.group_by_field(field_id).await?; + } else { + let _ = view_editor.did_update_field(field_id).await?; + } + Ok(()) } 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 5926f8c022..a205420498 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 @@ -382,11 +382,8 @@ async fn group_insert_single_select_option_test() { AssertGroupCount(5), ]; test.run_scripts(scripts).await; - - // the group at index 4 is the default_group, so the new insert group will be the - // index 3. - let group_3 = test.group_at_index(3).await; - assert_eq!(group_3.desc, new_option_name); + let new_group = test.group_at_index(0).await; + assert_eq!(new_group.desc, new_option_name); } #[tokio::test]