Merge branch 'gallery_filter' into 'v3'

Adds filtering to the gallery

See merge request openflexure/openflexure-microscope-server!621

src/openflexure_microscope_server/things/camera/__init__.py

@@ -295,11 +295,6 @@ class BaseCamera(OFMThing, ABC):
     # Register with gallery.
     _show_data_in_gallery = True
 
-    @property
-    def gallery_data_name(self) -> str:
-        """Name under which data shows up in gallery."""
-        return "Captures"
-
     @property
     def gallery_data_schema(self) -> type[CaptureInfo]:
         """The schema (BaseModel) for passing data to the gallery."""

src/openflexure_microscope_server/things/gallery.py

@@ -23,14 +23,14 @@ class GalleryCompatibleThing(Protocol):
     be picked up, but may throw an error when used.
     """
 
+    name: str
+
     # Ensure it is a Thing:
     _thing_server_interface: lt.ThingServerInterface
 
     # Ensure it is an OFMThing:
     show_data_in_gallery: bool
 
-    gallery_data_name: str
-
     gallery_data_schema: type[BaseModel]
 
     # Ignore D102: No docstrings for the protocol.
@@ -44,7 +44,9 @@ class GalleryThing(lt.Thing):
 
     all_ofm_things: Mapping[str, OFMThing] = lt.thing_slot()
 
-    _gallery_providing_things: Optional[Mapping[str, GalleryCompatibleThing]] = None
+    _gallery_providing_things: Optional[dict[str, GalleryCompatibleThing]] = None
+    _card_types: Optional[list[str]] = None
+    _card_type_map: dict[str, str] = {}
 
     @property
     def gallery_providing_things(self) -> Mapping[str, GalleryCompatibleThing]:
@@ -83,8 +85,45 @@ class GalleryThing(lt.Thing):
         # Cast the type of each thing to "GalleryCompatibleThing" as other Things have
         # been popped.
         self._gallery_providing_things = cast(
-            Mapping[str, GalleryCompatibleThing], gallery_providers
+            dict[str, GalleryCompatibleThing], gallery_providers
         )
+        self._set_card_types(self._gallery_providing_things)
+
+    def _set_card_types(
+        self, gallery_providers: dict[str, GalleryCompatibleThing]
+    ) -> None:
+        """Find the card types from each provider.
+
+        :param gallery_providers: The dictionary of gallery providing things. If card
+            data cannot be extracted for a Thing it will be popped from the provider
+            dictionary.
+        """
+        card_types: list[str] = []
+        # cache initial list of keys as it may change in the loop
+        keys = list(gallery_providers.keys())
+        for key in keys:
+            card_schema = gallery_providers[key].gallery_data_schema.schema()
+            props = card_schema["properties"]
+            if "card_type" not in props or "const" not in props["card_type"]:
+                self.logger.error(
+                    f"Data from {key} cannot be shown in gallery as data card doesn't "
+                    "have a static card_type."
+                )
+                gallery_providers.pop(key)
+                continue
+            card_type = props["card_type"]["const"]
+            if card_type not in card_types:
+                card_types.append(card_type)
+            self._card_type_map[key] = card_type
+
+        self._card_types = card_types
+
+    @lt.property
+    def card_types(self) -> list[str]:
+        """Names for the card types in the gallery."""
+        if self._card_types is None:
+            raise RuntimeError("Cannot access card_types before server has started.")
+        return self._card_types
 
     @lt.property
     def list_data(self) -> list[dict[str, Any]]:
@@ -108,10 +147,12 @@ class GalleryThing(lt.Thing):
         return data_list
 
     @lt.action
-    def delete_all_data(self) -> None:
+    def delete_all_data(self, card_types: list[str]) -> None:
-        """Delete all the gallery data on this microscope."""
+        """Delete all the gallery data on this microscope with the given card types."""
         for thing in self.gallery_providing_things.values():
-            try:
+            thing_card_type = self._card_type_map.get(thing.name)
-                thing.delete_all_gallery_items()
+            if thing_card_type in card_types:
-            except Exception as e:
+                try:
-                self.logger.exception(e)
+                    thing.delete_all_gallery_items()
+                except Exception as e:
+                    self.logger.exception(e)

src/openflexure_microscope_server/things/smart_scan.py

@@ -170,11 +170,6 @@ class SmartScanThing(OFMThing):
     # Register with gallery.
     _show_data_in_gallery = True
 
-    @property
-    def gallery_data_name(self) -> str:
-        """Name under which data shows up in gallery."""
-        return "Scans"
-
     @property
     def gallery_data_schema(self) -> type[scan_directories.ScanInfo]:
         """The schema (BaseModel) for passing data to the gallery."""

tests/unit_tests/test_gallery.py

@@ -2,7 +2,7 @@
 
 import logging
 from dataclasses import dataclass
-from typing import Callable, Optional
+from typing import Callable, Literal, Optional
 
 import pytest
 from pydantic import BaseModel
@@ -19,14 +19,19 @@ from openflexure_microscope_server.things.gallery import (
 from ..shared_utils.lt_test_utils import LabThingsTestEnv
 
 
-def test_error_if_accessing_gallery_things_before_started():
+def test_error_if_accessing_properties_before_started():
-    """Check that gallery_providing_things property errors before init."""
+    """Check that unfefined properties errors before enter."""
     thing = create_thing_without_server(GalleryThing)
     with pytest.raises(
         RuntimeError,
         match="Cannot access gallery_providing_things before server has started.",
     ):
         thing.gallery_providing_things
+    with pytest.raises(
+        RuntimeError,
+        match="Cannot access card_types before server has started.",
+    ):
+        thing.card_types
 
 
 class MockGalleryData(BaseModel):
@@ -34,6 +39,13 @@ class MockGalleryData(BaseModel):
 
     mock: str
     foobar: int
+    card_type: Literal["Mock"] = "Mock"
+
+
+class MockGalleryData2(MockGalleryData):
+    """Mock gallery data with a different card name."""
+
+    card_type: Literal["Mock2"] = "Mock2"
 
 
 class MinimalGalleryClass:
@@ -42,13 +54,11 @@ class MinimalGalleryClass:
     This should fail the protocol check as it is not an OFMThing
     """
 
