feat: alter some select option editor bloc events and add tests (#1264)

* chore: separate select and unselect events

* style: improve readability

* chore: don't send empty payloads if we can help it

* test: add select option text field test

* test: complete bloc test for select option

* test: delete all options between each test

* fix: keep insert order

* test: combine select and unselect tests

* chore: remove duplicate wait

Co-authored-by: appflowy <annie@appflowy.io>

frontend/app_flowy/lib/plugins/grid/application/cell/select_option_editor_bloc.dart

@@ -1,7 +1,6 @@
 import 'dart:async';
 
 import 'package:app_flowy/plugins/grid/application/cell/cell_service/cell_service.dart';
-import 'package:collection/collection.dart';
 import 'package:dartz/dartz.dart';
 import 'package:flowy_sdk/log.dart';
 import 'package:flowy_sdk/protobuf/flowy-grid/select_type_option.pb.dart';
@@ -46,19 +45,29 @@ class SelectOptionCellEditorBloc
             ));
           },
           deleteOption: (_DeleteOption value) {
-            _deleteOption(value.option);
+            _deleteOption([value.option]);
+          },
+          deleteAllOptions: (_DeleteAllOptions value) {
+            if (state.allOptions.isNotEmpty) {
+              _deleteOption(state.allOptions);
+            }
           },
           updateOption: (_UpdateOption value) {
             _updateOption(value.option);
           },
           selectOption: (_SelectOption value) {
-            _onSelectOption(value.optionId);
+            _selectOptionService.select(optionIds: [value.optionId]);
+          },
+          unSelectOption: (_UnSelectOption value) {
+            _selectOptionService.unSelect(optionIds: [value.optionId]);
           },
           trySelectOption: (_TrySelectOption value) {
             _trySelectOption(value.optionName, emit);
           },
           selectMultipleOptions: (_SelectMultipleOptions value) {
-            _selectMultipleOptions(value.optionNames);
+            if (value.optionNames.isNotEmpty) {
+              _selectMultipleOptions(value.optionNames);
+            }
             _filterOption(value.remainder, emit);
           },
           filterOption: (_SelectOptionFilter value) {
@@ -81,11 +90,8 @@ class SelectOptionCellEditorBloc
     result.fold((l) => {}, (err) => Log.error(err));
   }
 
-  void _deleteOption(SelectOptionPB option) async {
+  void _deleteOption(List<SelectOptionPB> options) async {
-    final result = await _selectOptionService.delete(
+    final result = await _selectOptionService.delete(options: options);
-      option: option,
-    );
-
     result.fold((l) => null, (err) => Log.error(err));
   }
 
@@ -97,16 +103,6 @@ class SelectOptionCellEditorBloc
     result.fold((l) => null, (err) => Log.error(err));
   }
 
-  void _onSelectOption(String optionId) {
-    final hasSelected = state.selectedOptions
-        .firstWhereOrNull((option) => option.id == optionId);
-    if (hasSelected != null) {
-      _selectOptionService.unSelect(optionIds: [optionId]);
-    } else {
-      _selectOptionService.select(optionIds: [optionId]);
-    }
-  }
-
   void _trySelectOption(
       String optionName, Emitter<SelectOptionEditorState> emit) {
     SelectOptionPB? matchingOption;
@@ -138,9 +134,19 @@ class SelectOptionCellEditorBloc
   }
 
   void _selectMultipleOptions(List<String> optionNames) {
-    final optionIds = state.options
+    // The options are unordered. So in order to keep the inserted [optionNames]
-        .where((e) => optionNames.contains(e.name))
+    // order, it needs to get the option id in the [optionNames] order.
-        .map((e) => e.id);
+    final lowerCaseNames = optionNames.map((e) => e.toLowerCase());
+    final Map<String, String> optionIdsMap = {};
+    for (final option in state.options) {
+      optionIdsMap[option.name.toLowerCase()] = option.id;
+    }
+
+    final optionIds = lowerCaseNames
+        .where((name) => optionIdsMap[name] != null)
+        .map((name) => optionIdsMap[name]!)
+        .toList();
+
     _selectOptionService.select(optionIds: optionIds);
   }
 
