Fix revision history display (#20417)

* Fix nested arrays

* Add unit test

* Fallback to original revision message on error

frontend/src/metabase/lib/revisions/components.jsx

@@ -29,26 +29,32 @@ RevisionTitle.propTypes = revisionTitlePropTypes;
 
 const revisionBatchedDescriptionPropTypes = {
   changes: PropTypes.arrayOf(PropTypes.node).isRequired,
+  fallback: PropTypes.string,
 };
 
-export function RevisionBatchedDescription({ changes }) {
+export function RevisionBatchedDescription({ changes, fallback }) {
   const formattedChanges = useMemo(() => {
-    const result = [];
+    let result = [];
 
     changes.forEach((change, i) => {
-      const isFirst = i === 0;
-      result.push(isFirst ? capitalizeChangeRecord(change) : change);
-      const isLast = i === changes.length - 1;
-      const isBeforeLast = i === changes.length - 2;
-      if (isBeforeLast) {
-        result.push(" " + t`and` + " ");
-      } else if (!isLast) {
-        result.push(", ");
+      try {
+        const isFirst = i === 0;
+        result.push(isFirst ? capitalizeChangeRecord(change) : change);
+        const isLast = i === changes.length - 1;
+        const isBeforeLast = i === changes.length - 2;
+        if (isBeforeLast) {
+          result.push(" " + t`and` + " ");
+        } else if (!isLast) {
+          result.push(", ");
+        }
+      } catch {
+        console.warn("Error formatting revision changes", changes);
+        result = fallback;
       }
     });
 
     return result;
-  }, [changes]);
+  }, [changes, fallback]);
 
   return <span>{formattedChanges}</span>;
 }
@@ -56,6 +62,9 @@ export function RevisionBatchedDescription({ changes }) {
 function capitalizeChangeRecord(change) {
   if (Array.isArray(change)) {
     const [first, ...rest] = change;
+    if (Array.isArray(first)) {
+      return capitalizeChangeRecord(first);
+    }
     return [capitalize(first, { lowercase: false }), ...rest];
   }
   return capitalize(change, { lowercase: false });

frontend/src/metabase/lib/revisions/components.unit.spec.js

 import React from "react";
-import { render, screen } from "@testing-library/react";
+import { render, renderWithProviders, screen } from "__support__/ui";
+import { getCollectionChangeDescription } from "./revisions";
 import { RevisionTitle, RevisionBatchedDescription } from "./components";
 
 describe("RevisionTitle", () => {
@@ -10,6 +11,16 @@ describe("RevisionTitle", () => {
 });
 
 describe("RevisionBatchedDescription", () => {
+  const originalWarn = console.warn;
+
+  beforeAll(() => {
+    console.warn = jest.fn();
+  });
+
+  afterAll(() => {
+    console.warn = originalWarn;
+  });
+
   it("correctly renders two change records", () => {
     render(
       <RevisionBatchedDescription
@@ -40,4 +51,24 @@ describe("RevisionBatchedDescription", () => {
     );
     expect(screen.queryByText("Renamed this and moved to Our analytics"));
   });
+
+  it("should handle nested messages (metabase#20414)", () => {
+    renderWithProviders(
+      <RevisionBatchedDescription
+        changes={[getCollectionChangeDescription(1, 2), "edited metadata"]}
+      />,
+    );
+    expect(screen.getByText(/Moved this to/)).toBeInTheDocument();
+    expect(screen.getByText(/edited metadata/)).toBeInTheDocument();
+  });
+
+  it("should use fallback when failing to format changes message", () => {
+    render(
+      <RevisionBatchedDescription
+        changes={[{ key: "try to parse this" }, -1, false]}
+        fallback="Just a fallback"
+      />,
+    );
+    expect(screen.getByText("Just a fallback")).toBeInTheDocument();
+  });
 });

frontend/src/metabase/lib/revisions/revisions.js

@@ -63,7 +63,7 @@ function getSeriesChangeDescription(prevCards, cards) {
     : t`removed series from a question`;
 }
 
-function getCollectionChangeDescription(prevCollectionId, collectionId) {
+export function getCollectionChangeDescription(prevCollectionId, collectionId) {
   const key = `collection-from-${prevCollectionId}-to-${collectionId}`;
   return [
     jt`moved this to ${(
@@ -245,7 +245,12 @@ export function getRevisionEventsForTimeline(
       // like "John added a description"
       if (isChangeEvent && isMultipleFieldsChange) {
         event.title = t`${username} edited this`;
-        event.description = <RevisionBatchedDescription changes={changes} />;
+        event.description = (
+          <RevisionBatchedDescription
+            changes={changes}
+            fallback={revision.description}
+          />
+        );
       } else {
         event.title = <RevisionTitle username={username} message={changes} />;
       }