Refactor user selection state handling using hook

This commit introduces `useUserSelectionState` compositional hook. it centralizes and allows reusing the logic for tracking and mutation user selection state across the application. The change aims to increase code reusability, simplify the code, improve testability, and adhere to the single responsibility principle. It makes the code more reliable against race conditions and removes the need for tracking deep changes. Other supporting changes: - Introduce `CardStateIndicator` component for displaying the card state indicator icon, improving the testability and separation of concerns. - Refactor `SelectionTypeHandler` to use functional code with more clear interfaces to simplify the code. It reduces complexity, increases maintainability and increase readability by explicitly separating mutating functions. - Add new unit tests and extend improving ones to cover the new logic introduced in this commit. Remove the need to mount a wrapper component to simplify and optimize some tests, using parameter injection to inject dependencies intead.
2023-11-10 13:16:53 +01:00
parent 7770a9b521
commit 58cd551a30
22 changed files with 700 additions and 470 deletions
--- a/tests/unit/presentation/components/Scripts/View/Tree/TreeViewAdapter/UseCollectionSelectionStateUpdater.spec.ts
+++ b/tests/unit/presentation/components/Scripts/View/Tree/TreeViewAdapter/UseCollectionSelectionStateUpdater.spec.ts
@@ -1,22 +1,19 @@
 import { describe, it, expect } from 'vitest';
-import { shallowMount } from '@vue/test-utils';
 import { useCollectionSelectionStateUpdater } from '@/presentation/components/Scripts/View/Tree/TreeViewAdapter/UseCollectionSelectionStateUpdater';
-import { InjectionKeys } from '@/presentation/injectionSymbols';
 import { HierarchyAccessStub } from '@tests/unit/shared/Stubs/HierarchyAccessStub';
 import { TreeNodeStateAccessStub } from '@tests/unit/shared/Stubs/TreeNodeStateAccessStub';
 import { TreeNodeStub } from '@tests/unit/shared/Stubs/TreeNodeStub';
-import { UseCollectionStateStub } from '@tests/unit/shared/Stubs/UseCollectionStateStub';
 import { TreeNodeCheckState } from '@/presentation/components/Scripts/View/Tree/TreeView/Node/State/CheckState';
-import { CategoryCollectionStateStub } from '@tests/unit/shared/Stubs/CategoryCollectionStateStub';
 import { UserSelectionStub } from '@tests/unit/shared/Stubs/UserSelectionStub';
 import { TreeNodeStateChangedEmittedEventStub } from '@tests/unit/shared/Stubs/TreeNodeStateChangedEmittedEventStub';
