From 50c9a6f15afaee643ba7b83d18c88cf3133e308b Mon Sep 17 00:00:00 2001
From: Brandon Haney <121782102+Brandon-Haney@users.noreply.github.com>
Date: Tue, 6 Oct 2026 22:32:45 -0500
Subject: [PATCH 3/6] Fix adding and deleting custom path mappings on the
Libraries tab
Delete targeted #path-mappings, which only existed on the retired Paths page,
so htmx dropped the request. Add targeted #orphan-mappings, which was only
rendered once a custom mapping existed, so the first one saved without
appearing. The Custom Mappings section is now always present and Add/Delete
re-render it, keeping every card's index current.
Cards address a mapping by list index, which shifts when any mapping is
deleted (including Remove on a library card). Cards now send the paths they
were rendered with and the server refuses a stale edit or delete rather than
acting on whichever mapping sits at that index; a library save that removes a
mapping also refreshes the custom cards out-of-band. Errors show in the page
alert area.
---
tests/test_custom_path_mappings.py | 251 ++++++++++++++++++
web/routers/settings.py | 114 +++++---
web/templates/settings/libraries.html | 21 +-
.../settings/partials/custom_mappings.html | 21 ++
.../settings/partials/path_mapping_card.html | 9 +-
5 files changed, 361 insertions(+), 55 deletions(-)
create mode 100644 tests/test_custom_path_mappings.py
create mode 100644 web/templates/settings/partials/custom_mappings.html
diff --git a/tests/test_custom_path_mappings.py b/tests/test_custom_path_mappings.py
new file mode 100644
index 00000000..5a4c9b13
--- /dev/null
+++ b/tests/test_custom_path_mappings.py
@@ -0,0 +1,251 @@
+"""Custom path mappings (not linked to a Plex library) on Settings → Libraries.
+
+Delete used to target #path-mappings, which only existed on the retired Paths
+page, so htmx dropped the request. Add targeted a container that was only
+rendered once a custom mapping existed. Cards address mappings by list index,
+which shifts whenever a mapping is deleted, so they also send the paths they
+were rendered with and the server refuses a stale card.
+
+Mounts only the settings router on a minimal app, backed by a real
+SettingsService over a temp settings file, with Plex library discovery mocked.
+"""
+
+import html
+import json
+import re
+import sys
+from unittest.mock import MagicMock, patch
+
+import pytest
+from fastapi import FastAPI
+from fastapi.testclient import TestClient
+
+sys.modules.setdefault('fcntl', MagicMock())
+for _mod in [
+ 'apscheduler', 'apscheduler.schedulers', 'apscheduler.schedulers.background',
+ 'apscheduler.triggers', 'apscheduler.triggers.cron', 'apscheduler.triggers.interval',
+ 'plexapi', 'plexapi.server',
+]:
+ sys.modules.setdefault(_mod, MagicMock())
+
+
+def _force_real_modules():
+ for name in ["web.config", "web.routers", "web.routers.settings",
+ "web.services", "web.services.settings_service", "web"]:
+ mod = sys.modules.get(name)
+ if isinstance(mod, MagicMock):
+ del sys.modules[name]
+ import web # noqa: F401
+ import web.config # noqa: F401
+ import web.routers.settings # noqa: F401
+ import web.services.settings_service # noqa: F401
+
+
+_force_real_modules()
+
+_MOVIES_LIB = {
+ "id": 4, "title": "Movies", "type": "movie", "type_label": "Movies",
+ "locations": ["/data/Movies/"],
+}
+
+
+def _mapping(name, plex, real, section_id=None):
+ m = {"name": name, "plex_path": plex, "real_path": real,
+ "cache_path": "/mnt/cache" + real[len("/mnt/user"):],
+ "cacheable": True, "enabled": True}
+ if section_id is not None:
+ m["section_id"] = section_id
+ return m
+
+
+MOVIES = _mapping("Movies", "/data/Movies/", "/mnt/user/Movies/", section_id=4)
+MOVIES_UHD = _mapping("Movies UHD", "/data/UHD/", "/mnt/user/UHD/", section_id=4)
+MUSIC = _mapping("Music", "/data/Music/", "/mnt/user/Music/")
+AUDIOBOOKS = _mapping("Audiobooks", "/data/Books/", "/mnt/user/Books/")
+
+
+@pytest.fixture
+def make_client(tmp_path):
+ def make(mappings):
+ settings_file = tmp_path / "plexcache_settings.json"
+ settings_file.write_text(json.dumps({
+ "PLEX_URL": "http://localhost:32400", "PLEX_TOKEN": "abc",
+ "valid_sections": [4], "path_mappings": mappings, "cache_dir": "/mnt/cache",
+ }, indent=2), encoding="utf-8")
+ return settings_file
+
+ patches = []
+
+ def client(mappings):
+ settings_file = make(mappings)
+ p1 = patch("web.services.settings_service.SETTINGS_FILE", settings_file)
+ p2 = patch("web.services.settings_service.DATA_DIR", tmp_path)
+ p1.start(); p2.start(); patches.extend([p1, p2])
+ from web.services.settings_service import SettingsService
+ svc = SettingsService()
+ svc._cached_settings = None
+ p3 = patch.object(svc, "get_plex_libraries", return_value=[_MOVIES_LIB])
+ p3.start(); patches.append(p3)
+ from web.routers import settings as settings_router
+ p4 = patch("web.routers.settings.get_settings_service", return_value=svc)
+ p4.start(); patches.append(p4)
+ app = FastAPI()
+ app.include_router(settings_router.router, prefix="/settings")
+ return TestClient(app), svc
+
+ yield client
+ for p in reversed(patches):
+ p.stop()
+
+
+def _names(svc):
+ return [m["name"] for m in svc._load_raw()["path_mappings"]]
+
+
+def _expected(m):
+ return {"expected_plex_path": m["plex_path"], "expected_real_path": m["real_path"]}
+
+
+def _edit_form(m, **changes):
+ form = {"name": m["name"], "plex_path": m["plex_path"], "real_path": m["real_path"],
+ "cache_path": m["cache_path"], "cacheable": "on", "enabled": "on", **_expected(m)}
+ form.update(changes)
+ return form
+
+
+# -- page wiring ------------------------------------------------------------
+
+def test_every_hx_target_on_the_libraries_page_exists(make_client):
+ """The Delete button pointed at an element this page never renders."""
+ client, _ = make_client([MOVIES, MOVIES_UHD, MUSIC])
+ page = client.get("/settings/libraries").text
+ # The alert container lives in settings/index.html's layout.
+ ids = set(re.findall(r'\bid="([^"]+)"', page))
+ targets = set(re.findall(r'hx-target="#([^"]+)"', page))
+ assert targets, "expected hx-target attributes on the page"
+ missing = sorted(t for t in targets if t not in ids)
+ assert not missing, missing
+
+
+def test_delete_url_carries_the_expected_paths(make_client):
+ """In the URL, not hx-vals: htmx 1.9 sends DELETE values in the body."""
+ kids = _mapping("Kids & Family", "/data/Kids & Family/", "/mnt/user/Kids/")
+ client, svc = make_client([MOVIES, MUSIC, kids])
+ page = client.get("/settings/libraries").text
+
+ def delete_url(index):
+ found = re.search(r'hx-delete="(/settings/paths/%d\?[^"]+)"' % index, page)
+ return html.unescape(found.group(1)) # what the browser sends
+
+ # The page's own URL deletes the right mapping, '&' in the path included.
+ r = client.delete(delete_url(2))
+ assert r.status_code == 200
+ assert _names(svc) == ["Movies", "Music"]
+
+ # Music's card is now stale-safe too: after removing Music elsewhere,
+ # its old URL is refused instead of deleting whatever sits at index 1.
+ svc.delete_path_mapping(1)
+ svc.add_path_mapping(AUDIOBOOKS)
+ r = client.delete(delete_url(1))
+ assert "changed since this page loaded" in r.text
+ assert _names(svc) == ["Movies", "Audiobooks"]
+
+
+def test_custom_section_rendered_even_when_empty(make_client):
+ client, _ = make_client([MOVIES])
+ page = client.get("/settings/libraries").text
+ assert 'id="custom-mappings"' in page
+ assert 'hx-target="#custom-mappings"' in page # the Add form
+ assert 'id="orphan-mappings"' not in page # no heading or list without cards
+
+
+# -- add --------------------------------------------------------------------
+
+def test_add_first_custom_mapping_returns_the_section(make_client):
+ client, svc = make_client([MOVIES])
+ r = client.post("/settings/paths", data={
+ "name": "Music", "plex_path": "/data/Music/", "real_path": "/mnt/user/Music/",
+ "cache_path": "/mnt/cache/Music/", "cacheable": "on", "enabled": "on"})
+ assert r.status_code == 200
+ assert r.text.lstrip().startswith('
' in r.text
+ assert 'hx-delete="/settings/paths/1?' in r.text
+
+
+def test_library_save_without_removal_leaves_custom_cards_alone(make_client):
+ client, _ = make_client([MOVIES, MUSIC])
+ form = {"name_0": "Movies", "plex_path_0": MOVIES["plex_path"], "real_path_0": MOVIES["real_path"],
+ "cache_path_0": MOVIES["cache_path"], "host_cache_path_0": "", "cacheable_0": "on"}
+ r = client.put("/settings/libraries/4/paths", data=form)
+ assert "hx-swap-oob" not in r.text
diff --git a/web/routers/settings.py b/web/routers/settings.py
index f2af8215..027d9403 100644
--- a/web/routers/settings.py
+++ b/web/routers/settings.py
@@ -4,6 +4,7 @@
import logging
import time
import uuid
+from html import escape as html_escape
import threading
from pathlib import Path
from typing import Dict, Any, List
@@ -425,6 +426,48 @@ def test_user_token(request: Request, form_data: ImmutableMultiDict = Depends(pa
return HTMLResponse(f'
Failed: {error}')
+def _custom_mappings(mappings: List[Dict[str, Any]]) -> List[Dict[str, Any]]:
+ """Mappings not linked to a Plex library, each tagged with its list index."""
+ custom = []
+ for i, m in enumerate(mappings):
+ if m.get("section_id") is None:
+ m_copy = dict(m)
+ m_copy["_index"] = i
+ custom.append(m_copy)
+ return custom
+
+
+def _render_custom_mappings(request: Request, mappings: List[Dict[str, Any]], oob: bool = False) -> str:
+ return templates.get_template("settings/partials/custom_mappings.html").render(
+ {"request": request, "orphan_mappings": _custom_mappings(mappings), "oob": oob})
+
+
+def _mapping_alert(level: str, message: str) -> HTMLResponse:
+ """Show a path mapping problem in the page alert area instead of the swap target."""
+ return HTMLResponse(
+ f"
{html_escape(message)}
",
+ headers={"HX-Retarget": "#settings-alert-container", "HX-Reswap": "innerHTML"},
+ )
+
+
+_STALE_MAPPING_MESSAGE = ("The path mappings changed since this page loaded. "
+ "Reload the page and try again.")
+
+
+def _is_stale_mapping(mappings: List[Dict[str, Any]], index: int,
+ expected_plex_path, expected_real_path) -> bool:
+ """Cards address a mapping by list index, which shifts when another mapping
+ is deleted. The card also sends the paths it was rendered with, so a stale
+ card is refused instead of acting on whichever mapping now sits at that index."""
+ if expected_plex_path is None and expected_real_path is None:
+ return False
+ if not 0 <= index < len(mappings):
+ return True
+ current = mappings[index]
+ return ((expected_plex_path is not None and current.get("plex_path") != expected_plex_path)
+ or (expected_real_path is not None and current.get("real_path") != expected_real_path))
+
+
@router.get("/paths", response_class=HTMLResponse)
def settings_paths(request: Request):
"""Path mappings tab — redirects to Libraries tab"""
@@ -469,19 +512,10 @@ def add_path_mapping(
success = settings_service.add_path_mapping(mapping)
if success:
- # Return the new mapping card with its index
- mappings = settings_service.get_path_mappings()
- index = len(mappings) - 1
- return templates.TemplateResponse(
- request,
- "settings/partials/path_mapping_card.html",
- {
- "mapping": mapping,
- "index": index,
- }
- )
+ # Re-render the whole Custom Mappings section (it may not exist yet)
+ return HTMLResponse(_render_custom_mappings(request, settings_service.get_path_mappings()))
else:
- return HTMLResponse("
Failed to add mapping
")
+ return _mapping_alert("error", "Failed to add mapping")
@router.put("/paths/{index}", response_class=HTMLResponse)
@@ -494,11 +528,17 @@ def update_path_mapping(
cache_path: str = Form(""),
host_cache_path: str = Form(""),
cacheable: str = Form(None),
- enabled: str = Form(None)
+ enabled: str = Form(None),
+ expected_plex_path: str = Form(None),
+ expected_real_path: str = Form(None),
):
"""Update an existing path mapping"""
settings_service = get_settings_service()
+ if _is_stale_mapping(settings_service.get_path_mappings(), index,
+ expected_plex_path, expected_real_path):
+ return _mapping_alert("warning", _STALE_MAPPING_MESSAGE)
+
# Non-blocking: log a warning if the cache_path looks risky (issue #136).
cache_path_warning = settings_service.warn_cache_path(cache_path)
if cache_path_warning:
@@ -529,26 +569,30 @@ def update_path_mapping(
}
)
else:
- return HTMLResponse("
Failed to update mapping
")
+ return _mapping_alert("error", "Failed to update mapping")
@router.delete("/paths/{index}", response_class=HTMLResponse)
-def delete_path_mapping(request: Request, index: int):
- """Delete a path mapping and return the updated list"""
+def delete_path_mapping(
+ request: Request,
+ index: int,
+ expected_plex_path: str = Query(None),
+ expected_real_path: str = Query(None),
+):
+ """Delete a custom path mapping and return the Custom Mappings section"""
settings_service = get_settings_service()
+ if _is_stale_mapping(settings_service.get_path_mappings(), index,
+ expected_plex_path, expected_real_path):
+ return _mapping_alert("warning", _STALE_MAPPING_MESSAGE)
+
success = settings_service.delete_path_mapping(index)
if success:
- # Return the full updated list with fresh indices
- mappings = settings_service.get_path_mappings()
- return templates.TemplateResponse(
- request,
- "settings/partials/path_mappings_list.html",
- {"mappings": mappings}
- )
+ # Fresh indices for every remaining card
+ return HTMLResponse(_render_custom_mappings(request, settings_service.get_path_mappings()))
else:
- return HTMLResponse("
Failed to delete mapping
")
+ return _mapping_alert("error", "Failed to delete mapping")
# =============================================================================
@@ -573,15 +617,13 @@ def settings_libraries(request: Request):
# Group mappings by section_id
library_mappings = {} # section_id -> list of mappings (with _index)
- orphan_mappings = [] # mappings without section_id
for i, m in enumerate(mappings):
- m_copy = dict(m)
- m_copy["_index"] = i
sid = m.get("section_id")
if sid is not None:
+ m_copy = dict(m)
+ m_copy["_index"] = i
library_mappings.setdefault(sid, []).append(m_copy)
- else:
- orphan_mappings.append(m_copy)
+ orphan_mappings = _custom_mappings(mappings)
# Build library cards
library_cards = []
@@ -760,11 +802,15 @@ async def update_library_paths(request: Request, section_id: int):
settings_service._rebuild_valid_sections(raw)
settings_service._save_raw(raw)
+ # Deleting shifted the list indices the Custom Mappings cards address,
+ # so refresh that section too (out-of-band).
+ custom_refresh = _render_custom_mappings(request, all_mappings, oob=True) if indices_to_delete else ""
+
# Re-render the full library card
libraries = settings_service.get_plex_libraries()
library = next((lib for lib in libraries if lib["id"] == section_id), None)
if not library:
- return HTMLResponse("
Saved
")
+ return HTMLResponse("
Saved
" + custom_refresh)
lib_maps = []
for i, m in enumerate(all_mappings):
@@ -780,11 +826,9 @@ async def update_library_paths(request: Request, section_id: int):
"has_mappings": bool(lib_maps),
}
- return templates.TemplateResponse(
- request,
- "settings/partials/library_card.html",
- {"card": card}
- )
+ card_html = templates.get_template("settings/partials/library_card.html").render(
+ {"request": request, "card": card})
+ return HTMLResponse(card_html + custom_refresh)
@router.put("/libraries/paths/{index}", response_class=HTMLResponse)
diff --git a/web/templates/settings/libraries.html b/web/templates/settings/libraries.html
index c7d4784c..39f064d5 100644
--- a/web/templates/settings/libraries.html
+++ b/web/templates/settings/libraries.html
@@ -22,22 +22,7 @@
Plex Libraries
{% endfor %}
- {% if orphan_mappings %}
-
-
-
- Custom Mappings
-
-
- These path mappings are not linked to a Plex library. They may have been created manually or before library linking was available.
-
-
- {% for mapping in orphan_mappings %}
- {% set index = mapping._index %}
- {% include "settings/partials/path_mapping_card.html" %}
- {% endfor %}
-
- {% endif %}
+ {% include "settings/partials/custom_mappings.html" %}
@@ -46,7 +31,9 @@
Add Custom Path Mapping
-