mirror of
https://github.com/penpot/penpot.git
synced 2026-10-29 14:00:37 +00:00
✨ Make copies follow their component through a branch merge
A merge or an update from main compared a component's main and each copy as separate shapes. A copy added on the other side kept the old look, and when both sides changed the component the dialog showed one conflict per copy, so a copy could end up disagreeing with its component without being marked as overridden. Copies now follow the merged component, as they do when the component is edited. Per copy attribute: if no side overrode it, it raises no conflict and follows the component; if one side overrode it, that side wins; only overrides on both sides that differ stay a conflict. After the merge or update applies, the copies of every local component whose main changed are synced, nested copies included, and the sync changes are validated and recorded with the rest. A clean merge of a copy no longer marks it as overridden on the target. AI-assisted-by: claude-opus-5-5
This commit is contained in:
Álvaro Tejero Cantero
parent
0c662250dc
commit
d7cce4a930
@ -1106,7 +1106,13 @@
|
||||
takes a safety snapshot of main, applies the merge to main through the
|
||||
production change pipeline, records the merge with its `comment`
|
||||
(`file_branch_merge`), deletes the branch and its file and notifies
|
||||
open clients via msgbus."
|
||||
open clients via msgbus.
|
||||
|
||||
Copies follow the merged components: a copy attr no side overrode is
|
||||
no conflict, and after the merge (and its repair) applies, the copies
|
||||
of every local component whose main it changed are synced on main
|
||||
(`bm/sync-copies`). The sync changes go into the same `file_change`
|
||||
row and the same validation as the merge."
|
||||
{::doc/added "2.16"
|
||||
::webhooks/event? true
|
||||
::sm/params schema:merge-file-branch
|
||||
@ -1283,24 +1289,39 @@
|
||||
;; under the pre-allocated ids the changes reference
|
||||
(copy-media-rows! conn main-id fresh-map new-media)
|
||||
|
||||
(let [merged (-> main-file
|
||||
(update :revn inc)
|
||||
(update :data #(cpc/process-changes % changes)))
|
||||
libs (bfc/get-resolved-file-libraries cfg merged)
|
||||
errors (not-empty (cfv/validate-file merged libs))
|
||||
repair (if errors
|
||||
(cfr/repair-file merged libs errors)
|
||||
[])
|
||||
merged (cond-> merged
|
||||
(seq repair)
|
||||
(update :data cpc/process-changes repair))
|
||||
(let [applied (-> main-file
|
||||
(update :revn inc)
|
||||
(update :data #(cpc/process-changes % changes)))
|
||||
libs (bfc/get-resolved-file-libraries cfg applied)
|
||||
errors (not-empty (cfv/validate-file applied libs))
|
||||
repair (if errors
|
||||
(cfr/repair-file applied libs errors)
|
||||
[])
|
||||
merged (cond-> applied
|
||||
(seq repair)
|
||||
(update :data cpc/process-changes repair))
|
||||
|
||||
;; copies follow the integrated components:
|
||||
;; the copies of every local component whose
|
||||
;; main the merge (or the repair) changed are
|
||||
;; synced, as the workspace syncs them after
|
||||
;; an edit (`bm/sync-copies`)
|
||||
{synced :data sync :changes}
|
||||
(bm/sync-copies main-id (:data merged)
|
||||
(into (bm/changed-components (:data main-file) changes)
|
||||
(bm/changed-components (:data applied) repair)))
|
||||
merged (assoc merged :data synced)
|
||||
libs (if (seq sync)
|
||||
(bfc/get-resolved-file-libraries cfg merged)
|
||||
libs)
|
||||
|
||||
;; the validator, not the merge, decides
|
||||
;; validity: the repaired result goes back
|
||||
;; to it, and whatever stays broken refuses
|
||||
;; the merge and rolls the transaction back
|
||||
errors (when errors
|
||||
(not-empty (cfv/validate-file merged libs)))]
|
||||
;; validity: the repaired and synced result
|
||||
;; goes back to it, and whatever stays broken
|
||||
;; refuses the merge and rolls the
|
||||
;; transaction back
|
||||
errors (when (or errors (seq sync))
|
||||
(not-empty (cfv/validate-file merged libs)))]
|
||||
(when errors
|
||||
(ex/raise :type :validation
|
||||
:code :merge-result-invalid
|
||||
@ -1319,7 +1340,9 @@
|
||||
:revn (:revn merged)
|
||||
:version (:version merged)
|
||||
:features (into-array (:features merged))
|
||||
:changes (blob/encode (into (vec changes) repair))}
|
||||
:changes (blob/encode (-> (vec changes)
|
||||
(into repair)
|
||||
(into sync)))}
|
||||
{::db/return-keys false})
|
||||
|
||||
;; a page the merge left as it was keeps its
|
||||
@ -1408,7 +1431,12 @@
|
||||
`{:status :unsupported}` for change kinds not yet translatable. On
|
||||
success it applies main's changes (and any conflicts resolved to main)
|
||||
to the branch, takes a safety snapshot, and repositions the merge base
|
||||
to the current state of main."
|
||||
to the current state of main.
|
||||
|
||||
Copies follow the integrated components as in a merge: after main's
|
||||
changes apply, the copies of every local component whose main they
|
||||
changed are synced on the branch, in the branch's id space
|
||||
(`bm/sync-copies`), before the squash records the branch's net."
|
||||
{::doc/added "2.16"
|
||||
::webhooks/event? true
|
||||
::sm/params schema:update-branch-from-main
|
||||
@ -1649,25 +1677,41 @@
|
||||
;; rows — or the remapped changes would name ids
|
||||
;; it does not hold and apply to nothing (a
|
||||
;; `:del-media` of a paired copy would keep it)
|
||||
updated (-> branch-file
|
||||
before (bm/remap-refs (:data branch-file) media-map)
|
||||
written (-> branch-file
|
||||
(update :revn inc)
|
||||
(update :data #(cpc/process-changes
|
||||
(bm/remap-refs % media-map)
|
||||
applied)))
|
||||
libs (bfc/get-resolved-file-libraries cfg updated)
|
||||
errors (not-empty (cfv/validate-file updated libs))
|
||||
(assoc :data (cpc/process-changes before applied)))
|
||||
libs (bfc/get-resolved-file-libraries cfg written)
|
||||
errors (not-empty (cfv/validate-file written libs))
|
||||
repair (if errors
|
||||
(cfr/repair-file updated libs errors)
|
||||
(cfr/repair-file written libs errors)
|
||||
[])
|
||||
updated (cond-> updated
|
||||
updated (cond-> written
|
||||
(seq repair)
|
||||
(update :data cpc/process-changes repair))
|
||||
|
||||
;; copies follow the integrated components: the
|
||||
;; copies of every local component whose main
|
||||
;; the update (or the repair) changed are
|
||||
;; synced, in the branch's id space the write
|
||||
;; runs in (`bm/sync-copies`). The squash below
|
||||
;; diffs `updated`, so it records the sync as
|
||||
;; branch work
|
||||
{synced :data sync :changes}
|
||||
(bm/sync-copies branch-file-id (:data updated)
|
||||
(into (bm/changed-components before applied)
|
||||
(bm/changed-components (:data written) repair)))
|
||||
updated (assoc updated :data synced)
|
||||
libs (if (seq sync)
|
||||
(bfc/get-resolved-file-libraries cfg updated)
|
||||
libs)
|
||||
|
||||
;; the validator, not the update, decides
|
||||
;; validity: the repaired result goes back to
|
||||
;; it, and whatever stays broken refuses the
|
||||
;; update and rolls the transaction back
|
||||
errors (when errors
|
||||
;; validity: the repaired and synced result
|
||||
;; goes back to it, and whatever stays broken
|
||||
;; refuses the update and rolls the
|
||||
;; transaction back
|
||||
errors (when (or errors (seq sync))
|
||||
(not-empty (cfv/validate-file updated libs)))]
|
||||
(when errors
|
||||
(ex/raise :type :validation
|
||||
@ -1686,7 +1730,9 @@
|
||||
:revn (:revn updated)
|
||||
:version (:version updated)
|
||||
:features (into-array (:features updated))
|
||||
:changes (blob/encode (into (vec applied) repair))}
|
||||
:changes (blob/encode (-> (vec applied)
|
||||
(into repair)
|
||||
(into sync)))}
|
||||
{::db/return-keys false})
|
||||
|
||||
;; SQUASH: the new base (main's current state)
|
||||
|
||||
@ -0,0 +1,304 @@
|
||||
;; 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 Sucursal en España SL
|
||||
|
||||
(ns backend-tests.branch-copies-follow-component-test
|
||||
"Copies follow their merged component. When a branch is merged into
|
||||
main, or main's changes are brought into the branch, the copies of a
|
||||
local component end up as Penpot's component sync leaves them after
|
||||
the integrated component: an attribute no side overrode by hand follows
|
||||
the component, an attribute one side overrode keeps that side's value,
|
||||
and only an attribute both sides overrode differently is a conflict.
|
||||
|
||||
Each test builds main with a local component (main `:mi-id`) and one
|
||||
copy of it (`:copy-id`), branches it, edits both sides and integrates.
|
||||
A component edit writes the main and, as the workspace's sync does,
|
||||
the untouched copy with `:ignore-touched`; an override writes the copy
|
||||
alone, which marks it touched."
|
||||
(:require
|
||||
[app.binfile.common :as bfc]
|
||||
[app.common.features :as cfeat]
|
||||
[app.common.files.validate :as cfv]
|
||||
[app.common.types.shape :as cts]
|
||||
[app.common.uuid :as uuid]
|
||||
[app.config :as cf]
|
||||
[app.db :as db]
|
||||
[app.rpc :as-alias rpc]
|
||||
[backend-tests.helpers :as th]
|
||||
[clojure.test :as t]))
|
||||
|
||||
(t/use-fixtures :once th/state-init)
|
||||
(t/use-fixtures :each th/database-reset)
|
||||
|
||||
(defn- save
|
||||
"Apply `changes` to `file-id` through the RPC, as the workspace does."
|
||||
[profile file-id changes]
|
||||
(let [file (th/db-get :file {:id file-id})
|
||||
out (th/command! {::th/type :update-file
|
||||
::rpc/profile-id (:id profile)
|
||||
:id file-id
|
||||
:session-id (uuid/random)
|
||||
:revn (:revn file)
|
||||
:vern (:vern file)
|
||||
:features cfeat/supported-features
|
||||
:changes changes})]
|
||||
(t/is (nil? (:error out)))))
|
||||
|
||||
(defn- get-file
|
||||
[profile file-id]
|
||||
(:result (th/command! {::th/type :get-file
|
||||
::rpc/profile-id (:id profile)
|
||||
:id file-id})))
|
||||
|
||||
(defn- component-frame
|
||||
[id attrs]
|
||||
(merge (cts/setup-shape {:id id :name "Button" :type :frame
|
||||
:x 0 :y 0 :width 100 :height 100
|
||||
:parent-id uuid/zero :frame-id uuid/zero})
|
||||
attrs))
|
||||
|
||||
(defn- add-root
|
||||
[page-id id obj]
|
||||
{:type :add-obj :page-id page-id :id id
|
||||
:parent-id uuid/zero :frame-id uuid/zero :obj obj})
|
||||
|
||||
(defn- fills
|
||||
[color]
|
||||
[{:fill-color color :fill-opacity 1}])
|
||||
|
||||
(defn- set-fill
|
||||
"A hand edit of the fill of `id`: on a copy it marks the fill
|
||||
overridden."
|
||||
[page-id id color]
|
||||
{:type :mod-obj :page-id page-id :id id
|
||||
:operations [{:type :set :attr :fills :val (fills color)}]})
|
||||
|
||||
(defn- sync-fill
|
||||
"The fill the workspace's component sync writes on an untouched copy."
|
||||
[page-id id color]
|
||||
{:type :mod-obj :page-id page-id :id id
|
||||
:operations [{:type :set :attr :fills :val (fills color) :ignore-touched true}]})
|
||||
|
||||
(defn- copy-of
|
||||
[page-id id component-id main-id component-file]
|
||||
(add-root page-id id (component-frame id {:component-root true
|
||||
:component-id component-id
|
||||
:component-file component-file
|
||||
:shape-ref main-id})))
|
||||
|
||||
(defn- edit-component
|
||||
"Paint the component's main `color` and sync the copy `copy-id`."
|
||||
[page-id mi-id copy-id color]
|
||||
[(set-fill page-id mi-id color)
|
||||
(sync-fill page-id copy-id color)])
|
||||
|
||||
(defn- setup
|
||||
"Main holding a local component (main `:mi-id`) with one copy
|
||||
(`:copy-id`), branched once. `n` numbers the profile and the file, so
|
||||
one test can build more than one."
|
||||
[n]
|
||||
(let [profile (th/create-profile* n {:is-active true})
|
||||
file (th/create-file* n {:profile-id (:id profile)
|
||||
:project-id (:default-project-id profile)
|
||||
:is-shared false})
|
||||
main-id (:id file)
|
||||
page-id (-> (get-file profile main-id) :data :pages first)
|
||||
comp-id (uuid/random)
|
||||
mi-id (uuid/random)
|
||||
copy-id (uuid/random)]
|
||||
(save profile main-id
|
||||
[(add-root page-id mi-id (component-frame mi-id {:component-root true
|
||||
:main-instance true
|
||||
:component-id comp-id
|
||||
:component-file main-id}))
|
||||
{:type :add-component :id comp-id :name "Button" :path ""
|
||||
:main-instance-id mi-id :main-instance-page page-id}
|
||||
(copy-of page-id copy-id comp-id mi-id main-id)])
|
||||
(let [branch (:result (th/command! {::th/type :create-file-branch
|
||||
::rpc/profile-id (:id profile)
|
||||
:file-id main-id
|
||||
:name "copies"}))]
|
||||
{:profile profile
|
||||
:main-id main-id
|
||||
:page-id page-id
|
||||
:comp-id comp-id
|
||||
:mi-id mi-id
|
||||
:copy-id copy-id
|
||||
:branch-id (:id branch)
|
||||
:branch-file-id (:branch-file-id branch)})))
|
||||
|
||||
(defn- merge-branch
|
||||
[{:keys [profile branch-id]} & {:as params}]
|
||||
(th/command! (merge {::th/type :merge-file-branch
|
||||
::rpc/profile-id (:id profile)
|
||||
:branch-id branch-id}
|
||||
params)))
|
||||
|
||||
(defn- update-branch
|
||||
[{:keys [profile branch-id]} & {:as params}]
|
||||
(th/command! (merge {::th/type :update-branch-from-main
|
||||
::rpc/profile-id (:id profile)
|
||||
:branch-id branch-id}
|
||||
params)))
|
||||
|
||||
(defn- shape
|
||||
[file page-id id]
|
||||
(get-in file [:data :pages-index page-id :objects id]))
|
||||
|
||||
(defn- fill
|
||||
[file page-id id]
|
||||
(-> (shape file page-id id) :fills first :fill-color))
|
||||
|
||||
(defn- validation-codes
|
||||
[file]
|
||||
(let [libs (db/run! th/*system* #(bfc/get-resolved-file-libraries % file))]
|
||||
(into #{} (map :code) (cfv/validate-file file libs))))
|
||||
|
||||
(defn- conflict-ids
|
||||
[out]
|
||||
(into #{} (map :id) (-> out :result :conflicts)))
|
||||
|
||||
;; --- a: one side edits the component, the other adds a copy
|
||||
|
||||
(t/deftest merge-syncs-a-copy-main-added-to-the-branch-component
|
||||
(with-redefs [cf/flags (conj cf/flags :branching)]
|
||||
(let [{:keys [profile main-id branch-file-id page-id comp-id mi-id copy-id] :as ctx} (setup 1)
|
||||
new-copy (uuid/random)]
|
||||
(save profile branch-file-id (edit-component page-id mi-id copy-id "#00ff00"))
|
||||
(save profile main-id [(copy-of page-id new-copy comp-id mi-id main-id)])
|
||||
|
||||
(let [out (merge-branch ctx)]
|
||||
(t/is (nil? (:error out)))
|
||||
(t/is (= :merged (-> out :result :status))))
|
||||
|
||||
(let [main (get-file profile main-id)]
|
||||
(t/is (= "#00ff00" (fill main page-id mi-id)))
|
||||
(t/is (= "#00ff00" (fill main page-id new-copy)))
|
||||
(t/is (= "#00ff00" (fill main page-id copy-id)))
|
||||
(t/is (empty? (:touched (shape main page-id copy-id))))
|
||||
(t/is (empty? (validation-codes main)))))))
|
||||
|
||||
(t/deftest update-syncs-a-copy-the-branch-added-to-main-component
|
||||
(with-redefs [cf/flags (conj cf/flags :branching)]
|
||||
(let [{:keys [profile main-id branch-file-id page-id comp-id mi-id copy-id] :as ctx} (setup 1)
|
||||
new-copy (uuid/random)]
|
||||
(save profile main-id (edit-component page-id mi-id copy-id "#0000ff"))
|
||||
(save profile branch-file-id [(copy-of page-id new-copy comp-id mi-id branch-file-id)])
|
||||
|
||||
(let [out (update-branch ctx)]
|
||||
(t/is (nil? (:error out)))
|
||||
(t/is (= :updated (-> out :result :status))))
|
||||
|
||||
(let [branch (get-file profile branch-file-id)]
|
||||
(t/is (= "#0000ff" (fill branch page-id mi-id)))
|
||||
(t/is (= "#0000ff" (fill branch page-id new-copy)))
|
||||
(t/is (= "#0000ff" (fill branch page-id copy-id)))
|
||||
(t/is (empty? (:touched (shape branch page-id copy-id))))
|
||||
(t/is (empty? (validation-codes branch)))))))
|
||||
|
||||
;; --- b: both sides edit the component, each syncs its copy
|
||||
|
||||
(defn- both-edit-component
|
||||
[{:keys [profile main-id branch-file-id page-id mi-id copy-id]}]
|
||||
(save profile branch-file-id (edit-component page-id mi-id copy-id "#00ff00"))
|
||||
(save profile main-id (edit-component page-id mi-id copy-id "#0000ff")))
|
||||
|
||||
(t/deftest merge-of-a-component-both-sides-edited-conflicts-on-the-component-only
|
||||
(with-redefs [cf/flags (conj cf/flags :branching)]
|
||||
(doseq [[side color] [[:branch "#00ff00"] [:main "#0000ff"]]]
|
||||
(t/testing (str "resolved to " side)
|
||||
(let [{:keys [profile main-id page-id mi-id copy-id] :as ctx} (setup (if (= :branch side) 1 2))]
|
||||
(both-edit-component ctx)
|
||||
|
||||
(let [out (merge-branch ctx)]
|
||||
(t/is (= :conflicts (-> out :result :status)))
|
||||
(t/is (= #{mi-id} (conflict-ids out))))
|
||||
|
||||
(let [out (merge-branch ctx :resolutions {mi-id side})]
|
||||
(t/is (nil? (:error out)))
|
||||
(t/is (= :merged (-> out :result :status))))
|
||||
|
||||
(let [main (get-file profile main-id)]
|
||||
(t/is (= color (fill main page-id mi-id)))
|
||||
(t/is (= color (fill main page-id copy-id)))
|
||||
(t/is (empty? (:touched (shape main page-id copy-id))))
|
||||
(t/is (empty? (validation-codes main)))))))))
|
||||
|
||||
(t/deftest update-of-a-component-both-sides-edited-conflicts-on-the-component-only
|
||||
(with-redefs [cf/flags (conj cf/flags :branching)]
|
||||
(doseq [[side color] [[:branch "#00ff00"] [:main "#0000ff"]]]
|
||||
(t/testing (str "resolved to " side)
|
||||
(let [{:keys [profile branch-file-id page-id mi-id copy-id] :as ctx} (setup (if (= :branch side) 1 2))]
|
||||
(both-edit-component ctx)
|
||||
|
||||
(let [out (update-branch ctx)]
|
||||
(t/is (= :conflicts (-> out :result :status)))
|
||||
(t/is (= #{mi-id} (conflict-ids out))))
|
||||
|
||||
(let [out (update-branch ctx :resolutions {mi-id side})]
|
||||
(t/is (nil? (:error out)))
|
||||
(t/is (= :updated (-> out :result :status))))
|
||||
|
||||
(let [branch (get-file profile branch-file-id)]
|
||||
(t/is (= color (fill branch page-id mi-id)))
|
||||
(t/is (= color (fill branch page-id copy-id)))
|
||||
(t/is (empty? (:touched (shape branch page-id copy-id))))
|
||||
(t/is (empty? (validation-codes branch)))))))))
|
||||
|
||||
;; --- c: one side overrides the copy, the other edits the component
|
||||
|
||||
(t/deftest merge-keeps-the-branch-override-of-a-copy
|
||||
(with-redefs [cf/flags (conj cf/flags :branching)]
|
||||
(let [{:keys [profile main-id branch-file-id page-id mi-id copy-id] :as ctx} (setup 1)]
|
||||
(save profile branch-file-id [(set-fill page-id copy-id "#ff0000")])
|
||||
(save profile main-id (edit-component page-id mi-id copy-id "#0000ff"))
|
||||
|
||||
(let [out (merge-branch ctx)]
|
||||
(t/is (nil? (:error out)))
|
||||
(t/is (= :merged (-> out :result :status))))
|
||||
|
||||
(let [main (get-file profile main-id)]
|
||||
(t/is (= "#0000ff" (fill main page-id mi-id)))
|
||||
(t/is (= "#ff0000" (fill main page-id copy-id)))
|
||||
(t/is (contains? (:touched (shape main page-id copy-id)) :fill-group))
|
||||
(t/is (empty? (validation-codes main)))))))
|
||||
|
||||
(t/deftest update-keeps-main-override-of-a-copy
|
||||
(with-redefs [cf/flags (conj cf/flags :branching)]
|
||||
(let [{:keys [profile main-id branch-file-id page-id mi-id copy-id] :as ctx} (setup 1)]
|
||||
(save profile main-id [(set-fill page-id copy-id "#ff0000")])
|
||||
(save profile branch-file-id (edit-component page-id mi-id copy-id "#00ff00"))
|
||||
|
||||
(let [out (update-branch ctx)]
|
||||
(t/is (nil? (:error out)))
|
||||
(t/is (= :updated (-> out :result :status))))
|
||||
|
||||
(let [branch (get-file profile branch-file-id)]
|
||||
(t/is (= "#00ff00" (fill branch page-id mi-id)))
|
||||
(t/is (= "#ff0000" (fill branch page-id copy-id)))
|
||||
(t/is (contains? (:touched (shape branch page-id copy-id)) :fill-group))
|
||||
(t/is (empty? (validation-codes branch)))))))
|
||||
|
||||
;; --- d: both sides override the copy differently
|
||||
|
||||
(t/deftest merge-of-a-copy-both-sides-overrode-is-a-conflict
|
||||
(with-redefs [cf/flags (conj cf/flags :branching)]
|
||||
(let [{:keys [profile main-id branch-file-id page-id copy-id] :as ctx} (setup 1)]
|
||||
(save profile branch-file-id [(set-fill page-id copy-id "#ff0000")])
|
||||
(save profile main-id [(set-fill page-id copy-id "#ffff00")])
|
||||
|
||||
(let [out (merge-branch ctx)]
|
||||
(t/is (= :conflicts (-> out :result :status)))
|
||||
(t/is (= #{copy-id} (conflict-ids out)))))))
|
||||
|
||||
(t/deftest update-of-a-copy-both-sides-overrode-is-a-conflict
|
||||
(with-redefs [cf/flags (conj cf/flags :branching)]
|
||||
(let [{:keys [profile main-id branch-file-id page-id copy-id] :as ctx} (setup 1)]
|
||||
(save profile branch-file-id [(set-fill page-id copy-id "#ff0000")])
|
||||
(save profile main-id [(set-fill page-id copy-id "#ffff00")])
|
||||
|
||||
(let [out (update-branch ctx)]
|
||||
(t/is (= :conflicts (-> out :result :status)))
|
||||
(t/is (= #{copy-id} (conflict-ids out)))))))
|
||||
@ -21,8 +21,12 @@
|
||||
reported payload names the DOCUMENTS either way (see `compute-merge`)."
|
||||
(:require
|
||||
[app.common.data :as d]
|
||||
[app.common.files.changes :as cfc]
|
||||
[app.common.files.changes-builder :as pcb]
|
||||
[app.common.files.helpers :as cfh]
|
||||
[app.common.logic.libraries :as cll]
|
||||
[app.common.types.component :as ctk]
|
||||
[app.common.types.components-list :as ctkl]
|
||||
[app.common.types.file :as ctf]
|
||||
[app.common.types.pages-list :as ctpl]
|
||||
[app.common.types.tokens-lib :as ctob]
|
||||
@ -334,6 +338,138 @@
|
||||
{:changes (into [] (mapcat :changes) results)
|
||||
:conflicts (into [] (mapcat :conflicts) results)})
|
||||
|
||||
;; --- Component copies ---
|
||||
;;
|
||||
;; A copy's attributes follow its component through Penpot's component
|
||||
;; sync unless the copy overrides them by hand, which the copy records as
|
||||
;; a sync group in its `:touched` set (`app.common.types.component`). The
|
||||
;; merge honours that per attribute of a copy both sides changed:
|
||||
;; - no side overrode it: it follows the merged component, so the
|
||||
;; target keeps its value and the sync run after the integration
|
||||
;; (`sync-copies`) sets it;
|
||||
;; - one side overrode it: that side's value wins;
|
||||
;; - both sides overrode it: a real conflict.
|
||||
;; The sides are the comparison roles: `:main` is `theirs` (the target),
|
||||
;; `:branch` is `ours` (the source).
|
||||
|
||||
(def ^:private text-content-groups
|
||||
"The groups a text `:content` change marks on top of `:content-group`
|
||||
(`app.common.types.text/get-diff-type`)."
|
||||
#{:text-content-text :text-content-attribute :text-content-structure})
|
||||
|
||||
(defn- overrode?
|
||||
"True when `touched` holds the sync `group` of `attr` and `shape` moved
|
||||
the attr off its `base` value: the side overrode it by hand."
|
||||
[group touched base shape attr]
|
||||
(and (contains? touched group)
|
||||
(not= (get base attr) (get shape attr))))
|
||||
|
||||
(defn- copy-conflict-sides
|
||||
"How the copy rule settles a shape both sides changed differently, or
|
||||
nil when the shape is not such a copy. `sb`/`st`/`so` are the
|
||||
classification shapes (stripped, see `strip-shapes`) of the base,
|
||||
`theirs` and `ours`; `t`/`o` the full ones, which carry `:touched`.
|
||||
|
||||
Returns `{:auto {attr side} :open #{attr}}` over the attrs that differ
|
||||
between the two sides: `:auto` holds the side the rule picks for each
|
||||
sync attr no more than one side overrode (`:main` when none did), and
|
||||
`:open` the attrs left to the user, the ones both sides overrode and
|
||||
the ones outside every sync group."
|
||||
[sb st so t o]
|
||||
(when (and (some? sb) (some? st) (some? so)
|
||||
(ctk/in-component-copy? t)
|
||||
(ctk/in-component-copy? o)
|
||||
(not= sb st) (not= sb so))
|
||||
(let [sb (strip-nil-attrs sb)
|
||||
st (strip-nil-attrs st)
|
||||
so (strip-nil-attrs so)
|
||||
tt (set (:touched t))
|
||||
ot (set (:touched o))]
|
||||
(when (and (not= sb st) (not= sb so) (not= st so))
|
||||
(reduce-kv
|
||||
(fn [acc attr _]
|
||||
(let [group (ctk/resolve-sync-group (:type so) attr)
|
||||
t? (and (some? group) (overrode? group tt sb st attr))
|
||||
o? (and (some? group) (overrode? group ot sb so attr))]
|
||||
(cond
|
||||
(or (nil? group) (and t? o?)) (update acc :open conj attr)
|
||||
o? (assoc-in acc [:auto attr] :branch)
|
||||
:else (assoc-in acc [:auto attr] :main))))
|
||||
{:auto {} :open #{}}
|
||||
(shallow-attr-diff st so))))))
|
||||
|
||||
(defn- settle-copy-conflicts
|
||||
"Apply the copy rule to the `:modify-modify` shape conflicts of one
|
||||
page's comparison `res`. A conflict whose attrs the rule settles
|
||||
entirely leaves `:conflicts`; it comes back as a `:modified` change
|
||||
over the attrs the source wins, if any, since the merge applies them.
|
||||
A conflict with open attrs stays, its `:changed-attrs` narrowed to
|
||||
them: the merge applies the settled ones whatever the user picks."
|
||||
[{:keys [conflicts] :as res} sbo sto soo to oo]
|
||||
(reduce
|
||||
(fn [acc {:keys [id reason changed-attrs] :as c}]
|
||||
(if-let [{:keys [auto open]} (when (= :modify-modify reason)
|
||||
(copy-conflict-sides (get sbo id) (get sto id) (get soo id)
|
||||
(get to id) (get oo id)))]
|
||||
(let [won (into [] (keep (fn [[attr side]] (when (= :branch side) attr))) auto)]
|
||||
(cond
|
||||
(seq open)
|
||||
(update acc :conflicts conj (assoc c :changed-attrs (select-keys changed-attrs open)))
|
||||
|
||||
(seq won)
|
||||
(update acc :changes conj (-> c
|
||||
(dissoc :reason :base :main :branch)
|
||||
(assoc :status :modified
|
||||
:changed-attrs (select-keys changed-attrs won))))
|
||||
|
||||
:else
|
||||
acc))
|
||||
(update acc :conflicts conj c)))
|
||||
(assoc res :conflicts [])
|
||||
conflicts))
|
||||
|
||||
(defn- copy-resolution
|
||||
"The per-attr resolution of a copy the rule settled (`sides`, see
|
||||
`copy-conflict-sides`): the user's `res` over the open attrs, a side
|
||||
keyword standing for every one of them, and the rule's side for the
|
||||
rest."
|
||||
[{:keys [auto open]} res]
|
||||
(merge (cond
|
||||
(keyword? res) (zipmap open (repeat res))
|
||||
(map? res) res
|
||||
:else {})
|
||||
auto))
|
||||
|
||||
(defn- copy-touched-ops
|
||||
"Operations that write the copy `t` from the source shape `o` without
|
||||
marking overrides the source does not have: every `:set` carries
|
||||
`:ignore-touched`, and one `:set-touched` gives the copy the source's
|
||||
sync groups for the attrs the operations take (all of them under
|
||||
`whole?`, when the source shape wins entirely) and its own for the
|
||||
rest. A plain `:set` on a copy marks the attr's group touched, which
|
||||
would freeze an attr the source merely synced from its component."
|
||||
[ops t o whole?]
|
||||
(let [ops (into []
|
||||
(comp (remove #(= :touched (:attr %)))
|
||||
(map #(assoc % :ignore-touched true)))
|
||||
ops)
|
||||
taken (into #{}
|
||||
(mapcat (fn [{:keys [attr]}]
|
||||
(when-let [group (ctk/resolve-sync-group (:type o) attr)]
|
||||
(if (and (= :content attr) (= :text (:type o)))
|
||||
(conj text-content-groups group)
|
||||
[group]))))
|
||||
ops)
|
||||
own (set (:touched t))
|
||||
touched (if whole?
|
||||
(set (:touched o))
|
||||
(into (into #{} (remove taken) own)
|
||||
(filter taken)
|
||||
(:touched o)))]
|
||||
(cond-> ops
|
||||
(and (seq ops) (not= touched own))
|
||||
(conj {:type :set-touched :touched (not-empty touched)}))))
|
||||
|
||||
(defn- presence-only
|
||||
"Keep only the existence-level results of a CONTENT-valued presence diff:
|
||||
added/deleted changes and delete conflicts. Content modifications of
|
||||
@ -881,7 +1017,11 @@
|
||||
:deleted (assoc e :shape (or (get bo (:id e))
|
||||
(get to (:id e))))
|
||||
e))
|
||||
passed (tree-policy-pass res bo to oo sbo sto reorders pid modes)]
|
||||
passed (-> (tree-policy-pass res bo to oo sbo sto reorders pid modes)
|
||||
;; a copy's attrs no more than one
|
||||
;; side overrode are not conflicts
|
||||
;; (see `copy-conflict-sides`)
|
||||
(settle-copy-conflicts sbo sto soo to oo))]
|
||||
{:changes (mapv (fn [e]
|
||||
;; a `:refuse` entry names the
|
||||
;; container it could not place
|
||||
@ -1409,7 +1549,14 @@
|
||||
|
||||
The `:tokens-lib` is diffed as well (`diff-tokens`), so token changes
|
||||
are reported here with their own kinds. The file's tokens source is
|
||||
reported under kind `:tokens-source`, entity id `:tokens-source`."
|
||||
reported under kind `:tokens-source`, entity id `:tokens-source`.
|
||||
|
||||
A component copy both sides changed is settled per attr by the copy
|
||||
rule (see `copy-conflict-sides`): only the attrs both sides overrode
|
||||
by hand, and the ones outside every sync group, stay in its conflict.
|
||||
A copy the rule settles entirely leaves `:conflicts`, and comes back
|
||||
as a `:modified` change over the attrs the source overrode, which the
|
||||
merge takes from it."
|
||||
([base main branch dir] (compute-merge* base main branch dir nil))
|
||||
([base main branch dir {:keys [only-pages]}]
|
||||
(compute-merge* base main branch dir only-pages)))
|
||||
@ -1771,20 +1918,39 @@
|
||||
own-drop)
|
||||
keep? (complement drop-ids)
|
||||
|
||||
;; a copy both sides changed takes, per attr, the side the copy
|
||||
;; rule picks (`copy-conflict-sides`) and the user's resolution
|
||||
;; only over the attrs the rule leaves open. Only the attr pass
|
||||
;; reads these; moves and restores keep the user's entity choice
|
||||
shape-res (reduce (fn [acc id]
|
||||
(if-let [sides (copy-conflict-sides (get sbo id) (get sto id) (get soo id)
|
||||
(get to id) (get oo id))]
|
||||
(assoc acc id (copy-resolution sides (get resolutions id)))
|
||||
acc))
|
||||
resolutions
|
||||
(filter keep? (keys sbo)))
|
||||
|
||||
;; modifications + deletions (additions handled below, ordered)
|
||||
mod-del
|
||||
(->> (flat-changes (into {} (filter (fn [[id _]] (keep? id))) sbo)
|
||||
(into {} (filter (fn [[id _]] (keep? id))) sto)
|
||||
(into {} (filter (fn [[id _]] (keep? id))) soo)
|
||||
resolutions
|
||||
shape-res
|
||||
(fn [_ _] nil)
|
||||
(fn [id _]
|
||||
(let [res (get resolutions id)
|
||||
ops (if (and (map? res)
|
||||
(not= (get sto id) (get sbo id))) ; genuine conflict -> per-attr
|
||||
(shape-resolved-ops (get sto id) (get soo id)
|
||||
(get to id) (get oo id) res)
|
||||
(shape-set-ops (get to id) (get oo id)))]
|
||||
(let [res (get shape-res id)
|
||||
t (get to id)
|
||||
o (get oo id)
|
||||
whole? (not (and (map? res)
|
||||
(not= (get sto id) (get sbo id)))) ; genuine conflict -> per-attr
|
||||
ops (if whole?
|
||||
(shape-set-ops t o)
|
||||
(shape-resolved-ops (get sto id) (get soo id) t o res))
|
||||
;; a copy keeps the overrides of the side
|
||||
;; each attr comes from, never new ones
|
||||
ops (if (or (ctk/in-component-copy? t) (ctk/in-component-copy? o))
|
||||
(copy-touched-ops ops t o whole?)
|
||||
ops)]
|
||||
(when (seq ops)
|
||||
{:type :mod-obj :page-id page-id :id id :operations ops})))
|
||||
(fn [id]
|
||||
@ -1992,7 +2158,15 @@
|
||||
shape it reparented there), so no emitted change can reference a parent
|
||||
the merge does not create; the alternative policy, refusing the
|
||||
exclusion and naming those shapes instead, stays documented and is not
|
||||
the working default."
|
||||
the working default.
|
||||
|
||||
A component copy takes, per attr, the side the copy rule picks
|
||||
(`copy-conflict-sides`) whatever `resolutions` says, which only
|
||||
decides the attrs the rule leaves open. The operations written on a
|
||||
copy never mark an override the source does not have
|
||||
(`copy-touched-ops`). The attrs the rule leaves at the target's value
|
||||
follow the component through `sync-copies`, which the caller runs
|
||||
after applying these changes."
|
||||
([base main branch]
|
||||
(compute-changes base main branch {}))
|
||||
([base main branch resolutions]
|
||||
@ -2335,3 +2509,57 @@
|
||||
set-presence set-rename-changes set-order-changes
|
||||
token-vals themes
|
||||
active-changes))})))
|
||||
|
||||
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
|
||||
;; COPIES FOLLOW THE INTEGRATED COMPONENTS
|
||||
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
|
||||
|
||||
(defn changed-components
|
||||
"Ids of the local components whose main instance `changes` modify,
|
||||
read against `data`, the document before they apply: the detection the
|
||||
workspace runs on every commit to sync copies
|
||||
(`app.common.files.changes/components-changed`). A deleted component
|
||||
has no copies left to follow it and is left out."
|
||||
[data changes]
|
||||
(into #{}
|
||||
(comp (mapcat #(cfc/components-changed data %))
|
||||
(filter #(some? (ctkl/get-component data %))))
|
||||
changes))
|
||||
|
||||
(defn sync-copies
|
||||
"Sync the copies of the local components `component-ids` in the
|
||||
document `data` of the file `file-id`, with Penpot's component sync
|
||||
(`app.common.logic.libraries/generate-sync-file-changes`): every attr a
|
||||
copy does not override takes its component's value. A sync that writes
|
||||
inside the main of another local component (a nested copy) makes that
|
||||
component sync in turn, as the workspace does, until no sync changes
|
||||
anything.
|
||||
|
||||
Returns `{:data <synced data> :changes [<applied changes>]}`. The
|
||||
merge and the update run it after integrating, so the copies the
|
||||
comparison left with the target's value, or never saw (one added on
|
||||
the other side), match the integrated components."
|
||||
[file-id data component-ids]
|
||||
(let [limit (inc (count (:components data)))]
|
||||
(loop [pending (set component-ids)
|
||||
data data
|
||||
out []
|
||||
round 0]
|
||||
(if (or (empty? pending) (> round limit))
|
||||
{:data data :changes out}
|
||||
(let [[data out queued]
|
||||
(reduce (fn [[data out queued] component-id]
|
||||
(let [libraries {file-id {:id file-id :data data}}
|
||||
redo (-> (pcb/empty-changes)
|
||||
(cll/generate-sync-file-changes nil :components file-id
|
||||
component-id file-id
|
||||
libraries file-id)
|
||||
:redo-changes)]
|
||||
(if (empty? redo)
|
||||
[data out queued]
|
||||
[(cfc/process-changes data redo)
|
||||
(into out redo)
|
||||
(into queued (changed-components data redo))])))
|
||||
[data out #{}]
|
||||
pending)]
|
||||
(recur queued data out (inc round)))))))
|
||||
|
||||
@ -8,7 +8,13 @@
|
||||
(:require
|
||||
[app.common.files.branch-merge :as bm]
|
||||
[app.common.files.changes :as cfc]
|
||||
[app.common.files.changes-builder :as pcb]
|
||||
[app.common.files.tokens :as cfo]
|
||||
[app.common.logic.shapes :as cls]
|
||||
[app.common.test-helpers.compositions :as tho]
|
||||
[app.common.test-helpers.files :as thf]
|
||||
[app.common.test-helpers.ids-map :as thi]
|
||||
[app.common.test-helpers.shapes :as ths]
|
||||
[app.common.types.page :as ctp]
|
||||
[app.common.types.shape :as cts]
|
||||
[app.common.types.shape-tree :as ctst]
|
||||
@ -1897,3 +1903,125 @@
|
||||
(t/testing "replaying the squashed net over the new base reproduces the branch"
|
||||
(t/is (= (tree-objects updated)
|
||||
(tree-objects (cfc/process-changes new-base net)))))))
|
||||
|
||||
;; --- copies follow their component (the copy rule) ---
|
||||
|
||||
(defn- copy-shape
|
||||
"A copy rect of the main `:m`: `fill` and an optional `:touched` set."
|
||||
[fill & {:keys [touched opacity]}]
|
||||
(cond-> {:id :k :name "K" :type :rect :shape-ref :m :fills [{:fill-color fill}]}
|
||||
(some? touched) (assoc :touched touched)
|
||||
(some? opacity) (assoc :opacity opacity)))
|
||||
|
||||
(defn- copy-ops
|
||||
"The operations `compute-changes` writes on the copy `:k`."
|
||||
[changes]
|
||||
(->> changes
|
||||
(filter #(and (= :mod-obj (:type %)) (= :k (:id %))))
|
||||
(mapcat :operations)
|
||||
(vec)))
|
||||
|
||||
(t/deftest copy-attr-no-side-overrode-follows-the-component
|
||||
;; both sides synced K from their own edit of the component: the attr
|
||||
;; follows the merged component, so the target keeps its value until
|
||||
;; the sync after the merge sets it
|
||||
(let [base (mkdata {:k (copy-shape "red")})
|
||||
main (mkdata {:k (copy-shape "blue")})
|
||||
branch (mkdata {:k (copy-shape "green")})
|
||||
r (bm/compute-merge base main branch :branch->main)]
|
||||
(t/is (empty? (:conflicts r)))
|
||||
(t/is (empty? (:changes r)))
|
||||
(t/is (empty? (copy-ops (:changes (bm/compute-changes base main branch {} r)))))))
|
||||
|
||||
(t/deftest copy-attr-the-source-overrode-wins
|
||||
(let [base (mkdata {:k (copy-shape "red")})
|
||||
main (mkdata {:k (copy-shape "blue")})
|
||||
branch (mkdata {:k (copy-shape "green" :touched #{:fill-group})})
|
||||
r (bm/compute-merge base main branch :branch->main)
|
||||
ops (copy-ops (:changes (bm/compute-changes base main branch {} r)))]
|
||||
(t/is (empty? (:conflicts r)))
|
||||
(t/testing "the summary reports the override as a change the merge applies"
|
||||
(t/is (= [[:k :modified #{:fills}]]
|
||||
(mapv (juxt :id :status (comp set keys :changed-attrs)) (:changes r)))))
|
||||
(t/testing "the copy takes the value and the override mark"
|
||||
(t/is (= [{:type :set :attr :fills :val [{:fill-color "green"}] :ignore-touched true}
|
||||
{:type :set-touched :touched #{:fill-group}}]
|
||||
ops)))))
|
||||
|
||||
(t/deftest copy-attr-the-target-overrode-stays
|
||||
(let [base (mkdata {:k (copy-shape "red")})
|
||||
main (mkdata {:k (copy-shape "blue" :touched #{:fill-group})})
|
||||
branch (mkdata {:k (copy-shape "green")})
|
||||
r (bm/compute-merge base main branch :branch->main)]
|
||||
(t/is (empty? (:conflicts r)))
|
||||
(t/is (empty? (:changes r)))
|
||||
(t/is (empty? (copy-ops (:changes (bm/compute-changes base main branch {} r)))))))
|
||||
|
||||
(t/deftest copy-attr-both-sides-overrode-is-a-conflict
|
||||
(let [base (mkdata {:k (copy-shape "red")})
|
||||
main (mkdata {:k (copy-shape "blue" :touched #{:fill-group})})
|
||||
branch (mkdata {:k (copy-shape "green" :touched #{:fill-group})})
|
||||
r (bm/compute-merge base main branch :branch->main)]
|
||||
(t/is (= [:k] (mapv :id (:conflicts r))))
|
||||
(t/is (= #{:fills} (set (keys (:changed-attrs (first (:conflicts r)))))))
|
||||
(t/testing "resolved to the branch, the copy takes its value and override"
|
||||
(t/is (= [{:type :set :attr :fills :val [{:fill-color "green"}] :ignore-touched true}]
|
||||
(copy-ops (:changes (bm/compute-changes base main branch {:k :branch} r))))))))
|
||||
|
||||
(t/deftest copy-conflict-narrows-to-the-attrs-both-sides-overrode
|
||||
;; both overrode the fill; only the branch overrode the opacity: the
|
||||
;; conflict asks about the fill, and the opacity comes from the branch
|
||||
;; whatever the user picks
|
||||
(let [base (mkdata {:k (copy-shape "red" :opacity 1)})
|
||||
main (mkdata {:k (copy-shape "blue" :opacity 1 :touched #{:fill-group})})
|
||||
branch (mkdata {:k (copy-shape "green" :opacity 0.5
|
||||
:touched #{:fill-group :layer-effects-group})})
|
||||
r (bm/compute-merge base main branch :branch->main)
|
||||
ops (copy-ops (:changes (bm/compute-changes base main branch {:k :main} r)))]
|
||||
(t/is (= #{:fills} (set (keys (:changed-attrs (first (:conflicts r)))))))
|
||||
(t/is (= [{:type :set :attr :opacity :val 0.5 :ignore-touched true}
|
||||
{:type :set-touched :touched #{:fill-group :layer-effects-group}}]
|
||||
ops))))
|
||||
|
||||
(t/deftest copy-rule-holds-updating-from-main
|
||||
;; the update direction: main is the source, the branch the target
|
||||
(let [base (mkdata {:k (copy-shape "red")})
|
||||
main (mkdata {:k (copy-shape "blue" :touched #{:fill-group})})
|
||||
branch (mkdata {:k (copy-shape "green")})
|
||||
r (bm/compute-merge base main branch :main->branch)
|
||||
ops (copy-ops (:changes (bm/compute-changes base branch main {} r)))]
|
||||
(t/is (empty? (:conflicts r)))
|
||||
(t/is (= [{:type :set :attr :fills :val [{:fill-color "blue"}] :ignore-touched true}
|
||||
{:type :set-touched :touched #{:fill-group}}]
|
||||
ops))))
|
||||
|
||||
(t/deftest clean-copy-change-marks-no-override
|
||||
;; the branch synced K from its component and main left K alone: the
|
||||
;; merge writes the value without marking it overridden
|
||||
(let [base (mkdata {:k (copy-shape "red")})
|
||||
branch (mkdata {:k (copy-shape "green")})]
|
||||
(t/is (= [{:type :set :attr :fills :val [{:fill-color "green"}] :ignore-touched true}]
|
||||
(copy-ops (:changes (bm/compute-changes base base branch)))))))
|
||||
|
||||
(t/deftest sync-copies-follows-nested-components
|
||||
;; a change to component1's main syncs its nested copy inside
|
||||
;; component2's main, which in turn syncs component2's copy
|
||||
(thi/test-fixture
|
||||
(fn []
|
||||
(let [file (-> (thf/sample-file :file1)
|
||||
(tho/add-nested-component-with-copy :component1 :main1-root :main1-child
|
||||
:component2 :main2-root :nested2-head
|
||||
:copy2-root))
|
||||
page (thf/current-page file)
|
||||
changes (cls/generate-update-shapes (pcb/empty-changes nil (:id page))
|
||||
#{(:id (ths/get-shape file :main1-child))}
|
||||
#(assoc % :fills (ths/sample-fills-color :fill-color "#fabada"))
|
||||
(:objects page)
|
||||
{})
|
||||
edited (thf/apply-changes file changes)
|
||||
scope (bm/changed-components (:data file) (:redo-changes changes))
|
||||
{:keys [data]} (bm/sync-copies (:id edited) (:data edited) scope)
|
||||
synced (assoc edited :data data)]
|
||||
(t/is (= #{(thi/id :component1)} scope))
|
||||
(t/is (= "#fabada" (tho/bottom-fill-color synced :nested2-head)))
|
||||
(t/is (= "#fabada" (tho/bottom-fill-color synced :copy2-root)))))))
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user