fix: default to full URL mode and short-circuit SSH URL validation in repo dialog
RepositoryCreateDialog fixes: - Change useAdvancedUrl default from false to true so full URL is the default - Move isSshUrl helper before the effect that references it - Short-circuit SSH URLs client-side in debounced validation so they always show as valid without depending on backend parseGitUrl behavior - Keeps submit-time SSH key requirement: error shown if SSH URL without key Tests: - Update repositories-settings-tab tests for full-URL default mode - Add SSH URL acceptance test with key selected (client-side short-circuit) - Add SSH URL rejection test without key selected Quality gates: tsc --noEmit pass, npm run build pass, 82/82 tests pass
This commit is contained in:
@@ -20,6 +20,7 @@ const mockRepositories = [
|
|||||||
owner_id: "user-1",
|
owner_id: "user-1",
|
||||||
is_mirror: false,
|
is_mirror: false,
|
||||||
remote_url: null,
|
remote_url: null,
|
||||||
|
ssh_key_id: null,
|
||||||
last_push: null,
|
last_push: null,
|
||||||
created_at: null,
|
created_at: null,
|
||||||
},
|
},
|
||||||
@@ -42,7 +43,59 @@ afterEach(() => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
describe("RepositoriesSettingsTab", () => {
|
describe("RepositoriesSettingsTab", () => {
|
||||||
it("opens create dialog and clones an existing repository", async () => {
|
it("opens create dialog and clones using full URL by default", async () => {
|
||||||
|
const listMock = vi
|
||||||
|
.spyOn(gitRepositoriesApi, "listRepositories")
|
||||||
|
.mockResolvedValue(mockRepositories);
|
||||||
|
const createMock = vi
|
||||||
|
.spyOn(gitRepositoriesApi, "createRepository")
|
||||||
|
.mockResolvedValue(mockRepositories[0]);
|
||||||
|
const parseMock = vi
|
||||||
|
.spyOn(gitRepositoriesApi, "parseGitUrl")
|
||||||
|
.mockResolvedValue({
|
||||||
|
original_url: "https://github.com/user/repo.git",
|
||||||
|
base_url: "https://github.com/user/repo.git",
|
||||||
|
is_valid_clone_url: true,
|
||||||
|
needs_parsing: false,
|
||||||
|
host: "github.com",
|
||||||
|
message: "Valid git repository URL",
|
||||||
|
error_code: null,
|
||||||
|
});
|
||||||
|
vi.spyOn(sshKeysApi, "listSSHKeys").mockResolvedValue([]);
|
||||||
|
|
||||||
|
render(<RepositoriesSettingsTab />);
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByText("Main Repo")).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByRole("button", { name: /add repository/i }));
|
||||||
|
fireEvent.change(screen.getByPlaceholderText(/repository-name/i), {
|
||||||
|
target: { value: "New Repo" },
|
||||||
|
});
|
||||||
|
fireEvent.change(
|
||||||
|
screen.getByPlaceholderText(/https:\/\/github.com\/user\/repo.git/i),
|
||||||
|
{
|
||||||
|
target: { value: "https://github.com/user/repo.git" },
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(parseMock).toHaveBeenCalledWith("https://github.com/user/repo.git");
|
||||||
|
});
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByRole("button", { name: /clone repository/i }));
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(createMock).toHaveBeenCalledWith("proj-1", {
|
||||||
|
name: "New Repo",
|
||||||
|
remote_url: "https://github.com/user/repo.git",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
expect(listMock).toHaveBeenCalledTimes(2);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("switches to owner/repo mode and requires SSH key for generated SSH URL", async () => {
|
||||||
const listMock = vi
|
const listMock = vi
|
||||||
.spyOn(gitRepositoriesApi, "listRepositories")
|
.spyOn(gitRepositoriesApi, "listRepositories")
|
||||||
.mockResolvedValue(mockRepositories);
|
.mockResolvedValue(mockRepositories);
|
||||||
@@ -68,6 +121,9 @@ describe("RepositoriesSettingsTab", () => {
|
|||||||
fireEvent.change(screen.getByPlaceholderText(/repository-name/i), {
|
fireEvent.change(screen.getByPlaceholderText(/repository-name/i), {
|
||||||
target: { value: "New Repo" },
|
target: { value: "New Repo" },
|
||||||
});
|
});
|
||||||
|
fireEvent.click(
|
||||||
|
screen.getByRole("button", { name: /use owner\/repo instead/i }),
|
||||||
|
);
|
||||||
fireEvent.change(screen.getByPlaceholderText(/owner/i), {
|
fireEvent.change(screen.getByPlaceholderText(/owner/i), {
|
||||||
target: { value: "alice" },
|
target: { value: "alice" },
|
||||||
});
|
});
|
||||||
@@ -92,14 +148,33 @@ describe("RepositoriesSettingsTab", () => {
|
|||||||
expect(listMock).toHaveBeenCalledTimes(2);
|
expect(listMock).toHaveBeenCalledTimes(2);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("uses advanced url fallback when requested", async () => {
|
it("accepts SSH URL in full URL mode when SSH key is selected", async () => {
|
||||||
const listMock = vi
|
const listMock = vi
|
||||||
.spyOn(gitRepositoriesApi, "listRepositories")
|
.spyOn(gitRepositoriesApi, "listRepositories")
|
||||||
.mockResolvedValue(mockRepositories);
|
.mockResolvedValue(mockRepositories);
|
||||||
const createMock = vi
|
const createMock = vi
|
||||||
.spyOn(gitRepositoriesApi, "createRepository")
|
.spyOn(gitRepositoriesApi, "createRepository")
|
||||||
.mockResolvedValue(mockRepositories[0]);
|
.mockResolvedValue(mockRepositories[0]);
|
||||||
vi.spyOn(sshKeysApi, "listSSHKeys").mockResolvedValue([]);
|
// SSH URLs are short-circuited client-side; parseGitUrl should NOT be called
|
||||||
|
const parseMock = vi
|
||||||
|
.spyOn(gitRepositoriesApi, "parseGitUrl")
|
||||||
|
.mockResolvedValue({
|
||||||
|
original_url: "",
|
||||||
|
base_url: null,
|
||||||
|
is_valid_clone_url: false,
|
||||||
|
needs_parsing: false,
|
||||||
|
host: null,
|
||||||
|
message: "",
|
||||||
|
error_code: null,
|
||||||
|
});
|
||||||
|
vi.spyOn(sshKeysApi, "listSSHKeys").mockResolvedValue([
|
||||||
|
{
|
||||||
|
id: "key-1",
|
||||||
|
name: "My Key",
|
||||||
|
public_key: "ssh-ed25519 AAA...",
|
||||||
|
created_at: "2024-01-01",
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
|
||||||
render(<RepositoriesSettingsTab />);
|
render(<RepositoriesSettingsTab />);
|
||||||
|
|
||||||
@@ -111,26 +186,80 @@ describe("RepositoriesSettingsTab", () => {
|
|||||||
fireEvent.change(screen.getByPlaceholderText(/repository-name/i), {
|
fireEvent.change(screen.getByPlaceholderText(/repository-name/i), {
|
||||||
target: { value: "New Repo" },
|
target: { value: "New Repo" },
|
||||||
});
|
});
|
||||||
fireEvent.click(
|
|
||||||
screen.getByRole("button", { name: /use full url instead/i }),
|
|
||||||
);
|
|
||||||
fireEvent.change(
|
fireEvent.change(
|
||||||
screen.getByPlaceholderText(/https:\/\/github.com\/user\/repo.git/i),
|
screen.getByPlaceholderText(/https:\/\/github.com\/user\/repo.git/i),
|
||||||
{
|
{
|
||||||
target: { value: "https://github.com/user/repo.git" },
|
target: { value: "git@github.com:user/repo.git" },
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
|
// Client-side short-circuit shows valid without calling backend
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByText(/valid git url/i)).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
expect(parseMock).not.toHaveBeenCalled();
|
||||||
|
|
||||||
|
const sshSelect = screen.getByRole("combobox", { name: /ssh key/i });
|
||||||
|
await waitFor(() =>
|
||||||
|
expect(sshSelect.querySelector('option[value="key-1"]')).toBeTruthy(),
|
||||||
|
);
|
||||||
|
(sshSelect as HTMLSelectElement).value = "key-1";
|
||||||
|
fireEvent.change(sshSelect);
|
||||||
|
|
||||||
fireEvent.click(screen.getByRole("button", { name: /clone repository/i }));
|
fireEvent.click(screen.getByRole("button", { name: /clone repository/i }));
|
||||||
|
|
||||||
await waitFor(() => {
|
await waitFor(() => {
|
||||||
expect(createMock).toHaveBeenCalledWith("proj-1", {
|
expect(createMock).toHaveBeenCalledWith("proj-1", {
|
||||||
name: "New Repo",
|
name: "New Repo",
|
||||||
remote_url: "https://github.com/user/repo.git",
|
remote_url: "git@github.com:user/repo.git",
|
||||||
|
ssh_key_id: "key-1",
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
expect(listMock).toHaveBeenCalledTimes(2);
|
expect(listMock).toHaveBeenCalledTimes(2);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("rejects SSH URL in full URL mode without an SSH key", async () => {
|
||||||
|
vi.spyOn(gitRepositoriesApi, "listRepositories").mockResolvedValue(
|
||||||
|
mockRepositories,
|
||||||
|
);
|
||||||
|
const createMock = vi
|
||||||
|
.spyOn(gitRepositoriesApi, "createRepository")
|
||||||
|
.mockResolvedValue(mockRepositories[0]);
|
||||||
|
vi.spyOn(sshKeysApi, "listSSHKeys").mockResolvedValue([
|
||||||
|
{
|
||||||
|
id: "key-1",
|
||||||
|
name: "My Key",
|
||||||
|
public_key: "ssh-ed25519 AAA...",
|
||||||
|
created_at: "2024-01-01",
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
|
||||||
|
render(<RepositoriesSettingsTab />);
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByText("Main Repo")).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByRole("button", { name: /add repository/i }));
|
||||||
|
fireEvent.change(screen.getByPlaceholderText(/repository-name/i), {
|
||||||
|
target: { value: "New Repo" },
|
||||||
|
});
|
||||||
|
fireEvent.change(
|
||||||
|
screen.getByPlaceholderText(/https:\/\/github.com\/user\/repo.git/i),
|
||||||
|
{
|
||||||
|
target: { value: "git@github.com:user/repo.git" },
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
// Do NOT select an SSH key
|
||||||
|
fireEvent.click(screen.getByRole("button", { name: /clone repository/i }));
|
||||||
|
|
||||||
|
expect(
|
||||||
|
screen.getByText(/an ssh key is required for ssh urls/i),
|
||||||
|
).toBeInTheDocument();
|
||||||
|
expect(createMock).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
it("shows validation when cloning without a remote url", async () => {
|
it("shows validation when cloning without a remote url", async () => {
|
||||||
vi.spyOn(gitRepositoriesApi, "listRepositories").mockResolvedValue(
|
vi.spyOn(gitRepositoriesApi, "listRepositories").mockResolvedValue(
|
||||||
mockRepositories,
|
mockRepositories,
|
||||||
@@ -145,13 +274,16 @@ describe("RepositoriesSettingsTab", () => {
|
|||||||
fireEvent.change(screen.getByPlaceholderText(/repository-name/i), {
|
fireEvent.change(screen.getByPlaceholderText(/repository-name/i), {
|
||||||
target: { value: "New Repo" },
|
target: { value: "New Repo" },
|
||||||
});
|
});
|
||||||
fireEvent.change(screen.getByPlaceholderText(/owner/i), {
|
fireEvent.change(
|
||||||
target: { value: "" },
|
screen.getByPlaceholderText(/https:\/\/github.com\/user\/repo.git/i),
|
||||||
});
|
{
|
||||||
|
target: { value: "" },
|
||||||
|
},
|
||||||
|
);
|
||||||
fireEvent.click(screen.getByRole("button", { name: /clone repository/i }));
|
fireEvent.click(screen.getByRole("button", { name: /clone repository/i }));
|
||||||
|
|
||||||
expect(
|
expect(
|
||||||
screen.getByText(/owner and repository name are required/i),
|
screen.getByText(/remote url is required for advanced cloning/i),
|
||||||
).toBeInTheDocument();
|
).toBeInTheDocument();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -37,7 +37,7 @@ export const RepositoryCreateDialog = ({
|
|||||||
const [owner, setOwner] = useState("");
|
const [owner, setOwner] = useState("");
|
||||||
const [repoName, setRepoName] = useState("");
|
const [repoName, setRepoName] = useState("");
|
||||||
const [advancedUrl, setAdvancedUrl] = useState("");
|
const [advancedUrl, setAdvancedUrl] = useState("");
|
||||||
const [useAdvancedUrl, setUseAdvancedUrl] = useState(false);
|
const [useAdvancedUrl, setUseAdvancedUrl] = useState(true);
|
||||||
const [formError, setFormError] = useState<string | null>(null);
|
const [formError, setFormError] = useState<string | null>(null);
|
||||||
const [urlValidation, setUrlValidation] = useState<{
|
const [urlValidation, setUrlValidation] = useState<{
|
||||||
status: UrlValidationStatus;
|
status: UrlValidationStatus;
|
||||||
@@ -47,6 +47,11 @@ export const RepositoryCreateDialog = ({
|
|||||||
const [selectedSshKey, setSelectedSshKey] = useState<string>("");
|
const [selectedSshKey, setSelectedSshKey] = useState<string>("");
|
||||||
const debounceTimer = useRef<ReturnType<typeof setTimeout> | null>(null);
|
const debounceTimer = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||||
|
|
||||||
|
const isSshUrl = (url: string): boolean => {
|
||||||
|
const u = url.trim().toLowerCase();
|
||||||
|
return u.startsWith("git@") || u.startsWith("ssh://");
|
||||||
|
};
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!open && debounceTimer.current) {
|
if (!open && debounceTimer.current) {
|
||||||
clearTimeout(debounceTimer.current);
|
clearTimeout(debounceTimer.current);
|
||||||
@@ -86,8 +91,26 @@ export const RepositoryCreateDialog = ({
|
|||||||
setUrlValidation({ status: "validating", result: null });
|
setUrlValidation({ status: "validating", result: null });
|
||||||
|
|
||||||
debounceTimer.current = setTimeout(async () => {
|
debounceTimer.current = setTimeout(async () => {
|
||||||
|
const trimmed = advancedUrl.trim();
|
||||||
|
// Short-circuit SSH URLs — the backend parseGitUrl may not always
|
||||||
|
// recognise them, but they are valid clone URLs by definition.
|
||||||
|
if (isSshUrl(trimmed)) {
|
||||||
|
setUrlValidation({
|
||||||
|
status: "valid",
|
||||||
|
result: {
|
||||||
|
original_url: trimmed,
|
||||||
|
base_url: trimmed,
|
||||||
|
is_valid_clone_url: true,
|
||||||
|
needs_parsing: false,
|
||||||
|
host: null,
|
||||||
|
message: "Valid SSH git URL",
|
||||||
|
error_code: null,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
return;
|
||||||
|
}
|
||||||
try {
|
try {
|
||||||
const result = await parseGitUrl(advancedUrl.trim());
|
const result = await parseGitUrl(trimmed);
|
||||||
if (result.is_valid_clone_url) {
|
if (result.is_valid_clone_url) {
|
||||||
setUrlValidation({ status: "valid", result });
|
setUrlValidation({ status: "valid", result });
|
||||||
} else if (result.needs_parsing) {
|
} else if (result.needs_parsing) {
|
||||||
@@ -124,11 +147,6 @@ export const RepositoryCreateDialog = ({
|
|||||||
onClose();
|
onClose();
|
||||||
};
|
};
|
||||||
|
|
||||||
const isSshUrl = (url: string): boolean => {
|
|
||||||
const u = url.trim().toLowerCase();
|
|
||||||
return u.startsWith("git@") || u.startsWith("ssh://");
|
|
||||||
};
|
|
||||||
|
|
||||||
const handleSubmit = async (event: React.FormEvent) => {
|
const handleSubmit = async (event: React.FormEvent) => {
|
||||||
event.preventDefault();
|
event.preventDefault();
|
||||||
setFormError(null);
|
setFormError(null);
|
||||||
|
|||||||
Reference in New Issue
Block a user