refactor: order object position and field service (#4118)

* refactor: create OrderObjectPositionPB

* refactor: use ObjectOrderPosition for creating rows

* refactor: field backend service

* refactor: use field_id for reordering fields

* style: reorder dependencies

* fix: changes on tauri

* feat: insert row above

* chore: don't pass group_id while duplicating a row

frontend/rust-lib/flowy-database2/src/entities/database_entities.rs

@@ -84,21 +84,17 @@ pub struct MoveFieldPayloadPB {
   pub view_id: String,
 
   #[pb(index = 2)]
-  pub field_id: String,
+  pub from_field_id: String,
 
   #[pb(index = 3)]
-  pub from_index: i32,
-
-  #[pb(index = 4)]
-  pub to_index: i32,
+  pub to_field_id: String,
 }
 
 #[derive(Clone)]
 pub struct MoveFieldParams {
   pub view_id: String,
-  pub field_id: String,
-  pub from_index: i32,
-  pub to_index: i32,
+  pub from_field_id: String,
+  pub to_field_id: String,
 }
 
 impl TryInto<MoveFieldParams> for MoveFieldPayloadPB {
@@ -106,12 +102,13 @@ impl TryInto<MoveFieldParams> for MoveFieldPayloadPB {
 
   fn try_into(self) -> Result<MoveFieldParams, Self::Error> {
     let view_id = NotEmptyStr::parse(self.view_id).map_err(|_| ErrorCode::DatabaseViewIdIsEmpty)?;
-    let item_id = NotEmptyStr::parse(self.field_id).map_err(|_| ErrorCode::InvalidParams)?;
+    let from_field_id =
+      NotEmptyStr::parse(self.from_field_id).map_err(|_| ErrorCode::InvalidParams)?;
+    let to_field_id = NotEmptyStr::parse(self.to_field_id).map_err(|_| ErrorCode::InvalidParams)?;
     Ok(MoveFieldParams {
       view_id: view_id.0,
-      field_id: item_id.0,
-      from_index: self.from_index,
-      to_index: self.to_index,
+      from_field_id: from_field_id.0,
+      to_field_id: to_field_id.0,
     })
   }
 }

frontend/rust-lib/flowy-database2/src/entities/field_entities.rs

@@ -12,6 +12,7 @@ use flowy_derive::{ProtoBuf, ProtoBuf_Enum};
 use flowy_error::ErrorCode;
 
 use crate::entities::parser::NotEmptyStr;
+use crate::entities::position_entities::OrderObjectPositionPB;
 use crate::impl_into_field_type;
 
 /// [FieldPB] defines a Field's attributes. Such as the name, field_type, and width. etc.
@@ -164,20 +165,7 @@ pub struct CreateFieldPayloadPB {
   pub type_option_data: Option<Vec<u8>>,
 
   #[pb(index = 5)]
-  pub field_position: CreateFieldPosition,
-
-  #[pb(index = 6, one_of)]
-  pub target_field_id: Option<String>,
-}
-
-#[derive(Debug, Default, ProtoBuf_Enum)]
-#[repr(u8)]
-pub enum CreateFieldPosition {
-  #[default]
-  End = 0,
-  Start = 1,
-  Before = 2,
-  After = 3,
+  pub field_position: OrderObjectPositionPB,
 }
 
 #[derive(Clone)]
@@ -204,34 +192,7 @@ impl TryInto<CreateFieldParams> for CreateFieldPayloadPB {
       None => None,
     };
 
-    let position = match &self.field_position {
-      CreateFieldPosition::Start => {
-        if self.target_field_id.is_some() {
-          return Err(ErrorCode::InvalidParams);
-        }
-        OrderObjectPosition::Start
-      },
-      CreateFieldPosition::End => {
-        if self.target_field_id.is_some() {
-          return Err(ErrorCode::InvalidParams);
-        }
-        OrderObjectPosition::End
-      },
-      CreateFieldPosition::Before => {
-        let field_id = self.target_field_id.ok_or(ErrorCode::InvalidParams)?;
-        let field_id = NotEmptyStr::parse(field_id)
-          .map_err(|_| ErrorCode::InvalidParams)?
-          .0;
-        OrderObjectPosition::Before(field_id)
-      },
-      CreateFieldPosition::After => {
-        let field_id = self.target_field_id.ok_or(ErrorCode::InvalidParams)?;
-        let field_id = NotEmptyStr::parse(field_id)
-          .map_err(|_| ErrorCode::InvalidParams)?
-          .0;
-        OrderObjectPosition::After(field_id)
-      },
-    };
+    let position = self.field_position.try_into()?;
 
     Ok(CreateFieldParams {
       view_id: view_id.0,

frontend/rust-lib/flowy-database2/src/entities/mod.rs

@@ -7,15 +7,16 @@ mod field_settings_entities;
 pub mod filter_entities;
 mod group_entities;
 pub mod parser;
+mod position_entities;
 mod row_entities;
 pub mod setting_entities;
+mod share_entities;
 mod sort_entities;
+mod type_option_entities;
 mod view_entities;
 
 #[macro_use]
 mod macros;
-mod share_entities;
-mod type_option_entities;
 
 pub use board_entities::*;
 pub use calendar_entities::*;
@@ -25,6 +26,7 @@ pub use field_entities::*;
 pub use field_settings_entities::*;
 pub use filter_entities::*;
 pub use group_entities::*;
+pub use position_entities::*;
 pub use row_entities::*;
 pub use setting_entities::*;
 pub use share_entities::*;

frontend/rust-lib/flowy-database2/src/entities/position_entities.rs

@@ -0,0 +1,89 @@
+use collab_database::views::OrderObjectPosition;
+use flowy_derive::{ProtoBuf, ProtoBuf_Enum};
+use flowy_error::ErrorCode;
+
+use crate::entities::parser::NotEmptyStr;
+
+#[derive(Debug, Default, ProtoBuf)]
+pub struct OrderObjectPositionPB {
+  #[pb(index = 1)]
+  pub position: OrderObjectPositionTypePB,
+
+  #[pb(index = 2, one_of)]
+  pub object_id: Option<String>,
+}
+
+impl OrderObjectPositionPB {
+  pub fn start() -> Self {
+    Self {
+      position: OrderObjectPositionTypePB::Start,
+      object_id: None,
+    }
+  }
+
+  pub fn end() -> Self {
+    Self {
+      position: OrderObjectPositionTypePB::End,
+      object_id: None,
+    }
+  }
+
+  pub fn before(object_id: String) -> Self {
+    Self {
+      position: OrderObjectPositionTypePB::Before,
+      object_id: Some(object_id),
+    }
+  }
+
+  pub fn after(object_id: String) -> Self {
+    Self {
+      position: OrderObjectPositionTypePB::After,
+      object_id: Some(object_id),
+    }
+  }
+}
+
+#[derive(Debug, Default, ProtoBuf_Enum)]
+#[repr(u8)]
+pub enum OrderObjectPositionTypePB {
+  #[default]
+  End = 0,
+  Start = 1,
+  Before = 2,
+  After = 3,
+}
+
+impl TryFrom<OrderObjectPositionPB> for OrderObjectPosition {
+  type Error = ErrorCode;
+
+  fn try_from(value: OrderObjectPositionPB) -> Result<Self, Self::Error> {
+    match value.position {
+      OrderObjectPositionTypePB::Start => {
+        if value.object_id.is_some() {
+          return Err(ErrorCode::InvalidParams);
+        }
+        Ok(OrderObjectPosition::Start)
+      },
+      OrderObjectPositionTypePB::End => {
+        if value.object_id.is_some() {
+          return Err(ErrorCode::InvalidParams);
+        }
+        Ok(OrderObjectPosition::End)
+      },
+      OrderObjectPositionTypePB::Before => {
+        let field_id = value.object_id.ok_or(ErrorCode::InvalidParams)?;
+        let field_id = NotEmptyStr::parse(field_id)
+          .map_err(|_| ErrorCode::InvalidParams)?
+          .0;
+        Ok(OrderObjectPosition::Before(field_id))
+      },
+      OrderObjectPositionTypePB::After => {
+        let field_id = value.object_id.ok_or(ErrorCode::InvalidParams)?;
+        let field_id = NotEmptyStr::parse(field_id)
+          .map_err(|_| ErrorCode::InvalidParams)?
+          .0;
+        Ok(OrderObjectPosition::After(field_id))
+      },
+    }
+  }
+}

frontend/rust-lib/flowy-database2/src/entities/row_entities.rs

@@ -1,12 +1,13 @@
 use std::collections::HashMap;
 
 use collab_database::rows::{Row, RowDetail, RowId};
-use collab_database::views::RowOrder;
+use collab_database::views::{OrderObjectPosition, RowOrder};
 
 use flowy_derive::ProtoBuf;
 use flowy_error::ErrorCode;
 
 use crate::entities::parser::NotEmptyStr;
+use crate::entities::position_entities::OrderObjectPositionPB;
 use crate::services::database::{InsertedRow, UpdatedRow};
 
 /// [RowPB] Describes a row. Has the id of the parent Block. Has the metadata of the row.
@@ -339,8 +340,8 @@ pub struct CreateRowPayloadPB {
   #[pb(index = 1)]
   pub view_id: String,
 
-  #[pb(index = 2, one_of)]
-  pub start_row_id: Option<String>,
+  #[pb(index = 2)]
+  pub row_position: OrderObjectPositionPB,
 
   #[pb(index = 3, one_of)]
   pub group_id: Option<String>,
@@ -358,7 +359,7 @@ pub struct RowDataPB {
 #[derive(Default)]
 pub struct CreateRowParams {
   pub view_id: String,
-  pub start_row_id: Option<RowId>,
+  pub row_position: OrderObjectPosition,
   pub group_id: Option<String>,
   pub cell_data_by_field_id: Option<HashMap<String, String>>,
 }
@@ -368,10 +369,10 @@ impl TryInto<CreateRowParams> for CreateRowPayloadPB {
 
   fn try_into(self) -> Result<CreateRowParams, Self::Error> {
     let view_id = NotEmptyStr::parse(self.view_id).map_err(|_| ErrorCode::ViewIdIsInvalid)?;
-    let start_row_id = self.start_row_id.map(RowId::from);
+    let position = self.row_position.try_into()?;
     Ok(CreateRowParams {
       view_id: view_id.0,
-      start_row_id,
+      row_position: position,
       group_id: self.group_id,
       cell_data_by_field_id: self.data.map(|data| data.cell_data_by_field_id),
     })

frontend/rust-lib/flowy-database2/src/event_handler.rs

@@ -2,7 +2,6 @@ use std::sync::{Arc, Weak};
 
 use collab_database::database::gen_row_id;
 use collab_database::rows::RowId;
-use collab_database::views::OrderObjectPosition;
 use tokio::sync::oneshot;
 
 use flowy_error::{FlowyError, FlowyResult};
@@ -345,14 +344,7 @@ pub(crate) async fn move_field_handler(
   let manager = upgrade_manager(manager)?;
   let params: MoveFieldParams = data.into_inner().try_into()?;
   let database_editor = manager.get_database_with_view_id(&params.view_id).await?;
-  database_editor
-    .move_field(
-      &params.view_id,
-      &params.field_id,
-      params.from_index,
-      params.to_index,
-    )
-    .await?;
+  database_editor.move_field(params).await?;
   Ok(())
 }
 
@@ -416,7 +408,7 @@ pub(crate) async fn duplicate_row_handler(
   let params: RowIdParams = data.into_inner().try_into()?;
   let database_editor = manager.get_database_with_view_id(&params.view_id).await?;
   database_editor
-    .duplicate_row(&params.view_id, params.group_id, &params.row_id)
+    .duplicate_row(&params.view_id, &params.row_id)
     .await;
   Ok(())
 }
@@ -431,7 +423,7 @@ pub(crate) async fn move_row_handler(
   let database_editor = manager.get_database_with_view_id(&params.view_id).await?;
   database_editor
     .move_row(&params.view_id, params.from_row_id, params.to_row_id)
-    .await;
+    .await?;
   Ok(())
 }
 
@@ -448,16 +440,12 @@ pub(crate) async fn create_row_handler(
     CellBuilder::with_cells(params.cell_data_by_field_id.unwrap_or_default(), &fields).build();
   let view_id = params.view_id;
   let group_id = params.group_id;
-  let position = match params.start_row_id {
-    Some(row_id) => OrderObjectPosition::After(row_id.into()),
-    None => OrderObjectPosition::Start,
-  };
   let params = collab_database::rows::CreateRowParams {
     id: gen_row_id(),
     cells,
     height: 60,
     visibility: true,
-    row_position: position,
+    row_position: params.row_position,
     timestamp: timestamp(),
   };
   match database_editor

frontend/rust-lib/flowy-database2/src/event_map.rs

@@ -168,8 +168,9 @@ pub enum DatabaseEvent {
   #[event(input = "DuplicateFieldPayloadPB")]
   DuplicateField = 21,
 
-  /// [MoveItem] event is used to move an item. For the moment, Item has two types defined in
-  /// [MoveItemTypePB].
+  /// [MoveFieldPB] event is used to reorder a field in a view. The
+  /// [MoveFieldPayloadPB] contains the `field_id` of the moved field and its
+  /// new position.
   #[event(input = "MoveFieldPayloadPB")]
   MoveField = 22,
 

frontend/rust-lib/flowy-database2/src/services/database/database_editor.rs

@@ -402,37 +402,47 @@ impl DatabaseEditor {
   }
 
   // consider returning a result. But most of the time, it should be fine to just ignore the error.
-  pub async fn duplicate_row(&self, view_id: &str, group_id: Option<String>, row_id: &RowId) {
+  pub async fn duplicate_row(&self, view_id: &str, row_id: &RowId) {
     let params = self.database.lock().duplicate_row(row_id);
     match params {
-      None => {
-        warn!("Failed to duplicate row: {}", row_id);
-      },
+      None => warn!("Failed to duplicate row: {}", row_id),
       Some(params) => {
-        let _ = self.create_row(view_id, group_id, params).await;
+        let _ = self.create_row(view_id, None, params).await;
       },
     }
   }
 
-  pub async fn move_row(&self, view_id: &str, from: RowId, to: RowId) {
+  pub async fn move_row(
+    &self,
+    view_id: &str,
+    from_row_id: RowId,
+    to_row_id: RowId,
+  ) -> FlowyResult<()> {
     let database = self.database.lock();
-    if let (Some(row_detail), Some(from_index), Some(to_index)) = (
-      database.get_row_detail(&from),
-      database.index_of_row(view_id, &from),
-      database.index_of_row(view_id, &to),
-    ) {
-      database.views.update_database_view(view_id, |view| {
-        view.move_row_order(from_index as u32, to_index as u32);
-      });
-      drop(database);
 
-      let delete_row_id = from.into_inner();
-      let insert_row = InsertedRowPB::new(RowMetaPB::from(row_detail)).with_index(to_index as i32);
+    let row_detail = database.get_row_detail(&from_row_id).ok_or_else(|| {
+      let msg = format!("Cannot find row {}", from_row_id);
+      FlowyError::internal().with_context(msg)
+    })?;
+
+    database.views.update_database_view(view_id, |view| {
+      view.move_row_order(&from_row_id, &to_row_id);
+    });
+
+    let new_index = database.index_of_row(view_id, &from_row_id);
+    drop(database);
+
+    if let Some(index) = new_index {
+      let delete_row_id = from_row_id.into_inner();
+      let insert_row = InsertedRowPB::new(RowMetaPB::from(row_detail)).with_index(index as i32);
       let changes = RowsChangePB::from_move(vec![delete_row_id], vec![insert_row]);
+
       send_notification(view_id, DatabaseNotification::DidUpdateViewRows)
         .payload(changes)
         .send();
     }
+
+    Ok(())
   }
 
   pub async fn create_row(
@@ -504,28 +514,34 @@ impl DatabaseEditor {
     )
   }
 
-  pub async fn move_field(
-    &self,
-    view_id: &str,
-    field_id: &str,
-    from: i32,
-    to: i32,
-  ) -> FlowyResult<()> {
-    let (database_id, field) = {
+  pub async fn move_field(&self, params: MoveFieldParams) -> FlowyResult<()> {
+    let (field, new_index) = {
       let database = self.database.lock();
-      database.views.update_database_view(view_id, |view_update| {
-        view_update.move_field_order(from as u32, to as u32);
-      });
-      let field = database.fields.get_field(field_id);
-      let database_id = database.get_database_id();
-      (database_id, field)
+
+      let field = database
+        .fields
+        .get_field(&params.from_field_id)
+        .ok_or_else(|| {
+          let msg = format!("Field with id: {} not found", &params.from_field_id);
+          FlowyError::internal().with_context(msg)
+        })?;
+
+      database
+        .views
+        .update_database_view(&params.view_id, |view_update| {
+          view_update.move_field_order(&params.from_field_id, &params.to_field_id);
+        });
+
+      let new_index = database.index_of_field(&params.view_id, &params.from_field_id);
+
+      (field, new_index)
     };
 
-    if let Some(field) = field {
-      let delete_field = FieldIdPB::from(field_id);
-      let insert_field = IndexFieldPB::from_field(field, to as usize);
+    if let Some(index) = new_index {
+      let delete_field = FieldIdPB::from(params.from_field_id);
+      let insert_field = IndexFieldPB::from_field(field, index);
       let notified_changeset = DatabaseFieldChangesetPB {
-        view_id: database_id,
+        view_id: params.view_id,
         inserted_fields: vec![insert_field],
         deleted_fields: vec![delete_field],
         updated_fields: vec![],
@@ -533,6 +549,7 @@ impl DatabaseEditor {
 
       self.notify_did_update_database(notified_changeset).await?;
     }
+
     Ok(())
   }
 
@@ -955,7 +972,7 @@ impl DatabaseEditor {
             .map(|row_detail| row_detail.row.id.clone())
         };
         if let Some(row_id) = to_row.clone() {
-          self.move_row(view_id, from_row.clone(), row_id).await;
+          self.move_row(view_id, from_row.clone(), row_id).await?;
         }
 
         if from_group == to_group {