fix: resolve four frontend/backend issues
- Fix ProjectsPage tests by wrapping renders in MemoryRouter (9 passing) - Improve session auto-naming to 'project / repo / tool' format - Add missing /users/me/sessions endpoint for sidebar session loading - Handle git history 500s: catch RuntimeError in endpoints, graceful empty repo handling - Add git status badge and discard-changes button to FileEditor toolbar Quality gates: tsc pass, build pass, Python syntax pass
This commit is contained in:
@@ -3,7 +3,7 @@
|
||||
import logging
|
||||
import uuid
|
||||
|
||||
from fastapi import APIRouter, Depends, Response, status
|
||||
from fastapi import APIRouter, Depends, HTTPException, Response, status
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from src.auth.dependencies import get_current_user, get_db_session, get_owned_project
|
||||
@@ -94,7 +94,14 @@ async def get_repository_history(
|
||||
from src.services.git.repository import get_repo_and_validate, ensure_repo_on_disk
|
||||
repo = await get_repo_and_validate(session, repo_id, project_id)
|
||||
ensure_repo_on_disk(repo)
|
||||
try:
|
||||
return get_commit_history(repo.path, branch=branch, limit=limit, offset=offset)
|
||||
except RuntimeError as e:
|
||||
logger.warning("Git history failed for %s: %s", repo.path, str(e))
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail=f"Git history unavailable: {str(e)}",
|
||||
) from e
|
||||
|
||||
|
||||
@router.get("/{project_id}/repositories/{repo_id}/commits/{commit_hash}")
|
||||
@@ -110,7 +117,14 @@ async def get_repository_commit(
|
||||
from src.services.git.repository import get_repo_and_validate, ensure_repo_on_disk
|
||||
repo = await get_repo_and_validate(session, repo_id, project_id)
|
||||
ensure_repo_on_disk(repo)
|
||||
try:
|
||||
return get_commit_detail(repo.path, commit_hash)
|
||||
except RuntimeError as e:
|
||||
logger.warning("Git commit detail failed for %s %s: %s", repo.path, commit_hash, str(e))
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail=f"Commit detail unavailable: {str(e)}",
|
||||
) from e
|
||||
|
||||
|
||||
# File browsing
|
||||
|
||||
@@ -2,10 +2,13 @@ import uuid
|
||||
from pathlib import Path
|
||||
|
||||
from fastapi import APIRouter, Depends, HTTPException, UploadFile, status
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from src.auth.dependencies import get_current_user, get_db_session
|
||||
from src.models.tool_instance import ToolInstance
|
||||
from src.models.user import User
|
||||
from src.schemas.tool_instance import SessionItemResponse, SessionListResponse
|
||||
from src.schemas.user import UserProfileResponse, UserProfileUpdate
|
||||
|
||||
router = APIRouter(prefix="/users", tags=["users"])
|
||||
@@ -131,3 +134,41 @@ async def upload_avatar(
|
||||
await session.commit()
|
||||
await session.refresh(user)
|
||||
return user
|
||||
|
||||
|
||||
@router.get(
|
||||
"/me/sessions",
|
||||
response_model=SessionListResponse,
|
||||
summary="Get current user sessions",
|
||||
description="Retrieve all tool instances (sessions) for the authenticated user.",
|
||||
)
|
||||
async def get_user_sessions(
|
||||
user: User = Depends(get_current_user),
|
||||
session: AsyncSession = Depends(get_db_session),
|
||||
) -> SessionListResponse:
|
||||
"""Return all tool instances for the current user with related names."""
|
||||
result = await session.execute(
|
||||
select(ToolInstance)
|
||||
.where(ToolInstance.owner_id == user.id)
|
||||
.order_by(ToolInstance.created_at.desc())
|
||||
)
|
||||
instances = result.scalars().all()
|
||||
|
||||
sessions = [
|
||||
SessionItemResponse(
|
||||
id=str(inst.id),
|
||||
display_name=inst.display_name,
|
||||
tool_type_name=inst.tool_type.display_name if inst.tool_type else "Unknown",
|
||||
tool_icon=inst.tool_type.icon if inst.tool_type else None,
|
||||
tool_type_interfaces=inst.tool_type.interfaces if inst.tool_type else [],
|
||||
repository_name=inst.repository.name if inst.repository else "Unknown",
|
||||
repository_id=str(inst.repository_id),
|
||||
project_name=inst.project.name if inst.project else "Unknown",
|
||||
project_id=str(inst.project_id),
|
||||
status=inst.status,
|
||||
url=inst.url,
|
||||
)
|
||||
for inst in instances
|
||||
]
|
||||
|
||||
return SessionListResponse(sessions=sessions)
|
||||
|
||||
@@ -15,3 +15,27 @@ class CreateInstanceRequest(BaseModel):
|
||||
config_profile_id: str | None = Field(
|
||||
default=None, description="Optional config profile ID to apply to the instance"
|
||||
)
|
||||
|
||||
|
||||
class SessionItemResponse(BaseModel):
|
||||
"""Lightweight session summary for sidebar and dashboard."""
|
||||
|
||||
model_config = {"extra": "ignore"}
|
||||
|
||||
id: str = Field(description="Session (tool instance) ID")
|
||||
display_name: str = Field(description="Display name of the session")
|
||||
tool_type_name: str = Field(description="Name of the tool type")
|
||||
tool_icon: str | None = Field(default=None, description="Icon URL for the tool type")
|
||||
tool_type_interfaces: list[str] = Field(default_factory=list, description="Supported interfaces")
|
||||
repository_name: str = Field(description="Name of the repository")
|
||||
repository_id: str = Field(description="Repository ID")
|
||||
project_name: str = Field(description="Name of the project")
|
||||
project_id: str = Field(description="Project ID")
|
||||
status: str = Field(description="Current status")
|
||||
url: str | None = Field(default=None, description="Access URL")
|
||||
|
||||
|
||||
class SessionListResponse(BaseModel):
|
||||
"""Response wrapping a list of session summaries."""
|
||||
|
||||
sessions: list[SessionItemResponse]
|
||||
|
||||
@@ -54,7 +54,7 @@ async def create_new_instance(
|
||||
|
||||
instance = ToolInstance(
|
||||
name=instance_name,
|
||||
display_name=display_name or f"{tool_type.display_name} - {repo.name}",
|
||||
display_name=display_name or f"{project.name} / {repo.name} / {tool_type.display_name}",
|
||||
tool_type_id=tool_type.id,
|
||||
repository_id=repo.id,
|
||||
project_id=project.id,
|
||||
|
||||
@@ -60,9 +60,12 @@ def get_commit_history(repo_path: str, branch: str | None = None, limit: int = 1
|
||||
|
||||
Returns structured data including commits, branches, and graph information.
|
||||
"""
|
||||
# Get list of branches
|
||||
# Get list of branches (may fail for empty repos)
|
||||
try:
|
||||
branches_output = _run_git_command(repo_path, ["branch", "-a", "--format=%(refname:short)"])
|
||||
branches = [b.strip() for b in branches_output.strip().split("\n") if b.strip()]
|
||||
except RuntimeError:
|
||||
branches = []
|
||||
|
||||
# Build git log command - use NULL bytes as separators to avoid parsing issues
|
||||
log_args = [
|
||||
@@ -76,7 +79,16 @@ def get_commit_history(repo_path: str, branch: str | None = None, limit: int = 1
|
||||
else:
|
||||
log_args.append("--all")
|
||||
|
||||
try:
|
||||
log_output = _run_git_command(repo_path, log_args)
|
||||
except RuntimeError:
|
||||
# Empty repo or no commits
|
||||
return {
|
||||
"commits": [],
|
||||
"branches": branches,
|
||||
"total_commits": 0,
|
||||
"graph_data": {"nodes": [], "edges": []},
|
||||
}
|
||||
|
||||
# Get branch info for each commit
|
||||
branch_map = _get_branch_map(repo_path)
|
||||
@@ -113,8 +125,11 @@ def get_commit_history(repo_path: str, branch: str | None = None, limit: int = 1
|
||||
)
|
||||
|
||||
# Get total commit count
|
||||
try:
|
||||
count_output = _run_git_command(repo_path, ["rev-list", "--all", "--count"])
|
||||
total_commits = int(count_output.strip()) if count_output.strip() else 0
|
||||
except RuntimeError:
|
||||
total_commits = 0
|
||||
|
||||
# Build graph data and generate graph symbols
|
||||
graph_data = _build_graph_data(commits)
|
||||
|
||||
@@ -9,14 +9,23 @@ import { Icon } from "../../ui/Icon";
|
||||
import { SyntaxHighlighter } from "./SyntaxHighlighter";
|
||||
import { detectLanguage } from "../../../utils/language";
|
||||
|
||||
interface GitFileStatus {
|
||||
modified: string[];
|
||||
added: string[];
|
||||
deleted: string[];
|
||||
untracked: string[];
|
||||
}
|
||||
|
||||
interface FileEditorProps {
|
||||
projectId: string;
|
||||
repoId: string;
|
||||
gitStatus?: GitFileStatus | null;
|
||||
}
|
||||
|
||||
export const FileEditor: React.FC<FileEditorProps> = ({
|
||||
projectId,
|
||||
repoId,
|
||||
gitStatus,
|
||||
}) => {
|
||||
const [searchParams] = useSearchParams();
|
||||
const { user } = useAuth();
|
||||
@@ -31,9 +40,49 @@ export const FileEditor: React.FC<FileEditorProps> = ({
|
||||
const [isBinary, setIsBinary] = useState(false);
|
||||
const [saving, setSaving] = useState(false);
|
||||
|
||||
const handleDiscard = async () => {
|
||||
if (!filePath) return;
|
||||
try {
|
||||
const response = await apiClient.get(
|
||||
`/projects/${projectId}/repositories/${repoId}/files/content`,
|
||||
{
|
||||
params: {
|
||||
branch,
|
||||
path: filePath,
|
||||
},
|
||||
}
|
||||
);
|
||||
const data = response.data;
|
||||
if (data.is_binary) {
|
||||
setIsBinary(true);
|
||||
setContent("Binary file - cannot display");
|
||||
setOriginalContent("");
|
||||
} else {
|
||||
setIsBinary(false);
|
||||
setContent(data.content);
|
||||
setOriginalContent(data.content);
|
||||
}
|
||||
setMode("view");
|
||||
} catch {
|
||||
setError("Failed to discard changes");
|
||||
}
|
||||
};
|
||||
|
||||
const branch = searchParams.get("branch") || "main";
|
||||
const filePath = searchParams.get("file");
|
||||
|
||||
const fileStatus = gitStatus
|
||||
? gitStatus.modified.includes(filePath || "")
|
||||
? "modified"
|
||||
: gitStatus.added.includes(filePath || "")
|
||||
? "added"
|
||||
: gitStatus.deleted.includes(filePath || "")
|
||||
? "deleted"
|
||||
: gitStatus.untracked.includes(filePath || "")
|
||||
? "untracked"
|
||||
: undefined
|
||||
: undefined;
|
||||
|
||||
const loadFile = useCallback(async () => {
|
||||
if (!filePath) {
|
||||
setContent("");
|
||||
@@ -158,7 +207,7 @@ export const FileEditor: React.FC<FileEditorProps> = ({
|
||||
<div className={styles.fileEditor}>
|
||||
<div className={styles.fileEditorToolbar}>
|
||||
<div className="file-breadcrumbs">
|
||||
{filePath.split("/").map((part, i, arr) => (
|
||||
{filePath?.split("/").map((part, i, arr) => (
|
||||
<span key={i}>
|
||||
{part}
|
||||
{i < arr.length - 1 && (
|
||||
@@ -168,6 +217,11 @@ export const FileEditor: React.FC<FileEditorProps> = ({
|
||||
))}
|
||||
</div>
|
||||
<div className={styles.fileActions}>
|
||||
{fileStatus && (
|
||||
<span className={`git-status-badge ${fileStatus}`} title={fileStatus}>
|
||||
{fileStatus === "modified" ? "M" : fileStatus === "added" ? "A" : fileStatus === "deleted" ? "D" : "?"}
|
||||
</span>
|
||||
)}
|
||||
{mode === "view" && !isBinary && (
|
||||
<button
|
||||
className="btn-primary"
|
||||
@@ -198,6 +252,15 @@ export const FileEditor: React.FC<FileEditorProps> = ({
|
||||
</>
|
||||
)}
|
||||
</button>
|
||||
<button
|
||||
className="btn-secondary"
|
||||
onClick={handleDiscard}
|
||||
type="button"
|
||||
title="Revert to last committed version"
|
||||
>
|
||||
<Icon name="undo" size="sm" />
|
||||
Discard
|
||||
</button>
|
||||
<button
|
||||
className="btn-secondary"
|
||||
onClick={handleCancel}
|
||||
|
||||
@@ -75,7 +75,8 @@ export type IconName =
|
||||
| "play"
|
||||
| "stop"
|
||||
| "terminal"
|
||||
| "arrow-left";
|
||||
| "arrow-left"
|
||||
| "undo";
|
||||
|
||||
const iconMap: Record<IconName, React.ComponentType<{ size?: number | string; weight?: "thin" | "light" | "regular" | "bold" | "fill" | "duotone" }>> = {
|
||||
dashboard: House,
|
||||
@@ -117,6 +118,7 @@ const iconMap: Record<IconName, React.ComponentType<{ size?: number | string; we
|
||||
stop: Stop,
|
||||
terminal: Terminal,
|
||||
"arrow-left": ArrowLeft,
|
||||
undo: ClockCounterClockwise,
|
||||
};
|
||||
|
||||
export interface IconProps {
|
||||
|
||||
@@ -1,9 +1,20 @@
|
||||
import { cleanup, fireEvent, render, screen, waitFor, within } from "@testing-library/react";
|
||||
import {
|
||||
cleanup,
|
||||
fireEvent,
|
||||
render,
|
||||
screen,
|
||||
waitFor,
|
||||
within,
|
||||
} from "@testing-library/react";
|
||||
import { MemoryRouter } from "react-router-dom";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
import { ProjectsPage } from "./ProjectsPage";
|
||||
import * as projectsApi from "../api/projects";
|
||||
|
||||
const renderWithRouter = (ui: React.ReactElement) =>
|
||||
render(<MemoryRouter>{ui}</MemoryRouter>);
|
||||
|
||||
const mockProjects = [
|
||||
{
|
||||
id: "proj-1",
|
||||
@@ -28,14 +39,16 @@ afterEach(() => {
|
||||
|
||||
describe("ProjectsPage", () => {
|
||||
it("renders loading state initially", () => {
|
||||
vi.spyOn(projectsApi, "listProjects").mockImplementation(() => new Promise(() => {}));
|
||||
render(<ProjectsPage />);
|
||||
vi.spyOn(projectsApi, "listProjects").mockImplementation(
|
||||
() => new Promise(() => {}),
|
||||
);
|
||||
renderWithRouter(<ProjectsPage />);
|
||||
expect(screen.getByText(/loading projects/i)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("renders project list after loading", async () => {
|
||||
vi.spyOn(projectsApi, "listProjects").mockResolvedValue(mockProjects);
|
||||
render(<ProjectsPage />);
|
||||
renderWithRouter(<ProjectsPage />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("Alpha Project")).toBeInTheDocument();
|
||||
@@ -46,7 +59,7 @@ describe("ProjectsPage", () => {
|
||||
|
||||
it("renders empty state when no projects", async () => {
|
||||
vi.spyOn(projectsApi, "listProjects").mockResolvedValue([]);
|
||||
render(<ProjectsPage />);
|
||||
renderWithRouter(<ProjectsPage />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText(/no projects yet/i)).toBeInTheDocument();
|
||||
@@ -55,7 +68,7 @@ describe("ProjectsPage", () => {
|
||||
|
||||
it("renders error state with retry button", async () => {
|
||||
vi.spyOn(projectsApi, "listProjects").mockRejectedValue(new Error("fail"));
|
||||
render(<ProjectsPage />);
|
||||
renderWithRouter(<ProjectsPage />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText(/failed to load projects/i)).toBeInTheDocument();
|
||||
@@ -64,10 +77,14 @@ describe("ProjectsPage", () => {
|
||||
});
|
||||
|
||||
it("opens create dialog and submits new project", async () => {
|
||||
const listMock = vi.spyOn(projectsApi, "listProjects").mockResolvedValue([]);
|
||||
const createMock = vi.spyOn(projectsApi, "createProject").mockResolvedValue(mockProjects[0]);
|
||||
const listMock = vi
|
||||
.spyOn(projectsApi, "listProjects")
|
||||
.mockResolvedValue([]);
|
||||
const createMock = vi
|
||||
.spyOn(projectsApi, "createProject")
|
||||
.mockResolvedValue(mockProjects[0]);
|
||||
|
||||
render(<ProjectsPage />);
|
||||
renderWithRouter(<ProjectsPage />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText(/no projects yet/i)).toBeInTheDocument();
|
||||
@@ -96,7 +113,7 @@ describe("ProjectsPage", () => {
|
||||
it("shows validation error when name is empty", async () => {
|
||||
vi.spyOn(projectsApi, "listProjects").mockResolvedValue([]);
|
||||
|
||||
render(<ProjectsPage />);
|
||||
renderWithRouter(<ProjectsPage />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText(/no projects yet/i)).toBeInTheDocument();
|
||||
@@ -110,35 +127,43 @@ describe("ProjectsPage", () => {
|
||||
|
||||
it("renders settings link for each project", async () => {
|
||||
vi.spyOn(projectsApi, "listProjects").mockResolvedValue(mockProjects);
|
||||
render(<ProjectsPage />);
|
||||
renderWithRouter(<ProjectsPage />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("Alpha Project")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
const alphaCard = screen.getByText("Alpha Project").closest(".project-card") as HTMLElement | null;
|
||||
const alphaCard = screen
|
||||
.getByText("Alpha Project")
|
||||
.closest(".project-card") as HTMLElement | null;
|
||||
if (!alphaCard) throw new Error("Card not found");
|
||||
|
||||
const settingsLink = within(alphaCard).getByRole("link", { name: /settings/i });
|
||||
const settingsLink = within(alphaCard).getByRole("link", {
|
||||
name: /settings/i,
|
||||
});
|
||||
expect(settingsLink).toBeInTheDocument();
|
||||
expect(settingsLink).toHaveAttribute("href", "/projects/proj-1/settings");
|
||||
});
|
||||
|
||||
it("renders open workspace link as rightmost action", async () => {
|
||||
vi.spyOn(projectsApi, "listProjects").mockResolvedValue(mockProjects);
|
||||
render(<ProjectsPage />);
|
||||
renderWithRouter(<ProjectsPage />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("Alpha Project")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
const alphaCard = screen.getByText("Alpha Project").closest(".project-card") as HTMLElement | null;
|
||||
const alphaCard = screen
|
||||
.getByText("Alpha Project")
|
||||
.closest(".project-card") as HTMLElement | null;
|
||||
if (!alphaCard) throw new Error("Card not found");
|
||||
|
||||
const actions = alphaCard.querySelector(".project-actions");
|
||||
if (!actions) throw new Error("Actions container not found");
|
||||
|
||||
const workspaceLink = within(alphaCard).getByRole("link", { name: /open workspace/i });
|
||||
const workspaceLink = within(alphaCard).getByRole("link", {
|
||||
name: /open workspace/i,
|
||||
});
|
||||
expect(workspaceLink).toBeInTheDocument();
|
||||
expect(workspaceLink).toHaveAttribute("href", "/projects/proj-1");
|
||||
|
||||
@@ -149,16 +174,22 @@ describe("ProjectsPage", () => {
|
||||
});
|
||||
|
||||
it("shows delete confirmation and deletes project", async () => {
|
||||
const listMock = vi.spyOn(projectsApi, "listProjects").mockResolvedValue(mockProjects);
|
||||
const deleteMock = vi.spyOn(projectsApi, "deleteProject").mockResolvedValue(undefined);
|
||||
const listMock = vi
|
||||
.spyOn(projectsApi, "listProjects")
|
||||
.mockResolvedValue(mockProjects);
|
||||
const deleteMock = vi
|
||||
.spyOn(projectsApi, "deleteProject")
|
||||
.mockResolvedValue(undefined);
|
||||
|
||||
render(<ProjectsPage />);
|
||||
renderWithRouter(<ProjectsPage />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("Alpha Project")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
const alphaCard = screen.getByText("Alpha Project").closest(".project-card") as HTMLElement | null;
|
||||
const alphaCard = screen
|
||||
.getByText("Alpha Project")
|
||||
.closest(".project-card") as HTMLElement | null;
|
||||
if (!alphaCard) throw new Error("Card not found");
|
||||
|
||||
fireEvent.click(within(alphaCard).getByRole("button", { name: /delete/i }));
|
||||
|
||||
@@ -198,7 +198,7 @@ export const RepoWorkspace = () => {
|
||||
)}
|
||||
<main className="workspace-main">
|
||||
{selectedRepoId && (
|
||||
<FileEditor projectId={projectId!} repoId={selectedRepoId} />
|
||||
<FileEditor projectId={projectId!} repoId={selectedRepoId} gitStatus={gitStatus} />
|
||||
)}
|
||||
</main>
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user