feat: reorder sort precedence (#4592)

* feat: reorder sorts

* chore: add tests, fix tests, fix tauri build and fix clippy

* fix: add missing import
This commit is contained in:
Richard Shiue
2024-02-05 13:52:59 +08:00
committed by GitHub
parent fda70ff560
commit a515715543
28 changed files with 589 additions and 134 deletions

View File

@ -84,7 +84,7 @@ pub struct DeleteFilterPayloadPB {
pub field_type: FieldType,
#[pb(index = 3)]
#[validate(custom = "lib_infra::validator_fn::required_not_empty_str")]
#[validate(custom = "crate::entities::utils::validate_filter_id")]
pub filter_id: String,
#[pb(index = 4)]
@ -103,7 +103,7 @@ pub struct UpdateFilterPayloadPB {
/// Create a new filter if the filter_id is None
#[pb(index = 3, one_of)]
#[validate(custom = "lib_infra::validator_fn::required_not_empty_str")]
#[validate(custom = "crate::entities::utils::validate_filter_id")]
pub filter_id: Option<String>,
#[pb(index = 4)]

View File

@ -35,3 +35,25 @@ pub use share_entities::*;
pub use sort_entities::*;
pub use type_option_entities::*;
pub use view_entities::*;
mod utils {
use fancy_regex::Regex;
use lib_infra::impl_regex_validator;
use validator::ValidationError;
impl_regex_validator!(
validate_filter_id,
Regex::new(r"^[A-Za-z0-9_-]{6}$").unwrap(),
"invalid filter_id"
);
impl_regex_validator!(
validate_sort_id,
Regex::new(r"^s:[A-Za-z0-9_-]{6}$").unwrap(),
"invalid sort_id"
);
impl_regex_validator!(
validate_group_id,
Regex::new(r"^g:[A-Za-z0-9_-]{6}$").unwrap(),
"invalid group_id"
);
}

View File

@ -15,7 +15,7 @@ use crate::entities::{
};
use crate::services::setting::{BoardLayoutSetting, CalendarLayoutSetting};
use super::BoardLayoutSettingPB;
use super::{BoardLayoutSettingPB, ReorderSortPayloadPB};
/// [DatabaseViewSettingPB] defines the setting options for the grid. Such as the filter, group, and sort.
#[derive(Eq, PartialEq, ProtoBuf, Debug, Default, Clone)]
@ -39,9 +39,8 @@ pub struct DatabaseViewSettingPB {
pub field_settings: RepeatedFieldSettingsPB,
}
#[derive(Debug, Clone, PartialEq, Eq, ProtoBuf_Enum, EnumIter)]
#[derive(Debug, Default, Clone, PartialEq, Eq, ProtoBuf_Enum, EnumIter)]
#[repr(u8)]
#[derive(Default)]
pub enum DatabaseLayoutPB {
#[default]
Grid = 0,
@ -96,6 +95,10 @@ pub struct DatabaseSettingChangesetPB {
#[pb(index = 7, one_of)]
#[validate]
pub reorder_sort: Option<ReorderSortPayloadPB>,
#[pb(index = 8, one_of)]
#[validate]
pub delete_sort: Option<DeleteSortPayloadPB>,
}

View File

@ -41,6 +41,15 @@ impl std::convert::From<Sort> for SortPB {
}
}
#[derive(Eq, PartialEq, ProtoBuf, Debug, Default, Clone)]
pub struct SortWithIndexPB {
#[pb(index = 1)]
pub index: u32,
#[pb(index = 2)]
pub sort: SortPB,
}
#[derive(Eq, PartialEq, ProtoBuf, Debug, Default, Clone)]
pub struct RepeatedSortPB {
#[pb(index = 1)]
@ -105,12 +114,28 @@ pub struct UpdateSortPayloadPB {
/// Create a new sort if the sort_id is None
#[pb(index = 4, one_of)]
#[validate(custom = "super::utils::validate_sort_id")]
pub sort_id: Option<String>,
#[pb(index = 5)]
pub condition: SortConditionPB,
}
#[derive(Debug, Default, Clone, Validate, ProtoBuf)]
pub struct ReorderSortPayloadPB {
#[pb(index = 1)]
#[validate(custom = "lib_infra::validator_fn::required_not_empty_str")]
pub view_id: String,
#[pb(index = 2)]
#[validate(custom = "super::utils::validate_sort_id")]
pub from_sort_id: String,
#[pb(index = 3)]
#[validate(custom = "super::utils::validate_sort_id")]
pub to_sort_id: String,
}
#[derive(ProtoBuf, Debug, Default, Clone, Validate)]
pub struct DeleteSortPayloadPB {
#[pb(index = 1)]
@ -118,7 +143,7 @@ pub struct DeleteSortPayloadPB {
pub view_id: String,
#[pb(index = 2)]
#[validate(custom = "lib_infra::validator_fn::required_not_empty_str")]
#[validate(custom = "super::utils::validate_sort_id")]
pub sort_id: String,
}
@ -128,7 +153,7 @@ pub struct SortChangesetNotificationPB {
pub view_id: String,
#[pb(index = 2)]
pub insert_sorts: Vec<SortPB>,
pub insert_sorts: Vec<SortWithIndexPB>,
#[pb(index = 3)]
pub delete_sorts: Vec<SortPB>,

View File

@ -88,28 +88,32 @@ pub(crate) async fn update_database_setting_handler(
) -> Result<(), FlowyError> {
let manager = upgrade_manager(manager)?;
let params = data.try_into_inner()?;
let editor = manager.get_database_with_view_id(&params.view_id).await?;
let database_editor = manager.get_database_with_view_id(&params.view_id).await?;
if let Some(update_filter) = params.update_filter {
editor
database_editor
.create_or_update_filter(update_filter.try_into()?)
.await?;
}
if let Some(delete_filter) = params.delete_filter {
editor.delete_filter(delete_filter).await?;
database_editor.delete_filter(delete_filter).await?;
}
if let Some(update_sort) = params.update_sort {
let _ = editor.create_or_update_sort(update_sort).await?;
let _ = database_editor.create_or_update_sort(update_sort).await?;
}
if let Some(reorder_sort) = params.reorder_sort {
database_editor.reorder_sort(reorder_sort).await?;
}
if let Some(delete_sort) = params.delete_sort {
editor.delete_sort(delete_sort).await?;
database_editor.delete_sort(delete_sort).await?;
}
if let Some(layout_type) = params.layout_type {
editor
database_editor
.update_view_layout(&params.view_id, layout_type.into())
.await?;
}

View File

@ -228,10 +228,16 @@ impl DatabaseEditor {
pub async fn create_or_update_sort(&self, params: UpdateSortPayloadPB) -> FlowyResult<Sort> {
let view_editor = self.database_views.get_view_editor(&params.view_id).await?;
let sort = view_editor.insert_or_update_sort(params).await?;
let sort = view_editor.v_create_or_update_sort(params).await?;
Ok(sort)
}
pub async fn reorder_sort(&self, params: ReorderSortPayloadPB) -> FlowyResult<()> {
let view_editor = self.database_views.get_view_editor(&params.view_id).await?;
view_editor.v_reorder_sort(params).await?;
Ok(())
}
pub async fn delete_sort(&self, params: DeleteSortPayloadPB) -> FlowyResult<()> {
let view_editor = self.database_views.get_view_editor(&params.view_id).await?;
view_editor.v_delete_sort(params).await?;
@ -1459,6 +1465,13 @@ impl DatabaseViewOperation for DatabaseViewOperationImpl {
self.database.lock().insert_sort(view_id, sort);
}
fn move_sort(&self, view_id: &str, from_sort_id: &str, to_sort_id: &str) {
self
.database
.lock()
.move_sort(view_id, from_sort_id, to_sort_id);
}
fn remove_sort(&self, view_id: &str, sort_id: &str) {
self.database.lock().remove_sort(view_id, sort_id);
}

View File

@ -17,8 +17,8 @@ use lib_dispatch::prelude::af_spawn;
use crate::entities::{
CalendarEventPB, DatabaseLayoutMetaPB, DatabaseLayoutSettingPB, DeleteFilterPayloadPB,
DeleteSortPayloadPB, FieldType, FieldVisibility, GroupChangesPB, GroupPB, InsertedRowPB,
LayoutSettingChangeset, LayoutSettingParams, RemoveCalculationChangesetPB, RowMetaPB,
RowsChangePB, SortChangesetNotificationPB, SortPB, UpdateCalculationChangesetPB,
LayoutSettingChangeset, LayoutSettingParams, RemoveCalculationChangesetPB, ReorderSortPayloadPB,
RowMetaPB, RowsChangePB, SortChangesetNotificationPB, SortPB, UpdateCalculationChangesetPB,
UpdateFilterParams, UpdateSortPayloadPB,
};
use crate::notification::{send_notification, DatabaseNotification};
@ -499,7 +499,7 @@ impl DatabaseViewEditor {
}
#[tracing::instrument(level = "trace", skip(self), err)]
pub async fn insert_or_update_sort(&self, params: UpdateSortPayloadPB) -> FlowyResult<Sort> {
pub async fn v_create_or_update_sort(&self, params: UpdateSortPayloadPB) -> FlowyResult<Sort> {
let is_exist = params.sort_id.is_some();
let sort_id = match params.sort_id {
None => gen_database_sort_id(),
@ -513,9 +513,11 @@ impl DatabaseViewEditor {
condition: params.condition.into(),
};
let mut sort_controller = self.sort_controller.write().await;
self.delegate.insert_sort(&self.view_id, sort.clone());
let changeset = if is_exist {
let mut sort_controller = self.sort_controller.write().await;
let notification = if is_exist {
sort_controller
.apply_changeset(SortChangeset::from_update(sort.clone()))
.await
@ -525,10 +527,29 @@ impl DatabaseViewEditor {
.await
};
drop(sort_controller);
notify_did_update_sort(changeset).await;
notify_did_update_sort(notification).await;
Ok(sort)
}
pub async fn v_reorder_sort(&self, params: ReorderSortPayloadPB) -> FlowyResult<()> {
self
.delegate
.move_sort(&self.view_id, &params.from_sort_id, &params.to_sort_id);
let notification = self
.sort_controller
.write()
.await
.apply_changeset(SortChangeset::from_reorder(
params.from_sort_id,
params.to_sort_id,
))
.await;
notify_did_update_sort(notification).await;
Ok(())
}
pub async fn v_delete_sort(&self, params: DeleteSortPayloadPB) -> FlowyResult<()> {
let notification = self
.sort_controller

View File

@ -75,6 +75,8 @@ pub trait DatabaseViewOperation: Send + Sync + 'static {
fn insert_sort(&self, view_id: &str, sort: Sort);
fn move_sort(&self, view_id: &str, from_sort_id: &str, to_sort_id: &str);
fn remove_sort(&self, view_id: &str, sort_id: &str);
fn get_all_sorts(&self, view_id: &str) -> Vec<Sort>;

View File

@ -13,8 +13,8 @@ use flowy_error::FlowyResult;
use lib_infra::future::Fut;
use lib_infra::priority_task::{QualityOfService, Task, TaskContent, TaskDispatcher};
use crate::entities::FieldType;
use crate::entities::SortChangesetNotificationPB;
use crate::entities::{FieldType, SortWithIndexPB};
use crate::services::cell::CellCache;
use crate::services::database_view::{DatabaseViewChanged, DatabaseViewChangedNotifier};
use crate::services::field::{default_order, TypeOptionCellExt};
@ -184,7 +184,10 @@ impl SortController {
if let Some(insert_sort) = changeset.insert_sort {
if let Some(sort) = self.delegate.get_sort(&self.view_id, &insert_sort.id).await {
notification.insert_sorts.push(sort.as_ref().into());
notification.insert_sorts.push(SortWithIndexPB {
index: self.sorts.len() as u32,
sort: sort.as_ref().into(),
});
self.sorts.push(sort);
}
}
@ -209,6 +212,23 @@ impl SortController {
}
}
if let Some((from_id, to_id)) = changeset.reorder_sort {
let moved_sort = self.delegate.get_sort(&self.view_id, &from_id).await;
let from_index = self.sorts.iter().position(|sort| sort.id == from_id);
let to_index = self.sorts.iter().position(|sort| sort.id == to_id);
if let (Some(sort), Some(from_index), Some(to_index)) = (moved_sort, from_index, to_index) {
self.sorts.remove(from_index);
self.sorts.insert(to_index, sort.clone());
notification.delete_sorts.push(sort.as_ref().into());
notification.insert_sorts.push(SortWithIndexPB {
index: to_index as u32,
sort: sort.as_ref().into(),
});
}
}
if !notification.is_empty() {
self
.gen_task(SortEvent::SortDidChanged, QualityOfService::UserInteractive)

View File

@ -126,6 +126,7 @@ pub struct SortChangeset {
pub(crate) insert_sort: Option<Sort>,
pub(crate) update_sort: Option<Sort>,
pub(crate) delete_sort: Option<String>,
pub(crate) reorder_sort: Option<(String, String)>,
}
impl SortChangeset {
@ -134,6 +135,7 @@ impl SortChangeset {
insert_sort: Some(sort),
update_sort: None,
delete_sort: None,
reorder_sort: None,
}
}
@ -142,6 +144,7 @@ impl SortChangeset {
insert_sort: None,
update_sort: Some(sort),
delete_sort: None,
reorder_sort: None,
}
}
@ -150,6 +153,16 @@ impl SortChangeset {
insert_sort: None,
update_sort: None,
delete_sort: Some(sort_id),
reorder_sort: None,
}
}
pub fn from_reorder(from_sort_id: String, to_sort_id: String) -> Self {
Self {
insert_sort: None,
update_sort: None,
delete_sort: None,
reorder_sort: Some((from_sort_id, to_sort_id)),
}
}
}

View File

@ -47,3 +47,55 @@ async fn sort_checkbox_and_then_text_by_descending_test() {
];
test.run_scripts(scripts).await;
}
#[tokio::test]
async fn reorder_sort_test() {
let mut test = DatabaseSortTest::new().await;
let checkbox_field = test.get_first_field(FieldType::Checkbox);
let text_field = test.get_first_field(FieldType::RichText);
// Use the same sort set up as above
let scripts = vec![
AssertCellContentOrder {
field_id: checkbox_field.id.clone(),
orders: vec!["Yes", "Yes", "No", "No", "No", "Yes", ""],
},
AssertCellContentOrder {
field_id: text_field.id.clone(),
orders: vec!["A", "", "C", "DA", "AE", "AE", "CB"],
},
InsertSort {
field: checkbox_field.clone(),
condition: SortCondition::Descending,
},
InsertSort {
field: text_field.clone(),
condition: SortCondition::Ascending,
},
AssertCellContentOrder {
field_id: checkbox_field.id.clone(),
orders: vec!["Yes", "Yes", "Yes", "No", "No", "", "No"],
},
AssertCellContentOrder {
field_id: text_field.id.clone(),
orders: vec!["A", "AE", "", "AE", "C", "CB", "DA"],
},
];
test.run_scripts(scripts).await;
let sorts = test.editor.get_all_sorts(&test.view_id).await.items;
let scripts = vec![
ReorderSort {
from_sort_id: sorts[1].id.clone(),
to_sort_id: sorts[0].id.clone(),
},
AssertCellContentOrder {
field_id: checkbox_field.id.clone(),
orders: vec!["Yes", "Yes", "No", "No", "", "No", "Yes"],
},
AssertCellContentOrder {
field_id: text_field.id.clone(),
orders: vec!["A", "AE", "AE", "C", "CB", "DA", ""],
},
];
test.run_scripts(scripts).await;
}

View File

@ -7,10 +7,12 @@ use collab_database::rows::RowId;
use futures::stream::StreamExt;
use tokio::sync::broadcast::Receiver;
use flowy_database2::entities::{DeleteSortPayloadPB, FieldType, UpdateSortPayloadPB};
use flowy_database2::entities::{
DeleteSortPayloadPB, FieldType, ReorderSortPayloadPB, UpdateSortPayloadPB,
};
use flowy_database2::services::cell::stringify_cell_data;
use flowy_database2::services::database_view::DatabaseViewChanged;
use flowy_database2::services::sort::{Sort, SortCondition};
use flowy_database2::services::sort::SortCondition;
use crate::database::database_editor::DatabaseEditorTest;
@ -19,6 +21,10 @@ pub enum SortScript {
field: Field,
condition: SortCondition,
},
ReorderSort {
from_sort_id: String,
to_sort_id: String,
},
DeleteSort {
sort_id: String,
},
@ -41,7 +47,6 @@ pub enum SortScript {
pub struct DatabaseSortTest {
inner: DatabaseEditorTest,
pub current_sort_rev: Option<Sort>,
recv: Option<Receiver<DatabaseViewChanged>>,
}
@ -50,7 +55,6 @@ impl DatabaseSortTest {
let editor_test = DatabaseEditorTest::new_grid().await;
Self {
inner: editor_test,
current_sort_rev: None,
recv: None,
}
}
@ -77,8 +81,25 @@ impl DatabaseSortTest {
field_type: FieldType::from(field.field_type),
condition: condition.into(),
};
let sort_rev = self.editor.create_or_update_sort(params).await.unwrap();
self.current_sort_rev = Some(sort_rev);
let _ = self.editor.create_or_update_sort(params).await.unwrap();
},
SortScript::ReorderSort {
from_sort_id,
to_sort_id,
} => {
self.recv = Some(
self
.editor
.subscribe_view_changed(&self.view_id)
.await
.unwrap(),
);
let params = ReorderSortPayloadPB {
view_id: self.view_id.clone(),
from_sort_id,
to_sort_id,
};
self.editor.reorder_sort(params).await.unwrap();
},
SortScript::DeleteSort { sort_id } => {
self.recv = Some(
@ -93,7 +114,6 @@ impl DatabaseSortTest {
sort_id,
};
self.editor.delete_sort(params).await.unwrap();
self.current_sort_rev = None;
},
SortScript::AssertCellContentOrder { field_id, orders } => {
let mut cells = vec![];

View File

@ -85,16 +85,21 @@ async fn sort_change_notification_by_update_text_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 scripts = vec![InsertSort {
field: text_field.clone(),
condition: SortCondition::Ascending,
}];
test.run_scripts(scripts).await;
let sort = test.current_sort_rev.as_ref().unwrap();
let scripts = vec![
DeleteSort {
sort_id: sort.id.clone(),
InsertSort {
field: text_field.clone(),
condition: SortCondition::Ascending,
},
AssertCellContentOrder {
field_id: text_field.id.clone(),
orders: vec!["A", "AE", "AE", "C", "CB", "DA", ""],
},
];
test.run_scripts(scripts).await;
let sort = test.editor.get_all_sorts(&test.view_id).await.items[0].clone();
let scripts = vec![
DeleteSort { sort_id: sort.id },
AssertCellContentOrder {
field_id: text_field.id.clone(),
orders: vec!["A", "", "C", "DA", "AE", "AE", "CB"],