mirror of
https://github.com/penpot/penpot.git
synced 2026-11-05 17:40:39 +00:00
✨ Make plugin writes fail while the workspace is read-only
A read-only workspace (saves failing, a user who cannot edit, a version preview) only hid the editing tools: plugins, the MCP among them, still changed shapes, and those edits were never saved. Every plugin write now throws a read-only error, whatever the throwValidationErrors flag, the same way a write to a removed shape does. `check-permission` with a write permission runs the new `check-writable`, which covers most writers. Writers without a permission check (most context methods such as create*, group, align or flatten, the z-order methods, applyTypography on a text range, removing a ruler guide) call it themselves, and shape setters get it through `check-shape-writable`. Reads, comments and undo blocks keep working, so a block opened before a lock still closes. The MCP instructions tell the agent what the read-only error means.
This commit is contained in:
parent
37a20435d2
commit
d47c029934
@ -8,6 +8,7 @@ Behaviors confirmed against the language/stdlib — do not re-derive from assump
|
||||
- The analyzer reads `:async` only from the fn name meta and the `fn` operator meta; list-level meta is ignored (would make a MetaFn). `(fn ^:async [] …)` puts the meta on argv (pre/post only, NOT async).
|
||||
- Valid: `(defn ^:async f)`, `(defn- ^:async f)`, `(^:async fn [] …)` (the latter is what stock `t/async` generates itself).
|
||||
- `^:async` fns never throw synchronously (rejected promises instead); `try/catch/finally` supported; continuations are microtasks, timers and RxJS schedulers are macrotasks (FIFO).
|
||||
- Inside an `^:async` fn, a nested call that needs a temp binding (e.g. calling a local fn `(g (f x))`, whose arg gets a `G__` binding) compiles to `(await (async function(){…})())`: `await` unwraps a returned promise, so `(settled (upload (make-ctx)))` blocks until the upload promise settles instead of passing the promise on (a test that pushes the answer later hangs, idle). Bind each step in its own `let` binding.
|
||||
- `!` suffix allowed cases (only exceptions to the no-new-`!` rule in `mem:clojure/design-rules`): mutating public `app.db` API (`insert!`, `update!`, `delete!`, `run!`, `tx-run!`, `exec!`, ...) and fns whose contract is to work over transients (stdlib `conj!`, `assoc!`, `dissoc!`, `disj!`, `persistent!` and custom helpers over them); any further case must be registered here with justification before use.
|
||||
- Penpot extends `clojure.core/Inst` in `app.common.time` ( JVM-only section), so `inst-ms` also accepts `Duration` (returns `.toMillis`, NOT epoch millis), `FileTime` and `OffsetDateTime`. It only works where that namespace is loaded; on a bare `Duration`, prefer explicit `.toMillis`, which reads as what it is (an amount, not an instant).
|
||||
- `cuerdas.core/index-of` returns `nil` (not `-1`) when the substring is absent, like `clojure.string/index-of`. `(pos? nil)` **throws on the JVM** but evaluates to `false` in CLJS (JS coercion), so `(if (pos? (str/index-of s "\n")) …)` works in the browser and crashes the backend. Guard with `some?` before `pos?` (this was `app.common.exceptions/first-line`).
|
||||
|
||||
@ -23,7 +23,9 @@
|
||||
|
||||
- Public Plugin API objects are lightweight handles, not durable snapshots. Most getters locate fresh state from `app.main.store/state` using hidden `$id`, `$file`, `$page`, etc.
|
||||
- `not-valid` logs by default but throws when the plugin flag `throwValidationErrors` is enabled. The MCP execute-code handler deliberately enables that flag while running code.
|
||||
- A write to a removed shape always throws `Shape not found`, whatever that flag: setters through the shape proxy's `:on-set u/check-shape-exists` (runs outside the error handling), and methods that must not vanish silently (`resize`, `rotate`) call `u/check-shape-located` first. Methods run inside `:on-error (u/handle-error ...)`, which logs instead of throwing when the flag is off, so it rethrows errors whose `ex-data :type` is `::u/shape-not-found`.
|
||||
- A write to a removed shape always throws `Shape not found`, whatever that flag: setters through the shape proxy's `:on-set u/check-shape-writable` (runs outside the error handling), and methods that must not vanish silently (`resize`, `rotate`) call `u/check-shape-located` first. Methods run inside `:on-error (u/handle-error ...)`, which logs instead of throwing when the flag is off, so it rethrows errors whose `ex-data :type` is `::u/shape-not-found` or `::r/read-only`.
|
||||
- A write in a read-only workspace (`[:workspace-global :read-only?]`: save-failure lock, user without edit rights, version preview) always throws, whatever that flag. `r/check-permission` with `content:write`/`library:write` calls `r/check-writable` first, which covers most writers; writers without a permission check (most top-level context methods in `api.cljs`: `create*`, `group`, `align*`, …; z-order methods; text range `applyTypography`; ruler guide `remove`) call `r/check-writable` themselves, and shape setters get it through `u/check-shape-writable`. A new writer without a write-permission check must call it too. `r/has-permission?` is the check without it, used by the undo blocks so a block opened before a lock still closes. Comments (`comment:write`) stay writable: they do not go through file saves. An async writer calls it again after each server await, before the next server write or commit (`rx/tap` on the response stream; `dwm/create-svg-shape-with-images` takes it as `:check-writable`), so a lock that lands mid-way rejects the promise with the read-only error. `r/has-permission?` is the check without it, used by the undo blocks so a block opened before a lock still closes. Comments (`comment:write`) stay writable: they do not go through file saves.
|
||||
- `penpotMgmt` (`app.plugins.management`) checks permissions with `r/has-permission?` (`granted?`), never `r/check-permission`: its calls act on files through the backend, often from the dashboard with no file open, so the workspace read-only check (`check-writable`) must not block them.
|
||||
- `naturalChildOrdering` and `throwValidationErrors` are stored per plugin under `[:plugins :flags plugin-id ...]`; changing default behavior affects automation and MCP diagnostics.
|
||||
- Plugin data is stored under keyword namespaces: private data uses `(keyword "plugin" plugin-id)`, shared data uses `(keyword "shared" namespace)`.
|
||||
|
||||
|
||||
@ -39,6 +39,7 @@ To assert over emitted event sequences, observe termination: subscribe through `
|
||||
- Auto-answering mocks lose deadline expressiveness (can't time answers), need teardown timer-cancellation, and post-teardown deliveries hit real implementations — explicit pushes + async delivery + `wait-for` won on every axis.
|
||||
- Teardown belongs to the terminal continuation, never to `finally`-around-triggers (it would dispose in-flight flows).
|
||||
- A body that awaits must be `(^:async fn …)` even if the rest is sync; sync sequences are atomic vs the event loop.
|
||||
- The `:esm` test build cannot load the CommonJS `sax` module behind `tubax`, so SVG parsing fails in tests (`dwm/valid-svg-string?` answers false, `dwm/svg->clj` errors): mock both with the parsed data.
|
||||
|
||||
From `frontend/`:
|
||||
- Full unit test run (always builds, suppressed output): `pnpm run test:quiet`.
|
||||
|
||||
@ -491,7 +491,10 @@
|
||||
:ignore-selection? true
|
||||
:change-selection? false}))))))
|
||||
(defn create-svg-shape-with-images
|
||||
[file-id id name svg-string position on-success on-error]
|
||||
"Uploads the images of `svg-string` and adds it as shape `id`.
|
||||
`check-writable` runs once the images are uploaded, before the shape is
|
||||
added; when it throws, no shape is added and `on-error` gets the error."
|
||||
[file-id id name svg-string position {:keys [check-writable on-success on-error]}]
|
||||
(ptk/reify ::create-svg-shape-with-images
|
||||
ptk/WatchEvent
|
||||
(watch [_ _ _]
|
||||
@ -501,6 +504,7 @@
|
||||
(fn [svg-data]
|
||||
(->> (svg/upload-images svg-data file-id)
|
||||
(rx/map #(assoc svg-data :image-data %)))))
|
||||
(rx/tap (fn [_] (check-writable)))
|
||||
(rx/map
|
||||
(fn [svg-data]
|
||||
(svg/add-svg-shapes
|
||||
|
||||
@ -65,6 +65,7 @@
|
||||
;;
|
||||
(defn create-shape
|
||||
[plugin-id type]
|
||||
(r/check-writable)
|
||||
(let [page (dsh/lookup-page @st/state)
|
||||
shape (cts/setup-shape {:type type
|
||||
:x 0 :y 0
|
||||
@ -205,6 +206,7 @@
|
||||
|
||||
:replaceColor
|
||||
(fn [shapes old-color new-color]
|
||||
(r/check-writable)
|
||||
(let [old-color (parser/parse-color-data old-color)
|
||||
new-color (parser/parse-color-data new-color)]
|
||||
(cond
|
||||
@ -261,6 +263,7 @@
|
||||
|
||||
:uploadMediaUrl
|
||||
(fn [name url]
|
||||
(r/check-writable)
|
||||
(cond
|
||||
(not (string? name))
|
||||
(u/not-valid plugin-id :uploadMedia-name name)
|
||||
@ -274,12 +277,15 @@
|
||||
(fn [resolve reject]
|
||||
(->> (dwm/upload-media-url name file-id url)
|
||||
(rx/take 1)
|
||||
;; Saves may have failed while the upload ran
|
||||
(rx/tap (fn [_] (r/check-writable)))
|
||||
(rx/map format/format-image)
|
||||
(rx/tap #(st/emit! (se/event plugin-id "add-media")))
|
||||
(rx/subs! resolve reject)))))))
|
||||
|
||||
:uploadMediaData
|
||||
(fn [name data mime-type]
|
||||
(r/check-writable)
|
||||
(let [file-id (:current-file-id @st/state)]
|
||||
(js/Promise.
|
||||
(fn [resolve reject]
|
||||
@ -307,12 +313,15 @@
|
||||
:on-image identity
|
||||
:on-svg identity})
|
||||
(rx/take 1)
|
||||
;; Saves may have failed while the upload ran
|
||||
(rx/tap (fn [_] (r/check-writable)))
|
||||
(rx/map format/format-image)
|
||||
(rx/tap #(st/emit! (se/event plugin-id "add-media")))
|
||||
(rx/subs! resolve reject))))))
|
||||
|
||||
:group
|
||||
(fn [shapes]
|
||||
(r/check-writable)
|
||||
(let [valid-shapes? (and (array? shapes) (every? shape/shape-proxy? shapes))
|
||||
file-id (:current-file-id @st/state)
|
||||
page-id (:current-page-id @st/state)
|
||||
@ -343,6 +352,7 @@
|
||||
|
||||
:ungroup
|
||||
(fn [group & rest]
|
||||
(r/check-writable)
|
||||
(let [valid-group? (shape/shape-proxy? group)
|
||||
valid-rest? (every? shape/shape-proxy? rest)
|
||||
shapes (when (and valid-group? valid-rest?) (concat [group] rest))
|
||||
@ -382,6 +392,7 @@
|
||||
|
||||
:createPath
|
||||
(fn []
|
||||
(r/check-writable)
|
||||
(let [page (dsh/lookup-page @st/state)
|
||||
shape (cts/setup-shape
|
||||
{:type :path
|
||||
@ -399,6 +410,7 @@
|
||||
|
||||
:createText
|
||||
(fn [text]
|
||||
(r/check-writable)
|
||||
(cond
|
||||
(not (string? text))
|
||||
(u/not-valid plugin-id :createText text)
|
||||
@ -436,6 +448,7 @@
|
||||
|
||||
:createShapeFromSvg
|
||||
(fn [svg-string]
|
||||
(r/check-writable)
|
||||
(cond
|
||||
(not (dwm/valid-svg-string? svg-string))
|
||||
(u/not-valid plugin-id :createShapeFromSvg svg-string)
|
||||
@ -450,6 +463,7 @@
|
||||
|
||||
:createShapeFromSvgWithImages
|
||||
(fn [svg-string]
|
||||
(r/check-writable)
|
||||
(js/Promise.
|
||||
(fn [resolve reject]
|
||||
(cond
|
||||
@ -464,12 +478,15 @@
|
||||
page-id (:current-page-id @st/state)]
|
||||
(st/emit! (dwm/create-svg-shape-with-images
|
||||
file-id id "svg" svg-string (gpt/point 0 0)
|
||||
#(resolve (shape/shape-proxy plugin-id file-id page-id id))
|
||||
reject)
|
||||
{;; Saves may have failed while the images uploaded
|
||||
:check-writable r/check-writable
|
||||
:on-success #(resolve (shape/shape-proxy plugin-id file-id page-id id))
|
||||
:on-error reject})
|
||||
(se/event plugin-id "create-shape" :type :text)))))))
|
||||
|
||||
:createBoolean
|
||||
(fn [bool-type shapes]
|
||||
(r/check-writable)
|
||||
(let [bool-type (keyword bool-type)]
|
||||
(cond
|
||||
(not (contains? cts/bool-types bool-type))
|
||||
@ -599,6 +616,7 @@
|
||||
|
||||
:createPage
|
||||
(fn []
|
||||
(r/check-writable)
|
||||
(let [file-id (:current-file-id @st/state)
|
||||
id (uuid/next)]
|
||||
(st/emit! (-> (dw/create-page {:page-id id :file-id file-id})
|
||||
@ -636,6 +654,7 @@
|
||||
|
||||
:alignHorizontal
|
||||
(fn [shapes direction]
|
||||
(r/check-writable)
|
||||
(let [dir (case direction
|
||||
"left" :hleft
|
||||
"center" :hcenter
|
||||
@ -654,6 +673,7 @@
|
||||
|
||||
:alignVertical
|
||||
(fn [shapes direction]
|
||||
(r/check-writable)
|
||||
(let [dir (case direction
|
||||
"top" :vtop
|
||||
"center" :vcenter
|
||||
@ -672,6 +692,7 @@
|
||||
|
||||
:distributeHorizontal
|
||||
(fn [shapes]
|
||||
(r/check-writable)
|
||||
(cond
|
||||
(or (not (array? shapes)) (not (every? shape/shape-proxy? shapes)))
|
||||
(u/not-valid plugin-id :distributeHorizontal-shapes "Not valid shapes")
|
||||
@ -682,6 +703,7 @@
|
||||
|
||||
:distributeVertical
|
||||
(fn [shapes]
|
||||
(r/check-writable)
|
||||
(cond
|
||||
(or (not (array? shapes)) (not (every? shape/shape-proxy? shapes)))
|
||||
(u/not-valid plugin-id :distributeVertical-shapes "Not valid shapes")
|
||||
@ -692,6 +714,7 @@
|
||||
|
||||
:flatten
|
||||
(fn [shapes]
|
||||
(r/check-writable)
|
||||
(cond
|
||||
(or (not (array? shapes)) (not (every? shape/shape-proxy? shapes)))
|
||||
(u/not-valid plugin-id :flatten-shapes "Not valid shapes")
|
||||
@ -707,6 +730,7 @@
|
||||
|
||||
:createVariantFromComponents
|
||||
(fn [shapes]
|
||||
(r/check-writable)
|
||||
(cond
|
||||
(or (not (array? shapes))
|
||||
(not (seq shapes))
|
||||
|
||||
@ -20,10 +20,12 @@
|
||||
(obj/reify {:name "HistorySubcontext"}
|
||||
:$plugin {:enumerable false :get (fn [] plugin-id)}
|
||||
|
||||
;; Undo blocks write nothing, so they work in a read-only workspace:
|
||||
;; a block opened before a lock can still be closed
|
||||
:undoBlockBegin
|
||||
(fn []
|
||||
(cond
|
||||
(not (r/check-permission plugin-id "content:write"))
|
||||
(not (r/has-permission? plugin-id "content:write"))
|
||||
(u/not-valid plugin-id :resize "Plugin doesn't have 'content:write' permission")
|
||||
|
||||
:else
|
||||
@ -34,7 +36,7 @@
|
||||
:undoBlockFinish
|
||||
(fn [block-id]
|
||||
(cond
|
||||
(not (r/check-permission plugin-id "content:write"))
|
||||
(not (r/has-permission? plugin-id "content:write"))
|
||||
(u/not-valid plugin-id :resize "Plugin doesn't have 'content:write' permission")
|
||||
|
||||
(not block-id)
|
||||
|
||||
@ -1392,6 +1392,8 @@
|
||||
team-id (:current-team-id @st/state)
|
||||
library-id (uuid/parse library-id)]
|
||||
(->> (rp/cmd! :get-team-shared-files {:team-id team-id})
|
||||
;; Saves may have failed while the list loaded
|
||||
(rx/tap (fn [_] (r/check-writable)))
|
||||
(rx/subs!
|
||||
(fn [shared-files]
|
||||
;; Only published libraries of the team can be linked
|
||||
|
||||
@ -90,6 +90,13 @@
|
||||
(when (<= 1 (count value) 250)
|
||||
value))))
|
||||
|
||||
(defn- granted?
|
||||
"True when the plugin was granted `permission`. Management calls act on
|
||||
files through the backend, not on the open workspace, so the workspace's
|
||||
read-only state does not apply to them."
|
||||
[plugin-id permission]
|
||||
(r/has-permission? plugin-id permission))
|
||||
|
||||
(defn- permission-error
|
||||
[permission]
|
||||
(js/Error. (str "Permission " permission " is not granted")))
|
||||
@ -110,7 +117,7 @@
|
||||
"Resolves with the single value of the stream built by `make-stream` when
|
||||
the plugin has `permission`."
|
||||
[plugin-id permission make-stream]
|
||||
(if (r/check-permission plugin-id permission)
|
||||
(if (granted? plugin-id permission)
|
||||
(js/Promise.
|
||||
(fn [resolve reject]
|
||||
(rx/subs! resolve #(reject (backend-error %)) (make-stream))))
|
||||
@ -294,7 +301,7 @@
|
||||
(fn [value]
|
||||
(let [name (valid-name value)]
|
||||
(cond
|
||||
(not (r/check-permission plugin-id "manage:teams"))
|
||||
(not (granted? plugin-id "manage:teams"))
|
||||
(u/not-valid plugin-id :name "Plugin doesn't have 'manage:teams' permission")
|
||||
|
||||
(nil? name)
|
||||
@ -340,7 +347,7 @@
|
||||
(fn [value]
|
||||
(let [name (valid-name value)]
|
||||
(cond
|
||||
(not (r/check-permission plugin-id "manage:projects"))
|
||||
(not (granted? plugin-id "manage:projects"))
|
||||
(u/not-valid plugin-id :name "Plugin doesn't have 'manage:projects' permission")
|
||||
|
||||
(nil? name)
|
||||
@ -355,7 +362,7 @@
|
||||
:set
|
||||
(fn [value]
|
||||
(cond
|
||||
(not (r/check-permission plugin-id "manage:projects"))
|
||||
(not (granted? plugin-id "manage:projects"))
|
||||
(u/not-valid plugin-id :pinned "Plugin doesn't have 'manage:projects' permission")
|
||||
|
||||
(not (boolean? value))
|
||||
@ -465,7 +472,7 @@
|
||||
(fn [value]
|
||||
(let [name (valid-name value)]
|
||||
(cond
|
||||
(not (r/check-permission plugin-id "content:write"))
|
||||
(not (granted? plugin-id "content:write"))
|
||||
(u/not-valid plugin-id :name "Plugin doesn't have 'content:write' permission")
|
||||
|
||||
(nil? name)
|
||||
@ -480,7 +487,7 @@
|
||||
:set
|
||||
(fn [value]
|
||||
(cond
|
||||
(not (r/check-permission plugin-id "library:write"))
|
||||
(not (granted? plugin-id "library:write"))
|
||||
(u/not-valid plugin-id :shared "Plugin doesn't have 'library:write' permission")
|
||||
|
||||
(not (boolean? value))
|
||||
@ -492,7 +499,7 @@
|
||||
|
||||
:open
|
||||
(fn []
|
||||
(if (r/check-permission plugin-id "content:read")
|
||||
(if (granted? plugin-id "content:read")
|
||||
(open-file (str (:id @data)) #js {:teamId (str (:team-id @data))})
|
||||
(js/Promise.reject (permission-error "content:read"))))
|
||||
|
||||
|
||||
@ -270,7 +270,8 @@
|
||||
(.error js/console "Rollback install failed:" err2))))))
|
||||
(.error js/console "Failed to remove plugin:" err))))))))
|
||||
|
||||
(defn check-permission
|
||||
(defn has-permission?
|
||||
"True when the plugin was granted `permission`."
|
||||
[plugin-id permission]
|
||||
(cond
|
||||
(= plugin-id "00000000-0000-0000-0000-000000000000") true
|
||||
@ -278,6 +279,29 @@
|
||||
:else (let [{:keys [permissions]} (dm/get-in @registry [:data plugin-id])]
|
||||
(contains? permissions permission))))
|
||||
|
||||
(defn check-writable
|
||||
"Throws while the workspace is read-only: saves are failing, the user
|
||||
cannot edit or a version is being previewed. A write made then would be
|
||||
lost, so it fails whatever the plugin's validation flag."
|
||||
[]
|
||||
(let [wglobal (:workspace-global @st/state)]
|
||||
(when (true? (:read-only? wglobal))
|
||||
(throw (ex-info (if (some? (:persistence-lock wglobal))
|
||||
"[PENPOT PLUGIN] The workspace is read-only because changes cannot be saved right now"
|
||||
"[PENPOT PLUGIN] The workspace is read-only")
|
||||
{:type ::read-only})))))
|
||||
|
||||
(def ^:private write-permissions
|
||||
#{"content:write" "library:write"})
|
||||
|
||||
(defn check-permission
|
||||
"True when the plugin was granted `permission`. A write permission also
|
||||
runs `check-writable`, which throws while the workspace is read-only."
|
||||
[plugin-id permission]
|
||||
(when (contains? write-permissions permission)
|
||||
(check-writable))
|
||||
(has-permission? plugin-id permission))
|
||||
|
||||
(defn get-plugin-data
|
||||
[state plugin-id]
|
||||
(get-in state [:profile :props :plugins :data plugin-id]))
|
||||
|
||||
@ -124,6 +124,7 @@
|
||||
|
||||
:remove
|
||||
(fn []
|
||||
(r/check-writable)
|
||||
(cond
|
||||
(not (u/page-active? page-id))
|
||||
(u/not-valid plugin-id :remove "Cannot modify a page that is not currently active")
|
||||
|
||||
@ -365,7 +365,7 @@
|
||||
|
||||
(let [data (u/locate-shape file-id page-id id)]
|
||||
(-> (obj/reify {:name "ShapeProxy"
|
||||
:on-set u/check-shape-exists
|
||||
:on-set u/check-shape-writable
|
||||
:on-error (u/handle-error plugin-id)}
|
||||
:$plugin {:enumerable false :get (fn [] plugin-id)}
|
||||
:$id {:enumerable false :get (fn [] id)}
|
||||
@ -1472,18 +1472,22 @@
|
||||
|
||||
:bringForward
|
||||
(fn []
|
||||
(r/check-writable)
|
||||
(st/emit! (dw/vertical-order-selected (z-order-location plugin-id file-id page-id id :up) [id])))
|
||||
|
||||
:sendBackward
|
||||
(fn []
|
||||
(r/check-writable)
|
||||
(st/emit! (dw/vertical-order-selected (z-order-location plugin-id file-id page-id id :down) [id])))
|
||||
|
||||
:bringToFront
|
||||
(fn []
|
||||
(r/check-writable)
|
||||
(st/emit! (dw/vertical-order-selected (z-order-location plugin-id file-id page-id id :top) [id])))
|
||||
|
||||
:sendToBack
|
||||
(fn []
|
||||
(r/check-writable)
|
||||
(st/emit! (dw/vertical-order-selected (z-order-location plugin-id file-id page-id id :bottom) [id])))
|
||||
|
||||
;; COMPONENTS
|
||||
|
||||
@ -431,6 +431,7 @@
|
||||
|
||||
:applyTypography
|
||||
(fn [typography]
|
||||
(r/check-writable)
|
||||
(let [typography (u/proxy->library-typography typography)
|
||||
attrs (-> typography
|
||||
(assoc :typography-ref-file file-id)
|
||||
|
||||
@ -20,6 +20,7 @@
|
||||
[app.main.data.helpers :as dsh]
|
||||
[app.main.data.workspace.tokens.library-edit :as dwtl]
|
||||
[app.main.store :as st]
|
||||
[app.plugins.register :as r]
|
||||
[app.util.object :as obj]
|
||||
[cuerdas.core :as str]))
|
||||
|
||||
@ -165,11 +166,14 @@
|
||||
(when (nil? (locate-shape file-id page-id id))
|
||||
(throw (shape-not-found id))))
|
||||
|
||||
(defn check-shape-exists
|
||||
"`check-shape-located` for the shape behind `proxy`."
|
||||
(defn check-shape-writable
|
||||
"Throws when the shape behind `proxy` no longer exists (see
|
||||
`check-shape-located`) or the workspace is read-only (see
|
||||
`r/check-writable`)."
|
||||
[proxy]
|
||||
(when (nil? (proxy->shape proxy))
|
||||
(throw (shape-not-found (obj/get proxy "$id")))))
|
||||
(throw (shape-not-found (obj/get proxy "$id"))))
|
||||
(r/check-writable))
|
||||
|
||||
(defn inside-component-copy?
|
||||
"True when `shape` is nested inside a component copy. The copy root itself is
|
||||
@ -601,8 +605,9 @@
|
||||
(let [explain (-> cause ex-data ::sm/explain)
|
||||
throw? (throw-validation-errors? plugin-id)]
|
||||
(cond
|
||||
;; A write to a removed shape always reaches the plugin
|
||||
(= ::shape-not-found (-> cause ex-data :type))
|
||||
;; A write to a removed shape or a read-only workspace always
|
||||
;; reaches the plugin
|
||||
(contains? #{::shape-not-found ::r/read-only} (-> cause ex-data :type))
|
||||
(throw cause)
|
||||
|
||||
;; If it's a clojure error we throw as a validation error
|
||||
|
||||
@ -457,7 +457,8 @@
|
||||
library-id (uuid/next)
|
||||
stream (rx/subject)
|
||||
events (atom [])
|
||||
state (atom {:current-file-id file-id :current-team-id team-id})]
|
||||
state (atom {:current-file-id file-id :current-team-id team-id
|
||||
:permissions {:can-edit true}})]
|
||||
(await
|
||||
(mock/with-mocks*
|
||||
{st/state state
|
||||
@ -490,7 +491,8 @@
|
||||
events (atom [])]
|
||||
(await
|
||||
(mock/with-mocks*
|
||||
{st/state (atom {:current-file-id file-id :current-team-id (uuid/next)})
|
||||
{st/state (atom {:current-file-id file-id :current-team-id (uuid/next)
|
||||
:permissions {:can-edit true}})
|
||||
st/emit! (mock/stub (fn [& emitted] (swap! events into emitted)))
|
||||
rp/cmd! (shared-files-cmd [{:id (uuid/next) :name "Other published"}])
|
||||
r/check-permission (mock/stub (constantly true))}
|
||||
@ -504,7 +506,8 @@
|
||||
events (atom [])]
|
||||
(await
|
||||
(mock/with-mocks*
|
||||
{st/state (atom {:current-file-id file-id :current-team-id (uuid/next)})
|
||||
{st/state (atom {:current-file-id file-id :current-team-id (uuid/next)
|
||||
:permissions {:can-edit true}})
|
||||
st/emit! (mock/stub (fn [& emitted] (swap! events into emitted)))
|
||||
rp/cmd! (shared-files-cmd [{:id file-id :name "Current file"}])
|
||||
r/check-permission (mock/stub (constantly true))}
|
||||
@ -558,3 +561,29 @@
|
||||
(into #{} (map #(obj/get % "$id")) (.-connected context))))
|
||||
(t/is (= #{(str visited-id) (str other-id)}
|
||||
(into #{} (map #(.-id %)) (await (.availableLibraries context))))))))))
|
||||
|
||||
;; Scenario: a library is linked; while the list of published libraries
|
||||
;; loads, a save fails and locks the workspace. Proves: the library is not
|
||||
;; linked and the promise rejects with the read-only error.
|
||||
(t/deftest ^:async connect-library-is-refused-when-a-save-fails-meanwhile
|
||||
(let [library-id (uuid/next)
|
||||
state (atom {:current-file-id (uuid/next)
|
||||
:current-team-id (uuid/next)
|
||||
:permissions {:can-edit true}})
|
||||
answer (rx/subject)
|
||||
events (atom [])]
|
||||
(await
|
||||
(mock/with-mocks*
|
||||
{st/state state
|
||||
st/emit! (mock/stub (fn [& emitted] (swap! events into emitted)))
|
||||
rp/cmd! (mock/stub (fn [cmd params]
|
||||
(swap! mock/rpc-calls conj {:cmd cmd :params params})
|
||||
(->> answer (rx/take 1) (rx/observe-on :async))))}
|
||||
(let [context (library/library-subcontext plugin-id)
|
||||
result (settled (.connectLibrary context (str library-id)))]
|
||||
(await (async/wait-for #(seq @mock/rpc-calls) "the published libraries are requested"))
|
||||
(swap! state assoc-in [:workspace-global :persistence-lock] {:read-only? false})
|
||||
(rx/push! answer [{:id library-id :name "Published"}])
|
||||
(let [{:keys [rejected]} (await result)]
|
||||
(t/is (str/includes? (ex-message rejected) "cannot be saved"))
|
||||
(t/is (empty? @events) "the library is not linked")))))))
|
||||
|
||||
@ -306,7 +306,7 @@
|
||||
[file* "name" nil]
|
||||
[file* "shared" 1]]]
|
||||
(t/is (thrown? js/Error (obj/set! proxy key value)) key))
|
||||
(with-redefs [r/check-permission (mock/stub (constantly false))]
|
||||
(with-redefs [r/has-permission? (mock/stub (constantly false))]
|
||||
(doseq [[proxy key value permission] [[team* "name" "Brand" "manage:teams"]
|
||||
[project* "name" "Icons" "manage:projects"]
|
||||
[project* "pinned" true "manage:projects"]
|
||||
@ -399,7 +399,7 @@
|
||||
(t/is (= "Expected a name with 1 to 250 characters" (await (message (.duplicate file* #js {:name " "})))))
|
||||
(t/is (= "Expected a project" (await (message (.moveTo file* #js {:id "invalid"})))))
|
||||
(t/is (= "Expected a team" (await (message (.moveTo project* nil)))))
|
||||
(with-redefs [r/check-permission (mock/stub (constantly false))]
|
||||
(with-redefs [r/has-permission? (mock/stub (constantly false))]
|
||||
(doseq [[promise permission] [[(.createTeam (management/create-context plugin-id) #js {:name "Brand"}) "manage:teams"]
|
||||
[(.listProjects team*) "content:read"]
|
||||
[(.createProject team* #js {:name "Icons"}) "manage:projects"]
|
||||
|
||||
211
frontend/test/frontend_tests/plugins/read_only_test.cljs
Normal file
211
frontend/test/frontend_tests/plugins/read_only_test.cljs
Normal file
@ -0,0 +1,211 @@
|
||||
;; 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.plugins.read-only-test
|
||||
"Plugin writes in a read-only workspace."
|
||||
(:require
|
||||
[app.common.test-helpers.files :as cthf]
|
||||
[app.common.uuid :as uuid]
|
||||
[app.main.data.helpers :as dsh]
|
||||
[app.main.data.workspace.media :as dwm]
|
||||
[app.main.repo :as rp]
|
||||
[app.main.store :as st]
|
||||
[app.plugins.api :as api]
|
||||
[app.plugins.register :as r]
|
||||
[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]
|
||||
[frontend-tests.helpers.wasm :as thw]))
|
||||
|
||||
;; Its validation flag is off, so only errors that always throw reach it
|
||||
(def ^:private plugin-id "00000000-0000-0000-0000-000000000000")
|
||||
|
||||
(defn- set-read-only
|
||||
[store read-only?]
|
||||
(swap! store assoc-in [:workspace-global :read-only?] read-only?))
|
||||
|
||||
(defn- shape-count
|
||||
[store]
|
||||
(count (dsh/lookup-page-objects @store)))
|
||||
|
||||
(defn- with-context
|
||||
"Calls `f` with a store holding a sample file and a plugin context on it."
|
||||
[f]
|
||||
(thw/with-wasm-mocks*
|
||||
(fn []
|
||||
(let [store (ths/setup-store (cthf/sample-file :file1 :page-label :page1))]
|
||||
(set! st/state store)
|
||||
(f store (api/create-context plugin-id))))))
|
||||
|
||||
(t/deftest writes-throw-in-a-read-only-workspace
|
||||
(with-context
|
||||
(fn [store ^js context]
|
||||
(let [^js board (.createBoard context)]
|
||||
(set-read-only store true)
|
||||
|
||||
(t/testing "a shape setter"
|
||||
(t/is (thrown-with-msg? js/Error #"read-only" (set! (.-name board) "Renamed")))
|
||||
(t/is (not= "Renamed" (.-name board))))
|
||||
|
||||
(t/testing "a shape method"
|
||||
(t/is (thrown-with-msg? js/Error #"read-only" (.bringToFront board))))
|
||||
|
||||
(t/testing "a context method without a permission check"
|
||||
(let [before (shape-count store)]
|
||||
(t/is (thrown-with-msg? js/Error #"read-only" (.createRectangle context)))
|
||||
(t/is (= before (shape-count store)))))
|
||||
|
||||
(t/testing "a context method with a permission check"
|
||||
(t/is (thrown-with-msg? js/Error #"read-only" (.createPage (.-currentFile context)))))))))
|
||||
|
||||
(t/deftest writes-work-again-once-the-workspace-is-editable
|
||||
(with-context
|
||||
(fn [store ^js context]
|
||||
(let [^js board (.createBoard context)]
|
||||
(set-read-only store true)
|
||||
(set-read-only store false)
|
||||
(set! (.-name board) "Renamed")
|
||||
(t/is (= "Renamed" (.-name board)))))))
|
||||
|
||||
(t/deftest a-save-failure-lock-says-why-the-workspace-is-read-only
|
||||
(with-context
|
||||
(fn [store ^js context]
|
||||
(swap! store update :workspace-global assoc
|
||||
:read-only? true
|
||||
:persistence-lock {:read-only? false})
|
||||
(t/is (thrown-with-msg? js/Error #"cannot be saved" (.createBoard context))))))
|
||||
|
||||
(t/deftest reads-and-undo-blocks-work-in-a-read-only-workspace
|
||||
(with-context
|
||||
(fn [store ^js context]
|
||||
(let [^js board (.createBoard context)]
|
||||
(set-read-only store true)
|
||||
(t/is (string? (.-name board)))
|
||||
(let [block (.undoBlockBegin (.-history context))]
|
||||
(t/is (some? block))
|
||||
(t/is (nil? (.undoBlockFinish (.-history context) block))))))))
|
||||
|
||||
(t/deftest read-only-keeps-permission-checks-of-reads
|
||||
(with-context
|
||||
(fn [store _]
|
||||
(set-read-only store true)
|
||||
(t/is (true? (r/check-permission plugin-id "content:read")))
|
||||
(t/is (true? (r/has-permission? plugin-id "content:write")))
|
||||
(t/is (thrown? js/Error (r/check-permission plugin-id "library:write"))))))
|
||||
|
||||
;; Writes that wait for the server before they change the file
|
||||
|
||||
(defn- settled
|
||||
"Resolves with `{:resolved value}` or `{:rejected cause}` once `p` settles,
|
||||
or with `{:timeout true}` when it stays pending for 2 seconds."
|
||||
[p]
|
||||
(js/Promise.race
|
||||
#js [(.then p (fn [v] {:resolved v}) (fn [e] {:rejected e}))
|
||||
(js/Promise. (fn [resolve] (js/setTimeout #(resolve {:timeout true}) 2000)))]))
|
||||
|
||||
(defn- lock-saves
|
||||
"Locks `store` as a failing save does."
|
||||
[store]
|
||||
(swap! store update :workspace-global assoc
|
||||
:read-only? true
|
||||
:persistence-lock {:read-only? false}))
|
||||
|
||||
(defn- held-rpc
|
||||
"An `rp/cmd!` that records each command in `calls` and answers with the
|
||||
next value pushed to `answer`."
|
||||
[calls answer]
|
||||
(mock/stub (fn [cmd _]
|
||||
(swap! calls conj cmd)
|
||||
(->> answer (rx/take 1) (rx/observe-on :async)))))
|
||||
|
||||
(def ^:private media-object
|
||||
{:id (uuid/next) :name "image" :width 10 :height 10 :mtype "image/png"})
|
||||
|
||||
(def ^:private svg-with-image
|
||||
(str "<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"10\" height=\"10\">"
|
||||
"<image href=\"data:image/png;base64,iVBORw0KGgo=\" width=\"10\" height=\"10\"/>"
|
||||
"</svg>"))
|
||||
|
||||
(defn- parsed-svg-with-image
|
||||
"What `dwm/svg->clj` gives for `svg-with-image`. The test build cannot
|
||||
load the XML parser, a CommonJS module."
|
||||
[[name _]]
|
||||
(rx/of {:tag :svg
|
||||
:name name
|
||||
:attrs {:xmlns "http://www.w3.org/2000/svg" :width "10" :height "10"}
|
||||
:content [{:tag :image
|
||||
:attrs {:href "data:image/png;base64,iVBORw0KGgo=" :width "10" :height "10"}
|
||||
:content []}]}))
|
||||
|
||||
(defn- read-only-error?
|
||||
[cause]
|
||||
(boolean (some->> cause ex-message (re-find #"cannot be saved"))))
|
||||
|
||||
;; Scenario: an SVG with an image is created; while its image uploads a
|
||||
;; save fails and locks the workspace. Proves: the shape is not added and
|
||||
;; the promise rejects with the read-only error.
|
||||
(t/deftest ^:async an-svg-whose-upload-ends-after-a-save-failure-is-refused
|
||||
(thw/setup-wasm-mocks!)
|
||||
(try
|
||||
(let [store (ths/setup-store (cthf/sample-file :file1 :page-label :page1))
|
||||
calls (atom [])
|
||||
answer (rx/subject)]
|
||||
(await
|
||||
(mock/with-mocks*
|
||||
{st/state store
|
||||
rp/cmd! (held-rpc calls answer)
|
||||
dwm/valid-svg-string? (constantly true)
|
||||
dwm/svg->clj (mock/stub parsed-svg-with-image)}
|
||||
(let [^js context (api/create-context plugin-id)
|
||||
before (shape-count store)
|
||||
result (settled (.createShapeFromSvgWithImages context svg-with-image))]
|
||||
(await (async/wait-for #(= [:upload-file-media-object] @calls) "the image upload starts"))
|
||||
(lock-saves store)
|
||||
(rx/push! answer media-object)
|
||||
(let [{:keys [rejected]} (await result)]
|
||||
(t/is (read-only-error? rejected))
|
||||
(await (async/settle))
|
||||
(t/is (= before (shape-count store)) "no shape is added"))))))
|
||||
(finally
|
||||
(thw/teardown-wasm-mocks!))))
|
||||
|
||||
;; Uploads `upload` with the context; while the request for `cmd` runs, a
|
||||
;; save fails and locks the workspace.
|
||||
(defn- ^:async upload-during-a-save-failure
|
||||
[upload cmd]
|
||||
(let [store (ths/setup-store (cthf/sample-file :file1 :page-label :page1))
|
||||
calls (atom [])
|
||||
answer (rx/subject)]
|
||||
(await
|
||||
(mock/with-mocks*
|
||||
{st/state store
|
||||
rp/cmd! (held-rpc calls answer)}
|
||||
;; Bound one by one: a nested call with a local fn would compile to
|
||||
;; an awaited IIFE, which waits for the upload itself
|
||||
(let [context (api/create-context plugin-id)
|
||||
result (settled (upload context))]
|
||||
(await (async/wait-for #(= [cmd] @calls) "the upload starts"))
|
||||
(lock-saves store)
|
||||
(rx/push! answer media-object)
|
||||
(let [{:keys [rejected]} (await result)]
|
||||
(t/is (read-only-error? rejected) (str cmd))))))))
|
||||
|
||||
;; Scenario: an image is uploaded from data and from a URL; while each
|
||||
;; upload runs a save fails and locks the workspace. Proves: the promise
|
||||
;; rejects with the read-only error. The media object stays on the server.
|
||||
(t/deftest ^:async media-uploaded-after-a-save-failure-is-refused
|
||||
(thw/setup-wasm-mocks!)
|
||||
(try
|
||||
(await (upload-during-a-save-failure
|
||||
#(.uploadMediaData ^js % "image" (js/Uint8Array. #js [1 2 3]) "image/png")
|
||||
:upload-file-media-object))
|
||||
(await (upload-during-a-save-failure
|
||||
#(.uploadMediaUrl ^js % "image" "https://example.com/image.png")
|
||||
:create-file-media-object-from-url))
|
||||
(finally
|
||||
(thw/teardown-wasm-mocks!))))
|
||||
@ -91,6 +91,7 @@
|
||||
[frontend-tests.plugins.page-active-validation-test]
|
||||
[frontend-tests.plugins.page-test]
|
||||
[frontend-tests.plugins.parser-test]
|
||||
[frontend-tests.plugins.read-only-test]
|
||||
[frontend-tests.plugins.register-test]
|
||||
[frontend-tests.plugins.shape-bugfixes-test]
|
||||
[frontend-tests.plugins.text-test]
|
||||
@ -281,6 +282,7 @@
|
||||
'frontend-tests.plugins.page-active-validation-test
|
||||
'frontend-tests.plugins.page-test
|
||||
'frontend-tests.plugins.parser-test
|
||||
'frontend-tests.plugins.read-only-test
|
||||
'frontend-tests.plugins.register-test
|
||||
'frontend-tests.plugins.shape-bugfixes-test
|
||||
'frontend-tests.plugins.text-test
|
||||
|
||||
@ -24,6 +24,11 @@ So if you need to read the result of your executions/observe properties affected
|
||||
before trying to observe changes.
|
||||
Every `execude_code` call waits for updates before applying your code, so you never need to call `waitForLayoutUpdate` at the beginning of your code.
|
||||
|
||||
## Read-Only Mode
|
||||
|
||||
While saves are failing, the workspace becomes read-only: every write through the API throws an error saying that the
|
||||
workspace is read-only. Do not retry in a loop; wait and try again later, or tell the user.
|
||||
|
||||
# The Structure of Penpot Designs
|
||||
|
||||
A Penpot design ultimately consists of shapes.
|
||||
|
||||
@ -18,6 +18,8 @@
|
||||
- **plugin-types, plugin-runtime:** Dashboard property setters now update values after backend success and log failures without interrupting the workspace. File moves reject projects in other teams, backend rejections include their type and code, and `project.fileCount` stays at its listing snapshot. Duplicated projects include their file count and pinned state.
|
||||
- **plugin-runtime:** Cancel plugin loads before they execute after unloading or logout, and allow reopening plugins that close during startup.
|
||||
- **plugins-runtime**: `library.connectLibrary()` now resolves once the library's assets are loaded; it used to resolve with an empty library the first time. `library.connected` and `library.availableLibraries()` ignore files opened earlier in the same tab, which used to show up as connected and be left out of the available libraries.
|
||||
- **plugins-runtime:** Writes now fail with an error while the workspace is read-only (saves failing, a user who cannot edit, a version preview), whatever the `throwValidationErrors` flag. They used to change the workspace with edits that would never be saved. Undo blocks still open and close.
|
||||
- **plugins-runtime:** Writes that wait for the server (`createShapeFromSvgWithImages`, `uploadMediaData`, `uploadMediaUrl`, `connectLibrary`) check again when the server answers. If the workspace turned read-only meanwhile (a save failed), the promise rejects with the read-only error: no shape is added and no library is linked. An image the server already stored stays in the file's media, unused.
|
||||
- **plugins-runtime:** `LibraryComponent.remove()` now rejects linked-library and missing component handles, and does nothing for an already deleted component.
|
||||
- **plugins-runtime**: An interaction obtained from `Shape.interactions` now keeps addressing that interaction instead of the position it held when the array was read. Removing every interaction of a shape from a single read removes all of them rather than leaving some behind, and writing through a held interaction after an earlier one is removed no longer lands on a different interaction.
|
||||
- **plugins-runtime**: `Shape.removeInteraction()` now rejects an interaction belonging to a different shape with a validation error, instead of removing whichever interaction sat at the same position on the target shape.
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user