mirror of
https://github.com/penpot/penpot.git
synced 2026-07-31 18:36:18 +00:00
🐛 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
This commit is contained in:
Andrey Antukh
GitHub
parent
fadb3124a0
commit
040080749b
@ -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}]
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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
|
||||
|
||||
99
frontend/test/frontend_tests/data/exports_assets_test.cljs
Normal file
99
frontend/test/frontend_tests/data/exports_assets_test.cljs
Normal file
@ -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))))
|
||||
@ -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
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user