fix(flutter-desktop): launch review issues for 0.7.2 (#6577)

* test: add test case for calendar filter

* fix: select option filter logic

* fix: calendar filters not applying if switched repeatedly

* fix: checklist cell editor improvements

* fix: nested scrolling in checklist

* fix: make filter logic match notion

* test: fix checklist cell test

* test: single select filter tests

* test: fix flutter test

* test: fix rust tests

* chore: fix clippy
This commit is contained in:
Richard Shiue 2024-10-18 13:48:50 +08:00 committed by GitHub
parent 42e55fe248
commit bd46fc11f4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
20 changed files with 106 additions and 261 deletions

View File

@ -299,6 +299,7 @@ void main() {
await tester.tapButton(finderForFieldType(FieldType.MultiSelect)); await tester.tapButton(finderForFieldType(FieldType.MultiSelect));
await tester.createOption(name: "asdf"); await tester.createOption(name: "asdf");
await tester.createOption(name: "qwer"); await tester.createOption(name: "qwer");
await tester.selectOption(name: "asdf");
await tester.dismissCellEditor(); await tester.dismissCellEditor();
await tester.tapDatabaseFilterButton(); await tester.tapDatabaseFilterButton();
@ -308,19 +309,31 @@ void main() {
await tester.tapOptionFilterWithName('asdf'); await tester.tapOptionFilterWithName('asdf');
await tester.dismissCellEditor(); await tester.dismissCellEditor();
tester.assertNumberOfEventsInCalendar(0);
await tester.tapFilterButtonInGrid('Tags');
await tester.tapOptionFilterWithName('asdf');
await tester.dismissCellEditor();
tester.assertNumberOfEventsInCalendar(1); tester.assertNumberOfEventsInCalendar(1);
await tester.tapFilterButtonInGrid('Tags');
await tester.tapOptionFilterWithName('asdf');
await tester.dismissCellEditor();
tester.assertNumberOfEventsInCalendar(0);
final secondOfThisMonth = DateTime(today.year, today.month, 2); final secondOfThisMonth = DateTime(today.year, today.month, 2);
await tester.doubleClickCalendarCell(secondOfThisMonth); await tester.doubleClickCalendarCell(secondOfThisMonth);
await tester.dismissEventEditor(); await tester.dismissEventEditor();
tester.assertNumberOfEventsInCalendar(2); tester.assertNumberOfEventsInCalendar(1);
await tester.openCalendarEvent(index: 0, date: secondOfThisMonth); await tester.openCalendarEvent(index: 0, date: secondOfThisMonth);
await tester.tapButton(finderForFieldType(FieldType.MultiSelect)); await tester.tapButton(finderForFieldType(FieldType.MultiSelect));
await tester.selectOption(name: "asdf"); await tester.selectOption(name: "asdf");
await tester.dismissCellEditor(); await tester.dismissCellEditor();
tester.assertNumberOfEventsInCalendar(1); tester.assertNumberOfEventsInCalendar(0);
await tester.tapFilterButtonInGrid('Tags'); await tester.tapFilterButtonInGrid('Tags');
await tester.changeSelectFilterCondition( await tester.changeSelectFilterCondition(

View File

@ -108,7 +108,7 @@ void main() {
await tester.tapOptionFilterWithName('s4'); await tester.tapOptionFilterWithName('s4');
// The row with 's4' should be shown. // The row with 's4' should be shown.
tester.assertNumberOfRowsInGridPage(1); tester.assertNumberOfRowsInGridPage(2);
await tester.pumpAndSettle(); await tester.pumpAndSettle();
}); });

View File

@ -539,14 +539,15 @@ extension AppFlowyDatabaseTest on WidgetTester {
Future<void> deleteChecklistTask({required int index}) async { Future<void> deleteChecklistTask({required int index}) async {
final task = find.byType(ChecklistItem).at(index); final task = find.byType(ChecklistItem).at(index);
await startGesture(getCenter(task), kind: PointerDeviceKind.mouse); await hoverOnWidget(
await pumpAndSettle(); task,
onHover: () async {
final button = find.byWidgetPredicate( final button = find.byWidgetPredicate(
(widget) => widget is FlowySvg && widget.svg == FlowySvgs.delete_s, (widget) => widget is FlowySvg && widget.svg == FlowySvgs.delete_s,
);
await tapButton(button);
},
); );
await tapButton(button);
} }
void assertPhantomChecklistItemAtIndex({required int index}) { void assertPhantomChecklistItemAtIndex({required int index}) {

View File

@ -1003,7 +1003,7 @@ class _SelectOptionFilterContentEditorState
isSelected, isSelected,
); );
}, },
indicator: _getIndicator(), indicator: MobileSelectedOptionIndicator.multi,
showMoreOptionsButton: false, showMoreOptionsButton: false,
); );
}, },
@ -1027,35 +1027,19 @@ class _SelectOptionFilterContentEditorState
} }
} }
MobileSelectedOptionIndicator _getIndicator() {
return (widget.filter.condition == SelectOptionFilterConditionPB.OptionIs ||
widget.filter.condition ==
SelectOptionFilterConditionPB.OptionIsNot) &&
widget.field.fieldType == FieldType.SingleSelect
? MobileSelectedOptionIndicator.single
: MobileSelectedOptionIndicator.multi;
}
void _onTapHandler( void _onTapHandler(
BuildContext context, BuildContext context,
List<SelectOptionPB> options, List<SelectOptionPB> options,
SelectOptionPB option, SelectOptionPB option,
bool isSelected, bool isSelected,
) { ) {
final selectedOptionIds = Set<String>.from(widget.filter.optionIds);
if (isSelected) { if (isSelected) {
final selectedOptionIds = Set<String>.from(widget.filter.optionIds) selectedOptionIds.remove(option.id);
..remove(option.id);
_updateSelectOptions(context, options, selectedOptionIds);
} else { } else {
final selectedOptionIds = widget.delegate.selectOption( selectedOptionIds.add(option.id);
widget.filter.optionIds,
option.id,
widget.filter.condition,
);
_updateSelectOptions(context, options, selectedOptionIds);
} }
_updateSelectOptions(context, options, selectedOptionIds);
} }
void _updateSelectOptions( void _updateSelectOptions(

View File

@ -93,8 +93,6 @@ final class SingleSelectOptionFilterCondition
return [ return [
SelectOptionFilterConditionPB.OptionIs, SelectOptionFilterConditionPB.OptionIs,
SelectOptionFilterConditionPB.OptionIsNot, SelectOptionFilterConditionPB.OptionIsNot,
SelectOptionFilterConditionPB.OptionContains,
SelectOptionFilterConditionPB.OptionDoesNotContain,
SelectOptionFilterConditionPB.OptionIsEmpty, SelectOptionFilterConditionPB.OptionIsEmpty,
SelectOptionFilterConditionPB.OptionIsNotEmpty, SelectOptionFilterConditionPB.OptionIsNotEmpty,
].map((e) => (e, e.i18n)).toList(); ].map((e) => (e, e.i18n)).toList();
@ -109,8 +107,6 @@ final class MultiSelectOptionFilterCondition
return [ return [
SelectOptionFilterConditionPB.OptionContains, SelectOptionFilterConditionPB.OptionContains,
SelectOptionFilterConditionPB.OptionDoesNotContain, SelectOptionFilterConditionPB.OptionDoesNotContain,
SelectOptionFilterConditionPB.OptionIs,
SelectOptionFilterConditionPB.OptionIsNot,
SelectOptionFilterConditionPB.OptionIsEmpty, SelectOptionFilterConditionPB.OptionIsEmpty,
SelectOptionFilterConditionPB.OptionIsNotEmpty, SelectOptionFilterConditionPB.OptionIsNotEmpty,
].map((e) => (e, e.i18n)).toList(); ].map((e) => (e, e.i18n)).toList();

View File

@ -353,14 +353,7 @@ final class SelectOptionFilter extends DatabaseFilter {
required List<String> optionIds, required List<String> optionIds,
}) { }) {
if (canAttachContent) { if (canAttachContent) {
if (fieldType == FieldType.SingleSelect && this.optionIds.addAll(optionIds);
(condition == SelectOptionFilterConditionPB.OptionIs ||
condition == SelectOptionFilterConditionPB.OptionIsNot) &&
optionIds.isNotEmpty) {
this.optionIds.add(optionIds.first);
} else {
this.optionIds.addAll(optionIds);
}
} }
} }
@ -377,7 +370,7 @@ final class SelectOptionFilter extends DatabaseFilter {
@override @override
String getContentDescription(FieldInfo field) { String getContentDescription(FieldInfo field) {
if (!canAttachContent) { if (!canAttachContent || optionIds.isEmpty) {
return condition.i18n; return condition.i18n;
} }
@ -436,13 +429,6 @@ final class SelectOptionFilter extends DatabaseFilter {
SelectOptionFilterConditionPB? condition, SelectOptionFilterConditionPB? condition,
List<String>? optionIds, List<String>? optionIds,
}) { }) {
final options = optionIds ?? this.optionIds;
if (fieldType == FieldType.SingleSelect &&
(condition == SelectOptionFilterConditionPB.OptionIs ||
condition == SelectOptionFilterConditionPB.OptionIsNot) &&
options.length > 1) {
options.removeRange(1, options.length);
}
return SelectOptionFilter( return SelectOptionFilter(
filterId: filterId, filterId: filterId,
fieldId: fieldId, fieldId: fieldId,

View File

@ -122,6 +122,7 @@ class CalendarBloc extends Bloc<CalendarEvent, CalendarState> {
deleteEventIds: deletedRowIds, deleteEventIds: deletedRowIds,
), ),
); );
emit(state.copyWith(deleteEventIds: const []));
}, },
didReceiveEvent: (CalendarEventData<CalendarDayEvent> event) { didReceiveEvent: (CalendarEventData<CalendarDayEvent> event) {
emit( emit(
@ -130,20 +131,11 @@ class CalendarBloc extends Bloc<CalendarEvent, CalendarState> {
newEvent: event, newEvent: event,
), ),
); );
emit(state.copyWith(newEvent: null));
}, },
openRowDetail: (row) { openRowDetail: (row) {
emit( emit(state.copyWith(openRow: row));
state.copyWith( emit(state.copyWith(openRow: null));
openRow: row,
),
);
},
resetOpenRowDetail: () {
emit(
state.copyWith(
openRow: null,
),
);
}, },
); );
}, },
@ -483,8 +475,6 @@ class CalendarEvent with _$CalendarEvent {
_DeleteEvent; _DeleteEvent;
const factory CalendarEvent.openRowDetail(RowMetaPB row) = _OpenRowDetail; const factory CalendarEvent.openRowDetail(RowMetaPB row) = _OpenRowDetail;
const factory CalendarEvent.resetOpenRowDetail() = _ResetRowDetail;
} }
@freezed @freezed

