diff --git a/CHANGES.md b/CHANGES.md index a8204ecd88..1f9447cd31 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -37,6 +37,7 @@ - Fix dotted style in strokes [Taiga #2312](https://tree.taiga.io/project/penpot/issue/2312) - Fix problem when resizing texts inside groups [Taiga #2310](https://tree.taiga.io/project/penpot/issue/2310) - Fix problem with multiple exports [Taiga #2468](https://tree.taiga.io/project/penpot/issue/2468) +- Allow import to continue from recoverable failures [#1412](https://github.com/penpot/penpot/issues/1412) ### :arrow_up: Deps updates ### :heart: Community contributions by (Thank you!) diff --git a/common/src/app/common/file_builder.cljc b/common/src/app/common/file_builder.cljc index 6fac15a102..0d6c96ff4a 100644 --- a/common/src/app/common/file_builder.cljc +++ b/common/src/app/common/file_builder.cljc @@ -12,7 +12,6 @@ [app.common.geom.shapes :as gsh] [app.common.pages.changes :as ch] [app.common.pages.init :as init] - [app.common.pages.spec :as spec] [app.common.spec :as us] [app.common.uuid :as uuid] [cuerdas.core :as str])) @@ -21,15 +20,14 @@ (def conjv (fnil conj [])) (def conjs (fnil conj #{})) -;; This flag controls if we should execute spec validation after every commit -(def verify-on-commit? true) - (defn- commit-change ([file change] (commit-change file change nil)) - ([file change {:keys [add-container?] - :or {add-container? false}}] + ([file change {:keys [add-container? + fail-on-spec?] + :or {add-container? false + fail-on-spec? false}}] (let [component-id (:current-component-id file) change (cond-> change (and add-container? (some? component-id)) @@ -39,11 +37,20 @@ (assoc :page-id (:current-page-id file) :frame-id (:current-frame-id file)))] - (when verify-on-commit? - (us/assert ::spec/change change)) - (-> file - (update :changes conjv change) - (update :data ch/process-changes [change] verify-on-commit?))))) + (when fail-on-spec? + (us/verify :app.common.pages.spec/change change)) + + (let [valid? (us/valid? :app.common.pages.spec/change change)] + #?(:cljs + (when-not valid? (.warn js/console "Invalid shape" (clj->js change)))) + + (cond-> file + valid? + (-> (update :changes conjv change) + (update :data ch/process-changes [change] false)) + + (not valid?) + (update :errors conjv change)))))) (defn- lookup-objects ([file] @@ -56,15 +63,16 @@ (get shape-id))) (defn- commit-shape [file obj] - (let [parent-id (-> file :parent-stack peek)] - (-> file - (commit-change - {:type :add-obj - :id (:id obj) - :obj obj - :parent-id parent-id} + (let [parent-id (-> file :parent-stack peek) + change {:type :add-obj + :id (:id obj) + :obj obj + :parent-id parent-id} - {:add-container? true})))) + fail-on-spec? (or (= :group (:type obj)) + (= :frame (:type obj)))] + + (commit-change file change {:add-container? true :fail-on-spec? fail-on-spec?}))) (defn setup-rect-selrect [obj] (let [rect (select-keys obj [:x :y :width :height]) @@ -421,35 +429,36 @@ (defn add-interaction [file from-id interaction-src] - (assert (some? (lookup-shape file from-id)) (str "Cannot locate shape with id " from-id)) + (let [shape (lookup-shape file from-id)] + (if (nil? shape) + file + (let [{:keys [event-type action-type]} (read-classifier interaction-src) + {:keys [delay]} (read-event-opts interaction-src) + {:keys [destination overlay-pos-type overlay-position url + close-click-outside background-overlay preserve-scroll]} + (read-action-opts interaction-src) - (let [{:keys [event-type action-type]} (read-classifier interaction-src) - {:keys [delay]} (read-event-opts interaction-src) - {:keys [destination overlay-pos-type overlay-position url - close-click-outside background-overlay preserve-scroll]} - (read-action-opts interaction-src) + interactions (-> shape + :interactions + (conjv + (d/without-nils {:event-type event-type + :action-type action-type + :delay delay + :destination destination + :overlay-pos-type overlay-pos-type + :overlay-position overlay-position + :url url + :close-click-outside close-click-outside + :background-overlay background-overlay + :preserve-scroll preserve-scroll})))] + (commit-change + file + {:type :mod-obj + :page-id (:current-page-id file) + :id from-id - interactions (-> (lookup-shape file from-id) - :interactions - (conjv - (d/without-nils {:event-type event-type - :action-type action-type - :delay delay - :destination destination - :overlay-pos-type overlay-pos-type - :overlay-position overlay-position - :url url - :close-click-outside close-click-outside - :background-overlay background-overlay - :preserve-scroll preserve-scroll})))] - (commit-change - file - {:type :mod-obj - :page-id (:current-page-id file) - :id from-id - - :operations - [{:type :set :attr :interactions :val interactions}]}))) + :operations + [{:type :set :attr :interactions :val interactions}]}))))) (defn generate-changes [file] diff --git a/common/src/app/common/spec.cljc b/common/src/app/common/spec.cljc index 19ef41255d..d568d2d9f6 100644 --- a/common/src/app/common/spec.cljc +++ b/common/src/app/common/spec.cljc @@ -31,6 +31,8 @@ (def max-safe-int (int 1e6)) (def min-safe-int (int -1e6)) +(def valid? s/valid?) + ;; --- Conformers (defn uuid-conformer @@ -216,8 +218,11 @@ :code :spec-validation :hint hint :ctx ctx + :value val ::s/problems (::s/problems data))))) + + (defmacro assert "Development only assertion macro." [spec x] diff --git a/frontend/resources/styles/main/partials/modal.scss b/frontend/resources/styles/main/partials/modal.scss index 67042f012a..e767a2c211 100644 --- a/frontend/resources/styles/main/partials/modal.scss +++ b/frontend/resources/styles/main/partials/modal.scss @@ -322,6 +322,10 @@ fill: $color-success; } + .icon-msg-warning { + fill: $color-warning; + } + .icon-close { transform: rotate(45deg); fill: $color-danger; @@ -392,6 +396,13 @@ fill: $color-white; } } + + &.warning { + background: $color-warning-lighter; + .icon { + background: $color-warning; + } + } } .error-message { diff --git a/frontend/src/app/main/ui/dashboard/import.cljs b/frontend/src/app/main/ui/dashboard/import.cljs index a076763648..014b0b7d38 100644 --- a/frontend/src/app/main/ui/dashboard/import.cljs +++ b/frontend/src/app/main/ui/dashboard/import.cljs @@ -98,7 +98,7 @@ (filter #(= :ready (:status %))) (mapv #(assoc % :status :importing)))) -(defn update-status [files file-id status progress] +(defn update-status [files file-id status progress errors] (->> files (mapv (fn [file] (cond-> file @@ -106,7 +106,10 @@ (assoc :status status) (and (= file-id (:file-id file)) (= status :import-progress)) - (assoc :progress progress)))))) + (assoc :progress progress) + + (= file-id (:file-id file)) + (assoc :errors errors)))))) (defn parse-progress-message [message] @@ -139,9 +142,10 @@ (let [loading? (or (= :analyzing (:status file)) (= :importing (:status file))) - load-success? (= :import-success (:status file)) analyze-error? (= :analyze-error (:status file)) + import-finish? (= :import-finish (:status file)) import-error? (= :import-error (:status file)) + import-warn? (d/not-empty? (:errors file)) ready? (= :ready (:status file)) is-shared? (:shared file) progress (:progress file) @@ -177,7 +181,8 @@ [:div.file-entry {:class (dom/classnames :loading loading? - :success load-success? + :success (and import-finish? (not import-warn?) (not import-error?)) + :warning (and import-finish? import-warn? (not import-error?)) :error (or import-error? analyze-error?) :editable (and ready? (not editing?)))} @@ -185,8 +190,9 @@ [:div.file-icon (cond loading? i/loader-pencil ready? i/logo-icon - load-success? i/tick + import-warn? i/msg-warning import-error? i/close + import-finish? i/tick analyze-error? i/close)] (if editing? @@ -212,7 +218,7 @@ [:div.error-message (tr "dashboard.import.import-error")] - (and (not load-success?) (some? progress)) + (and (not import-finish?) (some? progress)) [:div.progress-message (parse-progress-message progress)]) [:div.linked-libraries @@ -258,9 +264,8 @@ :project-id project-id :files files}) (rx/subs - (fn [{:keys [file-id status message] :as msg}] - (log/debug :msg msg) - (swap! state update :files update-status file-id status message)))))) + (fn [{:keys [file-id status message errors] :as msg}] + (swap! state update :files update-status file-id status message errors)))))) handle-cancel (mf/use-callback @@ -291,7 +296,8 @@ (st/emit! (modal/hide)) (when on-finish-import (on-finish-import)))) - success-files (->> @state :files (filter #(= (:status %) :import-success)) count) + warning-files (->> @state :files (filter #(and (= (:status %) :import-finish) (d/not-empty? (:errors %)))) count) + success-files (->> @state :files (filter #(and (= (:status %) :import-finish) (empty? (:errors %)))) count) pending-analysis? (> (->> @state :files (filter #(= (:status %) :analyzing)) count) 0) pending-import? (> (->> @state :files (filter #(= (:status %) :importing)) count) 0)] @@ -316,11 +322,15 @@ {:on-click handle-cancel} i/close]] [:div.modal-content - (when (and (= :importing (:status @state)) - (not pending-import?)) - [:div.feedback-banner - [:div.icon i/checkbox-checked] - [:div.message (tr "dashboard.import.import-message" success-files)]]) + (when (and (= :importing (:status @state)) (not pending-import?)) + (if (> warning-files 0) + [:div.feedback-banner.warning + [:div.icon i/msg-warning] + [:div.message (tr "dashboard.import.import-warning" warning-files success-files)]] + + [:div.feedback-banner + [:div.icon i/checkbox-checked] + [:div.message (tr "dashboard.import.import-message" success-files)]])) (for [file (->> (:files @state) (filterv (comp not :deleted?)))] (let [editing? (and (some? (:file-id file)) diff --git a/frontend/src/app/worker/import.cljs b/frontend/src/app/worker/import.cljs index 5849ca22ca..11473c0af6 100644 --- a/frontend/src/app/worker/import.cljs +++ b/frontend/src/app/worker/import.cljs @@ -176,7 +176,9 @@ (rx/tap #(reset! revn (:revn %))) (rx/ignore)) - (rp/mutation :persist-temp-file {:id file-id})))) + (->> (rp/mutation :persist-temp-file {:id file-id}) + ;; We use merge to keep some information not stored in back-end + (rx/map #(merge file %)))))) (defn upload-media-files "Upload a image to the backend and returns its id" @@ -457,8 +459,7 @@ (let [progress-str (rx/subject) context (assoc context :progress progress-str)] - (rx/merge - progress-str + [progress-str (->> (rx/of file) (rx/flat-map (partial process-pages context)) (rx/tap #(progress! context :process-colors)) @@ -470,7 +471,7 @@ (rx/tap #(progress! context :process-components)) (rx/flat-map (partial process-library-components context)) (rx/flat-map (partial send-changes context)) - (rx/tap #(rx/end! progress-str)))))) + (rx/tap #(rx/end! progress-str)))])) (defn create-files [context files] @@ -482,7 +483,6 @@ (rx/flat-map (fn [context] (->> (create-file context) - (rx/tap #(.log js/console "create-file" (clj->js %))) (rx/map #(vector % (first (get data (:file-id context))))))))) (->> (rx/from files) @@ -509,20 +509,29 @@ (let [context {:project-id project-id :resolve (resolve-factory)}] (->> (create-files context files) - (rx/catch #(.error js/console "IMPORT ERROR" %)) + (rx/catch #(.error js/console "IMPORT ERROR" (clj->js %))) (rx/flat-map (fn [[file data]] (->> (rx/concat (->> (uz/load-from-url (:uri data)) (rx/map #(-> context (assoc :zip %) (merge data))) - (rx/flat-map #(process-file % file))) - (rx/of - {:status :import-success - :file-id (:file-id data)})) + (rx/flat-map + (fn [context] + ;; process file retrieves a stream that will emit progress notifications + ;; and other that will emit the files once imported + (let [[progress-stream file-stream] (process-file context file)] + (rx/merge + progress-stream + (->> file-stream + (rx/map + (fn [file] + {:status :import-finish + :errors (:errors file) + :file-id (:file-id data)}))))))))) (rx/catch (fn [err] - (.error js/console "ERROR" (:file-id data) err) + (.error js/console "ERROR" (str (:file-id data)) (clj->js err) (clj->js (.-data err))) (rx/of {:status :import-error :file-id (:file-id data) :error (.-message err) diff --git a/frontend/translations/en.po b/frontend/translations/en.po index bd1bdcc96f..4f2e5c615c 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -3322,3 +3322,6 @@ msgstr "Insufficient members to leave team, you probably want to delete it." msgid "errors.auth.unable-to-login" msgstr "Looks like you are not authenticated or session expired." + +msgid "dashboard.import.import-warning" +msgstr "Some files containted invalid objects that have been removed." diff --git a/frontend/translations/es.po b/frontend/translations/es.po index 60f9b7b88d..025d67cbb0 100644 --- a/frontend/translations/es.po +++ b/frontend/translations/es.po @@ -3309,3 +3309,6 @@ msgstr "Actualizar" msgid "workspace.viewport.click-to-close-path" msgstr "Pulsar para cerrar la ruta" + +msgid "dashboard.import.import-warning" +msgstr "Algunos ficheros contenían objetos erroneos que no han sido importados."