refactor(ui): migrate GitHub views to React Query

- Migrate use-github-issues to useGitHubIssues query
- Migrate use-issue-comments to useGitHubIssueComments infinite query
- Migrate use-issue-validation to useGitHubValidations with mutations
- Migrate github-prs-view to useGitHubPRs query
- Support pagination for comments with useInfiniteQuery
- Remove manual loading state management

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Shirone
2026-01-15 16:21:49 +01:00
parent d1219a225c
commit c4e0a7cc96
4 changed files with 89 additions and 265 deletions

View File

@@ -1,79 +1,29 @@
import { useState, useEffect, useCallback, useRef } from 'react'; /**
import { createLogger } from '@automaker/utils/logger'; * GitHub Issues Hook
import { getElectronAPI, GitHubIssue } from '@/lib/electron'; *
* React Query-based hook for fetching GitHub issues.
*/
const logger = createLogger('GitHubIssues');
import { useAppStore } from '@/store/app-store'; import { useAppStore } from '@/store/app-store';
import { useGitHubIssues as useGitHubIssuesQuery } from '@/hooks/queries';
export function useGithubIssues() { export function useGithubIssues() {
const { currentProject } = useAppStore(); const { currentProject } = useAppStore();
const [openIssues, setOpenIssues] = useState<GitHubIssue[]>([]);
const [closedIssues, setClosedIssues] = useState<GitHubIssue[]>([]);
const [loading, setLoading] = useState(true);
const [refreshing, setRefreshing] = useState(false);
const [error, setError] = useState<string | null>(null);
const isMountedRef = useRef(true);
const fetchIssues = useCallback(async () => { const {
if (!currentProject?.path) { data,
if (isMountedRef.current) { isLoading: loading,
setError('No project selected'); isFetching: refreshing,
setLoading(false); error,
} refetch: refresh,
return; } = useGitHubIssuesQuery(currentProject?.path);
}
try {
if (isMountedRef.current) {
setError(null);
}
const api = getElectronAPI();
if (api.github) {
const result = await api.github.listIssues(currentProject.path);
if (isMountedRef.current) {
if (result.success) {
setOpenIssues(result.openIssues || []);
setClosedIssues(result.closedIssues || []);
} else {
setError(result.error || 'Failed to fetch issues');
}
}
}
} catch (err) {
if (isMountedRef.current) {
logger.error('Error fetching issues:', err);
setError(err instanceof Error ? err.message : 'Failed to fetch issues');
}
} finally {
if (isMountedRef.current) {
setLoading(false);
setRefreshing(false);
}
}
}, [currentProject?.path]);
useEffect(() => {
isMountedRef.current = true;
fetchIssues();
return () => {
isMountedRef.current = false;
};
}, [fetchIssues]);
const refresh = useCallback(() => {
if (isMountedRef.current) {
setRefreshing(true);
}
fetchIssues();
}, [fetchIssues]);
return { return {
openIssues, openIssues: data?.openIssues ?? [],
closedIssues, closedIssues: data?.closedIssues ?? [],
loading, loading,
refreshing, refreshing,
error, error: error instanceof Error ? error.message : error ? String(error) : null,
refresh, refresh,
}; };
} }

View File