View File

@ -177,7 +177,6 @@ class _CalendarPageState extends State<CalendarPage> {
databaseController: _calendarBloc.databaseController, databaseController: _calendarBloc.databaseController,
rowMeta: state.openRow!, rowMeta: state.openRow!,
); );
_calendarBloc.add(const CalendarEvent.resetOpenRowDetail());
} }
}, },
), ),

View File

@ -6,12 +6,6 @@ abstract class SelectOptionFilterDelegate {
const SelectOptionFilterDelegate(); const SelectOptionFilterDelegate();
List<SelectOptionPB> getOptions(FieldInfo fieldInfo); List<SelectOptionPB> getOptions(FieldInfo fieldInfo);
Set<String> selectOption(
List<String> currentOptionIds,
String optionId,
SelectOptionFilterConditionPB condition,
);
} }
class SingleSelectOptionFilterDelegateImpl class SingleSelectOptionFilterDelegateImpl
@ -23,37 +17,6 @@ class SingleSelectOptionFilterDelegateImpl
final parser = SingleSelectTypeOptionDataParser(); final parser = SingleSelectTypeOptionDataParser();
return parser.fromBuffer(fieldInfo.field.typeOptionData).options; return parser.fromBuffer(fieldInfo.field.typeOptionData).options;
} }
@override
Set<String> selectOption(
List<String> currentOptionIds,
String optionId,
SelectOptionFilterConditionPB condition,
) {
final selectOptionIds = Set<String>.from(currentOptionIds);
switch (condition) {
case SelectOptionFilterConditionPB.OptionIs:
case SelectOptionFilterConditionPB.OptionIsNot:
if (selectOptionIds.isNotEmpty) {
selectOptionIds.clear();
}
selectOptionIds.add(optionId);
break;
case SelectOptionFilterConditionPB.OptionContains:
case SelectOptionFilterConditionPB.OptionDoesNotContain:
selectOptionIds.add(optionId);
break;
case SelectOptionFilterConditionPB.OptionIsEmpty ||
SelectOptionFilterConditionPB.OptionIsNotEmpty:
selectOptionIds.clear();
break;
default:
throw UnimplementedError();
}
return selectOptionIds;
}
} }
class MultiSelectOptionFilterDelegateImpl class MultiSelectOptionFilterDelegateImpl
@ -66,12 +29,4 @@ class MultiSelectOptionFilterDelegateImpl
.fromBuffer(fieldInfo.field.typeOptionData) .fromBuffer(fieldInfo.field.typeOptionData)
.options; .options;
} }
@override
Set<String> selectOption(
List<String> currentOptionIds,
String optionId,
SelectOptionFilterConditionPB condition,
) =>
Set<String>.from(currentOptionIds)..add(optionId);
} }