-    gallery_data_name: str = "Mock"
-
     gallery_data_schema: type[BaseModel] = MockGalleryData
 
     def get_data_for_gallery(self) -> list[BaseModel]:
         """Return a list of Mock Gallery Data."""
-        return [MockGalleryData(mock="mock", foobar="foobar")]
+        return [self.gallery_data_schema(mock="mock", foobar="foobar")]
 
     def delete_all_gallery_items(self) -> None:
         """Mock deleting all the data."""
@@ -63,16 +73,20 @@ class MinimalGallerySource(OFMThing, MinimalGalleryClass):
     show_data_in_gallery = True
 
 
+class MinimalGallerySource2(MinimalGallerySource):
+    """Another Thing that can show data in the gallery with a different card type."""
+
+    gallery_data_schema: type[BaseModel] = MockGalleryData2
+
+
 class BadGallerySource(OFMThing):
     """An OFMThing that almost defines enough to show in the gallery.
 
-    Shouldn't match the protocol as ``gallery_data_name`` is missing.
+    Shouldn't match the protocol as ``gallery_data_schema`` is missing.
     """
 
     show_data_in_gallery = True
 
-    gallery_data_schema: type[BaseModel] = MockGalleryData
-
     def get_data_for_gallery(self) -> list[BaseModel]:
         """Return a list of Mock Gallery Data."""
         return [MockGalleryData(mock="mock", foobar="foobar")]
@@ -81,6 +95,31 @@ class BadGallerySource(OFMThing):
         """Mock deleting all the data."""
 
 
+class BadMockGalleryData(BaseModel):
+    """Mock gallery data without a card type."""
+
+    mock: str
+    foobar: int
+
+
+class BadGallerySource2(OFMThing):
+    """An OFMThing that defines enough to show in the gallery with bad card data.
+
+    Matches the protocol but will be rejected when setting card types.
+    """
+
+    show_data_in_gallery = True
+
+    gallery_data_schema: type[BaseModel] = BadMockGalleryData
+
+    def get_data_for_gallery(self) -> list[BaseModel]:
+        """Return a list of Mock Gallery Data."""
+        return [BadMockGalleryData(mock="mock", foobar="foobar")]
+
+    def delete_all_gallery_items(self) -> None:
+        """Mock deleting all the data."""
+
+
 def test_gallery_compatible_protocol():
     """Check the gallery compatible protocol detects compatible classes only."""
     # Minimal class has the gallery specific methods but is not an OFMThing.
