feat: enable sort by date + sort fix (#2953)

This commit is contained in:
Richard Shiue 2023-07-21 15:51:20 +08:00 committed by GitHub
parent b1378b4545
commit c368d855c7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 178 additions and 101 deletions

View File

@ -10,22 +10,22 @@ void main() {
group('grid', () {
testWidgets('add text sort', (tester) async {
await tester.openV020database();
// create a filter
// create a sort
await tester.tapDatabaseSortButton();
await tester.tapCreateSortByFieldType(FieldType.RichText, 'Name');
// check the text cell order
final textCells = <String>[
'',
'',
'',
'',
'',
'A',
'B',
'C',
'D',
'E',
'',
'',
'',
'',
'',
];
for (final (index, content) in textCells.indexed) {
await tester.assertCellContent(
@ -142,7 +142,6 @@ void main() {
// check the number cell order
for (final (index, content) in <String>[
'',
'-2',
'-1',
'0.1',
@ -152,6 +151,7 @@ void main() {
'10',
'11',
'12',
'',
].indexed) {
await tester.assertCellContent(
rowIndex: index,
@ -186,53 +186,74 @@ void main() {
await tester.pumpAndSettle();
});
testWidgets('add number and text sort', (tester) async {
testWidgets('add checkbox and number sort', (tester) async {
await tester.openV020database();
// create a filter
await tester.tapDatabaseSortButton();
await tester.tapCreateSortByFieldType(FieldType.Number, 'number');
await tester.tapCreateSortByFieldType(FieldType.Checkbox, 'Done');
// open the sort menu and select number order by descending
// open the sort menu and sort checkbox by descending
await tester.tapSortMenuInSettingBar();
await tester.tapSortButtonByName('number');
await tester.tapSortButtonByName('Done');
await tester.tapSortByDescending();
for (final (index, content) in <String>[
'12',
'11',
'10',
'2',
'1',
'0.2',
'0.1',
'-1',
'-2',
'',
for (final (index, content) in <bool>[
true,
true,
true,
true,
true,
false,
false,
false,
false,
false,
].indexed) {
await tester.assertCellContent(
await tester.assertCheckboxCell(
rowIndex: index,
fieldType: FieldType.Number,
content: content,
isSelected: content,
);
}
// add another sort, this time by number descending
await tester.tapSortMenuInSettingBar();
await tester.tapCreateSortByFieldTypeInSortMenu(
FieldType.RichText,
'Name',
FieldType.Number,
'number',
);
await tester.tapSortButtonByName('number');
await tester.tapSortByDescending();
// check checkbox cell order
for (final (index, content) in <bool>[
true,
true,
true,
true,
true,
false,
false,
false,
false,
false,
].indexed) {
await tester.assertCheckboxCell(
rowIndex: index,
isSelected: content,
);
}
// check number cell order
for (final (index, content) in <String>[
'1',
'0.2',
'0.1',
'-1',
'-2',
'12',
'11',
'10',
'2',
'',
'-1',
'-2',
'0.1',
'0.2',
'1',
].indexed) {
await tester.assertCellContent(
rowIndex: index,
@ -241,26 +262,6 @@ void main() {
);
}
// check text cell order
for (final (index, content) in <String>[
'',
'',
'',
'',
'',
'A',
'B',
'C',
'D',
'E',
].indexed) {
await tester.assertCellContent(
rowIndex: index,
fieldType: FieldType.RichText,
content: content,
);
}
await tester.pumpAndSettle();
});
});

View File

@ -790,6 +790,7 @@ class FieldInfo {
case FieldType.RichText:
case FieldType.Checkbox:
case FieldType.Number:
case FieldType.DateTime:
return true;
default:
return false;

View File

@ -143,4 +143,8 @@ impl TypeOptionCellDataCompare for CheckboxTypeOption {
(false, false) => default_order(),
}
}
fn exempt_from_cmp(&self, _: &<Self as TypeOption>::CellData) -> bool {
false
}
}

View File

@ -202,6 +202,10 @@ impl TypeOptionCellDataCompare for ChecklistTypeOption {
Ordering::Equal
}
}
fn exempt_from_cmp(&self, cell_data: &<Self as TypeOption>::CellData) -> bool {
cell_data.selected_option_ids.is_empty()
}
}
impl TypeOptionTransform for ChecklistTypeOption {}

View File

@ -302,4 +302,8 @@ impl TypeOptionCellDataCompare for DateTypeOption {
(None, None) => default_order(),
}
}
fn exempt_from_cmp(&self, cell_data: &<Self as TypeOption>::CellData) -> bool {
cell_data.timestamp.is_none()
}
}

View File

@ -260,6 +260,10 @@ impl TypeOptionCellDataCompare for NumberTypeOption {
(Err(_), Err(_)) => Ordering::Equal,
}
}
fn exempt_from_cmp(&self, cell_data: &<Self as TypeOption>::CellData) -> bool {
cell_data.0.is_empty()
}
}
impl std::default::Default for NumberTypeOption {
fn default() -> Self {

View File

@ -162,6 +162,10 @@ impl TypeOptionCellDataCompare for MultiSelectTypeOption {
}
default_order()
}
fn exempt_from_cmp(&self, cell_data: &<Self as TypeOption>::CellData) -> bool {
cell_data.is_empty()
}
}
#[cfg(test)]

View File

@ -146,6 +146,10 @@ impl TypeOptionCellDataCompare for SingleSelectTypeOption {
(None, None) => default_order(),
}
}
fn exempt_from_cmp(&self, cell_data: &<Self as TypeOption>::CellData) -> bool {
cell_data.is_empty()
}
}
#[cfg(test)]

View File

@ -155,6 +155,10 @@ impl TypeOptionCellDataCompare for RichTextTypeOption {
) -> Ordering {
cell_data.0.cmp(&other_cell_data.0)
}
fn exempt_from_cmp(&self, cell_data: &<Self as TypeOption>::CellData) -> bool {
cell_data.0.trim().is_empty()
}
}
#[derive(Clone)]

View File

@ -132,6 +132,8 @@ pub trait TypeOptionCellDataCompare: TypeOption {
cell_data: &<Self as TypeOption>::CellData,
other_cell_data: &<Self as TypeOption>::CellData,
) -> Ordering;
fn exempt_from_cmp(&self, cell_data: &<Self as TypeOption>::CellData) -> bool;
}
pub fn type_option_data_from_pb_or_default<T: Into<Bytes>>(

View File

@ -19,6 +19,7 @@ use crate::services::field::{
SingleSelectTypeOption, TypeOption, TypeOptionCellData, TypeOptionCellDataCompare,
TypeOptionCellDataFilter, TypeOptionTransform, URLTypeOption,
};
use crate::services::sort::SortCondition;
pub const CELL_DATA: &str = "data";
@ -45,7 +46,13 @@ pub trait TypeOptionCellDataHandler: Send + Sync + 'static {
field: &Field,
) -> FlowyResult<Cell>;
fn handle_cell_compare(&self, left_cell: &Cell, right_cell: &Cell, field: &Field) -> Ordering;
fn handle_cell_compare(
&self,
left_cell: &Cell,
right_cell: &Cell,
field: &Field,
sort_condition: SortCondition,
) -> Ordering;
fn handle_cell_filter(&self, field_type: &FieldType, field: &Field, cell: &Cell) -> bool;
@ -234,7 +241,13 @@ where
Ok(cell)
}
fn handle_cell_compare(&self, left_cell: &Cell, right_cell: &Cell, field: &Field) -> Ordering {
fn handle_cell_compare(
&self,
left_cell: &Cell,
right_cell: &Cell,
field: &Field,
sort_condition: SortCondition,
) -> Ordering {
let field_type = FieldType::from(field.field_type);
let left = self
.get_decoded_cell_data(left_cell, &field_type, field)
@ -242,7 +255,20 @@ where
let right = self
.get_decoded_cell_data(right_cell, &field_type, field)
.unwrap_or_default();
self.apply_cmp(&left, &right)
match (self.exempt_from_cmp(&left), self.exempt_from_cmp(&right)) {
(true, true) => Ordering::Equal,
(true, false) => Ordering::Greater,
(false, true) => Ordering::Less,
(false, false) => {
let order = self.apply_cmp(&left, &right);
// The order is calculated by Ascending. So reverse the order if the SortCondition is descending.
match sort_condition {
SortCondition::Ascending => order,
SortCondition::Descending => order.reverse(),
}
},
}
}
fn handle_cell_filter(&self, field_type: &FieldType, field: &Field, cell: &Cell) -> bool {

View File

@ -126,7 +126,12 @@ impl TypeOptionCellDataCompare for URLTypeOption {
) -> Ordering {
cell_data.data.cmp(&other_cell_data.data)
}
fn exempt_from_cmp(&self, cell_data: &<Self as TypeOption>::CellData) -> bool {
cell_data.data.is_empty()
}
}
fn auto_append_scheme(s: &str) -> String {
// Only support https scheme by now
match url::Url::parse(s) {

View File

@ -156,7 +156,7 @@ impl SortController {
}
let fields = self.delegate.get_fields(&self.view_id, None).await;
for sort in self.sorts.iter() {
for sort in self.sorts.iter().rev() {
rows
.par_sort_by(|left, right| cmp_row(&left.row, &right.row, sort, &fields, &self.cell_cache));
}
@ -240,35 +240,46 @@ fn cmp_row(
fields: &[Arc<Field>],
cell_data_cache: &CellCache,
) -> Ordering {
let order = match (
left.cells.get(&sort.field_id),
right.cells.get(&sort.field_id),
) {
(Some(left_cell), Some(right_cell)) => {
let field_type = sort.field_type.clone();
match fields
.iter()
.find(|field_rev| field_rev.id == sort.field_id)
{
None => default_order(),
Some(field_rev) => cmp_cell(
left_cell,
right_cell,
field_rev,
field_type,
cell_data_cache,
),
}
let field_type = sort.field_type.clone();
match fields
.iter()
.find(|field_rev| field_rev.id == sort.field_id)
{
None => default_order(),
Some(field_rev) => match (
left.cells.get(&sort.field_id),
right.cells.get(&sort.field_id),
) {
(Some(left_cell), Some(right_cell)) => cmp_cell(
left_cell,
right_cell,
field_rev,
field_type,
cell_data_cache,
sort.condition,
),
(Some(_), None) => {
if field_type.is_checkbox() {
match sort.condition {
SortCondition::Ascending => Ordering::Greater,
SortCondition::Descending => Ordering::Less,
}
} else {
Ordering::Less
}
},
(None, Some(_)) => {
if field_type.is_checkbox() {
match sort.condition {
SortCondition::Ascending => Ordering::Less,
SortCondition::Descending => Ordering::Greater,
}
} else {
Ordering::Greater
}
},
_ => default_order(),
},
(Some(_), None) => Ordering::Greater,
(None, Some(_)) => Ordering::Less,
_ => default_order(),
};
// The order is calculated by Ascending. So reverse the order if the SortCondition is descending.
match sort.condition {
SortCondition::Ascending => order,
SortCondition::Descending => order.reverse(),
}
}
@ -278,6 +289,7 @@ fn cmp_cell(
field: &Arc<Field>,
field_type: FieldType,
cell_data_cache: &CellCache,
sort_condition: SortCondition,
) -> Ordering {
match TypeOptionCellExt::new_with_cell_data_cache(field.as_ref(), Some(cell_data_cache.clone()))
.get_type_option_cell_data_handler(&field_type)
@ -285,7 +297,8 @@ fn cmp_cell(
None => default_order(),
Some(handler) => {
let cal_order = || {
let order = handler.handle_cell_compare(left_cell, right_cell, field.as_ref());
let order =
handler.handle_cell_compare(left_cell, right_cell, field.as_ref(), sort_condition);
Option::<Ordering>::Some(order)
};

View File

@ -18,7 +18,7 @@ async fn sort_checkbox_and_then_text_by_descending_test() {
field_id: text_field.id.clone(),
orders: vec!["A", "", "C", "DA", "AE", "AE"],
},
// // Insert checkbox sort
// Insert checkbox sort
InsertSort {
field: checkbox_field.clone(),
condition: SortCondition::Descending,
@ -31,10 +31,7 @@ async fn sort_checkbox_and_then_text_by_descending_test() {
field_id: text_field.id.clone(),
orders: vec!["A", "", "AE", "C", "DA", "AE"],
},
// Insert text sort. After inserting the text sort, the order of the rows
// will be changed.
// before: ["A", "", "AE", "C", "DA", "AE"]
// after: ["", "A", "AE", "AE", "C", "DA"]
// Insert text sort
InsertSort {
field: text_field.clone(),
condition: SortCondition::Ascending,
@ -45,7 +42,7 @@ async fn sort_checkbox_and_then_text_by_descending_test() {
},
AssertCellContentOrder {
field_id: text_field.id.clone(),
orders: vec!["", "A", "AE", "AE", "C", "DA"],
orders: vec!["A", "AE", "", "AE", "C", "DA"],
},
];
test.run_scripts(scripts).await;

View File

@ -16,7 +16,7 @@ async fn sort_text_with_checkbox_by_ascending_test() {
},
AssertCellContentOrder {
field_id: checkbox_field.id.clone(),
orders: vec!["Yes", "Yes", "No", "No", "No"],
orders: vec!["Yes", "Yes", "No", "No", "No", "Yes"],
},
InsertSort {
field: text_field.clone(),
@ -24,7 +24,11 @@ async fn sort_text_with_checkbox_by_ascending_test() {
},
AssertCellContentOrder {
field_id: text_field.id.clone(),
orders: vec!["", "A", "AE", "AE", "C", "DA"],
orders: vec!["A", "AE", "AE", "C", "DA", ""],
},
AssertCellContentOrder {
field_id: checkbox_field.id.clone(),
orders: vec!["Yes", "No", "Yes", "No", "No", "Yes"],
},
];
test.run_scripts(scripts).await;
@ -36,11 +40,11 @@ async fn sort_text_with_checkbox_by_ascending_test() {
},
AssertCellContentOrder {
field_id: text_field.id.clone(),
orders: vec!["", "A", "AE", "AE", "C", "DA"],
orders: vec!["A", "AE", "AE", "C", "DA", ""],
},
AssertCellContentOrder {
field_id: checkbox_field.id.clone(),
orders: vec!["Yes", "Yes", "Yes", "No", "No"],
orders: vec!["Yes", "Yes", "No", "No", "No", "Yes"],
},
];
test.run_scripts(scripts).await;

View File

@ -18,7 +18,7 @@ async fn sort_text_by_ascending_test() {
},
AssertCellContentOrder {
field_id: text_field.id.clone(),
orders: vec!["", "A", "AE", "AE", "C", "DA"],
orders: vec!["A", "AE", "AE", "C", "DA", ""],
},
];
test.run_scripts(scripts).await;
@ -35,7 +35,7 @@ async fn sort_change_notification_by_update_text_test() {
},
AssertCellContentOrder {
field_id: text_field.id.clone(),
orders: vec!["", "A", "AE", "AE", "C", "DA"],
orders: vec!["A", "AE", "AE", "C", "DA", ""],
},
// Wait the insert task to finish. The cost of time should be less than 200 milliseconds.
Wait { millis: 200 },
@ -45,12 +45,12 @@ async fn sort_change_notification_by_update_text_test() {
let row_details = test.get_rows().await;
let scripts = vec![
UpdateTextCell {
row_id: row_details[2].row.id.clone(),
row_id: row_details[1].row.id.clone(),
text: "E".to_string(),
},
AssertSortChanged {
old_row_orders: vec!["", "A", "E", "AE", "C", "DA"],
new_row_orders: vec!["", "A", "AE", "C", "DA", "E"],
old_row_orders: vec!["A", "E", "AE", "C", "DA", ""],
new_row_orders: vec!["A", "AE", "C", "DA", "E", ""],
},
];
test.run_scripts(scripts).await;
@ -263,7 +263,7 @@ async fn sort_multi_select_by_ascending_test() {
},
AssertCellContentOrder {
field_id: multi_select.id.clone(),
orders: vec!["", "", "", "Facebook", "Google,Facebook", "Google,Twitter"],
orders: vec!["Facebook", "Google,Facebook", "Google,Twitter", "", "", ""],
},
];
test.run_scripts(scripts).await;