mirror of
https://github.com/getpaseo/paseo.git
synced 2026-10-23 01:32:40 +00:00
fix(app): show a promoted git action once in the compact menu (#7355)
The compact Changes toolbar flattens every git action into one menu, and
`secondary` keeps the actions the policy can promote to `primary`. When the
policy promoted one of them - Pull behind origin, Push ahead of it, the merge
and PR actions - it rendered twice, under the same React key.
Dedupe by id in `actionsWithoutPrimary` so the split button and the command
center share one rule, cover it in policy.test.ts against real buildGitActions
output, and pin the rendered menu in a compact Changes browser spec.
(cherry picked from commit 9bb26cc471)
Co-authored-by: az33zy <10743275+az33zy@users.noreply.github.com>
This commit is contained in:
@@ -0,0 +1,115 @@
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { mkdtemp, writeFile } from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { test, expect, type Page } from "../support/fixtures";
|
||||
import { getServerId } from "../support/helpers/server-id";
|
||||
import { connectSeedClient } from "../support/helpers/seed-client";
|
||||
import { createTempGitRepo, resolveTempRoot } from "../support/helpers/workspace";
|
||||
import { buildHostWorkspaceRoute } from "../../src/utils/host-routes";
|
||||
|
||||
// The compact toolbar flattens every git action into one menu, and the policy keeps the actions
|
||||
// it can promote to `primary` in `secondary` as well. A promoted action must still show once.
|
||||
const cleanupTasks: Array<() => Promise<void>> = [];
|
||||
|
||||
test.afterEach(async () => {
|
||||
for (const task of cleanupTasks.splice(0)) {
|
||||
await task();
|
||||
}
|
||||
});
|
||||
|
||||
async function createRemoteRepo(prefix: string): Promise<string> {
|
||||
const repo = await createTempGitRepo(prefix, {
|
||||
withRemote: true,
|
||||
files: [{ path: "src/app.ts", content: "export const app = 1;\n" }],
|
||||
});
|
||||
cleanupTasks.push(repo.cleanup);
|
||||
// The helper puts the bare remote inside the checkout; keep it out of the status.
|
||||
await writeFile(path.join(repo.path, ".git/info/exclude"), "remote.git/\n");
|
||||
return repo.path;
|
||||
}
|
||||
|
||||
async function registerWorkspace(repoPath: string): Promise<string> {
|
||||
const client = await connectSeedClient();
|
||||
cleanupTasks.push(async () => {
|
||||
await client.close().catch(() => undefined);
|
||||
});
|
||||
const created = await client.createWorkspace({ source: { kind: "directory", path: repoPath } });
|
||||
if (!created.workspace) {
|
||||
throw new Error(created.error ?? `Failed to create workspace ${repoPath}`);
|
||||
}
|
||||
return created.workspace.id;
|
||||
}
|
||||
|
||||
async function openCompactActionsMenu(page: Page, workspaceId: string) {
|
||||
await page.setViewportSize({ width: 390, height: 844 });
|
||||
await page.goto(buildHostWorkspaceRoute(getServerId(), workspaceId));
|
||||
await page.reload();
|
||||
await page.getByTestId("workspace-explorer-toggle").first().click();
|
||||
const changesTab = page.getByTestId("explorer-tab-changes").filter({ visible: true });
|
||||
await expect(changesTab).toBeVisible({ timeout: 30_000 });
|
||||
await changesTab.click();
|
||||
const explorer = page.getByTestId("explorer-content-area").filter({ visible: true });
|
||||
await expect(explorer.getByTestId("changes-header")).toBeVisible({ timeout: 30_000 });
|
||||
|
||||
const trigger = explorer.getByTestId("changes-actions-menu-trigger");
|
||||
await expect(trigger).toBeVisible({ timeout: 30_000 });
|
||||
await trigger.click();
|
||||
const menu = page.getByTestId("changes-primary-cta-menu");
|
||||
await expect(menu).toBeVisible();
|
||||
return menu;
|
||||
}
|
||||
|
||||
test("compact git actions menu lists a promoted Pull once", async ({ page }) => {
|
||||
const repoPath = await createRemoteRepo("compact-menu-pull-");
|
||||
cleanupTasks.push(async () => {
|
||||
execFileSync("rm", ["-rf", repoPath]);
|
||||
});
|
||||
|
||||
// Advance origin so the clean checkout is behind it and Pull is the promoted action.
|
||||
const other = await mkdtemp(path.join(await resolveTempRoot(), "compact-menu-other-"));
|
||||
cleanupTasks.push(async () => {
|
||||
execFileSync("rm", ["-rf", other]);
|
||||
});
|
||||
execFileSync("git", ["clone", path.join(repoPath, "remote.git"), other], { stdio: "ignore" });
|
||||
execFileSync("git", ["config", "user.email", "e2e@paseo.test"], { cwd: other, stdio: "ignore" });
|
||||
execFileSync("git", ["config", "user.name", "Paseo E2E"], { cwd: other, stdio: "ignore" });
|
||||
await writeFile(path.join(other, "src/incoming.ts"), "export const incoming = 1;\n");
|
||||
execFileSync("git", ["add", "--all"], { cwd: other, stdio: "ignore" });
|
||||
execFileSync("git", ["commit", "-m", "Incoming commit"], { cwd: other, stdio: "ignore" });
|
||||
execFileSync("git", ["push", "origin", "main"], { cwd: other, stdio: "ignore" });
|
||||
execFileSync("git", ["fetch", "origin"], { cwd: repoPath, stdio: "ignore" });
|
||||
|
||||
const workspaceId = await registerWorkspace(repoPath);
|
||||
const menu = await openCompactActionsMenu(page, workspaceId);
|
||||
|
||||
await expect(menu.getByTestId("changes-menu-pull")).toHaveCount(1);
|
||||
await expect(menu.getByRole("menuitem")).toHaveText([
|
||||
"Pull",
|
||||
"Push",
|
||||
"Pull and push",
|
||||
"Archive workspace",
|
||||
]);
|
||||
});
|
||||
|
||||
test("compact git actions menu lists a promoted Push once", async ({ page }) => {
|
||||
const repoPath = await createRemoteRepo("compact-menu-push-");
|
||||
cleanupTasks.push(async () => {
|
||||
execFileSync("rm", ["-rf", repoPath]);
|
||||
});
|
||||
const workspaceId = await registerWorkspace(repoPath);
|
||||
|
||||
// Commit so the clean checkout is ahead of origin and Push is the promoted action.
|
||||
await writeFile(path.join(repoPath, "src/mine.ts"), "export const mine = 1;\n");
|
||||
execFileSync("git", ["add", "--all"], { cwd: repoPath, stdio: "ignore" });
|
||||
execFileSync("git", ["commit", "-m", "Local commit"], { cwd: repoPath, stdio: "ignore" });
|
||||
|
||||
const menu = await openCompactActionsMenu(page, workspaceId);
|
||||
|
||||
await expect(menu.getByTestId("changes-menu-push")).toHaveCount(1);
|
||||
await expect(menu.getByRole("menuitem")).toHaveText([
|
||||
"Push",
|
||||
"Pull",
|
||||
"Pull and push",
|
||||
"Archive workspace",
|
||||
]);
|
||||
});
|
||||
@@ -1,4 +1,4 @@
|
||||
import type { GitAction, GitActions } from "@/git/policy";
|
||||
import { actionsWithoutPrimary, type GitAction, type GitActions } from "@/git/policy";
|
||||
import type { KeyboardActionDefinition } from "@/keyboard/keyboard-action-dispatcher";
|
||||
import type {
|
||||
WorkspacePanelPlacement,
|
||||
@@ -581,9 +581,7 @@ export function buildWorkspaceCommandCenterContributions(
|
||||
...(source.capabilities.canSplitPanes ? buildPaneContributions(source) : []),
|
||||
];
|
||||
|
||||
const primary = source.gitActions.primary;
|
||||
for (const [index, action] of source.gitActions.secondary.entries()) {
|
||||
if (action.id === primary?.id) continue;
|
||||
for (const [index, action] of actionsWithoutPrimary(source.gitActions).entries()) {
|
||||
contributions.push(buildGitContribution(source, action, 100 + index, "query"));
|
||||
}
|
||||
|
||||
|
||||
@@ -15,7 +15,7 @@ import { Shortcut } from "@/components/ui/shortcut";
|
||||
import { useShortcutKeys } from "@/hooks/use-shortcut-keys";
|
||||
import { inlineUnistylesStyle } from "@/styles/unistyles-inline-style";
|
||||
import type { ShortcutKey } from "@/utils/format-shortcut";
|
||||
import type { GitAction, GitActions } from "@/git/policy";
|
||||
import { actionsWithoutPrimary, type GitAction, type GitActions } from "@/git/policy";
|
||||
import { useGitActionRunner } from "@/git/use-actions";
|
||||
import { buttonControlHeight, HEADER_CONTROL_HEIGHT } from "@/components/ui/control-geometry";
|
||||
|
||||
@@ -129,14 +129,10 @@ export function GitActionsSplitButton({
|
||||
[theme.colors.surface2],
|
||||
);
|
||||
|
||||
const menuOnlyActions = useMemo(
|
||||
() => [
|
||||
...(gitActions.primary ? [gitActions.primary] : []),
|
||||
...gitActions.secondary,
|
||||
...gitActions.menu,
|
||||
],
|
||||
[gitActions.menu, gitActions.primary, gitActions.secondary],
|
||||
);
|
||||
const menuOnlyActions = [
|
||||
...(gitActions.primary ? [gitActions.primary] : []),
|
||||
...actionsWithoutPrimary(gitActions),
|
||||
];
|
||||
|
||||
if (menuOnly) {
|
||||
if (menuOnlyActions.length === 0) {
|
||||
|
||||
@@ -2,7 +2,7 @@ import { afterEach, describe, expect, it } from "vitest";
|
||||
import { CheckoutPrStatusSchema } from "@getpaseo/protocol/messages";
|
||||
import { i18n } from "@/i18n/i18next";
|
||||
|
||||
import { buildGitActions, type BuildGitActionsInput } from "./policy";
|
||||
import { actionsWithoutPrimary, buildGitActions, type BuildGitActionsInput } from "./policy";
|
||||
import { deriveMergeCapability, type ForgeSpecificStatusFacts } from "./merge-capability";
|
||||
|
||||
type GithubMergeFactsFixture = ForgeSpecificStatusFacts & {
|
||||
@@ -190,6 +190,59 @@ describe("git-actions-policy", () => {
|
||||
expect(actions.primary).toMatchObject({ id: "pull", label: "Pull" });
|
||||
});
|
||||
|
||||
it("drops a promoted remote action from the remaining menu actions", () => {
|
||||
const pull = buildGitActions(createInput({ hasRemote: true, behindOfOrigin: 2 }));
|
||||
expect(pull.primary?.id).toBe("pull");
|
||||
expect(actionsWithoutPrimary(pull).map((action) => action.id)).toEqual([
|
||||
"push",
|
||||
"pull-and-push",
|
||||
"archive-workspace",
|
||||
]);
|
||||
|
||||
const push = buildGitActions(createInput({ hasRemote: true, aheadOfOrigin: 1 }));
|
||||
expect(push.primary?.id).toBe("push");
|
||||
expect(actionsWithoutPrimary(push).map((action) => action.id)).toEqual([
|
||||
"pull",
|
||||
"pull-and-push",
|
||||
"archive-workspace",
|
||||
]);
|
||||
});
|
||||
|
||||
it("drops a promoted merge action from the remaining menu actions", () => {
|
||||
const actions = buildGitActions(
|
||||
createInput({
|
||||
hasRemote: true,
|
||||
isOnBaseBranch: false,
|
||||
aheadCount: 2,
|
||||
hasPullRequest: true,
|
||||
pullRequestUrl: "https://example.com/pr/456",
|
||||
pullRequestState: "open",
|
||||
pullRequestMergeable: "MERGEABLE",
|
||||
pullRequestGithub: githubStatus({
|
||||
repository: {
|
||||
autoMergeAllowed: true,
|
||||
mergeCommitAllowed: true,
|
||||
squashMergeAllowed: false,
|
||||
rebaseMergeAllowed: false,
|
||||
viewerDefaultMergeMethod: "SQUASH",
|
||||
},
|
||||
}),
|
||||
shipDefault: "pr",
|
||||
}),
|
||||
);
|
||||
|
||||
expect(actions.primary).toMatchObject({ id: "merge-pr-merge", label: "Merge PR (merge)" });
|
||||
expect(actionsWithoutPrimary(actions).map((action) => action.id)).toEqual([
|
||||
"pull",
|
||||
"push",
|
||||
"pull-and-push",
|
||||
"merge-from-base",
|
||||
"merge-branch",
|
||||
"pr",
|
||||
"archive-workspace",
|
||||
]);
|
||||
});
|
||||
|
||||
it("keeps push clickable with a clearer message when the branch diverged", () => {
|
||||
const actions = buildGitActions(
|
||||
createInput({
|
||||
|
||||
@@ -312,6 +312,18 @@ export function buildGitActions(input: BuildGitActionsInput): GitActions {
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* The actions a flattened list shows after the primary. `secondary` keeps the actions the policy
|
||||
* can promote to `primary`, and the translator rebuilds those objects, so a flattening consumer
|
||||
* has to drop the promoted one by id or it renders it twice.
|
||||
*/
|
||||
export function actionsWithoutPrimary(gitActions: GitActions): GitAction[] {
|
||||
const primaryActionId = gitActions.primary?.id;
|
||||
return [...gitActions.secondary, ...gitActions.menu].filter(
|
||||
(action) => action.id !== primaryActionId,
|
||||
);
|
||||
}
|
||||
|
||||
function getPrimaryActionId(input: BuildGitActionsInput): GitActionId | null {
|
||||
if (input.shouldPromoteArchive) {
|
||||
return "archive-workspace";
|
||||
|
||||
Reference in New Issue
Block a user