View File

@ -25,12 +25,14 @@ class SelectOptionFilterConditionList extends StatelessWidget {
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
final conditions = (fieldType == FieldType.SingleSelect
? SingleSelectOptionFilterCondition().conditions
: MultiSelectOptionFilterCondition().conditions);
return PopoverActionList<ConditionWrapper>( return PopoverActionList<ConditionWrapper>(
asBarrier: true, asBarrier: true,
mutex: popoverMutex, mutex: popoverMutex,
direction: PopoverDirection.bottomWithCenterAligned, direction: PopoverDirection.bottomWithCenterAligned,
actions: SingleSelectOptionFilterCondition() actions: conditions
.conditions
.map( .map(
(action) => ConditionWrapper( (action) => ConditionWrapper(
action.$1, action.$1,

View File

@ -2,7 +2,6 @@ import 'package:appflowy/generated/flowy_svgs.g.dart';
import 'package:appflowy/plugins/database/application/field/field_info.dart'; import 'package:appflowy/plugins/database/application/field/field_info.dart';
import 'package:appflowy/plugins/database/application/field/filter_entities.dart'; import 'package:appflowy/plugins/database/application/field/filter_entities.dart';
import 'package:appflowy/plugins/database/grid/application/filter/filter_editor_bloc.dart'; import 'package:appflowy/plugins/database/grid/application/filter/filter_editor_bloc.dart';
import 'package:appflowy/plugins/database/grid/application/filter/select_option_loader.dart';
import 'package:appflowy/plugins/database/grid/presentation/layout/sizes.dart'; import 'package:appflowy/plugins/database/grid/presentation/layout/sizes.dart';
import 'package:appflowy/plugins/database/widgets/cell_editor/select_option_cell_editor.dart'; import 'package:appflowy/plugins/database/widgets/cell_editor/select_option_cell_editor.dart';
import 'package:appflowy_backend/protobuf/flowy-database2/select_option_entities.pb.dart'; import 'package:appflowy_backend/protobuf/flowy-database2/select_option_entities.pb.dart';
@ -17,14 +16,12 @@ class SelectOptionFilterList extends StatelessWidget {
super.key, super.key,
required this.filter, required this.filter,
required this.field, required this.field,
required this.delegate,
required this.options, required this.options,
required this.onTap, required this.onTap,
}); });
final SelectOptionFilter filter; final SelectOptionFilter filter;
final FieldInfo field; final FieldInfo field;
final SelectOptionFilterDelegate delegate;
final List<SelectOptionPB> options; final List<SelectOptionPB> options;
final VoidCallback onTap; final VoidCallback onTap;
@ -53,20 +50,13 @@ class SelectOptionFilterList extends StatelessWidget {
SelectOptionPB option, SelectOptionPB option,
bool isSelected, bool isSelected,
) { ) {
final selectedOptionIds = Set<String>.from(filter.optionIds);
if (isSelected) { if (isSelected) {
final selectedOptionIds = Set<String>.from(filter.optionIds) selectedOptionIds.remove(option.id);
..remove(option.id);
_updateSelectOptions(context, filter, selectedOptionIds);
} else { } else {
final selectedOptionIds = delegate.selectOption( selectedOptionIds.add(option.id);
filter.optionIds,
option.id,
filter.condition,
);
_updateSelectOptions(context, filter, selectedOptionIds);
} }
_updateSelectOptions(context, filter, selectedOptionIds);
onTap(); onTap();
} }

View File

@ -1,7 +1,6 @@
import 'package:appflowy/plugins/database/application/field/field_info.dart'; import 'package:appflowy/plugins/database/application/field/field_info.dart';
import 'package:appflowy/plugins/database/application/field/filter_entities.dart'; import 'package:appflowy/plugins/database/application/field/filter_entities.dart';
import 'package:appflowy/plugins/database/grid/application/filter/filter_editor_bloc.dart'; import 'package:appflowy/plugins/database/grid/application/filter/filter_editor_bloc.dart';
import 'package:appflowy_backend/protobuf/flowy-database2/protobuf.dart';
import 'package:appflowy_popover/appflowy_popover.dart'; import 'package:appflowy_popover/appflowy_popover.dart';
import 'package:flowy_infra_ui/flowy_infra_ui.dart'; import 'package:flowy_infra_ui/flowy_infra_ui.dart';
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
@ -76,11 +75,7 @@ class _SelectOptionFilterEditorState extends State<SelectOptionFilterEditor> {
SliverToBoxAdapter(child: _buildFilterPanel(filter, field)), SliverToBoxAdapter(child: _buildFilterPanel(filter, field)),
]; ];
if (![ if (filter.canAttachContent) {
SelectOptionFilterConditionPB.OptionIsEmpty,
SelectOptionFilterConditionPB.OptionIsNotEmpty,
].contains(filter.condition)) {
final delegate = filter.makeDelegate(field);
slivers slivers
..add(const SliverToBoxAdapter(child: VSpace(4))) ..add(const SliverToBoxAdapter(child: VSpace(4)))
..add( ..add(
@ -88,8 +83,7 @@ class _SelectOptionFilterEditorState extends State<SelectOptionFilterEditor> {
child: SelectOptionFilterList( child: SelectOptionFilterList(
filter: filter, filter: filter,
field: field, field: field,
delegate: delegate, options: filter.makeDelegate(field).getOptions(field),
options: delegate.getOptions(field),
onTap: () => popoverMutex.close(), onTap: () => popoverMutex.close(),
), ),
), ),

View File

@ -178,6 +178,7 @@ class _ChecklistItems extends StatelessWidget {
final children = _makeChildren(context, state); final children = _makeChildren(context, state);
return ReorderableListView.builder( return ReorderableListView.builder(
shrinkWrap: true, shrinkWrap: true,
physics: const NeverScrollableScrollPhysics(),
proxyDecorator: (child, index, _) => Material( proxyDecorator: (child, index, _) => Material(
color: Colors.transparent, color: Colors.transparent,
child: Stack( child: Stack(

View File

@ -235,9 +235,8 @@ class _ChecklistItemState extends State<ChecklistItem> {
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
final isFocusedOrHovered = final isFocusedOrHovered = isHovered || isFocused;
isHovered || isFocused || textFieldFocusNode.hasFocus; final color = isFocusedOrHovered || textFieldFocusNode.hasFocus
final color = isFocusedOrHovered
? AFThemeExtension.of(context).lightGreyHover ? AFThemeExtension.of(context).lightGreyHover
: Colors.transparent; : Colors.transparent;
return FocusableActionDetector( return FocusableActionDetector(
@ -259,14 +258,15 @@ class _ChecklistItemState extends State<ChecklistItem> {
borderRadius: Corners.s6Border, borderRadius: Corners.s6Border,
), ),
child: _buildChild( child: _buildChild(
isFocusedOrHovered, isFocusedOrHovered && !textFieldFocusNode.hasFocus,
), ),
), ),
); );
} }
Widget _buildChild(bool isFocusedOrHovered) { Widget _buildChild(bool showTrash) {
return Row( return Row(
crossAxisAlignment: CrossAxisAlignment.start,
children: [ children: [
ReorderableDragStartListener( ReorderableDragStartListener(
index: widget.index, index: widget.index,
@ -313,7 +313,7 @@ class _ChecklistItemState extends State<ChecklistItem> {
}, },
), ),
), ),
if (isFocusedOrHovered) if (showTrash)
ChecklistCellDeleteButton( ChecklistCellDeleteButton(
onPressed: () => context.read<ChecklistCellBloc>().add( onPressed: () => context.read<ChecklistCellBloc>().add(
ChecklistCellEvent.deleteTask(widget.task.data.id), ChecklistCellEvent.deleteTask(widget.task.data.id),

View File

@ -242,7 +242,7 @@ void main() {
createFilterPB( createFilterPB(
FieldType.SingleSelect, FieldType.SingleSelect,
SelectOptionFilterPB( SelectOptionFilterPB(
condition: SelectOptionFilterConditionPB.OptionContains, condition: SelectOptionFilterConditionPB.OptionIs,
optionIds: [], optionIds: [],
).writeToBuffer(), ).writeToBuffer(),
), ),
@ -252,7 +252,7 @@ void main() {
filterId: "FT", filterId: "FT",
fieldId: "FD", fieldId: "FD",
fieldType: FieldType.SingleSelect, fieldType: FieldType.SingleSelect,
condition: SelectOptionFilterConditionPB.OptionContains, condition: SelectOptionFilterConditionPB.OptionIs,
optionIds: const [], optionIds: const [],
), ),
), ),
@ -263,7 +263,7 @@ void main() {
createFilterPB( createFilterPB(
FieldType.SingleSelect, FieldType.SingleSelect,
SelectOptionFilterPB( SelectOptionFilterPB(
condition: SelectOptionFilterConditionPB.OptionContains, condition: SelectOptionFilterConditionPB.OptionIs,
optionIds: ['a'], optionIds: ['a'],
).writeToBuffer(), ).writeToBuffer(),
), ),
@ -273,33 +273,12 @@ void main() {
filterId: "FT", filterId: "FT",
fieldId: "FD", fieldId: "FD",
fieldType: FieldType.SingleSelect, fieldType: FieldType.SingleSelect,
condition: SelectOptionFilterConditionPB.OptionContains, condition: SelectOptionFilterConditionPB.OptionIs,
optionIds: const ['a'], optionIds: const ['a'],
), ),
), ),
); );
expect(
DatabaseFilter.fromPB(
createFilterPB(
FieldType.SingleSelect,
SelectOptionFilterPB(
condition: SelectOptionFilterConditionPB.OptionContains,
optionIds: ['a', 'b'],
).writeToBuffer(),
),
),
equals(
SelectOptionFilter(
filterId: "FT",
fieldId: "FD",
fieldType: FieldType.SingleSelect,
condition: SelectOptionFilterConditionPB.OptionContains,
optionIds: const ['a', 'b'],
),
),
);
expect( expect(
DatabaseFilter.fromPB( DatabaseFilter.fromPB(
createFilterPB( createFilterPB(
@ -316,7 +295,7 @@ void main() {
fieldId: "FD", fieldId: "FD",
fieldType: FieldType.SingleSelect, fieldType: FieldType.SingleSelect,
condition: SelectOptionFilterConditionPB.OptionIs, condition: SelectOptionFilterConditionPB.OptionIs,
optionIds: const ['a'], optionIds: const ['a', 'b'],
), ),
), ),
); );

View File

@ -60,8 +60,26 @@ impl ParseFilterData for SelectOptionFilterPB {
} }
impl SelectOptionFilterPB { impl SelectOptionFilterPB {
pub fn remove_extra_option_ids(mut self) -> Self { pub fn to_single_select_filter(mut self) -> Self {
self.option_ids.truncate(1); match self.condition {
SelectOptionFilterConditionPB::OptionContains
| SelectOptionFilterConditionPB::OptionDoesNotContain => {
self.condition = SelectOptionFilterConditionPB::OptionIs;
},
_ => {},
}
self
}
pub fn to_multi_select_filter(mut self) -> Self {
match self.condition {
SelectOptionFilterConditionPB::OptionIs | SelectOptionFilterConditionPB::OptionIsNot => {
self.condition = SelectOptionFilterConditionPB::OptionContains;
},
_ => {},
}
self self
} }
} }

View File

@ -55,16 +55,14 @@ impl SelectOptionFilterStrategy {
return false; return false;
} }
selected_option_ids.len() == option_ids.len() selected_option_ids.iter().all(|id| option_ids.contains(id))
&& selected_option_ids.iter().all(|id| option_ids.contains(id))
}, },
SelectOptionFilterStrategy::IsNot(option_ids) => { SelectOptionFilterStrategy::IsNot(option_ids) => {
if selected_option_ids.is_empty() { if selected_option_ids.is_empty() {
return true; return true;
} }
selected_option_ids.len() != option_ids.len() !selected_option_ids.iter().all(|id| option_ids.contains(id))
|| !selected_option_ids.iter().all(|id| option_ids.contains(id))
}, },
SelectOptionFilterStrategy::Contains(option_ids) => { SelectOptionFilterStrategy::Contains(option_ids) => {
if selected_option_ids.is_empty() { if selected_option_ids.is_empty() {
@ -98,18 +96,9 @@ impl SelectOptionFilterStrategy {
impl PreFillCellsWithFilter for SelectOptionFilterPB { impl PreFillCellsWithFilter for SelectOptionFilterPB {
fn get_compliant_cell(&self, field: &Field) -> Option<Cell> { fn get_compliant_cell(&self, field: &Field) -> Option<Cell> {
let get_non_empty_expected_options = || {
if !self.option_ids.is_empty() {
Some(self.option_ids.clone())
} else {
None
}
};
let option_ids = match self.condition { let option_ids = match self.condition {
SelectOptionFilterConditionPB::OptionIs => get_non_empty_expected_options(), SelectOptionFilterConditionPB::OptionIs | SelectOptionFilterConditionPB::OptionContains => {
SelectOptionFilterConditionPB::OptionContains => { self.option_ids.first().map(|id| vec![id.clone()])
get_non_empty_expected_options().map(|mut options| vec![options.swap_remove(0)])
}, },
SelectOptionFilterConditionPB::OptionIsNotEmpty => select_type_option_from_field(field) SelectOptionFilterConditionPB::OptionIsNotEmpty => select_type_option_from_field(field)
.ok() .ok()
@ -127,6 +116,7 @@ impl PreFillCellsWithFilter for SelectOptionFilterPB {
option_ids.map(|ids| insert_select_option_cell(ids, field)) option_ids.map(|ids| insert_select_option_cell(ids, field))
} }
} }
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use crate::entities::{SelectOptionFilterConditionPB, SelectOptionFilterPB}; use crate::entities::{SelectOptionFilterConditionPB, SelectOptionFilterPB};
@ -150,11 +140,12 @@ mod tests {
let option_2 = SelectOption::new("B"); let option_2 = SelectOption::new("B");
let filter = SelectOptionFilterPB { let filter = SelectOptionFilterPB {
condition: SelectOptionFilterConditionPB::OptionIsNotEmpty, condition: SelectOptionFilterConditionPB::OptionIsNotEmpty,
option_ids: vec![option_1.id.clone(), option_2.id.clone()], option_ids: vec![],
}; };
assert_eq!(filter.is_visible(&[]), Some(false)); assert_eq!(filter.is_visible(&[]), Some(false));
assert_eq!(filter.is_visible(&[option_1.clone()]), Some(true)); assert_eq!(filter.is_visible(&[option_1.clone()]), Some(true));
assert_eq!(filter.is_visible(&[option_1, option_2]), Some(true));
} }
#[test] #[test]
@ -185,8 +176,6 @@ mod tests {
(vec![], Some(false)), (vec![], Some(false)),
(vec![option_1.clone()], Some(true)), (vec![option_1.clone()], Some(true)),
(vec![option_2.clone()], Some(false)), (vec![option_2.clone()], Some(false)),
(vec![option_3.clone()], Some(false)),
(vec![option_1.clone(), option_2.clone()], Some(false)),
] { ] {
assert_eq!(filter.is_visible(&options), is_visible); assert_eq!(filter.is_visible(&options), is_visible);
} }
@ -198,12 +187,9 @@ mod tests {
}; };
for (options, is_visible) in [ for (options, is_visible) in [
(vec![], Some(false)), (vec![], Some(false)),
(vec![option_1.clone()], Some(false)), (vec![option_1.clone()], Some(true)),
(vec![option_1.clone(), option_2.clone()], Some(true)), (vec![option_2.clone()], Some(true)),
( (vec![option_3.clone()], Some(false)),
vec![option_1.clone(), option_2.clone(), option_3.clone()],
Some(false),
),
] { ] {
assert_eq!(filter.is_visible(&options), is_visible); assert_eq!(filter.is_visible(&options), is_visible);
} }
@ -223,7 +209,7 @@ mod tests {
for (options, is_visible) in [ for (options, is_visible) in [
(vec![], None), (vec![], None),
(vec![option_1.clone()], None), (vec![option_1.clone()], None),
(vec![option_1.clone(), option_2.clone()], None), (vec![option_2.clone()], None),
] { ] {
assert_eq!(filter.is_visible(&options), is_visible); assert_eq!(filter.is_visible(&options), is_visible);
} }
@ -238,7 +224,6 @@ mod tests {
(vec![option_1.clone()], Some(false)), (vec![option_1.clone()], Some(false)),
(vec![option_2.clone()], Some(true)), (vec![option_2.clone()], Some(true)),
(vec![option_3.clone()], Some(true)), (vec![option_3.clone()], Some(true)),
(vec![option_1.clone(), option_2.clone()], Some(true)),
] { ] {
assert_eq!(filter.is_visible(&options), is_visible); assert_eq!(filter.is_visible(&options), is_visible);
} }
@ -250,12 +235,9 @@ mod tests {
}; };
for (options, is_visible) in [ for (options, is_visible) in [
(vec![], Some(true)), (vec![], Some(true)),
(vec![option_1.clone()], Some(true)), (vec![option_1.clone()], Some(false)),
(vec![option_1.clone(), option_2.clone()], Some(false)), (vec![option_2.clone()], Some(false)),
( (vec![option_3.clone()], Some(true)),
vec![option_1.clone(), option_2.clone(), option_3.clone()],
Some(true),
),
] { ] {
assert_eq!(filter.is_visible(&options), is_visible); assert_eq!(filter.is_visible(&options), is_visible);
} }

View File

@ -282,10 +282,13 @@ impl FilterInner {
}, },
FieldType::SingleSelect => { FieldType::SingleSelect => {
let filter = let filter =
SelectOptionFilterPB::parse(condition as u8, content).remove_extra_option_ids(); SelectOptionFilterPB::parse(condition as u8, content).to_single_select_filter();
BoxAny::new(filter)
},
FieldType::MultiSelect => {
let filter = SelectOptionFilterPB::parse(condition as u8, content).to_multi_select_filter();
BoxAny::new(filter) BoxAny::new(filter)
}, },
FieldType::MultiSelect => BoxAny::new(SelectOptionFilterPB::parse(condition as u8, content)),
FieldType::Checklist => BoxAny::new(ChecklistFilterPB::parse(condition as u8, content)), FieldType::Checklist => BoxAny::new(ChecklistFilterPB::parse(condition as u8, content)),
FieldType::Checkbox => BoxAny::new(CheckboxFilterPB::parse(condition as u8, content)), FieldType::Checkbox => BoxAny::new(CheckboxFilterPB::parse(condition as u8, content)),
FieldType::Relation => BoxAny::new(RelationFilterPB::parse(condition as u8, content)), FieldType::Relation => BoxAny::new(RelationFilterPB::parse(condition as u8, content)),

View File

@ -44,52 +44,6 @@ async fn grid_filter_multi_select_is_not_empty_test() {
test.assert_number_of_visible_rows(5).await; test.assert_number_of_visible_rows(5).await;
} }
#[tokio::test]
async fn grid_filter_multi_select_is_test() {
let mut test = DatabaseFilterTest::new().await;
let field = test.get_first_field(FieldType::MultiSelect).await;
let mut options = test.get_multi_select_type_option(&field.id).await;
// Create Multi-Select "Is" filter with specific option IDs
test
.create_data_filter(
None,
FieldType::MultiSelect,
BoxAny::new(SelectOptionFilterPB {
condition: SelectOptionFilterConditionPB::OptionIs,
option_ids: vec![options.remove(0).id, options.remove(0).id],
}),
None,
)
.await;
// Assert the number of visible rows
test.assert_number_of_visible_rows(1).await;
}
#[tokio::test]
async fn grid_filter_multi_select_is_test2() {
let mut test = DatabaseFilterTest::new().await;
let field = test.get_first_field(FieldType::MultiSelect).await;
let mut options = test.get_multi_select_type_option(&field.id).await;
// Create Multi-Select "Is" filter with a specific option ID
test
.create_data_filter(
None,
FieldType::MultiSelect,
BoxAny::new(SelectOptionFilterPB {
condition: SelectOptionFilterConditionPB::OptionIs,
option_ids: vec![options.remove(1).id],
}),
None,
)
.await;
// Assert the number of visible rows
test.assert_number_of_visible_rows(1).await;
}
#[tokio::test] #[tokio::test]
async fn grid_filter_single_select_is_empty_test() { async fn grid_filter_single_select_is_empty_test() {
let mut test = DatabaseFilterTest::new().await; let mut test = DatabaseFilterTest::new().await;

View File

@ -1,5 +1,4 @@
use crate::database::pre_fill_cell_test::script::DatabasePreFillRowCellTest; use crate::database::pre_fill_cell_test::script::DatabasePreFillRowCellTest;
use collab_database::fields::select_type_option::SELECTION_IDS_SEPARATOR;
use flowy_database2::entities::{ use flowy_database2::entities::{
CheckboxFilterConditionPB, CheckboxFilterPB, DateFilterConditionPB, DateFilterPB, FieldType, CheckboxFilterConditionPB, CheckboxFilterPB, DateFilterConditionPB, DateFilterPB, FieldType,
FilterDataPB, SelectOptionFilterConditionPB, SelectOptionFilterPB, TextFilterConditionPB, FilterDataPB, SelectOptionFilterConditionPB, SelectOptionFilterPB, TextFilterConditionPB,
@ -223,9 +222,9 @@ async fn according_to_invalid_date_time_is_filter_test() {
#[tokio::test] #[tokio::test]
async fn according_to_select_option_is_filter_test() { async fn according_to_select_option_is_filter_test() {
let mut test = DatabasePreFillRowCellTest::new().await; let mut test = DatabasePreFillRowCellTest::new().await;
let multi_select_field = test.get_first_field(FieldType::MultiSelect).await; let single_select_field = test.get_first_field(FieldType::SingleSelect).await;
let options = test let options = test
.get_multi_select_type_option(&multi_select_field.id) .get_single_select_type_option(&single_select_field.id)
.await; .await;
let filtering_options = [options[1].clone(), options[2].clone()]; let filtering_options = [options[1].clone(), options[2].clone()];
@ -234,17 +233,16 @@ async fn according_to_select_option_is_filter_test() {
.map(|option| option.id.clone()) .map(|option| option.id.clone())
.collect(); .collect();
let stringified_expected = filtering_options let stringified_expected = filtering_options
.iter() .first()
.map(|option| option.name.clone()) .map(|option| option.name.clone())
.collect::<Vec<_>>() .unwrap_or_default();
.join(SELECTION_IDS_SEPARATOR);
test.assert_row_count(7).await; test.assert_row_count(7).await;
test test
.insert_filter(FilterDataPB { .insert_filter(FilterDataPB {
field_id: multi_select_field.id.clone(), field_id: single_select_field.id.clone(),
field_type: FieldType::MultiSelect, field_type: FieldType::SingleSelect,
data: SelectOptionFilterPB { data: SelectOptionFilterPB {
condition: SelectOptionFilterConditionPB::OptionIs, condition: SelectOptionFilterConditionPB::OptionIs,
option_ids: ids, option_ids: ids,
@ -255,16 +253,16 @@ async fn according_to_select_option_is_filter_test() {
.await; .await;
test.wait(100).await; test.wait(100).await;
test.assert_row_count(1).await; test.assert_row_count(2).await;
test.create_empty_row().await; test.create_empty_row().await;
test.wait(100).await; test.wait(100).await;
test.assert_row_count(2).await; test.assert_row_count(3).await;
test test
.assert_cell_existence(multi_select_field.id.clone(), 1, true) .assert_cell_existence(single_select_field.id.clone(), 1, true)
.await; .await;
test test
.assert_cell_content(multi_select_field.id, 1, stringified_expected) .assert_cell_content(single_select_field.id, 1, stringified_expected)
.await; .await;
} }