From b82c6326cf589205315fea8a5bf82b05991dbed2 Mon Sep 17 00:00:00 2001
From: Andrey Antukh <niwi@niwi.nz>
Date: Tue, 10 Sep 2024 11:49:05 +0200
Subject: [PATCH] :sparkles: Add better error reporting on changes validation

---
 common/src/app/common/files/changes.cljc            |  8 +++-----
 common/src/app/common/schema.cljc                   |  1 -
 frontend/src/app/main/data/changes.cljs             |  9 ++++++---
 frontend/src/app/main/data/workspace/libraries.cljs | 11 +++++++++--
 frontend/src/app/main/errors.cljs                   |  9 ++++++---
 5 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/common/src/app/common/files/changes.cljc b/common/src/app/common/files/changes.cljc
index 2ed1ab257e..3d8c6944ee 100644
--- a/common/src/app/common/files/changes.cljc
+++ b/common/src/app/common/files/changes.cljc
@@ -370,8 +370,8 @@
 (def valid-change?
   (sm/lazy-validator schema:change))
 
-(def valid-changes?
-  (sm/lazy-validator schema:changes))
+(def check-changes!
+  (sm/check-fn schema:changes))
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;; Specific helpers
@@ -446,9 +446,7 @@
    ;; When verify? false we spec the schema validation. Currently used
    ;; to make just 1 validation even if the changes are applied twice
    (when verify?
-     (dm/verify!
-      "expected valid changes"
-      (valid-changes? items)))
+     (check-changes! items))
 
    (binding [*touched-changes* (volatile! #{})]
      (let [result (reduce #(or (process-change %1 %2) %1) data items)
diff --git a/common/src/app/common/schema.cljc b/common/src/app/common/schema.cljc
index 7c73bb6a03..d88b9ab10a 100644
--- a/common/src/app/common/schema.cljc
+++ b/common/src/app/common/schema.cljc
@@ -255,7 +255,6 @@
 
 (declare ^:private lazy-schema)
 
-;; DEPRECATED: should not be used for new code
 (defn check-fn
   "Create a predefined check function"
   [s]
diff --git a/frontend/src/app/main/data/changes.cljs b/frontend/src/app/main/data/changes.cljs
index 4272f31579..080f193fa4 100644
--- a/frontend/src/app/main/data/changes.cljs
+++ b/frontend/src/app/main/data/changes.cljs
@@ -109,9 +109,12 @@
            file-id file-revn undo-group tags stack-undo? source]}]
 
   (dm/assert!
-   "expect valid vector of changes"
-   (and (cpc/valid-changes? redo-changes)
-        (cpc/valid-changes? undo-changes)))
+   "expect valid vector of changes for redo-changes"
+   (cpc/check-changes! redo-changes))
+
+  (dm/assert!
+   "expect valid vector of changes for undo-changes"
+   (cpc/check-changes! undo-changes))
 
   (let [commit-id (or commit-id (uuid/next))
         source    (d/nilv source :local)
diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs
index 41cb2528de..cdfb03432a 100644
--- a/frontend/src/app/main/data/workspace/libraries.cljs
+++ b/frontend/src/app/main/data/workspace/libraries.cljs
@@ -675,8 +675,15 @@
 
 (defn ext-library-changed
   [library-id modified-at revn changes]
-  (dm/assert! (uuid? library-id))
-  (dm/assert! (ch/valid-changes? changes))
+
+  (dm/assert!
+   "expected valid uuid for library-id"
+   (uuid? library-id))
+
+  (dm/assert!
+   "expected valid changes vector"
+   (ch/check-changes! changes))
+
   (ptk/reify ::ext-library-changed
     ptk/UpdateEvent
     (update [_ state]
diff --git a/frontend/src/app/main/errors.cljs b/frontend/src/app/main/errors.cljs
index 59891188b8..61ff775a35 100644
--- a/frontend/src/app/main/errors.cljs
+++ b/frontend/src/app/main/errors.cljs
@@ -9,7 +9,7 @@
   (:require
    [app.common.exceptions :as ex]
    [app.common.pprint :as pp]
-   [app.common.schema :as-alias sm]
+   [app.common.schema :as sm]
    [app.main.data.modal :as modal]
    [app.main.data.notifications :as ntf]
    [app.main.data.users :as du]
@@ -32,8 +32,11 @@
 
 (defn- print-explain!
   [data]
-  (when-let [explain (or (ex/explain data)
-                         (:explain data))]
+  (when-let [{:keys [errors] :as explain} (::sm/explain data)]
+    (let [errors (mapv #(update % :schema sm/form) errors)]
+      (pp/pprint errors {:width 100 :level 15 :length 20})))
+
+  (when-let [explain (:explain data)]
     (js/console.log explain)))
 
 (defn- print-trace!