@@ -118,18 +157,23 @@ def test_gallery_thing_finds_all_providers(simulation_test_env):
     assert camera in gallery.gallery_providing_things.values()
     assert smart_scan in gallery.gallery_providing_things.values()
 
+    # Also check the card types:
+    assert gallery.card_types == ["Capture", "Scan"]
+    assert gallery._card_type_map == {"camera": "Capture", "smart_scan": "Scan"}
+
     # Also check the runtime checkable protocol GalleryCompatibleThing works directly
     # when called with isinstance.
     assert not isinstance(snake_workflow, GalleryCompatibleThing)
     assert isinstance(smart_scan, GalleryCompatibleThing)
 
 
-def test_gallery_logs_for_bad_providers(caplog):
+@pytest.mark.parametrize("bad_source_class", [BadGallerySource, BadGallerySource2])
+def test_gallery_logs_for_bad_providers(bad_source_class, caplog):
     """Test that an error is logged if a gallery source doesn't match the protocol."""
     # Create a config with the minimal Thing and the bad Thing
     things = {
         "minimal_thing": MinimalGallerySource,
-        "bad_thing": BadGallerySource,
+        "bad_thing": bad_source_class,
         "gallery": GalleryThing,
     }
     with caplog.at_level(logging.INFO):
@@ -155,7 +199,7 @@ def minimal_gallery_env():
     """Return a minimal working environment for testing the gallery."""
     things = {
         "minimal_thing1": MinimalGallerySource,
-        "minimal_thing2": MinimalGallerySource,
+        "minimal_thing2": MinimalGallerySource2,
         "gallery": GalleryThing,
     }
     with LabThingsTestEnv(
@@ -178,8 +222,8 @@ def test_gallery_lists_data(minimal_gallery_env, mocker, caplog):
     ]
 
     thing_2_data = [
-        MockGalleryData(mock="thing_2", foobar=1),
+        MockGalleryData2(mock="thing_2", foobar=1),
-        MockGalleryData(mock="thing_2", foobar=2),
+        MockGalleryData2(mock="thing_2", foobar=2),
     ]
 
     # Also mock the get_data_for_gallery methods.
@@ -188,12 +232,12 @@ def test_gallery_lists_data(minimal_gallery_env, mocker, caplog):
 
     # Set the expected returns.
     thing_1_return = [
-        {"mock": "thing_1", "foobar": 1},
+        {"mock": "thing_1", "foobar": 1, "card_type": "Mock"},
-        {"mock": "thing_1", "foobar": 2},
+        {"mock": "thing_1", "foobar": 2, "card_type": "Mock"},
     ]
     thing_2_return = [
-        {"mock": "thing_2", "foobar": 1},
+        {"mock": "thing_2", "foobar": 1, "card_type": "Mock2"},
-        {"mock": "thing_2", "foobar": 2},
+        {"mock": "thing_2", "foobar": 2, "card_type": "Mock2"},
     ]
 
     # Check that the gallery returns the expected concatenated data
