fix: update auto-update fields on change group (#3833)

* fix: update auto-update fields on change group

auto-update fields should get updated when on moving row from one
group to another. They where getting updated only on changing row
properties directly.
Also added integration test for changing row groupd related to #3010.

* style: rust format

* fix: add board group test to runner

* fix: update auto updated fiels only when group has changed

* fix: apply group changeset before move_row

* fix: get from_row before move_row

* fix: groups method changed to get_all_groups

* fix: pass from group id in move group payload

We can't know the from group id from the row itself. Because a row
can exist in multiple groups (group by multiselect).

* fix: MoveGroupRowPayloadPB from_group_id index
This commit is contained in:
Mohammad Zolfaghari 2023-11-30 19:54:10 +03:30 committed by GitHub
parent d004c94a2f
commit 066bba95f5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 149 additions and 56 deletions

View File

@ -0,0 +1,50 @@
import 'package:appflowy/plugins/database_view/widgets/row/cells/select_option_cell/extension.dart';
import 'package:appflowy/plugins/database_view/widgets/row/row_property.dart';
import 'package:appflowy_backend/protobuf/flowy-folder2/view.pb.dart';
import 'package:flowy_infra_ui/flowy_infra_ui.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:integration_test/integration_test.dart';
import 'package:appflowy_board/appflowy_board.dart';
import '../util/util.dart';
void main() {
IntegrationTestWidgetsFlutterBinding.ensureInitialized();
group('board group test', () {
testWidgets('move row to another group', (tester) async {
const card1Name = 'Card 1';
await tester.initializeAppFlowy();
await tester.tapGoButton();
await tester.createNewPageWithName(layout: ViewLayoutPB.Board);
final card1 = find.ancestor(
of: find.findTextInFlowyText(card1Name),
matching: find.byType(AppFlowyGroupCard),
);
final doingGroup = find.findTextInFlowyText('Doing');
final doingGroupCenter = tester.getCenter(doingGroup);
final card1Center = tester.getCenter(card1);
await tester.timedDrag(
card1,
doingGroupCenter.translate(-card1Center.dx, -card1Center.dy),
const Duration(seconds: 1),
);
await tester.pumpAndSettle();
await tester.tap(card1);
await tester.pumpAndSettle();
final card1StatusFinder = find.descendant(
of: find.byType(RowPropertyList),
matching: find.descendant(
of: find.byType(SelectOptionTag),
matching: find.byType(FlowyText),
),
);
expect(card1StatusFinder, findsNWidgets(1));
final card1StatusText = tester.widget<FlowyText>(card1StatusFinder).text;
expect(card1StatusText, 'Doing');
});
});
}

View File

@ -2,6 +2,7 @@ import 'package:integration_test/integration_test.dart';
import 'board_row_test.dart' as board_row_test;
import 'board_add_row_test.dart' as board_add_row_test;
import 'board_group_test.dart' as board_group_test;
void startTesting() {
IntegrationTestWidgetsFlutterBinding.ensureInitialized();
@ -9,4 +10,5 @@ void startTesting() {
// Board integration tests
board_row_test.main();
board_add_row_test.main();
board_group_test.main();
}

View File

@ -198,12 +198,14 @@ class DatabaseController {
Future<Either<Unit, FlowyError>> moveGroupRow({
required RowMetaPB fromRow,
required String groupId,
required String fromGroupId,
required String toGroupId,
RowMetaPB? toRow,
}) {
return _databaseViewBackendSvc.moveGroupRow(
fromRowId: fromRow.id,
toGroupId: groupId,
fromGroupId: fromGroupId,
toGroupId: toGroupId,
toRowId: toRow?.id,
);
}

View File

@ -56,12 +56,14 @@ class DatabaseViewBackendService {
Future<Either<Unit, FlowyError>> moveGroupRow({
required RowId fromRowId,
required String fromGroupId,
required String toGroupId,
RowId? toRowId,
}) {
final payload = MoveGroupRowPayloadPB.create()
..viewId = viewId
..fromRowId = fromRowId
..fromGroupId = fromGroupId
..toGroupId = toGroupId;
if (toRowId != null) {

View File

@ -54,7 +54,8 @@ class BoardBloc extends Bloc<BoardEvent, BoardState> {
databaseController.moveGroupRow(
fromRow: fromRow,
toRow: toRow,
groupId: groupId,
fromGroupId: groupId,
toGroupId: groupId,
);
}
},
@ -65,7 +66,8 @@ class BoardBloc extends Bloc<BoardEvent, BoardState> {
databaseController.moveGroupRow(
fromRow: fromRow,
toRow: toRow,
groupId: toGroupId,
fromGroupId: fromGroupId,
toGroupId: toGroupId,
);
}
},

View File

@ -162,11 +162,15 @@ pub struct MoveGroupRowPayloadPB {
#[pb(index = 4, one_of)]
pub to_row_id: Option<String>,
#[pb(index = 5)]
pub from_group_id: String,
}
pub struct MoveGroupRowParams {
pub view_id: String,
pub from_row_id: RowId,
pub from_group_id: String,
pub to_group_id: String,
pub to_row_id: Option<RowId>,
}
@ -176,12 +180,15 @@ impl TryInto<MoveGroupRowParams> for MoveGroupRowPayloadPB {
fn try_into(self) -> Result<MoveGroupRowParams, Self::Error> {
let view_id = NotEmptyStr::parse(self.view_id).map_err(|_| ErrorCode::DatabaseViewIdIsEmpty)?;
let from_group_id =
NotEmptyStr::parse(self.from_group_id).map_err(|_| ErrorCode::GroupIdIsEmpty)?;
let to_group_id =
NotEmptyStr::parse(self.to_group_id).map_err(|_| ErrorCode::GroupIdIsEmpty)?;
Ok(MoveGroupRowParams {
view_id: view_id.0,
to_group_id: to_group_id.0,
from_group_id: from_group_id.0,
from_row_id: RowId::from(self.from_row_id),
to_row_id: self.to_row_id.map(RowId::from),
})

View File

@ -736,6 +736,7 @@ pub(crate) async fn move_group_row_handler(
database_editor
.move_group_row(
&params.view_id,
&params.from_group_id,
&params.to_group_id,
params.from_row_id,
params.to_row_id,

View File

@ -709,10 +709,6 @@ impl DatabaseEditor {
// Get the old row before updating the cell. It would be better to get the old cell
let old_row = { self.get_row_detail(view_id, &row_id) };
// Get all auto updated fields. It will be used to notify the frontend
// that the fields have been updated.
let auto_updated_fields = self.get_auto_updated_fields(view_id);
self.database.lock().update_row(&row_id, |row_update| {
row_update.update_cells(|cell_update| {
cell_update.insert(field_id, new_cell);
@ -721,37 +717,45 @@ impl DatabaseEditor {
let option_row = self.get_row_detail(view_id, &row_id);
if let Some(new_row_detail) = option_row {
let updated_row =
UpdatedRow::new(&new_row_detail.row.id).with_field_ids(vec![field_id.to_string()]);
let changes = RowsChangePB::from_update(updated_row.into());
send_notification(view_id, DatabaseNotification::DidUpdateViewRows)
.payload(changes)
.send();
for view in self.database_views.editors().await {
view
.v_did_update_row(&old_row, &new_row_detail, field_id)
.await;
view.v_did_update_row(&old_row, &new_row_detail).await;
}
}
let changeset = CellChangesetNotifyPB {
view_id: view_id.to_string(),
row_id: row_id.clone().into_inner(),
field_id: field_id.to_string(),
};
self
.notify_update_row(view_id, row_id, vec![changeset])
.await;
Ok(())
}
pub fn get_auto_updated_fields_changesets(
&self,
view_id: &str,
row_id: RowId,
) -> Vec<CellChangesetNotifyPB> {
// Get all auto updated fields. It will be used to notify the frontend
// that the fields have been updated.
let auto_updated_fields = self.get_auto_updated_fields(view_id);
// Collect all the updated field's id. Notify the frontend that all of them have been updated.
let mut auto_updated_field_ids = auto_updated_fields
let auto_updated_field_ids = auto_updated_fields
.into_iter()
.map(|field| field.id)
.collect::<Vec<String>>();
auto_updated_field_ids.push(field_id.to_string());
let changeset = auto_updated_field_ids
auto_updated_field_ids
.into_iter()
.map(|field_id| CellChangesetNotifyPB {
view_id: view_id.to_string(),
row_id: row_id.clone().into_inner(),
field_id,
})
.collect();
notify_did_update_cell(changeset).await;
Ok(())
.collect()
}
/// Just create an option for the field's type option. The option is save to the database.
@ -920,6 +924,7 @@ impl DatabaseEditor {
pub async fn move_group_row(
&self,
view_id: &str,
from_group: &str,
to_group: &str,
from_row: RowId,
to_row: Option<RowId>,
@ -933,17 +938,12 @@ impl DatabaseEditor {
)
},
Some(row_detail) => {
let mut row_changeset = RowChangeset::new(row_detail.row.id.clone());
let view = self.database_views.get_view_editor(view_id).await?;
let mut row_changeset = RowChangeset::new(row_detail.row.id.clone());
view
.v_move_group_row(&row_detail, &mut row_changeset, to_group, to_row.clone())
.await;
tracing::trace!("Row data changed: {:?}", row_changeset);
self.database.lock().update_row(&row_detail.row.id, |row| {
row.set_cells(Cells::from(row_changeset.cell_by_field_id.clone()));
});
let to_row = if to_row.is_some() {
to_row
} else {
@ -952,17 +952,25 @@ impl DatabaseEditor {
.last()
.map(|row_detail| row_detail.row.id.clone())
};
if let Some(row_id) = to_row {
self.move_row(view_id, from_row, row_id).await;
if let Some(row_id) = to_row.clone() {
self.move_row(view_id, from_row.clone(), row_id).await;
}
let cell_changesets = cell_changesets_from_cell_by_field_id(
if from_group == to_group {
return Ok(());
}
tracing::trace!("Row data changed: {:?}", row_changeset);
self.database.lock().update_row(&row_detail.row.id, |row| {
row.set_cells(Cells::from(row_changeset.cell_by_field_id.clone()));
});
let changesets = cell_changesets_from_cell_by_field_id(
view_id,
row_changeset.row_id,
row_changeset.cell_by_field_id,
);
notify_did_update_cell(cell_changesets).await;
self.notify_update_row(view_id, from_row, changesets).await;
},
}
@ -1165,6 +1173,19 @@ impl DatabaseEditor {
pub fn get_mutex_database(&self) -> &MutexDatabase {
&self.database
}
async fn notify_update_row(
&self,
view_id: &str,
row: RowId,
extra_changesets: Vec<CellChangesetNotifyPB>,
) {
let mut changesets = self.get_auto_updated_fields_changesets(view_id, row);
changesets.extend(extra_changesets);
notify_did_update_cell(changesets.clone()).await;
notify_did_update_row(changesets).await;
}
}
pub(crate) async fn notify_did_update_cell(changesets: Vec<CellChangesetNotifyPB>) {
@ -1174,6 +1195,22 @@ pub(crate) async fn notify_did_update_cell(changesets: Vec<CellChangesetNotifyPB
}
}
async fn notify_did_update_row(changesets: Vec<CellChangesetNotifyPB>) {
let row_id = changesets[0].row_id.clone();
let view_id = changesets[0].view_id.clone();
let field_ids = changesets
.iter()
.map(|changeset| changeset.field_id.to_string())
.collect();
let update_row = UpdatedRow::new(&row_id).with_field_ids(field_ids);
let update_changeset = RowsChangePB::from_update(update_row.into());
send_notification(&view_id, DatabaseNotification::DidUpdateViewRows)
.payload(update_changeset)
.send();
}
fn cell_changesets_from_cell_by_field_id(
view_id: &str,
row_id: RowId,

View File

@ -178,12 +178,7 @@ impl DatabaseViewEditor {
/// Notify the view that the row has been updated. If the view has groups,
/// send the group notification with [GroupRowsNotificationPB]. Otherwise,
/// send the view notification with [RowsChangePB]
pub async fn v_did_update_row(
&self,
old_row: &Option<RowDetail>,
row_detail: &RowDetail,
field_id: &str,
) {
pub async fn v_did_update_row(&self, old_row: &Option<RowDetail>, row_detail: &RowDetail) {
let result = self
.mut_group_controller(|group_controller, field| {
Ok(group_controller.did_update_group_row(old_row, row_detail, &field))
@ -214,13 +209,6 @@ impl DatabaseViewEditor {
notify_did_update_group_rows(changeset).await;
}
}
} else {
let update_row =
UpdatedRow::new(&row_detail.row.id).with_field_ids(vec![field_id.to_string()]);
let changeset = RowsChangePB::from_update(update_row.into());
send_notification(&self.view_id, DatabaseNotification::DidUpdateViewRows)
.payload(changeset)
.send();
}
// Each row update will trigger a filter and sort operation. We don't want

View File

@ -107,12 +107,8 @@ impl DatabaseGroupTest {
to_row_index,
} => {
let groups: Vec<GroupPB> = self.editor.load_groups(&self.view_id).await.unwrap().items;
let from_row = groups
.get(from_group_index)
.unwrap()
.rows
.get(from_row_index)
.unwrap();
let from_group = groups.get(from_group_index).unwrap();
let from_row = from_group.rows.get(from_row_index).unwrap();
let to_group = groups.get(to_group_index).unwrap();
let to_row = to_group.rows.get(to_row_index).unwrap();
let from_row = RowId::from(from_row.id.clone());
@ -120,7 +116,13 @@ impl DatabaseGroupTest {
self
.editor
.move_group_row(&self.view_id, &to_group.group_id, from_row, Some(to_row))
.move_group_row(
&self.view_id,
&from_group.group_id,
&to_group.group_id,
from_row,
Some(to_row),
)
.await
.unwrap();
},