fix(locations): include empty Locations-page entries (#854)

The integration only read locations from GET /api/v1/location, which
returns the distinct location values referenced by at least one spool.
Locations created on Spoolman's Locations settings page but not yet
assigned to a spool (e.g. 'External Spool Holder', 'Ordered') were
therefore missing from the Home Assistant location selector.

Add SpoolmanAPI.get_setting_locations(), reading the full UI-managed
list from GET /api/v1/setting/locations (a Setting whose JSON-encoded
'value' is a string array, decoded here). The coordinator now fetches
both endpoints independently and merges them, so empty locations show
up while spool-only locations are still covered. Each source degrades
gracefully on older Spoolman servers (404), unset/empty value and
malformed JSON; when both are unavailable it falls back to deriving
locations from spools as before.

Tests: API-level parsing/edge cases, coordinator merge + fallback, and
the characterization snapshot now lists the empty locations.

To the reporter: sorry — I misread the original report and the earlier
fix (switching to /api/v1/location in #839) didn't actually cover the
empty Locations-page entries you described. This should now be fixed.
Thanks for the precise write-up pointing at /api/v1/setting/locations.

Author: Disane87

custom_components/spoolman/classes/spoolman_api.py

@@ -88,10 +88,13 @@ async def backup(  # pragma: no cover — not called by the integration
             return payload
 
     async def get_locations(self) -> list[str]:
-        """Return the list of configured spool locations from Spoolman.
+        """Return the spool-referenced locations from Spoolman.
 
-        Uses ``GET /api/v1/location`` so that locations without any assigned
-        spool (e.g. empty AMS trays) are still selectable in HA.
+        Uses ``GET /api/v1/location``, which only lists location values that
+        are currently assigned to at least one spool. Locations created on
+        the Spoolman *Locations* settings page but not yet used by any spool
+        are NOT returned here — see :meth:`get_setting_locations` for the full
+        UI-managed list (issue #854).
         """
         _LOGGER.debug("SpoolmanAPI: get_locations")
         url = f"{self.base_url}/location"
@@ -102,6 +105,39 @@ async def get_locations(self) -> list[str]:
             _LOGGER.debug("SpoolmanAPI: get_locations response %s", payload)
             return [str(loc) for loc in payload if loc]
 
+    async def get_setting_locations(self) -> list[str]:
+        """Return the full Locations list from Spoolman's settings page.
+
+        Uses ``GET /api/v1/setting/locations``. Unlike ``/api/v1/location``
+        (which only lists locations referenced by a spool), this holds the
+        complete UI-managed list — including empty locations such as
+        "External Spool Holder" or "Ordered" (issue #854).
+
+        Spoolman returns a ``Setting`` object whose ``value`` is a
+        JSON-encoded string array (a string containing e.g. ``["Ordered"]``),
+        which we decode here. Robust against older servers (404), an
+        unset/empty value and malformed JSON — callers get an empty list in
+        those cases.
+        """
+        _LOGGER.debug("SpoolmanAPI: get_setting_locations")
+        url = f"{self.base_url}/setting/locations"
+        session = await self._get_session()
+        async with session.get(url) as response:
+            response.raise_for_status()
+            payload: dict[str, Any] = await response.json()
+            _LOGGER.debug("SpoolmanAPI: get_setting_locations response %s", payload)
+            raw = payload.get("value")
+            if not raw:
+                return []
+            try:
+                parsed = json.loads(raw) if isinstance(raw, str) else raw
+            except (ValueError, TypeError):
+                _LOGGER.debug("SpoolmanAPI: could not parse locations value %r", raw)
+                return []
+            if not isinstance(parsed, list):
+                return []
+            return [str(loc) for loc in parsed if loc]
+
     async def get_extra_fields(self, entity_type: str) -> dict[str, dict[str, Any]]:
         """Return extra-field metadata for the given entity type (e.g. ``spool``).
 

custom_components/spoolman/coordinator.py

@@ -105,13 +105,32 @@ async def _async_update_data(self) -> CoordinatorData:
                     "Could not fetch spool extra-field metadata: %s", exception
                 )
                 spool_extra_fields = {}
+            # Merge two location sources (issue #854):
+            #   * /setting/locations — the full UI-managed Locations page,
+            #     including empty locations not used by any spool.
+            #   * /location — locations actually referenced by spools, which
+            #     can include values absent from the settings list.
+            # Either endpoint may be missing on older Spoolman versions, so
+            # each is fetched independently and failures degrade gracefully.
             try:
-                locations = await self.spoolman_api.get_locations()
+                setting_locations = await self.spoolman_api.get_setting_locations()
+            except Exception as exception:
+                _LOGGER.debug("Could not fetch setting locations: %s", exception)
+                setting_locations = None
+            try:
+                spool_locations = await self.spoolman_api.get_locations()
             except Exception as exception:
                 # /location endpoint isn't on every Spoolman version; fall back
                 # to deriving from spools at the consumer side.
                 _LOGGER.debug("Could not fetch locations: %s", exception)
+                spool_locations = None
+            if setting_locations is None and spool_locations is None:
                 locations = None
+            else:
+                merged: set[str] = set()
+                merged.update(setting_locations or [])
+                merged.update(spool_locations or [])
+                locations = sorted(merged)
         except asyncio.CancelledError:  # pragma: no cover — shutdown path
             _LOGGER.debug("Data update was cancelled")
             raise

tests/conftest.py

@@ -112,6 +112,16 @@ def locations_data() -> list[str]:
     return _load_fixture("locations.json")
 
 
+@pytest.fixture
+def setting_locations_data() -> list[str]:
+    """Return the full UI Locations list served by ``GET /setting/locations``.
+
+    Includes empty locations ("External Spool Holder", "Ordered") absent from
+    the spool-referenced ``/location`` feed (issue #854).
+    """
+    return _load_fixture("setting_locations.json")
+
+
 @pytest.fixture
 def extra_fields_data() -> list[dict[str, Any]]:
     """Return extra-field metadata served by ``GET /field/spool``."""
@@ -123,6 +133,7 @@ def mock_spoolman_api(
     spools_data: list[dict[str, Any]],
     filaments_data: list[dict[str, Any]],
     locations_data: list[str],
+    setting_locations_data: list[str],
     extra_fields_data: list[dict[str, Any]],
 ) -> AsyncGenerator[aioresponses, None]:
     """Mock the Spoolman HTTP API. Default = healthy server, all endpoints respond."""
@@ -149,6 +160,13 @@ def mock_spoolman_api(
         mocked.get(
             re.compile(rf"{base}/filament.*"), payload=filaments_data, repeat=True
         )
+        # /setting/locations is registered before /location so the more
+        # specific Setting endpoint is matched first (issue #854).
+        mocked.get(
+            re.compile(rf"{base}/setting/locations"),
+            payload={"value": json.dumps(setting_locations_data), "is_set": True},
+            repeat=True,
+        )
         mocked.get(re.compile(rf"{base}/location"), payload=locations_data, repeat=True)
         mocked.get(
             re.compile(rf"{base}/field/spool"), payload=extra_fields_data, repeat=True

tests/fixtures/setting_locations.json

@@ -0,0 +1 @@
+["Shelf A", "Shelf B", "Drawer 1", "External Spool Holder", "Ordered"]

tests/snapshots/test_characterization.ambr

@@ -38,6 +38,8 @@
         'icon': 'mdi:map-marker',
         'options': list([
           'Drawer 1',
+          'External Spool Holder',
+          'Ordered',
           'Shelf A',
           'Shelf B',
         ]),
@@ -55,6 +57,8 @@
         'icon': 'mdi:map-marker',
         'options': list([
           'Drawer 1',
+          'External Spool Holder',
+          'Ordered',
           'Shelf A',
           'Shelf B',
         ]),

tests/test_api.py

@@ -86,3 +86,42 @@ async def test_close_only_closes_owned_session() -> None:
     await api.close()
     assert not session.closed
     await session.close()
+
+
+async def test_get_setting_locations_parses_json_value(hass: HomeAssistant) -> None:
+    """#854: the Setting's JSON-encoded ``value`` is decoded to a list of names."""
+    with aioresponses() as m:
+        m.get(
+            re.compile(rf"{re.escape(MOCK_URL)}api/v1/setting/locations"),
+            payload={"value": '["Ordered", "External Spool Holder"]', "is_set": True},
+        )
+        api = SpoolmanAPI(MOCK_URL, session=async_get_clientsession(hass))
+        assert await api.get_setting_locations() == ["Ordered", "External Spool Holder"]
+
+
+@pytest.mark.parametrize(
+    "payload",
+    [
+        {"value": "", "is_set": False},  # unset → empty default
+        {"value": None},  # explicit null
+        {},  # no value key at all
+        {"value": "{not json"},  # malformed JSON
+        {"value": '"a string"'},  # valid JSON but not a list
+        {"value": '["Ordered", "", null, "Ordered"]'},  # falsy entries dropped
+    ],
+)
+async def test_get_setting_locations_handles_edge_values(
+    hass: HomeAssistant, payload: dict[str, object]
+) -> None:
+    """Unset / malformed / non-list values degrade to a sane list, never raise."""
+    with aioresponses() as m:
+        m.get(
+            re.compile(rf"{re.escape(MOCK_URL)}api/v1/setting/locations"),
+            payload=payload,
+        )
+        api = SpoolmanAPI(MOCK_URL, session=async_get_clientsession(hass))
+        result = await api.get_setting_locations()
+
+    assert isinstance(result, list)
+    # The only payload yielding entries keeps non-falsy strings, duplicates intact:
+    assert result in ([], ["Ordered", "Ordered"])

tests/test_coordinator.py

@@ -69,6 +69,11 @@ async def test_extra_fields_endpoint_404_degrades(
             payload=filaments_data,
             repeat=True,
         )
+        m.get(
+            re.compile(rf"{re.escape(MOCK_URL)}api/v1/setting/locations.*"),
+            status=404,
+            repeat=True,
+        )
         m.get(
             re.compile(rf"{re.escape(MOCK_URL)}api/v1/location.*"),
             payload=locations_data,
@@ -92,7 +97,7 @@ async def test_locations_fallback_to_derived(
     spools_data: list[dict[str, Any]],
     filaments_data: list[dict[str, Any]],
 ) -> None:
-    """No /location endpoint → derive from spool.location values."""
+    """Neither /setting/locations nor /location → derive from spool.location."""
     config_entry.add_to_hass(hass)
     with aioresponses() as m:
         m.get(
@@ -105,6 +110,11 @@ async def test_locations_fallback_to_derived(
             payload=filaments_data,
             repeat=True,
         )
+        m.get(
+            re.compile(rf"{re.escape(MOCK_URL)}api/v1/setting/locations.*"),
+            status=404,
+            repeat=True,
+        )
         m.get(
             re.compile(rf"{re.escape(MOCK_URL)}api/v1/location.*"),
             status=404,
@@ -123,6 +133,108 @@ async def test_locations_fallback_to_derived(
     assert set(coord.data["locations"]) == {"Shelf A", "Shelf B"}
 
 
+async def test_locations_merge_setting_and_spool(
+    hass: HomeAssistant,
+    config_entry: MockConfigEntry,
+    mock_spoolman_api: aioresponses,
+) -> None:
+    """#854: empty Locations-page entries are merged with spool-referenced ones."""
+    config_entry.add_to_hass(hass)
+    coord = SpoolManCoordinator(hass, config_entry)
+    await coord.async_refresh()
+
+    assert coord.last_update_success
+    locations = set(coord.data["locations"])
+    # Empty locations only present in /setting/locations (the bug in #854):
+    assert {"External Spool Holder", "Ordered"} <= locations
+    # ...merged with the /location feed ("Drawer 1") and spool-derived ones:
+    assert {"Shelf A", "Shelf B", "Drawer 1"} <= locations
+
+
+async def test_setting_locations_404_falls_back_to_location(
+    hass: HomeAssistant,
+    config_entry: MockConfigEntry,
+    spools_data: list[dict[str, Any]],
+    filaments_data: list[dict[str, Any]],
+    locations_data: list[str],
+) -> None:
+    """Older Spoolman without /setting/locations still uses the /location feed."""
+    config_entry.add_to_hass(hass)
+    with aioresponses() as m:
+        m.get(
+            re.compile(rf"{re.escape(MOCK_URL)}api/v1/spool.*"),
+            payload=[s for s in spools_data if not s.get("archived")],
+            repeat=True,
+        )
+        m.get(
+            re.compile(rf"{re.escape(MOCK_URL)}api/v1/filament.*"),
+            payload=filaments_data,
+            repeat=True,
+        )
+        m.get(
+            re.compile(rf"{re.escape(MOCK_URL)}api/v1/setting/locations.*"),
+            status=404,
+            repeat=True,
+        )
+        m.get(
+            re.compile(rf"{re.escape(MOCK_URL)}api/v1/location.*"),
+            payload=locations_data,
+            repeat=True,
+        )
+        m.get(
+            re.compile(rf"{re.escape(MOCK_URL)}api/v1/field/spool.*"),
+            payload=[],
+            repeat=True,
+        )
+        coord = SpoolManCoordinator(hass, config_entry)
+        await coord.async_refresh()
+
+    assert coord.last_update_success
+    assert set(coord.data["locations"]) == {"Shelf A", "Shelf B", "Drawer 1"}
+
+
+async def test_setting_locations_malformed_value_degrades(
+    hass: HomeAssistant,
+    config_entry: MockConfigEntry,
+    spools_data: list[dict[str, Any]],
+    filaments_data: list[dict[str, Any]],
+    locations_data: list[str],
+) -> None:
+    """A malformed Setting value doesn't crash; /location still contributes."""
+    config_entry.add_to_hass(hass)
+    with aioresponses() as m:
+        m.get(
+            re.compile(rf"{re.escape(MOCK_URL)}api/v1/spool.*"),
+            payload=[s for s in spools_data if not s.get("archived")],
+            repeat=True,
+        )
+        m.get(
+            re.compile(rf"{re.escape(MOCK_URL)}api/v1/filament.*"),
+            payload=filaments_data,
+            repeat=True,
+        )
+        m.get(
+            re.compile(rf"{re.escape(MOCK_URL)}api/v1/setting/locations.*"),
+            payload={"value": "{not valid json", "is_set": True},
+            repeat=True,
+        )
+        m.get(
+            re.compile(rf"{re.escape(MOCK_URL)}api/v1/location.*"),
+            payload=locations_data,
+            repeat=True,
+        )
+        m.get(
+            re.compile(rf"{re.escape(MOCK_URL)}api/v1/field/spool.*"),
+            payload=[],
+            repeat=True,
+        )
+        coord = SpoolManCoordinator(hass, config_entry)
+        await coord.async_refresh()
+
+    assert coord.last_update_success
+    assert set(coord.data["locations"]) == {"Shelf A", "Shelf B", "Drawer 1"}
+
+
 async def test_total_remaining_weight_calculated(
     hass: HomeAssistant,
     config_entry: MockConfigEntry,