@@ -216,24 +260,53 @@ class GalleryDeleteTestCase:
     The default save kwargs assume a jpeg.
     """
 
+    cards_to_delete: list[str]
     thing_1_side_effect: Optional[Callable]
+    thing_1_call_count: int
     thing_2_call_count: int
     # Expect directly calling the action will raise the thing 1 side_effect error
     action_errors: bool
 
 
 GALLERY_DELETE_TEST_CASES = [
-    # No errors from thing 1, thing 2 gets called
+    # Delete both types of card.
     GalleryDeleteTestCase(
-        thing_1_side_effect=None, thing_2_call_count=1, action_errors=False
+        cards_to_delete=["Mock", "Mock2"],
+        thing_1_side_effect=None,
+        thing_1_call_count=1,
+        thing_2_call_count=1,
+        action_errors=False,
+    ),
+    # Delete first type of card.
+    GalleryDeleteTestCase(
+        cards_to_delete=["Mock"],
+        thing_1_side_effect=None,
+        thing_1_call_count=1,
+        thing_2_call_count=0,
+        action_errors=False,
+    ),
+    # Delete second type of card.
+    GalleryDeleteTestCase(
+        cards_to_delete=["Mock2"],
+        thing_1_side_effect=None,
+        thing_1_call_count=0,
+        thing_2_call_count=1,
+        action_errors=False,
     ),
     # Thing 1 errors, thing 2 is still called
     GalleryDeleteTestCase(
-        thing_1_side_effect=RuntimeError, thing_2_call_count=1, action_errors=False
+        cards_to_delete=["Mock", "Mock2"],
+        thing_1_side_effect=RuntimeError,
+        thing_1_call_count=1,
+        thing_2_call_count=1,
+        action_errors=False,
     ),
-    # Thing 1 raises invocation cancelled error. Thing 2 is not called. The InvocationError is raised
+    # Thing 1 raises invocation cancelled error. Thing 2 is not called.
+    # The InvocationError is raised
     GalleryDeleteTestCase(
+        cards_to_delete=["Mock", "Mock2"],
         thing_1_side_effect=InvocationCancelledError,
+        thing_1_call_count=1,
         thing_2_call_count=0,
         action_errors=True,
     ),
@@ -257,10 +330,10 @@ def test_gallery_calls_delete(test_case, minimal_gallery_env, mocker):
     # Call delete_all_data, checking for errors if the test case expects an error.
     if test_case.action_errors:
         with pytest.raises(test_case.thing_1_side_effect):
-            gallery.delete_all_data()
+            gallery.delete_all_data(test_case.cards_to_delete)
     else:
-        gallery.delete_all_data()
+        gallery.delete_all_data(test_case.cards_to_delete)
 
     # Check the call counts are as expected from the test case.
-    assert thing_1.delete_all_gallery_items.call_count == 1
+    assert thing_1.delete_all_gallery_items.call_count == test_case.thing_1_call_count
     assert thing_2.delete_all_gallery_items.call_count == test_case.thing_2_call_count

tests/unit_tests/test_simulated_camera.py

@@ -297,7 +297,6 @@ def test_gallery_responses(camera, mocker, caplog):
     with tempfile.TemporaryDirectory() as tmpdir, caplog.at_level(logging.INFO):
         camera._data_dir = tmpdir
 
-        assert camera.gallery_data_name == "Captures"
         assert camera.gallery_data_schema == CaptureInfo
 
         # When we start there are no captures

webapp/src/assets/less/variables.less

@@ -249,6 +249,48 @@
     }
 }
 
+// A button styled like text. For use in menus
+.ofm-text-button {
+  display: block;
+  background: none;
+  border: none;
+  padding: 8px 10px;
+  cursor: pointer;
+  color: @global-color;
+  width: 100%;
+  text-align: left;
+  font-weight: 600;
+}
+.hook-inverse() {
+    .ofm-text-button{
+        color: @inverse-global-color;
+    }
+}
+
+/*
+ * Opaque - button-like colours
+ *
+ * The buttons are transparent. If an element overlays others it should
+ * Look the same as the buttons, but have no transparency.
+ */
+.ofm-opaque-element{
+    background: #f5f5f5;
+}
+.hook-inverse() {
+    .ofm-opaque-element{
+        background: #282626;
+    }
+}
+
+.ofm-opaque-highlight:hover {
+  background: #fff;
+}
+.hook-inverse() {
+    .ofm-opaque-highlight:hover {
+        background: #000;
+    }
+}
+
 /*
  * Accordion
  */

webapp/src/components/appContent.vue

@@ -267,6 +267,7 @@ export default {
 
 #container-left {
   overflow: auto;
+  scrollbar-gutter: stable;
   background-color: rgba(180, 180, 180, 0.025);
   width: 100%;
   height: 100%;

webapp/src/components/genericComponents/multiSelectDropdown.vue

@@ -0,0 +1,150 @@
+<template>
+  <div ref="multiselect" class="multi-select">
+    <div
+      class="dropdown-top ofm-opaque-element ofm-opaque-highlight"
+      :class="{ open: isOpen }"
+      @click="isOpen = !isOpen"
+    >
+      <span class="dropdown-top-text">{{ title }}</span>
+      <span>{{ arrow }}</span>
+    </div>
+
+    <div v-if="isOpen" class="dropdown-menu ofm-opaque-element">
+      <button class="ofm-text-button ofm-opaque-highlight" @click="selectAll">Select All</button>
+      <button class="ofm-text-button ofm-opaque-highlight" @click="selectNone">Select None</button>
+      <hr class="dropdown-separator" />
+      <label v-for="option in options" :key="option" class="dropdown-item ofm-opaque-highlight">
+        <input
+          type="checkbox"
+          class="uk-checkbox"
+          :value="option"
+          :checked="modelValue.includes(option)"
+          @change="toggleOption(option)"
+        />
+        {{ option }}
+      </label>
+    </div>
+  </div>
+</template>
+
+<script>
+export default {
+  name: "MultiSelectDropdown",
+
+  props: {
+    modelValue: {
+      type: Array,
+      default: () => [],
+    },
+    options: {
+      type: Array,
+      default: () => [],
+    },
+    title: {
+      type: String,
+      default: "Filter",
+    },
+  },
+
+  emits: ["update:modelValue"],
+
+  data() {
+    return {
+      isOpen: false,
+    };
+  },
+
+  computed: {
+    arrow() {
+      return this.isOpen ? "▲" : "▼";
+    },
+  },
+
+  mounted() {
+    window.addEventListener("click", this.handleClickOutside);
+  },
+
+  beforeUnmount() {
+    window.removeEventListener("click", this.handleClickOutside);
+  },
+
+  methods: {
+    handleClickOutside(event) {
+      if (!this.$refs.multiselect.contains(event.target)) {
+        this.isOpen = false;
+      }
+    },
+    toggleOption(value) {
+      const selected = [...this.modelValue];
+
+      const index = selected.indexOf(value);
+
+      if (index > -1) {
+        selected.splice(index, 1);
+      } else {
+        selected.push(value);
+      }
+
+      this.$emit("update:modelValue", selected);
+    },
+
+    selectAll() {
+      this.$emit("update:modelValue", this.options);
+    },
+
+    selectNone() {
+      this.$emit("update:modelValue", []);
+    },
+  },
+};
+</script>
+
+<style lang="less" scoped>
+@import url("../../assets/less/variables.less");
+
+.multi-select {
+  position: relative;
+  width: 250px;
+}
+
+.dropdown-top {
+  border: 1px solid #ccc;
+  padding: 8px 12px;
+  border-radius: 4px;
+  cursor: pointer;
+  display: flex;
+  justify-content: space-between;
+  align-items: center;
+}
+
+.dropdown-top-text {
+  color: @global-primary-background;
+}
+
+.dropdown-top.open {
+  border-radius: 4px 4px 0 0;
+}
+
+.dropdown-menu {
+  position: absolute;
+  left: 0;
+  right: 0;
+  border-left: 1px solid #ccc;
+  border-right: 1px solid #ccc;
+  border-bottom: 1px solid #ccc;
+  border-radius: 0 0 4px 4px;
+  max-height: 250px;
+  overflow-y: auto;
+  z-index: 1000;
+}
+
+.dropdown-item {
+  display: block;
+  padding: 8px 12px;
+  cursor: pointer;
+}
+
+.dropdown-separator {
+  margin: 5px;
+}
+</style>

webapp/src/components/tabContentComponents/galleryContent.vue

@@ -1,10 +1,17 @@
 <template>
-  <div ref="galleryDisplay" class="galleryDisplay uk-padding uk-padding-remove-top">
+  <div ref="galleryDisplay" class="gallery-display uk-padding uk-padding-remove-top">
     <!-- Gallery nav bar -->
     <nav class="gallery-navbar uk-navbar-container uk-navbar-transparent" uk-navbar="mode: click">
       <!-- Right side buttons -->
       <div class="uk-navbar-right">
         <div class="uk-grid">
+          <div class="gallery-button">
+            <multi-select-dropdown
+              v-model="selectedCardTypes"
+              :options="allCardTypes"
+              title="Filter Gallery"
+            />
+          </div>
           <div class="gallery-button">
             <action-button
               class="uk-width-1-1"
@@ -25,11 +32,13 @@
               thing="gallery"
               action="delete_all_data"
               submit-label="Delete All"
+              :submit-data="{ card_types: selectedCardTypes }"
+              :is-disabled="totalPages == 0"
               :can-terminate="true"
               :button-primary="false"
               :modal-progress="true"
               :requires-confirmation="true"
-              :confirmation-message="'<p>Are you sure you want to delete all gallery data from the microscope?</p><p>This is <b>irreversible</b>!</p>'"
+              :confirmation-message="deleteAllConfirmationMessage"
               @error="modalError"
             />
           </div>
@@ -54,7 +63,7 @@
       <div class="gallery-grid uk-grid-match" uk-grid>
         <div v-if="noItems">
           <h2>Nothing to show</h2>
-          <p>There is no captured data to show..</p>
+          <p>There is no captured data to show.</p>
         </div>
         <div v-for="itemData in paginatedItems" :key="itemData.id">
           <gallery-card
@@ -75,14 +84,14 @@
 
 <script>
 import PaginateLinks from "@/components/genericComponents/paginateLinks.vue";
+import MultiSelectDropdown from "@/components/genericComponents/multiSelectDropdown.vue";
 import actionButton from "../labThingsComponents/actionButton.vue";
 import galleryCard from "./galleryComponents/galleryCard.vue";
 import galleryModal from "./galleryComponents/galleryViewer.vue";
 import { eventBus } from "../../eventBus.js";
 import { useIntersectionObserver } from "@vueuse/core";
 import { useSettingsStore } from "@/stores/settings.js";
-import { mapState, storeToRefs } from "pinia";
+import { mapState } from "pinia";
-import { watch } from "vue";
 
 // Export main app
 export default {
@@ -92,6 +101,7 @@ export default {
     galleryCard,
     galleryModal,
     PaginateLinks,
+    MultiSelectDropdown,
   },
 
   emits: ["scrollTop"],
@@ -103,33 +113,49 @@ export default {
       osdViewer: null,
       currentPage: 1,
       itemsPerPage: 18,
+      selectedCardTypes: [],
+      allCardTypes: [],
     };
   },
 
   computed: {
     ...mapState(useSettingsStore, ["baseUri", "ready"]),
+    filtered_items() {
+      return this.all_items.filter((item) => this.selectedCardTypes.includes(item.card_type));
+    },
     noItems() {
-      return !this.all_items || this.all_items?.length === 0;
+      return !this.filtered_items || this.filtered_items?.length === 0;
     },
     totalPages() {
-      return Math.ceil((this.all_items?.length || 0) / this.itemsPerPage);
+      return Math.ceil((this.filtered_items?.length || 0) / this.itemsPerPage);
     },
     paginatedItems() {
       const start = (this.currentPage - 1) * this.itemsPerPage;
-      return (this.all_items || []).slice(start, start + this.itemsPerPage);
+      return (this.filtered_items || []).slice(start, start + this.itemsPerPage);
+    },
+    deleteAllConfirmationMessage() {
+      return `
+        <p>Are you sure you want to delete all gallery data with the following types</p>
+        <ul>
+          ${this.selectedCardTypes.map((type) => `<li>${type}</li>`).join("\n")}
+        </ul>
+        <p>from the microscope?</p>
+        <p>This is <b>irreversible</b>!</p>
+      `;
+    },
+  },
+
+  watch: {
+    totalPages(newPageCount) {
+      if (this.currentPage > newPageCount) {
+        this.currentPage = Math.max(1, newPageCount);
+      } else if (this.currentPage < 1) {
+        this.currentPage == 1;
+      }
     },
   },
 
   async mounted() {
-    const store = useSettingsStore();
-    const { ready } = storeToRefs(store);
-    this.unwatchStoreFunction = watch(ready, (isReady) => {
-      if (isReady) {
-        this.refreshGallery();
-      } else {
-        this.all_items = [];
-      }
-    });
     useIntersectionObserver(
       this.$refs.galleryDisplay,
       ([{ isIntersecting }]) => {
@@ -139,6 +165,8 @@ export default {
         threshold: 0.0, // Adjust as needed
       },
     );
+    this.allCardTypes = await this.readThingProperty("gallery", "card_types");
+    this.selectedCardTypes = this.allCardTypes;
     // Update on mount (does nothing if not connected)
     await this.refreshGallery();
     // A global signal listener to perform a gallery refresh