mirror of
https://github.com/penpot/penpot.git
synced 2026-04-29 13:18:29 +00:00
🐛 Fix Plugin API token application for JS array of strings (#9166)
* 🐛 Fix Plugin API token application for JS array of strings Plugin code calling `shape.applyToken(token, ["fill"])` or `token.applyToShapes([rect], ["fill"])` from JavaScript supplies a JS array of strings. The plugin proxies expected a Clojure set of keywords, and two coupled defects made the calls silently no-op (or, with `throwValidationErrors` enabled, throw "check error"): 1. `token-attr-plugin->token-attr` only consulted its alias map when the input was already a keyword. String inputs like "fill" fell through to the identity branch, so the downstream `cto/token-attr?` predicate (which checks against a set of keywords) returned false for every string. Coerce strings to keywords first. 2. The `applyToken` / `applyToShapes` / `applyToSelected` schemas used plain `[:set ...]`, which has no `:decode/json` transformer for JS array → Clojure set coercion. Switch to the registered `[::sm/set ...]` (in `app.common.schema`) which provides the array → set decoder. After the switch, the standard JSON pipeline converts `["fill"]` to `#{"fill"}`, then the inner `[:and ::sm/keyword [:fn token-attr?]]` decodes each element to a keyword and validates it. Also extends the docstring on `token-attr-plugin->token-attr` to make the string-friendly contract explicit, and registers a new `tokens-test` ns under `frontend/test/frontend_tests/plugins/` with six `deftest` blocks covering: - known keywords passing through unchanged - keyword aliases (`:r1` → `:border-radius-top-left`, etc.) - string inputs coerced to keywords (regression for #9162) - `token-attr?` accepting both keyword and string inputs - `token-attr?` rejecting unknown attrs and nil Closes #9162 * 🐛 Fix wrong direction in plugin-name alias tests The added tests in tokens_test.cljs and the new docstring in tokens.cljs described the alias resolution in the wrong direction. The map is {:r1 :border-radius-top-left, …} then map-invert'd, so token-attr-plugin->token-attr maps verbose plugin-side names (:border-radius-top-left) to canonical internal short names (:r1), not the other way around. Inputs already in canonical form (:r1, :fill, "fill", …) pass through unchanged. Flipped the alias-resolution test expectations and the keyword/string-input cases, refreshed the docstring and the regression-coverage comment to match. --------- Co-authored-by: Andrey Antukh <niwi@niwi.nz>
This commit is contained in:
parent
9751ac2b41
commit
1eac3e2be5
@ -56,6 +56,7 @@
|
||||
- Fix plugin API `library.connectLibrary()` returning a non-Promise (or throwing synchronously) when the plugin lacks `library:write` permission — the method now always returns a `Promise` and rejects with a structured error message, matching the contract used by every other Promise-returning plugin method (`restore`, `remove`, `pin`, `saveVersion`, `findVersions`, …)
|
||||
- Fix LDAP provider params schema typo (`bind-passwor` → `bind-password`) introduced during the `clojure.spec` → `malli` migration; the schema slot now matches the runtime key actually read by `prepare-params` (`:password (:bind-password cfg)`) and `try-connectivity` (`(:bind-password cfg)`), so a wrong type for the password no longer slips through unvalidated
|
||||
- Fix `login-with-ldap` silently dropping its error message on the `ldap-not-initialized` restriction (typo `:hide` → `:hint`); the message `"ldap auth provider is not initialized"` now actually surfaces in logs and error responses instead of being discarded into an unread key
|
||||
- Fix Plugin API `shape.applyToken()` / `token.applyToShapes()` / `token.applyToSelected()` rejecting JS-array attribute lists like `["fill"]`: switched the inner schemas to `[::sm/set ...]` (which has the JS array → Clojure set decoder) and made `token-attr-plugin->token-attr` accept string inputs by coercing them to keywords before consulting the alias map [Github #9162](https://github.com/penpot/penpot/issues/9162)
|
||||
- Fix `PENPOT_OIDC_USER_INFO_SOURCE` flag being silently ignored (`userinfo` / `token`) in the OIDC callback, causing "incomplete user info" failures during registration [Github #9108](https://github.com/penpot/penpot/issues/9108)
|
||||
- Fix `get-view-only-bundle` crashing when a share-link viewer encounters a team member whose email lacks `@` (NullPointerException in `obfuscate-email`) or whose domain has no `.` (previously produced a dangling-dot `****@****.`); now the viewer-side obfuscation is nil-safe and omits the trailing dot when the domain has no TLD
|
||||
- Remove `corepack` from the MCP local launcher so it runs on Node.js 25+, where corepack is no longer bundled [Github #8877](https://github.com/penpot/penpot/issues/8877)
|
||||
|
||||
@ -1338,7 +1338,7 @@
|
||||
{:enumerable false
|
||||
:schema [:tuple
|
||||
[:fn token-proxy?]
|
||||
[:maybe [:set [:and ::sm/keyword [:fn token-attr?]]]]]
|
||||
[:maybe [::sm/set [:and ::sm/keyword [:fn token-attr?]]]]]
|
||||
:fn (fn [token attrs]
|
||||
(let [token (u/locate-token file-id (obj/get token "$set-id") (obj/get token "$id"))
|
||||
kw-attrs (into #{} (map token-attr-plugin->token-attr attrs))]
|
||||
|
||||
@ -49,8 +49,19 @@
|
||||
(get map:token-attr->token-attr-plugin k k))
|
||||
|
||||
(defn token-attr-plugin->token-attr
|
||||
"Resolve a plugin-side token attribute reference to its canonical
|
||||
internal keyword.
|
||||
|
||||
Accepts either a Clojure keyword (the canonical form, e.g. `:r1`,
|
||||
`:fill`) or a string (the natural shape that arrives from a JS plugin
|
||||
call such as `shape.applyToken(token, [\"fill\"])`). Converts strings
|
||||
to keywords first, then maps verbose plugin-side aliases (e.g.
|
||||
`:border-radius-top-left`) to their internal short form (e.g. `:r1`).
|
||||
Inputs that are already in canonical form (`:r1`, `:fill`, `\"fill\"`,
|
||||
…) pass through unchanged."
|
||||
[k]
|
||||
(get map:token-attr-plugin->token-attr k k))
|
||||
(let [k (cond-> k (string? k) keyword)]
|
||||
(get map:token-attr-plugin->token-attr k k)))
|
||||
|
||||
(defn applied-tokens-plugin->applied-tokens
|
||||
[value]
|
||||
@ -186,13 +197,13 @@
|
||||
{:enumerable false
|
||||
:schema [:tuple
|
||||
[:vector [:fn shape-proxy?]]
|
||||
[:maybe [:set [:and ::sm/keyword [:fn token-attr?]]]]]
|
||||
[:maybe [::sm/set [:and ::sm/keyword [:fn token-attr?]]]]]
|
||||
:fn (fn [shapes attrs]
|
||||
(apply-token-to-shapes plugin-id file-id set-id id (map #(obj/get % "$id") shapes) attrs))}
|
||||
|
||||
:applyToSelected
|
||||
{:enumerable false
|
||||
:schema [:tuple [:maybe [:set [:and ::sm/keyword [:fn token-attr?]]]]]
|
||||
:schema [:tuple [:maybe [::sm/set [:and ::sm/keyword [:fn token-attr?]]]]]
|
||||
:fn (fn [attrs]
|
||||
(let [selected (get-in @st/state [:workspace-local :selected])]
|
||||
(apply-token-to-shapes plugin-id file-id set-id id selected attrs)))}))
|
||||
|
||||
82
frontend/test/frontend_tests/plugins/tokens_test.cljs
Normal file
82
frontend/test/frontend_tests/plugins/tokens_test.cljs
Normal file
@ -0,0 +1,82 @@
|
||||
;; This Source Code Form is subject to the terms of the Mozilla Public
|
||||
;; License, v. 2.0. If a copy of the MPL was not distributed with this
|
||||
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
|
||||
;;
|
||||
;; Copyright (c) KALEIDOS INC
|
||||
|
||||
(ns frontend-tests.plugins.tokens-test
|
||||
(:require
|
||||
[app.plugins.tokens :as ptok]
|
||||
[cljs.test :as t :include-macros true]))
|
||||
|
||||
;; Regression coverage for issue #9162.
|
||||
;;
|
||||
;; Plugin code calling `shape.applyToken(token, ["fill"])` or
|
||||
;; `token.applyToShapes([rect], ["fill"])` from JavaScript supplies a JS
|
||||
;; array of strings. Penpot's plugin proxies expect a Clojure set of
|
||||
;; keywords. Two coupled defects made these calls silently no-op (or, with
|
||||
;; `throwValidationErrors` enabled, throw a "check error"):
|
||||
;;
|
||||
;; 1. `token-attr-plugin->token-attr` only consulted its alias map when
|
||||
;; the input was already a keyword — string inputs like "fill" or
|
||||
;; "border-radius-top-left" fell through to the identity branch
|
||||
;; unchanged, so the downstream `cto/token-attr?` predicate (which
|
||||
;; checks against a set of keywords) returned false.
|
||||
;; 2. The `applyToken` / `applyToShapes` / `applyToSelected` schemas used
|
||||
;; plain `[:set ...]`, which does not have a `:decode/json`
|
||||
;; transformer for the JS array → Clojure set coercion. Penpot's
|
||||
;; custom `[::sm/set ...]` does. Switching to the registered set type
|
||||
;; lets the standard JSON decoder pipeline turn the JS argument into
|
||||
;; a set of strings, after which the `[:and ::sm/keyword [:fn
|
||||
;; token-attr?]]` element schema coerces each string to a keyword and
|
||||
;; validates it.
|
||||
;;
|
||||
;; These helper-level tests pin the string-friendly conversion contract;
|
||||
;; the schema-level fix is covered by the existing plugin integration
|
||||
;; suite that exercises `applyToken` end-to-end.
|
||||
|
||||
(t/deftest token-attr-plugin->token-attr-passes-canonical-form-through
|
||||
;; Both already-canonical short names and unaliased names pass through
|
||||
;; unchanged.
|
||||
(t/is (= :fill (ptok/token-attr-plugin->token-attr :fill)))
|
||||
(t/is (= :stroke-color (ptok/token-attr-plugin->token-attr :stroke-color)))
|
||||
(t/is (= :r1 (ptok/token-attr-plugin->token-attr :r1)))
|
||||
(t/is (= :p2 (ptok/token-attr-plugin->token-attr :p2))))
|
||||
|
||||
(t/deftest token-attr-plugin->token-attr-resolves-verbose-plugin-aliases
|
||||
;; Plugin-side verbose names (e.g. `:border-radius-top-left`) map to
|
||||
;; their canonical short internal form (`:r1`) so plugin authors can
|
||||
;; spell the corner explicitly without the engine having to know both.
|
||||
(t/is (= :r1 (ptok/token-attr-plugin->token-attr :border-radius-top-left)))
|
||||
(t/is (= :r2 (ptok/token-attr-plugin->token-attr :border-radius-top-right)))
|
||||
(t/is (= :r3 (ptok/token-attr-plugin->token-attr :border-radius-bottom-right)))
|
||||
(t/is (= :r4 (ptok/token-attr-plugin->token-attr :border-radius-bottom-left)))
|
||||
(t/is (= :p1 (ptok/token-attr-plugin->token-attr :padding-top-left)))
|
||||
(t/is (= :m3 (ptok/token-attr-plugin->token-attr :margin-bottom-right))))
|
||||
|
||||
(t/deftest token-attr-plugin->token-attr-coerces-string-input
|
||||
;; This is the actual regression — JS plugin calls supply strings.
|
||||
(t/is (= :fill (ptok/token-attr-plugin->token-attr "fill")))
|
||||
(t/is (= :stroke-color (ptok/token-attr-plugin->token-attr "stroke-color")))
|
||||
;; Verbose plugin aliases work via the string path too.
|
||||
(t/is (= :r1 (ptok/token-attr-plugin->token-attr "border-radius-top-left")))
|
||||
(t/is (= :m3 (ptok/token-attr-plugin->token-attr "margin-bottom-right"))))
|
||||
|
||||
(t/deftest token-attr?-accepts-keyword-input
|
||||
(t/is (true? (boolean (ptok/token-attr? :fill))))
|
||||
(t/is (true? (boolean (ptok/token-attr? :stroke-color))))
|
||||
(t/is (true? (boolean (ptok/token-attr? :r1))))
|
||||
(t/is (true? (boolean (ptok/token-attr? :p2)))))
|
||||
|
||||
(t/deftest token-attr?-accepts-string-input
|
||||
;; Same JS-array-of-strings reproducer as the issue, exercised at the
|
||||
;; predicate layer the plugin schemas call into.
|
||||
(t/is (true? (boolean (ptok/token-attr? "fill"))))
|
||||
(t/is (true? (boolean (ptok/token-attr? "stroke-color"))))
|
||||
(t/is (true? (boolean (ptok/token-attr? "r1"))))
|
||||
(t/is (true? (boolean (ptok/token-attr? "m3")))))
|
||||
|
||||
(t/deftest token-attr?-rejects-unknown-input
|
||||
(t/is (false? (boolean (ptok/token-attr? :not-a-real-attr))))
|
||||
(t/is (false? (boolean (ptok/token-attr? "not-a-real-attr"))))
|
||||
(t/is (false? (boolean (ptok/token-attr? nil)))))
|
||||
@ -21,6 +21,7 @@
|
||||
[frontend-tests.main-errors-test]
|
||||
[frontend-tests.plugins.context-shapes-test]
|
||||
[frontend-tests.plugins.parser-test]
|
||||
[frontend-tests.plugins.tokens-test]
|
||||
[frontend-tests.svg-fills-test]
|
||||
[frontend-tests.tokens.import-export-test]
|
||||
[frontend-tests.tokens.logic.token-actions-test]
|
||||
@ -65,6 +66,7 @@
|
||||
'frontend-tests.logic.pasting-in-containers-test
|
||||
'frontend-tests.plugins.context-shapes-test
|
||||
'frontend-tests.plugins.parser-test
|
||||
'frontend-tests.plugins.tokens-test
|
||||
'frontend-tests.svg-fills-test
|
||||
'frontend-tests.tokens.import-export-test
|
||||
'frontend-tests.tokens.logic.token-actions-test
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user