fix(admin-ui): improve Agama Flow status update handling to avoid lon… #2532
Conversation
…g pending state (#2467)
faisalsiddique4400
requested review from
duttarnab and
syntrydy
as code owners
📝 WalkthroughWalkthroughRefactored the Agama project deployment flow in AgamaListPage.tsx by replacing React Query mutation abstractions with direct Axios API calls. Removed usePostAgamaPrj, getAgamaProject, and getGetAgamaProjectQueryKey imports and replaced their implementations with inline fetch, base64 decode, ZIP creation, and deployment posting logic. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
mo-auto
added
comp-admin-ui
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin-ui/plugins/auth-server/components/Agama/AgamaListPage.tsx (1)
554-558: Improve error handling specificity and state cleanup consistency.Two issues identified:
Line 556: The error message "File not found or deployment failed" is too generic. It doesn't differentiate between download failure, base64 decode failure, or deployment POST failure, making it difficult for users to understand what went wrong.
Line 557: Inconsistent state cleanup—
setRepoName(null)is called in the success path (line 552) but not in the error path. This leaves stale state after an error, potentially confusing users who see the previously selected repository still selected.Apply this diff to improve error handling:
} catch (error) { console.error('Error deploying project:', error) - toast.error('File not found or deployment failed') + const errorMessage = error instanceof Error ? error.message : 'Failed to deploy project' + toast.error(errorMessage) setFileLoading(false) + setRepoName(null) }
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/auth-server/components/Agama/AgamaListPage.tsx(1 hunks)
🔇 Additional comments (4)
admin-ui/plugins/auth-server/components/Agama/AgamaListPage.tsx (4)
516-517: LGTM!The project name extraction and state update are correct.
534-542: LGTM!The Blob/File creation and deployment POST request are implemented correctly. Good use of
encodeURIComponentfor the project name in the URL path.
544-553: LGTM!Success handling is comprehensive with proper toast notification, query invalidation, audit logging, and state cleanup.
502-559: Clarify how single query invalidation addresses long-pending status updates without continuous polling.The handleDeploy function calls
queryClient.invalidateQueries()after deployment, which triggers a single refetch. However, the PR objectives mention "fixing polling and state update logic to avoid long pending state," yet no polling mechanism (refetchIntervalor polling loop) exists in the code. Status is computed based on whetherfinishedAtis set by the backend.If backend processing is asynchronous, a single refetch won't continuously check status until completion. Clarify:
- Whether the deployment endpoint now responds after processing completes (synchronous), or
- If polling was added in a related component or hook that isn't visible in this change
| const downloadResponse = await AXIOS_INSTANCE.get(`/api/v1/agama-repo/download`, { | ||
| params: { downloadLink: downloadUrl }, | ||
| }) | ||
|
|
||
| if (!downloadResult) { | ||
| if (!downloadResponse.data) { | ||
| throw new Error('Failed to download project') | ||
| } | ||
| const base64Data = downloadResponse.data as string | ||
| const binaryString = atob(base64Data) | ||
| const bytes = new Uint8Array(binaryString.length) | ||
| for (let i = 0; i < binaryString.length; i++) { | ||
| bytes[i] = binaryString.charCodeAt(i) | ||
| } |
There was a problem hiding this comment.
Add type validation and error handling for base64 decoding.
The code has critical type safety and error handling gaps:
- Line 527: Type assertion
as stringis unsafe. If the backend returns a non-string (e.g., error JSON object, null, array), the code will fail at line 528. - Line 528:
atob()will throw anInvalidCharacterErrorif the input is not valid base64, but there's no try-catch around this specific operation. The outer try-catch at line 519 will catch it, but the error message at line 556 won't distinguish between download failure and decoding failure. - Line 524: The check
if (!downloadResponse.data)is insufficient—it won't catch empty strings or invalid formats.
If the backend returns an error response (e.g., {"error": "not found"}), the code will attempt to decode it as base64 and throw a cryptic error.
Apply this diff to add proper validation:
try {
const downloadResponse = await AXIOS_INSTANCE.get(`/api/v1/agama-repo/download`, {
params: { downloadLink: downloadUrl },
})
- if (!downloadResponse.data) {
+ if (!downloadResponse.data || typeof downloadResponse.data !== 'string') {
+ throw new Error('Invalid response from download endpoint')
+ }
+
+ if (downloadResponse.data.trim().length === 0) {
throw new Error('Failed to download project')
}
+
const base64Data = downloadResponse.data as string
+
+ // Validate base64 format before decoding
+ try {
- const binaryString = atob(base64Data)
+ const binaryString = atob(base64Data)
- const bytes = new Uint8Array(binaryString.length)
- for (let i = 0; i < binaryString.length; i++) {
- bytes[i] = binaryString.charCodeAt(i)
- }
+ const bytes = new Uint8Array(binaryString.length)
+ for (let i = 0; i < binaryString.length; i++) {
+ bytes[i] = binaryString.charCodeAt(i)
+ }
+ } catch (decodeError) {
+ console.error('Base64 decode error:', decodeError)
+ throw new Error('Invalid base64 data received from server')
+ }🤖 Prompt for AI Agents
In admin-ui/plugins/auth-server/components/Agama/AgamaListPage.tsx around lines
520 to 532, validate that downloadResponse.data is a non-empty string before
treating it as base64 (e.g., typeof check and trim length), and if invalid throw
a descriptive error; wrap the atob(base64Data) call in its own try-catch to
catch InvalidCharacterError and rethrow a clear "Invalid base64 data" error (or
return a specific failure), and ensure any thrown errors are distinguishable
from network/download failures so the outer error handling can report whether
the failure was downloading, empty/invalid type, or base64 decoding.
| await AXIOS_INSTANCE.post( | ||
| `/api/v1/agama-deployment/${encodeURIComponent(projectNameToUse)}`, | ||
| projectFile, | ||
| { headers: { 'Content-Type': 'application/zip' } }, | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting shared deployment logic into a reusable function.
The deployment POST logic here (lines 538-542) is duplicated in the submitData function (lines 201-205). Both functions:
- POST to
/api/v1/agama-deployment/${encodeURIComponent(projectName)} - Use identical headers:
{ 'Content-Type': 'application/zip' } - Have similar success handling (toast, query invalidation, audit logging, state cleanup)
Consider extracting the common deployment logic:
const deployAgamaProject = async (projectName: string, projectFile: File): Promise<void> => {
await AXIOS_INSTANCE.post(
`/api/v1/agama-deployment/${encodeURIComponent(projectName)}`,
projectFile,
{ headers: { 'Content-Type': 'application/zip' } },
)
dispatch(updateToast(true, 'success'))
await queryClient.invalidateQueries({ queryKey: getGetAgamaPrjQueryKey() })
await logAgamaCreation({} as Deployment, `Deployed Agama project: ${projectName}`)
}Then both submitData and handleDeploy can call this shared function, reducing duplication and improving maintainability.
🤖 Prompt for AI Agents
In admin-ui/plugins/auth-server/components/Agama/AgamaListPage.tsx around lines
201-205 and 538-542, the POST + success handling for deploying an Agama project
is duplicated; extract that logic into a single async helper (e.g.,
deployAgamaProject(projectName: string, projectFile: File)): it should perform
the AXIOS_INSTANCE.post to
`/api/v1/agama-deployment/${encodeURIComponent(projectName)}` with headers
'Content-Type: application/zip', dispatch the success toast, invalidate the
getAgama project query via
queryClient.invalidateQueries(getGetAgamaPrjQueryKey()), call
logAgamaCreation/audit with the project name, and return or throw errors as
appropriate; then replace the duplicated blocks in submitData (lines ~201-205)
and handleDeploy (lines 538-542) to call this helper and keep any
caller-specific state cleanup (e.g., clearing selected file or closing modal) in
the callers.
fix(admin-ui): improve Agama Flow status update handling to avoid long pending state (#2467)
Summary
Agama Flow status was remaining in the pending state for an extended period after deployment, causing confusion and making it difficult for users to determine whether processing had completed.
Fix Summary
Outcome
Closes
Closes: #2467
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.