Merge pull request #356 from os2display/feature/release-3-align-with-2.6.1

Align release 3.0.0 with 2.6.1

Author: tuj

CHANGELOG.md

@@ -22,7 +22,7 @@ All notable changes to this project will be documented in this file.
 - Aligned with v. 2.5.2.
 - Removed themes.
 - Added command to migrate config.json files.
-- Fix data fetching bug + tests
+- Fix data fetching bug and tests
 - Refactored screen layout commands.
 - Moved list components (search and checkboxes) around.
 - Aligned environment variable names.

assets/admin/components/slide/slide-manager.jsx

@@ -4,6 +4,7 @@ import { ulid } from "ulid";
 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 {
   enhancedApi,
@@ -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 the media array from all content fields to keep it in sync.
+    set(
+      localFormStateObject,
+      "media",
+      rebuildMediaFromContent(localFormStateObject.content),
+    );
 
     setFormStateObject(localFormStateObject);
     setMediaData(localMediaData);

assets/admin/components/slide/slide-media-utils.js

@@ -0,0 +1,61 @@
+/**
+ * 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.
+ * @returns {string[]} Deduplicated array of media IRIs.
+ */
+export default function rebuildMediaFromContent(content) {
+  const media = [];
+
+  const mediaIriRegex = /\/v2\/media\/.+/;
+
+  const isMediaIri = (value) =>
+    typeof value === "string" &&
+    !value.startsWith("TEMP--") &&
+    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;
+
+    // 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));
+  };
+
+  const fieldsToScan = new Set([]);
+
+  // Scan content for media references.
+  if (content && typeof content === "object") {
+    Object.keys(content).forEach((key) => fieldsToScan.add(key));
+  }
+
+  // Scan the entire content object (one traversal)
+  collectMediaFromValue(content);
+
+  return [...new Set(media)];
+}

assets/shared/templates/image-text.jsx

@@ -42,6 +42,7 @@ function renderSlide(slide, run, slideDone) {
  */
 function ImageText({ slide, content, run, slideDone, executionId }) {
   const imageTimeoutRef = useRef();
+  const imagesRef = useRef([]);
   const [images, setImages] = useState([]);
   const [currentImage, setCurrentImage] = useState();
   const logo = slide?.theme?.logo;
@@ -132,13 +133,15 @@ function ImageText({ slide, content, run, slideDone, executionId }) {
   }
 
   const changeImage = (newIndex) => {
-    if (newIndex < images.length) {
-      setCurrentImage(images[newIndex]);
+    const currentImages = imagesRef.current;
 
-      if (newIndex < images.length - 1) {
+    if (newIndex < currentImages.length) {
+      setCurrentImage(currentImages[newIndex]);
+
+      if (newIndex < currentImages.length - 1) {
         imageTimeoutRef.current = setTimeout(
           () => changeImage(newIndex + 1),
-          duration / images.length,
+          duration / currentImages.length,
         );
       }
     }
@@ -152,33 +155,41 @@ function ImageText({ slide, content, run, slideDone, executionId }) {
       );
 
       if (imageUrls?.length > 0) {
-        const newImages = imageUrls.map((url) => {
-          return {
-            url,
-            nodeRef: createRef(),
-          };
-        });
+        const newImages = imageUrls.map((url) => ({
+          url,
+          nodeRef: createRef(),
+        }));
+
+        imagesRef.current = newImages;
 
         setImages(newImages);
+      } else {
+        imagesRef.current = [];
+        setImages([]);
       }
     }
   }, [slide]);
 
   const startTheShow = () => {
-    if (images?.length > 0 && !currentImage) {
-      setCurrentImage(images[0]);
+    if (imageTimeoutRef.current) {
+      clearTimeout(imageTimeoutRef.current);
     }
 
-    // If there are multiple images, we are going to loop through these WITHIN the set duration.
-    if (images?.length > 1) {
+    const currentImages = imagesRef.current;
+
+    if (currentImages.length > 1) {
       // Kickoff the display of multiple images with the zero indexed
       changeImage(0);
+    } else if (currentImages.length === 1) {
+      setCurrentImage(currentImages[0]);
     }
   };
 
   useEffect(() => {
-    if (!currentImage) {
+    if (images.length > 0) {
       startTheShow();
+    } else {
+      setCurrentImage(undefined);
     }
   }, [images]);
 

assets/tests/admin/admin-slide-media-sync.spec.js

@@ -0,0 +1,116 @@
+import { test, expect } from "@playwright/test";
+import rebuildMediaFromContent from "../../admin/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 result = rebuildMediaFromContent(content);
+
+    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 result = rebuildMediaFromContent(content);
+
+    expect(result).toEqual(["/v2/media/1"]);
+  });
+
+  test("It removes media no longer referenced in any content field", () => {
+    const content = {
+      mainImage: ["/v2/media/2"],
+    };
+
+    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", () => {
+    const content = {
+      mainImage: [],
+      backgroundVideo: ["/v2/media/3"],
+    };
+
+    const result = rebuildMediaFromContent(content);
+
+    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 result = rebuildMediaFromContent(content);
+
+    expect(result).toEqual(["/v2/media/1"]);
+  });
+
+  test("It handles non-existent content fields gracefully", () => {
+    const content = {};
+
+    const result = rebuildMediaFromContent(content);
+
+    expect(result).toEqual([]);
+  });
+
+  test("It handles nested content field paths", () => {
+    const content = {
+      sections: {
+        hero: ["/v2/media/1"],
+      },
+    };
+
+    const result = rebuildMediaFromContent(content);
+
+    expect(result).toEqual(["/v2/media/1"]);
+  });
+
+  test("It ignores non-media content values when scanning top-level keys", () => {
+    const content = {
+      images: ["/v2/media/1"],
+      title: "Some text",
+      separator: true,
+      contacts: [{ name: "John", image: ["/v2/media/2"], tags: ["news"] }],
+    };
+
+    const result = rebuildMediaFromContent(content);
+
+    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", () => {
+    const content = {
+      images: ["/v2/media/1"],
+      tags: ["news", "sports"],
+    };
+
+    const result = rebuildMediaFromContent(content);
+
+    expect(result).toEqual(["/v2/media/1"]);
+    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
+
+    expect(() => rebuildMediaFromContent(circular)).not.toThrow();
+
+    const result = rebuildMediaFromContent(circular);
+    expect(result).toContain("/v2/media/1");
+  });
+});