+import { UseUserSelectionStateStub } from '@tests/unit/shared/Stubs/UseUserSelectionStateStub';

 describe('useCollectionSelectionStateUpdater', () => {
  describe('updateNodeSelection', () => {
    describe('when node is a branch node', () => {
      it('does nothing', () => {
        // arrange
-        const { returnObject, useStateStub } = mountWrapperComponent();
+        const { returnObject, useSelectionStateStub } = runHook();
        const mockEvent = new TreeNodeStateChangedEmittedEventStub()
          .withNode(
            createTreeNodeStub({
@@ -31,14 +28,13 @@ describe('useCollectionSelectionStateUpdater', () => {
        // act
        returnObject.updateNodeSelection(mockEvent);
        // assert
-        const modifyCall = useStateStub.callHistory.find((call) => call.methodName === 'modifyCurrentState');
-        expect(modifyCall).toBeUndefined();
+        expect(useSelectionStateStub.isSelectionModified()).to.equal(false);
      });
    });
    describe('when old and new check states are the same', () => {
      it('does nothing', () => {
        // arrange
-        const { returnObject, useStateStub } = mountWrapperComponent();
+        const { returnObject, useSelectionStateStub } = runHook();
        const mockEvent = new TreeNodeStateChangedEmittedEventStub()
          .withNode(
            createTreeNodeStub({
@@ -53,24 +49,22 @@ describe('useCollectionSelectionStateUpdater', () => {
        // act
        returnObject.updateNodeSelection(mockEvent);
        // assert
-        const modifyCall = useStateStub.callHistory.find((call) => call.methodName === 'modifyCurrentState');
-        expect(modifyCall).toBeUndefined();
+        expect(useSelectionStateStub.isSelectionModified()).to.equal(false);
      });
    });
    describe('when checkState is checked', () => {
      it('adds to selection if not already selected', () => {
        // arrange
-        const { returnObject, useStateStub } = mountWrapperComponent();
+        const { returnObject, useSelectionStateStub } = runHook();
        const selectionStub = new UserSelectionStub([]);
        selectionStub.isSelected = () => false;
-        useStateStub.withState(new CategoryCollectionStateStub().withSelection(selectionStub));
+        useSelectionStateStub.withUserSelection(selectionStub);
+        const node = createTreeNodeStub({
+          isBranch: false,
+          currentState: TreeNodeCheckState.Checked,
+        });
        const mockEvent = new TreeNodeStateChangedEmittedEventStub()
-          .withNode(
-            createTreeNodeStub({
-              isBranch: false,
-              currentState: TreeNodeCheckState.Checked,
-            }),
-          )
+          .withNode(node)
          .withCheckStateChange({
            oldState: TreeNodeCheckState.Unchecked,
            newState: TreeNodeCheckState.Checked,
@@ -78,17 +72,15 @@ describe('useCollectionSelectionStateUpdater', () => {
        // act
        returnObject.updateNodeSelection(mockEvent);
        // assert
-        const modifyCall = useStateStub.callHistory.find((call) => call.methodName === 'modifyCurrentState');
-        expect(modifyCall).toBeDefined();
-        const addSelectedScriptCall = selectionStub.callHistory.find((call) => call.methodName === 'addSelectedScript');
-        expect(addSelectedScriptCall).toBeDefined();
+        expect(useSelectionStateStub.isSelectionModified()).to.equal(true);
+        expect(selectionStub.isScriptAdded(node.id)).to.equal(true);
      });
      it('does nothing if already selected', () => {
        // arrange
-        const { returnObject, useStateStub } = mountWrapperComponent();
+        const { returnObject, useSelectionStateStub } = runHook();
        const selectionStub = new UserSelectionStub([]);
        selectionStub.isSelected = () => true;
-        useStateStub.withState(new CategoryCollectionStateStub().withSelection(selectionStub));
+        useSelectionStateStub.withUserSelection(selectionStub);
        const mockEvent = new TreeNodeStateChangedEmittedEventStub()
          .withNode(
            createTreeNodeStub({
@@ -103,24 +95,22 @@ describe('useCollectionSelectionStateUpdater', () => {
        // act
        returnObject.updateNodeSelection(mockEvent);
        // assert
-        const modifyCall = useStateStub.callHistory.find((call) => call.methodName === 'modifyCurrentState');
-        expect(modifyCall).toBeUndefined();
+        expect(useSelectionStateStub.isSelectionModified()).to.equal(false);
      });
    });
    describe('when checkState is unchecked', () => {
      it('removes from selection if already selected', () => {
        // arrange
-        const { returnObject, useStateStub } = mountWrapperComponent();
+        const { returnObject, useSelectionStateStub } = runHook();
        const selectionStub = new UserSelectionStub([]);
        selectionStub.isSelected = () => true;
-        useStateStub.withState(new CategoryCollectionStateStub().withSelection(selectionStub));
+        useSelectionStateStub.withUserSelection(selectionStub);
+        const node = createTreeNodeStub({
+          isBranch: false,
+          currentState: TreeNodeCheckState.Unchecked,
+        });
        const mockEvent = new TreeNodeStateChangedEmittedEventStub()
-          .withNode(
-            createTreeNodeStub({
-              isBranch: false,
-              currentState: TreeNodeCheckState.Unchecked,
-            }),
-          )
+          .withNode(node)
          .withCheckStateChange({
            oldState: TreeNodeCheckState.Checked,
            newState: TreeNodeCheckState.Unchecked,
@@ -128,17 +118,15 @@ describe('useCollectionSelectionStateUpdater', () => {
        // act
        returnObject.updateNodeSelection(mockEvent);
        // assert
-        const modifyCall = useStateStub.callHistory.find((call) => call.methodName === 'modifyCurrentState');
-        expect(modifyCall).toBeDefined();
-        const removeSelectedScriptCall = selectionStub.callHistory.find((call) => call.methodName === 'removeSelectedScript');
-        expect(removeSelectedScriptCall).toBeDefined();
+        expect(useSelectionStateStub.isSelectionModified()).to.equal(true);
+        expect(selectionStub.isScriptRemoved(node.id)).to.equal(true);
      });
      it('does nothing if not already selected', () => {
        // arrange
-        const { returnObject, useStateStub } = mountWrapperComponent();
+        const { returnObject, useSelectionStateStub } = runHook();
        const selectionStub = new UserSelectionStub([]);
        selectionStub.isSelected = () => false;
-        useStateStub.withState(new CategoryCollectionStateStub().withSelection(selectionStub));
+        useSelectionStateStub.withUserSelection(selectionStub);
        const mockEvent = new TreeNodeStateChangedEmittedEventStub()
          .withNode(
            createTreeNodeStub({
@@ -153,33 +141,18 @@ describe('useCollectionSelectionStateUpdater', () => {
        // act
        returnObject.updateNodeSelection(mockEvent);
        // assert
-        const modifyCall = useStateStub.callHistory.find((call) => call.methodName === 'modifyCurrentState');
-        expect(modifyCall).toBeUndefined();
+        expect(useSelectionStateStub.isSelectionModified()).to.equal(false);
      });
    });
  });
 });

-function mountWrapperComponent() {
-  const useStateStub = new UseCollectionStateStub();
-  let returnObject: ReturnType<typeof useCollectionSelectionStateUpdater>;
-
-  shallowMount({
-    setup() {
-      returnObject = useCollectionSelectionStateUpdater();
-    },
-    template: '<div></div>',
-  }, {
-    global: {
-      provide: {
-        [InjectionKeys.useCollectionState.key]: () => useStateStub.get(),
-      },
-    },
-  });
-
+function runHook() {
+  const useSelectionStateStub = new UseUserSelectionStateStub();
+  const returnObject = useCollectionSelectionStateUpdater(useSelectionStateStub.get());
  return {
    returnObject,
-    useStateStub,
+    useSelectionStateStub,
  };
 }

--- a/tests/unit/presentation/components/Scripts/View/Tree/TreeViewAdapter/UseSelectedScriptNodeIds.spec.ts
+++ b/tests/unit/presentation/components/Scripts/View/Tree/TreeViewAdapter/UseSelectedScriptNodeIds.spec.ts
@@ -1,39 +1,20 @@
 import { describe, it, expect } from 'vitest';
-import { shallowMount } from '@vue/test-utils';
-import { nextTick, watch } from 'vue';
 import { useSelectedScriptNodeIds } from '@/presentation/components/Scripts/View/Tree/TreeViewAdapter/UseSelectedScriptNodeIds';
-import { InjectionKeys } from '@/presentation/injectionSymbols';
-import { UseAutoUnsubscribedEventsStub } from '@tests/unit/shared/Stubs/UseAutoUnsubscribedEventsStub';
-import { UseCollectionStateStub } from '@tests/unit/shared/Stubs/UseCollectionStateStub';
-import { CategoryCollectionStateStub } from '@tests/unit/shared/Stubs/CategoryCollectionStateStub';
 import { SelectedScriptStub } from '@tests/unit/shared/Stubs/SelectedScriptStub';
 import { getScriptNodeId } from '@/presentation/components/Scripts/View/Tree/TreeViewAdapter/CategoryNodeMetadataConverter';
 import { IScript } from '@/domain/IScript';
-import { UserSelectionStub } from '@tests/unit/shared/Stubs/UserSelectionStub';
+import { UseUserSelectionStateStub } from '@tests/unit/shared/Stubs/UseUserSelectionStateStub';

 describe('useSelectedScriptNodeIds', () => {
  it('returns an empty array when no scripts are selected', () => {
    // arrange
-    const { useStateStub, returnObject } = mountWrapperComponent();
-    useStateStub.withState(new CategoryCollectionStateStub().withSelectedScripts([]));
+    const { useSelectionStateStub, returnObject } = runHook();
+    useSelectionStateStub.withSelectedScripts([]);
    // act
    const actualIds = returnObject.selectedScriptNodeIds.value;
    // assert
    expect(actualIds).to.have.lengthOf(0);
  });
-  it('initially registers the unsubscribe callback', () => {
-    // arrange
-    const eventsStub = new UseAutoUnsubscribedEventsStub();
-    // act
-    mountWrapperComponent({
-      useAutoUnsubscribedEvents: eventsStub,
-    });
-    // assert
-    const calls = eventsStub.events.callHistory;
-    expect(eventsStub.events.callHistory).has.lengthOf(1);
-    const call = calls.find((c) => c.methodName === 'unsubscribeAllAndRegister');
-    expect(call).toBeDefined();
-  });
  describe('returns correct node IDs for selected scripts', () => {
    it('immediately', () => {
      // arrange
@@ -45,12 +26,11 @@ describe('useSelectedScriptNodeIds', () => {
        [selectedScripts[0].script, 'expected-id-1'],
        [selectedScripts[1].script, 'expected-id-2'],
      ]);
-      const { useStateStub, returnObject } = mountWrapperComponent({
+      const useSelectionStateStub = new UseUserSelectionStateStub()
+        .withSelectedScripts(selectedScripts);
+      const { returnObject } = runHook({
        scriptNodeIdParser: createNodeIdParserFromMap(parsedNodeIds),
-      });
-      useStateStub.triggerOnStateChange({
-        newState: new CategoryCollectionStateStub().withSelectedScripts(selectedScripts),
-        immediateOnly: true,
+        useSelectionState: useSelectionStateStub,
      });
      // act
      const actualIds = returnObject.selectedScriptNodeIds.value;
@@ -59,35 +39,6 @@ describe('useSelectedScriptNodeIds', () => {
      expect(actualIds).to.have.lengthOf(expectedNodeIds.length);
      expect(actualIds).to.include.members(expectedNodeIds);
    });
-    it('when the collection state changes', () => {
-      // arrange
-      const initialScripts = [];
-      const changedScripts = [
-        new SelectedScriptStub('id-1'),
-        new SelectedScriptStub('id-2'),
-      ];
-      const parsedNodeIds = new Map<IScript, string>([
-        [changedScripts[0].script, 'expected-id-1'],
-        [changedScripts[1].script, 'expected-id-2'],
-      ]);
-      const { useStateStub, returnObject } = mountWrapperComponent({
-        scriptNodeIdParser: createNodeIdParserFromMap(parsedNodeIds),
-      });
-      useStateStub.triggerOnStateChange({
-        newState: new CategoryCollectionStateStub().withSelectedScripts(initialScripts),
-        immediateOnly: true,
-      });
-      // act
-      useStateStub.triggerOnStateChange({
-        newState: new CategoryCollectionStateStub().withSelectedScripts(changedScripts),
-        immediateOnly: false,
-      });
-      const actualIds = returnObject.selectedScriptNodeIds.value;
-      // assert
-      const expectedNodeIds = [...parsedNodeIds.values()];
-      expect(actualIds).to.have.lengthOf(expectedNodeIds.length);
-      expect(actualIds).to.include.members(expectedNodeIds);
-    });
    it('when the selection state changes', () => {
      // arrange
      const initialScripts = [];
@@ -99,18 +50,14 @@ describe('useSelectedScriptNodeIds', () => {
        [changedScripts[0].script, 'expected-id-1'],
        [changedScripts[1].script, 'expected-id-2'],
      ]);
-      const { useStateStub, returnObject } = mountWrapperComponent({
-        scriptNodeIdParser: createNodeIdParserFromMap(parsedNodeIds),
-      });
-      const userSelection = new UserSelectionStub([])
+      const useSelectionStateStub = new UseUserSelectionStateStub()
        .withSelectedScripts(initialScripts);
-      useStateStub.triggerOnStateChange({
-        newState: new CategoryCollectionStateStub()
-          .withSelection(userSelection),
-        immediateOnly: true,
+      const { returnObject } = runHook({
+        scriptNodeIdParser: createNodeIdParserFromMap(parsedNodeIds),
+        useSelectionState: useSelectionStateStub,
      });
      // act
-      userSelection.triggerSelectionChangedEvent(changedScripts);
+      useSelectionStateStub.withSelectedScripts(changedScripts);
      const actualIds = returnObject.selectedScriptNodeIds.value;
      // assert
      const expectedNodeIds = [...parsedNodeIds.values()];
@@ -118,96 +65,6 @@ describe('useSelectedScriptNodeIds', () => {
      expect(actualIds).to.include.members(expectedNodeIds);
    });
  });
-  describe('reactivity to state changes', () => {
-    describe('when the collection state changes', () => {
-      it('with new array references', async () => {
-        // arrange
-        const { useStateStub, returnObject } = mountWrapperComponent();
-        let isChangeTriggered = false;
-        watch(() => returnObject.selectedScriptNodeIds.value, () => {
-          isChangeTriggered = true;
-        });
-        // act
-        useStateStub.triggerOnStateChange({
-          newState: new CategoryCollectionStateStub(),
-          immediateOnly: false,
-        });
-        await nextTick();
-        // assert
-        expect(isChangeTriggered).to.equal(true);
-      });
-      it('with the same array reference', async () => {
-        // arrange
-        const sharedSelectedScriptsReference = [];
-        const initialCollectionState = new CategoryCollectionStateStub()
-          .withSelectedScripts(sharedSelectedScriptsReference);
-        const changedCollectionState = new CategoryCollectionStateStub()
-          .withSelectedScripts(sharedSelectedScriptsReference);
-        const { useStateStub, returnObject } = mountWrapperComponent();
-        useStateStub.triggerOnStateChange({
-          newState: initialCollectionState,
-          immediateOnly: true,
-        });
-        let isChangeTriggered = false;
-        watch(() => returnObject.selectedScriptNodeIds.value, () => {
-          isChangeTriggered = true;
-        });
-        // act
-        sharedSelectedScriptsReference.push(new SelectedScriptStub('new')); // mutate array using same reference
-        useStateStub.triggerOnStateChange({
-          newState: changedCollectionState,
-          immediateOnly: false,
-        });
-        await nextTick();
-        // assert
-        expect(isChangeTriggered).to.equal(true);
-      });
-    });
-    describe('when the selection state changes', () => {
-      it('with new array references', async () => {
-        // arrange
-        const { useStateStub, returnObject } = mountWrapperComponent();
-        const userSelection = new UserSelectionStub([])
-          .withSelectedScripts([]);
-        useStateStub.triggerOnStateChange({
-          newState: new CategoryCollectionStateStub()
-            .withSelection(userSelection),
-          immediateOnly: true,
-        });
-        let isChangeTriggered = false;
-        watch(() => returnObject.selectedScriptNodeIds.value, () => {
-          isChangeTriggered = true;
-        });
-        // act
-        userSelection.triggerSelectionChangedEvent([]);
-        await nextTick();
-        // assert
-        expect(isChangeTriggered).to.equal(true);
-      });
-      it('with the same array reference', async () => {
-        // arrange
-        const { useStateStub, returnObject } = mountWrapperComponent();
-        const sharedSelectedScriptsReference = [];
-        const userSelection = new UserSelectionStub([])
-          .withSelectedScripts(sharedSelectedScriptsReference);
-        useStateStub.triggerOnStateChange({
-          newState: new CategoryCollectionStateStub()
-            .withSelection(userSelection),
-          immediateOnly: true,
-        });
-        let isChangeTriggered = false;
-        watch(() => returnObject.selectedScriptNodeIds.value, () => {
-          isChangeTriggered = true;
-        });
-        // act
-        sharedSelectedScriptsReference.push(new SelectedScriptStub('new')); // mutate array using same reference
-        userSelection.triggerSelectionChangedEvent(sharedSelectedScriptsReference);
-        await nextTick();
-        // assert
-        expect(isChangeTriggered).to.equal(true);
-      });
-    });
-  });
 });

 type ScriptNodeIdParser = typeof getScriptNodeId;
@@ -222,33 +79,16 @@ function createNodeIdParserFromMap(scriptToIdMap: Map<IScript, string>): ScriptN
  };
 }

-function mountWrapperComponent(scenario?: {
+function runHook(scenario?: {
  readonly scriptNodeIdParser?: ScriptNodeIdParser,
-  readonly useAutoUnsubscribedEvents?: UseAutoUnsubscribedEventsStub,
+  readonly useSelectionState?: UseUserSelectionStateStub,
 }) {
-  const useStateStub = new UseCollectionStateStub();
+  const useSelectionStateStub = scenario?.useSelectionState ?? new UseUserSelectionStateStub();
  const nodeIdParser: ScriptNodeIdParser = scenario?.scriptNodeIdParser
    ?? ((script) => script.id);
-  let returnObject: ReturnType<typeof useSelectedScriptNodeIds>;
-
-  shallowMount({
-    setup() {
-      returnObject = useSelectedScriptNodeIds(nodeIdParser);
-    },
-    template: '<div></div>',
-  }, {
-    global: {
-      provide: {
-        [InjectionKeys.useCollectionState.key]:
-          () => useStateStub.get(),
-        [InjectionKeys.useAutoUnsubscribedEvents.key]:
-          () => (scenario?.useAutoUnsubscribedEvents ?? new UseAutoUnsubscribedEventsStub()).get(),
-      },
-    },
-  });
-
+  const returnObject = useSelectedScriptNodeIds(useSelectionStateStub.get(), nodeIdParser);
  return {
    returnObject,
-    useStateStub,
+    useSelectionStateStub,
  };
 }
--- a/tests/unit/presentation/components/Scripts/View/Tree/TreeViewAdapter/UseTreeViewFilterEvent.spec.ts
+++ b/tests/unit/presentation/components/Scripts/View/Tree/TreeViewAdapter/UseTreeViewFilterEvent.spec.ts
@@ -66,7 +66,7 @@ describe('UseTreeViewFilterEvent', () => {
          .withFilter(filterStub);
        const { returnObject } = mountWrapperComponent({ useStateStub: stateStub });
        let totalFilterUpdates = 0;
-        watch(() => returnObject.latestFilterEvent.value, () => {
+        watch(returnObject.latestFilterEvent, () => {
          totalFilterUpdates++;
        });
        // act