diff --git a/backend/resources/app/email/review-request/en.html b/backend/resources/app/email/review-request/en.html
index a9f1651d5a..1301724caa 100644
--- a/backend/resources/app/email/review-request/en.html
+++ b/backend/resources/app/email/review-request/en.html
@@ -186,7 +186,7 @@
|
- {{ 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 }}).
|
diff --git a/backend/resources/app/email/review-request/en.txt b/backend/resources/app/email/review-request/en.txt
index 84e12bb126..fb29439fc3 100644
--- a/backend/resources/app/email/review-request/en.txt
+++ b/backend/resources/app/email/review-request/en.txt
@@ -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 }}).
--
diff --git a/backend/src/app/migrations.clj b/backend/src/app/migrations.clj
index 4ffc31dadf..ddb2502c23 100644
--- a/backend/src/app/migrations.clj
+++ b/backend/src/app/migrations.clj
@@ -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]
diff --git a/backend/src/app/migrations/sql/0806-add-file-branch-merge-table.sql b/backend/src/app/migrations/sql/0806-add-file-branch-merge-table.sql
new file mode 100644
index 0000000000..63879fcdf4
--- /dev/null
+++ b/backend/src/app/migrations/sql/0806-add-file-branch-merge-table.sql
@@ -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);
diff --git a/backend/src/app/rpc/commands/files_branch.clj b/backend/src/app/rpc/commands/files_branch.clj
index 5adcfa7e4b..8dfd51f4c2 100644
--- a/backend/src/app/rpc/commands/files_branch.clj
+++ b/backend/src/app/rpc/commands/files_branch.clj
@@ -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
diff --git a/backend/test/backend_tests/rpc_file_branch_oplog_test.clj b/backend/test/backend_tests/rpc_file_branch_oplog_test.clj
index ae4b6d3a3a..b469c63862 100644
--- a/backend/test/backend_tests/rpc_file_branch_oplog_test.clj
+++ b/backend/test/backend_tests/rpc_file_branch_oplog_test.clj
@@ -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)))))))
diff --git a/backend/test/backend_tests/rpc_file_branch_test.clj b/backend/test/backend_tests/rpc_file_branch_test.clj
index bc01053a30..fed5b53d70 100644
--- a/backend/test/backend_tests/rpc_file_branch_test.clj
+++ b/backend/test/backend_tests/rpc_file_branch_test.clj
@@ -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)))))))
diff --git a/backend/test/backend_tests/rpc_file_pull_request_test.clj b/backend/test/backend_tests/rpc_file_pull_request_test.clj
index fc082eee7d..496695650f 100644
--- a/backend/test/backend_tests/rpc_file_pull_request_test.clj
+++ b/backend/test/backend_tests/rpc_file_pull_request_test.clj
@@ -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})
diff --git a/frontend/resources/images/icons/git-branch-minus.svg b/frontend/resources/images/icons/git-branch-minus.svg
new file mode 100644
index 0000000000..e1d7392190
--- /dev/null
+++ b/frontend/resources/images/icons/git-branch-minus.svg
@@ -0,0 +1,6 @@
+
diff --git a/frontend/resources/images/icons/git-compare-arrows.svg b/frontend/resources/images/icons/git-compare-arrows.svg
new file mode 100644
index 0000000000..4bb584c4ec
--- /dev/null
+++ b/frontend/resources/images/icons/git-compare-arrows.svg
@@ -0,0 +1,8 @@
+
diff --git a/frontend/resources/images/icons/git-graph.svg b/frontend/resources/images/icons/git-graph.svg
new file mode 100644
index 0000000000..715b79c033
--- /dev/null
+++ b/frontend/resources/images/icons/git-graph.svg
@@ -0,0 +1,8 @@
+
diff --git a/frontend/resources/images/icons/git-merge-conflict.svg b/frontend/resources/images/icons/git-merge-conflict.svg
new file mode 100644
index 0000000000..a0ecbf4632
--- /dev/null
+++ b/frontend/resources/images/icons/git-merge-conflict.svg
@@ -0,0 +1,7 @@
+
diff --git a/frontend/resources/images/icons/git-pull-request-create-arrow.svg b/frontend/resources/images/icons/git-pull-request-create-arrow.svg
new file mode 100644
index 0000000000..472b28b162
--- /dev/null
+++ b/frontend/resources/images/icons/git-pull-request-create-arrow.svg
@@ -0,0 +1,8 @@
+
diff --git a/frontend/src/app/main/data/workspace/branches.cljs b/frontend/src/app/main/data/workspace/branches.cljs
index be5396dc6f..ee5c27ad60 100644
--- a/frontend/src/app/main/data/workspace/branches.cljs
+++ b/frontend/src/app/main/data/workspace/branches.cljs
@@ -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
diff --git a/frontend/src/app/main/ui/ds/foundations/assets/icon.cljs b/frontend/src/app/main/ui/ds/foundations/assets/icon.cljs
index c3f17bce70..d287ff53ac 100644
--- a/frontend/src/app/main/ui/ds/foundations/assets/icon.cljs
+++ b/frontend/src/app/main/ui/ds/foundations/assets/icon.cljs
@@ -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")
diff --git a/frontend/src/app/main/ui/ds/product/branch_row.cljs b/frontend/src/app/main/ui/ds/product/branch_row.cljs
index 4e850fb875..a44a6cc3fb 100644
--- a/frontend/src/app/main/ui/ds/product/branch_row.cljs
+++ b/frontend/src/app/main/ui/ds/product/branch_row.cljs
@@ -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)}
diff --git a/frontend/src/app/main/ui/ds/product/branch_row.mdx b/frontend/src/app/main/ui/ds/product/branch_row.mdx
index c4813c0ced..b2657fe781 100644
--- a/frontend/src/app/main/ui/ds/product/branch_row.mdx
+++ b/frontend/src/app/main/ui/ds/product/branch_row.mdx
@@ -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))
diff --git a/frontend/src/app/main/ui/workspace/branch_switcher.cljs b/frontend/src/app/main/ui/workspace/branch_switcher.cljs
index 9cbf08a4cb..d652af2a57 100644
--- a/frontend/src/app/main/ui/workspace/branch_switcher.cljs
+++ b/frontend/src/app/main/ui/workspace/branch_switcher.cljs
@@ -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}]]])))
diff --git a/frontend/src/app/main/ui/workspace/branch_switcher.scss b/frontend/src/app/main/ui/workspace/branch_switcher.scss
index 8f7a794cab..8a7466fc13 100644
--- a/frontend/src/app/main/ui/workspace/branch_switcher.scss
+++ b/frontend/src/app/main/ui/workspace/branch_switcher.scss
@@ -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);
diff --git a/frontend/src/app/main/ui/workspace/main_menu.cljs b/frontend/src/app/main/ui/workspace/main_menu.cljs
index e9a4e2b837..8809a2ebc3 100644
--- a/frontend/src/app/main/ui/workspace/main_menu.cljs
+++ b/frontend/src/app/main/ui/workspace/main_menu.cljs
@@ -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]
diff --git a/frontend/src/app/main/ui/workspace/sidebar/branches.cljs b/frontend/src/app/main/ui/workspace/sidebar/branches.cljs
index d2d3c0b908..cb1b533c72 100644
--- a/frontend/src/app/main/ui/workspace/sidebar/branches.cljs
+++ b/frontend/src/app/main/ui/workspace/sidebar/branches.cljs
@@ -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")]]]]]])))
diff --git a/frontend/src/app/main/ui/workspace/sidebar/branches.scss b/frontend/src/app/main/ui/workspace/sidebar/branches.scss
index 41a3aa8715..ba94c39431 100644
--- a/frontend/src/app/main/ui/workspace/sidebar/branches.scss
+++ b/frontend/src/app/main/ui/workspace/sidebar/branches.scss
@@ -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 {
diff --git a/frontend/src/app/main/ui/workspace/sidebar/pull_requests.cljs b/frontend/src/app/main/ui/workspace/sidebar/pull_requests.cljs
index e511624e85..f9c485f97f 100644
--- a/frontend/src/app/main/ui/workspace/sidebar/pull_requests.cljs
+++ b/frontend/src/app/main/ui/workspace/sidebar/pull_requests.cljs
@@ -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")]
diff --git a/frontend/src/app/main/ui/workspace/sidebar/reviews.cljs b/frontend/src/app/main/ui/workspace/sidebar/reviews.cljs
new file mode 100644
index 0000000000..3289ad2c01
--- /dev/null
+++ b/frontend/src/app/main/ui/workspace/sidebar/reviews.cljs
@@ -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}])]))]]]))
diff --git a/frontend/src/app/main/ui/workspace/sidebar/reviews.scss b/frontend/src/app/main/ui/workspace/sidebar/reviews.scss
new file mode 100644
index 0000000000..63de4b3bb7
--- /dev/null
+++ b/frontend/src/app/main/ui/workspace/sidebar/reviews.scss
@@ -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);
+ }
+}
diff --git a/frontend/test/frontend_tests/data/workspace_branches_test.cljs b/frontend/test/frontend_tests/data/workspace_branches_test.cljs
index bedb5b630b..bd5d712932 100644
--- a/frontend/test/frontend_tests/data/workspace_branches_test.cljs
+++ b/frontend/test/frontend_tests/data/workspace_branches_test.cljs
@@ -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)))))
diff --git a/frontend/translations/en.po b/frontend/translations/en.po
index 841378027e..1f3a8077bb 100644
--- a/frontend/translations/en.po
+++ b/frontend/translations/en.po
@@ -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