From fa3f9c21fc97f2e2ed31f07f5252b051da2e60e7 Mon Sep 17 00:00:00 2001 From: appflowy Date: Fri, 7 Oct 2022 14:42:40 +0800 Subject: [PATCH] fix: state of AppFlowyBoard is wrong after reordering the group --- .../plugins/board/application/board_bloc.dart | 6 +-- .../application/board_data_controller.dart | 8 ++-- .../appflowy_board/lib/src/utils/log.dart | 4 ++ .../appflowy_board/lib/src/widgets/board.dart | 43 +++++++++--------- .../lib/src/widgets/board_data.dart | 10 ++++- .../lib/src/widgets/board_group/group.dart | 4 +- .../src/widgets/board_group/group_data.dart | 22 ++++++++-- .../reorder_phantom/phantom_controller.dart | 19 ++++---- .../flowy-grid/src/services/grid_editor.rs | 2 +- .../src/services/grid_view_editor.rs | 26 +++++------ .../src/services/grid_view_manager.rs | 24 +++++----- .../src/services/group/configuration.rs | 10 ++++- .../select_option_controller/util.rs | 7 +-- .../flowy-grid/tests/grid/group_test/test.rs | 44 +++++++++++++++++++ 14 files changed, 154 insertions(+), 75 deletions(-) diff --git a/frontend/app_flowy/lib/plugins/board/application/board_bloc.dart b/frontend/app_flowy/lib/plugins/board/application/board_bloc.dart index a02eb0dd4d..ce72413425 100644 --- a/frontend/app_flowy/lib/plugins/board/application/board_bloc.dart +++ b/frontend/app_flowy/lib/plugins/board/application/board_bloc.dart @@ -166,11 +166,11 @@ class BoardBloc extends Bloc { } } - void _moveGroup(String fromColumnId, String toColumnId) { + void _moveGroup(String fromGroupId, String toGroupId) { _rowService .moveGroup( - fromGroupId: fromColumnId, - toGroupId: toColumnId, + fromGroupId: fromGroupId, + toGroupId: toGroupId, ) .then((result) { result.fold((l) => null, (r) => add(BoardEvent.didReceiveError(r))); diff --git a/frontend/app_flowy/lib/plugins/board/application/board_data_controller.dart b/frontend/app_flowy/lib/plugins/board/application/board_data_controller.dart index 923bb4ef8c..641239e767 100644 --- a/frontend/app_flowy/lib/plugins/board/application/board_data_controller.dart +++ b/frontend/app_flowy/lib/plugins/board/application/board_data_controller.dart @@ -87,13 +87,13 @@ class BoardDataController { onUpdatedGroup.call(changeset.updateGroups); } - if (changeset.insertedGroups.isNotEmpty) { - onInsertedGroup.call(changeset.insertedGroups); - } - if (changeset.deletedGroups.isNotEmpty) { onDeletedGroup.call(changeset.deletedGroups); } + + if (changeset.insertedGroups.isNotEmpty) { + onInsertedGroup.call(changeset.insertedGroups); + } }, (e) => _onError?.call(e), ); diff --git a/frontend/app_flowy/packages/appflowy_board/lib/src/utils/log.dart b/frontend/app_flowy/packages/appflowy_board/lib/src/utils/log.dart index 0b8d436d81..b73d28eed2 100644 --- a/frontend/app_flowy/packages/appflowy_board/lib/src/utils/log.dart +++ b/frontend/app_flowy/packages/appflowy_board/lib/src/utils/log.dart @@ -32,4 +32,8 @@ class Log { 'AppFlowyBoard: ❗️[Trace] - ${DateTime.now().second}=> $message'); } } + + static void error(String? message) { + debugPrint('AppFlowyBoard: ❌[Error] - ${DateTime.now().second}=> $message'); + } } diff --git a/frontend/app_flowy/packages/appflowy_board/lib/src/widgets/board.dart b/frontend/app_flowy/packages/appflowy_board/lib/src/widgets/board.dart index 824e0e4a79..a295b575cd 100644 --- a/frontend/app_flowy/packages/appflowy_board/lib/src/widgets/board.dart +++ b/frontend/app_flowy/packages/appflowy_board/lib/src/widgets/board.dart @@ -11,11 +11,11 @@ import 'reorder_phantom/phantom_controller.dart'; import '../rendering/board_overlay.dart'; class AppFlowyBoardScrollController { - AppFlowyBoardState? _groupState; + AppFlowyBoardState? _boardState; void scrollToBottom(String groupId, {void Function(BuildContext)? completed}) { - _groupState?.reorderFlexActionMap[groupId]?.scrollToBottom(completed); + _boardState?.reorderFlexActionMap[groupId]?.scrollToBottom(completed); } } @@ -92,11 +92,7 @@ class AppFlowyBoard extends StatelessWidget { /// final AppFlowyBoardScrollController? boardScrollController; - final AppFlowyBoardState _groupState = AppFlowyBoardState(); - - late final BoardPhantomController _phantomController; - - AppFlowyBoard({ + const AppFlowyBoard({ required this.controller, required this.cardBuilder, this.background, @@ -107,12 +103,7 @@ class AppFlowyBoard extends StatelessWidget { this.groupConstraints = const BoxConstraints(maxWidth: 200), this.config = const AppFlowyBoardConfig(), Key? key, - }) : super(key: key) { - _phantomController = BoardPhantomController( - delegate: controller, - groupsState: _groupState, - ); - } + }) : super(key: key); @override Widget build(BuildContext context) { @@ -120,8 +111,14 @@ class AppFlowyBoard extends StatelessWidget { value: controller, child: Consumer( builder: (context, notifier, child) { + final boardState = AppFlowyBoardState(); + BoardPhantomController phantomController = BoardPhantomController( + delegate: controller, + groupsState: boardState, + ); + if (boardScrollController != null) { - boardScrollController!._groupState = _groupState; + boardScrollController!._boardState = boardState; } return _AppFlowyBoardContent( @@ -129,14 +126,14 @@ class AppFlowyBoard extends StatelessWidget { dataController: controller, scrollController: scrollController, scrollManager: boardScrollController, - groupState: _groupState, + boardState: boardState, background: background, - delegate: _phantomController, + delegate: phantomController, groupConstraints: groupConstraints, cardBuilder: cardBuilder, footerBuilder: footerBuilder, headerBuilder: headerBuilder, - phantomController: _phantomController, + phantomController: phantomController, onReorder: controller.moveGroup, ); }, @@ -154,7 +151,7 @@ class _AppFlowyBoardContent extends StatefulWidget { final ReorderFlexConfig reorderFlexConfig; final BoxConstraints groupConstraints; final AppFlowyBoardScrollController? scrollManager; - final AppFlowyBoardState groupState; + final AppFlowyBoardState boardState; final AppFlowyBoardCardBuilder cardBuilder; final AppFlowyBoardHeaderBuilder? headerBuilder; final AppFlowyBoardFooterBuilder? footerBuilder; @@ -167,7 +164,7 @@ class _AppFlowyBoardContent extends StatefulWidget { required this.delegate, required this.dataController, required this.scrollManager, - required this.groupState, + required this.boardState, this.scrollController, this.background, required this.groupConstraints, @@ -199,7 +196,7 @@ class _AppFlowyBoardContentState extends State<_AppFlowyBoardContent> { reorderFlexId: widget.dataController.identifier, acceptedReorderFlexId: widget.dataController.groupIds, delegate: widget.delegate, - columnsState: widget.groupState, + columnsState: widget.boardState, ); final reorderFlex = ReorderFlex( @@ -253,7 +250,7 @@ class _AppFlowyBoardContentState extends State<_AppFlowyBoardContent> { ); final reorderFlexAction = ReorderFlexActionImpl(); - widget.groupState.reorderFlexActionMap[columnData.id] = + widget.boardState.reorderFlexActionMap[columnData.id] = reorderFlexAction; return ChangeNotifierProvider.value( @@ -274,8 +271,8 @@ class _AppFlowyBoardContentState extends State<_AppFlowyBoardContent> { onReorder: widget.dataController.moveGroupItem, cornerRadius: widget.config.cornerRadius, backgroundColor: widget.config.groupBackgroundColor, - dragStateStorage: widget.groupState, - dragTargetKeys: widget.groupState, + dragStateStorage: widget.boardState, + dragTargetKeys: widget.boardState, reorderFlexAction: reorderFlexAction, ); diff --git a/frontend/app_flowy/packages/appflowy_board/lib/src/widgets/board_data.dart b/frontend/app_flowy/packages/appflowy_board/lib/src/widgets/board_data.dart index 1cc7c2e433..43dd9728fc 100644 --- a/frontend/app_flowy/packages/appflowy_board/lib/src/widgets/board_data.dart +++ b/frontend/app_flowy/packages/appflowy_board/lib/src/widgets/board_data.dart @@ -138,7 +138,11 @@ class AppFlowyBoardController extends ChangeNotifier /// groups or get ready to reinitialize the [AppFlowyBoard]. void clear() { _groupDatas.clear(); + for (final group in _groupControllers.values) { + group.dispose(); + } _groupControllers.clear(); + notifyListeners(); } @@ -223,6 +227,8 @@ class AppFlowyBoardController extends ChangeNotifier final fromGroupController = getGroupController(fromGroupId)!; final toGroupController = getGroupController(toGroupId)!; final fromGroupItem = fromGroupController.removeAt(fromGroupIndex); + if (fromGroupItem == null) return; + if (toGroupController.items.length > toGroupIndex) { assert(toGroupController.items[toGroupIndex] is PhantomGroupItem); @@ -283,7 +289,9 @@ class AppFlowyBoardController extends ChangeNotifier Log.trace( '[$BoardPhantomController] update $groupId:$index to $groupId:$newIndex'); final item = groupController.removeAt(index, notify: false); - groupController.insert(newIndex, item, notify: false); + if (item != null) { + groupController.insert(newIndex, item, notify: false); + } } } } diff --git a/frontend/app_flowy/packages/appflowy_board/lib/src/widgets/board_group/group.dart b/frontend/app_flowy/packages/appflowy_board/lib/src/widgets/board_group/group.dart index 880a81f666..b0c69b1070 100644 --- a/frontend/app_flowy/packages/appflowy_board/lib/src/widgets/board_group/group.dart +++ b/frontend/app_flowy/packages/appflowy_board/lib/src/widgets/board_group/group.dart @@ -156,9 +156,9 @@ class _AppFlowyBoardGroupState extends State { widget.onDragStarted?.call(index); }, onReorder: ((fromIndex, toIndex) { - if (widget.phantomController.isFromGroup(widget.groupId)) { + if (widget.phantomController.shouldReorder(widget.groupId)) { widget.onReorder(widget.groupId, fromIndex, toIndex); - widget.phantomController.transformIndex(fromIndex, toIndex); + widget.phantomController.updateIndex(fromIndex, toIndex); } }), onDragEnded: () { diff --git a/frontend/app_flowy/packages/appflowy_board/lib/src/widgets/board_group/group_data.dart b/frontend/app_flowy/packages/appflowy_board/lib/src/widgets/board_group/group_data.dart index d26949e20a..659e87fadd 100644 --- a/frontend/app_flowy/packages/appflowy_board/lib/src/widgets/board_group/group_data.dart +++ b/frontend/app_flowy/packages/appflowy_board/lib/src/widgets/board_group/group_data.dart @@ -52,8 +52,17 @@ class AppFlowyGroupController extends ChangeNotifier with EquatableMixin { /// * [notify] the default value of [notify] is true, it will notify the /// listener. Set to false if you do not want to notify the listeners. /// - AppFlowyGroupItem removeAt(int index, {bool notify = true}) { - assert(index >= 0); + AppFlowyGroupItem? removeAt(int index, {bool notify = true}) { + if (groupData._items.length <= index) { + Log.error( + 'Fatal error, index is out of bounds. Index: $index, len: ${groupData._items.length}'); + return null; + } + + if (index < 0) { + Log.error('Invalid index:$index'); + return null; + } Log.debug('[$AppFlowyGroupController] $groupData remove item at $index'); final item = groupData._items.removeAt(index); @@ -73,12 +82,17 @@ class AppFlowyGroupController extends ChangeNotifier with EquatableMixin { /// Move the item from [fromIndex] to [toIndex]. It will do nothing if the /// [fromIndex] equal to the [toIndex]. bool move(int fromIndex, int toIndex) { - assert(fromIndex >= 0); assert(toIndex >= 0); + if (groupData._items.length < fromIndex) { + Log.error( + 'Out of bounds error. index: $fromIndex should not greater than ${groupData._items.length}'); + return false; + } if (fromIndex == toIndex) { return false; } + Log.debug( '[$AppFlowyGroupController] $groupData move item from $fromIndex to $toIndex'); final item = groupData._items.removeAt(fromIndex); @@ -126,7 +140,7 @@ class AppFlowyGroupController extends ChangeNotifier with EquatableMixin { Log.debug('[$AppFlowyGroupController] $groupData add $newItem'); } else { if (index >= groupData._items.length) { - Log.warn( + Log.error( '[$AppFlowyGroupController] unexpected items length, index should less than the count of the items. Index: $index, items count: ${items.length}'); return; } diff --git a/frontend/app_flowy/packages/appflowy_board/lib/src/widgets/reorder_phantom/phantom_controller.dart b/frontend/app_flowy/packages/appflowy_board/lib/src/widgets/reorder_phantom/phantom_controller.dart index ad2778f6d8..7e991b34cd 100644 --- a/frontend/app_flowy/packages/appflowy_board/lib/src/widgets/reorder_phantom/phantom_controller.dart +++ b/frontend/app_flowy/packages/appflowy_board/lib/src/widgets/reorder_phantom/phantom_controller.dart @@ -46,15 +46,23 @@ class BoardPhantomController extends OverlapDragTargetDelegate required this.groupsState, }); - bool isFromGroup(String groupId) { + /// Determines whether the group should perform reorder + /// + /// Returns `true` if the fromGroupId and toGroupId of the phantomRecord + /// equal to the passed in groupId. + /// + /// Returns `true` if the phantomRecord is null + /// + bool shouldReorder(String groupId) { if (phantomRecord != null) { - return phantomRecord!.fromGroupId == groupId; + return phantomRecord!.toGroupId == groupId && + phantomRecord!.fromGroupId == groupId; } else { return true; } } - void transformIndex(int fromIndex, int toIndex) { + void updateIndex(int fromIndex, int toIndex) { if (phantomRecord == null) { return; } @@ -69,7 +77,6 @@ class BoardPhantomController extends OverlapDragTargetDelegate /// Remove the phantom in the group when the group is end dragging. void groupEndDragging(String groupId) { phantomState.setGroupIsDragging(groupId, false); - if (phantomRecord == null) return; final fromGroupId = phantomRecord!.fromGroupId; @@ -246,10 +253,6 @@ class PhantomRecord { }); void updateFromGroupIndex(int index) { - if (fromGroupIndex == index) { - return; - } - fromGroupIndex = index; } 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 29f7b759b8..3607a1ce74 100644 --- a/frontend/rust-lib/flowy-grid/src/services/grid_editor.rs +++ b/frontend/rust-lib/flowy-grid/src/services/grid_editor.rs @@ -623,7 +623,7 @@ impl GridRevisionEditor { self.view_manager .move_group_row(row_rev, to_group_id, to_row_id.clone(), |row_changeset| { wrap_future(async move { - tracing::trace!("Move group row cause row data changed: {:?}", row_changeset); + tracing::trace!("Row data changed: {:?}", row_changeset); let cell_changesets = row_changeset .cell_by_field_id .into_iter() diff --git a/frontend/rust-lib/flowy-grid/src/services/grid_view_editor.rs b/frontend/rust-lib/flowy-grid/src/services/grid_view_editor.rs index 6316135f5a..ce0edbef6f 100644 --- a/frontend/rust-lib/flowy-grid/src/services/grid_view_editor.rs +++ b/frontend/rust-lib/flowy-grid/src/services/grid_view_editor.rs @@ -79,7 +79,7 @@ impl GridViewRevisionEditor { Ok(json_str) } - pub(crate) async fn will_create_row(&self, row_rev: &mut RowRevision, params: &CreateRowParams) { + pub(crate) async fn will_create_view_row(&self, row_rev: &mut RowRevision, params: &CreateRowParams) { if params.group_id.is_none() { return; } @@ -92,7 +92,7 @@ impl GridViewRevisionEditor { .await; } - pub(crate) async fn did_create_row(&self, row_pb: &RowPB, params: &CreateRowParams) { + pub(crate) async fn did_create_view_row(&self, row_pb: &RowPB, params: &CreateRowParams) { // Send the group notification if the current view has groups match params.group_id.as_ref() { None => {} @@ -115,7 +115,7 @@ impl GridViewRevisionEditor { } #[tracing::instrument(level = "trace", skip_all)] - pub(crate) async fn did_delete_row(&self, row_rev: &RowRevision) { + pub(crate) async fn did_delete_view_row(&self, row_rev: &RowRevision) { // Send the group notification if the current view has groups; let changesets = self .mut_group_controller(|group_controller, field_rev| group_controller.did_delete_row(row_rev, &field_rev)) @@ -129,7 +129,7 @@ impl GridViewRevisionEditor { } } - pub(crate) async fn did_update_row(&self, row_rev: &RowRevision) { + pub(crate) async fn did_update_view_row(&self, row_rev: &RowRevision) { let changesets = self .mut_group_controller(|group_controller, field_rev| group_controller.did_update_row(row_rev, &field_rev)) .await; @@ -141,7 +141,7 @@ impl GridViewRevisionEditor { } } - pub(crate) async fn move_group_row( + pub(crate) async fn move_view_group_row( &self, row_rev: &RowRevision, row_changeset: &mut RowChangeset, @@ -167,14 +167,14 @@ impl GridViewRevisionEditor { } /// Only call once after grid view editor initialized #[tracing::instrument(level = "trace", skip(self))] - pub(crate) async fn load_groups(&self) -> FlowyResult> { + pub(crate) async fn load_view_groups(&self) -> FlowyResult> { let groups = self.group_controller.read().await.groups(); tracing::trace!("Number of groups: {}", groups.len()); Ok(groups.into_iter().map(GroupPB::from).collect()) } #[tracing::instrument(level = "trace", skip(self), err)] - pub(crate) async fn move_group(&self, params: MoveGroupParams) -> FlowyResult<()> { + pub(crate) async fn move_view_group(&self, params: MoveGroupParams) -> FlowyResult<()> { let _ = self .group_controller .write() @@ -206,13 +206,13 @@ impl GridViewRevisionEditor { self.group_controller.read().await.field_id().to_owned() } - pub(crate) async fn get_setting(&self) -> GridSettingPB { + pub(crate) async fn get_view_setting(&self) -> GridSettingPB { let field_revs = self.field_delegate.get_field_revs().await; let grid_setting = make_grid_setting(&*self.pad.read().await, &field_revs); grid_setting } - pub(crate) async fn get_filters(&self) -> Vec { + pub(crate) async fn get_view_filters(&self) -> Vec { let field_revs = self.field_delegate.get_field_revs().await; match self.pad.read().await.get_all_filters(&field_revs) { None => vec![], @@ -245,7 +245,7 @@ impl GridViewRevisionEditor { Ok(()) } - pub(crate) async fn delete_group(&self, params: DeleteGroupParams) -> FlowyResult<()> { + pub(crate) async fn delete_view_group(&self, params: DeleteGroupParams) -> FlowyResult<()> { self.modify(|pad| { let changeset = pad.delete_filter(¶ms.field_id, ¶ms.field_type_rev, ¶ms.group_id)?; Ok(changeset) @@ -253,7 +253,7 @@ impl GridViewRevisionEditor { .await } - pub(crate) async fn insert_filter(&self, params: InsertFilterParams) -> FlowyResult<()> { + pub(crate) async fn insert_view_filter(&self, params: InsertFilterParams) -> FlowyResult<()> { self.modify(|pad| { let filter_rev = FilterConfigurationRevision { id: gen_grid_filter_id(), @@ -267,7 +267,7 @@ impl GridViewRevisionEditor { .await } - pub(crate) async fn delete_filter(&self, delete_filter: DeleteFilterParams) -> FlowyResult<()> { + pub(crate) async fn delete_view_filter(&self, delete_filter: DeleteFilterParams) -> FlowyResult<()> { self.modify(|pad| { let changeset = pad.delete_filter( &delete_filter.field_id, @@ -324,7 +324,7 @@ impl GridViewRevisionEditor { } async fn notify_did_update_setting(&self) { - let setting = self.get_setting().await; + let setting = self.get_view_setting().await; send_dart_notification(&self.view_id, GridNotification::DidUpdateGridSetting) .payload(setting) .send(); 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 98b8ee51b9..65ed1af6c5 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 @@ -65,14 +65,14 @@ impl GridViewManager { /// When the row was created, we may need to modify the [RowRevision] according to the [CreateRowParams]. pub(crate) async fn will_create_row(&self, row_rev: &mut RowRevision, params: &CreateRowParams) { for view_editor in self.view_editors.iter() { - view_editor.will_create_row(row_rev, params).await; + view_editor.will_create_view_row(row_rev, params).await; } } /// Notify the view that the row was created. For the moment, the view is just sending notifications. pub(crate) async fn did_create_row(&self, row_pb: &RowPB, params: &CreateRowParams) { for view_editor in self.view_editors.iter() { - view_editor.did_create_row(row_pb, params).await; + view_editor.did_create_view_row(row_pb, params).await; } } @@ -84,7 +84,7 @@ impl GridViewManager { } Some(row_rev) => { for view_editor in self.view_editors.iter() { - view_editor.did_update_row(&row_rev).await; + view_editor.did_update_view_row(&row_rev).await; } } } @@ -102,33 +102,33 @@ impl GridViewManager { pub(crate) async fn did_delete_row(&self, row_rev: Arc) { for view_editor in self.view_editors.iter() { - view_editor.did_delete_row(&row_rev).await; + view_editor.did_delete_view_row(&row_rev).await; } } pub(crate) async fn get_setting(&self) -> FlowyResult { let view_editor = self.get_default_view_editor().await?; - Ok(view_editor.get_setting().await) + Ok(view_editor.get_view_setting().await) } pub(crate) async fn get_filters(&self) -> FlowyResult> { let view_editor = self.get_default_view_editor().await?; - Ok(view_editor.get_filters().await) + Ok(view_editor.get_view_filters().await) } pub(crate) async fn insert_or_update_filter(&self, params: InsertFilterParams) -> FlowyResult<()> { let view_editor = self.get_default_view_editor().await?; - view_editor.insert_filter(params).await + view_editor.insert_view_filter(params).await } pub(crate) async fn delete_filter(&self, params: DeleteFilterParams) -> FlowyResult<()> { let view_editor = self.get_default_view_editor().await?; - view_editor.delete_filter(params).await + view_editor.delete_view_filter(params).await } pub(crate) async fn load_groups(&self) -> FlowyResult { let view_editor = self.get_default_view_editor().await?; - let groups = view_editor.load_groups().await?; + let groups = view_editor.load_view_groups().await?; Ok(RepeatedGridGroupPB { items: groups }) } @@ -139,12 +139,12 @@ impl GridViewManager { pub(crate) async fn delete_group(&self, params: DeleteGroupParams) -> FlowyResult<()> { let view_editor = self.get_default_view_editor().await?; - view_editor.delete_group(params).await + view_editor.delete_view_group(params).await } pub(crate) async fn move_group(&self, params: MoveGroupParams) -> FlowyResult<()> { let view_editor = self.get_default_view_editor().await?; - let _ = view_editor.move_group(params).await?; + let _ = view_editor.move_view_group(params).await?; Ok(()) } @@ -161,7 +161,7 @@ impl GridViewManager { let mut row_changeset = RowChangeset::new(row_rev.id.clone()); let view_editor = self.get_default_view_editor().await?; let group_changesets = view_editor - .move_group_row(&row_rev, &mut row_changeset, &to_group_id, to_row_id.clone()) + .move_view_group_row(&row_rev, &mut row_changeset, &to_group_id, to_row_id.clone()) .await; if !row_changeset.is_empty() { diff --git a/frontend/rust-lib/flowy-grid/src/services/group/configuration.rs b/frontend/rust-lib/flowy-grid/src/services/group/configuration.rs index 8f3df66727..595af8174a 100644 --- a/frontend/rust-lib/flowy-grid/src/services/group/configuration.rs +++ b/frontend/rust-lib/flowy-grid/src/services/group/configuration.rs @@ -119,12 +119,20 @@ where self.mut_configuration(|configuration| { let from_index = configuration.groups.iter().position(|group| group.id == from_id); let to_index = configuration.groups.iter().position(|group| group.id == to_id); - tracing::info!("Configuration groups: {:?} ", configuration.groups); if let (Some(from), Some(to)) = &(from_index, to_index) { tracing::trace!("Move group from index:{:?} to index:{:?}", from_index, to_index); let group = configuration.groups.remove(*from); configuration.groups.insert(*to, group); } + tracing::debug!( + "Group order: {:?} ", + configuration + .groups + .iter() + .map(|group| group.name.clone()) + .collect::>() + .join(",") + ); from_index.is_some() && to_index.is_some() })?; 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 9c3e71bc3e..e5cbc8a8ec 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 @@ -80,9 +80,9 @@ pub fn move_group_row(group: &mut Group, context: &mut MoveGroupRowContext) -> O }; // Remove the row in which group contains it - if from_index.is_some() { + if let Some(from_index) = &from_index { changeset.deleted_rows.push(row_rev.id.clone()); - tracing::debug!("Group:{} remove row:{}", group.id, row_rev.id); + tracing::debug!("Group:{} remove {} at {}", group.id, row_rev.id, from_index); group.remove_row(&row_rev.id); } @@ -97,10 +97,11 @@ pub fn move_group_row(group: &mut Group, context: &mut MoveGroupRowContext) -> O } Some(to_index) => { if to_index < group.number_of_row() { - tracing::debug!("Group:{} insert row:{} at {} ", group.id, row_rev.id, to_index); + tracing::debug!("Group:{} insert {} at {} ", group.id, row_rev.id, to_index); inserted_row.index = Some(to_index as i32); group.insert_row(to_index, row_pb); } else { + tracing::warn!("Mote to index: {} is out of bounds", to_index); tracing::debug!("Group:{} append row:{}", group.id, row_rev.id); group.add_row(row_pb); } 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 42fdc6b61c..2040379c4c 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 @@ -350,6 +350,36 @@ async fn group_move_from_default_group_test() { #[tokio::test] async fn group_move_group_test() { + let mut test = GridGroupTest::new().await; + let group_0 = test.group_at_index(0).await; + let group_1 = test.group_at_index(1).await; + let scripts = vec![ + MoveGroup { + from_group_index: 0, + to_group_index: 1, + }, + AssertGroupRowCount { + group_index: 0, + row_count: 2, + }, + AssertGroup { + group_index: 0, + expected_group: group_1, + }, + AssertGroupRowCount { + group_index: 1, + row_count: 2, + }, + AssertGroup { + group_index: 1, + expected_group: group_0, + }, + ]; + test.run_scripts(scripts).await; +} + +#[tokio::test] +async fn group_move_group_row_after_move_group_test() { let mut test = GridGroupTest::new().await; let group_0 = test.group_at_index(0).await; let group_1 = test.group_at_index(1).await; @@ -366,6 +396,20 @@ async fn group_move_group_test() { group_index: 1, expected_group: group_0, }, + MoveRow { + from_group_index: 0, + from_row_index: 0, + to_group_index: 1, + to_row_index: 0, + }, + AssertGroupRowCount { + group_index: 0, + row_count: 1, + }, + AssertGroupRowCount { + group_index: 1, + row_count: 3, + }, ]; test.run_scripts(scripts).await; }