Fix case sensitivity, add tests, remove shift-click in tree filters

client/src/javascript/components/sidebar/LocationFilters.tsx

@@ -6,52 +6,46 @@ import {LocationTreeNode} from '@shared/types/Taxonomy';
 import SidebarFilter from './SidebarFilter';
 import TorrentFilterStore from '../../stores/TorrentFilterStore';
 
+const buildLocationFilterTree = (location: LocationTreeNode): ReactNode => {
+  if (location.children.length === 1 && location.containedCount === location.children[0].containedCount) {
+    const onlyChild = location.children[0];
+    const separator = onlyChild.fullPath.includes('/') ? '/' : '\\';
+    return buildLocationFilterTree({
+      ...onlyChild,
+      directoryName: location.directoryName + separator + onlyChild.directoryName,
+    });
+  }
+
+  const children = location.children.map(buildLocationFilterTree);
+
+  return (
+    <SidebarFilter
+      handleClick={(filter: string | '', event: KeyboardEvent | MouseEvent | TouchEvent) =>
+        TorrentFilterStore.setLocationFilters(filter, event)
+      }
+      count={location.containedCount}
+      key={location.fullPath}
+      isActive={
+        (location.fullPath === '' && !TorrentFilterStore.locationFilter.length) ||
+        TorrentFilterStore.locationFilter.includes(location.fullPath)
+      }
+      name={location.directoryName}
+      slug={location.fullPath}
+      size={location.containedSize}
+    >
+      {(children.length && children) || undefined}
+    </SidebarFilter>
+  );
+};
+
 const LocationFilters: FC = observer(() => {
   const {i18n} = useLingui();
 
-  const locations = Object.keys(TorrentFilterStore.taxonomy.locationCounts);
-
-  if (locations.length === 1 && locations[0] === '') {
+  if (TorrentFilterStore.taxonomy.locationTree.containedCount === 0) {
     return null;
   }
 
-  const buildLocationFilterTree = (location: LocationTreeNode): ReactNode => {
-    if (
-      location.children.length === 1 &&
-      TorrentFilterStore.taxonomy.locationCounts[location.fullPath] ===
-        TorrentFilterStore.taxonomy.locationCounts[location.children[0].fullPath]
-    ) {
-      const onlyChild = location.children[0];
-      const separator = onlyChild.fullPath.includes('/') ? '/' : '\\';
-      return buildLocationFilterTree({
-        ...onlyChild,
-        directoryName: location.directoryName + separator + onlyChild.directoryName,
-      });
-    }
-
-    const children = location.children.map(buildLocationFilterTree);
-
-    return (
-      <SidebarFilter
-        handleClick={(filter: string | '', event: KeyboardEvent | MouseEvent | TouchEvent) =>
-          TorrentFilterStore.setLocationFilters(filter, event)
-        }
-        count={TorrentFilterStore.taxonomy.locationCounts[location.fullPath] || 0}
-        key={location.fullPath}
-        isActive={
-          (location.fullPath === '' && !TorrentFilterStore.locationFilter.length) ||
-          TorrentFilterStore.locationFilter.includes(location.fullPath)
-        }
-        name={location.directoryName}
-        slug={location.fullPath}
-        size={TorrentFilterStore.taxonomy.locationSizes[location.fullPath]}
-      >
-        {(children.length && children) || undefined}
-      </SidebarFilter>
-    );
-  };
-
-  const filterElements = TorrentFilterStore.taxonomy.locationTree.map(buildLocationFilterTree);
+  const filterElements = buildLocationFilterTree(TorrentFilterStore.taxonomy.locationTree);
 
   const title = i18n._('filter.location.title');
 

client/src/javascript/stores/TorrentFilterStore.ts

@@ -15,9 +15,7 @@ class TorrentFilterStore {
   filterTrigger = false;
 
   taxonomy: Taxonomy = {
-    locationCounts: {},
-    locationSizes: {},
-    locationTree: [],
+    locationTree: {directoryName: '', fullPath: '', children: [], containedCount: 0, containedSize: 0},
     statusCounts: {},
     tagCounts: {},
     tagSizes: {},
@@ -62,9 +60,8 @@ class TorrentFilterStore {
   }
 
   setLocationFilters(filter: string | '', event: KeyboardEvent | MouseEvent | TouchEvent) {
-    const locations = Object.keys(this.taxonomy.locationCounts).sort((a, b) => a.localeCompare(b));
-
-    this.computeFilters(locations, this.locationFilter, filter, event);
+    // keys: [] to disable shift-clicking as it doesn't make sense in a tree
+    this.computeFilters([], this.locationFilter, filter, event);
     this.filterTrigger = !this.filterTrigger;
   }
 
@@ -103,7 +100,7 @@ class TorrentFilterStore {
   ) {
     if (newFilter === ('' as T)) {
       currentFilters.splice(0);
-    } else if (event.shiftKey) {
+    } else if (event.shiftKey && keys.length) {
       if (currentFilters.length) {
         const lastKey = currentFilters[currentFilters.length - 1];
         const lastKeyIndex = keys.indexOf(lastKey);

server/services/taxonomyService.test.ts

@@ -0,0 +1,104 @@
+import {LocationTreeNode} from '@shared/types/Taxonomy';
+import TaxonomyService from '../../server/services/taxonomyService';
+import {UserInDatabase} from '@shared/schema/Auth';
+import os from 'os';
+
+type LocationRecord = {[key: string]: LocationRecord | null};
+const toTreeNodes = (locations: LocationRecord, separator = '/', basePath = '') =>
+  Object.keys(locations).reduce((parentNodes, locationKey) => {
+    const fullPath = locationKey !== '' ? basePath + separator + locationKey : locationKey;
+    const subLocations = locations[locationKey];
+    if (subLocations) {
+      const parent = {
+        directoryName: locationKey,
+        fullPath: fullPath,
+        children: toTreeNodes(subLocations, separator, fullPath),
+        containedCount: 0,
+        containedSize: 0,
+      };
+      for (const child of parent.children) {
+        parent.containedCount += child.containedCount;
+        parent.containedSize += child.containedSize;
+      }
+      parentNodes.push(parent);
+    } else {
+      parentNodes.push({
+        directoryName: locationKey,
+        fullPath: fullPath,
+        children: [],
+        containedCount: '' !== locationKey ? 1 : 0,
+        containedSize: '' !== locationKey ? 10 : 0,
+      });
+    }
+    return parentNodes;
+  }, [] as LocationTreeNode[]);
+
+describe('taxonomyService', () => {
+  describe('incrementLocationCountsAndSizes() - locationTree', () => {
+    for (const locationsAndExpected of [
+      // No torrents
+      {locations: [] as string[], expected: toTreeNodes({'': null})[0]},
+      // Single root
+      {
+        locations: ['/mnt/dir1/file1', '/mnt/dir1/file2', '/mnt/dir2/file3', '/mnt/file4'],
+        expected: toTreeNodes({
+          '': {
+            mnt: {dir1: {file1: null, file2: null}, dir2: {file3: null}, file4: null},
+          },
+        })[0],
+      },
+      // Multiple roots including overlapping case
+      {
+        locations: ['/mnt/file1', '/mnt/file2', '/mount/directory1/file3', '/Mount/directory2/file4'],
+        expected: toTreeNodes({
+          '': {
+            mnt: {file1: null, file2: null},
+            mount: {directory1: {file3: null}},
+            Mount: {directory2: {file4: null}},
+          },
+        })[0],
+      },
+    ]) {
+      const {locations, expected} = locationsAndExpected;
+
+      it(`builds Linux-style case-sensitive location tree correctly from ${locations}`, () => {
+        const taxonomyService = new TaxonomyService({} as UserInDatabase);
+        const mock = jest.spyOn(os, 'platform');
+        mock.mockImplementation(() => 'linux');
+
+        for (const location of locations) taxonomyService.incrementLocationCountsAndSizes(location, 10);
+
+        expect(taxonomyService.taxonomy.locationTree).toMatchObject(expected);
+
+        mock.mockRestore();
+      });
+    }
+
+    for (const locationsAndExpected of [
+      // Multiple roots including overlapping case
+      {
+        locations: ['/mnt/file1', '/mnt/file2', '/mount/directory1/file3', '/Mount/directory2/file4'],
+        expected: toTreeNodes({
+          '': {
+            mnt: {file1: null, file2: null},
+            mount: {directory1: {file3: null}, directory2: {file4: null}},
+          },
+        })[0],
+      },
+    ]) {
+      const {locations, expected} = locationsAndExpected;
+
+      it(`builds Mac-style case-insensitive location tree correctly from ${locations}`, () => {
+        const taxonomyService = new TaxonomyService({} as UserInDatabase);
+        const mock = jest.spyOn(os, 'platform');
+        mock.mockImplementation(() => 'darwin');
+
+        for (const location of locations) taxonomyService.incrementLocationCountsAndSizes(location, 10);
+
+        expect(taxonomyService.taxonomy.locationTree).toMatchObject(expected);
+
+        mock.mockRestore();
+      });
+    }
+  });
+});