github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-10-29 05:50:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

✨ Add merge reviews and tidy up the branching workspace

Browse Source
This commit is contained in:
elhombretecla 2026-10-23 10:32:21 +02:00
parent dcedd40a42
commit c1dec5d4f0
27 changed files with 1555 additions and 501 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
backend/resources/app/email/review-request/en.html
View File

@ -186,7 +186,7 @@
<td align="left" style="font-size:0px;padding:10px 25px;word-break:break-word;">
<div
style="font-family:Source Sans Pro, sans-serif;font-size:16px;line-height:150%;text-align:left;color:#000000;">
<span style="font-weight:bold;">{{ source-user }}</span> has requested your review on the pull request "{{ pull-request-title }}" ({{ pull-request-route }}).</div>
<span style="font-weight:bold;">{{ source-user }}</span> has requested your review on the design review "{{ pull-request-title }}" ({{ pull-request-route }}).</div>
</td>
</tr>
<tr>

2
backend/resources/app/email/review-request/en.txt
View File

@ -1,6 +1,6 @@
Hello {{name|abbreviate:25}}!
{{ source-user }} has requested your review on the pull request "{{ pull-request-title }}" ({{ pull-request-route }}).
{{ source-user }} has requested your review on the design review "{{ pull-request-title }}" ({{ pull-request-route }}).
--

5
backend/src/app/migrations.clj
View File

@ -542,7 +542,10 @@
:fn (mg/resource "app/migrations/sql/0804-add-file-branch-change-changes-count.sql")}
{:name "0805-stamp-file-branch-change-changes-count"
:fn mg0805/migrate}])
:fn mg0805/migrate}
{:name "0806-add-file-branch-merge-table"
:fn (mg/resource "app/migrations/sql/0806-add-file-branch-merge-table.sql")}])
(defn apply-migrations!
[pool name migrations]

33
backend/src/app/migrations/sql/0806-add-file-branch-merge-table.sql Normal file
View File

@ -0,0 +1,33 @@
-- The record of every merge of a branch into its source file (main): who
-- merged it, when, the comment they left and, when the merge accepted a
-- design review, that review. A merge deletes the branch, and the objects
-- GC later deletes its file, which takes the `file_branch` and
-- `file_pull_request` rows with it through their cascades. So this table
-- keeps its own copy of the names it shows and holds no foreign key to
-- either table: the record outlives both. It goes away with main.
CREATE TABLE file_branch_merge (
id uuid PRIMARY KEY,
created_at timestamptz NOT NULL DEFAULT clock_timestamp(),
-- main: the file the branch was merged into
file_id uuid NOT NULL REFERENCES file(id) ON DELETE CASCADE DEFERRABLE,
-- the merged branch, by id and by the name it had when it was merged
file_branch_id uuid NOT NULL,
branch_name text NOT NULL,
merged_by uuid NULL REFERENCES profile(id) ON DELETE SET NULL DEFERRABLE,
comment text NULL,
-- the design review the merge accepted, if any
pull_request_id uuid NULL,
pull_request_title text NULL
);
CREATE INDEX file_branch_merge__file_id__created_at__idx
ON file_branch_merge(file_id, created_at DESC);
CREATE INDEX file_branch_merge__merged_by__idx
ON file_branch_merge(merged_by);

120
backend/src/app/rpc/commands/files_branch.clj
View File

@ -403,6 +403,36 @@
:profile-id profile-id
:session-id session-id}}))
;; --- Helpers: merge record
(def ^:private sql:get-open-branch-pull-request
"SELECT id, title
FROM file_pull_request
WHERE file_branch_id = ?
AND status = 'open'
AND deleted_at IS NULL")
(defn- record-merge!
"Insert the record of a merge of `branch` into its source file (see the
`file_branch_merge` migration). The open pull request of the branch is
the design review the merge accepts, so it is read here, before the
merge closes it. A blank comment is stored as no comment. Must run
inside the merge transaction."
[conn {:keys [id source-file-id name]} {:keys [profile-id comment created-at]}]
(let [pr (db/exec-one! conn [sql:get-open-branch-pull-request id])
comment (when-not (str/blank? comment) (str/trim comment))]
(db/insert! conn :file-branch-merge
{:id (uuid/next)
:created-at created-at
:file-id source-file-id
:file-branch-id id
:branch-name name
:merged-by profile-id
:comment comment
:pull-request-id (:id pr)
:pull-request-title (:title pr)}
{::db/return-keys false})))
;; --- COMMAND: create-file-branch
(defn- check-branch-name!
@ -593,6 +623,7 @@
JOIN file AS sf ON (sf.id = fb.source_file_id)
WHERE fb.source_file_id = ?
AND fb.deleted_at IS NULL
AND fb.status != 'merged'
AND (?::boolean OR fb.status = 'open')
ORDER BY fb.created_at DESC")
@ -958,11 +989,9 @@
;; deselected, which the merge resolves to the target side
[:excluded {:optional true} [::sm/set :string]]
[:expected-main-revn {:optional true} ::sm/int]
;; when false (the default) the branch and its file are logically
;; deleted right after a successful merge, in the SAME transaction (so
;; merged copies do not pile up); when true the branch is kept as
;; "merged", staying readable with status merged until it is deleted.
[:keep-branch {:optional true} ::sm/boolean]])
;; what the user says about the merge; kept in its record
;; (`file_branch_merge`), shown in the list of merges of main
[:comment {:optional true} [:string {:max 4000}]]])
(sv/defmethod ::merge-file-branch
"Merge a branch into its source file (main).
@ -974,15 +1003,16 @@
kinds the engine cannot translate (`bm/unsupported-kinds`) make it return
`{:status :unsupported}` so no change is silently dropped. Otherwise it
takes a safety snapshot of main, applies the merge to main through the
production change pipeline, marks the branch merged and notifies open
clients via msgbus."
production change pipeline, records the merge with its `comment`
(`file_branch_merge`), deletes the branch and its file and notifies
open clients via msgbus."
{::doc/added "2.16"
::webhooks/event? true
::sm/params schema:merge-file-branch
::climit/id [[:merge-file-branch/by-profile ::rpc/profile-id]
[:merge-file-branch/global]]}
[{:keys [::mbus/msgbus] :as cfg}
{:keys [::rpc/profile-id ::rpc/session-id branch-id resolutions expected-main-revn keep-branch excluded]}]
{:keys [::rpc/profile-id ::rpc/session-id branch-id resolutions expected-main-revn comment excluded]}]
(bp/with-policies
(check-branching-enabled!)
(let [branch (db/get* cfg :file-branch {:id branch-id})]
@ -1011,7 +1041,7 @@
;; advisory lock the normal update-file path takes; stable order
;; to avoid deadlocks): without the branch lock, edits saved to
;; the branch while the merge runs would be silently lost when
;; the branch is marked merged (and possibly deleted) below.
;; the branch is marked merged and deleted below.
(run! (partial db/xact-lock! conn) (sort [main-id branch-file-id]))
;; the row read before the lock is stale by now: a competing
@ -1073,15 +1103,16 @@
finish-branch!
(fn [ts]
;; mark merged. The pinned base snapshot stays for a
;; KEPT branch (its state is derived from that
;; snapshot and the op log, so releasing the pin
;; would leave it unreadable once the snapshot GC
;; runs); delete-branch! releases it on the delete
;; path. Unless the user chose to keep it the branch
;; and its file are deleted in this same transaction
;; record the merge, mark the branch merged and
;; delete it and its file in this same transaction
;; (no client-driven second call, no window where a
;; crash leaves a stale copy).
;; crash leaves a stale copy); delete-branch!
;; releases the pinned base snapshot. The record is
;; written first: it reads the open pull request the
;; lines below close.
(record-merge! conn branch {:profile-id profile-id
:comment comment
:created-at ts})
(db/update! conn :file-branch
{:status "merged"
:merged-at ts
@ -1097,10 +1128,9 @@
{:profile-id profile-id
:status "merged"
:deleted-at (ct/in-future delay)})
(when-not keep-branch
(vswap! messages conj
(delete-branch! cfg branch {:profile-id profile-id
:session-id session-id})))))]
(vswap! messages conj
(delete-branch! cfg branch {:profile-id profile-id
:session-id session-id}))))]
(cond
(seq unresolved)
(audited {:status :conflicts :conflicts conflicts :main-revn (:revn main-file)}
@ -1784,6 +1814,7 @@
"SELECT count(*) AS count
FROM file_branch AS fb
WHERE fb.deleted_at IS NULL
AND fb.status != 'merged'
AND fb.source_file_id = coalesce((SELECT b.source_file_id
FROM file_branch AS b
WHERE b.branch_file_id = ?
@ -1791,8 +1822,8 @@
?)")
(sv/defmethod ::get-file-branch-count
"How many branches the file has, archived and merged ones included, as
the listing with `:include-archived` returns them. When the file is
"How many branches the file has, archived ones included, as the
listing with `:include-archived` returns them. When the file is
itself a branch, the branches of its source (main) are counted.
The workspace header asks this on every file open to decide whether to
@ -1810,6 +1841,49 @@
(select-keys [:count]))))
{:count 0}))
;; --- COMMAND QUERY: get-file-branch-merges
(def ^:private schema:get-file-branch-merges
[:map {:title "get-file-branch-merges"}
[:file-id ::sm/uuid]])
(def ^:private sql:resolve-source-file
"SELECT coalesce((SELECT b.source_file_id
FROM file_branch AS b
WHERE b.branch_file_id = ?
AND b.deleted_at IS NULL),
?) AS id")
(def ^:private sql:get-file-branch-merges
"SELECT fbm.id,
fbm.created_at,
fbm.file_id,
fbm.file_branch_id,
fbm.branch_name,
fbm.merged_by,
fbm.comment,
fbm.pull_request_id,
fbm.pull_request_title
FROM file_branch_merge AS fbm
WHERE fbm.file_id = ?
ORDER BY fbm.created_at DESC")
(sv/defmethod ::get-file-branch-merges
"List the merges into a file, newest first: each one names the merged
branch, who merged it and when, the comment left with it and the design
review it accepted, if any. When the file is itself a branch, the
merges into its source (main) are listed. The records outlive the
branches they name (see the `file_branch_merge` migration)."
{::doc/added "2.16"
::sm/params schema:get-file-branch-merges}
[cfg {:keys [::rpc/profile-id file-id]}]
(check-branching-enabled!)
(db/run! cfg
(fn [{:keys [::db/conn] :as cfg}]
(let [main-id (:id (db/exec-one! conn [sql:resolve-source-file file-id file-id]))]
(files/check-read-permissions! cfg profile-id main-id)
(db/exec! conn [sql:get-file-branch-merges main-id])))))
;; --- COMMAND: update-file-branch (rename / description)
(def ^:private schema:update-file-branch

81
backend/test/backend_tests/rpc_file_branch_oplog_test.clj
View File

@ -15,7 +15,6 @@
[app.common.data :as d]
[app.common.features :as cfeat]
[app.common.files.migrations :as fmg]
[app.common.time :as ct]
[app.common.types.shape :as cts]
[app.common.uuid :as uuid]
[app.config :as cf]
@ -172,70 +171,6 @@
mf (th/db-get :file {:id (:id file)})]
(t/is (= (:revn mf) (:base-revn row))))))))
(t/deftest merge-with-keep-branch-leaves-the-branch-readable
(with-redefs [cf/flags (conj cf/flags :branching)]
(let [profile (th/create-profile* 1 {:is-active true})
proj-id (:default-project-id profile)
file (th/create-file* 1 {:profile-id (:id profile)
:project-id proj-id
:is-shared false})
create (create-branch* profile (:id file) "oplog-keep")
branch-file-id (:branch-file-id create)
branch-id (:id create)
color-id (uuid/random)]
(apply-change* profile branch-file-id
{:type :add-color
:color {:id color-id :name "C" :color "#112233" :opacity 1}})
(let [out (th/command! {::th/type :merge-file-branch
::rpc/profile-id (:id profile)
:branch-id branch-id
:keep-branch true})]
(t/is (nil? (:error out)))
(t/is (= :merged (-> out :result :status))))
(t/testing "the kept branch still derives its state"
(let [out (th/command! {::th/type :get-file
::rpc/profile-id (:id profile)
:id branch-file-id})]
(t/is (nil? (:error out)))
(t/is (contains? (-> out :result :data :colors) color-id))))
(t/testing "the base pin survives while the branch is kept"
(let [rows (->> (th/db-query :file-change {:file-id (:id file)})
(filter #(some-> (:label %) (str/starts-with? "branch-base/"))))]
(t/is (seq rows))
(t/is (every? (fn [{:keys [deleted-at]}]
(or (nil? deleted-at)
(ct/is-after? deleted-at (ct/in-future {:days 400}))))
rows)))))))
(t/deftest a-kept-merged-branch-refuses-saves
(with-redefs [cf/flags (conj cf/flags :branching)]
(let [profile (th/create-profile* 1 {:is-active true})
proj-id (:default-project-id profile)
file (th/create-file* 1 {:profile-id (:id profile)
:project-id proj-id
:is-shared false})
create (create-branch* profile (:id file) "oplog-merged")
branch-file-id (:branch-file-id create)
branch-id (:id create)]
(th/command! {::th/type :merge-file-branch
::rpc/profile-id (:id profile)
:branch-id branch-id
:keep-branch true})
(let [out (apply-change* profile branch-file-id
{:type :add-color
:color {:id (uuid/random) :name "C" :color "#112233" :opacity 1}})]
(t/is (th/ex-of-type? (:error out) :validation))
(t/is (th/ex-of-code? (:error out) :branch-merged)))
(t/testing "nothing reaches the op log"
(t/is (empty? (oplog-rows branch-file-id)))))))
(defn- add-color* [profile file-id]
(apply-change* profile file-id
{:type :add-color
@ -1408,8 +1343,7 @@
;; a media row MAIN owns
(let [merge (-> (th/command! {::th/type :merge-file-branch
::rpc/profile-id (:id profile)
:branch-id branch-id
:keep-branch true})
:branch-id branch-id})
:result)]
(t/is (= :merged (:status merge)))
(let [ref (-> (derived-obj profile main-id page-id shape-id)
@ -1462,14 +1396,13 @@
;; drops the row (it filters by branch ownership) and main ends up
;; referencing media it does not own.
(with-redefs [cf/flags (conj cf/flags :branching)]
(let [{:keys [profile main-id branch-id branch-file-id page-id shape-id]}
(let [{:keys [profile main-id branch-id page-id shape-id]}
(foreign-media-setup)
out
(th/command! {::th/type :merge-file-branch
::rpc/profile-id (:id profile)
:branch-id branch-id
:keep-branch true})]
:branch-id branch-id})]
(t/is (nil? (:error out)))
(t/is (= :merged (-> out :result :status)))
@ -1477,10 +1410,4 @@
(let [ref (-> (derived-obj profile main-id page-id shape-id) media-ref)
row (th/db-get :file-media-object {:id ref})]
(t/is (some? row))
(t/is (= main-id (:file-id row))))
;; the branch keeps deriving its own state from the kept op log
(let [ref (-> (derived-obj profile branch-file-id page-id shape-id) media-ref)
row (th/db-get :file-media-object {:id ref})]
(t/is (some? row))
(t/is (= branch-file-id (:file-id row)))))))
(t/is (= main-id (:file-id row)))))))

120
backend/test/backend_tests/rpc_file_branch_test.clj
View File

@ -797,31 +797,69 @@
(ct/is-before? deleted-at (ct/in-future {:days 400}))))
rows))))))
(t/deftest merge-with-keep-branch-archives-it
(t/deftest merge-records-itself-and-deletes-the-branch
(with-redefs [cf/flags (conj cf/flags :branching)]
(let [profile (th/create-profile* 1 {:is-active true})
proj-id (:default-project-id profile)
file (th/create-file* 1 {:profile-id (:id profile)
:project-id proj-id})
create (:result (th/command! {::th/type :create-file-branch
create! (fn [name]
(:result (th/command! {::th/type :create-file-branch
::rpc/profile-id (:id profile)
:file-id (:id file)
:name name})))
merge! (fn [branch-id params]
(th/command! (merge {::th/type :merge-file-branch
::rpc/profile-id (:id profile)
:file-id (:id file)
:name "kept"}))
branch-id (:id create)
branch-file-id (:branch-file-id create)]
:branch-id branch-id}
params)))
first-b (create! "first")
second-b (create! "second")]
(let [out (th/command! {::th/type :merge-file-branch
::rpc/profile-id (:id profile)
:branch-id branch-id
:keep-branch true})]
(let [out (merge! (:id first-b) {:comment " Ships the new header "})]
(t/is (nil? (:error out)))
(t/is (= :merged (-> out :result :status))))
(let [[row] (th/db-query :file-branch {:id branch-id})
bf (th/db-get :file {:id branch-file-id})]
(t/is (= "merged" (:status row)))
(t/is (nil? (:deleted-at row)))
(t/is (nil? (:deleted-at bf)))))))
(t/testing "the merged branch and its file are deleted"
(let [[row] (th/db-query :file-branch {:id (:id first-b)})
bf (th/db-get :file {:id (:branch-file-id first-b)} {::db/remove-deleted false})]
(t/is (= "merged" (:status row)))
(t/is (some? (:deleted-at row)))
(t/is (some? (:deleted-at bf)))))
(let [out (merge! (:id second-b) {:comment " "})]
(t/is (nil? (:error out))))
(t/testing "the merges are listed newest first, from main or a branch"
(let [out (th/command! {::th/type :get-file-branch-merges
::rpc/profile-id (:id profile)
:file-id (:id file)})
merges (:result out)]
(t/is (nil? (:error out)))
(t/is (= ["second" "first"] (mapv :branch-name merges)))
(t/is (every? #(= (:id profile) (:merged-by %)) merges))
(t/is (every? #(= (:id file) (:file-id %)) merges))
(t/is (= [nil "Ships the new header"] (mapv :comment merges))
"the comment is trimmed, and a blank one is no comment")
(t/is (every? (comp nil? :pull-request-id) merges))))
(t/testing "a merged branch leaves the branch listing"
(let [out (th/command! {::th/type :get-file-branches
::rpc/profile-id (:id profile)
:file-id (:id file)
:include-archived true})]
(t/is (empty? (:result out))))))))
(t/deftest the-merges-of-a-file-need-read-permissions
(with-redefs [cf/flags (conj cf/flags :branching)]
(let [profile (th/create-profile* 1 {:is-active true})
other (th/create-profile* 2 {:is-active true})
file (th/create-file* 1 {:profile-id (:id profile)
:project-id (:default-project-id profile)})
out (th/command! {::th/type :get-file-branch-merges
::rpc/profile-id (:id other)
:file-id (:id file)})]
(t/is (th/ex-of-type? (:error out) :not-found)))))
(t/deftest merge-requires-main-edition-permissions
(with-redefs [cf/flags (conj cf/flags :branching)]
@ -1185,8 +1223,7 @@
(t/testing "merging brings the new media with a MAIN-owned row"
(let [out (th/command! {::th/type :merge-file-branch
::rpc/profile-id (:id profile)
:branch-id branch-id
:keep-branch true})]
:branch-id branch-id})]
(t/is (nil? (:error out)))
(t/is (= :merged (-> out :result :status))))
@ -1243,8 +1280,7 @@
(let [out (th/command! {::th/type :merge-file-branch
::rpc/profile-id (:id profile)
:branch-id branch-id
:keep-branch true})]
:branch-id branch-id})]
(t/is (nil? (:error out)))
(t/is (= :merged (-> out :result :status))))
@ -1860,9 +1896,9 @@
(t/is (= "merged" (:status row)))
(t/is (some? (:deleted-at row))))))))))
(t/deftest delete-keeps-the-status-the-branch-carries
;; deleting a kept merged branch must not rewrite its status: the
;; record is the only place left that says the branch was merged
(t/deftest a-merged-branch-is-gone-for-delete
;; a merge deletes its branch: deleting it again finds nothing and does
;; not rewrite the status that says the branch was merged
(with-redefs [cf/flags (conj cf/flags :branching)]
(let [profile (th/create-profile* 1 {:is-active true})
proj-id (:default-project-id profile)
@ -1871,7 +1907,7 @@
create (:result (th/command! {::th/type :create-file-branch
::rpc/profile-id (:id profile)
:file-id (:id file)
:name "kept"}))
:name "merged"}))
branch-id (:id create)
branch-file-id (:branch-file-id create)
cid (uuid/random)]
@ -1879,14 +1915,12 @@
{:type :add-color :color {:id cid :name "C" :color "#112233" :opacity 1}})
(let [out (th/command! {::th/type :merge-file-branch
::rpc/profile-id (:id profile)
:branch-id branch-id
:keep-branch true})]
:branch-id branch-id})]
(t/is (= :merged (-> out :result :status))))
(let [out (th/command! {::th/type :delete-file-branch
::rpc/profile-id (:id profile)
:id branch-id})]
(t/is (nil? (:error out)))
(t/is (= :deleted (-> out :result :status))))
(t/is (= :branch-not-found (-> out :error ex-data :code))))
(let [[row] (th/db-query :file-branch {:id branch-id})]
(t/is (= "merged" (:status row)))
(t/is (some? (:deleted-at row)))))))
@ -2298,8 +2332,8 @@
(t/deftest merge-refuses-a-branch-merged-under-the-lock
;; two merges of one open branch: the winner commits before the
;; outer merge takes any lock. The outer merge must refuse instead of
;; trusting its stale "open" row and deleting the branch the winner
;; kept.
;; trusting its stale "open" row: merging the branch a second time,
;; recording a second merge and deleting it again.
(with-redefs [cf/flags (conj cf/flags :branching)]
(let [profile (th/create-profile* 1 {:is-active true})
proj-id (:default-project-id profile)
@ -2321,26 +2355,26 @@
db/xact-lock!
(fn [conn id]
;; the winning merge runs to completion BEFORE
;; the outer one takes any lock, and keeps the
;; branch so the outer merge still has its file
;; the outer one takes any lock
(when (compare-and-set! raced? false true)
(reset! merged (th/command! {::th/type :merge-file-branch
::rpc/profile-id (:id profile)
:branch-id branch-id
:keep-branch true})))
:branch-id branch-id})))
(orig-lock conn id))]
(let [out (th/command! {::th/type :merge-file-branch
::rpc/profile-id (:id profile)
:branch-id branch-id})]
(t/is (= :branch-not-open (-> out :error ex-data :code)))))
(t/is (= :branch-not-found (-> out :error ex-data :code)))))
(t/is (= :merged (-> @merged :result :status)))
(t/testing "main holds the first merge's revn only"
(t/is (= (-> @merged :result :revn)
(:revn (first (th/db-query :file {:id (:id file)}))))))
(t/testing "the branch the winning merge kept is still kept"
(t/testing "the winning merge is the only one recorded"
(t/is (= 1 (count (th/db-query :file-branch-merge {:file-branch-id branch-id})))))
(t/testing "the branch is merged and deleted once"
(let [[row] (th/db-query :file-branch {:id branch-id})]
(t/is (= "merged" (:status row)))
(t/is (nil? (:deleted-at row)))))))))
(t/is (some? (:deleted-at row)))))))))
(t/deftest merge-releases-the-base-the-lock-shows
;; the merge releases the base snapshot pin (on the delete path), and
@ -2514,15 +2548,17 @@
:expected-main-revn 9999})]
(t/is (= :file-modified (-> out :error ex-data :code)))
(t/is (empty? @seen)))
;; the kept branch means :file-merged is the only message
;; the merge deletes the branch: its file's clients hear of the
;; deletion and main's of the merge
(let [out (th/command! {::th/type :merge-file-branch
::rpc/profile-id (:id profile)
:branch-id branch-id
:keep-branch true})]
:branch-id branch-id})]
(t/is (= :merged (-> out :result :status))))
(let [[{:keys [message main-revn]}] @seen]
(t/is (= 1 (count @seen)))
(t/is (= :file-merged (:type message)))
(let [by-type (into {} (map (juxt (comp :type :message) identity)) @seen)
{:keys [message main-revn]} (get by-type :file-merged)]
(t/is (= 2 (count @seen)))
(t/is (= #{:file-deleted :file-merged} (set (keys by-type))))
(t/is (= branch-file-id (get-in by-type [:file-deleted :message :file-id])))
;; the merge was already committed when the message left
(t/is (= (:revn message) main-revn)))))))

