From a8d9b6026fea03d7085146c92d049126dbe33964 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 20:35:11 +0000 Subject: [PATCH] fix: stop a film's save from erasing its custom field values The edit form read its stored custom values from `detail.customValues`. `movie_detail_entity` attaches them as `detail.movie.custom_values`; `customValues` exists only on sync-mutation results and on the PUT response, and nothing ever sets it on a detail payload. So every input rendered empty on every load. That is not a display fault on its own. `collectMovieEditCustomValues` turns an empty input into a null, `replace_movie_custom_values` deletes the row for a null, and `saveMovieCustomValues` sends the whole collected map with every save. Opening a film, correcting its title and pressing save therefore erased every custom value on that film. Nothing errored, nothing logged, and the loss was only visible to whoever went looking for a value they had entered earlier. The two commits that introduced it are a pair: 8b91677 attached the values to the detail entity, fb19a1e read them back under a different name. No test touched the read path, so both halves looked finished. Both sides are now asserted, because a rename on either one alone restores the same silence: `test_next_custom_fields_ui.py` pins the accessor, and `test_next_custom_fields_routes.py` pins the payload key by writing a value and reading it back off `GET /api/next/movies/`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PUwMZZ585LnTxaWMUuKVgT --- app/backend/next_views_ui.py | 8 +++- .../tests/test_next_custom_fields_routes.py | 26 +++++++++++ .../tests/test_next_custom_fields_ui.py | 44 +++++++++++++++++++ 3 files changed, 77 insertions(+), 1 deletion(-) diff --git a/app/backend/next_views_ui.py b/app/backend/next_views_ui.py index b93f087f..35d3dc2e 100644 --- a/app/backend/next_views_ui.py +++ b/app/backend/next_views_ui.py @@ -32127,8 +32127,14 @@ class shipped unstyled and fell back to the browser default button. */ const container = document.getElementById("movieEditCustomFields"); if (!container) return; const definitions = customFieldDefinitions().filter((field) => !field.archivedAt); + // `detail.movie.custom_values`, which is where `movie_detail_entity` + // attaches them. This read `detail.customValues`, a key only sync + // mutation results and the PUT response ever carry -- so every input + // rendered empty, `collectMovieEditCustomValues` turned each empty input + // into a null, and saving a film for any unrelated reason deleted every + // custom value it had. Silent on both sides. const values = new Map( - (detail?.customValues || []).map((item) => [String(item.key), item.value]) + (detail?.movie?.custom_values || []).map((item) => [String(item.key), item.value]) ); container.classList.toggle("hidden", !definitions.length); container.innerHTML = definitions.map((field) => { diff --git a/app/backend/tests/test_next_custom_fields_routes.py b/app/backend/tests/test_next_custom_fields_routes.py index a775962e..60572e45 100644 --- a/app/backend/tests/test_next_custom_fields_routes.py +++ b/app/backend/tests/test_next_custom_fields_routes.py @@ -191,6 +191,32 @@ def test_clearing_a_value_removes_the_row(self): self.assertEqual(response.status_code, 200, response.get_data(as_text=True)) self.assertEqual(response.get_json()["customValues"], []) + def test_a_stored_value_comes_back_on_the_detail_payload_the_edit_form_reads(self): + """The round trip, not just the write. + + The write always worked. What did not was reading it back: the edit form + looked for `detail.customValues`, and the detail payload attaches + `detail.movie.custom_values`. Every input rendered empty, an empty input + is sent as a null, and a null deletes the row -- so saving a film for + any unrelated reason erased its custom values. + + Asserting the payload key here and the accessor in + `test_next_custom_fields_ui.py` is deliberate: a rename on either side + alone would restore exactly the same silence. + """ + field = self._field() + movie_id = self._movie() + self.client.put( + f"/api/next/movies/{movie_id}/custom-values", + json={"values": {field["key"]: "Shelf B"}}, + ) + response = self.client.get(f"/api/next/movies/{movie_id}") + self.assertEqual(response.status_code, 200, response.get_data(as_text=True)) + detail = response.get_json()["detail"] + self.assertNotIn("customValues", detail) + values = {row["key"]: row["value"] for row in detail["movie"]["custom_values"]} + self.assertEqual(values.get(field["key"]), "Shelf B") + def test_a_value_on_a_missing_movie_is_a_404(self): field = self._field() response = self.client.put( diff --git a/app/backend/tests/test_next_custom_fields_ui.py b/app/backend/tests/test_next_custom_fields_ui.py index dd5f85af..5e85d869 100644 --- a/app/backend/tests/test_next_custom_fields_ui.py +++ b/app/backend/tests/test_next_custom_fields_ui.py @@ -143,6 +143,50 @@ def test_definitions_reach_the_spa_on_the_snapshot(self): self.assertIn("state?.customFields", block) +class TheEditFormReadsTheKeyTheServerSendsTests(unittest.TestCase): + """The stored values have to arrive from the payload that carries them. + + `renderMovieEditCustomFields` read `detail.customValues`. `movie_detail_entity` + attaches them as `detail.movie.custom_values`; `customValues` exists only on + sync-mutation results and on the PUT response, and nothing ever assigns it + on a detail payload. So every input rendered empty on every load. + + That is not merely a display fault. `collectMovieEditCustomValues` turns an + empty input into `null`, `replace_movie_custom_values` deletes the row for a + `null`, and `saveMovieCustomValues` sends the whole collected map on every + save -- so opening a film, changing its title and pressing save silently + erased every custom value on that film. Nothing errored and nothing logged; + the values were simply gone the next time anyone looked. + + Both halves are asserted, because a rename on either side reintroduces the + same silence: the accessor here, and the key the server actually attaches. + """ + + @classmethod + def setUpClass(cls): + cls.source = _source() + + def test_the_renderer_reads_the_detail_payloads_own_key(self): + start = self.source.index("function renderMovieEditCustomFields") + block = self.source[start : start + 3200] + self.assertIn("detail?.movie?.custom_values", block) + + def test_the_renderer_does_not_read_a_key_no_detail_payload_carries(self): + start = self.source.index("function renderMovieEditCustomFields") + block = self.source[start : start + 3200] + self.assertNotIn("detail?.customValues", block) + + def test_the_server_attaches_the_values_the_renderer_reads(self): + with open(os.path.join(BACKEND_DIR, "next_app.py"), encoding="utf-8") as handle: + source = handle.read() + start = source.index("def movie_detail_entity") + block = source[start : start + 900] + self.assertIn("attach_movie_custom_values(conn, [movie])", block) + start = source.index("def attach_movie_custom_values") + block = source[start : start + 2500] + self.assertIn('"custom_values"', block) + + class SnapshotPublishesDefinitionsTests(unittest.TestCase): def test_the_dashboard_snapshot_carries_them_in_both_shapes(self): with open(os.path.join(BACKEND_DIR, "next_app.py"), encoding="utf-8") as handle: