From bd46fc11f45d8879b3fb325825be0bafbb135cd9 Mon Sep 17 00:00:00 2001 From: Richard Shiue <71320345+richardshiue@users.noreply.github.com> Date: Fri, 18 Oct 2024 13:48:50 +0800 Subject: [PATCH] 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 --- .../database/database_calendar_test.dart | 17 ++++++- .../database/database_filter_test.dart | 2 +- .../shared/database_test_op.dart | 15 +++--- .../view/database_filter_bottom_sheet.dart | 26 ++--------- .../view/database_filter_condition_list.dart | 4 -- .../application/field/filter_entities.dart | 18 +------- .../calendar/application/calendar_bloc.dart | 18 ++------ .../calendar/presentation/calendar_page.dart | 1 - .../filter/select_option_loader.dart | 45 ------------------ .../select_option/condition_list.dart | 6 ++- .../choicechip/select_option/option_list.dart | 18 ++------ .../select_option/select_option.dart | 10 +--- .../desktop_row_detail_checklist_cell.dart | 1 + .../cell_editor/checklist_cell_editor.dart | 12 ++--- .../filter/filter_entities_test.dart | 31 ++----------- .../filter_entities/select_option_filter.rs | 22 ++++++++- .../selection_type_option/select_filter.rs | 46 ++++++------------- .../src/services/filter/entities.rs | 7 ++- .../filter_test/select_option_filter_test.rs | 46 ------------------- .../pre_fill_row_according_to_filter_test.rs | 22 ++++----- 20 files changed, 106 insertions(+), 261 deletions(-) diff --git a/frontend/appflowy_flutter/integration_test/desktop/database/database_calendar_test.dart b/frontend/appflowy_flutter/integration_test/desktop/database/database_calendar_test.dart index d976499586..eb1d67ffcd 100644 --- a/frontend/appflowy_flutter/integration_test/desktop/database/database_calendar_test.dart +++ b/frontend/appflowy_flutter/integration_test/desktop/database/database_calendar_test.dart @@ -299,6 +299,7 @@ void main() { await tester.tapButton(finderForFieldType(FieldType.MultiSelect)); await tester.createOption(name: "asdf"); await tester.createOption(name: "qwer"); + await tester.selectOption(name: "asdf"); await tester.dismissCellEditor(); await tester.tapDatabaseFilterButton(); @@ -308,19 +309,31 @@ void main() { await tester.tapOptionFilterWithName('asdf'); await tester.dismissCellEditor(); + tester.assertNumberOfEventsInCalendar(0); + + await tester.tapFilterButtonInGrid('Tags'); + await tester.tapOptionFilterWithName('asdf'); + await tester.dismissCellEditor(); + 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); await tester.doubleClickCalendarCell(secondOfThisMonth); await tester.dismissEventEditor(); - tester.assertNumberOfEventsInCalendar(2); + tester.assertNumberOfEventsInCalendar(1); await tester.openCalendarEvent(index: 0, date: secondOfThisMonth); await tester.tapButton(finderForFieldType(FieldType.MultiSelect)); await tester.selectOption(name: "asdf"); await tester.dismissCellEditor(); - tester.assertNumberOfEventsInCalendar(1); + tester.assertNumberOfEventsInCalendar(0); await tester.tapFilterButtonInGrid('Tags'); await tester.changeSelectFilterCondition( diff --git a/frontend/appflowy_flutter/integration_test/desktop/database/database_filter_test.dart b/frontend/appflowy_flutter/integration_test/desktop/database/database_filter_test.dart index 47ca742c6e..8e79445503 100644 --- a/frontend/appflowy_flutter/integration_test/desktop/database/database_filter_test.dart +++ b/frontend/appflowy_flutter/integration_test/desktop/database/database_filter_test.dart @@ -108,7 +108,7 @@ void main() { await tester.tapOptionFilterWithName('s4'); // The row with 's4' should be shown. - tester.assertNumberOfRowsInGridPage(1); + tester.assertNumberOfRowsInGridPage(2); await tester.pumpAndSettle(); }); diff --git a/frontend/appflowy_flutter/integration_test/shared/database_test_op.dart b/frontend/appflowy_flutter/integration_test/shared/database_test_op.dart index 5261b9818c..ad279acb13 100644 --- a/frontend/appflowy_flutter/integration_test/shared/database_test_op.dart +++ b/frontend/appflowy_flutter/integration_test/shared/database_test_op.dart @@ -539,14 +539,15 @@ extension AppFlowyDatabaseTest on WidgetTester { Future deleteChecklistTask({required int index}) async { final task = find.byType(ChecklistItem).at(index); - await startGesture(getCenter(task), kind: PointerDeviceKind.mouse); - await pumpAndSettle(); - - final button = find.byWidgetPredicate( - (widget) => widget is FlowySvg && widget.svg == FlowySvgs.delete_s, + await hoverOnWidget( + task, + onHover: () async { + final button = find.byWidgetPredicate( + (widget) => widget is FlowySvg && widget.svg == FlowySvgs.delete_s, + ); + await tapButton(button); + }, ); - - await tapButton(button); } void assertPhantomChecklistItemAtIndex({required int index}) { diff --git a/frontend/appflowy_flutter/lib/mobile/presentation/database/view/database_filter_bottom_sheet.dart b/frontend/appflowy_flutter/lib/mobile/presentation/database/view/database_filter_bottom_sheet.dart index e63fa8da57..e48d5bf8e1 100644 --- a/frontend/appflowy_flutter/lib/mobile/presentation/database/view/database_filter_bottom_sheet.dart +++ b/frontend/appflowy_flutter/lib/mobile/presentation/database/view/database_filter_bottom_sheet.dart @@ -1003,7 +1003,7 @@ class _SelectOptionFilterContentEditorState isSelected, ); }, - indicator: _getIndicator(), + indicator: MobileSelectedOptionIndicator.multi, 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( BuildContext context, List options, SelectOptionPB option, bool isSelected, ) { + final selectedOptionIds = Set.from(widget.filter.optionIds); if (isSelected) { - final selectedOptionIds = Set.from(widget.filter.optionIds) - ..remove(option.id); - - _updateSelectOptions(context, options, selectedOptionIds); + selectedOptionIds.remove(option.id); } else { - final selectedOptionIds = widget.delegate.selectOption( - widget.filter.optionIds, - option.id, - widget.filter.condition, - ); - - _updateSelectOptions(context, options, selectedOptionIds); + selectedOptionIds.add(option.id); } + _updateSelectOptions(context, options, selectedOptionIds); } void _updateSelectOptions( diff --git a/frontend/appflowy_flutter/lib/mobile/presentation/database/view/database_filter_condition_list.dart b/frontend/appflowy_flutter/lib/mobile/presentation/database/view/database_filter_condition_list.dart index 0dfde2a543..d3d0b9bcdd 100644 --- a/frontend/appflowy_flutter/lib/mobile/presentation/database/view/database_filter_condition_list.dart +++ b/frontend/appflowy_flutter/lib/mobile/presentation/database/view/database_filter_condition_list.dart @@ -93,8 +93,6 @@ final class SingleSelectOptionFilterCondition return [ SelectOptionFilterConditionPB.OptionIs, SelectOptionFilterConditionPB.OptionIsNot, - SelectOptionFilterConditionPB.OptionContains, - SelectOptionFilterConditionPB.OptionDoesNotContain, SelectOptionFilterConditionPB.OptionIsEmpty, SelectOptionFilterConditionPB.OptionIsNotEmpty, ].map((e) => (e, e.i18n)).toList(); @@ -109,8 +107,6 @@ final class MultiSelectOptionFilterCondition return [ SelectOptionFilterConditionPB.OptionContains, SelectOptionFilterConditionPB.OptionDoesNotContain, - SelectOptionFilterConditionPB.OptionIs, - SelectOptionFilterConditionPB.OptionIsNot, SelectOptionFilterConditionPB.OptionIsEmpty, SelectOptionFilterConditionPB.OptionIsNotEmpty, ].map((e) => (e, e.i18n)).toList(); diff --git a/frontend/appflowy_flutter/lib/plugins/database/application/field/filter_entities.dart b/frontend/appflowy_flutter/lib/plugins/database/application/field/filter_entities.dart index e93893154d..46867e1f97 100644 --- a/frontend/appflowy_flutter/lib/plugins/database/application/field/filter_entities.dart +++ b/frontend/appflowy_flutter/lib/plugins/database/application/field/filter_entities.dart @@ -353,14 +353,7 @@ final class SelectOptionFilter extends DatabaseFilter { required List optionIds, }) { if (canAttachContent) { - if (fieldType == FieldType.SingleSelect && - (condition == SelectOptionFilterConditionPB.OptionIs || - condition == SelectOptionFilterConditionPB.OptionIsNot) && - optionIds.isNotEmpty) { - this.optionIds.add(optionIds.first); - } else { - this.optionIds.addAll(optionIds); - } + this.optionIds.addAll(optionIds); } } @@ -377,7 +370,7 @@ final class SelectOptionFilter extends DatabaseFilter { @override String getContentDescription(FieldInfo field) { - if (!canAttachContent) { + if (!canAttachContent || optionIds.isEmpty) { return condition.i18n; } @@ -436,13 +429,6 @@ final class SelectOptionFilter extends DatabaseFilter { SelectOptionFilterConditionPB? condition, List? 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( filterId: filterId, fieldId: fieldId, diff --git a/frontend/appflowy_flutter/lib/plugins/database/calendar/application/calendar_bloc.dart b/frontend/appflowy_flutter/lib/plugins/database/calendar/application/calendar_bloc.dart index 003fd3494b..6798bd9934 100644 --- a/frontend/appflowy_flutter/lib/plugins/database/calendar/application/calendar_bloc.dart +++ b/frontend/appflowy_flutter/lib/plugins/database/calendar/application/calendar_bloc.dart @@ -122,6 +122,7 @@ class CalendarBloc extends Bloc { deleteEventIds: deletedRowIds, ), ); + emit(state.copyWith(deleteEventIds: const [])); }, didReceiveEvent: (CalendarEventData event) { emit( @@ -130,20 +131,11 @@ class CalendarBloc extends Bloc { newEvent: event, ), ); + emit(state.copyWith(newEvent: null)); }, openRowDetail: (row) { - emit( - state.copyWith( - openRow: row, - ), - ); - }, - resetOpenRowDetail: () { - emit( - state.copyWith( - openRow: null, - ), - ); + emit(state.copyWith(openRow: row)); + emit(state.copyWith(openRow: null)); }, ); }, @@ -483,8 +475,6 @@ class CalendarEvent with _$CalendarEvent { _DeleteEvent; const factory CalendarEvent.openRowDetail(RowMetaPB row) = _OpenRowDetail; - - const factory CalendarEvent.resetOpenRowDetail() = _ResetRowDetail; } @freezed diff --git a/frontend/appflowy_flutter/lib/plugins/database/calendar/presentation/calendar_page.dart b/frontend/appflowy_flutter/lib/plugins/database/calendar/presentation/calendar_page.dart index 96d48ff8f2..d6b0f9638a 100644 --- a/frontend/appflowy_flutter/lib/plugins/database/calendar/presentation/calendar_page.dart +++ b/frontend/appflowy_flutter/lib/plugins/database/calendar/presentation/calendar_page.dart @@ -177,7 +177,6 @@ class _CalendarPageState extends State { databaseController: _calendarBloc.databaseController, rowMeta: state.openRow!, ); - _calendarBloc.add(const CalendarEvent.resetOpenRowDetail()); } }, ), diff --git a/frontend/appflowy_flutter/lib/plugins/database/grid/application/filter/select_option_loader.dart b/frontend/appflowy_flutter/lib/plugins/database/grid/application/filter/select_option_loader.dart index 703be6414f..0e7c59bbb6 100644 --- a/frontend/appflowy_flutter/lib/plugins/database/grid/application/filter/select_option_loader.dart +++ b/frontend/appflowy_flutter/lib/plugins/database/grid/application/filter/select_option_loader.dart @@ -6,12 +6,6 @@ abstract class SelectOptionFilterDelegate { const SelectOptionFilterDelegate(); List getOptions(FieldInfo fieldInfo); - - Set selectOption( - List currentOptionIds, - String optionId, - SelectOptionFilterConditionPB condition, - ); } class SingleSelectOptionFilterDelegateImpl @@ -23,37 +17,6 @@ class SingleSelectOptionFilterDelegateImpl final parser = SingleSelectTypeOptionDataParser(); return parser.fromBuffer(fieldInfo.field.typeOptionData).options; } - - @override - Set selectOption( - List currentOptionIds, - String optionId, - SelectOptionFilterConditionPB condition, - ) { - final selectOptionIds = Set.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 @@ -66,12 +29,4 @@ class MultiSelectOptionFilterDelegateImpl .fromBuffer(fieldInfo.field.typeOptionData) .options; } - - @override - Set selectOption( - List currentOptionIds, - String optionId, - SelectOptionFilterConditionPB condition, - ) => - Set.from(currentOptionIds)..add(optionId); } diff --git a/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/widgets/filter/choicechip/select_option/condition_list.dart b/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/widgets/filter/choicechip/select_option/condition_list.dart index 4840ccc48a..a8c8f69016 100644 --- a/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/widgets/filter/choicechip/select_option/condition_list.dart +++ b/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/widgets/filter/choicechip/select_option/condition_list.dart @@ -25,12 +25,14 @@ class SelectOptionFilterConditionList extends StatelessWidget { @override Widget build(BuildContext context) { + final conditions = (fieldType == FieldType.SingleSelect + ? SingleSelectOptionFilterCondition().conditions + : MultiSelectOptionFilterCondition().conditions); return PopoverActionList( asBarrier: true, mutex: popoverMutex, direction: PopoverDirection.bottomWithCenterAligned, - actions: SingleSelectOptionFilterCondition() - .conditions + actions: conditions .map( (action) => ConditionWrapper( action.$1, diff --git a/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/widgets/filter/choicechip/select_option/option_list.dart b/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/widgets/filter/choicechip/select_option/option_list.dart index 29ca24d5f0..3e9db2df1b 100644 --- a/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/widgets/filter/choicechip/select_option/option_list.dart +++ b/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/widgets/filter/choicechip/select_option/option_list.dart @@ -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/filter_entities.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/widgets/cell_editor/select_option_cell_editor.dart'; import 'package:appflowy_backend/protobuf/flowy-database2/select_option_entities.pb.dart'; @@ -17,14 +16,12 @@ class SelectOptionFilterList extends StatelessWidget { super.key, required this.filter, required this.field, - required this.delegate, required this.options, required this.onTap, }); final SelectOptionFilter filter; final FieldInfo field; - final SelectOptionFilterDelegate delegate; final List options; final VoidCallback onTap; @@ -53,20 +50,13 @@ class SelectOptionFilterList extends StatelessWidget { SelectOptionPB option, bool isSelected, ) { + final selectedOptionIds = Set.from(filter.optionIds); if (isSelected) { - final selectedOptionIds = Set.from(filter.optionIds) - ..remove(option.id); - - _updateSelectOptions(context, filter, selectedOptionIds); + selectedOptionIds.remove(option.id); } else { - final selectedOptionIds = delegate.selectOption( - filter.optionIds, - option.id, - filter.condition, - ); - - _updateSelectOptions(context, filter, selectedOptionIds); + selectedOptionIds.add(option.id); } + _updateSelectOptions(context, filter, selectedOptionIds); onTap(); } diff --git a/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/widgets/filter/choicechip/select_option/select_option.dart b/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/widgets/filter/choicechip/select_option/select_option.dart index d4d461f3a2..1aa7acac47 100644 --- a/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/widgets/filter/choicechip/select_option/select_option.dart +++ b/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/widgets/filter/choicechip/select_option/select_option.dart @@ -1,7 +1,6 @@ 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/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:flowy_infra_ui/flowy_infra_ui.dart'; import 'package:flutter/material.dart'; @@ -76,11 +75,7 @@ class _SelectOptionFilterEditorState extends State { SliverToBoxAdapter(child: _buildFilterPanel(filter, field)), ]; - if (![ - SelectOptionFilterConditionPB.OptionIsEmpty, - SelectOptionFilterConditionPB.OptionIsNotEmpty, - ].contains(filter.condition)) { - final delegate = filter.makeDelegate(field); + if (filter.canAttachContent) { slivers ..add(const SliverToBoxAdapter(child: VSpace(4))) ..add( @@ -88,8 +83,7 @@ class _SelectOptionFilterEditorState extends State { child: SelectOptionFilterList( filter: filter, field: field, - delegate: delegate, - options: delegate.getOptions(field), + options: filter.makeDelegate(field).getOptions(field), onTap: () => popoverMutex.close(), ), ), diff --git a/frontend/appflowy_flutter/lib/plugins/database/widgets/cell/desktop_row_detail/desktop_row_detail_checklist_cell.dart b/frontend/appflowy_flutter/lib/plugins/database/widgets/cell/desktop_row_detail/desktop_row_detail_checklist_cell.dart index 4890e75771..34489af254 100644 --- a/frontend/appflowy_flutter/lib/plugins/database/widgets/cell/desktop_row_detail/desktop_row_detail_checklist_cell.dart +++ b/frontend/appflowy_flutter/lib/plugins/database/widgets/cell/desktop_row_detail/desktop_row_detail_checklist_cell.dart @@ -178,6 +178,7 @@ class _ChecklistItems extends StatelessWidget { final children = _makeChildren(context, state); return ReorderableListView.builder( shrinkWrap: true, + physics: const NeverScrollableScrollPhysics(), proxyDecorator: (child, index, _) => Material( color: Colors.transparent, child: Stack( diff --git a/frontend/appflowy_flutter/lib/plugins/database/widgets/cell_editor/checklist_cell_editor.dart b/frontend/appflowy_flutter/lib/plugins/database/widgets/cell_editor/checklist_cell_editor.dart index 3eb7453f73..ae924fc3ef 100644 --- a/frontend/appflowy_flutter/lib/plugins/database/widgets/cell_editor/checklist_cell_editor.dart +++ b/frontend/appflowy_flutter/lib/plugins/database/widgets/cell_editor/checklist_cell_editor.dart @@ -235,9 +235,8 @@ class _ChecklistItemState extends State { @override Widget build(BuildContext context) { - final isFocusedOrHovered = - isHovered || isFocused || textFieldFocusNode.hasFocus; - final color = isFocusedOrHovered + final isFocusedOrHovered = isHovered || isFocused; + final color = isFocusedOrHovered || textFieldFocusNode.hasFocus ? AFThemeExtension.of(context).lightGreyHover : Colors.transparent; return FocusableActionDetector( @@ -259,14 +258,15 @@ class _ChecklistItemState extends State { borderRadius: Corners.s6Border, ), child: _buildChild( - isFocusedOrHovered, + isFocusedOrHovered && !textFieldFocusNode.hasFocus, ), ), ); } - Widget _buildChild(bool isFocusedOrHovered) { + Widget _buildChild(bool showTrash) { return Row( + crossAxisAlignment: CrossAxisAlignment.start, children: [ ReorderableDragStartListener( index: widget.index, @@ -313,7 +313,7 @@ class _ChecklistItemState extends State { }, ), ), - if (isFocusedOrHovered) + if (showTrash) ChecklistCellDeleteButton( onPressed: () => context.read().add( ChecklistCellEvent.deleteTask(widget.task.data.id), diff --git a/frontend/appflowy_flutter/test/bloc_test/grid_test/filter/filter_entities_test.dart b/frontend/appflowy_flutter/test/bloc_test/grid_test/filter/filter_entities_test.dart index fa4031ea90..12afca48f8 100644 --- a/frontend/appflowy_flutter/test/bloc_test/grid_test/filter/filter_entities_test.dart +++ b/frontend/appflowy_flutter/test/bloc_test/grid_test/filter/filter_entities_test.dart @@ -242,7 +242,7 @@ void main() { createFilterPB( FieldType.SingleSelect, SelectOptionFilterPB( - condition: SelectOptionFilterConditionPB.OptionContains, + condition: SelectOptionFilterConditionPB.OptionIs, optionIds: [], ).writeToBuffer(), ), @@ -252,7 +252,7 @@ void main() { filterId: "FT", fieldId: "FD", fieldType: FieldType.SingleSelect, - condition: SelectOptionFilterConditionPB.OptionContains, + condition: SelectOptionFilterConditionPB.OptionIs, optionIds: const [], ), ), @@ -263,7 +263,7 @@ void main() { createFilterPB( FieldType.SingleSelect, SelectOptionFilterPB( - condition: SelectOptionFilterConditionPB.OptionContains, + condition: SelectOptionFilterConditionPB.OptionIs, optionIds: ['a'], ).writeToBuffer(), ), @@ -273,33 +273,12 @@ void main() { filterId: "FT", fieldId: "FD", fieldType: FieldType.SingleSelect, - condition: SelectOptionFilterConditionPB.OptionContains, + condition: SelectOptionFilterConditionPB.OptionIs, 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( DatabaseFilter.fromPB( createFilterPB( @@ -316,7 +295,7 @@ void main() { fieldId: "FD", fieldType: FieldType.SingleSelect, condition: SelectOptionFilterConditionPB.OptionIs, - optionIds: const ['a'], + optionIds: const ['a', 'b'], ), ), ); diff --git a/frontend/rust-lib/flowy-database2/src/entities/filter_entities/select_option_filter.rs b/frontend/rust-lib/flowy-database2/src/entities/filter_entities/select_option_filter.rs index d6775cf374..37fd35aaa9 100644 --- a/frontend/rust-lib/flowy-database2/src/entities/filter_entities/select_option_filter.rs +++ b/frontend/rust-lib/flowy-database2/src/entities/filter_entities/select_option_filter.rs @@ -60,8 +60,26 @@ impl ParseFilterData for SelectOptionFilterPB { } impl SelectOptionFilterPB { - pub fn remove_extra_option_ids(mut self) -> Self { - self.option_ids.truncate(1); + pub fn to_single_select_filter(mut self) -> Self { + 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 } } diff --git a/frontend/rust-lib/flowy-database2/src/services/field/type_options/selection_type_option/select_filter.rs b/frontend/rust-lib/flowy-database2/src/services/field/type_options/selection_type_option/select_filter.rs index d6ff1d4fda..af1ff97368 100644 --- a/frontend/rust-lib/flowy-database2/src/services/field/type_options/selection_type_option/select_filter.rs +++ b/frontend/rust-lib/flowy-database2/src/services/field/type_options/selection_type_option/select_filter.rs @@ -55,16 +55,14 @@ impl SelectOptionFilterStrategy { 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) => { if selected_option_ids.is_empty() { 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) => { if selected_option_ids.is_empty() { @@ -98,18 +96,9 @@ impl SelectOptionFilterStrategy { impl PreFillCellsWithFilter for SelectOptionFilterPB { fn get_compliant_cell(&self, field: &Field) -> Option { - 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 { - SelectOptionFilterConditionPB::OptionIs => get_non_empty_expected_options(), - SelectOptionFilterConditionPB::OptionContains => { - get_non_empty_expected_options().map(|mut options| vec![options.swap_remove(0)]) + SelectOptionFilterConditionPB::OptionIs | SelectOptionFilterConditionPB::OptionContains => { + self.option_ids.first().map(|id| vec![id.clone()]) }, SelectOptionFilterConditionPB::OptionIsNotEmpty => select_type_option_from_field(field) .ok() @@ -127,6 +116,7 @@ impl PreFillCellsWithFilter for SelectOptionFilterPB { option_ids.map(|ids| insert_select_option_cell(ids, field)) } } + #[cfg(test)] mod tests { use crate::entities::{SelectOptionFilterConditionPB, SelectOptionFilterPB}; @@ -150,11 +140,12 @@ mod tests { let option_2 = SelectOption::new("B"); let filter = SelectOptionFilterPB { 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(&[option_1.clone()]), Some(true)); + assert_eq!(filter.is_visible(&[option_1, option_2]), Some(true)); } #[test] @@ -185,8 +176,6 @@ mod tests { (vec![], Some(false)), (vec![option_1.clone()], Some(true)), (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); } @@ -198,12 +187,9 @@ mod tests { }; for (options, is_visible) in [ (vec![], Some(false)), - (vec![option_1.clone()], Some(false)), - (vec![option_1.clone(), option_2.clone()], Some(true)), - ( - vec![option_1.clone(), option_2.clone(), option_3.clone()], - Some(false), - ), + (vec![option_1.clone()], Some(true)), + (vec![option_2.clone()], Some(true)), + (vec![option_3.clone()], Some(false)), ] { assert_eq!(filter.is_visible(&options), is_visible); } @@ -223,7 +209,7 @@ mod tests { for (options, is_visible) in [ (vec![], 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); } @@ -238,7 +224,6 @@ mod tests { (vec![option_1.clone()], Some(false)), (vec![option_2.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); } @@ -250,12 +235,9 @@ mod tests { }; for (options, is_visible) in [ (vec![], Some(true)), - (vec![option_1.clone()], Some(true)), - (vec![option_1.clone(), option_2.clone()], Some(false)), - ( - vec![option_1.clone(), option_2.clone(), option_3.clone()], - Some(true), - ), + (vec![option_1.clone()], Some(false)), + (vec![option_2.clone()], Some(false)), + (vec![option_3.clone()], Some(true)), ] { assert_eq!(filter.is_visible(&options), is_visible); } diff --git a/frontend/rust-lib/flowy-database2/src/services/filter/entities.rs b/frontend/rust-lib/flowy-database2/src/services/filter/entities.rs index 7fefc95729..070dd5b889 100644 --- a/frontend/rust-lib/flowy-database2/src/services/filter/entities.rs +++ b/frontend/rust-lib/flowy-database2/src/services/filter/entities.rs @@ -282,10 +282,13 @@ impl FilterInner { }, FieldType::SingleSelect => { 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) }, - FieldType::MultiSelect => BoxAny::new(SelectOptionFilterPB::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::Relation => BoxAny::new(RelationFilterPB::parse(condition as u8, content)), diff --git a/frontend/rust-lib/flowy-database2/tests/database/filter_test/select_option_filter_test.rs b/frontend/rust-lib/flowy-database2/tests/database/filter_test/select_option_filter_test.rs index ed6c2a62ca..4a92f4c93f 100644 --- a/frontend/rust-lib/flowy-database2/tests/database/filter_test/select_option_filter_test.rs +++ b/frontend/rust-lib/flowy-database2/tests/database/filter_test/select_option_filter_test.rs @@ -44,52 +44,6 @@ async fn grid_filter_multi_select_is_not_empty_test() { 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] async fn grid_filter_single_select_is_empty_test() { let mut test = DatabaseFilterTest::new().await; diff --git a/frontend/rust-lib/flowy-database2/tests/database/pre_fill_cell_test/pre_fill_row_according_to_filter_test.rs b/frontend/rust-lib/flowy-database2/tests/database/pre_fill_cell_test/pre_fill_row_according_to_filter_test.rs index a40dac350e..a0b3e5da3e 100644 --- a/frontend/rust-lib/flowy-database2/tests/database/pre_fill_cell_test/pre_fill_row_according_to_filter_test.rs +++ b/frontend/rust-lib/flowy-database2/tests/database/pre_fill_cell_test/pre_fill_row_according_to_filter_test.rs @@ -1,5 +1,4 @@ use crate::database::pre_fill_cell_test::script::DatabasePreFillRowCellTest; -use collab_database::fields::select_type_option::SELECTION_IDS_SEPARATOR; use flowy_database2::entities::{ CheckboxFilterConditionPB, CheckboxFilterPB, DateFilterConditionPB, DateFilterPB, FieldType, FilterDataPB, SelectOptionFilterConditionPB, SelectOptionFilterPB, TextFilterConditionPB, @@ -223,9 +222,9 @@ async fn according_to_invalid_date_time_is_filter_test() { #[tokio::test] async fn according_to_select_option_is_filter_test() { 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 - .get_multi_select_type_option(&multi_select_field.id) + .get_single_select_type_option(&single_select_field.id) .await; 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()) .collect(); let stringified_expected = filtering_options - .iter() + .first() .map(|option| option.name.clone()) - .collect::>() - .join(SELECTION_IDS_SEPARATOR); + .unwrap_or_default(); test.assert_row_count(7).await; test .insert_filter(FilterDataPB { - field_id: multi_select_field.id.clone(), - field_type: FieldType::MultiSelect, + field_id: single_select_field.id.clone(), + field_type: FieldType::SingleSelect, data: SelectOptionFilterPB { condition: SelectOptionFilterConditionPB::OptionIs, option_ids: ids, @@ -255,16 +253,16 @@ async fn according_to_select_option_is_filter_test() { .await; test.wait(100).await; - test.assert_row_count(1).await; + test.assert_row_count(2).await; test.create_empty_row().await; test.wait(100).await; - test.assert_row_count(2).await; + test.assert_row_count(3).await; test - .assert_cell_existence(multi_select_field.id.clone(), 1, true) + .assert_cell_existence(single_select_field.id.clone(), 1, true) .await; test - .assert_cell_content(multi_select_field.id, 1, stringified_expected) + .assert_cell_content(single_select_field.id, 1, stringified_expected) .await; }