From 040080749b94bb3ba466d937ce3eefb7697b8432 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 30 Jul 2026 12:49:19 +0200 Subject: [PATCH] :bug: Fix shape export failures when export name is nil or empty (#10852) Use cuerdas blank-name handling directly when normalizing frontend export payloads. Replace nil or blank export names with the object-id string in request-simple-export, request-multiple-export, clipboard export, and plugin direct export payloads so that the backend always receives a valid name. Add focused frontend tests for nil/blank name normalization and normalized request params. AI-assisted-by: nex-n2-pro --- .../src/app/main/data/exports/assets.cljs | 187 ++++++++++-------- .../app/main/data/workspace/clipboard.cljs | 21 +- frontend/src/app/plugins/shape.cljs | 15 +- .../data/exports_assets_test.cljs | 99 ++++++++++ frontend/test/frontend_tests/runner.cljs | 2 + 5 files changed, 223 insertions(+), 101 deletions(-) create mode 100644 frontend/test/frontend_tests/data/exports_assets_test.cljs diff --git a/frontend/src/app/main/data/exports/assets.cljs b/frontend/src/app/main/data/exports/assets.cljs index 24d72e5e92..b3adaebade 100644 --- a/frontend/src/app/main/data/exports/assets.cljs +++ b/frontend/src/app/main/data/exports/assets.cljs @@ -20,10 +20,27 @@ [app.util.dom :as dom] [app.util.websocket :as ws] [beicon.v2.core :as rx] + [cuerdas.core :as str] [potok.v2.core :as ptk])) (def default-timeout 5000) +(defn normalize-export + [{:keys [object-id name] :as export}] + (assoc export :name (if (str/blank? name) + (str object-id) + name))) + +(defn- normalize-exports + [exports] + (mapv normalize-export exports)) + +(defn- normalize-export-shapes-params + [{:keys [exports] :as params}] + (cond-> params + (seq exports) + (assoc :exports (normalize-exports exports)))) + (defn toggle-detail-visibililty [] (ptk/reify ::toggle-detail-visibililty @@ -181,104 +198,106 @@ (defn request-simple-export [{:keys [export]}] - (ptk/reify ::request-simple-export - ptk/UpdateEvent - (update [_ state] - (cond-> state - (not (use-wasm-export? state export)) - (update :export assoc :in-progress true :id uuid/zero))) + (let [export (normalize-export export)] + (ptk/reify ::request-simple-export + ptk/UpdateEvent + (update [_ state] + (cond-> state + (not (use-wasm-export? state export)) + (update :export assoc :in-progress true :id uuid/zero))) - ptk/WatchEvent - (watch [_ state _] - (if (use-wasm-export? state export) - (do - (case (:type export) - :pdf (wasm.exports/export-pdf export) - (wasm.exports/export-image export)) - (rx/empty)) - (let [profile-id (:profile-id state) - params {:exports [export] - :profile-id profile-id - :cmd :export-shapes - :wait true - :is-wasm (wasm-export-enabled? state)}] - (rx/concat - (dwp/force-persist-and-wait 400) + ptk/WatchEvent + (watch [_ state _] + (if (use-wasm-export? state export) + (do + (case (:type export) + :pdf (wasm.exports/export-pdf export) + (wasm.exports/export-image export)) + (rx/empty)) + (let [profile-id (:profile-id state) + params (normalize-export-shapes-params {:exports [export] + :profile-id profile-id + :cmd :export-shapes + :wait true + :is-wasm (wasm-export-enabled? state)})] + (rx/concat + (dwp/force-persist-and-wait 400) - (->> (rp/cmd! :export params) - (rx/map (fn [{:keys [filename mtype uri]}] - (dom/trigger-download-uri filename mtype uri) - (clear-export-state uuid/zero))) - (rx/catch (fn [cause] - (rx/concat - (rx/of (clear-export-state uuid/zero)) - (rx/throw cause))))))))))) + (->> (rp/cmd! :export params) + (rx/map (fn [{:keys [filename mtype uri]}] + (dom/trigger-download-uri filename mtype uri) + (clear-export-state uuid/zero))) + (rx/catch (fn [cause] + (rx/concat + (rx/of (clear-export-state uuid/zero)) + (rx/throw cause)))))))))))) (defn request-multiple-export [{:keys [exports cmd name] :or {cmd :export-shapes} :as params}] - (ptk/reify ::request-multiple-export - ptk/WatchEvent - (watch [_ state _] - (let [resource-id (volatile! nil) - profile-id (:profile-id state) - ws-conn (:ws-conn state) - params (cond-> - {:exports exports - :cmd cmd - :profile-id profile-id - :force-multiple true - :is-wasm (wasm-export-enabled? state)} - (some? name) - (assoc :name name)) + (let [exports (normalize-exports exports)] + (ptk/reify ::request-multiple-export + ptk/WatchEvent + (watch [_ state _] + (let [resource-id (volatile! nil) + profile-id (:profile-id state) + ws-conn (:ws-conn state) + params (cond-> + {:exports exports + :cmd cmd + :profile-id profile-id + :force-multiple true + :is-wasm (wasm-export-enabled? state)} + (some? name) + (assoc :name name)) - progress-stream - (->> (ws/get-rcv-stream ws-conn) - (rx/filter ws/message-event?) - (rx/map :payload) - (rx/filter #(= :export-update (:type %))) - (rx/filter #(= @resource-id (:resource-id %))) - (rx/share)) + progress-stream + (->> (ws/get-rcv-stream ws-conn) + (rx/filter ws/message-event?) + (rx/map :payload) + (rx/filter #(= :export-update (:type %))) + (rx/filter #(= @resource-id (:resource-id %))) + (rx/share)) - stopper - (rx/filter #(or (= "ended" (:status %)) - (= "error" (:status %))) - progress-stream)] + stopper + (rx/filter #(or (= "ended" (:status %)) + (= "error" (:status %))) + progress-stream)] - (swap! st/ongoing-tasks conj :export) + (swap! st/ongoing-tasks conj :export) - (rx/merge - ;; Force that all data is persisted; best effort. - (rx/of ::dwp/force-persist) + (rx/merge + ;; Force that all data is persisted; best effort. + (rx/of ::dwp/force-persist) - ;; Launch the exportation process and stores the resource id - ;; locally. - (->> (rp/cmd! :export params) - (rx/map (fn [{:keys [id] :as resource}] - (vreset! resource-id id) - (initialize-export-status exports cmd resource)))) + ;; Launch the exportation process and stores the resource id + ;; locally. + (->> (rp/cmd! :export params) + (rx/map (fn [{:keys [id] :as resource}] + (vreset! resource-id id) + (initialize-export-status exports cmd resource)))) - ;; We proceed to update the export state with incoming - ;; progress updates. We delay the stopper for give some time - ;; to update the status with ended or errored status before - ;; close the stream. - (->> progress-stream - (rx/map update-export-status) - (rx/take-until (rx/delay 500 stopper)) - (rx/finalize (fn [] - (swap! st/ongoing-tasks disj :export)))) + ;; We proceed to update the export state with incoming + ;; progress updates. We delay the stopper for give some time + ;; to update the status with ended or errored status before + ;; close the stream. + (->> progress-stream + (rx/map update-export-status) + (rx/take-until (rx/delay 500 stopper)) + (rx/finalize (fn [] + (swap! st/ongoing-tasks disj :export)))) - ;; We hide need to hide the ui elements of the export after - ;; some interval. We also delay a little bit more the stopper - ;; for ensure that after some security time, the stream is - ;; completely closed. - (->> progress-stream - (rx/filter #(= "ended" (:status %))) - (rx/take 1) - (rx/delay default-timeout) - (rx/map #(clear-export-state @resource-id)) - (rx/take-until (rx/delay 6000 stopper)))))))) + ;; We hide need to hide the ui elements of the export after + ;; some interval. We also delay a little bit more the stopper + ;; for ensure that after some security time, the stream is + ;; completely closed. + (->> progress-stream + (rx/filter #(= "ended" (:status %))) + (rx/take 1) + (rx/delay default-timeout) + (rx/map #(clear-export-state @resource-id)) + (rx/take-until (rx/delay 6000 stopper))))))))) (defn request-export [{:keys [exports] :as params}] diff --git a/frontend/src/app/main/data/workspace/clipboard.cljs b/frontend/src/app/main/data/workspace/clipboard.cljs index c87a5707f9..eca273832a 100644 --- a/frontend/src/app/main/data/workspace/clipboard.cljs +++ b/frontend/src/app/main/data/workspace/clipboard.cljs @@ -34,6 +34,7 @@ [app.config :as cf] [app.main.data.changes :as dch] [app.main.data.event :as ev] + [app.main.data.exports.assets :as de] [app.main.data.exports.wasm :as wasm.exports] [app.main.data.helpers :as dsh] [app.main.data.notifications :as ntf] @@ -1147,16 +1148,16 @@ page-id (:current-page-id state) selected (first (dsh/lookup-selected state)) - export {:file-id file-id - :page-id page-id - :object-id selected - ;; webp would be preferrable, but PNG is the most supported image MIME type by clipboard APIs. - :type :png - ;; Always use 2 to ensure good enough quality for wireframes. - :scale 2 - :suffix "" - :enabled true - :name ""} + export (de/normalize-export {:file-id file-id + :page-id page-id + :object-id selected + ;; webp would be preferrable, but PNG is the most supported image MIME type by clipboard APIs. + :type :png + ;; Always use 2 to ensure good enough quality for wireframes. + :scale 2 + :suffix "" + :enabled true + :name ""}) ;; Create a deferred promise immediately, before any async operations. ;; Registering the clipboard write NOW preserves the user-gesture security diff --git a/frontend/src/app/plugins/shape.cljs b/frontend/src/app/plugins/shape.cljs index 4a1ea9bdf9..725f69c3c8 100644 --- a/frontend/src/app/plugins/shape.cljs +++ b/frontend/src/app/plugins/shape.cljs @@ -34,6 +34,7 @@ [app.common.types.text :as txt] [app.common.uuid :as uuid] [app.config :as cf] + [app.main.data.exports.assets :as de] [app.main.data.exports.wasm :as wasm.exports] [app.main.data.persistence :as dwp] [app.main.data.plugins :as dp] @@ -1547,13 +1548,13 @@ :profile-id (:profile-id @st/state) :wait true :is-wasm false - :exports [{:file-id file-id - :page-id page-id - :object-id id - :name (:name shape) - :type (:type value :png) - :suffix (:suffix value "") - :scale (:scale value 1)}]}] + :exports [(de/normalize-export {:file-id file-id + :page-id page-id + :object-id id + :name (:name shape) + :type (:type value :png) + :suffix (:suffix value "") + :scale (:scale value 1)})]}] (js/Promise. (fn [resolve reject] ;; The exporter renders the file from its persisted diff --git a/frontend/test/frontend_tests/data/exports_assets_test.cljs b/frontend/test/frontend_tests/data/exports_assets_test.cljs new file mode 100644 index 0000000000..d4ae9edea8 --- /dev/null +++ b/frontend/test/frontend_tests/data/exports_assets_test.cljs @@ -0,0 +1,99 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC Sucursal en España SL + +(ns frontend-tests.data.exports-assets-test + (:require + [app.common.uuid :as uuid] + [app.main.data.exports.assets :as de] + [app.main.data.persistence :as dwp] + [app.main.repo :as repo] + [app.main.store :as st] + [app.util.dom :as dom] + [app.util.websocket :as ws] + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.events :as the] + [frontend-tests.helpers.mock :as mock] + [potok.v2.core :as ptk])) + +(def ^:private export {:id (uuid/next) + :object-id (uuid/next) + :type :png + :suffix "" + :scale 1}) + +(defn- export-with-name + [name] + (merge export {:name name})) + +(defn- test-state + [] + {:profile-id (:id export) + :ws-conn nil}) + +(t/deftest normalize-export-preserves-existing-name + (t/is (= (export-with-name "Layer 1") + (de/normalize-export (export-with-name "Layer 1"))))) + +(t/deftest normalize-export-replaces-nil-name-with-object-id + (t/is (= (export-with-name (str (:object-id export))) + (de/normalize-export (assoc export :name nil))))) + +(t/deftest normalize-export-replaces-empty-name-with-object-id + (t/is (= (export-with-name (str (:object-id export))) + (de/normalize-export (assoc export :name ""))))) + +(t/deftest request-simple-export-sends-normalized-export + (t/async done + (let [export (export-with-name "") + observed (atom nil)] + (mock/with-mocks {repo/cmd! (mock/stub (fn [_ params] + (reset! observed params) + (rx/of {:filename "export.png" + :mtype "image/png" + :uri "blob:export"}))) + dwp/force-persist-and-wait (mock/stub (fn [_] (rx/of ::force-persisted))) + dom/trigger-download-uri (mock/stub (fn [& _] nil))} + (fn [done'] + (let [completed (fn [_state] + (t/is (= (export-with-name (str (:object-id export))) + (-> @observed :exports first))))] + (ptk/emit! (the/prepare-store (test-state) done' completed) + (de/request-simple-export {:export export}) + :the/end))) + done)))) + +(t/deftest request-multiple-export-sends-normalized-enabled-exports + (t/async done + (let [exports [{:id "enabled-1" + :object-id "enabled-1" + :shape {:id "enabled-1"} + :type :png + :suffix "" + :scale 1 + :enabled true + :name ""}] + observed (atom nil)] + (mock/with-mocks {repo/cmd! (mock/stub (fn [_ params] + (reset! observed params) + (rx/of {:id (:id export)}))) + ws/get-rcv-stream (mock/stub (fn [_] (rx/empty))) + st/ongoing-tasks (atom #{})} + (fn [done'] + (let [completed (fn [_state] + (t/is (= [{:id "enabled-1" + :object-id "enabled-1" + :shape {:id "enabled-1"} + :type :png + :suffix "" + :scale 1 + :enabled true + :name "enabled-1"}] + (:exports @observed))))] + (ptk/emit! (the/prepare-store (test-state) done' completed) + (de/request-multiple-export {:exports exports}) + :the/end))) + done)))) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index 450f6978bd..7972b4e039 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -7,6 +7,7 @@ [frontend-tests.basic-shapes-test] [frontend-tests.code-gen-style-test] [frontend-tests.copy-as-svg-test] + [frontend-tests.data.exports-assets-test] [frontend-tests.data.nitrate-test] [frontend-tests.data.repo-test] [frontend-tests.data.store-test] @@ -88,6 +89,7 @@ 'frontend-tests.data.nitrate-test 'frontend-tests.data.repo-test 'frontend-tests.data.store-test + 'frontend-tests.data.exports-assets-test 'frontend-tests.errors-test 'frontend-tests.main-errors-test 'frontend-tests.data.uploads-test