@@ -162,8 +168,10 @@ class SelectOptionCellEditorBloc
           return;
         }
         return result.fold(
-          (data) => add(SelectOptionEditorEvent.didReceiveOptions(
+          (data) => add(
-              data.options, data.selectOptions)),
+            SelectOptionEditorEvent.didReceiveOptions(
+                data.options, data.selectOptions),
+          ),
           (err) {
             Log.error(err);
             return null;
@@ -225,10 +233,13 @@ class SelectOptionEditorEvent with _$SelectOptionEditorEvent {
       _NewOption;
   const factory SelectOptionEditorEvent.selectOption(String optionId) =
       _SelectOption;
+  const factory SelectOptionEditorEvent.unSelectOption(String optionId) =
+      _UnSelectOption;
   const factory SelectOptionEditorEvent.updateOption(SelectOptionPB option) =
       _UpdateOption;
   const factory SelectOptionEditorEvent.deleteOption(SelectOptionPB option) =
       _DeleteOption;
+  const factory SelectOptionEditorEvent.deleteAllOptions() = _DeleteAllOptions;
   const factory SelectOptionEditorEvent.filterOption(String optionName) =
       _SelectOptionFilter;
   const factory SelectOptionEditorEvent.trySelectOption(String optionName) =

frontend/app_flowy/lib/plugins/grid/application/cell/select_option_service.dart

@@ -45,11 +45,10 @@ class SelectOptionService {
     return GridEventUpdateSelectOption(payload).send();
   }
 
-  Future<Either<Unit, FlowyError>> delete({
+  Future<Either<Unit, FlowyError>> delete(
-    required SelectOptionPB option,
+      {required Iterable<SelectOptionPB> options}) {
-  }) {
     final payload = SelectOptionChangesetPayloadPB.create()
-      ..deleteOptions.add(option)
+      ..deleteOptions.addAll(options)
       ..cellIdentifier = _cellIdentifier();
 
     return GridEventUpdateSelectOption(payload).send();

frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/select_option_cell/select_option_editor.dart

@@ -261,9 +261,15 @@ class _SelectOptionCellState extends State<_SelectOptionCell> {
         child: SelectOptionTagCell(
           option: widget.option,
           onSelected: (option) {
-            context
+            if (widget.isSelected) {
-                .read<SelectOptionCellEditorBloc>()
+              context
-                .add(SelectOptionEditorEvent.selectOption(option.id));
+                  .read<SelectOptionCellEditorBloc>()
+                  .add(SelectOptionEditorEvent.unSelectOption(option.id));
+            } else {
+              context
+                  .read<SelectOptionCellEditorBloc>()
+                  .add(SelectOptionEditorEvent.selectOption(option.id));
+            }
           },
           children: [
             if (widget.isSelected)

frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/select_option_cell/text_field.dart

@@ -96,7 +96,7 @@ class _SelectOptionTextFieldState extends State<SelectOptionTextField> {
               }
 
               if (text.isNotEmpty) {
-                widget.onSubmitted(text);
+                widget.onSubmitted(text.trim());
                 focusNode.requestFocus();
               }
             },
@@ -132,26 +132,25 @@ class _SelectOptionTextFieldState extends State<SelectOptionTextField> {
       return;
     }
 
-    final trimmedText = text.trim();
+    final trimmedText = text.trimLeft();
     List<String> splits = [];
     String currentString = '';
 
     // split the string into tokens
     for (final char in trimmedText.split('')) {
-      if (!widget.textSeparators.contains(char)) {
+      if (widget.textSeparators.contains(char)) {
-        currentString += char;
+        if (currentString.isNotEmpty) {
+          splits.add(currentString.trim());
+        }
+        currentString = '';
         continue;
       }
-      if (currentString.isNotEmpty) {
+      currentString += char;
-        splits.add(currentString);
-      }
-      currentString = '';
     }
     // add the remainder (might be '')
     splits.add(currentString);
 
-    final submittedOptions =
+    final submittedOptions = splits.sublist(0, splits.length - 1).toList();
-        splits.sublist(0, splits.length - 1).map((e) => e.trim()).toList();
 
     final remainder = splits.elementAt(splits.length - 1).trimLeft();
     editingController.text = remainder;

frontend/app_flowy/test/bloc_test/grid_test/select_option_bloc_test.dart

@@ -1,6 +1,9 @@
 import 'package:app_flowy/plugins/grid/application/cell/cell_service/cell_service.dart';
 import 'package:app_flowy/plugins/grid/application/cell/select_option_editor_bloc.dart';
+import 'package:app_flowy/plugins/grid/application/prelude.dart';
+import 'package:dartz/dartz.dart';
 import 'package:flowy_sdk/protobuf/flowy-grid/field_entities.pb.dart';
+import 'package:flowy_sdk/protobuf/flowy-grid/select_type_option.pb.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:bloc_test/bloc_test.dart';
 import 'util.dart';
@@ -18,6 +21,28 @@ void main() {
           await cellTest.makeCellController(FieldType.SingleSelect);
     });
 
+    blocTest<SelectOptionCellEditorBloc, SelectOptionEditorState>(
+      "delete options",
+      build: () {
+        final bloc = SelectOptionCellEditorBloc(cellController: cellController);
+        bloc.add(const SelectOptionEditorEvent.initial());
+        return bloc;
+      },
+      act: (bloc) async {
+        bloc.add(const SelectOptionEditorEvent.newOption("A"));
+        await Future.delayed(gridResponseDuration());
+        bloc.add(const SelectOptionEditorEvent.newOption("B"));
+        await Future.delayed(gridResponseDuration());
+        bloc.add(const SelectOptionEditorEvent.newOption("C"));
+        await Future.delayed(gridResponseDuration());
+        bloc.add(const SelectOptionEditorEvent.deleteAllOptions());
+      },
+      wait: gridResponseDuration(),
+      verify: (bloc) {
+        assert(bloc.state.options.isEmpty);
+      },
+    );
+
     blocTest<SelectOptionCellEditorBloc, SelectOptionEditorState>(
       "create option",
       build: () {
@@ -25,12 +50,163 @@ void main() {
         bloc.add(const SelectOptionEditorEvent.initial());
         return bloc;
       },
-      act: (bloc) => bloc.add(const SelectOptionEditorEvent.newOption("A")),
+      act: (bloc) async {
+        _removeFieldOptions(bloc);
+        bloc.add(const SelectOptionEditorEvent.newOption("A"));
+      },
+      wait: gridResponseDuration(),
+      verify: (bloc) {
+        expect(bloc.state.options.length, 1);
+        expect(bloc.state.options[0].name, "A");
+      },
+    );
+
+    blocTest<SelectOptionCellEditorBloc, SelectOptionEditorState>(
+      "delete option",
+      build: () {
+        final bloc = SelectOptionCellEditorBloc(cellController: cellController);
+        bloc.add(const SelectOptionEditorEvent.initial());
+        return bloc;
+      },
+      act: (bloc) async {
+        _removeFieldOptions(bloc);
+        bloc.add(const SelectOptionEditorEvent.newOption("A"));
+        await Future.delayed(gridResponseDuration());
+        bloc.add(SelectOptionEditorEvent.deleteOption(bloc.state.options[0]));
+      },
+      wait: gridResponseDuration(),
+      verify: (bloc) {
+        assert(bloc.state.options.isEmpty);
+      },
+    );
+
+    blocTest<SelectOptionCellEditorBloc, SelectOptionEditorState>(
+      "update option",
+      build: () {
+        final bloc = SelectOptionCellEditorBloc(cellController: cellController);
+        bloc.add(const SelectOptionEditorEvent.initial());
+        return bloc;
+      },
+      act: (bloc) async {
+        _removeFieldOptions(bloc);
+        bloc.add(const SelectOptionEditorEvent.newOption("A"));
+        await Future.delayed(gridResponseDuration());
+        SelectOptionPB optionUpdate = bloc.state.options[0]
+          ..color = SelectOptionColorPB.Aqua
+          ..name = "B";
+        bloc.add(SelectOptionEditorEvent.updateOption(optionUpdate));
+      },
       wait: gridResponseDuration(),
       verify: (bloc) {
         assert(bloc.state.options.length == 1);
-        assert(bloc.state.options[0].name == "A");
+        expect(bloc.state.options[0].color, SelectOptionColorPB.Aqua);
+        expect(bloc.state.options[0].name, "B");
+      },
+    );
+
+    blocTest<SelectOptionCellEditorBloc, SelectOptionEditorState>(
+      "select/unselect option",
+      build: () {
+        final bloc = SelectOptionCellEditorBloc(cellController: cellController);
+        bloc.add(const SelectOptionEditorEvent.initial());
+        return bloc;
+      },
+      act: (bloc) async {
+        _removeFieldOptions(bloc);
+        bloc.add(const SelectOptionEditorEvent.newOption("A"));
+        await Future.delayed(gridResponseDuration());
+        expect(bloc.state.selectedOptions.length, 1);
+        final optionId = bloc.state.options[0].id;
+        bloc.add(SelectOptionEditorEvent.unSelectOption(optionId));
+        await Future.delayed(gridResponseDuration());
+        assert(bloc.state.selectedOptions.isEmpty);
+        bloc.add(SelectOptionEditorEvent.selectOption(optionId));
+      },
+      wait: gridResponseDuration(),
+      verify: (bloc) {
+        assert(bloc.state.selectedOptions.length == 1);
+        expect(bloc.state.selectedOptions[0].name, "A");
+      },
+    );
+
+    blocTest<SelectOptionCellEditorBloc, SelectOptionEditorState>(
+      "select an option or create one",
+      build: () {
+        final bloc = SelectOptionCellEditorBloc(cellController: cellController);
+        bloc.add(const SelectOptionEditorEvent.initial());
+        return bloc;
+      },
+      act: (bloc) async {
+        _removeFieldOptions(bloc);
+        bloc.add(const SelectOptionEditorEvent.newOption("A"));
+        await Future.delayed(gridResponseDuration());
+        bloc.add(const SelectOptionEditorEvent.trySelectOption("B"));
+        await Future.delayed(gridResponseDuration());
+        bloc.add(const SelectOptionEditorEvent.trySelectOption("A"));
+      },
+      wait: gridResponseDuration(),
+      verify: (bloc) {
+        assert(bloc.state.selectedOptions.length == 1);
+        assert(bloc.state.options.length == 2);
+        expect(bloc.state.selectedOptions[0].name, "A");
+      },
+    );
+
+    blocTest<SelectOptionCellEditorBloc, SelectOptionEditorState>(
+      "select multiple options",
+      build: () {
+        final bloc = SelectOptionCellEditorBloc(cellController: cellController);
+        bloc.add(const SelectOptionEditorEvent.initial());
+        return bloc;
+      },
+      act: (bloc) async {
+        _removeFieldOptions(bloc);
+        bloc.add(const SelectOptionEditorEvent.newOption("A"));
+        await Future.delayed(gridResponseDuration());
+        bloc.add(const SelectOptionEditorEvent.newOption("B"));
+        await Future.delayed(gridResponseDuration());
+        bloc.add(const SelectOptionEditorEvent.selectMultipleOptions(
+            ["A", "B", "C"], "x"));
+      },
+      wait: gridResponseDuration(),
+      verify: (bloc) {
+        assert(bloc.state.selectedOptions.length == 1);
+        expect(bloc.state.selectedOptions[0].name, "A");
+        expect(bloc.state.filter, const Some("x"));
+      },
+    );
+
+    blocTest<SelectOptionCellEditorBloc, SelectOptionEditorState>(
+      "filter options",
+      build: () {
+        final bloc = SelectOptionCellEditorBloc(cellController: cellController);
+        bloc.add(const SelectOptionEditorEvent.initial());
+        return bloc;
+      },
+      act: (bloc) async {
+        _removeFieldOptions(bloc);
+        bloc.add(const SelectOptionEditorEvent.newOption("abcd"));
+        await Future.delayed(gridResponseDuration());
+        bloc.add(const SelectOptionEditorEvent.newOption("aaaa"));
+        await Future.delayed(gridResponseDuration());
+        bloc.add(const SelectOptionEditorEvent.newOption("defg"));
+        await Future.delayed(gridResponseDuration());
+        bloc.add(const SelectOptionEditorEvent.filterOption("a"));
+      },
+      wait: gridResponseDuration(),
+      verify: (bloc) {
+        expect(bloc.state.options.length, 2);
+        expect(bloc.state.allOptions.length, 3);
+        expect(bloc.state.createOption, const Some("a"));
+        expect(bloc.state.filter, const Some("a"));
       },
     );
   });
 }
+
+void _removeFieldOptions(SelectOptionCellEditorBloc bloc) async {
+  if (bloc.state.options.isNotEmpty) {
+    bloc.add(const SelectOptionEditorEvent.deleteAllOptions());
+    await Future.delayed(gridResponseDuration());
+  }
+}

frontend/app_flowy/test/widget_test/select_option_text_field_test.dart

@@ -0,0 +1,90 @@
+import 'dart:collection';
+
+import 'package:app_flowy/plugins/grid/presentation/widgets/cell/select_option_cell/text_field.dart';
+import 'package:flowy_infra/theme.dart';
+import 'package:flowy_sdk/protobuf/flowy-grid/protobuf.dart';
+import 'package:flutter/material.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:provider/provider.dart';
+import 'package:textfield_tags/textfield_tags.dart';
+
+import '../bloc_test/grid_test/util.dart';
+
+void main() {
+  setUpAll(() {
+    AppFlowyGridTest.ensureInitialized();
+  });
+
+  group('text_field.dart', () {
+    String submit = '';
+    String remainder = '';
+    List<String> select = [];
+
+    final textField = SelectOptionTextField(
+      options: const [],
+      selectedOptionMap: LinkedHashMap<String, SelectOptionPB>(),
+      distanceToText: 0.0,
+      tagController: TextfieldTagsController(),
+      onSubmitted: (text) => submit = text,
+      onPaste: (options, remaining) {
+        remainder = remaining;
+        select = options;
+      },
+      newText: (_) {},
+      textSeparators: const [','],
+      textController: TextEditingController(),
+    );
+
+    testWidgets('SelectOptionTextField callback outputs',
+        (WidgetTester tester) async {
+      await tester.pumpWidget(
+        MaterialApp(
+          home: Material(
+            child: Provider<AppTheme>.value(
+              value: AppTheme.fromType(ThemeType.light),
+              child: textField,
+            ),
+          ),
+        ),
+      );
+
+      // test that the input field exists
+      expect(find.byType(TextField), findsOneWidget);
+
+      // simulate normal input
+      await tester.enterText(find.byType(TextField), 'abcd');
+      expect(remainder, 'abcd');
+
+      await tester.enterText(find.byType(TextField), ' ');
+      expect(remainder, '');
+
+      // test submit functionality (aka pressing enter)
+      await tester.enterText(find.byType(TextField), 'an option');
+      await tester.testTextInput.receiveAction(TextInputAction.done);
+      expect(submit, 'an option');
+
+      await tester.enterText(find.byType(TextField), ' another one ');
+      await tester.testTextInput.receiveAction(TextInputAction.done);
+      expect(submit, 'another one');
+
+      // test inputs containing commas
+      await tester.enterText(find.byType(TextField), ' abcd,');
+      expect(remainder, '');
+      expect(select, ['abcd']);
+
+      await tester.enterText(find.byType(TextField), ',acd, aaaa ');
+      expect(remainder, 'aaaa ');
+      expect(select, ['acd']);
+
+      await tester.enterText(find.byType(TextField), 'a a, bbbb , ');
+      expect(remainder, '');
+      expect(select, ['a a', 'bbbb']);
+
+      // test paste followed by submit
+      await tester.enterText(find.byType(TextField), 'aaa, bbb, c');
+      await tester.testTextInput.receiveAction(TextInputAction.done);
+      expect(select, ['aaa', 'bbb']);
+      expect(submit, 'c');
+    });
+  });
+}