From a6c6aa819c332b185bbd07cfd674d595bc786773 Mon Sep 17 00:00:00 2001 From: Richard Shiue <71320345+richardshiue@users.noreply.github.com> Date: Tue, 20 Feb 2024 13:17:46 +0800 Subject: [PATCH] fix: newly-created rows aren't being automatically sorted (#4661) * fix: newly-created rows aren't being automatically sorted * chore: new database view collab notification and add tests --- .../src/services/database_view/notifier.rs | 4 ++- .../src/services/database_view/view_editor.rs | 15 ++++++-- .../src/services/sort/controller.rs | 36 ++++++++++++++++++- .../src/services/sort/entities.rs | 7 ++++ .../tests/database/sort_test/script.rs | 24 +++++++++++++ .../database/sort_test/single_sort_test.rs | 28 ++++++++++++++- 6 files changed, 108 insertions(+), 6 deletions(-) diff --git a/frontend/rust-lib/flowy-database2/src/services/database_view/notifier.rs b/frontend/rust-lib/flowy-database2/src/services/database_view/notifier.rs index 03267a7cf0..9a1cbecf98 100644 --- a/frontend/rust-lib/flowy-database2/src/services/database_view/notifier.rs +++ b/frontend/rust-lib/flowy-database2/src/services/database_view/notifier.rs @@ -6,7 +6,7 @@ use crate::entities::{ }; use crate::notification::{send_notification, DatabaseNotification}; use crate::services::filter::FilterResultNotification; -use crate::services::sort::{ReorderAllRowsResult, ReorderSingleRowResult}; +use crate::services::sort::{InsertSortedRowResult, ReorderAllRowsResult, ReorderSingleRowResult}; use async_stream::stream; use futures::stream::StreamExt; use tokio::sync::broadcast; @@ -16,6 +16,7 @@ pub enum DatabaseViewChanged { FilterNotification(FilterResultNotification), ReorderAllRowsNotification(ReorderAllRowsResult), ReorderSingleRowNotification(ReorderSingleRowResult), + InsertSortedRowNotification(InsertSortedRowResult), CalculationValueNotification(CalculationChangesetNotificationPB), } @@ -78,6 +79,7 @@ impl DatabaseViewChangedReceiverRunner { .payload(reorder_row) .send() }, + DatabaseViewChanged::InsertSortedRowNotification(_result) => {}, DatabaseViewChanged::CalculationValueNotification(notification) => send_notification( ¬ification.view_id, DatabaseNotification::DidUpdateCalculation, diff --git a/frontend/rust-lib/flowy-database2/src/services/database_view/view_editor.rs b/frontend/rust-lib/flowy-database2/src/services/database_view/view_editor.rs index 48e731cbe1..a81c1733bb 100644 --- a/frontend/rust-lib/flowy-database2/src/services/database_view/view_editor.rs +++ b/frontend/rust-lib/flowy-database2/src/services/database_view/view_editor.rs @@ -161,7 +161,7 @@ impl DatabaseViewEditor { .payload(changes) .send(); self - .gen_view_tasks(row_detail.row.id.clone(), "".to_string()) + .gen_did_create_row_view_tasks(row_detail.row.id.clone()) .await; } @@ -257,7 +257,7 @@ impl DatabaseViewEditor { // Each row update will trigger a calculations, filter and sort operation. We don't want // to block the main thread, so we spawn a new task to do the work. self - .gen_view_tasks(row_detail.row.id.clone(), field_id) + .gen_did_update_row_view_tasks(row_detail.row.id.clone(), field_id) .await; } @@ -1071,7 +1071,7 @@ impl DatabaseViewEditor { } } - async fn gen_view_tasks(&self, row_id: RowId, field_id: String) { + async fn gen_did_update_row_view_tasks(&self, row_id: RowId, field_id: String) { let weak_filter_controller = Arc::downgrade(&self.filter_controller); let weak_sort_controller = Arc::downgrade(&self.sort_controller); let weak_calculations_controller = Arc::downgrade(&self.calculations_controller); @@ -1095,4 +1095,13 @@ impl DatabaseViewEditor { } }); } + + async fn gen_did_create_row_view_tasks(&self, row_id: RowId) { + let weak_sort_controller = Arc::downgrade(&self.sort_controller); + af_spawn(async move { + if let Some(sort_controller) = weak_sort_controller.upgrade() { + sort_controller.read().await.did_create_row(row_id).await; + } + }); + } } diff --git a/frontend/rust-lib/flowy-database2/src/services/sort/controller.rs b/frontend/rust-lib/flowy-database2/src/services/sort/controller.rs index b1d709728f..c15ceddc75 100644 --- a/frontend/rust-lib/flowy-database2/src/services/sort/controller.rs +++ b/frontend/rust-lib/flowy-database2/src/services/sort/controller.rs @@ -21,7 +21,8 @@ use crate::services::field::{ default_order, TimestampCellData, TimestampCellDataWrapper, TypeOptionCellExt, }; use crate::services::sort::{ - ReorderAllRowsResult, ReorderSingleRowResult, Sort, SortChangeset, SortCondition, + InsertSortedRowResult, ReorderAllRowsResult, ReorderSingleRowResult, Sort, SortChangeset, + SortCondition, }; pub trait SortDelegate: Send + Sync { @@ -93,6 +94,17 @@ impl SortController { } } + pub async fn did_create_row(&self, row_id: RowId) { + if !self.sorts.is_empty() { + self + .gen_task( + SortEvent::NewRowInserted(row_id), + QualityOfService::Background, + ) + .await; + } + } + // #[tracing::instrument(name = "process_sort_task", level = "trace", skip_all, err)] pub async fn process(&mut self, predicate: &str) -> FlowyResult<()> { let event_type = SortEvent::from_str(predicate).unwrap(); @@ -118,6 +130,7 @@ impl SortController { }, SortEvent::RowDidChanged(row_id) => { let old_row_index = self.row_index_cache.get(&row_id).cloned(); + self.sort_rows(&mut row_details).await; let new_row_index = self.row_index_cache.get(&row_id).cloned(); match (old_row_index, new_row_index) { @@ -140,6 +153,26 @@ impl SortController { _ => tracing::trace!("The row index cache is outdated"), } }, + SortEvent::NewRowInserted(row_id) => { + self.sort_rows(&mut row_details).await; + let row_index = self.row_index_cache.get(&row_id).cloned(); + match row_index { + Some(row_index) => { + let notification = InsertSortedRowResult { + row_id: row_id.clone(), + view_id: self.view_id.clone(), + index: row_index, + }; + self.row_index_cache.insert(row_id, row_index); + let _ = self + .notifier + .send(DatabaseViewChanged::InsertSortedRowNotification( + notification, + )); + }, + _ => tracing::trace!("The row index cache is outdated"), + } + }, } Ok(()) } @@ -313,6 +346,7 @@ fn cmp_cell( enum SortEvent { SortDidChanged, RowDidChanged(RowId), + NewRowInserted(RowId), DeleteAllSorts, } diff --git a/frontend/rust-lib/flowy-database2/src/services/sort/entities.rs b/frontend/rust-lib/flowy-database2/src/services/sort/entities.rs index 68246f8b1e..59bc65cac3 100644 --- a/frontend/rust-lib/flowy-database2/src/services/sort/entities.rs +++ b/frontend/rust-lib/flowy-database2/src/services/sort/entities.rs @@ -121,6 +121,13 @@ pub struct ReorderSingleRowResult { pub new_index: usize, } +#[derive(Clone)] +pub struct InsertSortedRowResult { + pub view_id: String, + pub row_id: RowId, + pub index: usize, +} + #[derive(Debug, Default)] pub struct SortChangeset { pub(crate) insert_sort: Option, diff --git a/frontend/rust-lib/flowy-database2/tests/database/sort_test/script.rs b/frontend/rust-lib/flowy-database2/tests/database/sort_test/script.rs index 7e12235fe1..6020b2dba1 100644 --- a/frontend/rust-lib/flowy-database2/tests/database/sort_test/script.rs +++ b/frontend/rust-lib/flowy-database2/tests/database/sort_test/script.rs @@ -36,6 +36,7 @@ pub enum SortScript { row_id: RowId, text: String, }, + AddNewRow, AssertSortChanged { old_row_orders: Vec<&'static str>, new_row_orders: Vec<&'static str>, @@ -145,6 +146,28 @@ impl DatabaseSortTest { ); self.update_text_cell(row_id, &text).await.unwrap(); }, + SortScript::AddNewRow => { + self.recv = Some( + self + .editor + .subscribe_view_changed(&self.view_id) + .await + .unwrap(), + ); + self + .editor + .create_row( + &self.view_id, + None, + collab_database::rows::CreateRowParams { + id: collab_database::database::gen_row_id(), + timestamp: collab_database::database::timestamp(), + ..Default::default() + }, + ) + .await + .unwrap(); + }, SortScript::AssertSortChanged { new_row_orders, old_row_orders, @@ -195,6 +218,7 @@ async fn assert_sort_changed( old_row_orders.insert(changed.new_index, old); assert_eq!(old_row_orders, new_row_orders); }, + DatabaseViewChanged::InsertSortedRowNotification(_changed) => {}, _ => {}, } }) diff --git a/frontend/rust-lib/flowy-database2/tests/database/sort_test/single_sort_test.rs b/frontend/rust-lib/flowy-database2/tests/database/sort_test/single_sort_test.rs index 2620c3149b..be2a6c0e4d 100644 --- a/frontend/rust-lib/flowy-database2/tests/database/sort_test/single_sort_test.rs +++ b/frontend/rust-lib/flowy-database2/tests/database/sort_test/single_sort_test.rs @@ -81,10 +81,36 @@ async fn sort_change_notification_by_update_text_test() { test.run_scripts(scripts).await; } +#[tokio::test] +async fn sort_after_new_row_test() { + let mut test = DatabaseSortTest::new().await; + let checkbox_field = test.get_first_field(FieldType::Checkbox); + let scripts = vec![ + AssertCellContentOrder { + field_id: checkbox_field.id.clone(), + orders: vec!["Yes", "Yes", "No", "No", "No", "Yes", ""], + }, + InsertSort { + field: checkbox_field.clone(), + condition: SortCondition::Ascending, + }, + AssertCellContentOrder { + field_id: checkbox_field.id.clone(), + orders: vec!["No", "No", "No", "", "Yes", "Yes", "Yes"], + }, + AddNewRow {}, + AssertCellContentOrder { + field_id: checkbox_field.id, + orders: vec!["No", "No", "No", "", "", "Yes", "Yes", "Yes"], + }, + ]; + test.run_scripts(scripts).await; +} + #[tokio::test] async fn sort_text_by_ascending_and_delete_sort_test() { let mut test = DatabaseSortTest::new().await; - let text_field = test.get_first_field(FieldType::RichText).clone(); + let text_field = test.get_first_field(FieldType::RichText); let scripts = vec![ InsertSort { field: text_field.clone(),