diff --git a/.serena/memories/frontend/workspace-state-persistence-subtleties.md b/.serena/memories/frontend/workspace-state-persistence-subtleties.md index ceddce4588..1543bfdf39 100644 --- a/.serena/memories/frontend/workspace-state-persistence-subtleties.md +++ b/.serena/memories/frontend/workspace-state-persistence-subtleties.md @@ -18,7 +18,7 @@ ## Commits, undo, persistence -- `commit-changes` refuses to create commits unless `:permissions :can-edit` is true. It captures file revn/vern, selected-before, features, tags, undo group, and translation flag into a `::commit` event. A refusal answers with an empty stream, never nil, so callers subscribe uniformly instead of branching on nil. +- `commit-changes` refuses to create commits unless `:permissions :can-edit` is true, and while the save-failure lock holds (`plk/locked?`) unless the params carry `:ignore-save-lock true`: only `compensate-commits` (rollbacks run while locked: a role-change release, a slow-cycle answer `:rejected`), the two `#{:position-data}` commits and `dwl/touch-component` (derived from commits already made). Every other commit, user edit or not (deleted-font fix, stale-text resize, component sync), is refused while locked; never add an origin list. Remote commits go through `dch/commit`, never refused. It captures file revn/vern, selected-before, features, tags, undo group, and translation flag into a `::commit` event. A refusal answers with an empty stream, never nil, so callers subscribe uniformly instead of branching on nil. - Applying a remote commit first rolls back the pending local commits of its own file (`pcm/pending-commits`: queued entries, then `[:persistence :buffer]`; other files' commits never touch its data), applies the remote changes, then replays pending local redo changes. Index updates are emitted for undo, remote redo, and replayed redo paths. Leaving the buffer out makes a remote change to the same attr win locally but lose on the server. - Local commits are independently consumed by undo, persistence, WASM model updates, thumbnail/library watchers, and text position-data recalculation. - Persistence buffers local commits in the store (`buffer-commit` → `[:persistence :buffer]`, stamped with `:remote-seq`): status becomes pending after about 200ms; the 3s debounce or `::force-persist` emits `flush-buffer`, which merges the buffer per file into `append-commit`s, and each append removes its own commits from the buffer (a commit arriving mid-flush stays). `initialize-persistence-state` flushes a leftover buffer. Queue entries (`pcm/queue-entry`) hold the entry keys of the first commit (id, created-at, origin, features, file-id/revn/vern) and `:commits` (id, origin, undo-group, redo/undo changes, remote-seq of each merged commit), no merged change lists; every reader goes through `:commits`. The save sends `:changes-with-metadata`, one group per commit with `:commit-id`, never flat `:changes`. @@ -41,7 +41,7 @@ - `persistence-failed` emits a `save-failed` audit event (`:cause-type :code :retrying :queued-commits`) whenever the user is notified (`notify-failure?`, same rule as the toast), including delegated auth failures. - `start-undo-transaction` stamps the transaction with a fresh `:commit-group` (dropped from the history entry); `commit-changes` uses `(or undo-group (:undo-group tx) (:commit-group tx) (uuid/next))`, so all commits of one transaction are one rollback unit. Pre-setting `:undo-group` instead breaks entries whose group is set later (sync transactions). - Undo transactions can stay open only temporarily; timed-out pending transactions are force-committed after about 20s. Undo entries are capped at 50. -- Undo/redo are ignored while a normal editor/drawing interaction is active, except grid-layout edition handles undo through this path. +- Undo/redo are ignored while a normal editor/drawing interaction is active, except grid-layout edition handles undo through this path. `undo`, `redo` and `undo-to-index` also do nothing while the save lock holds: `materialize-undo` would move the index while the commit is refused. - After local commits and when render-wasm is active, text shapes get derived `:position-data` recomputed in a separate commit tagged `#{:position-data}`; that tag is excluded from the position-data watcher to avoid loops. ## Refs diff --git a/.serena/memories/frontend/workspace-transform-subtleties.md b/.serena/memories/frontend/workspace-transform-subtleties.md index 958e121c9c..c96e68bca3 100644 --- a/.serena/memories/frontend/workspace-transform-subtleties.md +++ b/.serena/memories/frontend/workspace-transform-subtleties.md @@ -6,6 +6,7 @@ - `apply-modifiers*` is the lower-level commit path once object/text modifiers are ready. It updates frame guides, frame comment threads, and then emits `update-shapes` with `:reg-objects? true`. - Transform commits restrict diff attrs to `transform-attrs` to avoid scanning unrelated shape attrs. - Text transforms may carry derived `:position-data`; `assoc-position-data` attaches it while preserving the original text shape context. +- A drag commits when its `mse/drag-stopper` fires, `:interrupt` (Escape) included. The save-failure lock emits `::plk/cancel-gestures` first: drags built on `plk/gesture-stopper` (start-move, start-resize, start-rotate, start-move-line-point, box/curve/line drawing) see a `cancelled?` flag set before the stop and emit `dwt/cancel-transform` (`clear-local-transform` + `finish-transform`) or `cancel-drawing` instead. A new drag gesture must use it (or, in a component, drop the drag when read-only turns on): `commit-changes` refuses the commit anyway, but the preview (WASM modifiers/props) would stay. ## Component-copy touched suppression diff --git a/frontend/src/app/main/data/changes.cljs b/frontend/src/app/main/data/changes.cljs index b72ab94118..e114ed26b7 100644 --- a/frontend/src/app/main/data/changes.cljs +++ b/frontend/src/app/main/data/changes.cljs @@ -17,6 +17,7 @@ [app.main.data.event :as ev] [app.main.data.helpers :as dsh] [app.main.data.persistence.commits :as pcm] + [app.main.data.persistence.lock :as plk] [app.main.data.workspace :as-alias dw] [app.main.features :as features] [app.main.worker :as mw] @@ -276,9 +277,12 @@ undo-group, they will be undone or redone in a single step. Without it, a commit made while an undo transaction is open joins the transaction's group, and any other gets a new one. + - ignore-save-lock: commit even while the save-failure lock holds. Only for + commits that are not user edits: the undo of rolled-back + commits and data derived from commits already made. " [{:keys [redo-changes undo-changes save-undo? undo-group tags stack-undo? file-id - translation? skip-component-sync?] + translation? skip-component-sync? ignore-save-lock] :or {save-undo? true stack-undo? false tags #{}} @@ -296,10 +300,12 @@ ;; Historical previews must not create edits to the live file. Check ;; this when creating commits so previously queued edits can still save. + ;; While saves fail, a user edit would queue behind the failed save. ;; Refusals answer with an empty stream (never nil) so callers can ;; uniformly subscribe and observe termination. (if (and (:can-edit permissions) - (not (dm/get-in state [:workspace-global :preview-id]))) + (not (dm/get-in state [:workspace-global :preview-id])) + (or ignore-save-lock (not (plk/locked? state)))) (do (log/trace :hint "commit-changes" :redo-changes redo-changes) (let [selected (dm/get-in state [:workspace-local :selected]) diff --git a/frontend/src/app/main/data/persistence.cljs b/frontend/src/app/main/data/persistence.cljs index 0e384a90f8..1513ca5730 100644 --- a/frontend/src/app/main/data/persistence.cljs +++ b/frontend/src/app/main/data/persistence.cljs @@ -616,7 +616,9 @@ :file-id file-id :save-undo? false ;; Its own group, never that of an open undo transaction - :undo-group (uuid/next))) + :undo-group (uuid/next) + ;; Rollbacks also run while saves fail + :ignore-save-lock true)) (let [{:keys [file-revn file-vern features]} (peek commits)] (append-commit (pcm/queue-entry diff --git a/frontend/src/app/main/data/persistence/lock.cljs b/frontend/src/app/main/data/persistence/lock.cljs index b9ad3cd8ce..95e7ddba92 100644 --- a/frontend/src/app/main/data/persistence/lock.cljs +++ b/frontend/src/app/main/data/persistence/lock.cljs @@ -9,6 +9,7 @@ (:require [app.common.data.macros :as dm] [app.main.data.workspace.edition :as dwe] + [app.util.mouse :as mse] [beicon.v2.core :as rx] [potok.v2.core :as ptk])) @@ -19,9 +20,35 @@ (or (= :error (:status pstate)) (true? (:recovering pstate)))) +(defn locked? + "True while the workspace is read-only because saves fail." + [state] + (some? (dm/get-in state [:workspace-global :persistence-lock]))) + +(defn cancel-gestures? + "True for the event that cancels the gestures in progress when saves + start failing; it comes before the `:interrupt` that stops them, so a + drag puts its shapes back instead of committing its partial move." + [event] + (ptk/type? ::cancel-gestures event)) + +(defn gesture-stopper + "Stops a drag as `mse/drag-stopper` does, and also when the gestures in + progress are cancelled because saves fail; `cancelled?` turns true + before that stop reaches the drag. A drag that sees it ends without + committing anything." + ([stream cancelled?] + (gesture-stopper stream cancelled? nil)) + ([stream cancelled? opts] + (rx/merge (mse/drag-stopper stream opts) + (->> stream + (rx/filter cancel-gestures?) + (rx/tap (fn [_] (vreset! cancelled? true))))))) + (defn- lock-on-failure "Puts the workspace in read-only mode and keeps the read-only value it - had, which `unlock-after-failure` brings back." + had, which `unlock-after-failure` brings back. Gestures in progress are + cancelled, so they commit nothing." [] (ptk/reify ::lock-on-failure ptk/UpdateEvent @@ -36,7 +63,8 @@ ptk/WatchEvent (watch [_ _ _] - (rx/of :interrupt + (rx/of (ptk/data-event ::cancel-gestures) + :interrupt (dwe/clear-edition-mode))))) (defn- unlock-after-failure diff --git a/frontend/src/app/main/data/workspace.cljs b/frontend/src/app/main/data/workspace.cljs index 28e14bc3bd..7fa3d39b77 100644 --- a/frontend/src/app/main/data/workspace.cljs +++ b/frontend/src/app/main/data/workspace.cljs @@ -243,7 +243,9 @@ :save-undo? false :origin it :tags #{:position-data} - :skip-component-sync? true})) + :skip-component-sync? true + ;; Derived from the text, so it follows it while saves fail + :ignore-save-lock true})) (rx/empty))))))) (defn- workspace-initialized @@ -579,7 +581,9 @@ {:redo-changes changes :undo-changes [] :save-undo? false :tags #{:position-data} - :skip-component-sync? true}))))) + :skip-component-sync? true + ;; Derived from commits already made, rollbacks included + :ignore-save-lock true}))))) (rx/take-until stoper-s))) (->> stream diff --git a/frontend/src/app/main/data/workspace/drawing/box.cljs b/frontend/src/app/main/data/workspace/drawing/box.cljs index 18f510f722..3699229bd7 100644 --- a/frontend/src/app/main/data/workspace/drawing/box.cljs +++ b/frontend/src/app/main/data/workspace/drawing/box.cljs @@ -21,11 +21,11 @@ [app.common.types.shape.layout :as ctl] [app.common.uuid :as uuid] [app.main.data.helpers :as dsh] + [app.main.data.persistence.lock :as plk] [app.main.data.workspace.drawing.common :as common] [app.main.snap :as snap] [app.main.streams :as ms] [app.util.array :as array] - [app.util.mouse :as mse] [beicon.v2.core :as rx] [potok.v2.core :as ptk])) @@ -93,7 +93,8 @@ (ptk/reify ::handle-drawing ptk/WatchEvent (watch [_ state stream] - (let [stopper (mse/drag-stopper stream) + (let [cancelled? (volatile! false) + stopper (plk/gesture-stopper stream cancelled?) layout (get state :workspace-layout) zoom (dm/get-in state [:workspace-local :zoom] 1) @@ -149,5 +150,7 @@ (fn [[_ shift? mod? point]] #(update-drawing % initial (cond-> point snap-pixel? (gpt/round-step 1)) shift? mod? snap-pixel?)))))) - (->> (rx/of (common/handle-finish-drawing)) + (->> (rx/defer #(rx/of (if @cancelled? + (common/cancel-drawing) + (common/handle-finish-drawing)))) (rx/delay 100))))))) diff --git a/frontend/src/app/main/data/workspace/drawing/common.cljs b/frontend/src/app/main/data/workspace/drawing/common.cljs index 930d91a280..bb54665da3 100644 --- a/frontend/src/app/main/data/workspace/drawing/common.cljs +++ b/frontend/src/app/main/data/workspace/drawing/common.cljs @@ -132,3 +132,16 @@ ;; Let the current mouse event finish before clearing drawing state. (->> (rx/of (clear-drawing {:preserve-tool? (= tool :curve)})) (rx/delay 0))))))) + +(defn cancel-drawing + "Ends the drawing in progress without adding its shape, as when saves + start failing mid-gesture." + [] + (ptk/reify ::cancel-drawing + ptk/UpdateEvent + (update [_ state] + (update state :workspace-drawing dissoc :object)) + + ptk/WatchEvent + (watch [_ _ _] + (rx/of (handle-finish-drawing))))) diff --git a/frontend/src/app/main/data/workspace/drawing/curve.cljs b/frontend/src/app/main/data/workspace/drawing/curve.cljs index efc670b492..f15636a5f5 100644 --- a/frontend/src/app/main/data/workspace/drawing/curve.cljs +++ b/frontend/src/app/main/data/workspace/drawing/curve.cljs @@ -17,9 +17,9 @@ [app.common.types.shape.layout :as ctl] [app.common.uuid :as uuid] [app.main.data.helpers :as dsh] + [app.main.data.persistence.lock :as plk] [app.main.data.workspace.drawing.common :as common] [app.main.streams :as ms] - [app.util.mouse :as mse] [app.util.path.simplify-curve :as ups] [beicon.v2.core :as rx] [potok.v2.core :as ptk])) @@ -104,19 +104,22 @@ (ptk/reify ::handle-drawing ptk/WatchEvent (watch [_ _ stream] - (let [stopper (mse/drag-stopper stream) - mouse (rx/sample 10 ms/mouse-position) - shape (cts/setup-shape {:type :path - :initialized? true - :frame-id uuid/zero - :parent-id uuid/zero - ::points []})] + (let [cancelled? (volatile! false) + stopper (plk/gesture-stopper stream cancelled?) + mouse (rx/sample 10 ms/mouse-position) + shape (cts/setup-shape {:type :path + :initialized? true + :frame-id uuid/zero + :parent-id uuid/zero + ::points []})] (rx/concat (rx/of #(update % :workspace-drawing assoc :object shape)) (->> mouse (rx/map insert-point) (rx/take-until stopper)) - (rx/of - (setup-frame) - (finish-drawing) - (common/handle-finish-drawing))))))) + (rx/defer + #(if @cancelled? + (rx/of (common/cancel-drawing)) + (rx/of (setup-frame) + (finish-drawing) + (common/handle-finish-drawing))))))))) diff --git a/frontend/src/app/main/data/workspace/drawing/line.cljs b/frontend/src/app/main/data/workspace/drawing/line.cljs index 10fc76f6b7..355862d3f3 100644 --- a/frontend/src/app/main/data/workspace/drawing/line.cljs +++ b/frontend/src/app/main/data/workspace/drawing/line.cljs @@ -27,9 +27,9 @@ [app.common.types.shape.layout :as ctl] [app.common.uuid :as uuid] [app.main.data.helpers :as dsh] + [app.main.data.persistence.lock :as plk] [app.main.data.workspace.drawing.common :as common] [app.main.streams :as ms] - [app.util.mouse :as mse] [beicon.v2.core :as rx] [potok.v2.core :as ptk])) @@ -128,17 +128,18 @@ (ptk/reify ::handle-drawing ptk/WatchEvent (watch [_ state stream] - (let [stopper (mse/drag-stopper stream) - zoom (dm/get-in state [:workspace-local :zoom] 1) - start @ms/mouse-position - shape (cts/setup-shape {:type :path - :initialized? true - :click-draw? true - :frame-id uuid/zero - :parent-id uuid/zero - :strokes (arrow-strokes arrow?) - :fills [] - :content (path/points->content [start start])})] + (let [cancelled? (volatile! false) + stopper (plk/gesture-stopper stream cancelled?) + zoom (dm/get-in state [:workspace-local :zoom] 1) + start @ms/mouse-position + shape (cts/setup-shape {:type :path + :initialized? true + :click-draw? true + :frame-id uuid/zero + :parent-id uuid/zero + :strokes (arrow-strokes arrow?) + :fills [] + :content (path/points->content [start start])})] (rx/concat (rx/of #(update % :workspace-drawing assoc :object shape)) @@ -152,7 +153,9 @@ (rx/map (partial update-endpoint start)) (rx/take-until stopper)) - (rx/of - (setup-frame) - (finalize) - (common/handle-finish-drawing)))))))) + (rx/defer + #(if @cancelled? + (rx/of (common/cancel-drawing)) + (rx/of (setup-frame) + (finalize) + (common/handle-finish-drawing)))))))))) diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index c096727e46..5a6758fc92 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -1457,7 +1457,9 @@ (rx/of (dch/commit-changes {:origin it :redo-changes (:redo-changes changes) :undo-changes [] - :save-undo? false})))))) + :save-undo? false + ;; Follows a change already made, a rollback included + :ignore-save-lock true})))))) (defn component-changed "Notify that the component with the given id has changed, so it needs to be updated diff --git a/frontend/src/app/main/data/workspace/transforms.cljs b/frontend/src/app/main/data/workspace/transforms.cljs index 33719208a7..3e5f8ced3a 100644 --- a/frontend/src/app/main/data/workspace/transforms.cljs +++ b/frontend/src/app/main/data/workspace/transforms.cljs @@ -35,6 +35,7 @@ [app.main.data.changes :as dch] [app.main.data.event :as ev] [app.main.data.helpers :as dsh] + [app.main.data.persistence.lock :as plk] [app.main.data.workspace.collapse :as dwc] [app.main.data.workspace.modifiers :as dwm] [app.main.data.workspace.selection :as dws] @@ -48,7 +49,6 @@ [app.util.array :as array] [app.util.dom :as dom] [app.util.keyboard :as kbd] - [app.util.mouse :as mse] [beicon.v2.core :as rx] [potok.v2.core :as ptk])) @@ -149,6 +149,16 @@ (effect [_ _ _] (ms/clear-transform-preview!)))) +(defn cancel-transform + "Drops the preview of a cancelled drag, so its shapes stay where they + were and nothing is committed." + [] + (ptk/reify ::cancel-transform + ptk/WatchEvent + (watch [_ _ _] + (rx/of (dwm/clear-local-transform) + (finish-transform))))) + ;; -- Resize -------------------------------------------------------- (defn- shape-has-image-fill? @@ -341,13 +351,14 @@ (rx/empty) (let [initial-position @ms/mouse-position - stopper (mse/drag-stopper stream) - layout (:workspace-layout state) - page-id (:current-page-id state) - focus (:workspace-focus-selected state) - zoom (dm/get-in state [:workspace-local :zoom] 1) - objects (dsh/lookup-page-objects state page-id) - shape-ids (filterv (comp not :blocked (d/getf objects)) ids)] + cancelled? (volatile! false) + stopper (plk/gesture-stopper stream cancelled?) + layout (:workspace-layout state) + page-id (:current-page-id state) + focus (:workspace-focus-selected state) + zoom (dm/get-in state [:workspace-local :zoom] 1) + objects (dsh/lookup-page-objects state page-id) + shape-ids (filterv (comp not :blocked (d/getf objects)) ids)] (if (empty? shape-ids) (rx/empty) @@ -384,6 +395,7 @@ (->> resize-events-stream (rx/take-until stopper) (rx/last) + (rx/filter (fn [_] (not @cancelled?))) (rx/map #(dwm/apply-wasm-modifiers (dwm/create-modif-tree shape-ids %) @@ -416,13 +428,17 @@ (rx/take-until stopper) (rx/mapcat (fn [] modifiers-stream))) - (if (features/active-feature? state "render-wasm/v1") - (rx/of - (finish-transform)) + (rx/defer + #(cond + @cancelled? + (rx/of (cancel-transform)) - (rx/of - (dwm/apply-modifiers) - (finish-transform)))))))))))) + (features/active-feature? state "render-wasm/v1") + (rx/of (finish-transform)) + + :else + (rx/of (dwm/apply-modifiers) + (finish-transform))))))))))))) (defn start-move-line-point "Drags one endpoint of a straight path while keeping the other fixed." @@ -441,7 +457,8 @@ start-point (path.helpers/segment->point (nth content index)) other-point (path.helpers/segment->point (nth content (if (zero? index) 1 0))) - stopper (mse/drag-stopper stream) + cancelled? (volatile! false) + stopper (plk/gesture-stopper stream cancelled?) ;; Shift constrains the endpoint around the fixed point. position-stream @@ -485,8 +502,10 @@ (rx/last) (rx/mapcat (fn [pos] - (rx/of (move-endpoint start-point false) - (move-endpoint pos true)))))) + (if @cancelled? + (rx/of (move-endpoint start-point false)) + (rx/of (move-endpoint start-point false) + (move-endpoint pos true))))))) (rx/of #(assoc-in % [:workspace-local :transform] nil))))))) (defn trigger-bounding-box-cloaking @@ -618,7 +637,8 @@ ptk/WatchEvent (watch [_ state stream] - (let [stopper (mse/drag-stopper stream) + (let [cancelled? (volatile! false) + stopper (plk/gesture-stopper stream cancelled?) group (gsh/shapes->rect shapes) group-center (grc/rect->center group) initial-angle (gpt/angle @ms/mouse-position group-center) @@ -657,10 +677,13 @@ (->> angle-stream (rx/take-until stopper) (rx/last) + (rx/filter (fn [_] (not @cancelled?))) (rx/map #(dwm/apply-wasm-modifiers (rotation-modifiers % shapes group-center) :ignore-snap-pixel true)))) - (rx/of (finish-transform))) + (rx/defer #(if @cancelled? + (rx/of (cancel-transform)) + (rx/of (finish-transform))))) (let [emit-modifiers (fn [angle] (dwm/set-rotation-modifiers angle shapes group-center))] @@ -676,8 +699,10 @@ (rx/take-until stopper) (rx/last) (rx/map emit-modifiers))) - (rx/of (dwm/apply-modifiers) - (finish-transform))))))))) + (rx/defer #(if @cancelled? + (rx/of (cancel-transform)) + (rx/of (dwm/apply-modifiers) + (finish-transform))))))))))) (defn increase-rotation "Rotate shapes a fixed angle, from a keyboard action." @@ -728,7 +753,7 @@ (watch [_ state stream] (let [initial (deref ms/mouse-position) - stopper (mse/drag-stopper stream {:interrupt? false}) + stopper (plk/gesture-stopper stream (volatile! false) {:interrupt? false}) zoom (get-in state [:workspace-local :zoom] 1) ;; We toggle the selection so we don't have to wait for the event @@ -821,10 +846,11 @@ duplicate-move-started? (get-in state [:workspace-local :duplicate-move-started?] false) - stopper (mse/drag-stopper stream) - layout (get state :workspace-layout) - zoom (get-in state [:workspace-local :zoom] 1) - focus (:workspace-focus-selected state) + cancelled? (volatile! false) + stopper (plk/gesture-stopper stream cancelled?) + layout (get state :workspace-layout) + zoom (get-in state [:workspace-local :zoom] 1) + focus (:workspace-focus-selected state) exclude-frames (into #{} @@ -976,15 +1002,17 @@ (rx/mapcat (fn [[[_ target-frame drop-index drop-cell] [modifiers snap-ignore-axis]]] (let [undo-id (js/Symbol)] - (rx/of - (dwu/start-undo-transaction undo-id) - (dwm/apply-wasm-modifiers modifiers - :snap-ignore-axis snap-ignore-axis - :undo-transation? false - :subtree-ids-by-id subtree-ids-by-id) - (move-shapes-to-frame ids target-frame drop-index drop-cell) - (finish-transform) - (dwu/commit-undo-transaction undo-id)))))))) + (if @cancelled? + (rx/of (cancel-transform)) + (rx/of + (dwu/start-undo-transaction undo-id) + (dwm/apply-wasm-modifiers modifiers + :snap-ignore-axis snap-ignore-axis + :undo-transation? false + :subtree-ids-by-id subtree-ids-by-id) + (move-shapes-to-frame ids target-frame drop-index drop-cell) + (finish-transform) + (dwu/commit-undo-transaction undo-id))))))))) (rx/merge (->> modifiers-stream @@ -1015,13 +1043,15 @@ (rx/mapcat (fn [[[_ target-frame drop-index drop-cell] [modifiers snap-ignore-axis]]] (let [undo-id (js/Symbol)] - (rx/of (dwu/start-undo-transaction undo-id) - ;; Apply the exact final modifiers; the preview may drop the last frame. - (dwm/set-modifiers modifiers false false {:snap-ignore-axis snap-ignore-axis}) - (dwm/apply-modifiers {:undo-transation? false}) - (move-shapes-to-frame ids target-frame drop-index drop-cell) - (finish-transform) - (dwu/commit-undo-transaction undo-id))))))))))))))) + (if @cancelled? + (rx/of (cancel-transform)) + (rx/of (dwu/start-undo-transaction undo-id) + ;; Apply the exact final modifiers; the preview may drop the last frame. + (dwm/set-modifiers modifiers false false {:snap-ignore-axis snap-ignore-axis}) + (dwm/apply-modifiers {:undo-transation? false}) + (move-shapes-to-frame ids target-frame drop-index drop-cell) + (finish-transform) + (dwu/commit-undo-transaction undo-id)))))))))))))))) (def valid-directions #{:up :down :right :left}) diff --git a/frontend/src/app/main/data/workspace/undo.cljs b/frontend/src/app/main/data/workspace/undo.cljs index b0bf81fe7e..769f90a929 100644 --- a/frontend/src/app/main/data/workspace/undo.cljs +++ b/frontend/src/app/main/data/workspace/undo.cljs @@ -46,6 +46,7 @@ [app.main.data.changes :as dch] [app.main.data.common :as dcm] [app.main.data.helpers :as dsh] + [app.main.data.persistence.lock :as plk] [beicon.v2.core :as rx] [potok.v2.core :as ptk])) @@ -331,8 +332,11 @@ (let [objects (dsh/lookup-page-objects state) edition (get-in state [:workspace-local :edition]) drawing (get state :workspace-drawing)] - (when-not (and (or (some? edition) (some? (:object drawing))) - (not (ctl/grid-layout? objects edition))) + ;; While saves fail the history stays still: its commit would be + ;; refused after the index moved + (when-not (or (plk/locked? state) + (and (or (some? edition) (some? (:object drawing))) + (not (ctl/grid-layout? objects edition)))) (let [undo (:workspace-undo state) items (:items undo) index (or (:index undo) (dec (count items)))] @@ -374,9 +378,10 @@ edition (get-in state [:workspace-local :edition]) drawing (get state :workspace-drawing)] - ;; Editors handle their own undo's - (when (or (and (nil? edition) (nil? (:object drawing))) - (ctl/grid-layout? objects edition)) + ;; Editors handle their own undo's; nothing is undone while saves fail + (when (and (not (plk/locked? state)) + (or (and (nil? edition) (nil? (:object drawing))) + (ctl/grid-layout? objects edition))) (let [undo (:workspace-undo state) items (:items undo) index (or (:index undo) (dec (count items)))] @@ -416,9 +421,10 @@ edition (get-in state [:workspace-local :edition]) drawing (get state :workspace-drawing)] - ;; Editors handle their own undo's - (when (or (and (nil? edition) (nil? (:object drawing))) - (ctl/grid-layout? objects edition)) + ;; Editors handle their own undo's; nothing is undone while saves fail + (when (and (not (plk/locked? state)) + (or (and (nil? edition) (nil? (:object drawing))) + (ctl/grid-layout? objects edition))) (let [undo (:workspace-undo state) items (:items undo) index (or (:index undo) (dec (count items)))] diff --git a/frontend/src/app/main/ui/flex_controls/common.cljs b/frontend/src/app/main/ui/flex_controls/common.cljs index 72bed0cf0f..b4f1e7ab6f 100644 --- a/frontend/src/app/main/ui/flex_controls/common.cljs +++ b/frontend/src/app/main/ui/flex_controls/common.cljs @@ -1,7 +1,10 @@ (ns app.main.ui.flex-controls.common (:require [app.main.constants :as mconst] + [app.main.data.workspace.transforms :as dwt] + [app.main.store :as st] [app.main.streams :as ms] + [app.main.ui.context :as ctx] [app.main.ui.formats :as fmt] [rumext.v2 :as mf])) @@ -14,6 +17,19 @@ ;; `ms/clear-transform-preview!` for the rationale. (def clear-transform-preview! ms/clear-transform-preview!) +(defn use-read-only-cancel + "Answers whether the workspace is read-only, and cancels the resize in + progress (`resizing` holds a truthy value while dragging) when it turns + read-only, as when saves fail: the preview is dropped and the release + commits nothing." + [resizing] + (let [read-only? (mf/use-ctx ctx/workspace-read-only?)] + (mf/with-effect [read-only?] + (when (and read-only? @resizing) + (reset! resizing nil) + (st/emit! (dwt/cancel-transform)))) + read-only?)) + ;; ------------------------------------------------ ;; CONSTANTS ;; ------------------------------------------------ diff --git a/frontend/src/app/main/ui/flex_controls/gap.cljs b/frontend/src/app/main/ui/flex_controls/gap.cljs index f7d409a336..bb00f37586 100644 --- a/frontend/src/app/main/ui/flex_controls/gap.cljs +++ b/frontend/src/app/main/ui/flex_controls/gap.cljs @@ -38,15 +38,17 @@ last-pos (mf/use-var nil) negate? (:resize-negate? rect-data) axis (:resize-axis rect-data) + read-only? (fcc/use-read-only-cancel resizing) on-pointer-down (mf/use-fn - (mf/deps frame-id gap-type gap) + (mf/deps frame-id gap-type gap read-only?) (fn [event] - (dom/capture-pointer event) - (reset! resizing gap-type) - (reset! start (dom/get-client-position event)) - (reset! original-value (:initial-value rect-data)))) + (when-not read-only? + (dom/capture-pointer event) + (reset! resizing gap-type) + (reset! start (dom/get-client-position event)) + (reset! original-value (:initial-value rect-data))))) calc-modifiers (mf/use-fn @@ -76,11 +78,14 @@ (st/emit! (dwm/apply-wasm-modifiers modifiers) (dwt/finish-transform)))) + ;; Nothing to apply for a resize the lock cancelled + (when (and (not (features/active-feature? @st/state "render-wasm/v1")) + (= @resizing gap-type)) + (st/emit! (dwm/apply-modifiers))) + (reset! resizing nil) (reset! start nil) - (reset! original-value 0) - (when (not (features/active-feature? @st/state "render-wasm/v1")) - (st/emit! (dwm/apply-modifiers))))) + (reset! original-value 0))) on-pointer-move (mf/use-fn diff --git a/frontend/src/app/main/ui/flex_controls/margin.cljs b/frontend/src/app/main/ui/flex_controls/margin.cljs index 8638776132..5b3a4aba81 100644 --- a/frontend/src/app/main/ui/flex_controls/margin.cljs +++ b/frontend/src/app/main/ui/flex_controls/margin.cljs @@ -31,15 +31,17 @@ last-pos (mf/use-var nil) negate? (true? (:resize-negate? rect-data)) axis (:resize-axis rect-data) + read-only? (fcc/use-read-only-cancel resizing?) on-pointer-down (mf/use-fn - (mf/deps shape-id margin-num margin) + (mf/deps shape-id margin-num margin read-only?) (fn [event] - (dom/capture-pointer event) - (reset! resizing? true) - (reset! start (dom/get-client-position event)) - (reset! original-value (:initial-value rect-data)))) + (when-not read-only? + (dom/capture-pointer event) + (reset! resizing? true) + (reset! start (dom/get-client-position event)) + (reset! original-value (:initial-value rect-data))))) calc-modifiers (mf/use-fn @@ -76,17 +78,19 @@ (fn [event] (dom/release-pointer event) - (when (features/active-feature? @st/state "render-wasm/v1") - (let [[_ modifiers] (calc-modifiers @last-pos)] - (st/emit! (dwm/apply-wasm-modifiers modifiers) - (dwt/finish-transform)))) + ;; Nothing to apply for a resize the lock cancelled + (when @resizing? + (when (features/active-feature? @st/state "render-wasm/v1") + (let [[_ modifiers] (calc-modifiers @last-pos)] + (st/emit! (dwm/apply-wasm-modifiers modifiers) + (dwt/finish-transform)))) + + (when (not (features/active-feature? @st/state "render-wasm/v1")) + (st/emit! (dwm/apply-modifiers)))) (reset! resizing? false) (reset! start nil) - (reset! original-value 0) - - (when (not (features/active-feature? @st/state "render-wasm/v1")) - (st/emit! (dwm/apply-modifiers))))) + (reset! original-value 0))) on-pointer-move (mf/use-fn diff --git a/frontend/src/app/main/ui/flex_controls/padding.cljs b/frontend/src/app/main/ui/flex_controls/padding.cljs index eb4a9680ef..2f898f91b7 100644 --- a/frontend/src/app/main/ui/flex_controls/padding.cljs +++ b/frontend/src/app/main/ui/flex_controls/padding.cljs @@ -31,15 +31,17 @@ last-pos (mf/use-var nil) negate? (true? (:resize-negate? rect-data)) axis (:resize-axis rect-data) + read-only? (fcc/use-read-only-cancel resizing?) on-pointer-down (mf/use-fn - (mf/deps frame-id rect-data padding-num) + (mf/deps frame-id rect-data padding-num read-only?) (fn [event] - (dom/capture-pointer event) - (reset! resizing? true) - (reset! start (dom/get-client-position event)) - (reset! original-value (:initial-value rect-data)))) + (when-not read-only? + (dom/capture-pointer event) + (reset! resizing? true) + (reset! start (dom/get-client-position event)) + (reset! original-value (:initial-value rect-data))))) calc-modifiers (mf/use-fn @@ -76,17 +78,19 @@ (fn [event] (dom/release-pointer event) - (when (features/active-feature? @st/state "render-wasm/v1") - (let [[_ modifiers] (calc-modifiers @last-pos)] - (st/emit! (dwm/apply-wasm-modifiers modifiers) - (dwt/finish-transform)))) + ;; Nothing to apply for a resize the lock cancelled + (when @resizing? + (when (features/active-feature? @st/state "render-wasm/v1") + (let [[_ modifiers] (calc-modifiers @last-pos)] + (st/emit! (dwm/apply-wasm-modifiers modifiers) + (dwt/finish-transform)))) + + (when (not (features/active-feature? @st/state "render-wasm/v1")) + (st/emit! (dwm/apply-modifiers)))) (reset! resizing? false) (reset! start nil) - (reset! original-value 0) - - (when (not (features/active-feature? @st/state "render-wasm/v1")) - (st/emit! (dwm/apply-modifiers))))) + (reset! original-value 0))) on-pointer-move (mf/use-fn diff --git a/frontend/src/app/main/ui/workspace/shapes/text/v2_editor.cljs b/frontend/src/app/main/ui/workspace/shapes/text/v2_editor.cljs index 3f918f8af2..04c7d0c83d 100644 --- a/frontend/src/app/main/ui/workspace/shapes/text/v2_editor.cljs +++ b/frontend/src/app/main/ui/workspace/shapes/text/v2_editor.cljs @@ -19,6 +19,7 @@ [app.config :as cf] [app.main.data.helpers :as dsh] [app.main.data.persistence :as-alias dps] + [app.main.data.persistence.lock :as plk] [app.main.data.workspace :as dw] [app.main.data.workspace.texts :as dwt] [app.main.features :as features] @@ -189,10 +190,12 @@ ;; A rollback that reverts the text or its styles puts the reverted ;; content in the editor, which would otherwise keep showing and - ;; saving the rejected one + ;; saving the rejected one. Saves failing do the same with the last + ;; committed content, since the typing not yet committed is dropped. rolled-back-sub (->> st/stream - (rx/filter (ptk/type? ::dps/rolled-back)) + (rx/filter #(or (ptk/type? ::dps/rolled-back %) + (plk/cancel-gestures? %))) (rx/subs! (fn [_] (let [content (-> (dsh/lookup-page-objects @st/state) (get shape-id) diff --git a/frontend/src/app/main/ui/workspace/sidebar/options.cljs b/frontend/src/app/main/ui/workspace/sidebar/options.cljs index b753def734..fffd6aa97b 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options.cljs @@ -262,6 +262,10 @@ (mf/use-ctx ctx/permissions) render-context-lost? (mf/deref refs/render-context-lost?) + ;; While saves fail the workspace is locked and nothing may be edited + save-locked? + (some? (mf/deref refs/persistence-lock)) + options-mode (mf/deref refs/options-mode-global) @@ -289,7 +293,7 @@ (on-option-tab-change "design"))))) [:div {:class (stl/css :tool-window)} - (if (and (:can-edit permissions) (not render-context-lost?)) + (if (and (:can-edit permissions) (not render-context-lost?) (not save-locked?)) [:> tab-switcher* {:tabs options-tabs :on-change on-option-tab-change :selected (name options-mode) diff --git a/frontend/src/app/main/ui/workspace/viewport/grid_layout_editor.cljs b/frontend/src/app/main/ui/workspace/viewport/grid_layout_editor.cljs index 2f92b9e19e..c92f565ea3 100644 --- a/frontend/src/app/main/ui/workspace/viewport/grid_layout_editor.cljs +++ b/frontend/src/app/main/ui/workspace/viewport/grid_layout_editor.cljs @@ -180,6 +180,12 @@ (when on-drag-delta (on-drag-delta event (gpt/to-vec start pos))) (when on-drag-position (on-drag-position event pt))))))] + ;; Unmounted mid-drag, as when saves fail and edition ends, the drag + ;; never gets its release: its preview is dropped and nothing is committed + (mf/with-effect [] + #(when (mf/ref-val dragging-ref) + (st/emit! (dwt/cancel-transform)))) + {:handle-pointer-down handle-pointer-down :handle-lost-pointer-capture handle-lost-pointer-capture :handle-pointer-move handle-pointer-move})) diff --git a/frontend/src/app/main/ui/workspace/viewport/guides.cljs b/frontend/src/app/main/ui/workspace/viewport/guides.cljs index 69e0df5a31..47dae84486 100644 --- a/frontend/src/app/main/ui/workspace/viewport/guides.cljs +++ b/frontend/src/app/main/ui/workspace/viewport/guides.cljs @@ -184,6 +184,16 @@ :new-position new-position :new-frame-id new-frame-id))))))] + ;; A drag in progress when the workspace turns read-only (saves fail) + ;; is dropped, so the guide goes back and its release changes nothing + (mf/with-effect [read-only?] + (when read-only? + (mf/set-ref-val! dragging-ref false) + (mf/set-ref-val! start-ref nil) + (mf/set-ref-val! start-pos-ref nil) + (when (some? (:new-position @state)) + (swap! state assoc :new-position nil)))) + {:on-pointer-enter on-pointer-enter :on-pointer-leave on-pointer-leave :on-pointer-down on-pointer-down diff --git a/frontend/test/frontend_tests/data/persistence_lock_test.cljs b/frontend/test/frontend_tests/data/persistence_lock_test.cljs new file mode 100644 index 0000000000..41ec6d38f9 --- /dev/null +++ b/frontend/test/frontend_tests/data/persistence_lock_test.cljs @@ -0,0 +1,209 @@ +;; 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 SUBSIDIARY SL + +(ns frontend-tests.data.persistence-lock-test + "Tests for the commits made while saves fail and the workspace is + locked (`app.main.data.persistence.lock`): user edits are refused, the + undo of rolled-back commits and derived data still commit." + (:require + [app.common.test-helpers.components :as cthc] + [app.common.test-helpers.compositions :as ctho] + [app.common.test-helpers.files :as cthf] + [app.common.test-helpers.ids-map :as cthi] + [app.common.test-helpers.shapes :as cths] + [app.common.uuid :as uuid] + [app.main.data.changes :as dch] + [app.main.data.persistence :as dps] + [app.main.data.persistence.lock :as plk] + [app.main.data.workspace :as dw] + [app.main.data.workspace.libraries :as dwl] + [app.main.data.workspace.undo :as dwu] + [app.main.repo :as rp] + [app.render-wasm.api :as wasm.api] + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.async :as async] + [frontend-tests.helpers.mock :as mock] + [frontend-tests.helpers.persistence :as thp] + [frontend-tests.helpers.state :as ths] + [potok.v2.core :as ptk])) + +(t/use-fixtures :each {:before cthi/reset-idmap!}) + +(defn- server-error + "A save answer that fails for good: an unexpected server error." + [] + (rp/handle-response {:status 500 + :body {:type :server-error :code :unhandled} + :uri "https://penpot.example.com/api/main/methods/update-file"})) + +(defn- answers + "A transport that gives each save, in order, the next of `responses` + (functions returning the answer's observable)." + [& responses] + (let [pending (atom responses)] + (fn [_ _] + (let [respond (first @pending)] + (swap! pending rest) + (respond))))) + +(defn- saved + [revn] + #(rx/of {:revn revn})) + +(def ^:private lock + "The lock a failed save leaves, set by hand." + #(assoc-in % [:workspace-global :persistence-lock] {:read-only? false})) + +(defn- record-commits + "Records, for each commit on `store`, whether the workspace was locked + when it was made. Answers the subscription and the atom." + [store] + (let [made (atom []) + sub (->> (ptk/input-stream store) + (rx/filter dch/commit?) + (rx/subs! (fn [_] (swap! made conj (plk/locked? @store)))))] + [sub made])) + +;; Scenario: a save fails for good and the workspace locks; the user then +;; renames the shape through some UI the lock did not hide. Proves: the +;; edit makes no commit, so nothing queues behind the failed save and the +;; shape keeps its name. +(t/deftest ^:async an-edit-while-locked-makes-no-commit + (await + (thp/with-persistence + (^:async fn [{:keys [file-id store]}] + (let [page-id (uuid/next) + shape-id (uuid/next) + [sub made] (record-commits store)] + (ptk/emit! store + (thp/open-file file-id page-id (thp/file-data page-id shape-id "B")) + (thp/rename-commit file-id page-id shape-id "A" "B") + ::dps/force-persist) + (await (async/wait-for #(plk/locked? @store) "the failed save locks the workspace")) + (reset! made []) + (ptk/emit! store + (dch/commit-changes {:redo-changes [(thp/rename page-id shape-id "C")] + :undo-changes [(thp/rename page-id shape-id "B")]}) + ::dps/force-persist) + (await (async/settle)) + (rx/dispose! sub) + (t/is (empty? @made) "no commit is made") + (t/is (= "B" (thp/shape-name store file-id page-id shape-id))) + (t/is (empty? (get-in @store [:persistence :buffer]))) + (t/is (= 1 (count (get-in @store [:persistence :queue]))) + "the queue holds only the failed edit"))) + (answers server-error)))) + +;; Scenario: the user presses Ctrl+Z while the workspace is locked. +;; Proves: the history stays where it was, since its commit would be +;; refused after the index moved. +(t/deftest ^:async undo-and-redo-do-nothing-while-locked + (let [file (-> (cthf/sample-file :file1) + (ctho/add-rect :rect1 :name "A")) + store (ths/setup-store file {:renderer :svg}) + page-id (cthf/current-page-id file) + rect-id (cthi/id :rect1) + rename (fn [from to] + (dch/commit-changes {:redo-changes [(thp/rename page-id rect-id to)] + :undo-changes [(thp/rename page-id rect-id from)]})) + group (uuid/next) + entry {:undo-changes [(thp/rename page-id rect-id "A")] + :redo-changes [(thp/rename page-id rect-id "B")] + :undo-group group + :undo-groups #{group} + :tags #{}} + name-of #(:name (cths/get-shape (ths/get-file-from-state @store) :rect1))] + (ptk/emit! store (rename "A" "B") #(assoc % :workspace-undo {:items [entry] :index 0})) + (await (async/wait-for #(= "B" (name-of)) "the shape is renamed")) + (ptk/emit! store lock dwu/undo) + (await (async/settle)) + (t/is (= "B" (name-of)) "undo changes nothing") + (t/is (= 0 (get-in @store [:workspace-undo :index]))) + (ptk/emit! store #(update % :workspace-global dissoc :persistence-lock) dwu/undo) + (await (async/wait-for #(= "A" (name-of)) "undo works once unlocked")) + (ptk/emit! store lock dwu/redo) + (await (async/settle)) + (t/is (= "A" (name-of)) "redo changes nothing") + (t/is (= -1 (get-in @store [:workspace-undo :index]))))) + +;; Scenario: one action renames a shape in a library and in this file; +;; the library's save lands, this file's fails for good and the workspace +;; locks. Edit rights come back with a role change, which releases the +;; failed edit while the lock still holds. Proves: the undo of the +;; library's saved rename still commits and is saved, so the library goes +;; back with the released edit. +(t/deftest ^:async a-release-undoes-saved-commits-while-locked + (await + (thp/with-persistence + (^:async fn [{:keys [file-id store requests]}] + (let [lib-id (uuid/next) + page-id (uuid/next) + shape-id (uuid/next) + lshape-id (uuid/next) + group (uuid/next) + linked (ptk/data-event ::dch/commit + (assoc @(thp/rename-commit lib-id page-id lshape-id "X" "Y") + :undo-group group)) + commit (ptk/data-event ::dch/commit + (assoc @(thp/rename-commit file-id page-id shape-id "A" "B") + :undo-group group)) + [sub made] (record-commits store)] + (ptk/emit! store (thp/open-file-with-library file-id lib-id page-id + (thp/file-data page-id shape-id "B") + (thp/file-data page-id lshape-id "Y"))) + (ptk/emit! store linked commit ::dps/force-persist) + (await (async/wait-for #(plk/locked? @store) "this file's failed save locks the workspace")) + (reset! made []) + (ptk/emit! store (ptk/data-event :app.main.data.common/change-team-role)) + (await (async/wait-for #(seq @made) "the library's rename is undone")) + (t/is (= [true] @made) "the undo commits while the lock holds") + (t/is (= "X" (thp/shape-name store lib-id page-id lshape-id))) + (t/is (= "A" (thp/shape-name store file-id page-id shape-id))) + (ptk/emit! store ::dps/force-persist) + (await (async/wait-for #(= :saved (get-in @store [:persistence :status])) + "the undo is saved")) + (rx/dispose! sub) + (t/is (= 3 (count @requests))) + (t/is (= lib-id (get-in (nth @requests 2) [1 :id]))) + (t/is (not (plk/locked? @store))))) + (answers (saved 1) server-error (saved 2))))) + +;; Scenario: texts without layout data are measured while the workspace is +;; locked, as on a reload during the lock. Proves: the derived layout still +;; commits, so the text and its layout data stay in step. +(t/deftest ^:async text-layout-data-commits-while-locked + (let [file (-> (cthf/sample-file :file1) + (ctho/add-text :text1 "hello")) + store (ths/setup-store file {:renderer :svg}) + position-data [{:x 0 :y 0 :width 80 :height 16 :fills [] :text "hello"}] + [sub made] (record-commits store)] + (await + (mock/with-mocks* + {wasm.api/calculate-position-data (mock/stub (fn [_] position-data))} + (ptk/emit! store lock (dw/update-page-position-data)) + (await (async/wait-for #(seq @made) "the layout data is committed")))) + (rx/dispose! sub) + (t/is (= [true] @made)) + (t/is (= position-data + (:position-data (cths/get-shape (ths/get-file-from-state @store) :text1)))))) + +;; Scenario: a main component changes while the workspace is locked, by the +;; undo of a rolled-back commit. Proves: its modification time still +;; commits, so other files see the component changed again. +(t/deftest ^:async a-component-touch-commits-while-locked + (let [file (-> (cthf/sample-file :file1) + (ctho/add-simple-component :component1 :main-root :main-child)) + comp-id (cthi/id :component1) + ;; No modification time yet, so the touch shows whatever the clock + file (update-in file [:data :components comp-id] dissoc :modified-at) + store (ths/setup-store file {:renderer :svg}) + [sub made] (record-commits store)] + (ptk/emit! store lock (dwl/touch-component comp-id)) + (await (async/wait-for #(seq @made) "the touch is committed")) + (rx/dispose! sub) + (t/is (= [true] @made)) + (t/is (some? (:modified-at (cthc/get-component (ths/get-file-from-state @store) :component1)))))) diff --git a/frontend/test/frontend_tests/logic/drag_on_save_failure_test.cljs b/frontend/test/frontend_tests/logic/drag_on_save_failure_test.cljs new file mode 100644 index 0000000000..7ccf1c9d9c --- /dev/null +++ b/frontend/test/frontend_tests/logic/drag_on_save_failure_test.cljs @@ -0,0 +1,214 @@ +;; 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 SUBSIDIARY SL + +(ns frontend-tests.logic.drag-on-save-failure-test + "Tests for drags and drawings in progress when saves start failing and + the workspace locks." + (:require + [app.common.data :as d] + [app.common.geom.point :as gpt] + [app.common.geom.rect :as grc] + [app.common.test-helpers.compositions :as ctho] + [app.common.test-helpers.files :as cthf] + [app.common.test-helpers.ids-map :as cthi] + [app.common.test-helpers.shapes :as cths] + [app.common.types.path :as path] + [app.main.data.changes :as dch] + [app.main.data.persistence.lock :as plk] + [app.main.data.workspace.drawing :as dwd] + [app.main.data.workspace.drawing.curve :as-alias dwdc] + [app.main.data.workspace.transforms :as dwt] + [app.main.snap :as snap] + [app.main.streams :as ms] + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.async :as async] + [frontend-tests.helpers.mock :as mock] + [frontend-tests.helpers.state :as ths] + [potok.v2.core :as ptk])) + +(t/use-fixtures :each {:before cthi/reset-idmap!}) + +(defn- shape-rect + [store] + (-> (ths/get-file-from-state @store) + (cths/get-shape :rect1) + :points + (grc/points->rect))) + +(defn- ^:async drag + "Drags rect1 with the gesture `start` makes, from (120, 220) to (160, 260), + then emits `stop`. Resolves to the commits made and the store." + [start stop] + (let [file (-> (cthf/sample-file :file1) + (ctho/add-rect :rect1 :x 100 :y 200 :width 50 :height 50)) + rect (cths/get-shape file :rect1) + store (ths/setup-store file {:renderer :svg}) + commits (atom 0) + sub (->> (ptk/input-stream store) + (rx/filter dch/commit?) + (rx/subs! (fn [_] (swap! commits inc))))] + (await + (mock/with-mocks* + ;; Called with seven arguments, more than a stub forwards + {snap/closest-snap-move (fn [& _] (rx/of (gpt/point 0 0))) + snap/closest-snap-point (fn [& _] (rx/of (gpt/point 0 0))) + ;; Moves a DOM node the tests lack + dwt/set-ghost-displacement (mock/stub (fn [_] (ptk/data-event ::ghost-moved)))} + (try + (rx/push! ms/mouse-position (gpt/point 120 220)) + (ptk/emit! store + #(assoc-in % [:workspace-local :selected] (d/ordered-set (:id rect))) + (start rect)) + (rx/push! ms/mouse-position (gpt/point 160 260)) + (await (async/wait-for #(seq (:workspace-modifiers @store)) "the drag shows a preview")) + (ptk/emit! store stop) + (await (async/settle)) + (finally + (rx/push! ms/mouse-position nil) + (rx/dispose! sub))))) + {:commits @commits :store store})) + +(def ^:private save-failure + "Saves fail for good: the workspace locks." + (ptk/reify ::save-failure + ptk/UpdateEvent + (update [_ state] + (assoc state :persistence {:status :error})) + + ptk/WatchEvent + (watch [_ _ _] + (rx/of (plk/sync-failure-lock))))) + +(defn- move + [rect] + (dwt/start-move (gpt/point 120 220) [(:id rect)])) + +(defn- rotate + [rect] + (dwt/start-rotate [rect])) + +;; Scenario: the user drags a shape and stops it with Escape. Proves: the +;; drag commits where it stopped (the case the lock must not follow). +(t/deftest ^:async an-interrupted-move-commits-its-position + (let [{:keys [commits store]} (await (drag move :interrupt))] + (t/is (pos? commits)) + (t/is (= 140 (:x (shape-rect store)))))) + +;; Scenario: saves fail while the user drags a shape, or rotates it. Proves: +;; the lock cancels the gesture, the shape goes back to where it was and +;; nothing is committed behind the failed save. +(defn- check-cancelled + [{:keys [commits store]}] + (t/is (some? (get-in @store [:workspace-global :persistence-lock])) "the workspace is locked") + (t/is (= 0 commits) "nothing is committed") + (t/is (empty? (:workspace-modifiers @store)) "the preview is dropped") + (t/is (nil? (get-in @store [:workspace-local :transform]))) + (t/is (= (grc/make-rect 100 200 50 50) (shape-rect store)) + "the shape stays where it was")) + +(t/deftest ^:async a-save-failure-cancels-a-move-in-progress + (check-cancelled (await (drag move save-failure)))) + +(t/deftest ^:async a-save-failure-cancels-a-rotation-in-progress + (check-cancelled (await (drag rotate save-failure)))) + +;; -- Drawing --------------------------------------------------------- + +(defn- drawn-shapes + "Shapes of the current page other than the root." + [store] + (let [page-id (:current-page-id @store) + objects (get-in (ths/get-file-from-state @store) [:data :pages-index page-id :objects])] + (remove #(= (:id %) (:parent-id %)) (vals objects)))) + +(defn- ^:async draw + "Starts drawing with `tool` on an empty page and moves the pointer with + `move` (gets the store, resolves once the drawing shows), then emits + `stop` and waits for the drawing to end. Resolves to the commits made + and the store." + [tool move stop] + (let [file (cthf/sample-file :file1) + store (ths/setup-store file {:renderer :svg}) + commits (atom 0) + sub (->> (ptk/input-stream store) + (rx/filter dch/commit?) + (rx/subs! (fn [_] (swap! commits inc))))] + (await + (mock/with-mocks* + ;; Called with seven arguments, more than a stub forwards + {snap/closest-snap-point (fn [& _] (rx/of (gpt/point 0 0)))} + (try + (rx/push! ms/mouse-position (gpt/point 100 100)) + (ptk/emit! store (dwd/handle-drawing tool)) + (await (move store)) + (ptk/emit! store stop) + ;; A box drawing ends 100 ms after its stop + (await (async/wait-for #(nil? (get-in @store [:workspace-drawing :object])) + "the drawing ends" {:max-ticks 500})) + (await (async/settle)) + (finally + (rx/push! ms/mouse-position nil) + (rx/dispose! sub))))) + {:commits @commits :store store})) + +(defn- ^:async drag-box + [store] + (rx/push! ms/mouse-position (gpt/point 160 180)) + (await (async/wait-for #(< 50 (get-in @store [:workspace-drawing :object :width] 0)) + "the box follows the pointer"))) + +(defn- ^:async drag-curve + [store] + (doseq [x [110 130 150 170]] + (rx/push! ms/mouse-position (gpt/point x (+ x 20))) + (await (async/wait-for #(some (fn [p] (= x (:x p))) + (get-in @store [:workspace-drawing :object ::dwdc/points])) + "the curve takes the point")))) + +(defn- ^:async click-path + "Stands for the pen clicks: the drawn path gets two nodes." + [store] + (await (async/wait-for #(some? (get-in @store [:workspace-drawing :object])) "the pen starts")) + (ptk/emit! store #(assoc-in % [:workspace-drawing :object :content] + (path/content [{:command :move-to :params {:x 100 :y 100}} + {:command :line-to :params {:x 180 :y 160}}])))) + +(defn- check-drawn + [{:keys [commits store]}] + (t/is (pos? commits)) + (t/is (= 1 (count (drawn-shapes store))) "the shape is added")) + +(defn- check-not-drawn + [{:keys [commits store]}] + (t/is (plk/locked? @store) "the workspace is locked") + (t/is (= 0 commits) "nothing is committed") + (t/is (empty? (drawn-shapes store)) "no shape is added") + (t/is (nil? (get-in @store [:workspace-drawing :object])) "no drawing is left on screen")) + +;; Scenario: the user draws a rectangle, a curve or a path and stops it +;; with Escape. Proves: the shape is added (the case the lock must not +;; follow). +(t/deftest ^:async an-interrupted-rectangle-is-added + (check-drawn (await (draw :rect drag-box :interrupt)))) + +(t/deftest ^:async an-interrupted-curve-is-added + (check-drawn (await (draw :curve drag-curve :interrupt)))) + +(t/deftest ^:async an-interrupted-path-is-added + (check-drawn (await (draw :path click-path :interrupt)))) + +;; Scenario: saves fail while the user draws a rectangle, a curve or a +;; path. Proves: the lock drops the drawing: no shape is added, nothing is +;; committed behind the failed save and no half-drawn shape stays. +(t/deftest ^:async a-save-failure-drops-a-rectangle-in-progress + (check-not-drawn (await (draw :rect drag-box save-failure)))) + +(t/deftest ^:async a-save-failure-drops-a-curve-in-progress + (check-not-drawn (await (draw :curve drag-curve save-failure)))) + +(t/deftest ^:async a-save-failure-drops-a-path-in-progress + (check-not-drawn (await (draw :path click-path save-failure)))) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index c9192a7d31..e7f4ae6778 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -20,6 +20,7 @@ [frontend-tests.data.modal-test] [frontend-tests.data.nitrate-test] [frontend-tests.data.persistence-commits-test] + [frontend-tests.data.persistence-lock-test] [frontend-tests.data.persistence-retry-test] [frontend-tests.data.persistence-rollback-test] [frontend-tests.data.persistence-test] @@ -59,6 +60,7 @@ [frontend-tests.logic.components-and-tokens] [frontend-tests.logic.copy-paste-typography-test] [frontend-tests.logic.copying-and-duplicating-test] + [frontend-tests.logic.drag-on-save-failure-test] [frontend-tests.logic.frame-guides-test] [frontend-tests.logic.groups-test] [frontend-tests.logic.ignore-tree-test] @@ -204,6 +206,7 @@ 'frontend-tests.data.modal-test 'frontend-tests.data.nitrate-test 'frontend-tests.data.persistence-commits-test + 'frontend-tests.data.persistence-lock-test 'frontend-tests.data.persistence-retry-test 'frontend-tests.data.persistence-rollback-test 'frontend-tests.data.persistence-test @@ -249,6 +252,7 @@ 'frontend-tests.logic.components-and-tokens 'frontend-tests.logic.copy-paste-typography-test 'frontend-tests.logic.copying-and-duplicating-test + 'frontend-tests.logic.drag-on-save-failure-test 'frontend-tests.logic.frame-guides-test 'frontend-tests.logic.groups-test 'frontend-tests.logic.ignore-tree-test