fix: save group rev without apply change

Author: appflowy

frontend/rust-lib/flowy-error/src/errors.rs

@@ -66,6 +66,7 @@ impl FlowyError {
     static_flowy_error!(user_not_exist, ErrorCode::UserNotExist);
     static_flowy_error!(text_too_long, ErrorCode::TextTooLong);
     static_flowy_error!(invalid_data, ErrorCode::InvalidData);
+    static_flowy_error!(out_of_bounds, ErrorCode::OutOfBounds);
 }
 
 impl std::convert::From<ErrorCode> for FlowyError {

frontend/rust-lib/flowy-grid/src/entities/group_entities/group_changeset.rs

@@ -109,10 +109,19 @@ pub struct GroupViewChangesetPB {
     pub view_id: String,
 
     #[pb(index = 2)]
-    pub inserted_groups: Vec<GroupPB>,
+    pub inserted_groups: Vec<InsertedGroupPB>,
 
     #[pb(index = 3)]
     pub deleted_groups: Vec<String>,
 }
 
 impl GroupViewChangesetPB {}
+
+#[derive(Debug, Default, ProtoBuf)]
+pub struct InsertedGroupPB {
+    #[pb(index = 1)]
+    pub group: GroupPB,
+
+    #[pb(index = 2)]
+    pub index: i32,
+}

frontend/rust-lib/flowy-grid/src/services/grid_view_editor.rs

@@ -1,14 +1,12 @@
 use crate::dart_notification::{send_dart_notification, GridNotification};
 use crate::entities::{
     CreateFilterParams, CreateRowParams, DeleteFilterParams, GridFilterConfiguration, GridLayout, GridLayoutPB,
-    GridSettingChangesetParams, GridSettingPB, GroupPB, GroupRowsChangesetPB, GroupViewChangesetPB, InsertedRowPB,
+    GridSettingPB, GroupPB, GroupRowsChangesetPB, GroupViewChangesetPB, InsertedGroupPB, InsertedRowPB,
     MoveGroupParams, RepeatedGridConfigurationFilterPB, RepeatedGridGroupConfigurationPB, RowPB,
 };
 use crate::services::grid_editor_task::GridServiceTaskScheduler;
 use crate::services::grid_view_manager::{GridViewFieldDelegate, GridViewRowDelegate};
-use crate::services::group::{
-    default_group_configuration, GroupConfigurationReader, GroupConfigurationWriter, GroupService,
-};
+use crate::services::group::{GroupConfigurationReader, GroupConfigurationWriter, GroupService};
 use flowy_error::{FlowyError, FlowyResult};
 use flowy_grid_data_model::revision::{
     gen_grid_filter_id, FieldRevision, FieldTypeRevision, FilterConfigurationRevision, GroupConfigurationRevision,
@@ -19,6 +17,7 @@ use flowy_sync::client_grid::{GridViewRevisionChangeset, GridViewRevisionPad};
 use flowy_sync::entities::revision::Revision;
 use lib_infra::future::{wrap_future, AFFuture, FutureResult};
 use std::collections::HashMap;
+
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Arc;
 use tokio::sync::RwLock;
@@ -37,6 +36,7 @@ pub struct GridViewRevisionEditor {
 }
 
 impl GridViewRevisionEditor {
+    #[tracing::instrument(level = "trace", skip_all, err)]
     pub(crate) async fn new(
         user_id: &str,
         token: &str,
@@ -109,7 +109,7 @@ impl GridViewRevisionEditor {
         // Send the group notification if the current view has groups;
         if let Some(changesets) = self
             .group_service
-            .write()
+            .read()
             .await
             .did_delete_row(row_rev, |field_id| self.field_delegate.get_field_rev(&field_id))
             .await
@@ -123,7 +123,7 @@ impl GridViewRevisionEditor {
     pub(crate) async fn did_update_row(&self, row_rev: &RowRevision) {
         if let Some(changesets) = self
             .group_service
-            .write()
+            .read()
             .await
             .did_update_row(row_rev, |field_id| self.field_delegate.get_field_rev(&field_id))
             .await
@@ -142,7 +142,7 @@ impl GridViewRevisionEditor {
     ) {
         if let Some(changesets) = self
             .group_service
-            .write()
+            .read()
             .await
             .did_move_row(row_rev, row_changeset, upper_row_id, |field_id| {
                 self.field_delegate.get_field_rev(&field_id)
@@ -156,11 +156,13 @@ impl GridViewRevisionEditor {
         }
     }
 
+    #[tracing::instrument(level = "trace", skip(self))]
     pub(crate) async fn load_groups(&self) -> FlowyResult<Vec<GroupPB>> {
         let groups = if !self.did_load_group.load(Ordering::SeqCst) {
             self.did_load_group.store(true, Ordering::SeqCst);
             let field_revs = self.field_delegate.get_field_revs().await;
             let row_revs = self.row_delegate.gv_row_revs().await;
+
             match self
                 .group_service
                 .write()
@@ -174,11 +176,37 @@ impl GridViewRevisionEditor {
         } else {
             self.group_service.read().await.groups().await
         };
+
+        tracing::trace!("Number of groups: {}", groups.len());
         Ok(groups.into_iter().map(GroupPB::from).collect())
     }
 
     pub(crate) async fn move_group(&self, params: MoveGroupParams) -> FlowyResult<()> {
-        todo!()
+        let _ = self
+            .group_service
+            .read()
+            .await
+            .move_group(&params.from_group_id, &params.to_group_id)
+            .await?;
+
+        match self.group_service.read().await.get_group(&params.from_group_id).await {
+            None => {}
+            Some((index, group)) => {
+                let inserted_group = InsertedGroupPB {
+                    group: GroupPB::from(group),
+                    index: index as i32,
+                };
+
+                let changeset = GroupViewChangesetPB {
+                    view_id: "".to_string(),
+                    inserted_groups: vec![inserted_group],
+                    deleted_groups: vec![params.from_group_id.clone()],
+                };
+
+                self.notify_did_update_view(changeset).await;
+            }
+        }
+        Ok(())
     }
 
     pub(crate) async fn get_setting(&self) -> GridSettingPB {
@@ -291,17 +319,19 @@ impl RevisionObjectBuilder for GridViewRevisionPadBuilder {
 struct GroupConfigurationReaderImpl(Arc<RwLock<GridViewRevisionPad>>);
 
 impl GroupConfigurationReader for GroupConfigurationReaderImpl {
-    fn get_group_configuration(&self, field_rev: Arc<FieldRevision>) -> AFFuture<Arc<GroupConfigurationRevision>> {
+    fn get_group_configuration(
+        &self,
+        field_rev: Arc<FieldRevision>,
+    ) -> AFFuture<Option<Arc<GroupConfigurationRevision>>> {
         let view_pad = self.0.clone();
         wrap_future(async move {
-            let view_pad = view_pad.read().await;
-            let configurations = view_pad.get_groups(&field_rev.id, &field_rev.ty);
-            match configurations {
-                None => {
-                    let default_configuration = default_group_configuration(&field_rev);
-                    Arc::new(default_configuration)
-                }
-                Some(configuration) => configuration,
+            let mut groups = view_pad.read().await.groups.get_objects(&field_rev.id, &field_rev.ty)?;
+
+            if groups.is_empty() {
+                None
+            } else {
+                debug_assert_eq!(groups.len(), 1);
+                Some(groups.pop().unwrap())
             }
         })
     }
@@ -328,17 +358,25 @@ impl GroupConfigurationWriter for GroupConfigurationWriterImpl {
         let field_id = field_id.to_owned();
 
         wrap_future(async move {
-            match view_pad.write().await.get_mut_group(
-                &field_id,
-                &field_type,
-                &configuration_id,
-                |group_configuration| {
-                    group_configuration.content = content;
-                },
-            )? {
-                None => Ok(()),
-                Some(changeset) => apply_change(&user_id, rev_manager, changeset).await,
+            let is_contained = view_pad.read().await.contains_group(&field_id, &field_type);
+            let changeset = if is_contained {
+                view_pad.write().await.with_mut_group(
+                    &field_id,
+                    &field_type,
+                    &configuration_id,
+                    |group_configuration| {
+                        group_configuration.content = content;
+                    },
+                )?
+            } else {
+                let group_rev = GroupConfigurationRevision::new(field_id.clone(), field_type, content)?;
+                view_pad.write().await.insert_group(&field_id, &field_type, group_rev)?
+            };
+
+            if let Some(changeset) = changeset {
+                let _ = apply_change(&user_id, rev_manager, changeset).await?;
             }
+            Ok(())
         })
     }
 }
@@ -371,3 +409,17 @@ pub fn make_grid_setting(view_pad: &GridViewRevisionPad, field_revs: &[Arc<Field
         group_configuration_by_field_id: groups_by_field_id,
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use lib_ot::core::TextDelta;
+
+    #[test]
+    fn test() {
+        let s1 = r#"[{"insert":"{\"view_id\":\"fTURELffPr\",\"grid_id\":\"fTURELffPr\",\"layout\":0,\"filters\":[],\"groups\":[]}"}]"#;
+        let _delta_1 = TextDelta::from_json(s1).unwrap();
+
+        let s2 = r#"[{"retain":195},{"insert":"{\\\"group_id\\\":\\\"wD9i\\\",\\\"visible\\\":true},{\\\"group_id\\\":\\\"xZtv\\\",\\\"visible\\\":true},{\\\"group_id\\\":\\\"tFV2\\\",\\\"visible\\\":true}"},{"retain":10}]"#;
+        let _delta_2 = TextDelta::from_json(s2).unwrap();
+    }
+}

frontend/rust-lib/flowy-grid/src/services/grid_view_manager.rs

@@ -1,6 +1,6 @@
 use crate::entities::{
-    CreateFilterParams, CreateRowParams, DeleteFilterParams, GridFilterConfiguration, GridSettingChangesetParams,
-    GridSettingPB, MoveGroupParams, RepeatedGridGroupPB, RowPB,
+    CreateFilterParams, CreateRowParams, DeleteFilterParams, GridFilterConfiguration, GridSettingPB, MoveGroupParams,
+    RepeatedGridGroupPB, RowPB,
 };
 use crate::manager::GridUser;
 use crate::services::grid_editor_task::GridServiceTaskScheduler;
@@ -122,7 +122,7 @@ impl GridViewManager {
 
     pub(crate) async fn move_group(&self, params: MoveGroupParams) -> FlowyResult<()> {
         let view_editor = self.get_default_view_editor().await?;
-        let _s = view_editor.move_group(params).await?;
+        let _ = view_editor.move_group(params).await?;
         Ok(())
     }
 
@@ -175,12 +175,11 @@ async fn make_view_editor(
     row_delegate: Arc<dyn GridViewRowDelegate>,
     scheduler: Arc<dyn GridServiceTaskScheduler>,
 ) -> FlowyResult<GridViewRevisionEditor> {
-    tracing::trace!("Open view:{} editor", view_id);
-
     let rev_manager = make_grid_view_rev_manager(user, view_id).await?;
     let user_id = user.user_id()?;
     let token = user.token()?;
     let view_id = view_id.to_owned();
+
     GridViewRevisionEditor::new(
         &user_id,
         &token,
@@ -194,7 +193,6 @@ async fn make_view_editor(
 }
 
 pub async fn make_grid_view_rev_manager(user: &Arc<dyn GridUser>, view_id: &str) -> FlowyResult<RevisionManager> {
-    tracing::trace!("Open view:{} editor", view_id);
     let user_id = user.user_id()?;
     let pool = user.db_pool()?;
 

frontend/rust-lib/flowy-grid/src/services/group/configuration.rs

@@ -1,5 +1,5 @@
-use crate::services::group::Group;
-use flowy_error::FlowyResult;
+use crate::services::group::{default_group_configuration, Group};
+use flowy_error::{FlowyError, FlowyResult};
 use flowy_grid_data_model::revision::{
     FieldRevision, FieldTypeRevision, GroupConfigurationContent, GroupConfigurationRevision, GroupRecordRevision,
 };
@@ -9,7 +9,10 @@ use lib_infra::future::AFFuture;
 use std::sync::Arc;
 
 pub trait GroupConfigurationReader: Send + Sync + 'static {
-    fn get_group_configuration(&self, field_rev: Arc<FieldRevision>) -> AFFuture<Arc<GroupConfigurationRevision>>;
+    fn get_group_configuration(
+        &self,
+        field_rev: Arc<FieldRevision>,
+    ) -> AFFuture<Option<Arc<GroupConfigurationRevision>>>;
 }
 
 pub trait GroupConfigurationWriter: Send + Sync + 'static {
@@ -39,7 +42,22 @@ where
         reader: Arc<dyn GroupConfigurationReader>,
         writer: Arc<dyn GroupConfigurationWriter>,
     ) -> FlowyResult<Self> {
-        let configuration_rev = reader.get_group_configuration(field_rev.clone()).await;
+        let configuration_rev = match reader.get_group_configuration(field_rev.clone()).await {
+            None => {
+                let default_group_configuration = default_group_configuration(&field_rev);
+                writer
+                    .save_group_configuration(
+                        &field_rev.id,
+                        field_rev.ty,
+                        &default_group_configuration.id,
+                        default_group_configuration.content.clone(),
+                    )
+                    .await?;
+                Arc::new(default_group_configuration)
+            }
+            Some(configuration) => configuration,
+        };
+
         let configuration_id = configuration_rev.id.clone();
         let configuration = C::from_configuration_content(&configuration_rev.content)?;
         Ok(Self {
@@ -63,6 +81,8 @@ where
         let (group_revs, groups) = merge_groups(self.configuration.get_groups(), groups);
         self.configuration.set_groups(group_revs);
         let _ = self.save_configuration().await?;
+
+        tracing::trace!("merge new groups: {}", groups.len());
         groups.into_iter().for_each(|group| {
             self.groups_map.insert(group.id.clone(), group);
         });
@@ -97,8 +117,25 @@ where
         self.groups_map.get_mut(group_id)
     }
 
-    pub(crate) fn get_group(&mut self, group_id: &str) -> Option<&Group> {
-        self.groups_map.get(group_id)
+    pub(crate) fn move_group(&mut self, from_group_id: &str, to_group_id: &str) -> FlowyResult<()> {
+        let from_group_index = self.groups_map.get_index_of(from_group_id);
+        let to_group_index = self.groups_map.get_index_of(to_group_id);
+        match (from_group_index, to_group_index) {
+            (Some(from_index), Some(to_index)) => {
+                self.groups_map.swap_indices(from_index, to_index);
+                self.configuration.swap_group(from_group_id, to_group_id);
+                Ok(())
+            }
+            _ => Err(FlowyError::out_of_bounds()),
+        }
+    }
+
+    // Returns the index and group specified by the group_id
+    pub(crate) fn get_group(&self, group_id: &str) -> Option<(usize, &Group)> {
+        match (self.groups_map.get_index_of(group_id), self.groups_map.get(group_id)) {
+            (Some(index), Some(group)) => Some((index, group)),
+            _ => None,
+        }
     }
 
     pub async fn save_configuration(&self) -> FlowyResult<()> {
@@ -111,17 +148,17 @@ where
     }
 }
 
-impl<T> GroupConfigurationReader for Arc<T>
-where
-    T: GroupConfigurationReader,
-{
-    fn get_group_configuration(&self, field_rev: Arc<FieldRevision>) -> AFFuture<Arc<GroupConfigurationRevision>> {
-        (**self).get_group_configuration(field_rev)
-    }
-}
+// impl<T> GroupConfigurationReader for Arc<T>
+// where
+//     T: GroupConfigurationReader,
+// {
+//     fn get_group_configuration(&self, field_rev: Arc<FieldRevision>) -> AFFuture<Arc<GroupConfigurationRevision>> {
+//         (**self).get_group_configuration(field_rev)
+//     }
+// }
 
 fn merge_groups(old_group_revs: &[GroupRecordRevision], groups: Vec<Group>) -> (Vec<GroupRecordRevision>, Vec<Group>) {
-    // tracing::trace!("Merge group: old: {}, new: {}", old_group.len(), groups.len());
+    tracing::trace!("Merge group: old: {}, new: {}", old_group_revs.len(), groups.len());
     if old_group_revs.is_empty() {
         let new_groups = groups
             .iter()