From 16ed09a303daa1bca7ff77d38962ab9df614ffeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Moya?= Date: Fri, 10 Dec 2021 10:50:18 +0100 Subject: [PATCH 1/3] :bug: Fix error importing file with null destination in one interaction --- common/src/app/common/types/interactions.cljc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/common/src/app/common/types/interactions.cljc b/common/src/app/common/types/interactions.cljc index 3f54ee2a8..838dbff9c 100644 --- a/common/src/app/common/types/interactions.cljc +++ b/common/src/app/common/types/interactions.cljc @@ -72,17 +72,17 @@ (s/keys :opt-un [::destination ::preserve-scroll])) (defmethod action-opts-spec :open-overlay [_] - (s/keys :req-un [::destination - ::overlay-position + (s/keys :req-un [::overlay-position ::overlay-pos-type] - :opt-un [::close-click-outside + :opt-un [::destination + ::close-click-outside ::background-overlay])) (defmethod action-opts-spec :toggle-overlay [_] - (s/keys :req-un [::destination - ::overlay-position + (s/keys :req-un [::overlay-position ::overlay-pos-type] - :opt-un [::close-click-outside + :opt-un [::destination + ::close-click-outside ::background-overlay])) (defmethod action-opts-spec :close-overlay [_] From a3016b8400141b3d980fcf1128445e9e38d5427a Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 10 Dec 2021 12:19:12 +0100 Subject: [PATCH 2/3] :sparkles: Make the media uploading idempotent. --- backend/src/app/rpc/mutations/media.clj | 28 +++++++++------ backend/test/app/services_media_test.clj | 45 ++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/backend/src/app/rpc/mutations/media.clj b/backend/src/app/rpc/mutations/media.clj index 90e5e0c14..8f9075cf1 100644 --- a/backend/src/app/rpc/mutations/media.clj +++ b/backend/src/app/rpc/mutations/media.clj @@ -92,6 +92,16 @@ :content-type mtype :expired-at (dt/in-future {:minutes 30})})))) +;; NOTE: we use the `on conflict do update` instead of `do nothing` +;; because postgresql does not returns anything if no update is +;; performed, the `do update` does the trick. + +(def sql:create-file-media-object + "insert into file_media_object (id, file_id, is_local, name, media_id, thumbnail_id, width, height, mtype) + values (?, ?, ?, ?, ?, ?, ?, ?, ?) + on conflict (id) do update set created_at=file_media_object.created_at + returning *") + (defn create-file-media-object [{:keys [conn storage] :as cfg} {:keys [id file-id is-local name content] :as params}] (media/validate-media-type (:content-type content)) @@ -117,17 +127,15 @@ thumb (when thumb (sto/put-object storage {:content (sto/content (:data thumb) (:size thumb)) :content-type (:mtype thumb)}))] - (db/insert! conn :file-media-object - {:id (or id (uuid/next)) - :file-id file-id - :is-local is-local - :name name - :media-id (:id image) - :thumbnail-id (:id thumb) - :width (:width source-info) - :height (:height source-info) - :mtype source-mtype}))) + (db/exec-one! conn [sql:create-file-media-object + (or id (uuid/next)) + file-id is-local name + (:id image) + (:id thumb) + (:width source-info) + (:height source-info) + source-mtype]))) ;; --- Create File Media Object (from URL) diff --git a/backend/test/app/services_media_test.clj b/backend/test/app/services_media_test.clj index a0e9c9780..1e8bd1ffe 100644 --- a/backend/test/app/services_media_test.clj +++ b/backend/test/app/services_media_test.clj @@ -86,3 +86,48 @@ (t/is (= 312043 (:size mobj1))) (t/is (= 3887 (:size mobj2))))) )) + + +(t/deftest media-object-upload-idempotency + (let [prof (th/create-profile* 1) + proj (th/create-project* 1 {:profile-id (:id prof) + :team-id (:default-team-id prof)}) + file (th/create-file* 1 {:profile-id (:id prof) + :project-id (:default-project-id prof) + :is-shared false}) + mfile {:filename "sample.jpg" + :tempfile (th/tempfile "app/test_files/sample.jpg") + :content-type "image/jpeg" + :size 312043} + + params {::th/type :upload-file-media-object + :profile-id (:id prof) + :file-id (:id file) + :is-local true + :name "testfile" + :content mfile + :id (uuid/next)}] + + ;; First try + (let [{:keys [result error] :as out} (th/mutation! params)] + ;; (th/print-result! out) + (t/is (nil? error)) + (t/is (= (:id params) (:id result))) + (t/is (= (:file-id params) (:file-id result))) + (t/is (= 800 (:width result))) + (t/is (= 800 (:height result))) + (t/is (= "image/jpeg" (:mtype result))) + (t/is (uuid? (:media-id result))) + (t/is (uuid? (:thumbnail-id result)))) + + ;; Second try + (let [{:keys [result error] :as out} (th/mutation! params)] + ;; (th/print-result! out) + (t/is (nil? error)) + (t/is (= (:id params) (:id result))) + (t/is (= (:file-id params) (:file-id result))) + (t/is (= 800 (:width result))) + (t/is (= 800 (:height result))) + (t/is (= "image/jpeg" (:mtype result))) + (t/is (uuid? (:media-id result))) + (t/is (uuid? (:thumbnail-id result)))))) From 384f0a05c6a4765bd069f1674f822a8d6231b132 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 9 Dec 2021 16:49:06 +0100 Subject: [PATCH 3/3] :bug: Fix race condition issues on workspace. --- frontend/src/app/main/data/workspace.cljs | 14 ++++---- .../src/app/main/data/workspace/changes.cljs | 4 +-- .../src/app/main/data/workspace/common.cljs | 6 ++++ .../src/app/main/data/workspace/groups.cljs | 2 +- .../app/main/data/workspace/persistence.cljs | 29 +++++++++++++---- .../src/app/main/data/workspace/texts.cljs | 32 +++++++++---------- .../viewport/thumbnail_renderer.cljs | 16 +++------- 7 files changed, 58 insertions(+), 45 deletions(-) diff --git a/frontend/src/app/main/data/workspace.cljs b/frontend/src/app/main/data/workspace.cljs index 8e905b220..ba952977e 100644 --- a/frontend/src/app/main/data/workspace.cljs +++ b/frontend/src/app/main/data/workspace.cljs @@ -266,12 +266,10 @@ (ptk/reify ::finalize-page ptk/UpdateEvent (update [_ state] - (let [page-id (or page-id (get-in state [:workspace-data :pages 0])) - local (-> (:workspace-local state) - (dissoc - :edition - :edit-path - :selected))] + (let [local (-> (:workspace-local state) + (dissoc :edition + :edit-path + :selected))] (-> state (assoc-in [:workspace-cache page-id] local) (dissoc :current-page-id :workspace-local :trimmed-page :workspace-drawing)))))) @@ -348,8 +346,7 @@ (declare purge-page) (declare go-to-file) -;; TODO: properly handle positioning on undo. - +;; TODO: for some reason, the page-id here in some circumstances is `nil` (defn delete-page [id] (ptk/reify ::delete-page @@ -669,6 +666,7 @@ (watch [_ _ _] (rx/of (dch/update-shapes [id] #(merge % attrs)))))) + (defn start-rename-shape [id] (us/verify ::us/uuid id) diff --git a/frontend/src/app/main/data/workspace/changes.cljs b/frontend/src/app/main/data/workspace/changes.cljs index 7b481bff3..309d47c52 100644 --- a/frontend/src/app/main/data/workspace/changes.cljs +++ b/frontend/src/app/main/data/workspace/changes.cljs @@ -114,9 +114,9 @@ :changes changes})))) (defn commit-changes - [{:keys [redo-changes undo-changes origin save-undo? file-id] + [{:keys [redo-changes undo-changes + origin save-undo? file-id] :or {save-undo? true}}] - (log/debug :msg "commit-changes" :js/redo-changes redo-changes :js/undo-changes undo-changes) diff --git a/frontend/src/app/main/data/workspace/common.cljs b/frontend/src/app/main/data/workspace/common.cljs index 6cb0af4f7..6b4a1c41d 100644 --- a/frontend/src/app/main/data/workspace/common.cljs +++ b/frontend/src/app/main/data/workspace/common.cljs @@ -31,6 +31,12 @@ (s/def ::set-of-string (s/every string? :kind set?)) (s/def ::ordered-set-of-uuid (s/every uuid? :kind d/ordered-set?)) +(defn initialized? + "Check if the state is properly intialized in a workspace. This means + it has the `:current-page-id` and `:current-file-id` properly set." + [state] + (and (uuid? (:current-file-id state)) + (uuid? (:current-page-id state)))) ;; --- Helpers diff --git a/frontend/src/app/main/data/workspace/groups.cljs b/frontend/src/app/main/data/workspace/groups.cljs index c3a8acd0a..1489e9007 100644 --- a/frontend/src/app/main/data/workspace/groups.cljs +++ b/frontend/src/app/main/data/workspace/groups.cljs @@ -136,7 +136,7 @@ (defn prepare-remove-group [page-id group objects] - (let [shapes (:shapes group) + (let [shapes (into [] (:shapes group)) ; ensure we always have vector parent-id (cp/get-parent (:id group) objects) parent (get objects parent-id) diff --git a/frontend/src/app/main/data/workspace/persistence.cljs b/frontend/src/app/main/data/workspace/persistence.cljs index f24d2a280..f16b63783 100644 --- a/frontend/src/app/main/data/workspace/persistence.cljs +++ b/frontend/src/app/main/data/workspace/persistence.cljs @@ -56,13 +56,14 @@ (rx/filter dch/commit-changes?) (rx/debounce 2000) (rx/merge stoper forcer)) - - local-file? #(as-> (:file-id %) event-file-id - (or (nil? event-file-id) - (= event-file-id file-id))) - library-file? #(as-> (:file-id %) event-file-id - (and (some? event-file-id) - (not= event-file-id file-id))) + local-file? + #(as-> (:file-id %) event-file-id + (or (nil? event-file-id) + (= event-file-id file-id))) + library-file? + #(as-> (:file-id %) event-file-id + (and (some? event-file-id) + (not= event-file-id file-id))) on-dirty (fn [] @@ -565,6 +566,20 @@ [frame-id] (ptk/event ::update-frame-thumbnail {:frame-id frame-id})) +(defn update-shape-thumbnail + "An event that is succeptible to be executed out of the main flow, so + it need to correctly handle the situation that there are no page-id + or file-is loaded." + [shape-id thumbnail-data] + (ptk/reify ::update-shape-thumbnail + ptk/WatchEvent + (watch [_ state _] + (when (and (dwc/initialized? state) + (uuid? shape-id)) + (rx/of (dch/update-shapes [shape-id] + #(assoc % :thumbnail thumbnail-data) + {:save-undo? false})))))) + (defn- extract-frame-changes "Process a changes set in a commit to extract the frames that are channging" [[event [old-objects new-objects]]] diff --git a/frontend/src/app/main/data/workspace/texts.cljs b/frontend/src/app/main/data/workspace/texts.cljs index 53e123c16..54421964e 100644 --- a/frontend/src/app/main/data/workspace/texts.cljs +++ b/frontend/src/app/main/data/workspace/texts.cljs @@ -54,23 +54,23 @@ (ptk/reify ::finalize-editor-state ptk/WatchEvent (watch [_ state _] - (let [content (-> (get-in state [:workspace-editor-state id]) - (ted/get-editor-current-content))] + (when (dwc/initialized? state) + (let [content (-> (get-in state [:workspace-editor-state id]) + (ted/get-editor-current-content))] + (if (ted/content-has-text? content) + (let [content (d/merge (ted/export-content content) + (dissoc (:content shape) :children))] + (rx/merge + (rx/of (update-editor-state shape nil)) + (when (and (not= content (:content shape)) + (some? (:current-page-id state))) + (rx/of + (dch/update-shapes [id] #(assoc % :content content)) + (dwu/commit-undo-transaction))))) - (if (ted/content-has-text? content) - (let [content (d/merge (ted/export-content content) - (dissoc (:content shape) :children))] - (rx/merge - (rx/of (update-editor-state shape nil)) - (when (and (not= content (:content shape)) - (some? (:current-page-id state))) - (rx/of - (dch/update-shapes [id] #(assoc % :content content)) - (dwu/commit-undo-transaction))))) - - (when (some? id) - (rx/of (dws/deselect-shape id) - (dwc/delete-shapes #{id})))))))) + (when (some? id) + (rx/of (dws/deselect-shape id) + (dwc/delete-shapes #{id}))))))))) (defn initialize-editor-state [{:keys [id content] :as shape} decorator] diff --git a/frontend/src/app/main/ui/workspace/viewport/thumbnail_renderer.cljs b/frontend/src/app/main/ui/workspace/viewport/thumbnail_renderer.cljs index 95f27e5a9..85a9ab316 100644 --- a/frontend/src/app/main/ui/workspace/viewport/thumbnail_renderer.cljs +++ b/frontend/src/app/main/ui/workspace/viewport/thumbnail_renderer.cljs @@ -6,7 +6,6 @@ (ns app.main.ui.workspace.viewport.thumbnail-renderer (:require - [app.main.data.workspace.changes :as dwc] [app.main.data.workspace.persistence :as dwp] [app.main.store :as st] [app.util.dom :as dom] @@ -80,7 +79,7 @@ "Component in charge of creating thumbnails and storing them" {::mf/wrap-props false} [props] - (let [objects (obj/get props "objects") + (let [objects (obj/get props "objects") background (obj/get props "background") ;; Id of the current frame being rendered @@ -97,12 +96,9 @@ updates-stream (mf/use-memo - (fn [] - (let [update-events - (->> st/stream - (rx/filter dwp/update-frame-thumbnail?))] - (->> (rx/zip update-events next) - (rx/map first))))) + #(let [update-events (rx/filter dwp/update-frame-thumbnail? st/stream)] + (->> (rx/zip update-events next) + (rx/map first)))) on-thumbnail-data (mf/use-callback @@ -111,9 +107,7 @@ (reset! shape-id nil) (timers/schedule (fn [] - (st/emit! (dwc/update-shapes [@shape-id] - #(assoc % :thumbnail data) - {:save-undo? false})) + (st/emit! (dwp/update-shape-thumbnail @shape-id data)) (rx/push! next :next))))) on-frame-not-found