From c524c7d44c11a310d2deef5c4eecdecf665859af Mon Sep 17 00:00:00 2001 From: turegjorup Date: Fri, 27 Feb 2026 10:32:55 +0100 Subject: [PATCH 01/11] 6592: Fix slide media array not syncing with content on media removal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The media array sent to the API was append-only — removing or replacing media in a content field left stale IRIs in the media array. Additionally, clearing one media field would wipe all media associations including those from other fields. Extract rebuildMediaFromContent utility that derives the media array from content fields on every change, and add tests. Co-Authored-By: Claude Opus 4.6 --- e2e/slide-media-sync.spec.js | 91 +++++++++++++++++++++++ src/components/slide/slide-manager.jsx | 19 ++--- src/components/slide/slide-media-utils.js | 29 ++++++++ 3 files changed, 130 insertions(+), 9 deletions(-) create mode 100644 e2e/slide-media-sync.spec.js create mode 100644 src/components/slide/slide-media-utils.js diff --git a/e2e/slide-media-sync.spec.js b/e2e/slide-media-sync.spec.js new file mode 100644 index 000000000..641b95ca8 --- /dev/null +++ b/e2e/slide-media-sync.spec.js @@ -0,0 +1,91 @@ +import { test, expect } from "@playwright/test"; +import { rebuildMediaFromContent } from "../src/components/slide/slide-media-utils"; + +test.describe("Slide media sync", () => { + test("It returns media IRIs referenced in content fields", () => { + const content = { + mainImage: ["/v2/media/1", "/v2/media/2"], + backgroundVideo: ["/v2/media/3"], + }; + const mediaFields = ["mainImage", "backgroundVideo"]; + + const result = rebuildMediaFromContent(content, mediaFields); + + expect(result).toEqual(["/v2/media/1", "/v2/media/2", "/v2/media/3"]); + }); + + test("It excludes TEMP-- IDs that have not been uploaded yet", () => { + const content = { + mainImage: ["TEMP--abc123", "/v2/media/1"], + }; + const mediaFields = ["mainImage"]; + + const result = rebuildMediaFromContent(content, mediaFields); + + expect(result).toEqual(["/v2/media/1"]); + }); + + test("It removes media no longer referenced in any content field", () => { + // This is the core bug fix scenario: media/1 was previously in mainImage + // but has been replaced by media/2. The rebuilt array must NOT contain + // media/1 since it is no longer in any content field. + const content = { + mainImage: ["/v2/media/2"], + }; + const mediaFields = ["mainImage"]; + + const result = rebuildMediaFromContent(content, mediaFields); + + expect(result).toEqual(["/v2/media/2"]); + expect(result).not.toContain("/v2/media/1"); + }); + + test("It returns empty array when all media is removed from content", () => { + // Second bug fix scenario: clearing one media field used to wipe all + // media including those from other fields. Now it correctly rebuilds + // from all fields. + const content = { + mainImage: [], + backgroundVideo: ["/v2/media/3"], + }; + const mediaFields = ["mainImage", "backgroundVideo"]; + + const result = rebuildMediaFromContent(content, mediaFields); + + expect(result).toEqual(["/v2/media/3"]); + }); + + test("It deduplicates media used across multiple content fields", () => { + const content = { + mainImage: ["/v2/media/1"], + thumbnail: ["/v2/media/1"], + }; + const mediaFields = ["mainImage", "thumbnail"]; + + const result = rebuildMediaFromContent(content, mediaFields); + + expect(result).toEqual(["/v2/media/1"]); + }); + + test("It handles non-existent content fields gracefully", () => { + const content = {}; + const mediaFields = ["mainImage"]; + + const result = rebuildMediaFromContent(content, mediaFields); + + expect(result).toEqual([]); + }); + + test("It handles nested content field paths", () => { + const content = { + sections: { + hero: ["/v2/media/1"], + }, + }; + const mediaFields = ["sections.hero"]; + + const result = rebuildMediaFromContent(content, mediaFields); + + expect(result).toEqual(["/v2/media/1"]); + }); +}); diff --git a/src/components/slide/slide-manager.jsx b/src/components/slide/slide-manager.jsx index 8487387e4..daa670950 100644 --- a/src/components/slide/slide-manager.jsx +++ b/src/components/slide/slide-manager.jsx @@ -2,6 +2,7 @@ import { React, useEffect, useState, useContext } from "react"; import { useTranslation } from "react-i18next"; import get from "lodash.get"; import set from "lodash.set"; +import { rebuildMediaFromContent } from "./slide-media-utils"; import { ulid } from "ulid"; import PropTypes from "prop-types"; import { useDispatch } from "react-redux"; @@ -337,13 +338,11 @@ function SlideManager({ const localFormStateObject = { ...formStateObject }; const localMediaData = { ...mediaData }; // Set field as a field to look into for new references. - setMediaFields([...new Set([...mediaFields, fieldId])]); + const updatedMediaFields = [...new Set([...mediaFields, fieldId])]; + setMediaFields(updatedMediaFields); const newField = []; - if (Array.isArray(fieldValue) && fieldValue.length === 0) { - localFormStateObject.media = []; - } // Handle each entry in field. if (Array.isArray(fieldValue)) { fieldValue.forEach((entry) => { @@ -383,17 +382,19 @@ function SlideManager({ !Object.prototype.hasOwnProperty.call(localMediaData, entry["@id"]) ) { set(localMediaData, entry["@id"], entry); - - localFormStateObject.media.push(entry["@id"]); } } }); } set(localFormStateObject.content, fieldId, newField); - set(localFormStateObject, "media", [ - ...new Set([...localFormStateObject.media]), - ]); + + // Rebuild media array from all content fields to keep it in sync. + set( + localFormStateObject, + "media", + rebuildMediaFromContent(localFormStateObject.content, updatedMediaFields) + ); setFormStateObject(localFormStateObject); setMediaData(localMediaData); diff --git a/src/components/slide/slide-media-utils.js b/src/components/slide/slide-media-utils.js new file mode 100644 index 000000000..e8006a7af --- /dev/null +++ b/src/components/slide/slide-media-utils.js @@ -0,0 +1,29 @@ +import get from "lodash.get"; + +/** + * Rebuild the media array from all content fields that reference media. + * + * This ensures that the top-level `media` array (sent to the API as + * slide_media associations) always matches the media actually referenced + * in the slide's `content` object. + * + * @param {object} content - The slide content object. + * @param {string[]} mediaFields - Field names in content that hold media refs. + * @returns {string[]} Deduplicated array of media IRIs. + */ +export function rebuildMediaFromContent(content, mediaFields) { + const media = []; + + mediaFields.forEach((fieldName) => { + const fieldData = get(content, fieldName); + if (Array.isArray(fieldData)) { + fieldData.forEach((mediaId) => { + if (typeof mediaId === "string" && !mediaId.startsWith("TEMP--")) { + media.push(mediaId); + } + }); + } + }); + + return [...new Set(media)]; +} From 0d832fd9a17b46ce0cde7bd7b6170a9054be77f6 Mon Sep 17 00:00:00 2001 From: turegjorup Date: Fri, 27 Feb 2026 10:39:46 +0100 Subject: [PATCH 02/11] 6592: Fix lint errors and add changelog entry Co-Authored-By: Claude Opus 4.6 --- CHANGELOG.md | 3 +++ e2e/slide-media-sync.spec.js | 2 +- src/components/slide/slide-manager.jsx | 2 +- src/components/slide/slide-media-utils.js | 8 ++++---- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 71b5cea17..4d377ce6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +- [#295](https://github.com/os2display/display-admin-client/pull/295) + - Fixed slide media array not syncing with content on media removal. + ## [2.6.0] - 2025-12-05 - [#293](https://github.com/os2display/display-admin-client/pull/293) diff --git a/e2e/slide-media-sync.spec.js b/e2e/slide-media-sync.spec.js index 641b95ca8..4146a5db1 100644 --- a/e2e/slide-media-sync.spec.js +++ b/e2e/slide-media-sync.spec.js @@ -1,5 +1,5 @@ import { test, expect } from "@playwright/test"; -import { rebuildMediaFromContent } from "../src/components/slide/slide-media-utils"; +import rebuildMediaFromContent from "../src/components/slide/slide-media-utils"; test.describe("Slide media sync", () => { test("It returns media IRIs referenced in content fields", () => { diff --git a/src/components/slide/slide-manager.jsx b/src/components/slide/slide-manager.jsx index daa670950..5bd1f30b1 100644 --- a/src/components/slide/slide-manager.jsx +++ b/src/components/slide/slide-manager.jsx @@ -2,12 +2,12 @@ import { React, useEffect, useState, useContext } from "react"; import { useTranslation } from "react-i18next"; import get from "lodash.get"; import set from "lodash.set"; -import { rebuildMediaFromContent } from "./slide-media-utils"; import { ulid } from "ulid"; import PropTypes from "prop-types"; import { useDispatch } from "react-redux"; import dayjs from "dayjs"; import { useNavigate } from "react-router-dom"; +import rebuildMediaFromContent from "./slide-media-utils"; import UserContext from "../../context/user-context"; import { api, diff --git a/src/components/slide/slide-media-utils.js b/src/components/slide/slide-media-utils.js index e8006a7af..05a3221e1 100644 --- a/src/components/slide/slide-media-utils.js +++ b/src/components/slide/slide-media-utils.js @@ -3,15 +3,15 @@ import get from "lodash.get"; /** * Rebuild the media array from all content fields that reference media. * - * This ensures that the top-level `media` array (sent to the API as - * slide_media associations) always matches the media actually referenced - * in the slide's `content` object. + * This ensures that the top-level `media` array (sent to the API as slide_media + * associations) always matches the media actually referenced in the slide's + * `content` object. * * @param {object} content - The slide content object. * @param {string[]} mediaFields - Field names in content that hold media refs. * @returns {string[]} Deduplicated array of media IRIs. */ -export function rebuildMediaFromContent(content, mediaFields) { +export default function rebuildMediaFromContent(content, mediaFields) { const media = []; mediaFields.forEach((fieldName) => { From c3f922171135dfb85481f586415a3e4cbf6c313c Mon Sep 17 00:00:00 2001 From: turegjorup Date: Fri, 27 Feb 2026 13:58:02 +0100 Subject: [PATCH 03/11] 6592: Scan all top-level content keys for media references rebuildMediaFromContent only scanned explicitly tracked mediaFields, which start empty and only get populated when handleMedia is called. This meant the first media change on a slide with multiple media fields would drop media from untouched fields. Now also scans all top-level content keys to catch untracked media arrays. Co-Authored-By: Claude Opus 4.6 --- e2e/slide-media-sync.spec.js | 35 +++++++++++++++++++++++ src/components/slide/slide-media-utils.js | 9 +++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/e2e/slide-media-sync.spec.js b/e2e/slide-media-sync.spec.js index 4146a5db1..bbcb9c51a 100644 --- a/e2e/slide-media-sync.spec.js +++ b/e2e/slide-media-sync.spec.js @@ -88,4 +88,39 @@ test.describe("Slide media sync", () => { expect(result).toEqual(["/v2/media/1"]); }); + + test("It includes media from untracked content fields", () => { + // When only one media field has been touched via handleMedia, + // mediaFields only contains that field. Media from other content + // fields must still be included by scanning top-level content keys. + const content = { + images: ["/v2/media/1"], + backgroundImage: ["/v2/media/2"], + }; + const mediaFields = ["images"]; // only images was touched + + const result = rebuildMediaFromContent(content, mediaFields); + + expect(result).toContain("/v2/media/1"); + expect(result).toContain("/v2/media/2"); + }); + + test("It ignores non-media content values when scanning top-level keys", () => { + // Content has both media arrays and plain string/object values. + // Only string array entries should be picked up as media. + const content = { + images: ["/v2/media/1"], + title: "Some text", + separator: true, + contacts: [{ name: "John", image: ["/v2/media/2"] }], + }; + const mediaFields = []; + + const result = rebuildMediaFromContent(content, mediaFields); + + // images field is picked up via top-level scan + expect(result).toContain("/v2/media/1"); + // contacts is an array of objects, not strings — objects are skipped + expect(result).not.toContain("/v2/media/2"); + }); }); diff --git a/src/components/slide/slide-media-utils.js b/src/components/slide/slide-media-utils.js index 05a3221e1..70d4df9e6 100644 --- a/src/components/slide/slide-media-utils.js +++ b/src/components/slide/slide-media-utils.js @@ -13,8 +13,15 @@ import get from "lodash.get"; */ export default function rebuildMediaFromContent(content, mediaFields) { const media = []; + const fieldsToScan = new Set(mediaFields); - mediaFields.forEach((fieldName) => { + // Also scan top-level content keys to catch media fields not yet + // tracked via handleMedia (e.g. on first edit of another field). + if (content && typeof content === "object") { + Object.keys(content).forEach((key) => fieldsToScan.add(key)); + } + + fieldsToScan.forEach((fieldName) => { const fieldData = get(content, fieldName); if (Array.isArray(fieldData)) { fieldData.forEach((mediaId) => { From 1119b477d80798dc0f887c03406235556ebc209e Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Mon, 2 Mar 2026 07:22:25 +0100 Subject: [PATCH 04/11] 6592: Added check for media uri pattern --- e2e/slide-media-sync.spec.js | 16 ++++++++++++++++ src/components/slide/slide-media-utils.js | 19 ++++++++++++------- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/e2e/slide-media-sync.spec.js b/e2e/slide-media-sync.spec.js index bbcb9c51a..5e7360669 100644 --- a/e2e/slide-media-sync.spec.js +++ b/e2e/slide-media-sync.spec.js @@ -123,4 +123,20 @@ test.describe("Slide media sync", () => { // contacts is an array of objects, not strings — objects are skipped expect(result).not.toContain("/v2/media/2"); }); + + test("It does not include non-media string arrays from content", () => { + // Regression: previously any array-of-strings could be treated as media, + // e.g. tags/categories/etc. Only actual media IRIs should be returned. + const content = { + images: ["/v2/media/1"], + tags: ["news", "sports"], + }; + const mediaFields = []; // rely on top-level scan + + const result = rebuildMediaFromContent(content, mediaFields); + + expect(result).toEqual(["/v2/media/1"]); + expect(result).not.toContain("news"); + expect(result).not.toContain("sports"); + }); }); diff --git a/src/components/slide/slide-media-utils.js b/src/components/slide/slide-media-utils.js index 70d4df9e6..b0706e169 100644 --- a/src/components/slide/slide-media-utils.js +++ b/src/components/slide/slide-media-utils.js @@ -15,6 +15,11 @@ export default function rebuildMediaFromContent(content, mediaFields) { const media = []; const fieldsToScan = new Set(mediaFields); + const isMediaIri = (value) => + typeof value === "string" && + !value.startsWith("TEMP--") && + value.includes("/v2/media/"); + // Also scan top-level content keys to catch media fields not yet // tracked via handleMedia (e.g. on first edit of another field). if (content && typeof content === "object") { @@ -23,13 +28,13 @@ export default function rebuildMediaFromContent(content, mediaFields) { fieldsToScan.forEach((fieldName) => { const fieldData = get(content, fieldName); - if (Array.isArray(fieldData)) { - fieldData.forEach((mediaId) => { - if (typeof mediaId === "string" && !mediaId.startsWith("TEMP--")) { - media.push(mediaId); - } - }); - } + if (!Array.isArray(fieldData)) return; + + fieldData.forEach((candidate) => { + if (isMediaIri(candidate)) { + media.push(candidate); + } + }); }); return [...new Set(media)]; From 1d33b7b835e32f29d7eac20d0863dc334986a5ea Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Mon, 2 Mar 2026 07:29:45 +0100 Subject: [PATCH 05/11] 6592: Replaced includes with regex --- src/components/slide/slide-media-utils.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/components/slide/slide-media-utils.js b/src/components/slide/slide-media-utils.js index b0706e169..558cf2b56 100644 --- a/src/components/slide/slide-media-utils.js +++ b/src/components/slide/slide-media-utils.js @@ -15,13 +15,15 @@ export default function rebuildMediaFromContent(content, mediaFields) { const media = []; const fieldsToScan = new Set(mediaFields); + const mediaIriRegex = /\/v2\/media\/.+/; + const isMediaIri = (value) => typeof value === "string" && !value.startsWith("TEMP--") && - value.includes("/v2/media/"); + mediaIriRegex.test(value); - // Also scan top-level content keys to catch media fields not yet - // tracked via handleMedia (e.g. on first edit of another field). + // Also, scan top-level content keys to catch media fields not yet + // tracked via handleMedia (e.g. on the first edit of another field). if (content && typeof content === "object") { Object.keys(content).forEach((key) => fieldsToScan.add(key)); } @@ -37,5 +39,7 @@ export default function rebuildMediaFromContent(content, mediaFields) { }); }); + console.log("media", [...new Set(media)]); + return [...new Set(media)]; } From 1a1715379fa968b40a8b30d2ff86b76c31d83f99 Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Mon, 2 Mar 2026 07:32:02 +0100 Subject: [PATCH 06/11] 6592: Removed console log --- src/components/slide/slide-media-utils.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/components/slide/slide-media-utils.js b/src/components/slide/slide-media-utils.js index 558cf2b56..e737f0518 100644 --- a/src/components/slide/slide-media-utils.js +++ b/src/components/slide/slide-media-utils.js @@ -39,7 +39,5 @@ export default function rebuildMediaFromContent(content, mediaFields) { }); }); - console.log("media", [...new Set(media)]); - return [...new Set(media)]; } From 665f0a8b232dea6df653248c97c79dd2a6bd0c38 Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Mon, 2 Mar 2026 08:14:28 +0100 Subject: [PATCH 07/11] 6592: Changed to handle nested media fields in content --- e2e/slide-media-sync.spec.js | 9 +++---- src/components/slide/slide-media-utils.js | 30 +++++++++++++++++------ 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/e2e/slide-media-sync.spec.js b/e2e/slide-media-sync.spec.js index 5e7360669..00b76a0b0 100644 --- a/e2e/slide-media-sync.spec.js +++ b/e2e/slide-media-sync.spec.js @@ -112,7 +112,7 @@ test.describe("Slide media sync", () => { images: ["/v2/media/1"], title: "Some text", separator: true, - contacts: [{ name: "John", image: ["/v2/media/2"] }], + contacts: [{ name: "John", image: ["/v2/media/2"], tags: ["news"] }], }; const mediaFields = []; @@ -120,13 +120,12 @@ test.describe("Slide media sync", () => { // images field is picked up via top-level scan expect(result).toContain("/v2/media/1"); - // contacts is an array of objects, not strings — objects are skipped - expect(result).not.toContain("/v2/media/2"); + expect(result).toContain("/v2/media/2"); + expect(result).not.toContain("news"); }); test("It does not include non-media string arrays from content", () => { - // Regression: previously any array-of-strings could be treated as media, - // e.g. tags/categories/etc. Only actual media IRIs should be returned. + // Only actual media IRIs should be returned. const content = { images: ["/v2/media/1"], tags: ["news", "sports"], diff --git a/src/components/slide/slide-media-utils.js b/src/components/slide/slide-media-utils.js index e737f0518..3be19c8c5 100644 --- a/src/components/slide/slide-media-utils.js +++ b/src/components/slide/slide-media-utils.js @@ -22,6 +22,28 @@ export default function rebuildMediaFromContent(content, mediaFields) { !value.startsWith("TEMP--") && mediaIriRegex.test(value); + const collectMediaFromValue = (value, seen = new Set()) => { + if (value === null || value === undefined) return; + + if (typeof value === "string") { + if (isMediaIri(value)) media.push(value); + return; + } + + if (typeof value !== "object") return; + + // Avoid potential circular references (defensive; content is usually JSON) + if (seen.has(value)) return; + seen.add(value); + + if (Array.isArray(value)) { + value.forEach((item) => collectMediaFromValue(item, seen)); + return; + } + + Object.values(value).forEach((item) => collectMediaFromValue(item, seen)); + }; + // Also, scan top-level content keys to catch media fields not yet // tracked via handleMedia (e.g. on the first edit of another field). if (content && typeof content === "object") { @@ -30,13 +52,7 @@ export default function rebuildMediaFromContent(content, mediaFields) { fieldsToScan.forEach((fieldName) => { const fieldData = get(content, fieldName); - if (!Array.isArray(fieldData)) return; - - fieldData.forEach((candidate) => { - if (isMediaIri(candidate)) { - media.push(candidate); - } - }); + collectMediaFromValue(fieldData); }); return [...new Set(media)]; From ef9292be17ddbdba30c044ec3f07e7c0c5181efc Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Mon, 2 Mar 2026 09:52:10 +0100 Subject: [PATCH 08/11] 6592: Added comments --- src/components/slide/slide-media-utils.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/components/slide/slide-media-utils.js b/src/components/slide/slide-media-utils.js index 3be19c8c5..67bfb0f4f 100644 --- a/src/components/slide/slide-media-utils.js +++ b/src/components/slide/slide-media-utils.js @@ -23,24 +23,31 @@ export default function rebuildMediaFromContent(content, mediaFields) { mediaIriRegex.test(value); const collectMediaFromValue = (value, seen = new Set()) => { + // 1) Ignore empty values early (nothing to scan) if (value === null || value === undefined) return; + // 2) If it's a string, it might be a media IRI; validate and collect it if (typeof value === "string") { if (isMediaIri(value)) media.push(value); return; } + // 3) If it's not an object (e.g. number/boolean/function), it cannot contain nested media if (typeof value !== "object") return; - // Avoid potential circular references (defensive; content is usually JSON) + // 4) Defensive guard against circular references: + // - JSON content won't have cycles, but runtime objects might. + // - If we've seen this object/array already, stop to avoid infinite recursion. if (seen.has(value)) return; seen.add(value); + // 5) If it's an array, scan each element (elements can be strings, objects, or more arrays) if (Array.isArray(value)) { value.forEach((item) => collectMediaFromValue(item, seen)); return; } + // 6) Otherwise it's a plain object: scan its property values recursively Object.values(value).forEach((item) => collectMediaFromValue(item, seen)); }; From f5972c6a5576ce7eff511a2155bee778588bb097 Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Mon, 2 Mar 2026 10:09:00 +0100 Subject: [PATCH 09/11] 6592: Added test for infinite recursion protection --- e2e/slide-media-sync.spec.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/e2e/slide-media-sync.spec.js b/e2e/slide-media-sync.spec.js index 00b76a0b0..65962b50a 100644 --- a/e2e/slide-media-sync.spec.js +++ b/e2e/slide-media-sync.spec.js @@ -138,4 +138,15 @@ test.describe("Slide media sync", () => { expect(result).not.toContain("news"); expect(result).not.toContain("sports"); }); + + test("It avoids infinite recursion when content contains circular references", () => { + const circular = { images: ["/v2/media/1"] }; + circular.self = circular; // create an explicit cycle + + // If we didn't track `seen`, this would crash. + expect(() => rebuildMediaFromContent(circular, [])).not.toThrow(); + + const result = rebuildMediaFromContent(circular, []); + expect(result).toContain("/v2/media/1"); + }); }); From db137b95d1128419158aa595e0c7a06a28286076 Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Mon, 2 Mar 2026 12:02:16 +0100 Subject: [PATCH 10/11] 6592: Removed unused mediaFields parameter --- e2e/slide-media-sync.spec.js | 58 +++++------------------ src/components/slide/slide-manager.jsx | 4 +- src/components/slide/slide-media-utils.js | 15 +++--- 3 files changed, 19 insertions(+), 58 deletions(-) diff --git a/e2e/slide-media-sync.spec.js b/e2e/slide-media-sync.spec.js index 65962b50a..f8e939ca5 100644 --- a/e2e/slide-media-sync.spec.js +++ b/e2e/slide-media-sync.spec.js @@ -7,9 +7,8 @@ test.describe("Slide media sync", () => { mainImage: ["/v2/media/1", "/v2/media/2"], backgroundVideo: ["/v2/media/3"], }; - const mediaFields = ["mainImage", "backgroundVideo"]; - const result = rebuildMediaFromContent(content, mediaFields); + const result = rebuildMediaFromContent(content); expect(result).toEqual(["/v2/media/1", "/v2/media/2", "/v2/media/3"]); }); @@ -18,39 +17,30 @@ test.describe("Slide media sync", () => { const content = { mainImage: ["TEMP--abc123", "/v2/media/1"], }; - const mediaFields = ["mainImage"]; - const result = rebuildMediaFromContent(content, mediaFields); + const result = rebuildMediaFromContent(content); expect(result).toEqual(["/v2/media/1"]); }); test("It removes media no longer referenced in any content field", () => { - // This is the core bug fix scenario: media/1 was previously in mainImage - // but has been replaced by media/2. The rebuilt array must NOT contain - // media/1 since it is no longer in any content field. const content = { mainImage: ["/v2/media/2"], }; - const mediaFields = ["mainImage"]; - const result = rebuildMediaFromContent(content, mediaFields); + const result = rebuildMediaFromContent(content); expect(result).toEqual(["/v2/media/2"]); expect(result).not.toContain("/v2/media/1"); }); test("It returns empty array when all media is removed from content", () => { - // Second bug fix scenario: clearing one media field used to wipe all - // media including those from other fields. Now it correctly rebuilds - // from all fields. const content = { mainImage: [], backgroundVideo: ["/v2/media/3"], }; - const mediaFields = ["mainImage", "backgroundVideo"]; - const result = rebuildMediaFromContent(content, mediaFields); + const result = rebuildMediaFromContent(content); expect(result).toEqual(["/v2/media/3"]); }); @@ -60,18 +50,16 @@ test.describe("Slide media sync", () => { mainImage: ["/v2/media/1"], thumbnail: ["/v2/media/1"], }; - const mediaFields = ["mainImage", "thumbnail"]; - const result = rebuildMediaFromContent(content, mediaFields); + const result = rebuildMediaFromContent(content); expect(result).toEqual(["/v2/media/1"]); }); test("It handles non-existent content fields gracefully", () => { const content = {}; - const mediaFields = ["mainImage"]; - const result = rebuildMediaFromContent(content, mediaFields); + const result = rebuildMediaFromContent(content); expect(result).toEqual([]); }); @@ -82,57 +70,34 @@ test.describe("Slide media sync", () => { hero: ["/v2/media/1"], }, }; - const mediaFields = ["sections.hero"]; - const result = rebuildMediaFromContent(content, mediaFields); + const result = rebuildMediaFromContent(content); expect(result).toEqual(["/v2/media/1"]); }); - test("It includes media from untracked content fields", () => { - // When only one media field has been touched via handleMedia, - // mediaFields only contains that field. Media from other content - // fields must still be included by scanning top-level content keys. - const content = { - images: ["/v2/media/1"], - backgroundImage: ["/v2/media/2"], - }; - const mediaFields = ["images"]; // only images was touched - - const result = rebuildMediaFromContent(content, mediaFields); - - expect(result).toContain("/v2/media/1"); - expect(result).toContain("/v2/media/2"); - }); - test("It ignores non-media content values when scanning top-level keys", () => { - // Content has both media arrays and plain string/object values. - // Only string array entries should be picked up as media. const content = { images: ["/v2/media/1"], title: "Some text", separator: true, contacts: [{ name: "John", image: ["/v2/media/2"], tags: ["news"] }], }; - const mediaFields = []; - const result = rebuildMediaFromContent(content, mediaFields); + const result = rebuildMediaFromContent(content); - // images field is picked up via top-level scan expect(result).toContain("/v2/media/1"); expect(result).toContain("/v2/media/2"); expect(result).not.toContain("news"); }); test("It does not include non-media string arrays from content", () => { - // Only actual media IRIs should be returned. const content = { images: ["/v2/media/1"], tags: ["news", "sports"], }; - const mediaFields = []; // rely on top-level scan - const result = rebuildMediaFromContent(content, mediaFields); + const result = rebuildMediaFromContent(content); expect(result).toEqual(["/v2/media/1"]); expect(result).not.toContain("news"); @@ -143,10 +108,9 @@ test.describe("Slide media sync", () => { const circular = { images: ["/v2/media/1"] }; circular.self = circular; // create an explicit cycle - // If we didn't track `seen`, this would crash. - expect(() => rebuildMediaFromContent(circular, [])).not.toThrow(); + expect(() => rebuildMediaFromContent(circular)).not.toThrow(); - const result = rebuildMediaFromContent(circular, []); + const result = rebuildMediaFromContent(circular); expect(result).toContain("/v2/media/1"); }); }); diff --git a/src/components/slide/slide-manager.jsx b/src/components/slide/slide-manager.jsx index 5bd1f30b1..6552fe5da 100644 --- a/src/components/slide/slide-manager.jsx +++ b/src/components/slide/slide-manager.jsx @@ -389,11 +389,11 @@ function SlideManager({ set(localFormStateObject.content, fieldId, newField); - // Rebuild media array from all content fields to keep it in sync. + // Rebuild the media array from all content fields to keep it in sync. set( localFormStateObject, "media", - rebuildMediaFromContent(localFormStateObject.content, updatedMediaFields) + rebuildMediaFromContent(localFormStateObject.content) ); setFormStateObject(localFormStateObject); diff --git a/src/components/slide/slide-media-utils.js b/src/components/slide/slide-media-utils.js index 67bfb0f4f..089edfb2c 100644 --- a/src/components/slide/slide-media-utils.js +++ b/src/components/slide/slide-media-utils.js @@ -8,12 +8,10 @@ import get from "lodash.get"; * `content` object. * * @param {object} content - The slide content object. - * @param {string[]} mediaFields - Field names in content that hold media refs. * @returns {string[]} Deduplicated array of media IRIs. */ -export default function rebuildMediaFromContent(content, mediaFields) { +export default function rebuildMediaFromContent(content) { const media = []; - const fieldsToScan = new Set(mediaFields); const mediaIriRegex = /\/v2\/media\/.+/; @@ -51,16 +49,15 @@ export default function rebuildMediaFromContent(content, mediaFields) { Object.values(value).forEach((item) => collectMediaFromValue(item, seen)); }; - // Also, scan top-level content keys to catch media fields not yet - // tracked via handleMedia (e.g. on the first edit of another field). + const fieldsToScan = new Set([]); + + // Scan content for media references. if (content && typeof content === "object") { Object.keys(content).forEach((key) => fieldsToScan.add(key)); } - fieldsToScan.forEach((fieldName) => { - const fieldData = get(content, fieldName); - collectMediaFromValue(fieldData); - }); + // Scan the entire content object (one traversal) + collectMediaFromValue(content); return [...new Set(media)]; } From 1ab75741b6129d61493eb1a6433052f06f0ab247 Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Mon, 2 Mar 2026 12:26:47 +0100 Subject: [PATCH 11/11] 6592: Removed unused import --- src/components/slide/slide-media-utils.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/components/slide/slide-media-utils.js b/src/components/slide/slide-media-utils.js index 089edfb2c..8c78bfb41 100644 --- a/src/components/slide/slide-media-utils.js +++ b/src/components/slide/slide-media-utils.js @@ -1,5 +1,3 @@ -import get from "lodash.get"; - /** * Rebuild the media array from all content fields that reference media. *