Skip to content

ECOPROJECT-3446 | feat: Implement asynchronous job handling for RVToo… #215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

ECOPROJECT-3446 | feat: Implement asynchronous job handling for RVToo… #215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 37 additions & 2 deletions src/migration-wizard/contexts/discovery-sources/Provider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,12 +165,47 @@ export const Provider: React.FC<PropsWithChildren> = (props) => {
await listAssessments();
return assessment;
} else if (sourceType === 'rvtools' && rvToolFile) {
const assessment = await assessmentService.createFromRVTools(
// Start async job
const job = await assessmentService.createFromRVTools(
assessmentName,
rvToolFile,
);

// Poll job status every 2 seconds until completed or failed
const MAX_POLL_ATTEMPTS = 150; // 5 minutes at 2s intervals
let pollAttempts = 0;
let currentJob = job;
while (
currentJob.status === 'pending' ||
currentJob.status === 'running'
) {
if (++pollAttempts > MAX_POLL_ATTEMPTS) {
throw new Error(
`RVTools job processing timed out after ${MAX_POLL_ATTEMPTS * 2} seconds`,
);
}
// Wait 2 seconds before checking again
await new Promise((resolve) => setTimeout(resolve, 2000));
currentJob = await assessmentService.getRVToolsJobStatus(job.id);
}

// Check final status
if (currentJob.status === 'failed') {
throw new Error(
currentJob.error || 'Failed to create assessment from RVTools',
);
}

// Job completed successfully - refresh assessments list
await listAssessments();
return assessment;

// Return a minimal assessment object (the actual assessment is in the list)
// We need to return something for type compatibility, but the actual
// assessment data will come from the refreshed list
return {
id: currentJob.assessment_id || '',
name: assessmentName,
} as Assessment;
Comment on lines +205 to +208
Copy link

@coderabbitai coderabbitai bot Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Guard against missing assessment_id in completed job.

If currentJob.assessment_id is undefined or null after successful completion, using an empty string as fallback (line 206) could cause downstream issues in code that expects a valid assessment ID. This likely indicates a backend problem that should be surfaced rather than silently handled.

Consider adding validation:

+        // Validate that we got an assessment_id from the completed job
+        if (!currentJob.assessment_id) {
+          throw new Error(
+            'RVTools job completed but no assessment_id was returned',
+          );
+        }
+
         // Return a minimal assessment object (the actual assessment is in the list)
         // We need to return something for type compatibility, but the actual
         // assessment data will come from the refreshed list
         return {
-          id: currentJob.assessment_id || '',
+          id: currentJob.assessment_id,
           name: assessmentName,
         } as Assessment;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return {
id: currentJob.assessment_id || '',
name: assessmentName,
} as Assessment;
// Validate that we got an assessment_id from the completed job
if (!currentJob.assessment_id) {
throw new Error(
'RVTools job completed but no assessment_id was returned',
);
}
// Return a minimal assessment object (the actual assessment is in the list)
// We need to return something for type compatibility, but the actual
// assessment data will come from the refreshed list
return {
id: currentJob.assessment_id,
name: assessmentName,
} as Assessment;
🤖 Prompt for AI Agents 
In src/migration-wizard/contexts/discovery-sources/Provider.tsx around lines 205
to 208, the code currently falls back to an empty string when
currentJob.assessment_id is missing; instead validate that assessment_id exists
after job completion and surface an error if it does not. Update the function to
check if currentJob.assessment_id is null/undefined and, if so, either throw a
descriptive Error or return a rejected Promise (and log the condition) so the
caller can handle it, otherwise return the Assessment object with the guaranteed
id; ensure the error message includes job id/context to aid debugging.

} else if (sourceType === 'agent' && sourceId) {
const assessment = await assessmentApi.createAssessment({
assessmentForm: {
Expand Down
50 changes: 39 additions & 11 deletions src/pages/assessment/assessmentService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Assessment, Inventory } from '@migration-planner-ui/api-client/models';
import { Inventory } from '@migration-planner-ui/api-client/models';

export interface CreateAssessmentRequest {
name: string;
Expand All @@ -8,6 +8,14 @@ export interface CreateAssessmentRequest {
rvToolFile?: File;
}

export interface AsyncJob {
id: string;
status: 'pending' | 'running' | 'completed' | 'failed';
error?: string;
assessment_id?: string;
created_at: string;
}

export class AssessmentService {
private baseUrl: string;
private fetchApi: typeof fetch;
Expand All @@ -18,9 +26,9 @@ export class AssessmentService {
}

/**
* Create assessment from RVTools file (Excel)
* Create assessment from RVTools file (Excel) - Returns job ID for async processing
*/
async createFromRVTools(name: string, rvToolFile: File): Promise<Assessment> {
async createFromRVTools(name: string, rvToolFile: File): Promise<AsyncJob> {
const formData = new FormData();
formData.append('name', name);

Expand All @@ -30,11 +38,14 @@ export class AssessmentService {
});
formData.append('file', blob, rvToolFile.name);

const response = await this.fetchApi(`${this.baseUrl}/api/v1/assessments`, {
method: 'POST',
// Let browser set Content-Type automatically with correct boundary
body: formData,
});
const response = await this.fetchApi(
`${this.baseUrl}/api/v1/assessments/rvtools`,
{
method: 'POST',
// Let browser set Content-Type automatically with correct boundary
body: formData,
},
);

if (!response.ok) {
const errorText = await response.text();
Expand All @@ -44,9 +55,26 @@ export class AssessmentService {
return response.json();
}

async createAssessment(
request: CreateAssessmentRequest,
): Promise<Assessment> {
/**
* Get RVTools job status by job ID
*/
async getRVToolsJobStatus(jobId: string): Promise<AsyncJob> {
const response = await this.fetchApi(
`${this.baseUrl}/api/v1/rvtools/jobs/${jobId}`,
{
method: 'GET',
},
);

if (!response.ok) {
const errorText = await response.text();
throw new Error(`Failed to get RVTools job status: ${errorText}`);
}

return response.json();
}

async createAssessment(request: CreateAssessmentRequest): Promise<AsyncJob> {
const assessmentName =
request.name || `Assessment-${new Date().toISOString()}`;

Expand Down
Loading