@@ -1,9 +1,7 @@
import { useState, useEffect, useCallback, useRef } from 'react'; import { useMemo, useCallback } from 'react';
import { createLogger } from '@automaker/utils/logger'; import type { GitHubComment } from '@/lib/electron';
import { getElectronAPI, GitHubComment } from '@/lib/electron';
const logger = createLogger('IssueComments');
import { useAppStore } from '@/store/app-store'; import { useAppStore } from '@/store/app-store';
import { useGitHubIssueComments } from '@/hooks/queries';
interface UseIssueCommentsResult { interface UseIssueCommentsResult {
comments: GitHubComment[]; comments: GitHubComment[];
@@ -18,119 +16,36 @@ interface UseIssueCommentsResult {
export function useIssueComments(issueNumber: number | null): UseIssueCommentsResult { export function useIssueComments(issueNumber: number | null): UseIssueCommentsResult {
const { currentProject } = useAppStore(); const { currentProject } = useAppStore();
const [comments, setComments] = useState<GitHubComment[]>([]);
const [totalCount, setTotalCount] = useState(0);
const [loading, setLoading] = useState(false);
const [loadingMore, setLoadingMore] = useState(false);
const [hasNextPage, setHasNextPage] = useState(false);
const [endCursor, setEndCursor] = useState<string | undefined>(undefined);
const [error, setError] = useState<string | null>(null);
const isMountedRef = useRef(true);
const fetchComments = useCallback( // Use React Query infinite query
async (cursor?: string) => { const { data, isLoading, isFetchingNextPage, hasNextPage, fetchNextPage, refetch, error } =
if (!currentProject?.path || !issueNumber) { useGitHubIssueComments(currentProject?.path, issueNumber ?? undefined);
return;
}
const isLoadingMore = !!cursor; // Flatten all pages into a single comments array
const comments = useMemo(() => {
return data?.pages.flatMap((page) => page.comments) ?? [];
}, [data?.pages]);
try { // Get total count from the first page
if (isMountedRef.current) { const totalCount = data?.pages[0]?.totalCount ?? 0;
setError(null);
if (isLoadingMore) {
setLoadingMore(true);
} else {
setLoading(true);
}
}
const api = getElectronAPI();
if (api.github) {
const result = await api.github.getIssueComments(
currentProject.path,
issueNumber,
cursor
);
if (isMountedRef.current) {
if (result.success) {
if (isLoadingMore) {
// Append new comments
setComments((prev) => [...prev, ...(result.comments || [])]);
} else {
// Replace all comments
setComments(result.comments || []);
}
setTotalCount(result.totalCount || 0);
setHasNextPage(result.hasNextPage || false);
setEndCursor(result.endCursor);
} else {
setError(result.error || 'Failed to fetch comments');
}
}
}
} catch (err) {
if (isMountedRef.current) {
logger.error('Error fetching comments:', err);
setError(err instanceof Error ? err.message : 'Failed to fetch comments');
}
} finally {
if (isMountedRef.current) {
setLoading(false);
setLoadingMore(false);
}
}
},
[currentProject?.path, issueNumber]
);
// Reset and fetch when issue changes
useEffect(() => {
isMountedRef.current = true;
if (issueNumber) {
// Reset state when issue changes
setComments([]);
setTotalCount(0);
setHasNextPage(false);
setEndCursor(undefined);
setError(null);
fetchComments();
} else {
// Clear comments when no issue is selected
setComments([]);
setTotalCount(0);
setHasNextPage(false);
setEndCursor(undefined);
setLoading(false);
setError(null);
}
return () => {
isMountedRef.current = false;
};
}, [issueNumber, fetchComments]);
const loadMore = useCallback(() => { const loadMore = useCallback(() => {
if (hasNextPage && endCursor && !loadingMore) { if (hasNextPage && !isFetchingNextPage) {
fetchComments(endCursor); fetchNextPage();
} }
}, [hasNextPage, endCursor, loadingMore, fetchComments]); }, [hasNextPage, isFetchingNextPage, fetchNextPage]);
const refresh = useCallback(() => { const refresh = useCallback(() => {
setComments([]); refetch();
setEndCursor(undefined); }, [refetch]);
fetchComments();
}, [fetchComments]);
return { return {
comments, comments,
totalCount, totalCount,
loading, loading: isLoading,
loadingMore, loadingMore: isFetchingNextPage,
hasNextPage, hasNextPage: hasNextPage ?? false,
error, error: error instanceof Error ? error.message : null,
loadMore, loadMore,
refresh, refresh,
}; };

View File

@@ -13,6 +13,7 @@ import type { LinkedPRInfo, PhaseModelEntry, ModelId } from '@automaker/types';
import { useAppStore } from '@/store/app-store'; import { useAppStore } from '@/store/app-store';
import { toast } from 'sonner'; import { toast } from 'sonner';
import { isValidationStale } from '../utils'; import { isValidationStale } from '../utils';
import { useValidateIssue, useMarkValidationViewed } from '@/hooks/mutations';
const logger = createLogger('IssueValidation'); const logger = createLogger('IssueValidation');
@@ -46,6 +47,10 @@ export function useIssueValidation({
new Map() new Map()
); );
const audioRef = useRef<HTMLAudioElement | null>(null); const audioRef = useRef<HTMLAudioElement | null>(null);
// React Query mutations
const validateIssueMutation = useValidateIssue(currentProject?.path ?? '');
const markViewedMutation = useMarkValidationViewed(currentProject?.path ?? '');
// Refs for stable event handler (avoids re-subscribing on state changes) // Refs for stable event handler (avoids re-subscribing on state changes)
const selectedIssueRef = useRef<GitHubIssue | null>(null); const selectedIssueRef = useRef<GitHubIssue | null>(null);
const showValidationDialogRef = useRef(false); const showValidationDialogRef = useRef(false);
@@ -240,7 +245,7 @@ export function useIssueValidation({
} }
// Check if already validating this issue // Check if already validating this issue
if (validatingIssues.has(issue.number)) { if (validatingIssues.has(issue.number) || validateIssueMutation.isPending) {
toast.info(`Validation already in progress for issue #${issue.number}`); toast.info(`Validation already in progress for issue #${issue.number}`);
return; return;
} }
@@ -254,11 +259,6 @@ export function useIssueValidation({
return; return;
} }
// Start async validation in background (no dialog - user will see badge when done)
toast.info(`Starting validation for issue #${issue.number}`, {
description: 'You will be notified when the analysis is complete',
});
// Use provided model override or fall back to phaseModels.validationModel // Use provided model override or fall back to phaseModels.validationModel
// Extract model string and thinking level from PhaseModelEntry (handles both old string format and new object format) // Extract model string and thinking level from PhaseModelEntry (handles both old string format and new object format)
const effectiveModelEntry = modelEntry const effectiveModelEntry = modelEntry
@@ -276,40 +276,22 @@ export function useIssueValidation({
const thinkingLevelToUse = normalizedEntry.thinkingLevel; const thinkingLevelToUse = normalizedEntry.thinkingLevel;
const reasoningEffortToUse = normalizedEntry.reasoningEffort; const reasoningEffortToUse = normalizedEntry.reasoningEffort;
try { // Use mutation to trigger validation (toast is handled by mutation)
const api = getElectronAPI(); validateIssueMutation.mutate({
if (api.github?.validateIssue) { issue,
const validationInput = { model: modelToUse,
issueNumber: issue.number, thinkingLevel: thinkingLevelToUse,
issueTitle: issue.title, reasoningEffort: reasoningEffortToUse,
issueBody: issue.body || '', comments,
issueLabels: issue.labels.map((l) => l.name), linkedPRs,
comments, // Include comments if provided });
linkedPRs, // Include linked PRs if provided
};
const result = await api.github.validateIssue(
currentProject.path,
validationInput,
modelToUse,
thinkingLevelToUse,
reasoningEffortToUse
);
if (!result.success) {
toast.error(result.error || 'Failed to start validation');
}
// On success, the result will come through the event stream
}
} catch (err) {
logger.error('Validation error:', err);
toast.error(err instanceof Error ? err.message : 'Failed to validate issue');
}
}, },
[ [
currentProject?.path, currentProject?.path,
validatingIssues, validatingIssues,
cachedValidations, cachedValidations,
phaseModels.validationModel, phaseModels.validationModel,
validateIssueMutation,
onValidationResultChange, onValidationResultChange,
onShowValidationDialogChange, onShowValidationDialogChange,
] ]
@@ -325,10 +307,8 @@ export function useIssueValidation({
// Mark as viewed if not already viewed // Mark as viewed if not already viewed
if (!cached.viewedAt && currentProject?.path) { if (!cached.viewedAt && currentProject?.path) {
try { markViewedMutation.mutate(issue.number, {
const api = getElectronAPI(); onSuccess: () => {
if (api.github?.markValidationViewed) {
await api.github.markValidationViewed(currentProject.path, issue.number);
// Update local state // Update local state
setCachedValidations((prev) => { setCachedValidations((prev) => {
const next = new Map(prev); const next = new Map(prev);
@@ -341,16 +321,15 @@ export function useIssueValidation({
} }
return next; return next;
}); });
} },
} catch (err) { });
logger.error('Failed to mark validation as viewed:', err);
}
} }
} }
}, },
[ [
cachedValidations, cachedValidations,
currentProject?.path, currentProject?.path,
markViewedMutation,
onValidationResultChange, onValidationResultChange,
onShowValidationDialogChange, onShowValidationDialogChange,
] ]
@@ -361,5 +340,6 @@ export function useIssueValidation({
cachedValidations, cachedValidations,
handleValidateIssue, handleValidateIssue,
handleViewCachedValidation, handleViewCachedValidation,
isValidating: validateIssueMutation.isPending,
}; };
} }

View File

@@ -1,59 +1,36 @@
import { useState, useEffect, useCallback } from 'react'; /**
import { createLogger } from '@automaker/utils/logger'; * GitHub PRs View
*
* Displays pull requests using React Query for data fetching.
*/
import { useState, useCallback } from 'react';
import { GitPullRequest, Loader2, RefreshCw, ExternalLink, GitMerge, X } from 'lucide-react'; import { GitPullRequest, Loader2, RefreshCw, ExternalLink, GitMerge, X } from 'lucide-react';
import { getElectronAPI, GitHubPR } from '@/lib/electron'; import { getElectronAPI, type GitHubPR } from '@/lib/electron';
import { useAppStore } from '@/store/app-store'; import { useAppStore } from '@/store/app-store';
import { Button } from '@/components/ui/button'; import { Button } from '@/components/ui/button';
import { Markdown } from '@/components/ui/markdown'; import { Markdown } from '@/components/ui/markdown';
import { cn } from '@/lib/utils'; import { cn } from '@/lib/utils';
import { useGitHubPRs } from '@/hooks/queries';
const logger = createLogger('GitHubPRsView');
export function GitHubPRsView() { export function GitHubPRsView() {
const [openPRs, setOpenPRs] = useState<GitHubPR[]>([]);
const [mergedPRs, setMergedPRs] = useState<GitHubPR[]>([]);
const [loading, setLoading] = useState(true);
const [refreshing, setRefreshing] = useState(false);
const [error, setError] = useState<string | null>(null);
const [selectedPR, setSelectedPR] = useState<GitHubPR | null>(null); const [selectedPR, setSelectedPR] = useState<GitHubPR | null>(null);
const { currentProject } = useAppStore(); const { currentProject } = useAppStore();
const fetchPRs = useCallback(async () => { const {
if (!currentProject?.path) { data,
setError('No project selected'); isLoading: loading,
setLoading(false); isFetching: refreshing,
return; error,
} refetch,
} = useGitHubPRs(currentProject?.path);
try { const openPRs = data?.openPRs ?? [];
setError(null); const mergedPRs = data?.mergedPRs ?? [];
const api = getElectronAPI();
if (api.github) {
const result = await api.github.listPRs(currentProject.path);
if (result.success) {
setOpenPRs(result.openPRs || []);
setMergedPRs(result.mergedPRs || []);
} else {
setError(result.error || 'Failed to fetch pull requests');
}
}
} catch (err) {
logger.error('Error fetching PRs:', err);
setError(err instanceof Error ? err.message : 'Failed to fetch pull requests');
} finally {
setLoading(false);
setRefreshing(false);
}
}, [currentProject?.path]);
useEffect(() => {
fetchPRs();
}, [fetchPRs]);
const handleRefresh = useCallback(() => { const handleRefresh = useCallback(() => {
setRefreshing(true); refetch();
fetchPRs(); }, [refetch]);
}, [fetchPRs]);
const handleOpenInGitHub = useCallback((url: string) => { const handleOpenInGitHub = useCallback((url: string) => {
const api = getElectronAPI(); const api = getElectronAPI();
@@ -98,7 +75,9 @@ export function GitHubPRsView() {
<GitPullRequest className="h-12 w-12 text-destructive" /> <GitPullRequest className="h-12 w-12 text-destructive" />
</div> </div>
<h2 className="text-lg font-medium mb-2">Failed to Load Pull Requests</h2> <h2 className="text-lg font-medium mb-2">Failed to Load Pull Requests</h2>
<p className="text-muted-foreground max-w-md mb-4">{error}</p> <p className="text-muted-foreground max-w-md mb-4">
{error instanceof Error ? error.message : 'Failed to fetch pull requests'}
</p>
<Button variant="outline" onClick={handleRefresh}> <Button variant="outline" onClick={handleRefresh}>
<RefreshCw className="h-4 w-4 mr-2" /> <RefreshCw className="h-4 w-4 mr-2" />
Try Again Try Again
@@ -196,9 +175,9 @@ export function GitHubPRsView() {
<div className="flex items-center justify-between p-3 border-b border-border bg-muted/30"> <div className="flex items-center justify-between p-3 border-b border-border bg-muted/30">
<div className="flex items-center gap-2 min-w-0"> <div className="flex items-center gap-2 min-w-0">
{selectedPR.state === 'MERGED' ? ( {selectedPR.state === 'MERGED' ? (
<GitMerge className="h-4 w-4 text-purple-500 flex-shrink-0" /> <GitMerge className="h-4 w-4 text-purple-500 shrink-0" />
) : ( ) : (
<GitPullRequest className="h-4 w-4 text-green-500 flex-shrink-0" /> <GitPullRequest className="h-4 w-4 text-green-500 shrink-0" />
)} )}
<span className="text-sm font-medium truncate"> <span className="text-sm font-medium truncate">
#{selectedPR.number} {selectedPR.title} #{selectedPR.number} {selectedPR.title}
@@ -209,7 +188,7 @@ export function GitHubPRsView() {
</span> </span>
)} )}
</div> </div>
<div className="flex items-center gap-2 flex-shrink-0"> <div className="flex items-center gap-2 shrink-0">
<Button <Button
variant="outline" variant="outline"
size="sm" size="sm"
@@ -341,16 +320,16 @@ function PRRow({
onClick={onClick} onClick={onClick}
> >
{pr.state === 'MERGED' ? ( {pr.state === 'MERGED' ? (
<GitMerge className="h-4 w-4 text-purple-500 mt-0.5 flex-shrink-0" /> <GitMerge className="h-4 w-4 text-purple-500 mt-0.5 shrink-0" />
) : ( ) : (
<GitPullRequest className="h-4 w-4 text-green-500 mt-0.5 flex-shrink-0" /> <GitPullRequest className="h-4 w-4 text-green-500 mt-0.5 shrink-0" />
)} )}
<div className="flex-1 min-w-0"> <div className="flex-1 min-w-0">
<div className="flex items-center gap-2"> <div className="flex items-center gap-2">
<span className="text-sm font-medium truncate">{pr.title}</span> <span className="text-sm font-medium truncate">{pr.title}</span>
{pr.isDraft && ( {pr.isDraft && (
<span className="px-1.5 py-0.5 text-[10px] font-medium rounded bg-muted text-muted-foreground flex-shrink-0"> <span className="px-1.5 py-0.5 text-[10px] font-medium rounded bg-muted text-muted-foreground shrink-0">
Draft Draft
</span> </span>
)} )}
@@ -401,7 +380,7 @@ function PRRow({
<Button <Button
variant="ghost" variant="ghost"
size="sm" size="sm"
className="flex-shrink-0 opacity-0 group-hover:opacity-100" className="shrink-0 opacity-0 group-hover:opacity-100"
onClick={(e) => { onClick={(e) => {
e.stopPropagation(); e.stopPropagation();
onOpenExternal(); onOpenExternal();