feat: move GitLab MR linking to task menus (#1868)

apps/web/components/gitlab/mr-topbar-button.test.ts

@@ -1,8 +1,66 @@
+import { createElement } from "react";
+import { render, screen } from "@testing-library/react";
 import { describe, expect, it, vi } from "vitest";
-import { mrTriggerClass, openDesktopMRReview, openMobileMRReview } from "./mr-topbar-button";
+import {
+  MRTopbarButton,
+  mrTriggerClass,
+  openDesktopMRReview,
+  openMobileMRReview,
+} from "./mr-topbar-button";
 import type { TaskMR } from "@/lib/types/gitlab";
 
+const gitlabMocks = vi.hoisted(() => ({ mrs: [] as TaskMR[] }));
+
+vi.mock("@/components/state-provider", () => ({
+  useAppStore: (selector: (state: unknown) => unknown) =>
+    selector({
+      tasks: { activeTaskId: "task-1" },
+      workspaces: { activeId: "workspace-1" },
+      repositories: { itemsByWorkspaceId: { "workspace-1": [] } },
+      removeTaskMR: vi.fn(),
+    }),
+}));
+
+vi.mock("@/hooks/domains/gitlab/use-task-mr", () => ({
+  useGitLabAvailable: () => true,
+  useTaskMRs: () => gitlabMocks.mrs,
+  useWorkspaceMRs: vi.fn(),
+}));
+
+vi.mock("@/hooks/domains/kanban/use-task-by-id", () => ({
+  useTaskById: () => ({ id: "task-1", repositories: [] }),
+}));
+
+vi.mock("@/components/toast-provider", () => ({
+  useToast: () => ({ toast: vi.fn() }),
+}));
+
+vi.mock("./task-mr-link-dialog", () => ({ TaskMRLinkDialog: () => null }));
+
 describe("mrTriggerClass", () => {
+  it("does not render a generic link control for an unlinked task", () => {
+    gitlabMocks.mrs = [];
+    render(createElement(MRTopbarButton));
+
+    expect(screen.queryByRole("button", { name: "Link GitLab merge request" })).toBeNull();
+  });
+
+  it("keeps the linked merge request status control", () => {
+    gitlabMocks.mrs = [
+      {
+        id: "association-1",
+        mr_iid: 81,
+        state: "opened",
+        project_path: "group/project",
+        mr_url: "https://gitlab.example/group/project/-/merge_requests/81",
+      } as TaskMR,
+    ];
+    render(createElement(MRTopbarButton));
+
+    const trigger = screen.getByTestId("mr-topbar-button");
+    expect(trigger.getAttribute("data-mr-iid")).toBe("81");
+  });
+
   it("uses a 44px trigger on mobile", () => {
     expect(mrTriggerClass(true, true)).toContain("h-11");
     expect(mrTriggerClass(true, true)).toContain("w-11");

apps/web/components/gitlab/mr-topbar-button.tsx

@@ -8,7 +8,6 @@ import {
   IconClock,
   IconExternalLink,
   IconGitMerge,
-  IconLink,
   IconPlus,
   IconUnlink,
   IconX,
@@ -241,19 +240,7 @@ function MRTopbarControl({
       />
     );
   }
-  if (!gitlabAvailable) return null;
-  return (
-    <Button
-      size={compact ? "icon-sm" : "sm"}
-      variant="outline"
-      className={mrTriggerClass(compact, mobile)}
-      onClick={onLink}
-      aria-label="Link GitLab merge request"
-    >
-      <IconLink className="h-4 w-4 text-orange-500" />
-      {!compact && <span className="text-xs font-medium">Link MR</span>}
-    </Button>
-  );
+  return null;
 }
 
 export const MRTopbarButton = memo(function MRTopbarButton({

apps/web/components/gitlab/task-mr-link-dialog.test.tsx

@@ -1,9 +1,29 @@
-import { cleanup, fireEvent, render, screen } from "@testing-library/react";
+import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react";
 import { afterEach, describe, expect, it, vi } from "vitest";
 import type { Repository } from "@/lib/types/http";
 import { TaskMRLinkDialog } from "./task-mr-link-dialog";
 
 const appState = { setTaskMR: vi.fn() };
+const createTaskMR = vi.hoisted(() => vi.fn().mockResolvedValue({}));
+const repositories = [
+  {
+    id: "repository-1",
+    name: "kandev",
+    provider_owner: "platform",
+    provider_name: "kandev",
+  },
+  {
+    id: "repository-2",
+    name: "docs",
+    provider_owner: "platform",
+    provider_name: "docs",
+  },
+] as Repository[];
+const taskRepositories = repositories.map((repository) => ({ repository_id: repository.id }));
+const mrURL = "https://gitlab.example.test/platform/kandev/-/merge_requests/81";
+const mergeRequestURLLabel = "Merge request URL";
+const linkMergeRequestButton = "Link merge request";
+const workspaceId = "workspace-1";
 
 vi.mock("@/components/state-provider", () => ({
   useAppStore: (selector: (state: typeof appState) => unknown) => selector(appState),
@@ -13,52 +33,127 @@ vi.mock("@/components/toast-provider", () => ({
   useToast: () => ({ toast: vi.fn() }),
 }));
 
-vi.mock("@/lib/api/domains/gitlab-api", () => ({
-  createTaskMR: vi.fn(),
-}));
+vi.mock("@/lib/api/domains/gitlab-api", () => ({ createTaskMR }));
 
-afterEach(cleanup);
+function dialog({
+  open = true,
+  repositoryOptions = repositories,
+  taskRepositoryLinks = taskRepositories,
+}: {
+  open?: boolean;
+  repositoryOptions?: Repository[];
+  taskRepositoryLinks?: { repository_id: string }[];
+} = {}) {
+  return (
+    <TaskMRLinkDialog
+      open={open}
+      onOpenChange={vi.fn()}
+      taskId="task-1"
+      workspaceId={workspaceId}
+      taskRepositories={taskRepositoryLinks}
+      repositories={repositoryOptions}
+    />
+  );
+}
+
+afterEach(() => {
+  cleanup();
+  vi.clearAllMocks();
+});
 
 describe("TaskMRLinkDialog", () => {
-  it("preserves the typed URL when repository props refresh while open", () => {
-    const repository = {
-      id: "repository-1",
-      name: "kandev",
-      provider_owner: "platform",
-      provider_name: "kandev",
-    } as Repository;
-    const props = {
-      open: true,
-      onOpenChange: vi.fn(),
-      taskId: "task-1",
-      workspaceId: "workspace-1",
-    };
-    const { rerender } = render(
-      <TaskMRLinkDialog
-        {...props}
-        taskRepositories={[{ repository_id: repository.id }]}
-        repositories={[repository]}
-      />,
-    );
+  it("submits the default repository when initially mounted open", async () => {
+    render(dialog());
 
-    const url = screen.getByLabelText("Merge request URL");
-    fireEvent.change(url, {
-      target: { value: "https://gitlab.example.test/platform/kandev/-/merge_requests/81" },
+    fireEvent.change(screen.getByLabelText(mergeRequestURLLabel), {
+      target: { value: mrURL },
     });
+    fireEvent.click(screen.getByRole("button", { name: linkMergeRequestButton }));
 
+    await waitFor(() => expect(createTaskMR).toHaveBeenCalledTimes(1));
+    expect(createTaskMR).toHaveBeenCalledWith(
+      {
+        task_id: "task-1",
+        repository_id: "repository-1",
+        mr_url: mrURL,
+      },
+      workspaceId,
+    );
+  });
+
+  it("uses the default repository when repositories hydrate after opening", async () => {
+    const { rerender } = render(dialog({ repositoryOptions: [], taskRepositoryLinks: [] }));
+
+    rerender(dialog());
+    fireEvent.change(screen.getByLabelText(mergeRequestURLLabel), {
+      target: { value: mrURL },
+    });
+    fireEvent.click(screen.getByRole("button", { name: linkMergeRequestButton }));
+
+    await waitFor(() => expect(createTaskMR).toHaveBeenCalledTimes(1));
+    expect(createTaskMR).toHaveBeenCalledWith(
+      expect.objectContaining({ repository_id: "repository-1" }),
+      workspaceId,
+    );
+  });
+
+  it("preserves the typed URL and repository selection when props refresh while open", async () => {
+    const { rerender } = render(dialog());
+
+    const url = screen.getByLabelText(mergeRequestURLLabel);
+    fireEvent.change(url, {
+      target: { value: mrURL },
+    });
+    fireEvent.click(screen.getByRole("combobox", { name: "Task repository" }));
+    fireEvent.click(await screen.findByRole("option", { name: "platform/docs" }));
+
+    const newlyHydratedRepository = {
+      id: "repository-3",
+      name: "api",
+      provider_owner: "platform",
+      provider_name: "api",
+    } as Repository;
     rerender(
-      <TaskMRLinkDialog
-        {...props}
-        taskRepositories={[{ repository_id: repository.id }]}
-        repositories={[{ ...repository }]}
-      />,
+      dialog({
+        repositoryOptions: [newlyHydratedRepository, ...repositories],
+        taskRepositoryLinks: [{ repository_id: newlyHydratedRepository.id }, ...taskRepositories],
+      }),
     );
 
-    expect((url as HTMLInputElement).value).toBe(
-      "https://gitlab.example.test/platform/kandev/-/merge_requests/81",
+    expect((url as HTMLInputElement).value).toBe(mrURL);
+    fireEvent.click(screen.getByRole("button", { name: linkMergeRequestButton }));
+
+    await waitFor(() => expect(createTaskMR).toHaveBeenCalledTimes(1));
+    expect(createTaskMR).toHaveBeenCalledWith(
+      expect.objectContaining({
+        repository_id: "repository-2",
+        mr_url: mrURL,
+      }),
+      workspaceId,
+    );
+  });
+
+  it("resets the URL and selection to the current default after closing", async () => {
+    const { rerender } = render(dialog());
+
+    fireEvent.change(screen.getByLabelText(mergeRequestURLLabel), {
+      target: { value: mrURL },
+    });
+    const reorderedLinks = [taskRepositories[1], taskRepositories[0]];
+    rerender(dialog({ open: false, taskRepositoryLinks: reorderedLinks }));
+    rerender(dialog({ taskRepositoryLinks: reorderedLinks }));
+
+    const url = screen.getByLabelText(mergeRequestURLLabel) as HTMLInputElement;
+    expect(url.value).toBe("");
+    fireEvent.change(url, {
+      target: { value: "https://gitlab.example.test/platform/docs/-/merge_requests/12" },
+    });
+    fireEvent.click(screen.getByRole("button", { name: linkMergeRequestButton }));
+
+    await waitFor(() => expect(createTaskMR).toHaveBeenCalledTimes(1));
+    expect(createTaskMR).toHaveBeenCalledWith(
+      expect.objectContaining({ repository_id: "repository-2" }),
+      workspaceId,
     );
-    expect(
-      (screen.getByRole("button", { name: "Link merge request" }) as HTMLButtonElement).disabled,
-    ).toBe(false);
   });
 });

apps/web/components/gitlab/task-mr-link-dialog.tsx

@@ -116,13 +116,19 @@ export function TaskMRLinkDialog({
   taskRepositories: TaskRepositoryLink[];
   repositories: Repository[];
 }) {
+  const options = useRepositoryOptions(repositories, taskRepositories);
+  const defaultRepositoryID = options[0]?.id ?? NO_REPOSITORY;
   const [mrURL, setMRURL] = useState("");
-  const [repositoryID, setRepositoryID] = useState(NO_REPOSITORY);
+  const [repositoryID, setRepositoryID] = useState(defaultRepositoryID);
   const [submitting, setSubmitting] = useState(false);
   const setTaskMR = useAppStore((state) => state.setTaskMR);
   const { toast } = useToast();
-  const options = useRepositoryOptions(repositories, taskRepositories);
-  const defaultRepositoryID = options[0]?.id ?? NO_REPOSITORY;
+
+  useEffect(() => {
+    if (repositoryID === NO_REPOSITORY && defaultRepositoryID !== NO_REPOSITORY) {
+      setRepositoryID(defaultRepositoryID);
+    }
+  }, [defaultRepositoryID, repositoryID]);
 
   useEffect(() => {
     if (open) return;