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

✨ Tell merged and archived branches apart (#12572)

Browse Source 
* 🐛 Refuse saves to a branch kept after its merge

A branch kept after its merge stays an ordinary file with status
"merged", so every save passed the edition check and appended ops to
an op log that no command integrates any more. Refuse such saves with
a :branch-merged validation error instead of stranding them. An
archived branch still takes saves: restoring it brings them back.

AI-assisted-by: claude-opus-5-5
Signed-off-by: Álvaro Tejero Cantero <alvorithm@teje.ro>

* ✨ Tell merged and archived branches apart

The workspace treated "merged" and "archived" as one state: an
archived branch read "Merged branch" and lost every action, and a
merged row offered Restore, which the backend always refused.

Each status now has its own presentation. A kept merged branch keeps
"Merged branch" and View in main, and opens read-only: the workspace
permissions drop :can-edit while it is the open file, the way a
viewer gets a read-only workspace. An archived branch says so in the
banner and offers Compare and Restore. The header notes each status,
the switcher lists merged and archived branches in two collapsible
groups, the all-branches dialog gets a Merged tab, and the branch info
dialog offers Restore and Compare for an archived branch.

AI-assisted-by: claude-opus-5-5
Signed-off-by: Álvaro Tejero Cantero <alvorithm@teje.ro>

* ✨ Refuse saves to an archived branch

An archived branch is now read-only, like a merged one: the user
restores it, or turns it into an ordinary file, to keep editing.
persist-branch-file! refuses its saves with a :branch-archived
validation error, distinct from :branch-merged so the client can say
which way out applies. A restored branch takes saves again, and so
does an archived branch turned into an ordinary file.

AI-assisted-by: claude-opus-5-5
Signed-off-by: Álvaro Tejero Cantero <alvorithm@teje.ro>

* ✨ Open archived branches read-only with their ways out

An archived branch now opens read-only, like a merged one. Its banner,
its branch info dialog and its row in the all-branches dialog offer
the two ways back to editing, Restore and Turn into an ordinary file,
next to Compare. The header and switcher note reads "Archived, read
only".

A save the server refuses because the branch is merged or archived
now shows a warning that names the way out, instead of the generic
autosave error with a report link.

AI-assisted-by: claude-opus-5-5
Signed-off-by: Álvaro Tejero Cantero <alvorithm@teje.ro>

---------

Signed-off-by: Álvaro Tejero Cantero <alvorithm@teje.ro>
This commit is contained in:
Álvaro Tejero-Cantero 2026-10-22 20:54:15 +02:00 committed by GitHub
parent d669eec0c0
commit dcedd40a42
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 561 additions and 101 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
backend/src/app/rpc/commands/files_update.clj
View File

@ -429,6 +429,25 @@
:hint "branch metadata not found for branch file"
:file-id (:id file)))
;; Only an open branch takes saves. A branch kept after its merge has
;; its work in main, and no command integrates ops appended after the
;; merge. An archived branch is read-only until it is restored or
;; turned into an ordinary file. Refuse instead of stranding the ops.
(case (:status branch)
"merged"
(ex/raise :type :validation
:code :branch-merged
:hint "a merged branch is read-only"
:file-id (:id file))
"archived"
(ex/raise :type :validation
:code :branch-archived
:hint "an archived branch is read-only"
:file-id (:id file))
nil)
(db/update! conn :project
{:modified-at modified-at}
{:id (:project-id file)}

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

@ -211,6 +211,80 @@
(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
:color {:id (uuid/random) :name "C" :color "#112233" :opacity 1}}))
(defn- archived-branch*
"A profile and an archived branch of one of its files."
[name]
(let [profile (th/create-profile* 1 {:is-active true})
file (th/create-file* 1 {:profile-id (:id profile)
:project-id (:default-project-id profile)
:is-shared false})
create (create-branch* profile (:id file) name)]
(th/command! {::th/type :archive-file-branch
::rpc/profile-id (:id profile)
:id (:id create)})
[profile create]))
(t/deftest an-archived-branch-refuses-saves
(with-redefs [cf/flags (conj cf/flags :branching)]
(let [[profile create] (archived-branch* "oplog-archived")
branch-file-id (:branch-file-id create)
out (add-color* profile branch-file-id)]
(t/is (th/ex-of-type? (:error out) :validation))
(t/is (th/ex-of-code? (:error out) :branch-archived))
(t/is (empty? (oplog-rows branch-file-id))))))
(t/deftest a-restored-branch-takes-saves-again
(with-redefs [cf/flags (conj cf/flags :branching)]
(let [[profile create] (archived-branch* "oplog-restored")
branch-file-id (:branch-file-id create)]
(th/command! {::th/type :archive-file-branch
::rpc/profile-id (:id profile)
:id (:id create)
:archived false})
(let [out (add-color* profile branch-file-id)]
(t/is (nil? (:error out)))
(t/is (= 1 (count (oplog-rows branch-file-id))))))))
(t/deftest an-archived-branch-turned-into-a-file-takes-saves
(with-redefs [cf/flags (conj cf/flags :branching)]
(let [[profile create] (archived-branch* "oplog-materialized")
branch-file-id (:branch-file-id create)
out (th/command! {::th/type :materialize-file-branch
::rpc/profile-id (:id profile)
:branch-file-id branch-file-id})]
(t/is (nil? (:error out)))
(t/is (nil? (:error (add-color* profile branch-file-id)))))))
(t/deftest update-from-main-compares-in-main-id-space
;; A branch stores no data: its document is the base snapshot plus the op
;; log, so every reference it inherited names MAIN. A local reference is a

4
frontend/playwright/data/pull-requests/archive-file-branch-restore.json Normal file
View File

@ -0,0 +1,4 @@
{
"~:id": "~uc7ce0794-0992-8105-8004-38f280443998",
"~:status": "open"
}

15
frontend/playwright/data/pull-requests/get-file-branch-info-archived.json Normal file
View File

@ -0,0 +1,15 @@
{
"~:id": "~uc7ce0794-0992-8105-8004-38f280443998",
"~:branch-file-id": "~uc7ce0794-0992-8105-8004-38f280443849",
"~:source-file-id": "~uc7ce0794-0992-8105-8004-38f280443997",
"~:name": "parked",
"~:description": null,
"~:status": "archived",
"~:created-by": "~uc7ce0794-0992-8105-8004-38e630f29a9b",
"~:created-at": "~m1713536343369",
"~:updated-at": "~m1713873823633",
"~:source-name": "New File 1",
"~:ahead": 0,
"~:behind": 0,
"~:conflicts": 0
}

16
frontend/playwright/data/pull-requests/get-file-branch-info-merged.json Normal file
View File

@ -0,0 +1,16 @@
{
"~:id": "~uc7ce0794-0992-8105-8004-38f280443998",
"~:branch-file-id": "~uc7ce0794-0992-8105-8004-38f280443849",
"~:source-file-id": "~uc7ce0794-0992-8105-8004-38f280443997",
"~:name": "shipped",
"~:description": null,
"~:status": "merged",
"~:created-by": "~uc7ce0794-0992-8105-8004-38e630f29a9b",
"~:created-at": "~m1713536343369",
"~:updated-at": "~m1713873823633",
"~:source-name": "New File 1",
"~:ahead": 0,
"~:behind": 0,
"~:conflicts": 0,
"~:merged-at": "~m1713873823633"
}

32
frontend/playwright/data/pull-requests/get-file-branches-closed.json Normal file
View File

@ -0,0 +1,32 @@
[
{
"~:id": "~uc7ce0794-0992-8105-8004-38f280443991",
"~:branch-file-id": "~uc7ce0794-0992-8105-8004-38f280443851",
"~:source-file-id": "~uc7ce0794-0992-8105-8004-38f280443997",
"~:name": "shipped",
"~:description": null,
"~:status": "merged",
"~:created-by": "~uc7ce0794-0992-8105-8004-38e630f29a9b",
"~:created-at": "~m1713536343369",
"~:updated-at": "~m1713873823633",
"~:source-name": "New File 1",
"~:ahead": 0,
"~:behind": 0,
"~:conflicts": 0
},
{
"~:id": "~uc7ce0794-0992-8105-8004-38f280443992",
"~:branch-file-id": "~uc7ce0794-0992-8105-8004-38f280443852",
"~:source-file-id": "~uc7ce0794-0992-8105-8004-38f280443997",
"~:name": "parked",
"~:description": null,
"~:status": "archived",
"~:created-by": "~uc7ce0794-0992-8105-8004-38e630f29a9b",
"~:created-at": "~m1713536343369",
"~:updated-at": "~m1713873823633",
"~:source-name": "New File 1",
"~:ahead": 0,
"~:behind": 0,
"~:conflicts": 0
}
]

152
frontend/playwright/ui/specs/merged-and-archived-branches.spec.js Normal file
View File

@ -0,0 +1,152 @@
import { test, expect } from "@playwright/test";
import { WorkspacePage } from "../pages/WorkspacePage";
test.beforeEach(async ({ page }) => {
await WorkspacePage.init(page);
await WorkspacePage.mockConfigFlags(page, ["enable-branching"]);
});
const setupBranchFile = async (workspacePage, branchInfo) => {
await workspacePage.setupEmptyFile();
await workspacePage.mockRPCs({
"get-file-branch-info?file-id=*": `pull-requests/${branchInfo}`,
"get-file-branches?file-id=*":
"pull-requests/get-file-branches-closed.json",
"get-file-pull-requests?file-id=*":
"pull-requests/get-file-branches-empty.json",
"get-branch-diff?branch-id=*": "pull-requests/get-branch-diff-empty.json",
});
};
test("a kept merged branch opens read-only", async ({ page }) => {
const workspacePage = new WorkspacePage(page);
await setupBranchFile(workspacePage, "get-file-branch-info-merged.json");
await workspacePage.goToWorkspace();
const banner = page.locator("#branch-banner");
await expect(banner.getByText("Merged branch")).toBeVisible();
await expect(
banner.getByRole("button", { name: "View in main" }),
).toBeVisible();
await expect(banner.getByRole("button", { name: "Compare" })).toBeHidden();
await expect(banner.getByRole("button", { name: "Restore" })).toBeHidden();
await expect(page.getByText("Merged into main, read only")).toBeVisible();
await expect(
page.getByRole("toolbar", { name: "Drawing tools" }),
).toBeHidden();
});
test("an archived branch opens read-only and offers its ways out", async ({
page,
}) => {
const workspacePage = new WorkspacePage(page);
await setupBranchFile(workspacePage, "get-file-branch-info-archived.json");
await workspacePage.mockRPC(
"archive-file-branch",
"pull-requests/archive-file-branch-restore.json",
);
await workspacePage.goToWorkspace();
const banner = page.locator("#branch-banner");
await expect(banner.getByText("Archived branch")).toBeVisible();
await expect(banner.getByText("Merged branch")).toBeHidden();
await expect(banner.getByRole("button", { name: "Compare" })).toBeVisible();
await expect(
banner.getByRole("button", { name: "Turn into an ordinary file" }),
).toBeVisible();
await expect(page.getByText("Archived, read only")).toBeVisible();
await expect(
page.getByRole("toolbar", { name: "Drawing tools" }),
).toBeHidden();
// restoring reopens the branch, and the banner follows its new status
await workspacePage.mockRPC(
"get-file-branch-info?file-id=*",
"pull-requests/get-file-branch-info-branch.json",
);
const restore = page.waitForRequest(
(req) =>
req.url().includes("archive-file-branch") &&
/"~:archived":\s*false/.test(req.postData() ?? ""),
);
await banner.getByRole("button", { name: "Restore" }).click();
await restore;
await expect(banner.getByText("1 change not in main")).toBeVisible();
await expect(
page.getByRole("toolbar", { name: "Drawing tools" }),
).toBeVisible();
});
test("an archived branch asks before turning into an ordinary file", async ({
page,
}) => {
const workspacePage = new WorkspacePage(page);
await setupBranchFile(workspacePage, "get-file-branch-info-archived.json");
await workspacePage.goToWorkspace();
const banner = page.locator("#branch-banner");
await banner
.getByRole("button", { name: "Turn into an ordinary file" })
.click();
await expect(
page.getByText("Turn this branch into an ordinary file"),
).toBeVisible();
});
test("the switcher groups merged and archived branches apart", async ({
page,
}) => {
const workspacePage = new WorkspacePage(page);
await setupBranchFile(workspacePage, "get-file-branch-info-none.json");
await workspacePage.goToWorkspace();
await page.getByTitle("Switch branch").click();
const popover = page.getByRole("dialog", { name: "Branches" });
await popover.getByRole("button", { name: /^Merged/ }).click();
await expect(popover.getByText("shipped")).toBeVisible();
await expect(popover.getByText("parked")).toBeHidden();
await popover.getByRole("button", { name: /^Archived/ }).click();
await expect(popover.getByText("parked")).toBeVisible();
await expect(popover.getByText("Archived, read only")).toBeVisible();
});
test("the branch list offers Restore for archived branches only", async ({
page,
}) => {
const workspacePage = new WorkspacePage(page);
await setupBranchFile(workspacePage, "get-file-branch-info-none.json");
await workspacePage.goToWorkspace();
await page.getByTitle("Switch branch").click();
await page.getByText("All branches").click();
await page.getByRole("tab", { name: "Merged (1)" }).click();
await page.getByRole("button", { name: "Options", exact: true }).click();
await expect(
page.getByRole("menuitem", { name: "Branch info" }),
).toBeVisible();
await expect(page.getByRole("menuitem", { name: "Restore" })).toBeHidden();
await expect(page.getByRole("menuitem", { name: "Compare" })).toBeHidden();
await expect(
page.getByRole("menuitem", { name: "Turn into an ordinary file" }),
).toBeHidden();
await page.keyboard.press("Escape");
await page.getByRole("tab", { name: "Archived (1)" }).click();
await page.getByRole("button", { name: "Options", exact: true }).click();
await expect(page.getByRole("menuitem", { name: "Restore" })).toBeVisible();
await expect(
page.getByRole("menuitem", { name: "Turn into an ordinary file" }),
).toBeVisible();
await page.getByRole("menuitem", { name: "Branch info" }).click();
const info = page.getByRole("dialog").filter({ hasText: "Branch info" });
await expect(info.getByRole("button", { name: "Restore" })).toBeVisible();
await expect(info.getByRole("button", { name: "Compare" })).toBeVisible();
await expect(
info.getByRole("button", { name: "Turn into an ordinary file" }),
).toBeVisible();
});

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

@ -241,6 +241,8 @@
(rx/catch (fn [cause] (rx/of (refusal-notification cause (tr "workspace.branches.lifecycle.error")))))))))
(defn archive-branch
"Archive a branch, or restore it with `archived?` false. The context
refreshes as well, so the banner of the open branch follows its status."
([id] (archive-branch id true))
([id archived?]
(assert (uuid? id) "expected valid uuid for `id`")
@ -248,7 +250,7 @@
ptk/WatchEvent
(watch [_ _ _]
(->> (rp/cmd! :archive-file-branch {:id id :archived archived?})
(rx/mapcat (fn [_] (rx/of (fetch-branches))))
(rx/mapcat (fn [_] (rx/of (fetch-branches) (fetch-branch-context))))
(rx/catch (fn [cause] (rx/of (refusal-notification cause (tr "workspace.branches.lifecycle.error"))))))))))
(defn delete-branch

13
frontend/src/app/main/errors.cljs
View File

@ -526,11 +526,22 @@
(defn flash-persistence
[cause]
(let [data (ex-data cause)]
(if (delegated-persistence-failure? data)
(cond
(delegated-persistence-failure? data)
;; The persistence state wraps the failure and records the original
;; type under :cause-type; dispatch on it to reach the cause's handler.
(on-error (-> (exception->error-data cause)
(assoc :type (or (:cause-type data) (:type data)))))
;; A merged or archived branch is read-only and the server refuses its
;; saves: an expected refusal, so it is explained, not reported.
(= :branch-merged (:code data))
(st/emit! (ntf/warn (tr "errors.save-refused-merged-branch")))
(= :branch-archived (:code data))
(st/emit! (ntf/warn (tr "errors.save-refused-archived-branch")))
:else
(flash :cause cause
:type :handled
:timeout nil

16
frontend/src/app/main/refs.cljs
View File

@ -74,8 +74,22 @@
(get projects project-id)))
st/state))
(defn- open-closed-branch?
"True when the open file is a merged or an archived branch. A merged
branch has its work in main, and an archived one is set aside until it
is restored or turned into an ordinary file: both open read-only."
[state]
(let [ctx (:workspace-branch-context state)]
(and (contains? #{"merged" "archived"} (get-in ctx [:info :status]))
(= (:file-id ctx) (:current-file-id state)))))
(def permissions
(l/derived (l/key :permissions) team))
"The team permissions, without edition on a merged or archived branch."
(l/derived (fn [state]
(cond-> (:permissions (dsh/lookup-team state))
(open-closed-branch? state)
(assoc :can-edit false)))
st/state))
(def teams
(l/derived (l/key :teams) st/state))

60
frontend/src/app/main/ui/workspace.cljs
View File

@ -147,13 +147,6 @@
:overlay true
:file-loading true}])
(defn- make-team-ref
[team-id]
(l/derived (fn [state]
(let [teams (get state :teams)]
(get teams team-id)))
st/state))
(defn- make-file-ref
[file-id]
(l/derived (fn [state]
@ -213,18 +206,17 @@
(let [layout (mf/deref refs/workspace-layout)
wglobal (mf/deref refs/workspace-global)
team-ref (mf/with-memo [team-id]
(make-team-ref team-id))
file-ref (mf/with-memo [file-id]
(make-file-ref file-id))
team (mf/deref team-ref)
file (mf/deref file-ref)
file-loaded? (get file ::has-data)
file-name (:name file)
permissions (:permissions team)
;; a merged or archived branch takes no edits: `refs/permissions`
;; drops `:can-edit` for it, and the contexts below pass that on
permissions (mf/deref refs/permissions)
read-only? (mf/deref refs/workspace-read-only?)
read-only? (or read-only? (not (:can-edit permissions)))
@ -286,28 +278,30 @@
[:> (mf/provider ctx/current-file-id) {:value file-id}
[:> (mf/provider ctx/current-page-id) {:value page-id}
[:> (mf/provider ctx/design-tokens) {:value design-tokens?}
[:> (mf/provider ctx/workspace-read-only?) {:value read-only?}
[:> modal-container*]
[:> components-debugger*]
[:section {:class (stl/css :workspace)
:style {:background-color background-color
:touch-action "none"
:position "relative"}}
[:> context-menu*]
(when (and file-loaded? page-id)
[:> workspace-inner*
{:page-id page-id
:file-id file-id
:file file
:wglobal wglobal
:layout layout}])
(when (or (not (and file-loaded? page-id))
;; in wasm renderer, extend the pixel loader until the first frame is rendered
;; but do not apply it when switching pages
(and wasm-renderer-enabled?
(not file-loaded?)
(not @first-frame-rendered?)))
[:> workspace-loader*])]]]]]]))
[:> (mf/provider ctx/permissions) {:value permissions}
[:> (mf/provider ctx/can-edit?) {:value (:can-edit permissions)}
[:> (mf/provider ctx/workspace-read-only?) {:value read-only?}
[:> modal-container*]
[:> components-debugger*]
[:section {:class (stl/css :workspace)
:style {:background-color background-color
:touch-action "none"
:position "relative"}}
[:> context-menu*]
(when (and file-loaded? page-id)
[:> workspace-inner*
{:page-id page-id
:file-id file-id
:file file
:wglobal wglobal
:layout layout}])
(when (or (not (and file-loaded? page-id))
;; in wasm renderer, extend the pixel loader until the first frame is rendered
;; but do not apply it when switching pages
(and wasm-renderer-enabled?
(not file-loaded?)
(not @first-frame-rendered?)))
[:> workspace-loader*])]]]]]]]]))
(mf/defc workspace-page*
{::mf/lazy-load true}

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

@ -102,9 +102,35 @@
:is-current (boolean current)
:data-branch (dm/str (:branch-file-id row))
:on-click on-open}
(if (:is-main row)
(main-subtitle (:is-library row))
[:> row-status* {:row row :pr pr}])]])
;; merged and archived branches carry no change counts
(cond
(:is-main row) (main-subtitle (:is-library row))
(= "merged" (:status row)) (tr "workspace.branches.header.merged")
(= "archived" (:status row)) (tr "workspace.branches.header.archived")
:else [:> row-status* {:row row :pr pr}])]])
(mf/defc closed-group*
"A collapsed section of the popover for branches out of the flow: its
title and count open it on click."
{::mf/private true}
[{:keys [label rows on-open]}]
(let [open* (mf/use-state false)
open? (deref open*)
on-toggle (mf/use-fn #(swap! open* not))]
(when (seq rows)
[:*
[:button {:class (stl/css :action)
:aria-expanded open?
:on-click on-toggle}
[:> i/icon* {:icon-id (if open? i/arrow-down i/arrow-right) :size "s"}]
[:span {:class (stl/css :action-label)} label]
[:span {:class (stl/css :action-aside)} (dm/str (count rows))]]
(when open?
[:ul {:class (stl/css :rows)}
(for [row rows]
[:> switcher-row* {:key (dm/str (:branch-file-id row))
:row row
:on-open on-open}])])])))
(mf/defc switcher-popover*
{::mf/private true}
@ -113,8 +139,6 @@
filter* (mf/use-state "")
filter-v (deref filter*)
archived* (mf/use-state false)
show-archived? (deref archived*)
matches?
(fn [row]
@ -138,17 +162,13 @@
main-row (cons main-row)
:always (filter matches?))
archived
(->> rows
(remove #(= "open" (:status %)))
(filter matches?))
with-status
(fn [status]
(filter #(and (= status (:status %)) (matches? %)) rows))
on-filter-change
(mf/use-fn #(reset! filter* (dom/get-target-val %)))
on-toggle-archived
(mf/use-fn #(swap! archived* not))
on-open
(mf/use-fn
(fn [event]
@ -231,20 +251,12 @@
:pr (get prs-by-branch (:id row))
:on-open on-open}])]])
(when (seq archived)
[:*
[:button {:class (stl/css :action)
:aria-expanded show-archived?
:on-click on-toggle-archived}
[:> i/icon* {:icon-id (if show-archived? i/arrow-down i/arrow-right) :size "s"}]
[:span {:class (stl/css :action-label)} (tr "workspace.branches.archived")]
[:span {:class (stl/css :action-aside)} (dm/str (count archived))]]
(when show-archived?
[:ul {:class (stl/css :rows)}
(for [row archived]
[:> switcher-row* {:key (dm/str (:branch-file-id row))
:row 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}]])
[:hr {:class (stl/css :divider)}]
@ -285,8 +297,6 @@
:created-by (:id profile)
:branch-file-id file-id})
merged? (contains? #{"merged" "archived"} (:status ctx))
;; clicks inside the row (chip and popover) must not close it
bar-ref (mf/use-ref nil)
@ -357,8 +367,19 @@
:class (stl/css :status-note)}
(main-subtitle (:is-shared file))]
^boolean merged?
nil
;; merged and archived branches open read-only and count no
;; changes: the note says which of the two it is
(= "merged" (:status ctx))
[:> text* {:as "span"
:typography t/body-small
:class (stl/css :status-note)}
(tr "workspace.branches.header.merged")]
(= "archived" (:status ctx))
[:> text* {:as "span"
:typography t/body-small
:class (stl/css :status-note)}
(tr "workspace.branches.header.archived")]
(in-sync? ctx pr)
[:> text* {:as "span"

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

@ -439,7 +439,7 @@
:on-click on-submit}
(tr "workspace.branches.merge.confirm-accept")]]]))
;; --- Confirmation for relevant branch actions (merge / update)
;; --- Confirmation for relevant branch actions (merge / update / materialize)
(defn- confirm-merge!
"Open the merge dialog for `branch` (merge into main is irreversible; the
@ -463,6 +463,17 @@
:on-accept (fn [_] (st/emit! (dwb/update-branch-from-main branch
{:excluded excluded})))}))))
(defn- confirm-materialize!
"Ask for confirmation before turning `branch` into an ordinary file: the
branch stops being one, so it can no longer be compared or merged."
[branch]
(st/emit! (modal/show {:type :confirm
:title (tr "workspace.branches.materialize.title")
:message (tr "workspace.branches.materialize.message" (:name branch))
:accept-label (tr "workspace.branches.materialize.accept")
:accept-style :danger
:on-accept (fn [_] (st/emit! (dwb/materialize-branch (:branch-file-id branch))))})))
;; --- Diff counts: known, or honestly unknown
;;
;; A row whose comparison could not be made carries no counts at all (and
@ -488,7 +499,9 @@
author? (= (:created-by branch) (:id profile))
status (:status branch)
merged? (= "merged" status)
archived? (contains? #{"archived" "merged"} status)
archived? (= "archived" status)
;; only an open branch carries change counts
open? (= "open" status)
editing? (mf/use-state false)
desc* (mf/use-state (or (:description branch) ""))
@ -506,7 +519,13 @@
(st/emit! (dwb/set-branch-description (:id branch) (str/trim desc)))
(reset! editing? false)))
on-compare (mf/use-fn (mf/deps branch)
#(modal/show! :branch-compare {:branch 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))))
on-materialize (mf/use-fn (mf/deps branch)
#(confirm-materialize! branch))]
[:> modal* {:is-open true
:on-open-change (prui/on-dismiss on-close)}
[:> prui/dialog-header* {:icon i/git-branch
@ -569,7 +588,7 @@
[:dd {:class (stl/css :info-val)}
(some-> (:updated-at branch) ct/inst ct/timeago)]])
(when-not archived?
(when open?
[:div {:class (stl/css :info-row)}
[:dt {:class (stl/css :info-key)} (tr "workspace.branches.info.changes")]
[:dd {:class (stl/css :info-val :info-counts)}
@ -595,7 +614,16 @@
(-> (:merged-at branch) ct/inst (ct/format-inst :localized-date-time))]])]]
[:> modal-footer* {}
(when-not archived?
;; 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}
(tr "workspace.branches.compare")])
[:> button* {:variant "primary" :on-click on-close} (tr "labels.close")]]]))
@ -609,7 +637,11 @@
ahead (:ahead entry)
behind (:behind entry)
main? (:is-main entry)
archived? (contains? #{"archived" "merged"} (:status entry))
merged? (= "merged" (:status entry))
archived? (= "archived" (:status entry))
;; merged and archived branches are out of the flow: no counts, no
;; branch work, and dimmed in the list
closed? (or merged? archived?)
;; `pr` is the branch's open pull request (nil when none): the
;; review is a STATE of the branch, so the entry surfaces it as a
@ -761,15 +793,10 @@
(fn [event]
(dom/stop-propagation event)
(on-set-menu false)
(st/emit! (modal/show {:type :confirm
:title (tr "workspace.branches.materialize.title")
:message (tr "workspace.branches.materialize.message" (:name entry))
:accept-label (tr "workspace.branches.materialize.accept")
:accept-style :danger
:on-accept (fn [_] (st/emit! (dwb/materialize-branch (:branch-file-id entry))))}))))]
(confirm-materialize! entry)))]
[:li {:class (stl/css-case :branch-entry true
:is-archived archived?
:is-archived closed?
:is-current current)
:role "button"
:on-click (when-not current on-open)}
@ -805,7 +832,7 @@
(dm/str approvals "/" total)])])]
[:div {:class (stl/css :branch-entry-aside)}
(when (and (not archived?) (not main?))
(when (and (not closed?) (not main?))
[:div {:class (stl/css :branch-entry-counts)}
(if (counts-error? entry)
[:span {:class (stl/css :count-error)
@ -839,17 +866,17 @@
:on-click on-toggle-menu}])}
[:> menu-item* {:id "info" :on-action on-info}
(tr "workspace.branches.menu.info")]
(when-not archived?
(when-not merged?
[:> menu-item* {:id "compare" :on-action on-compare}
(tr "workspace.branches.compare")])
(when (and (not archived?) (pos? (or behind 0)))
(when (and (not closed?) (pos? (or behind 0)))
[:> menu-item* {:id "update" :on-action on-update}
(tr "workspace.branches.update")])
;; review lifecycle: a branch without an open pull request offers to
;; request one; with it, the review actions take its place (the
;; review either ends merging the branch or is cancelled, leaving
;; the branch as a normal branch again)
(when (and (not archived?) (dwpr/enabled?) (nil? pr))
(when (and (not closed?) (dwpr/enabled?) (nil? pr))
[:> menu-item* {:id "request-review" :on-action on-request-review}
(tr "workspace.pull-requests.menu.request-review")])
(when (some? pr)
@ -867,16 +894,18 @@
(when (and (some? pr) (or author? admin?))
[:> menu-item* {:id "cancel-review" :on-action on-cancel-review}
(tr "workspace.pull-requests.actions.close")])
(when-not archived?
(when-not closed?
[:> menu-item* {:id "rename" :on-action on-start-rename}
(tr "labels.rename")])
[:> menu-item* {:id "archive" :on-action on-archive}
(tr (if archived?
"workspace.branches.menu.restore"
"workspace.branches.menu.archive"))]
;; only an open branch can leave the branch engine: once merged or
;; archived there is nothing left to turn into an ordinary file
(when-not archived?
;; a merged branch cannot go back to work: its changes are in main
(when-not merged?
[:> menu-item* {:id "archive" :on-action on-archive}
(tr (if archived?
"workspace.branches.menu.restore"
"workspace.branches.menu.archive"))])
;; a merged branch has its work in main already: only an open or an
;; archived branch can leave the branch engine as an ordinary file
(when-not merged?
[:> menu-item* {:id "materialize" :on-action on-materialize}
(tr "workspace.branches.materialize.action")])
[:> menu-item* {:id "delete" :on-action on-delete}
@ -886,11 +915,13 @@
(defn- branches-for-tab
"The branch rows a tab of the all-branches dialog lists: the open ones,
the open ones the profile created, or the merged and archived ones."
the open ones the profile created, the merged ones, or the archived
ones."
[tab rows profile-id]
(case tab
"mine" (filter #(and (= "open" (:status %)) (= profile-id (:created-by %))) rows)
"archived" (remove #(= "open" (:status %)) rows)
"merged" (filter #(= "merged" (:status %)) rows)
"archived" (filter #(= "archived" (:status %)) rows)
(filter #(= "open" (:status %)) rows)))
(mf/defc branches-list-dialog*
@ -937,6 +968,7 @@
(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)
@ -1840,18 +1872,23 @@
(defn- banner-status
"What the bottom banner says about the open branch, most pressing
first and in the order of its primary action: merged, conflicts, main
moved, an open review, branch work."
first and in the order of its primary action: merged, archived,
conflicts, main moved, an open review, branch work."
[ctx pr]
(let [ahead (or (:ahead ctx) 0)
behind (or (:behind ctx) 0)
conflicts (or (:conflicts ctx) 0)]
(cond
(contains? #{"merged" "archived"} (:status ctx))
(= "merged" (:status ctx))
{:id :merged
:icon i/git-merge
:label (tr "workspace.branches.banner.merged")}
(= "archived" (:status ctx))
{:id :archived
:icon i/git-branch
:label (tr "workspace.branches.banner.archived")}
(counts-error? ctx)
{:id :counts-error
:icon i/triangle-alert
@ -1887,7 +1924,8 @@
(mf/defc branch-context-banner*
[{:keys [file-id]}]
(let [ctx (mf/deref branch-context)
merged? (contains? #{"merged" "archived"} (:status ctx))
merged? (= "merged" (:status ctx))
archived? (= "archived" (:status ctx))
collapsed* (mf/use-state false)
collapsed? (deref collapsed*)
@ -1915,6 +1953,8 @@
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-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}))
on-merge
@ -1980,7 +2020,7 @@
[:div {:id "branch-banner"
:ref banner-ref
:class (stl/css-case :branch-banner true
:branch-banner-merged merged?
:branch-banner-merged (or merged? archived?)
:is-collapsed collapsed?)}
[:button {:class (stl/css :branch-banner-handle)
:title (tr (if collapsed?
@ -2001,12 +2041,32 @@
[:span {:class (stl/css :branch-banner-title)} (:label status)]]
[:div {:class (stl/css :branch-banner-separator)}]
(if merged?
(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)}
[:> 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")]]
:else
(let [conflicts (or (:conflicts ctx) 0)]
[:div {:class (stl/css :branch-banner-actions)}
[:> button* {:variant "secondary"

19
frontend/test/frontend_tests/main_errors_test.cljs
View File

@ -786,3 +786,22 @@
(let [state (ptk/update (last @events) {})]
(t/is (nil? (get-in state [:notification :timeout])))
(t/is (some? (get-in state [:notification :links 0])))))))))
(t/deftest closed-branch-save-refusals-explain-themselves
(t/testing "a save refused on a merged or archived branch warns without a report"
(doseq [code [:branch-merged :branch-archived]]
(let [events (atom [])
scheduled (atom [])]
(with-redefs [st/emit! (mock/stub (fn [& emitted] (swap! events into emitted)))
tm/schedule (mock/stub (fn [callback]
(swap! scheduled conj callback)))]
(errors/flash-persistence (ex-info "read-only branch" {:type :validation :code code}))
(doseq [callback @scheduled] (callback))
(t/is (empty? (report-events @events)) "no error report for an expected refusal")
(t/is (= 1 (count @events)))
(let [state (ptk/update (first @events) {})]
(t/is (= :warning (get-in state [:notification :level])))
(t/is (= (i18n/tr (if (= code :branch-merged)
"errors.save-refused-merged-branch"
"errors.save-refused-archived-branch"))
(get-in state [:notification :content])))))))))

31
frontend/translations/en.po
View File

@ -1735,12 +1735,23 @@ msgstr ""
msgid "errors.registration-disabled"
msgstr "The registration is currently disabled."
#: src/app/main/errors.cljs:538
#: src/app/main/errors.cljs:549
msgid "errors.save-failed"
msgstr ""
"Autosave is not working due to an error. Contact support to report the "
"error and reload to continue from your last saved version."
#: src/app/main/errors.cljs:542
msgid "errors.save-refused-archived-branch"
msgstr ""
"This branch is archived and read only. Restore it or turn it into an "
"ordinary file to keep editing."
#: src/app/main/errors.cljs:539
msgid "errors.save-refused-merged-branch"
msgstr ""
"This branch is merged into main and read only. Make your changes in main."
#: src/app/main/data/persistence.cljs:307
msgid "errors.save-retrying"
msgstr "Connection lost. Retrying to save your changes."
@ -6348,6 +6359,10 @@ msgstr "Ungroup"
msgid "workspace.branches.archived"
msgstr "Archived"
#: src/app/main/ui/workspace/sidebar/branches.cljs:1840
msgid "workspace.branches.banner.archived"
msgstr "Archived branch"
#, unused
msgid "workspace.branches.banner.collapse"
msgstr "Collapse"
@ -6923,6 +6938,14 @@ msgstr "No branches yet"
msgid "workspace.branches.header-badge-tooltip"
msgstr "On branch %s from main"
#: src/app/main/ui/workspace/branch_switcher.cljs:98, src/app/main/ui/workspace/branch_switcher.cljs:315
msgid "workspace.branches.header.archived"
msgstr "Archived, read only"
#: src/app/main/ui/workspace/branch_switcher.cljs:97, src/app/main/ui/workspace/branch_switcher.cljs:309
msgid "workspace.branches.header.merged"
msgstr "Merged into main, read only"
#: src/app/main/ui/workspace/branch_switcher.cljs:295
msgid "workspace.branches.header-badge-tooltip-library"
msgstr "On branch %s from main. Library consumers can adopt changes from main only."
@ -7160,7 +7183,7 @@ msgstr "Default branch (read by library consumers)"
msgid "workspace.branches.materialize.accept"
msgstr "Turn it into a file"
#: src/app/main/ui/workspace/sidebar/branches.cljs:882
#: src/app/main/ui/workspace/sidebar/branches.cljs:609, src/app/main/ui/workspace/sidebar/branches.cljs:895, src/app/main/ui/workspace/sidebar/branches.cljs:2026
msgid "workspace.branches.materialize.action"
msgstr "Turn into an ordinary file"
@ -7261,6 +7284,10 @@ 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"