feat(web): Add force resync buttons for repo & connections #610
Conversation
This comment has been minimized.
This comment has been minimized.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughIntroduces force resync buttons for connections and repositories via a new backend API server class. The server exposes POST endpoints Changes
Sequence DiagramsequenceDiagram
actor User
participant Frontend as Frontend UI
participant ServerAction as Server Action<br/>(withAuthV2)
participant WorkerAPI as Worker API<br/>(Port 3060)
participant Manager as Job Manager
participant DB as Database
User->>Frontend: Click "Trigger sync"<br/>button
Frontend->>Frontend: Set isSubmitting=true
Frontend->>ServerAction: Call syncConnection(id)<br/>or indexRepo(id)
rect rgb(200, 220, 255)
Note over ServerAction: Authorization Check
ServerAction->>ServerAction: Verify OWNER role
alt Role valid
ServerAction->>WorkerAPI: POST /api/sync-connection<br/>or /api/index-repo
else Role invalid
ServerAction-->>Frontend: ServiceError
end
end
rect rgb(200, 235, 200)
Note over WorkerAPI: Validation & Job Creation
WorkerAPI->>WorkerAPI: Validate input (zod)
WorkerAPI->>DB: Fetch connection/repo
alt Resource found
WorkerAPI->>Manager: createJobs()
Manager->>DB: Enqueue job
Manager-->>WorkerAPI: Return jobId[]
WorkerAPI-->>ServerAction: {jobId}
else Not found
WorkerAPI-->>ServerAction: 404 error
end
end
ServerAction->>Frontend: Return jobId
Frontend->>Frontend: Set isSubmitting=false
Frontend->>Frontend: Show success toast
Frontend->>Frontend: router.refresh()
Frontend->>User: Update UI with<br/>new job status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
packages/web/src/app/[domain]/repos/components/repoJobsTable.tsx (1)
197-215: Consider extracting jobId from success toast message.The success toast displays the jobId, but this information is included inline in the message string. If you need to make the jobId clickable or copyable in the future, consider storing it separately.
Optional improvement:
if (!isServiceError(response)) { const { jobId } = response; toast({ - description: `✅ Repository sync triggered successfully. Job ID: ${jobId}`, + description: ( + <div className="flex items-center gap-2"> + <span>✅ Repository sync triggered successfully. Job ID:</span> + <code className="text-xs">{jobId}</code> + </div> + ), })packages/web/src/features/workerApi/README.md (1)
1-1: Consider expanding the documentation.While the current description is accurate, adding examples of the available endpoints (
/api/sync-connection,/api/index-repo) and their request/response formats would improve developer experience.Example enhancement:
# Worker API This folder contains utilities to interact with the internal worker REST API. See packages/backend/api.ts ## Available Endpoints - `POST /api/sync-connection` - Trigger a connection sync - `POST /api/index-repo` - Trigger a repository index ## Usage ```typescript import { syncConnection, indexRepo } from './actions'; // Sync a connection const result = await syncConnection(connectionId); // Index a repository const result = await indexRepo(repoId);</blockquote></details> <details> <summary>packages/web/src/features/workerApi/actions.ts (1)</summary><blockquote> `11-59`: **Consider extracting common fetch logic.** Both actions share nearly identical structure (fetch → check response → parse → validate). A helper function could reduce duplication: ```typescript const callWorkerApi = async <T extends { jobId: string }>( endpoint: string, body: Record<string, unknown>, errorMessage: string ): Promise<T | ServiceError> => { const response = await fetch(`${WORKER_API_URL}${endpoint}`, { method: 'POST', body: JSON.stringify(body), headers: { 'Content-Type': 'application/json' }, }); if (!response.ok) { return unexpectedError(errorMessage); } const data = await response.json(); const schema = z.object({ jobId: z.string() }); return schema.parse(data) as T; }; export const syncConnection = async (connectionId: number) => sew(() => withAuthV2(({ role }) => withMinimumOrgRole(role, OrgRole.OWNER, () => callWorkerApi('/api/sync-connection', { connectionId }, 'Failed to sync connection') ) ) );packages/web/src/app/[domain]/settings/connections/components/connectionJobsTable.tsx (1)
195-212: Consider using finally block for loading state cleanup.The loading state reset should be in a
finallyblock to ensure it's always executed, even if an unexpected error occurs:const onSyncButtonClick = React.useCallback(async () => { setIsSyncSubmitting(true); - const response = await syncConnection(connectionId); - - if (!isServiceError(response)) { - const { jobId } = response; - toast({ - description: `✅ Connection synced successfully. Job ID: ${jobId}`, - }) - router.refresh(); - } else { - toast({ - description: `❌ Failed to sync connection. ${response.message}`, - }); + try { + const response = await syncConnection(connectionId); + + if (!isServiceError(response)) { + const { jobId } = response; + toast({ + description: `✅ Connection synced successfully. Job ID: ${jobId}`, + }) + router.refresh(); + } else { + toast({ + description: `❌ Failed to sync connection. ${response.message}`, + }); + } + } finally { + setIsSyncSubmitting(false); } - - setIsSyncSubmitting(false); }, [connectionId, router, toast]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (13)
CHANGELOG.md(1 hunks)packages/backend/package.json(1 hunks)packages/backend/src/api.ts(1 hunks)packages/backend/src/index.ts(3 hunks)packages/backend/src/promClient.ts(1 hunks)packages/backend/src/repoIndexManager.ts(2 hunks)packages/web/src/app/[domain]/repos/[id]/page.tsx(4 hunks)packages/web/src/app/[domain]/repos/components/repoJobsTable.tsx(5 hunks)packages/web/src/app/[domain]/settings/connections/[id]/page.tsx(3 hunks)packages/web/src/app/[domain]/settings/connections/components/connectionJobsTable.tsx(3 hunks)packages/web/src/features/workerApi/README.md(1 hunks)packages/web/src/features/workerApi/actions.ts(1 hunks)packages/web/src/withAuthV2.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*
📄 CodeRabbit inference engine (.cursor/rules/style.mdc)
Filenames should always be camelCase. Exception: if there are filenames in the same directory with a format other than camelCase, use that format to keep things consistent.
Files:
packages/web/src/features/workerApi/README.mdCHANGELOG.mdpackages/web/src/app/[domain]/repos/[id]/page.tsxpackages/web/src/app/[domain]/repos/components/repoJobsTable.tsxpackages/backend/src/index.tspackages/web/src/withAuthV2.tspackages/backend/src/repoIndexManager.tspackages/backend/src/api.tspackages/web/src/features/workerApi/actions.tspackages/backend/package.jsonpackages/backend/src/promClient.tspackages/web/src/app/[domain]/settings/connections/components/connectionJobsTable.tsxpackages/web/src/app/[domain]/settings/connections/[id]/page.tsx
🔇 Additional comments (17)
CHANGELOG.md (1)
14-16: LGTM!The changelog entry correctly documents the new force resync feature and follows the established format.
packages/web/src/app/[domain]/repos/components/repoJobsTable.tsx (2)
182-190: LGTM!The component signature correctly extends the props with
repoIdandisIndexButtonVisiblefor conditional button rendering.
281-305: LGTM!The button group layout and conditional rendering logic are well-structured. The LoadingButton properly shows loading state during async operations.
packages/web/src/app/[domain]/repos/[id]/page.tsx (2)
55-58: LGTM!The user role retrieval and error handling are correctly implemented. This enables role-based access control for the index button visibility.
181-186: LGTM!The role-based visibility check (
userRole === OrgRole.OWNER) correctly restricts the index button to owners only. This aligns with security best practices for sensitive operations.packages/backend/src/index.ts (3)
5-5: LGTM!The import of
express-async-errorsfor side effects is correct. This package automatically wraps Express async handlers to catch rejected promises.
77-82: LGTM!The Api instance is correctly instantiated with all required dependencies (promClient, prisma, connectionManager, repoIndexManager).
111-111: LGTM!The Api instance is properly disposed during cleanup, ensuring graceful shutdown of the HTTP server and any associated resources.
packages/backend/src/repoIndexManager.ts (1)
195-226: LGTM!The method visibility change to
publicand the addition of the return value (jobs.map(job => job.id)) correctly enable external callers to trigger indexing jobs and receive the created job IDs. This is necessary for the new API endpoints to return job IDs to clients.packages/web/src/app/[domain]/settings/connections/[id]/page.tsx (2)
28-31: LGTM!The input validation correctly handles invalid connection IDs by returning
notFound()when the parsed ID isNaN. This prevents runtime errors from invalid route parameters.
178-181: LGTM!The
connectionIdprop is correctly passed toConnectionJobsTable, enabling the sync functionality. This change aligns with the corresponding updates to the table component.packages/web/src/withAuthV2.ts (1)
180-184: LGTM! Explicit return type improves type safety.The addition of an explicit return type
Promise<T | ServiceError>enhances type safety and makes the function signature clearer. This aligns well with the worker API actions that rely on this typing.packages/web/src/app/[domain]/settings/connections/components/connectionJobsTable.tsx (1)
264-286: LGTM! Well-structured UI controls.The button group provides clear actions with appropriate visual feedback. The use of
LoadingButtonfor the sync action ensures good UX during async operations.packages/backend/src/promClient.ts (1)
3-3: LGTM! Good separation of concerns.Making
registrypublic enables theApiclass to expose metrics via HTTP while keeping metric collection logic inPromClient. This refactor properly separates HTTP server concerns from metrics collection.packages/backend/src/api.ts (3)
91-92: Verify array destructuring safety.Same concern as in
syncConnection: the destructuring assumes at least one job ID is returned. Verify thatRepoIndexManager.createJobsalways returns at least one element, or add validation as suggested above.
95-102: LGTM! Proper graceful shutdown.The
disposemethod correctly wrapsserver.closein a Promise for proper async cleanup. This ensures graceful shutdown of the HTTP server.
65-67: No changes required to array destructuring—current usage is safe.The
ConnectionManager.createJobsmethod (at line 120) returnsjobs.map(job => job.id). When called with a single-element array[connection], it reliably creates exactly one job and returns a single-element array. Since the connection is validated at line 60 before being passed tocreateJobs, the destructuringconst [jobId]is safe.The defensive check suggested in the review would only be necessary if empty arrays were possible inputs in this code path, which they are not.
The following adds force re-sync buttons for repo indexing and connection syncing. There are located in the repo details page and connections detail page, respectively.
This is accomplished by adding a internal api endpoint in the worker that exposes two endpoints,
/api/sync-connectionand/api/index-repo. Server actions proxy the requests to the worker endpoint.Summary by CodeRabbit
Release Notes
New Features
Documentation