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
This commit is contained in:
Richard Shiue
2024-02-20 13:17:46 +08:00
committed by GitHub
parent 15c9a67028
commit a6c6aa819c
6 changed files with 108 additions and 6 deletions

View File

@ -6,7 +6,7 @@ use crate::entities::{
}; };
use crate::notification::{send_notification, DatabaseNotification}; use crate::notification::{send_notification, DatabaseNotification};
use crate::services::filter::FilterResultNotification; use crate::services::filter::FilterResultNotification;
use crate::services::sort::{ReorderAllRowsResult, ReorderSingleRowResult}; use crate::services::sort::{InsertSortedRowResult, ReorderAllRowsResult, ReorderSingleRowResult};
use async_stream::stream; use async_stream::stream;
use futures::stream::StreamExt; use futures::stream::StreamExt;
use tokio::sync::broadcast; use tokio::sync::broadcast;
@ -16,6 +16,7 @@ pub enum DatabaseViewChanged {
FilterNotification(FilterResultNotification), FilterNotification(FilterResultNotification),
ReorderAllRowsNotification(ReorderAllRowsResult), ReorderAllRowsNotification(ReorderAllRowsResult),
ReorderSingleRowNotification(ReorderSingleRowResult), ReorderSingleRowNotification(ReorderSingleRowResult),
InsertSortedRowNotification(InsertSortedRowResult),
CalculationValueNotification(CalculationChangesetNotificationPB), CalculationValueNotification(CalculationChangesetNotificationPB),
} }
@ -78,6 +79,7 @@ impl DatabaseViewChangedReceiverRunner {
.payload(reorder_row) .payload(reorder_row)
.send() .send()
}, },
DatabaseViewChanged::InsertSortedRowNotification(_result) => {},
DatabaseViewChanged::CalculationValueNotification(notification) => send_notification( DatabaseViewChanged::CalculationValueNotification(notification) => send_notification(
&notification.view_id, &notification.view_id,
DatabaseNotification::DidUpdateCalculation, DatabaseNotification::DidUpdateCalculation,

View File

@ -161,7 +161,7 @@ impl DatabaseViewEditor {
.payload(changes) .payload(changes)
.send(); .send();
self self
.gen_view_tasks(row_detail.row.id.clone(), "".to_string()) .gen_did_create_row_view_tasks(row_detail.row.id.clone())
.await; .await;
} }
@ -257,7 +257,7 @@ impl DatabaseViewEditor {
// Each row update will trigger a calculations, filter and sort operation. We don't want // 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. // to block the main thread, so we spawn a new task to do the work.
self 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; .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_filter_controller = Arc::downgrade(&self.filter_controller);
let weak_sort_controller = Arc::downgrade(&self.sort_controller); let weak_sort_controller = Arc::downgrade(&self.sort_controller);
let weak_calculations_controller = Arc::downgrade(&self.calculations_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;
}
});
}
} }

View File

@ -21,7 +21,8 @@ use crate::services::field::{
default_order, TimestampCellData, TimestampCellDataWrapper, TypeOptionCellExt, default_order, TimestampCellData, TimestampCellDataWrapper, TypeOptionCellExt,
}; };
use crate::services::sort::{ use crate::services::sort::{
ReorderAllRowsResult, ReorderSingleRowResult, Sort, SortChangeset, SortCondition, InsertSortedRowResult, ReorderAllRowsResult, ReorderSingleRowResult, Sort, SortChangeset,
SortCondition,
}; };
pub trait SortDelegate: Send + Sync { 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)] // #[tracing::instrument(name = "process_sort_task", level = "trace", skip_all, err)]
pub async fn process(&mut self, predicate: &str) -> FlowyResult<()> { pub async fn process(&mut self, predicate: &str) -> FlowyResult<()> {
let event_type = SortEvent::from_str(predicate).unwrap(); let event_type = SortEvent::from_str(predicate).unwrap();
@ -118,6 +130,7 @@ impl SortController {
}, },
SortEvent::RowDidChanged(row_id) => { SortEvent::RowDidChanged(row_id) => {
let old_row_index = self.row_index_cache.get(&row_id).cloned(); let old_row_index = self.row_index_cache.get(&row_id).cloned();
self.sort_rows(&mut row_details).await; self.sort_rows(&mut row_details).await;
let new_row_index = self.row_index_cache.get(&row_id).cloned(); let new_row_index = self.row_index_cache.get(&row_id).cloned();
match (old_row_index, new_row_index) { match (old_row_index, new_row_index) {
@ -140,6 +153,26 @@ impl SortController {
_ => tracing::trace!("The row index cache is outdated"), _ => 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(()) Ok(())
} }
@ -313,6 +346,7 @@ fn cmp_cell(
enum SortEvent { enum SortEvent {
SortDidChanged, SortDidChanged,
RowDidChanged(RowId), RowDidChanged(RowId),
NewRowInserted(RowId),
DeleteAllSorts, DeleteAllSorts,
} }

View File

@ -121,6 +121,13 @@ pub struct ReorderSingleRowResult {
pub new_index: usize, pub new_index: usize,
} }
#[derive(Clone)]
pub struct InsertSortedRowResult {
pub view_id: String,
pub row_id: RowId,
pub index: usize,
}
#[derive(Debug, Default)] #[derive(Debug, Default)]
pub struct SortChangeset { pub struct SortChangeset {
pub(crate) insert_sort: Option<Sort>, pub(crate) insert_sort: Option<Sort>,

View File

@ -36,6 +36,7 @@ pub enum SortScript {
row_id: RowId, row_id: RowId,
text: String, text: String,
}, },
AddNewRow,
AssertSortChanged { AssertSortChanged {
old_row_orders: Vec<&'static str>, old_row_orders: Vec<&'static str>,
new_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(); 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 { SortScript::AssertSortChanged {
new_row_orders, new_row_orders,
old_row_orders, old_row_orders,
@ -195,6 +218,7 @@ async fn assert_sort_changed(
old_row_orders.insert(changed.new_index, old); old_row_orders.insert(changed.new_index, old);
assert_eq!(old_row_orders, new_row_orders); assert_eq!(old_row_orders, new_row_orders);
}, },
DatabaseViewChanged::InsertSortedRowNotification(_changed) => {},
_ => {}, _ => {},
} }
}) })

View File

@ -81,10 +81,36 @@ async fn sort_change_notification_by_update_text_test() {
test.run_scripts(scripts).await; 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] #[tokio::test]
async fn sort_text_by_ascending_and_delete_sort_test() { async fn sort_text_by_ascending_and_delete_sort_test() {
let mut test = DatabaseSortTest::new().await; 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![ let scripts = vec![
InsertSort { InsertSort {
field: text_field.clone(), field: text_field.clone(),