84
backend/test/backend_tests/rpc_file_pull_request_test.clj
View File

@ -492,7 +492,14 @@
(t/is (= "merged" (:status row)))
(t/is (some? (:closed-at row)))
;; the row survives as history even though the branch is gone
(t/is (nil? (:deleted-at row))))))
(t/is (nil? (:deleted-at row))))
;; the record of the merge names the review it accepted, so the
;; list of merges keeps it once the review is gone
(let [merges (:result (th/command! {::th/type :get-file-branch-merges
::rpc/profile-id (:id author)
:file-id (:id file)}))]
(t/is (= [(:id pr)] (mapv :pull-request-id merges)))
(t/is (= [(:title pr)] (mapv :pull-request-title merges))))))
(t/testing "deleting the branch closes the pull request"
(let [branch (create-branch* author file "delete-me")
@ -585,81 +592,6 @@
(t/is (= :branch-not-open (-> out :error ex-data :code)))
(t/is (= "closed" (:status (first (th/db-query :file-pull-request {:id (:id pr)}))))))))))))
(t/deftest create-serializes-against-a-kept-concurrent-merge
;; same race, but the merge keeps the branch: its row stays readable
;; with status "merged" and its file is not deleted, so without the
;; re-read the create sails past the stale status check and INSERTS a
;; pull request for a branch the merge already closed.
(with-redefs [cf/flags pr-flags]
(let [author (th/create-profile* 1 {:is-active true})
file (th/create-file* 1 {:profile-id (:id author)
:project-id (:default-project-id author)})
branch (create-branch* author file "contested")
branch-id (:id branch)
branch-file-id (:branch-file-id branch)]
(add-color* author branch-file-id "Brand")
(let [orig-lock db/xact-lock!
merging? (atom false)
merged (atom nil)]
(with-redefs [db/xact-lock!
(fn [conn id]
;; the first lock the create takes is the signal:
;; merge the same branch through the pool in its
;; own transaction BEFORE the create locks
(when (compare-and-set! merging? false true)
(reset! merged (th/command! {::th/type :merge-file-branch
::rpc/profile-id (:id author)
:branch-id branch-id
:keep-branch true})))
(orig-lock conn id))]
(let [out (th/command! {::th/type :create-pull-request
::rpc/profile-id (:id author)
:branch-id branch-id
:title "review me"})]
(t/is (= :merged (-> @merged :result :status)))
(t/is (= "merged" (:status (first (th/db-query :file-branch {:id branch-id})))))
(t/is (= :branch-not-open (-> out :error ex-data :code)))
(t/is (= [] (th/db-query :file-pull-request {:file-branch-id branch-id})))))))))
(t/deftest reopen-serializes-against-a-kept-concurrent-merge
;; same race for the reopen: the kept merge leaves the branch row
;; readable with status "merged", so without the re-read the reopen
;; sails past the stale status check and REOPENS the closed pull
;; request over a branch the merge already closed.
(with-redefs [cf/flags pr-flags]
(let [author (th/create-profile* 1 {:is-active true})
file (th/create-file* 1 {:profile-id (:id author)
:project-id (:default-project-id author)})
branch (create-branch* author file "contested")
branch-id (:id branch)
branch-file-id (:branch-file-id branch)]
(add-color* author branch-file-id "Brand")
(let [pr (:result (create-pr* author branch-id {}))]
(t/is (nil? (:error (th/command! {::th/type :close-pull-request
::rpc/profile-id (:id author)
:id (:id pr)}))))
(let [orig-lock db/xact-lock!
merging? (atom false)
merged (atom nil)]
(with-redefs [db/xact-lock!
(fn [conn id]
;; the first lock the reopen takes is the signal:
;; merge the same branch through the pool in its
;; own transaction BEFORE the reopen locks
(when (compare-and-set! merging? false true)
(reset! merged (th/command! {::th/type :merge-file-branch
::rpc/profile-id (:id author)
:branch-id branch-id
:keep-branch true})))
(orig-lock conn id))]
(let [out (th/command! {::th/type :reopen-pull-request
::rpc/profile-id (:id author)
:id (:id pr)})]
(t/is (= :merged (-> @merged :result :status)))
(t/is (= "merged" (:status (first (th/db-query :file-branch {:id branch-id})))))
(t/is (= :branch-not-open (-> out :error ex-data :code)))
(t/is (= "closed" (:status (first (th/db-query :file-pull-request {:id (:id pr)}))))))))))))
(t/deftest closed-pull-requests-listing
(with-redefs [cf/flags pr-flags]
(let [author (th/create-profile* 1 {:is-active true})

6
frontend/resources/images/icons/git-branch-minus.svg Normal file
View File

@ -0,0 +1,6 @@
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="none" stroke-width="1.5" stroke-linecap="round" stroke-linejoin="round">
<path d="M15 6a9 9 0 0 0-9 9V3"/>
<path d="M21 18h-6"/>
<circle cx="18" cy="6" r="3"/>
<circle cx="6" cy="18" r="3"/>
</svg>
Side by Side

After

Width:  |  Height:  |  Size: 280 B

8
frontend/resources/images/icons/git-compare-arrows.svg Normal file
View File

@ -0,0 +1,8 @@
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="none" stroke-width="1.5" stroke-linecap="round" stroke-linejoin="round">
<circle cx="5" cy="6" r="3"/>
<path d="M12 6h5a2 2 0 0 1 2 2v7"/>
<path d="m15 9-3-3 3-3"/>
<circle cx="19" cy="18" r="3"/>
<path d="M12 18H7a2 2 0 0 1-2-2V9"/>
<path d="m9 15 3 3-3 3"/>
</svg>
Side by Side

After

Width:  |  Height:  |  Size: 357 B

8
frontend/resources/images/icons/git-graph.svg Normal file
View File

@ -0,0 +1,8 @@
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="none" stroke-width="1.5" stroke-linecap="round" stroke-linejoin="round">
<circle cx="5" cy="6" r="3"/>
<path d="M5 9v6"/>
<circle cx="5" cy="18" r="3"/>
<path d="M12 3v18"/>
<circle cx="19" cy="6" r="3"/>
<path d="M16 15.7A9 9 0 0 0 19 9"/>
</svg>
Side by Side

After

Width:  |  Height:  |  Size: 338 B

7
frontend/resources/images/icons/git-merge-conflict.svg Normal file
View File

@ -0,0 +1,7 @@
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="none" stroke-width="1.5" stroke-linecap="round" stroke-linejoin="round">
<path d="M12 6h4a2 2 0 0 1 2 2v7"/>
<path d="M6 12v9"/>
<path d="m8.5 3.5-5 5"/>
<path d="m8.5 8.5-5-5"/>
<circle cx="18" cy="18" r="3"/>
</svg>
Side by Side

After

Width:  |  Height:  |  Size: 304 B

8
frontend/resources/images/icons/git-pull-request-create-arrow.svg Normal file
View File

@ -0,0 +1,8 @@
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="none" stroke-width="1.5" stroke-linecap="round" stroke-linejoin="round">
<circle cx="5" cy="6" r="3"/>
<path d="M5 9v12"/>
<path d="m15 9-3-3 3-3"/>
<path d="M12 6h5a2 2 0 0 1 2 2v3"/>
<path d="M19 15v6"/>
<path d="M22 18h-6"/>
</svg>
Side by Side

After

Width:  |  Height:  |  Size: 325 B

97
frontend/src/app/main/data/workspace/branches.cljs
View File

@ -18,6 +18,7 @@
[app.util.dom :as dom]
[app.util.i18n :refer [tr]]
[beicon.v2.core :as rx]
[cuerdas.core :as str]
[potok.v2.core :as ptk]))
(defn- reload-file-window
@ -137,6 +138,33 @@
(rx/catch (fn [_]
(rx/of (update-branches-state {:status :error})))))))))
(defn- set-branch-merges
[merges-state]
(ptk/reify ::set-branch-merges
ptk/UpdateEvent
(update [_ state]
(assoc state :workspace-branch-merges merges-state))))
(defn fetch-branch-merges
"Load the merges into the open file's main, newest first, into
`:workspace-branch-merges` (`{:status :loading|:loaded|:error}` plus
`:data`): who merged each branch, when, and the comment left with it.
The command resolves a branch to its main, so it is asked about the
open file as is. A failure is kept as an error, never as an empty list,
so it cannot read as a file with no merges."
[]
(ptk/reify ::fetch-branch-merges
ptk/UpdateEvent
(update [_ state]
(assoc state :workspace-branch-merges {:status :loading}))
ptk/WatchEvent
(watch [_ state _]
(when-let [file-id (:current-file-id state)]
(->> (rp/cmd! :get-file-branch-merges {:file-id file-id})
(rx/map #(set-branch-merges {:status :loaded :data %}))
(rx/catch (fn [_] (rx/of (set-branch-merges {:status :error})))))))))
(defn fetch-branching-limits
"Load the size limits this instance refuses beyond, so the panel can
state them before a user meets one. The numbers live in the backend
@ -254,15 +282,24 @@
(rx/catch (fn [cause] (rx/of (refusal-notification cause (tr "workspace.branches.lifecycle.error"))))))))))
(defn delete-branch
[id]
(assert (uuid? id) "expected valid uuid for `id`")
(ptk/reify ::delete-branch
ptk/WatchEvent
(watch [_ _ _]
(->> (rp/cmd! :delete-file-branch {:id id})
(rx/mapcat (fn [_] (rx/of (ntf/success (tr "workspace.branches.lifecycle.deleted"))
(fetch-branches))))
(rx/catch (fn [cause] (rx/of (refusal-notification cause (tr "workspace.branches.lifecycle.error")))))))))
"Delete a branch. Deleting the open branch takes its file away, so the
caller passes `open-file-id` (the file it comes from) to land there once
the branch is gone; this session is not sent away by the deletion it
made itself."
([id] (delete-branch id nil))
([id open-file-id]
(assert (uuid? id) "expected valid uuid for `id`")
(ptk/reify ::delete-branch
ptk/WatchEvent
(watch [_ _ _]
(->> (rp/cmd! :delete-file-branch {:id id})
(rx/mapcat (fn [_] (rx/concat
(rx/of (ntf/success (tr "workspace.branches.lifecycle.deleted"))
(fetch-branches))
(if (uuid? open-file-id)
(rx/of (open-branch open-file-id))
(rx/empty)))))
(rx/catch (fn [cause] (rx/of (refusal-notification cause (tr "workspace.branches.lifecycle.error"))))))))))
(defn materialize-branch
"Turn a branch into an ordinary file: `branch-file-id` is the branch's
@ -361,18 +398,19 @@
(defn show-branches
"The entry point of the File menu and the Ctrl/Cmd+Alt+B shortcut. A
file that has branches gets the branch switcher; one with none gets the
create-branch dialog, since the switcher only exists once a branch does.
No-op when the branching feature flag is disabled."
file that has branches gets the branches dialog (the full list, the
same the switcher's \"All branches\" opens, which shows this shortcut);
one with none gets the create-branch dialog. No-op when the branching
feature flag is disabled."
[]
(ptk/reify ::show-branches
ptk/WatchEvent
(watch [_ state _]
(when (contains? cf/flags :branching)
(if (has-branches? state)
(rx/of (toggle-branch-switcher true))
(let [file (get-in state [:files (:current-file-id state)])]
(rx/of (modal/show :create-branch {:file-name (:name file)}))))))))
(rx/of (toggle-branch-switcher false)
(modal/show :branches-list {}))
(rx/of (modal/show :create-branch {})))))))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; COMPARE (read-only 3-way diff)
@ -524,7 +562,7 @@
`main-revn` is main's revn when the command computed those conflicts. It
is stored with them because the modal's own diff fetch can still be in
flight when the user applies; see `conflict-main-revn`."
[{:keys [branch mode conflicts main-revn excluded]}]
[{:keys [branch mode conflicts main-revn excluded comment]}]
(ptk/reify ::open-conflict-resolutions
ptk/UpdateEvent
(update [_ state]
@ -536,11 +574,13 @@
ptk/WatchEvent
(watch [_ _ _]
;; `excluded` is the per-change selection the merge/update attempt
;; was made with; the resolution modal applies it on retry
;; was made with, and `comment` what the user said about the merge;
;; the resolution modal sends both again on retry
(rx/of (modal/show :branch-conflicts {:branch branch
:mode mode
:conflicts conflicts
:excluded excluded})))))
:excluded excluded
:comment comment})))))
(defn- conflict-main-revn
"Main's revn when the server computed the conflict set stored for
@ -662,16 +702,14 @@
(defn merge-branch
"Merge `branch` (a branch row/context map with at least `:id`) into main.
On success the branch is marked merged server-side and open clients
reload main via the `:file-merged` msgbus event.
On success the server records the merge and deletes the branch and its
file, and open clients reload main via the `:file-merged` msgbus event.
Options:
- `:resolutions` — optional `{entity-id (:main|:branch)}` map used to
resolve conflicts before integrating.
- `:keep-branch` — when true the merged branch is kept (it shows up
under \"Archived\"); when false (default) the server deletes the
branch and its file in the same transaction, so merged copies do not
pile up and eat disk space.
- `:comment` — what the user says about the merge, kept with its
record and shown in the reviews dialog.
- `:excluded` — the per-change selection (change keys,
`app.common.files.branch-merge/change-key`) naming the changes the
user deselected, which main keeps its own value for.
@ -683,7 +721,7 @@
before that diff has loaded carry the revn of the conflict answer instead
(`conflict-main-revn`). Unresolved conflicts open the resolution modal."
([branch] (merge-branch branch nil))
([branch {:keys [resolutions keep-branch excluded]}]
([branch {:keys [resolutions comment excluded]}]
(assert (uuid? (:id branch)) "expected a branch row with a valid `:id`")
(ptk/reify ::merge-branch
ptk/WatchEvent
@ -700,7 +738,7 @@
(cond-> {:branch-id branch-id}
(seq resolutions) (assoc :resolutions resolutions)
(seq excluded) (assoc :excluded excluded)
keep-branch (assoc :keep-branch true)
(not (str/blank? comment)) (assoc :comment (str/trim comment))
(some? main-revn) (assoc :expected-main-revn main-revn)))
(rx/mapcat
(fn [{:keys [status source-file-id conflicts] :as answer}]
@ -708,15 +746,16 @@
;; the merged result lives in main: take the user there
;; to see it (navigate if elsewhere, hard-reload if
;; already on main). Other clients reload via the
;; `:file-merged` event; the branch deletion (when not
;; kept) already happened server-side.
;; `:file-merged` event; the branch deletion already
;; happened server-side.
:merged (rx/of (ntf/success (tr "workspace.branches.merge.success"))
(show-merge-result source-file-id))
:conflicts (rx/of (open-conflict-resolutions {:branch branch
:mode :merge
:conflicts conflicts
:main-revn (:main-revn answer)
:excluded excluded}))
:excluded excluded
:comment comment}))
:unsupported (rx/of (ntf/warn (tr "workspace.branches.merge.unsupported")))
(rx/of (ntf/error (tr "workspace.branches.merge.error"))))))
(rx/catch

5
frontend/src/app/main/ui/ds/foundations/assets/icon.cljs
View File

@ -155,11 +155,16 @@
(def ^:icon-id gap-horizontal "gap-horizontal")
(def ^:icon-id gap-vertical "gap-vertical")
(def ^:icon-id git-branch "git-branch")
(def ^:icon-id git-branch-minus "git-branch-minus")
(def ^:icon-id git-branch-plus "git-branch-plus")
(def ^:icon-id git-commit "git-commit")
(def ^:icon-id git-commit-vertical "git-commit-vertical")
(def ^:icon-id git-compare-arrows "git-compare-arrows")
(def ^:icon-id git-graph "git-graph")
(def ^:icon-id git-merge "git-merge")
(def ^:icon-id git-merge-conflict "git-merge-conflict")
(def ^:icon-id git-pull-request-arrow "git-pull-request-arrow")
(def ^:icon-id git-pull-request-create-arrow "git-pull-request-create-arrow")
(def ^:icon-id graphics "graphics")
(def ^:icon-id grid "grid")
(def ^:icon-id grid-column "grid-column")

42
frontend/src/app/main/ui/ds/product/branch_row.cljs
View File

@ -23,23 +23,47 @@
(mf/defc branch-row*
"A branch in a short list, as a button that opens it. The current one is
marked and cannot be opened again. `children` go to the end of the row:
usually a `branch-status*`."
usually a `branch-status*`. A label cut short by the row's width shows
in full on hover."
{::mf/schema schema:branch-row}
[{:keys [class label is-current children] :rest props}]
(let [is-current (d/nilv is-current false)
props (mf/spread-props props
{:class [class (stl/css-case :branch-row true
:is-current is-current)]
:type "button"
:disabled is-current
:aria-current (when is-current "true")})]
(let [is-current (d/nilv is-current false)
label-ref (mf/use-ref nil)
truncated* (mf/use-state false)
truncated? (deref truncated*)
check-truncated
(mf/use-fn
(fn []
(when-let [node (mf/ref-val label-ref)]
(reset! truncated* (> (.-scrollWidth node) (.-clientWidth node))))))
;; the title goes on the button, not the label: the current row is
;; disabled and still has to show it
props (mf/spread-props props
{:class [class (stl/css-case :branch-row true
:is-current is-current)]
:type "button"
:title (when truncated? label)
:disabled is-current
:aria-current (when is-current "true")})]
(mf/with-effect [label]
(check-truncated)
(when-let [node (mf/ref-val label-ref)]
(let [observer (js/ResizeObserver. check-truncated)]
(.observe observer node)
#(.disconnect observer))))
[:> :button props
[:> icon* {:icon-id (if is-current i/tick i/git-branch)
:size "s"
:class (stl/css :icon)}]
[:> text* {:as "span"
:typography t/body-small
:class (stl/css :label)}
:class (stl/css :label)
:ref label-ref}
label]
(when (some? children)
[:span {:class (stl/css :aside)}

4
frontend/src/app/main/ui/ds/product/branch_row.mdx
View File

@ -31,6 +31,10 @@ Children go to the end of the row, usually a `BranchStatus`.
The label is mandatory. The row is a `button`; any other prop
(`on-click`, `data-*`) goes to it.
A label too long for the row ends in an ellipsis, and the row then
carries the full label as its `title`, so it shows on hover (the current
row too, though it is disabled). A label that fits gets no `title`.
```clj
[:> branch-row* {:label (:name branch)
:data-branch (dm/str (:branch-file-id branch))

56
frontend/src/app/main/ui/workspace/branch_switcher.cljs
View File

@ -29,6 +29,8 @@
[app.main.ui.ds.product.branch-status :refer [branch-status*]]
[app.main.ui.workspace.menu-shortcuts :refer [shortcuts*]]
[app.main.ui.workspace.sidebar.pull-requests :as prui]
;; registers the reviews dialog the popover opens
[app.main.ui.workspace.sidebar.reviews]
[app.util.dom :as dom]
[app.util.i18n :refer [tr]]
[cuerdas.core :as str]
@ -84,14 +86,6 @@
(zero? (or (:behind row) 0))
(zero? (or (:conflicts row) 0))))
(defn- main-subtitle
"The note beside main. Files that use a shared library read its main,
so on a library it says so."
[library?]
(if library?
(tr "workspace.branches.main-subtitle-library")
(tr "workspace.branches.main-subtitle")))
;; --- Popover
(mf/defc switcher-row*
@ -104,10 +98,17 @@
:on-click on-open}
;; merged and archived branches carry no change counts
(cond
(:is-main row) (main-subtitle (:is-library row))
(:is-main row) (tr "workspace.branches.main-subtitle")
(= "merged" (:status row)) (tr "workspace.branches.header.merged")
(= "archived" (:status row)) (tr "workspace.branches.header.archived")
:else [:> row-status* {:row row :pr pr}])]])
:else [:> row-status* {:row row :pr pr}])]
;; files that use a shared library read its main: said under the row,
;; so the note never takes the room of the branch name
(when (and (:is-main row) (:is-library row))
[:> text* {:as "div"
:typography t/body-small
:class (stl/css :row-note)}
(tr "workspace.branches.main-library-note")])])
(mf/defc closed-group*
"A collapsed section of the popover for branches out of the flow: its
@ -134,7 +135,7 @@
(mf/defc switcher-popover*
{::mf/private true}
[{:keys [file current prs-by-branch on-close]}]
[{:keys [current prs-by-branch on-close]}]
(let [{:keys [data status]} (mf/deref branches)
filter* (mf/use-state "")
@ -177,10 +178,17 @@
on-create
(mf/use-fn
(mf/deps file on-close)
(mf/deps on-close)
(fn [_]
(on-close)
(modal/show! :create-branch {:file-name (:name file)})))
(modal/show! :create-branch {})))
on-show-reviews
(mf/use-fn
(mf/deps on-close)
(fn [_]
(on-close)
(modal/show! :branch-reviews {})))
on-show-all
(mf/use-fn
@ -251,9 +259,6 @@
:pr (get prs-by-branch (:id row))
:on-open on-open}])]])
[:> closed-group* {:label (tr "workspace.branches.merged")
:rows (with-status "merged")
:on-open on-open}]
[:> closed-group* {:label (tr "workspace.branches.archived")
:rows (with-status "archived")
:on-open on-open}]])
@ -262,12 +267,18 @@
;; branches only grow from main: on a branch there is nothing to create
(when (:is-main current)
[:button {:class (stl/css :action) :on-click on-create}
[:> i/icon* {:icon-id i/git-branch-plus :size "s"}]
[:span {:class (stl/css :action-label)} (tr "workspace.branches.switcher.new")]])
[:*
[:button {:class (stl/css :action) :on-click on-create}
[:> i/icon* {:icon-id i/git-branch-plus :size "s"}]
[:span {:class (stl/css :action-label)} (tr "workspace.branches.switcher.new")]]
[:hr {:class (stl/css :divider)}]])
[:button {:class (stl/css :action) :on-click on-show-reviews}
[:> i/icon* {:icon-id i/git-pull-request-arrow :size "s"}]
[:span {:class (stl/css :action-label)} (tr "workspace.branches.switcher.reviews")]]
[:button {:class (stl/css :action) :on-click on-show-all}
[:> i/icon* {:icon-id i/menu :size "s"}]
[:> i/icon* {:icon-id i/git-graph :size "s"}]
[:span {:class (stl/css :action-label)} (tr "workspace.branches.switcher.all")]
[:> shortcuts* {:id :open-branches}]]]))
@ -365,7 +376,7 @@
[:> text* {:as "span"
:typography t/body-small
:class (stl/css :status-note)}
(main-subtitle (:is-shared file))]
(tr "workspace.branches.main-subtitle")]
;; merged and archived branches open read-only and count no
;; changes: the note says which of the two it is
@ -391,7 +402,6 @@
[:> row-status* {:row ctx :pr pr}]))
[:& dropdown {:show open? :on-close on-close :container bar-ref}
[:> switcher-popover* {:file file
:current current
[:> switcher-popover* {:current current
:prs-by-branch prs-by-branch
:on-close on-close}]]])))

15
frontend/src/app/main/ui/workspace/branch_switcher.scss
View File

@ -24,8 +24,12 @@
padding: var(--sp-xs) var(--sp-m) var(--sp-s);
}
// the note gives way before the branch name in the chip does
.status-note {
flex-shrink: 0;
min-inline-size: 0;
overflow: hidden;
white-space: nowrap;
text-overflow: ellipsis;
color: var(--color-foreground-secondary);
}
@ -71,6 +75,15 @@
list-style: none;
}
// lined up with the end of the row above, where its subtitle sits
.row-note {
padding-block-end: var(--sp-xs);
padding-inline: var(--sp-s);
color: var(--color-foreground-secondary);
font-style: italic;
text-align: end;
}
.list-note {
padding: var(--sp-s);
color: var(--color-foreground-secondary);

4
frontend/src/app/main/ui/workspace/main_menu.cljs
View File

@ -968,8 +968,8 @@
::ev/origin "workspace:menu"})
(modal/show :plugin-management {}))))
;; the switcher when the file has branches; the create dialog when
;; it has none, since the switcher only exists once a branch does
;; the branches dialog when the file has branches; the create
;; dialog when it has none
show-branches
(mf/use-fn
(fn [event]

486
frontend/src/app/main/ui/workspace/sidebar/branches.cljs
View File

@ -22,7 +22,7 @@
[app.main.ui.ds.controls.checkbox :refer [checkbox*]]
[app.main.ui.ds.controls.input :refer [input*]]
[app.main.ui.ds.foundations.assets.icon :as i]
[app.main.ui.ds.layout.menu :refer [menu* menu-item*]]
[app.main.ui.ds.layout.menu :refer [menu* menu-item* menu-separator*]]
[app.main.ui.ds.layout.modal :refer [modal* modal-header* modal-content* modal-footer*]]
[app.main.ui.ds.layout.tab-switcher :refer [tab-switcher*]]
[app.main.ui.ds.notifications.context-notification :refer [context-notification*]]
@ -232,7 +232,7 @@
(mf/defc create-branch-dialog*
{::mf/register modal/components
::mf/register-as :create-branch}
[{:keys [file-name]}]
[]
(let [;; a branch only grows from main, so the open file is main
library? (boolean (:is-shared (mf/deref refs/file)))
branch-name* (mf/use-state "")
@ -261,8 +261,7 @@
[:> modal* {:is-open true
:on-open-change (prui/on-dismiss on-close)}
[:> prui/dialog-header* {:icon i/git-branch
:title (tr "workspace.branches.create.title")
:subtitle (tr "workspace.branches.create.subtitle" (or file-name ""))}]
:title (tr "workspace.branches.create.title")}]
[:> modal-content* {:class (stl/css :dialog-content)}
[:> input* {:label (tr "workspace.branches.create.name-label")
@ -303,6 +302,64 @@
:on-click on-submit}
(tr "workspace.branches.create.submit")]]]))
;; --- Rename branch dialog (modal)
;;
;; The branch list renames in place; the bottom bar has no room for that,
;; so it asks for the new name here.
(mf/defc rename-branch-dialog*
{::mf/register modal/components
::mf/register-as :rename-branch}
[{:keys [branch]}]
(let [branch-name* (mf/use-state (or (:name branch) ""))
branch-name (deref branch-name*)
value (str/trim branch-name)
valid? (and (not (str/blank? value))
(not= value (:name branch)))
on-name-change
(mf/use-fn #(reset! branch-name* (dom/get-target-val %)))
on-close
(mf/use-fn #(st/emit! (modal/hide)))
on-submit
(mf/use-fn
(mf/deps branch value valid?)
(fn [_]
(when valid?
(st/emit! (dwb/rename-branch (:id branch) value)
(modal/hide)))))
on-key-down
(mf/use-fn
(mf/deps on-submit)
(fn [event]
(when (kbd/enter? event)
(on-submit event))))]
[:> modal* {:is-open true
:on-open-change (prui/on-dismiss on-close)}
[:> prui/dialog-header* {:icon i/git-branch
:title (tr "workspace.branches.rename.title")
:subtitle (:name branch)}]
[:> modal-content* {:class (stl/css :dialog-content)}
[:> input* {:label (tr "workspace.branches.create.name-label")
:icon i/git-branch
:value branch-name
:auto-focus true
:on-change on-name-change
:on-key-down on-key-down}]]
[:> modal-footer* {}
[:> button* {:variant "secondary" :on-click on-close}
(tr "labels.cancel")]
[:> button* {:variant "primary"
:disabled (not valid?)
:on-click on-submit}
(tr "labels.rename")]]]))
;; --- Preview limits dialog (modal)
;;
;; Every refusal the branching commands can emit has a line here, so a user
@ -381,8 +438,8 @@
{::mf/register modal/components
::mf/register-as :merge-branch}
[{:keys [branch excluded]}]
(let [archive?* (mf/use-state false)
archive? (deref archive?*)
(let [comment* (mf/use-state "")
comment (deref comment*)
;; The branch name and "main" are highlighted (foreground-primary)
;; within the otherwise secondary-colored prompt. We wrap each of
@ -396,17 +453,17 @@
(str sep (tr "workspace.branches.main") sep))]
(str/split msg sep))
on-toggle-archive
(mf/use-fn #(swap! archive?* not))
on-comment-change
(mf/use-fn #(reset! comment* (dom/get-target-val %)))
on-close
(mf/use-fn #(st/emit! (modal/hide)))
on-submit
(mf/use-fn
(mf/deps branch archive? excluded)
(mf/deps branch comment excluded)
(fn [_]
(st/emit! (dwb/merge-branch branch {:keep-branch archive? :excluded excluded})
(st/emit! (dwb/merge-branch branch {:comment comment :excluded excluded})
(modal/hide))))]
[:> modal* {:is-open true
@ -420,16 +477,20 @@
[:span {:key idx :class (stl/css :modal-message-emphasis)} part]
[:span {:key idx} part]))]
[:> checkbox* {:id "merge-archive-branch"
:label (tr "workspace.branches.merge.archive-label")
:checked archive?
:on-change on-toggle-archive}]
;; kept with the record of the merge, in the reviews dialog
[:div {:class (stl/css :field)}
[:label {:class (stl/css :field-label)
:for "merge-branch-comment"}
(tr "workspace.branches.merge.comment-label")]
[:textarea {:id "merge-branch-comment"
:class (stl/css :textarea)
:value comment
:max-length 4000
:on-change on-comment-change
:rows 3}]]
(if archive?
[:> context-notification* {:level :warning :type :context}
(tr "workspace.branches.merge.archive-warning")]
[:> context-notification* {:level :info :type :context}
(tr "workspace.branches.merge.delete-info")])]
[:> context-notification* {:level :info :type :context}
(tr "workspace.branches.merge.delete-info")]]
[:> modal-footer* {}
[:> button* {:variant "secondary" :on-click on-close}
@ -442,8 +503,8 @@
;; --- Confirmation for relevant branch actions (merge / update / materialize)
(defn- confirm-merge!
"Open the merge dialog for `branch` (merge into main is irreversible; the
dialog also lets the user choose whether to keep the branch archived).
"Open the merge dialog for `branch` (merge into main is irreversible and
deletes the branch; the dialog asks for a comment for its record).
`excluded` is the compare dialog's per-change selection, if any."
([branch] (confirm-merge! branch nil))
([branch excluded]
@ -474,6 +535,45 @@
:accept-style :danger
:on-accept (fn [_] (st/emit! (dwb/materialize-branch (:branch-file-id branch))))})))
(defn- confirm-archive!
"Ask for confirmation before archiving `branch`, or restoring it with
`archived?` false. The confirmation takes the place of any open dialog."
[branch archived?]
(st/emit! (modal/show {:type :confirm
:title (if archived?
(tr "workspace.branches.archive.title")
(tr "workspace.branches.restore.title"))
:message (if archived?
(tr "workspace.branches.archive.message" (:name branch))
(tr "workspace.branches.restore.message" (:name branch)))
:accept-label (if archived?
(tr "workspace.branches.menu.archive")
(tr "workspace.branches.menu.restore"))
:accept-style :primary
:on-accept (fn [_] (st/emit! (dwb/archive-branch (:id branch) archived?)))})))
(defn- confirm-delete!
"Ask for confirmation before deleting `branch`. `open-file-id`, when
given, is the file to land on afterwards: the one the open branch comes
from, because deleting it takes the open file away."
([branch] (confirm-delete! branch nil))
([branch open-file-id]
(st/emit! (modal/show {:type :confirm
:title (tr "workspace.branches.delete.title")
:message (tr "workspace.branches.delete.message" (:name branch))
:accept-label (tr "labels.delete")
:accept-style :danger
:on-accept (fn [_] (st/emit! (dwb/delete-branch (:id branch) open-file-id)))}))))
;; --- Profiles shown as avatars
(defn- avatar-profile
"The fields of a profile an avatar reads. The logged-in profile carries
more, such as a `:theme` that is nil until the user picks one, which the
avatar's schema refuses (it takes a string or nothing)."
[profile]
(select-keys profile [:id :fullname :email :photo-id :photo-url]))
;; --- Diff counts: known, or honestly unknown
;;
;; A row whose comparison could not be made carries no counts at all (and
@ -495,7 +595,7 @@
(let [profile (mf/deref refs/profile)
profiles (mf/deref refs/profiles)
author (or (get profiles (:created-by branch))
(when (= (:created-by branch) (:id profile)) profile))
(when (= (:created-by branch) (:id profile)) (avatar-profile profile)))
author? (= (:created-by branch) (:id profile))
status (:status branch)
merged? (= "merged" status)
@ -521,9 +621,7 @@
on-compare (mf/use-fn (mf/deps branch)
#(modal/show! :branch-compare {:branch branch}))
on-restore (mf/use-fn (mf/deps branch)
(fn [_]
(st/emit! (modal/hide)
(dwb/archive-branch (:id branch) false))))
#(confirm-archive! branch false))
on-materialize (mf/use-fn (mf/deps branch)
#(confirm-materialize! branch))]
[:> modal* {:is-open true
@ -613,20 +711,24 @@
[:dd {:class (stl/css :info-val)}
(-> (:merged-at branch) ct/inst (ct/format-inst :localized-date-time))]])]]
;; the dialog closes from its header; the footer only acts on the
;; branch. The work of a merged branch is in main: there is nothing
;; to compare
[:> modal-footer* {}
;; an archived branch is read-only: it comes back to work restored,
;; or leaves the branch engine as an ordinary file
(when archived?
[:> button* {:variant "secondary" :icon i/history :on-click on-restore}
(tr "workspace.branches.menu.restore")])
(when archived?
[:> button* {:variant "secondary" :icon i/document :on-click on-materialize}
(tr "workspace.branches.materialize.action")])
;; the work of a merged branch is in main: there is nothing to compare
(when-not merged?
[:> button* {:variant "secondary" :icon i/switch :on-click on-compare}
[:> button* {:variant (if archived? "secondary" "primary")
:icon i/git-compare-arrows
:on-click on-compare}
(tr "workspace.branches.compare")])
[:> button* {:variant "primary" :on-click on-close} (tr "labels.close")]]]))
;; an archived branch is read-only: it leaves the branch engine as an
;; ordinary file, or comes back to work restored, its primary action.
;; The buttons follow the bottom bar of an open archived branch
(when archived?
[:*
[:> button* {:variant "secondary" :icon i/git-branch-minus :on-click on-materialize}
(tr "workspace.branches.materialize.action")]
[:> button* {:variant "primary" :icon i/history :on-click on-restore}
(tr "workspace.branches.menu.restore")]])]]))
;; --- Branch list entry
@ -769,7 +871,7 @@
(fn [event]
(dom/stop-propagation event)
(on-set-menu false)
(st/emit! (dwb/archive-branch (:id entry) (not archived?)))))
(confirm-archive! entry (not archived?))))
on-delete
(mf/use-fn
@ -777,12 +879,7 @@
(fn [event]
(dom/stop-propagation event)
(on-set-menu false)
(st/emit! (modal/show {:type :confirm
:title (tr "workspace.branches.delete.title")
:message (tr "workspace.branches.delete.message" (:name entry))
:accept-label (tr "labels.delete")
:accept-style :danger
:on-accept (fn [_] (st/emit! (dwb/delete-branch (:id entry))))}))))
(confirm-delete! entry)))
;; the exit door: the branch stops being a branch but keeps its
;; content, and the confirmation says so, because the branch row and
@ -908,6 +1005,7 @@
(when-not merged?
[:> menu-item* {:id "materialize" :on-action on-materialize}
(tr "workspace.branches.materialize.action")])
[:> menu-separator* {}]
[:> menu-item* {:id "delete" :on-action on-delete}
(tr "labels.delete")]]])]]))
@ -915,12 +1013,11 @@
(defn- branches-for-tab
"The branch rows a tab of the all-branches dialog lists: the open ones,
the open ones the profile created, the merged ones, or the archived
ones."
the open ones the profile created, or the archived ones. A merge deletes
its branch, so no tab lists merged branches."
[tab rows profile-id]
(case tab
"mine" (filter #(and (= "open" (:status %)) (= profile-id (:created-by %))) rows)
"merged" (filter #(= "merged" (:status %)) rows)
"archived" (filter #(= "archived" (:status %)) rows)
(filter #(= "open" (:status %)) rows)))
@ -937,7 +1034,7 @@
profile-id (:id profile)
profiles (cond-> profiles
profile-id (assoc profile-id profile))
profile-id (assoc profile-id (avatar-profile profile)))
prs-by-branch
(mf/with-memo [prs]
@ -955,7 +1052,16 @@
;; only one entry's options dropdown may be open at a time
open-menu* (mf/use-state nil)
open-menu (deref open-menu*)
set-menu (mf/use-fn (fn [k open?] (reset! open-menu* (when open? k))))
;; the DS menu closes every other open menu when one opens, and
;; each of them answers with `open?` false: closing one entry's
;; menu must not clear the menu another entry just opened
set-menu (mf/use-fn
(fn [k open?]
(swap! open-menu* (fn [current]
(cond
open? k
(= current k) nil
:else current)))))
matching
(mf/with-memo [data filter-v]
@ -968,7 +1074,6 @@
(let [label (fn [text t] (dm/str text " (" (count (branches-for-tab t data profile-id)) ")"))]
[{:id "mine" :label (label (tr "workspace.branches.list.tab.mine") "mine")}
{:id "active" :label (label (tr "workspace.branches.list.tab.active") "active")}
{:id "merged" :label (label (tr "workspace.branches.merged") "merged")}
{:id "archived" :label (label (tr "workspace.branches.archived") "archived")}]))
rows (branches-for-tab tab matching profile-id)
@ -976,14 +1081,16 @@
on-tab-change (mf/use-fn #(reset! tab* %))
on-filter-change (mf/use-fn #(reset! filter* (dom/get-target-val %)))
;; the input's wrapper takes the full width it is given: this
;; box sets that width, so the tabs keep room for their labels
search
(mf/html
[:> input* {:variant "dense"
:icon i/search
:class (stl/css :branches-list-search)
:placeholder (tr "workspace.branches.search.placeholder")
:value filter-v
:on-change on-filter-change}])
[:div {:class (stl/css :branches-list-search)}
[:> input* {:variant "dense"
:icon i/search
:placeholder (tr "workspace.branches.search.placeholder")
:value filter-v
:on-change on-filter-change}]])
on-open-change
(mf/use-fn
@ -995,10 +1102,7 @@
(mf/use-fn #(modal/show! :branch-limits {}))
on-create
(mf/use-fn
(mf/deps file)
(fn [_]
(modal/show! :create-branch {:file-name (:name file)})))
(mf/use-fn #(modal/show! :create-branch {}))
on-retry
(mf/use-fn #(st/emit! (dwb/init-branches-state)))]
@ -1186,7 +1290,7 @@
(cond
(nil? item)
[:div {:class (stl/css :compare-detail-empty)}
[:> empty-state* {:icon i/switch
[:> empty-state* {:icon i/git-compare-arrows
:text (tr "workspace.branches.compare.select-hint")}]]
:else
@ -1291,7 +1395,7 @@
:on-open-change (prui/on-dismiss on-close)
:size "xlarge"}
[:> prui/dialog-header*
{:icon i/switch
{:icon i/git-compare-arrows
:title (tr "workspace.branches.compare.title")
:subtitle
(mf/html
@ -1332,7 +1436,7 @@
(cond
(= status :loading)
[:div {:class (stl/css :compare-empty)}
[:> empty-state* {:icon i/switch
[:> empty-state* {:icon i/git-compare-arrows
:text (tr "workspace.branches.compare.loading")}]]
(= status :error)
@ -1342,7 +1446,7 @@
(empty? items)
[:div {:class (stl/css :compare-empty)}
[:> empty-state* {:icon i/git-branch
[:> empty-state* {:icon i/git-compare-arrows
:text (tr "workspace.branches.compare.empty")}]]
:else
@ -1428,7 +1532,7 @@
(cond
conflicts?
[:> button* {:variant "primary"
:icon i/git-merge
:icon i/git-merge-conflict
:on-click on-resolve}
(tr "workspace.branches.conflicts.resolve")]
@ -1651,16 +1755,11 @@
(mf/defc branch-conflicts-dialog*
{::mf/register modal/components
::mf/register-as :branch-conflicts}
[{:keys [branch mode conflicts excluded]}]
[{:keys [branch mode conflicts excluded comment]}]
(let [{:keys [diff selected resolutions status]} (mf/deref branch-diff)
loading? (= status :loading)
;; merge mode only: whether to keep the branch archived after merge
keep?* (mf/use-state false)
keep? (deref keep?*)
on-toggle-keep (mf/use-fn #(swap! keep?* not))
diff-meta (:meta diff)
;; per-side header subtitle: base is pinned at branch creation, main
;; and branch show how recent each one is ("3 hours ago"); the branch
@ -1705,12 +1804,13 @@
#(when sel (st/emit! (dwb/set-conflict-resolution (:id sel) :main))))
on-use-branch (mf/use-fn (mf/deps sel)
#(when sel (st/emit! (dwb/set-conflict-resolution (:id sel) :branch))))
on-apply (mf/use-fn (mf/deps branch resolutions mode keep? excluded)
;; a merge sends again the comment of the merge dialog
on-apply (mf/use-fn (mf/deps branch resolutions mode excluded comment)
#(st/emit! (if (= mode :update)
(dwb/update-branch-from-main branch {:resolutions resolutions
:excluded excluded})
(dwb/merge-branch branch {:resolutions resolutions
:keep-branch keep?
:comment comment
:excluded excluded}))))]
;; the fetched diff backs both the fallback conflict set and the side
@ -1724,7 +1824,7 @@
[:> modal* {:is-open true
:on-open-change (prui/on-dismiss on-close)
:size "xlarge"}
[:> prui/dialog-header* {:icon i/triangle-alert
[:> prui/dialog-header* {:icon i/git-merge-conflict
:tone :warning
:title (tr "workspace.branches.conflicts.title")
:subtitle (tr "workspace.branches.conflicts.subtitle" (:name branch))}
@ -1747,12 +1847,12 @@
(cond
loading?
[:div {:class (stl/css :compare-empty)}
[:> empty-state* {:icon i/git-merge
[:> empty-state* {:icon i/git-merge-conflict
:text (tr "workspace.branches.compare.loading")}]]
(empty? conflicts)
[:div {:class (stl/css :compare-empty)}
[:> empty-state* {:icon i/git-merge
[:> empty-state* {:icon i/git-merge-conflict
:text (tr "workspace.branches.conflicts.empty")}]]
:else
@ -1837,13 +1937,6 @@
:end
(mf/html
[:*
;; applying a MERGE deletes the branch unless the user opts to
;; keep it archived — same choice the plain merge dialog offers
(when (not= mode :update)
[:> checkbox* {:id "conflicts-archive-branch"
:label (tr "workspace.branches.merge.archive-label")
:checked keep?
:on-change on-toggle-keep}])
[:> button* {:variant "secondary" :on-click on-close} (tr "labels.cancel")]
[:> button* {:variant "primary"
:icon i/git-merge
@ -1945,6 +2038,27 @@
(= (:id ctx) (:file-branch-id %))))
(first)))
profile (mf/deref refs/profile)
team (mf/deref refs/team)
pr-author? (and (some? open-pr) (= (:created-by open-pr) (:id profile)))
admin? (boolean (get-in team [:permissions :is-admin]))
can-merge? (boolean (get-in team [:permissions :can-edit]))
open? (not (or merged? archived?))
ahead (or (:ahead ctx) 0)
behind (or (:behind ctx) 0)
;; the one action an open branch offers as its primary button, most
;; pressing first; the options menu leaves it out
primary
(when open?
(cond
(pos? (or (:conflicts ctx) 0)) :resolve
(pos? behind) :update
(and prs-enabled? (some? open-pr)) :open-review
(and prs-enabled? (pos? ahead)) :request-review
(pos? ahead) :merge))
menu-open* (mf/use-state false)
menu-open? (deref menu-open*)
on-toggle-menu (mf/use-fn #(swap! menu-open* not))
@ -1953,7 +2067,7 @@
on-compare (mf/use-fn (mf/deps ctx) #(modal/show! :branch-compare {:branch ctx}))
on-update (mf/use-fn (mf/deps ctx) #(confirm-update! ctx))
on-open-main (mf/use-fn (mf/deps ctx) #(st/emit! (dwb/open-branch (:source-file-id ctx))))
on-restore (mf/use-fn (mf/deps ctx) #(st/emit! (dwb/archive-branch (:id ctx) false)))
on-restore (mf/use-fn (mf/deps ctx) #(confirm-archive! ctx false))
on-materialize (mf/use-fn (mf/deps ctx) #(confirm-materialize! ctx))
on-resolve (mf/use-fn (mf/deps ctx) #(modal/show! :branch-conflicts {:branch ctx :mode :merge}))
@ -1974,6 +2088,27 @@
(mf/deps open-pr)
#(st/emit! (dwpr/open-pull-request open-pr)))
;; the same options as the branch's entry in the branch list;
;; deleting the open branch lands on the file it comes from
on-menu-action
(mf/use-fn
(mf/deps ctx open-pr)
(fn [action]
(case action
"info" (modal/show! :branch-info {:branch ctx})
"update" (confirm-update! ctx)
"request-review" (modal/show! :create-pull-request {:branch ctx})
"open-review" (st/emit! (dwpr/open-pull-request open-pr))
"review-details" (modal/show! :pull-request-info {:pr open-pr})
"publish-review" (st/emit! (dwpr/update-pull-request-snapshot (:id open-pr)))
"merge" (confirm-merge! ctx)
"cancel-review" (prui/confirm-close! open-pr)
"rename" (modal/show! :rename-branch {:branch ctx})
"archive" (confirm-archive! ctx true)
"materialize" (confirm-materialize! ctx)
"delete" (confirm-delete! ctx (:source-file-id ctx))
nil)))
;; publishes the room the banner takes from the bottom of the canvas
;; whenever its box changes size, which is what keeps the floating
;; colour/typography palette bar clear of it in both the collapsed and
@ -2041,85 +2176,122 @@
[:span {:class (stl/css :branch-banner-title)} (:label status)]]
[:div {:class (stl/css :branch-banner-separator)}]
(cond
merged?
[:> button* {:variant "secondary"
:icon i/arrow-up-right
:on-click on-open-main}
(tr "workspace.branches.banner.view-main")]
;; an archived branch was set aside unmerged and is read-only: it
;; can be compared with main, brought back to work, or taken out
;; as an ordinary file
archived?
[:div {:class (stl/css :branch-banner-actions)}
[:div {:class (stl/css :branch-banner-actions)}
(cond
merged?
[:> button* {:variant "secondary"
:icon i/switch
:on-click on-compare}
(tr "workspace.branches.compare")]
[:> button* {:variant "secondary"
:icon i/document
:on-click on-materialize}
(tr "workspace.branches.materialize.action")]
[:> button* {:variant "primary"
:icon i/history
:on-click on-restore}
(tr "workspace.branches.menu.restore")]]
:icon i/arrow-up-right
:on-click on-open-main}
(tr "workspace.branches.banner.view-main")]
:else
(let [conflicts (or (:conflicts ctx) 0)]
[:div {:class (stl/css :branch-banner-actions)}
[:> button* {:variant "secondary"
:icon i/switch
:on-click on-compare}
(tr "workspace.branches.compare")]
;; an archived branch was set aside unmerged and is read-only: it
;; can be compared with main, brought back to work, or taken out
;; as an ordinary file
archived?
[:*
[:> button* {:variant "secondary"
:icon i/git-compare-arrows
:on-click on-compare}
(tr "workspace.branches.compare")]
[:> button* {:variant "secondary"
:icon i/git-branch-minus
:on-click on-materialize}
(tr "workspace.branches.materialize.action")]
[:> button* {:variant "primary"
:icon i/history
:on-click on-restore}
(tr "workspace.branches.menu.restore")]]
(cond
(pos? conflicts)
[:> button* {:variant "primary"
:icon i/triangle-alert
:on-click on-resolve}
(tr "workspace.branches.conflicts.resolve")]
:else
[:*
[:> button* {:variant "secondary"
:icon i/git-compare-arrows
:on-click on-compare}
(tr "workspace.branches.compare")]
(pos? (or (:behind ctx) 0))
[:> button* {:variant "primary"
:icon i/status-update
:on-click on-update}
(tr "workspace.branches.update")]
;; with pull requests enabled the primary flow is asking for a
;; review (merging moves to the options menu)
(case primary
:resolve
[:> button* {:variant "primary"
:icon i/git-merge-conflict
:on-click on-resolve}
(tr "workspace.branches.conflicts.resolve")]
;; with pull requests enabled the primary flow is asking
;; for a review (merging moves behind the kebab menu)
(and prs-enabled? (some? open-pr))
[:> button* {:variant "primary"
:icon i/git-pull-request-arrow
:on-click on-open-review}
(tr "workspace.pull-requests.actions.open-review")]
:update
[:> button* {:variant "primary"
:icon i/status-update
:on-click on-update}
(tr "workspace.branches.update")]
(and prs-enabled? (pos? (or (:ahead ctx) 0)))
[:> button* {:variant "primary"
:icon i/git-pull-request-arrow
:on-click on-request-review}
(tr "workspace.pull-requests.menu.request-review")]
:open-review
[:> button* {:variant "primary"
:icon i/git-pull-request-arrow
:on-click on-open-review}
(tr "workspace.pull-requests.actions.open-review")]
(pos? (or (:ahead ctx) 0))
[:> button* {:variant "primary"
:icon i/git-merge
:on-click on-merge}
(tr "workspace.branches.merge.action")])
:request-review
[:> button* {:variant "primary"
:icon i/git-pull-request-create-arrow
:on-click on-request-review}
(tr "workspace.pull-requests.menu.request-review")]
(when prs-enabled?
;; the banner sits at the bottom of the canvas: its menu
;; opens upwards
[:div {:class (stl/css :branch-banner-menu)}
[:> menu* {:is-open menu-open?
:on-open-change on-menu-change
:placement "top end"
:trigger
(mf/html
[:> icon-button* {:variant "ghost"
:icon i/menu
:aria-label (tr "labels.options")
:aria-pressed menu-open?
:on-click on-toggle-menu}])}
[:> menu-item* {:id "merge" :on-action on-merge}
(tr "workspace.pull-requests.actions.merge")]]])]))]])))
:merge
[:> button* {:variant "primary"
:icon i/git-merge
:on-click on-merge}
(tr "workspace.branches.merge.action")]
nil)])
;; the branch's options, minus what the banner already offers as
;; a button; the banner sits at the bottom of the canvas, so the
;; menu opens upwards
[:div {:class (stl/css :branch-banner-menu)}
[:> menu* {:is-open menu-open?
:on-open-change on-menu-change
:on-action on-menu-action
:placement "top end"
:trigger
(mf/html
[:> icon-button* {:variant "ghost"
:icon i/menu
:aria-label (tr "labels.options")
:aria-pressed menu-open?
:on-click on-toggle-menu}])}
[:> menu-item* {:id "info"}
(tr "workspace.branches.menu.info")]
(when (and open? (pos? behind) (not= primary :update))
[:> menu-item* {:id "update"}
(tr "workspace.branches.update")])
(when (and open? prs-enabled? (nil? open-pr) (not= primary :request-review))
[:> menu-item* {:id "request-review"}
(tr "workspace.pull-requests.menu.request-review")])
(when (and (some? open-pr) (not= primary :open-review))
[:> menu-item* {:id "open-review"}
(tr "workspace.pull-requests.actions.open-review")])
(when (some? open-pr)
[:> menu-item* {:id "review-details"}
(tr "workspace.pull-requests.menu.review-details")])
(when pr-author?
[:> menu-item* {:id "publish-review"}
(tr "workspace.pull-requests.menu.publish")])
(when (and open? prs-enabled? (not= primary :merge)
(or (nil? open-pr) can-merge?))
[:> menu-item* {:id "merge"}
(tr "workspace.pull-requests.actions.merge")])
(when (and (some? open-pr) (or pr-author? admin?))
[:> menu-item* {:id "cancel-review"}
(tr "workspace.pull-requests.actions.close")])
(when open?
[:> menu-item* {:id "rename"}
(tr "labels.rename")])
(when open?
[:> menu-item* {:id "archive"}
(tr "workspace.branches.menu.archive")])
(when open?
[:> menu-item* {:id "materialize"}
(tr "workspace.branches.materialize.action")])
[:> menu-separator* {}]
[:> menu-item* {:id "delete"}
(tr "labels.delete")]]]]]])))

10
frontend/src/app/main/ui/workspace/sidebar/branches.scss
View File

@ -14,6 +14,7 @@
.branches-empty {
display: flex;
flex-direction: column;
align-items: center;
gap: var(--sp-xxxl);
margin: var(--sp-xxxl) auto;
inline-size: $sz-200;
@ -154,8 +155,8 @@
}
// Merge-dialog confirmation prompt: a regular-weight, secondary-color
// paragraph sitting between the title and the archive controls, with a
// touch more breathing room before the checkbox.
// paragraph sitting between the title and the comment field, with a
// touch more breathing room before it.
.modal-message {
@include use-typography("body-large");
@ -1112,7 +1113,7 @@
.branch-banner {
position: absolute;
inset-block-end: var(--sp-m);
inset-block-end: var(--sp-xs);
inset-inline-start: 50%;
transform: translateX(-50%);
max-inline-size: 90vw;
@ -1253,8 +1254,7 @@
}
.branches-list-search {
inline-size: $sz-224;
margin-inline-start: auto;
inline-size: $sz-192;
}
.branches-list-entries {

6
frontend/src/app/main/ui/workspace/sidebar/pull_requests.cljs
View File

@ -210,7 +210,7 @@
[:> modal* {:is-open true
:on-open-change (on-dismiss on-close)
:size "large"}
[:> dialog-header* {:icon i/git-pull-request-arrow
[:> dialog-header* {:icon i/git-pull-request-create-arrow
:title (tr "workspace.pull-requests.create.title")
:subtitle (tr "workspace.pull-requests.create.subtitle" (or (:name branch) ""))}]
@ -255,7 +255,7 @@
[:> button* {:variant "secondary" :on-click on-close}
(tr "labels.cancel")]
[:> button* {:variant "primary"
:icon i/git-pull-request-arrow
:icon i/git-pull-request-create-arrow
:disabled (not valid?)
:on-click on-submit}
(tr "workspace.pull-requests.create.submit")]]]))
@ -608,7 +608,7 @@
[:div {:class (stl/css :pr-banner-actions)}
[:> button* {:variant "secondary"
:icon i/switch
:icon i/git-compare-arrows
:on-click on-compare}
(tr "workspace.branches.compare")]

405
frontend/src/app/main/ui/workspace/sidebar/reviews.cljs Normal file
View File

@ -0,0 +1,405 @@
;; 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 app.main.ui.workspace.sidebar.reviews
"The reviews dialog: the merges into main, each with who made it, when
and the comment left with it, and the design reviews (pull requests) of
the file. It follows the all-branches dialog: tabs with their counts,
a search box and one list per tab."
(:require-macros [app.main.style :as stl])
(:require
[app.common.data.macros :as dm]
[app.common.time :as ct]
[app.main.data.modal :as modal]
[app.main.data.workspace.branches :as dwb]
[app.main.data.workspace.pull-requests :as dwpr]
[app.main.refs :as refs]
[app.main.store :as st]
[app.main.ui.ds.buttons.button :refer [button*]]
[app.main.ui.ds.buttons.icon-button :refer [icon-button*]]
[app.main.ui.ds.controls.input :refer [input*]]
[app.main.ui.ds.foundations.assets.icon :as i]
[app.main.ui.ds.layout.menu :refer [menu* menu-item*]]
[app.main.ui.ds.layout.modal :refer [modal* modal-header* modal-content*]]
[app.main.ui.ds.layout.tab-switcher :refer [tab-switcher*]]
[app.main.ui.ds.product.avatar :refer [avatar*]]
[app.main.ui.ds.product.branch-pill :refer [branch-pill*]]
[app.main.ui.ds.product.empty-state :refer [empty-state*]]
[app.main.ui.workspace.sidebar.pull-requests :as prui :refer [pr-state-badge*]]
[app.util.dom :as dom]
[app.util.i18n :refer [tr]]
[app.util.keyboard :as kbd]
[cuerdas.core :as str]
[okulary.core :as l]
[rumext.v2 :as mf]))
(def ^:private merges
(l/derived :workspace-branch-merges st/state))
(defn- matches?
"Whether any of `values` contains the search text, ignoring case. An
empty search matches everything."
[filter-v values]
(or (str/blank? filter-v)
(let [needle (str/lower filter-v)]
(some #(str/includes? (str/lower (or % "")) needle) values))))
;; --- Shared pieces
(mf/defc clamped-text*
"A text of any length shown on two lines at most. When it does not fit,
a \"See more\" link shows the rest and \"See less\" folds it back. The
link is a button of its own, so it never acts on the entry around it."
{::mf/private true}
[{:keys [text]}]
(let [text-ref (mf/use-ref nil)
expanded* (mf/use-state false)
expanded? (deref expanded*)
overflow* (mf/use-state false)
overflow? (deref overflow*)
on-toggle
(mf/use-fn
(fn [event]
(dom/stop-propagation event)
(swap! expanded* not)))]
;; measured only while folded: unfolded, the text always fits
(mf/with-effect [text expanded?]
(when-not expanded?
(when-let [node (mf/ref-val text-ref)]
(let [measure #(reset! overflow* (> (.-scrollHeight node) (.-clientHeight node)))
observer (js/ResizeObserver. measure)]
(measure)
(.observe observer node)
#(.disconnect observer)))))
[:div {:class (stl/css :clamped)}
[:p {:ref text-ref
:class (stl/css-case :clamped-text true
:is-folded (not expanded?))}
text]
(when (or overflow? expanded?)
[:button {:type "button"
:class (stl/css :see-more)
:aria-expanded expanded?
:on-click on-toggle}
(if expanded?
(tr "workspace.branches.reviews.see-less")
(tr "workspace.branches.reviews.see-more"))])]))
(mf/defc entry-date*
"When something happened, as time ago, with the full date on hover."
{::mf/private true}
[{:keys [date]}]
(when date
(let [date (ct/inst date)]
[:time {:class (stl/css :entry-date)
:date-time (ct/format-inst date :iso)
:title (ct/format-inst date :localized-date-time)}
(ct/timeago date)])))
(mf/defc route*
"The branch the changes come from and main, where they land."
{::mf/private true}
[{:keys [branch-name is-review]}]
[:div {:class (stl/css :entry-route)}
[:> branch-pill* {:label (or branch-name "—")
:is-review is-review}]
[:> i/icon* {:icon-id i/arrow-right
:size "s"
:class (stl/css :entry-route-arrow)}]
[:> branch-pill* {:label (tr "workspace.branches.main")
:is-main true}]])
(mf/defc author*
"The profile behind an entry: avatar and name. A profile that left the
team is no longer among the team's profiles and shows as unknown."
{::mf/private true}
[{:keys [profile]}]
(if profile
[:> avatar* {:profile profile
:variant "M"
:class (stl/css :entry-avatar)}]
[:div {:class (stl/css :entry-avatar :entry-avatar-unknown)}
[:> i/icon* {:icon-id i/user :size "s"}]]))
;; --- Merge reviews
(mf/defc merge-entry*
{::mf/private true}
[{:keys [merge profiles]}]
(let [author (get profiles (:merged-by merge))]
[:li {:class (stl/css :entry)}
[:> author* {:profile author}]
[:div {:class (stl/css :entry-body)}
[:div {:class (stl/css :entry-head)}
[:span {:class (stl/css :entry-title)}
(or (:fullname author) (tr "workspace.branches.reviews.unknown-author"))]
[:> entry-date* {:date (:created-at merge)}]]
[:div {:class (stl/css :entry-meta)}
[:span {:class (stl/css :entry-meta-text)}
(tr "workspace.branches.reviews.merged")]
[:> route* {:branch-name (:branch-name merge)}]]
;; the merge accepted a design review: name it, it may be gone by now
(when-let [title (:pull-request-title merge)]
[:div {:class (stl/css :entry-review)}
[:> i/icon* {:icon-id i/git-pull-request-arrow :size "s"}]
[:span {:class (stl/css :entry-review-title)}
(tr "workspace.branches.reviews.from-review" title)]])
(if (str/blank? (:comment merge))
[:p {:class (stl/css :entry-no-comment)}
(tr "workspace.branches.reviews.no-comment")]
[:> clamped-text* {:text (:comment merge)}])]]))
;; --- Design reviews
(mf/defc design-review-entry*
"A design review: the entry opens it while it is open; a finished one
(merged or closed) can no longer be opened, so the entry shows its
details instead. Its options menu shows the details of every review and
reopens a closed one for its author or a team admin, as the details
dialog does."
{::mf/private true}
[{:keys [pr profiles]}]
(let [author (get profiles (:created-by pr))
profile (mf/deref refs/profile)
team (mf/deref refs/team)
open? (= "open" (:status pr))
closed? (= "closed" (:status pr))
can-reopen? (and closed?
(or (= (:created-by pr) (:id profile))
(boolean (get-in team [:permissions :is-admin]))))
[approvals total] (prui/current-approvals pr)
;; each entry keeps its own menu state: the DS menu closes every
;; other open menu by itself when one opens
menu-open* (mf/use-state false)
menu-open? (deref menu-open*)
on-toggle-menu (mf/use-fn #(swap! menu-open* not))
on-menu-change (mf/use-fn #(reset! menu-open* %))
on-menu-action
(mf/use-fn
(mf/deps pr)
(fn [action]
(case action
"details" (modal/show! :pull-request-info {:pr pr})
"reopen" (st/emit! (dwpr/reopen-pull-request (:id pr)))
nil)))
on-open
(mf/use-fn
(mf/deps pr open?)
(fn [_]
(if open?
(st/emit! (modal/hide)
(dwpr/open-pull-request pr))
(modal/show! :pull-request-info {:pr pr}))))
on-key-down
(mf/use-fn
(mf/deps on-open)
(fn [event]
(when (or (kbd/enter? event) (kbd/space? event))
(dom/prevent-default event)
(on-open event))))]
[:li {:class (stl/css :entry :is-clickable)
:role "button"
:tab-index 0
:title (if open?
(tr "workspace.pull-requests.actions.open-review")
(tr "workspace.pull-requests.menu.review-details"))
:on-click on-open
:on-key-down on-key-down}
[:> author* {:profile author}]
[:div {:class (stl/css :entry-body)}
[:div {:class (stl/css :entry-head)}
[:span {:class (stl/css :entry-title)} (:title pr)]
[:> pr-state-badge* {:pr pr}]
[:> entry-date* {:date (:created-at pr)}]]
[:div {:class (stl/css :entry-meta)}
[:span {:class (stl/css :entry-meta-text)}
(or (:fullname author) (tr "workspace.branches.reviews.unknown-author"))]
[:> route* {:branch-name (:branch-name pr) :is-review open?}]
(when (pos? total)
[:span {:class (stl/css :entry-approvals)
:title (tr "workspace.pull-requests.info.approvals")}
[:> i/icon* {:icon-id i/tick :size "s"}]
(dm/str approvals "/" total)])]
(when-not (str/blank? (:description pr))
[:> clamped-text* {:text (:description pr)}])]
;; the entry opens the review on click and on Enter: neither the
;; menu, its trigger nor its items (portaled, but still in this
;; React tree) may reach it
[:div {:class (stl/css :entry-aside)
:on-click dom/stop-propagation
:on-key-down dom/stop-propagation}
[:> menu* {:is-open menu-open?
:on-open-change on-menu-change
:on-action on-menu-action
:placement "bottom end"
:trigger
(mf/html
[:> icon-button* {:variant "ghost"
:icon i/menu
:aria-label (tr "labels.options")
:aria-pressed menu-open?
:on-click on-toggle-menu}])}
[:> menu-item* {:id "details"}
(tr "workspace.pull-requests.menu.review-details")]
(when can-reopen?
[:> menu-item* {:id "reopen"}
(tr "workspace.pull-requests.actions.reopen")])]]]))
;; --- Dialog
(mf/defc list-state*
"What a tab shows while its list is not there: loading, a failure with
a retry, or an empty list."
{::mf/private true}
[{:keys [status icon empty-text on-retry]}]
(case status
:loading
[:div {:class (stl/css :reviews-empty) :role "status"}
[:> empty-state* {:icon i/git-pull-request-arrow
:text (tr "workspace.branches.reviews.loading")}]]
:error
[:div {:class (stl/css :reviews-empty) :role "alert"}
[:> empty-state* {:icon i/triangle-alert
:text (tr "workspace.branches.reviews.error")}]
[:> button* {:variant "secondary"
:icon i/reload
:on-click on-retry}
(tr "labels.retry")]]
[:div {:class (stl/css :reviews-empty)}
[:> empty-state* {:icon icon :text empty-text}]]))
(mf/defc reviews-dialog*
{::mf/register modal/components
::mf/register-as :branch-reviews}
[]
(let [merges-state (mf/deref merges)
prs-state (mf/deref refs/pull-requests)
profiles (mf/deref refs/profiles)
prs-enabled? (dwpr/enabled?)
tab* (mf/use-state "merges")
tab (deref tab*)
filter* (mf/use-state "")
filter-v (deref filter*)
all-merges (:data merges-state)
all-prs (when prs-enabled? (:data prs-state))
;; the search reads what each entry shows: the branch, the
;; people and the texts
shown-merges
(mf/with-memo [all-merges filter-v profiles]
(filterv #(matches? filter-v [(:branch-name %)
(:comment %)
(:pull-request-title %)
(get-in profiles [(:merged-by %) :fullname])])
all-merges))
shown-prs
(mf/with-memo [all-prs filter-v profiles]
(filterv #(matches? filter-v [(:title %)
(:description %)
(:branch-name %)
(get-in profiles [(:created-by %) :fullname])])
all-prs))
tabs
(mf/with-memo [all-merges all-prs prs-enabled?]
(cond-> [{:id "merges"
:label (dm/str (tr "workspace.branches.reviews.tab.merges")
" (" (count all-merges) ")")}]
prs-enabled?
(conj {:id "design"
:label (dm/str (tr "workspace.branches.reviews.tab.design")
" (" (count all-prs) ")")})))
on-tab-change (mf/use-fn #(reset! tab* %))
on-filter-change (mf/use-fn #(reset! filter* (dom/get-target-val %)))
;; the input's wrapper takes the full width it is given: this
;; box sets that width, so the tabs keep room for their labels
search
(mf/html
[:div {:class (stl/css :reviews-search)}
[:> input* {:variant "dense"
:icon i/search
:placeholder (tr "workspace.branches.reviews.search")
:value filter-v
:on-change on-filter-change}]])
on-open-change
(mf/use-fn
(fn [open?]
(when-not open?
(st/emit! (modal/hide)))))
on-retry-merges
(mf/use-fn #(st/emit! (dwb/fetch-branch-merges)))]
;; before paint, so a list from an earlier opening never shows in
;; place of the loading state
(mf/with-layout-effect []
(st/emit! (dwb/fetch-branch-merges))
(when (dwpr/enabled?)
(st/emit! (dwpr/fetch-pull-requests))))
[:> modal* {:is-open true
:on-open-change on-open-change
:size "large"}
[:> modal-header* {:title (tr "workspace.branches.reviews.title")}]
[:> modal-content* {:class (stl/css :reviews-content)}
[:> tab-switcher* {:tabs tabs
:selected tab
:on-change on-tab-change
:scrollable-panel true
:action-button-position "end"
:action-button search
:class (stl/css :reviews-tabs)}
(if (= tab "design")
(cond
(= :loading (:status prs-state))
[:> list-state* {:status :loading}]
(empty? shown-prs)
[:> list-state* {:icon i/git-pull-request-arrow
:empty-text (if (seq all-prs)
(tr "workspace.branches.reviews.no-results")
(tr "workspace.branches.reviews.design.empty"))}]
:else
[:ul {:class (stl/css :entries)}
(for [pr shown-prs]
[:> design-review-entry* {:key (dm/str (:id pr))
:pr pr
:profiles profiles}])])
(cond
(contains? #{:loading :error} (:status merges-state))
[:> list-state* {:status (:status merges-state)
:on-retry on-retry-merges}]
(empty? shown-merges)
[:> list-state* {:icon i/git-merge
:empty-text (if (seq all-merges)
(tr "workspace.branches.reviews.no-results")
(tr "workspace.branches.reviews.merges.empty"))}]
:else
[:ul {:class (stl/css :entries)}
(for [merge shown-merges]
[:> merge-entry* {:key (dm/str (:id merge))
:merge merge
:profiles profiles}])]))]]]))

240
frontend/src/app/main/ui/workspace/sidebar/reviews.scss Normal file
View File

@ -0,0 +1,240 @@
// 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
@use "ds/_borders.scss" as *;
@use "ds/_sizes.scss" as *;
@use "ds/mixins.scss" as *;
@use "ds/typography.scss" as *;
// --- Dialog: the layout of the all-branches dialog
.reviews-content {
display: flex;
flex-direction: column;
gap: var(--sp-m);
block-size: $sz-480;
min-block-size: 0;
padding: 0;
}
.reviews-tabs {
flex: 1;
min-block-size: 0;
}
.reviews-search {
inline-size: $sz-192;
}
.reviews-empty {
display: flex;
flex-direction: column;
align-items: center;
gap: var(--sp-xxxl);
margin: var(--sp-xxxl) auto;
inline-size: $sz-200;
}
// --- Entries
.entries {
display: flex;
flex-direction: column;
gap: var(--sp-xs);
margin: 0;
padding: var(--sp-m) 0 0;
list-style: none;
}
.entry {
// the card of the comment or the description: a step lighter than the
// row, also while a clickable row is hovered
--entry-card-bg-color: var(--color-background-tertiary);
display: grid;
grid-template-columns: auto 1fr auto;
align-items: start;
gap: var(--sp-m);
padding: var(--sp-m) var(--sp-s);
border-radius: $br-8;
&.is-clickable {
cursor: pointer;
&:hover {
--entry-card-bg-color: var(--color-background-quaternary);
background-color: var(--color-background-tertiary);
}
&:focus-visible {
outline: $b-1 solid var(--color-accent-primary);
}
}
}
.entry-avatar {
flex-shrink: 0;
}
// a profile that left the team: a neutral tile the size of the avatar
.entry-avatar-unknown {
display: flex;
align-items: center;
justify-content: center;
inline-size: $sz-32;
block-size: $sz-32;
border-radius: $br-circle;
background-color: var(--color-background-quaternary);
color: var(--color-foreground-secondary);
}
.entry-body {
display: flex;
flex-direction: column;
gap: var(--sp-xs);
min-inline-size: 0;
}
.entry-head {
display: flex;
align-items: center;
gap: var(--sp-s);
min-inline-size: 0;
}
.entry-title {
@include use-typography("body-medium");
min-inline-size: 0;
overflow: hidden;
white-space: nowrap;
text-overflow: ellipsis;
color: var(--color-foreground-primary);
}
.entry-date {
@include use-typography("body-small");
flex-shrink: 0;
margin-inline-start: auto;
color: var(--color-foreground-secondary);
}
.entry-meta {
display: flex;
flex-wrap: wrap;
align-items: center;
gap: var(--sp-s);
min-inline-size: 0;
}
.entry-meta-text {
@include use-typography("body-small");
color: var(--color-foreground-secondary);
}
.entry-route {
display: flex;
align-items: center;
gap: var(--sp-xs);
min-inline-size: 0;
}
.entry-route-arrow {
flex-shrink: 0;
color: var(--color-foreground-secondary);
}
.entry-review {
@include use-typography("body-small");
display: flex;
align-items: center;
gap: var(--sp-xs);
min-inline-size: 0;
color: var(--color-foreground-secondary);
}
.entry-review-title {
min-inline-size: 0;
overflow: hidden;
white-space: nowrap;
text-overflow: ellipsis;
}
.entry-approvals {
@include use-typography("body-small");
display: inline-flex;
align-items: center;
gap: var(--sp-xxs);
color: var(--color-foreground-secondary);
}
// the empty card of the branch info dialog's description
.entry-no-comment {
@include use-typography("body-medium");
margin: 0;
margin-block-start: var(--sp-xs);
padding: var(--sp-s);
border: $b-1 dashed var(--color-background-quaternary);
border-radius: $br-8;
font-style: italic;
color: var(--color-foreground-secondary);
}
.entry-aside {
display: flex;
align-items: center;
}
// --- A long text folded to two lines, in the card of the branch info
// dialog's description
.clamped {
display: flex;
flex-direction: column;
align-items: flex-start;
gap: var(--sp-xs);
margin-block-start: var(--sp-xs);
padding: var(--sp-s);
border-radius: $br-8;
background-color: var(--entry-card-bg-color);
}
.clamped-text {
@include use-typography("body-medium");
margin: 0;
color: var(--color-foreground-primary);
white-space: pre-line;
overflow-wrap: anywhere;
}
.is-folded {
@include two-line-text-ellipsis;
}
.see-more {
@include use-typography("body-small");
padding: 0;
border: none;
background: none;
color: var(--color-accent-primary);
cursor: pointer;
&:hover {
text-decoration: underline;
}
&:focus-visible {
outline: $b-1 solid var(--color-accent-primary);
}
}

71
frontend/test/frontend_tests/data/workspace_branches_test.cljs
View File

@ -223,6 +223,29 @@
(done'))
done))))
(t/deftest the-comment-travels-with-the-merge-and-its-conflicts
(t/async done
(let [calls (atom [])
conflict-id (uuid/next)]
(mock/with-mocks
{rp/cmd! (cmd-stub calls conflict-id)}
(fn [done']
(let [branch {:id (uuid/next)}
store (make-store)]
(ptk/emit! store (dwb/merge-branch branch {:comment " Ships the header "}))
(t/is (= "Ships the header" (:comment (last-params calls :merge)))
"the comment is trimmed")
(t/testing "the conflict answer hands the comment to the modal"
(t/is (= :branch-conflicts (get-in @store [::modal/modal :type])))
(t/is (= " Ships the header "
(get-in @store [::modal/modal :props :comment]))))
(t/testing "a blank comment sends no `:comment` at all"
(ptk/emit! store (dwb/merge-branch branch {:comment " "}))
(t/is (not (contains? (last-params calls :merge) :comment))))
(rx/dispose! store))
(done'))
done))))
;;; --- Branch switcher: the entry point of the menu and the shortcut ---
(defn- make-store-with
@ -310,9 +333,10 @@
(t/is (true? (dwb/has-branches? @store))
"a count that failed never hides the switcher")
(ptk/emit! store (dwb/show-branches))
(await (async/wait-for #(true? (:workspace-branch-switcher @store))
"the shortcut opens the switcher, which retries"))
(t/is (nil? (::modal/modal @store)) "no create dialog on an unknown count")
(await (async/wait-for #(some? (::modal/modal @store))
"the shortcut opens the branches dialog, which retries"))
(t/is (= :branches-list (get-in @store [::modal/modal :type]))
"no create dialog on an unknown count")
(rx/dispose! store)))))
(t/deftest ^:async a-listing-after-a-failed-count-settles-it
@ -375,15 +399,15 @@
(t/is (true? (:workspace-branch-switcher @store)) "forcing it open twice keeps it open")
(rx/dispose! store)))
(t/deftest ^:async show-branches-opens-the-switcher-when-there-are-branches
(t/deftest ^:async show-branches-opens-the-branches-dialog-when-there-are-branches
(await (mock/with-mocks*
{cf/flags (conj cf/flags :branching)}
(let [store (make-store-with main-with-branches)]
(let [store (make-store-with (assoc main-with-branches :workspace-branch-switcher true))]
(ptk/emit! store (dwb/show-branches))
(await (async/wait-for #(true? (:workspace-branch-switcher @store))
"the switcher opens"))
(await (async/settle))
(t/is (nil? (::modal/modal @store)) "no dialog opens")
(await (async/wait-for #(some? (::modal/modal @store))
"the branches dialog opens"))
(t/is (= :branches-list (get-in @store [::modal/modal :type])))
(t/is (false? (:workspace-branch-switcher @store)) "the switcher closes")
(rx/dispose! store)))))
(t/deftest ^:async show-branches-asks-for-the-first-branch-when-there-are-none
@ -394,7 +418,6 @@
(await (async/wait-for #(some? (::modal/modal @store))
"the create dialog opens"))
(t/is (= :create-branch (get-in @store [::modal/modal :type])))
(t/is (= "Checkout" (get-in @store [::modal/modal :props :file-name])))
(t/is (not (:workspace-branch-switcher @store)))
(rx/dispose! store)))))
@ -449,3 +472,31 @@
(t/is (= [{:file-id branch-id}] @navigations)
"it opens the branch that was created")
(rx/dispose! store))))))
;;; --- Reviews dialog: the merges into main ---
(t/deftest ^:async the-merges-load-for-the-open-file
(let [calls (atom [])
merges [{:id (uuid/next) :branch-name "header" :comment "Ships it"}]]
(await (mock/with-mocks*
{rp/cmd! (mock/stub (fn [cmd params]
(swap! calls conj [cmd params])
(rx/of merges)))}
(let [store (make-store-with main-without-branches)]
(ptk/emit! store (dwb/fetch-branch-merges))
(await (async/wait-for #(= :loaded (get-in @store [:workspace-branch-merges :status]))
"the merges land"))
(t/is (= merges (get-in @store [:workspace-branch-merges :data])))
(t/is (= [[:get-file-branch-merges {:file-id file-id}]] @calls)
"the open file is asked as is: the command resolves a branch to main")
(rx/dispose! store))))))
(t/deftest ^:async a-failed-merges-load-is-an-error-not-an-empty-list
(await (mock/with-mocks*
{rp/cmd! (mock/stub (fn [_ _] (rx/throw (ex-info "network" {}))))}
(let [store (make-store-with main-without-branches)]
(ptk/emit! store (dwb/fetch-branch-merges))
(await (async/wait-for #(= :error (get-in @store [:workspace-branch-merges :status]))
"the failure lands"))
(t/is (nil? (get-in @store [:workspace-branch-merges :data])))
(rx/dispose! store)))))

131
frontend/translations/en.po
View File

@ -6355,6 +6355,14 @@ msgstr "Text Transform"
msgid "workspace.assets.ungroup"
msgstr "Ungroup"
msgid "workspace.branches.archive.message"
msgstr ""
"Archive branch \"%s\"? It becomes read-only and leaves the active list. You "
"can restore it later."
msgid "workspace.branches.archive.title"
msgstr "Archive branch"
#: src/app/main/ui/workspace/sidebar/branches.cljs:836
msgid "workspace.branches.archived"
msgstr "Archived"
@ -6909,10 +6917,6 @@ msgstr "e.g. redesign-checkout"
msgid "workspace.branches.create.submit"
msgstr "Create branch"
#: src/app/main/ui/workspace/sidebar/branches.cljs:276
msgid "workspace.branches.create.subtitle"
msgstr "Derived from main · %s"
#: src/app/main/data/workspace/branches.cljs:201
msgid "workspace.branches.create.success"
msgstr "Branch \"%s\" created"
@ -7171,14 +7175,13 @@ msgstr "Main"
msgid "workspace.branches.main-advanced"
msgstr "Main has new changes. You can update this branch"
msgid "workspace.branches.main-library-note"
msgstr "Read by library consumers"
#: src/app/main/ui/workspace/sidebar/branches.cljs:659
msgid "workspace.branches.main-subtitle"
msgstr "Default branch"
#: src/app/main/ui/workspace/branch_switcher.cljs:88
msgid "workspace.branches.main-subtitle-library"
msgstr "Default branch (read by library consumers)"
#: src/app/main/ui/workspace/sidebar/branches.cljs:779
msgid "workspace.branches.materialize.accept"
msgstr "Turn it into a file"
@ -7225,15 +7228,8 @@ msgstr "Restore"
msgid "workspace.branches.merge.action"
msgstr "Merge into main"
#: src/app/main/ui/workspace/sidebar/branches.cljs:370, src/app/main/ui/workspace/sidebar/branches.cljs:1526
msgid "workspace.branches.merge.archive-label"
msgstr "Archive this branch"
#: src/app/main/ui/workspace/sidebar/branches.cljs:376
msgid "workspace.branches.merge.archive-warning"
msgstr ""
"Archived branches keep a full copy of the file so they can be restored "
"later. Keeping many archived branches can take up a lot of storage."
msgid "workspace.branches.merge.comment-label"
msgstr "Merge comment"
#: src/app/main/ui/workspace/sidebar/branches.cljs:386
msgid "workspace.branches.merge.confirm-accept"
@ -7256,9 +7252,7 @@ msgstr "This branch has conflicts that must be resolved before merging"
#: src/app/main/ui/workspace/sidebar/branches.cljs:378
msgid "workspace.branches.merge.delete-info"
msgstr ""
"After merging, this branch and its file will be deleted to free up storage. "
"Archive it if you might need to restore it later."
msgstr "After merging, this branch and its file will be deleted to free up storage."
#: src/app/main/data/workspace/branches.cljs:451, src/app/main/data/workspace/branches.cljs:459
msgid "workspace.branches.merge.error"
@ -7284,14 +7278,66 @@ msgstr ""
"This branch changes elements that can't be merged automatically yet "
"(components, pages or tokens)"
#: src/app/main/ui/workspace/branch_switcher.cljs:208, src/app/main/ui/workspace/sidebar/branches.cljs:943
msgid "workspace.branches.merged"
msgstr "Merged"
#, unused
msgid "workspace.branches.new"
msgstr "New"
msgid "workspace.branches.rename.title"
msgstr "Rename branch"
msgid "workspace.branches.restore.message"
msgstr ""
"Restore branch \"%s\"? It goes back to the active list and you can edit it "
"again."
msgid "workspace.branches.restore.title"
msgstr "Restore branch"
msgid "workspace.branches.reviews.design.empty"
msgstr "No design reviews yet"
msgid "workspace.branches.reviews.error"
msgstr "Couldn't load the merges"
msgid "workspace.branches.reviews.from-review"
msgstr "From the design review \"%s\""
msgid "workspace.branches.reviews.loading"
msgstr "Loading reviews…"
msgid "workspace.branches.reviews.merged"
msgstr "Merged"
msgid "workspace.branches.reviews.merges.empty"
msgstr "No branch has been merged into main yet"
msgid "workspace.branches.reviews.no-comment"
msgstr "No comment"
msgid "workspace.branches.reviews.no-results"
msgstr "No reviews match your search"
msgid "workspace.branches.reviews.search"
msgstr "Search review…"
msgid "workspace.branches.reviews.see-less"
msgstr "See less"
msgid "workspace.branches.reviews.see-more"
msgstr "See more"
msgid "workspace.branches.reviews.tab.design"
msgstr "Design reviews"
msgid "workspace.branches.reviews.tab.merges"
msgstr "Merge reviews"
msgid "workspace.branches.reviews.title"
msgstr "Reviews"
msgid "workspace.branches.reviews.unknown-author"
msgstr "Unknown user"
#: src/app/main/ui/workspace/sidebar/branches.cljs:787
msgid "workspace.branches.search.placeholder"
msgstr "Search branch…"
@ -7332,6 +7378,9 @@ msgstr "New branch"
msgid "workspace.branches.switcher.new-from-main"
msgstr "Branches are created from main"
msgid "workspace.branches.switcher.reviews"
msgstr "Reviews"
#: src/app/main/ui/workspace/branch_switcher.cljs:309
msgid "workspace.branches.switcher.tooltip"
msgstr "Switch branch"
@ -9369,7 +9418,7 @@ msgstr "TRY PLUGIN"
#: src/app/main/ui/workspace/sidebar/pull_requests.cljs:426, src/app/main/ui/workspace/sidebar/pull_requests.cljs:505, src/app/main/ui/workspace/sidebar/pull_requests.cljs:564
msgid "workspace.pull-requests.actions.close"
msgstr "Cancel pull request"
msgstr "Cancel design review"
#: src/app/main/ui/workspace/sidebar/pull_requests.cljs:440, src/app/main/ui/workspace/sidebar/pull_requests.cljs:561, src/app/main/ui/workspace/sidebar/pull_requests.cljs:754
msgid "workspace.pull-requests.actions.merge"
@ -9381,7 +9430,7 @@ msgstr "Open review"
#: src/app/main/ui/workspace/sidebar/pull_requests.cljs:430, src/app/main/ui/workspace/sidebar/pull_requests.cljs:567
msgid "workspace.pull-requests.actions.reopen"
msgstr "Reopen pull request"
msgstr "Reopen design review"
#: src/app/main/ui/workspace/sidebar/pull_requests.cljs:359, src/app/main/ui/workspace/sidebar/pull_requests.cljs:537, src/app/main/ui/workspace/sidebar/pull_requests.cljs:722
msgid "workspace.pull-requests.badge.conflicts"
@ -9409,7 +9458,7 @@ msgstr "Review"
#: src/app/main/ui/workspace/sidebar/pull_requests.cljs:710
msgid "workspace.pull-requests.banner.reviewing"
msgstr "Reviewing pull request"
msgstr "Reviewing the design review"
#: src/app/main/ui/workspace/sidebar/pull_requests.cljs:504
msgid "workspace.pull-requests.close.message"
@ -9419,11 +9468,11 @@ msgstr ""
#: src/app/main/ui/workspace/sidebar/pull_requests.cljs:503
msgid "workspace.pull-requests.close.title"
msgstr "Cancel pull request"
msgstr "Cancel design review"
#: src/app/main/data/workspace/pull_requests.cljs:142, src/app/main/data/workspace/pull_requests.cljs:255
msgid "workspace.pull-requests.create.already-exists"
msgstr "This branch already has an open pull request"
msgstr "This branch already has an open design review"
#: src/app/main/ui/workspace/sidebar/pull_requests.cljs:167
msgid "workspace.pull-requests.create.description-label"
@ -9435,7 +9484,7 @@ msgstr "What changed, and what needs review or validation?"
#: src/app/main/data/workspace/pull_requests.cljs:143
msgid "workspace.pull-requests.create.error"
msgstr "An error occurred while creating the pull request"
msgstr "An error occurred while creating the design review"
#: src/app/main/ui/workspace/sidebar/pull_requests.cljs:193
msgid "workspace.pull-requests.create.info"
@ -9462,7 +9511,7 @@ msgstr "Reviewers"
#: src/app/main/ui/workspace/sidebar/pull_requests.cljs:202
msgid "workspace.pull-requests.create.submit"
msgstr "Create pull request"
msgstr "Create design review"
#: src/app/main/ui/workspace/sidebar/pull_requests.cljs:152
msgid "workspace.pull-requests.create.subtitle"
@ -9470,15 +9519,15 @@ msgstr "Request a review of the changes on \"%s\""
#: src/app/main/data/workspace/pull_requests.cljs:137
msgid "workspace.pull-requests.create.success"
msgstr "Pull request \"%s\" created"
msgstr "Design review \"%s\" created"
#: src/app/main/ui/workspace/sidebar/pull_requests.cljs:150
msgid "workspace.pull-requests.create.title"
msgstr "Create pull request"
msgstr "Create design review"
#: src/app/main/ui/workspace/left_header.cljs:163
msgid "workspace.pull-requests.header-badge-tooltip"
msgstr "Reviewing pull request: %s"
msgstr "Reviewing the design review: %s"
#: src/app/main/ui/workspace/left_header.cljs:177
msgid "workspace.pull-requests.header-label"
@ -9514,19 +9563,19 @@ msgstr "Review updated"
#: src/app/main/data/workspace/pull_requests.cljs:252
msgid "workspace.pull-requests.lifecycle.branch-gone"
msgstr "The pull request's branch is no longer open"
msgstr "The design review's branch is no longer open"
#: src/app/main/data/workspace/pull_requests.cljs:230
msgid "workspace.pull-requests.lifecycle.closed"
msgstr "Pull request closed"
msgstr "Design review closed"
#: src/app/main/data/workspace/pull_requests.cljs:160, src/app/main/data/workspace/pull_requests.cljs:187, src/app/main/data/workspace/pull_requests.cljs:213, src/app/main/data/workspace/pull_requests.cljs:237, src/app/main/data/workspace/pull_requests.cljs:257
msgid "workspace.pull-requests.lifecycle.error"
msgstr "An error occurred while updating the pull request"
msgstr "An error occurred while updating the design review"
#: src/app/main/data/workspace/pull_requests.cljs:246
msgid "workspace.pull-requests.lifecycle.reopened"
msgstr "Pull request reopened"
msgstr "Design review reopened"
#: src/app/main/ui/workspace/sidebar/pull_requests.cljs:555, src/app/main/ui/workspace/sidebar/pull_requests.cljs:731
msgid "workspace.pull-requests.menu.info"
@ -9550,7 +9599,7 @@ msgstr "New"
#: src/app/main/data/workspace/pull_requests.cljs:396
msgid "workspace.pull-requests.preview.unavailable"
msgstr "The review sandbox is not available (the pull request may be closed)"
msgstr "The review sandbox is not available (the design review may be closed)"
#: src/app/main/data/workspace/pull_requests.cljs:182
msgid "workspace.pull-requests.publish.success"
@ -9618,7 +9667,7 @@ msgstr "Submit review"
#: src/app/main/ui/workspace/sidebar/pull_requests.cljs:632
msgid "workspace.pull-requests.section.empty"
msgstr "No pull requests yet. Create one from a branch to request a review."
msgstr "No design reviews yet. Create one from a branch to request a review."
#: src/app/main/ui/workspace/sidebar/pull_requests.cljs:639
msgid "workspace.pull-requests.section.history"
@ -9626,7 +9675,7 @@ msgstr "Finished"
#: src/app/main/ui/workspace/sidebar/pull_requests.cljs:611
msgid "workspace.pull-requests.section.title"
msgstr "Pull requests"
msgstr "Design reviews"
#: src/app/main/ui/workspace/sidebar/pull_requests.cljs:100
#, unused