✨ Workflow - Source platform discover and import applications wizard #2545
Conversation
Resolves: https://issues.redhat.com/browse/MTA-5249 Resolves: konveyor#2289 Adds a wizard to discover and import applications from a source platform: - The wizard captures filter data from the user. - The filters are added to the import task. - The set of filters are defined by a schema. - The schema is pulled from domain, variant, subject "platform", "cloudfoundry", "filter" - `SchemaDefinedFields` to be used to input the schema data Signed-off-by: Scott J Dickerson <[email protected]>
Signed-off-by: Scott J Dickerson <[email protected]>
Signed-off-by: Scott J Dickerson <[email protected]>
Signed-off-by: Scott J Dickerson <[email protected]>
…just the meta-data Signed-off-by: Scott J Dickerson <[email protected]>
Signed-off-by: Scott J Dickerson <[email protected]>
- use reducer for wizard level state - steps have their own forms as needed Signed-off-by: Scott J Dickerson <[email protected]>
Signed-off-by: Scott J Dickerson <[email protected]>
Signed-off-by: Scott J Dickerson <[email protected]>
Signed-off-by: Scott J Dickerson <[email protected]>
Signed-off-by: Scott J Dickerson <[email protected]>
WalkthroughAdds a Discover & Import wizard and related UI (components, hooks, types, translations) to create and submit platform application import tasks; refactors task models and moves task REST functions into a dedicated rest/tasks module; renames platform discovery schema API/hook and enriches platforms UI with per-platform task status. Changes
Sequence Diagram(s)sequenceDiagram
actor User
User->>SourcePlatforms Page: Click "Discover & Import"
SourcePlatforms Page->>DiscoverImportWizard: open(platform)
DiscoverImportWizard->>FilterInput: load schema for platform.kind
FilterInput->>Schemas API: getPlatformDiscoveryFilterSchema(kind)
Schemas API-->>FilterInput: schema | none
User->>DiscoverImportWizard: submit (filters)
DiscoverImportWizard->>useStartPlatformApplicationImport: submitTask(platform, filters)
useStartPlatformApplicationImport->>Tasks REST: createTask(kind="application-import", platform, data)
Tasks REST-->>useStartPlatformApplicationImport: created task
useStartPlatformApplicationImport->>Tasks REST: submitTask(id)
Tasks REST-->>useStartPlatformApplicationImport: accepted
useStartPlatformApplicationImport-->>DiscoverImportWizard: { success[], failure[] }
DiscoverImportWizard->>Notifications: show toasts
DiscoverImportWizard-->>User: render Results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🔭 Outside diff range comments (5)
client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx (1)
266-303: Excluded rule tags are wired to labels.excluded instead of rules.tags.excludedForm field excludedRulesTags semantically maps to rule tags, but you assign it to labels.excluded. This likely breaks tag-based rule exclusion on the backend, and may also incorrectly exclude labels.
Proposed fix: put excludedRulesTags under rules.tags.excluded and leave labels.excluded for label exclusions (empty if none collected).
Apply this diff inside the rules object:
- rules: { - labels: { - included: Array.from( + rules: { + labels: { + included: Array.from( new Set<string>([ ...fieldValues.selectedTargetLabels .filter( (label) => getParsedLabel(label.label).labelType !== "source" ) .map((label) => label.label) .filter(Boolean), ...fieldValues.selectedSourceLabels .map((label) => label.label) .filter(Boolean), ]) ), - excluded: fieldValues.excludedRulesTags, + excluded: [], }, path: fieldValues.customRulesFiles.length > 0 ? "/rules" : "", + ...(fieldValues.excludedRulesTags?.length + ? { tags: { excluded: fieldValues.excludedRulesTags } } + : {}), ...(fieldValues.rulesKind === "repository" && { repository: { kind: fieldValues?.repositoryType, url: fieldValues?.sourceRepository?.trim(), branch: fieldValues?.branch?.trim(), path: fieldValues?.rootPath?.trim(), }, }),client/src/app/pages/applications/useDecoratedApplications.ts (2)
74-91: Group by application safely; also ensure per-kind arrays are sorted by recencyFiltering tasks without application is good. However, without explicit sorting, tasks[0] per kind may not be the latest. Consider sorting each kind’s tasks by started/createTime desc to make “current” task deterministic.
- const groupedByIdByKind = mapEntries(byApplicationId, (id, tasks) => [ - id, - { - ...group(tasks, (task) => task.kind ?? task.addon ?? ""), - } as TasksGroupedByKind, - ]); + const groupedByIdByKind = mapEntries(byApplicationId, (_id, tasksForApp) => { + const byKind = group(tasksForApp, (task) => task.kind ?? task.addon ?? ""); + const byKindSorted = mapEntries(byKind, (kind, arr) => [ + kind, + [...arr].sort((a, b) => { + const at = new Date(a.started ?? a.createTime).getTime(); + const bt = new Date(b.started ?? b.createTime).getTime(); + return bt - at; + }), + ]); + return [_id, byKindSorted as TasksGroupedByKind]; + });
129-131: Type mismatch: defaulting tasksByKind to []When there are no tasks, tasksByKind should be an object, not an array. Using [] can cause runtime issues with listify and property access.
- const tasksByKind = tasksByIdByKind[app.id] ?? []; + const tasksByKind: TasksGroupedByKind = tasksByIdByKind[app.id] ?? {};client/src/app/components/task-manager/TaskManagerDrawer.tsx (2)
155-158: Collapsed title can render “undefined” when application is absentThe collapsed title always interpolates applicationName, which will display “undefined” for platform-only tasks. Prefer a robust subject that falls back to platformName or a placeholder.
Apply this diff to build a subject and avoid undefined in the UI:
const starttime = dayjs(task.started ?? task.createTime); - const title = expanded - ? `${task.id} (${task.addon})` - : `${task.id} (${task.addon}) - ${task.applicationName} - ${ - task.priority ?? 0 - }`; + const subject = task.applicationName ?? task.platformName ?? "—"; + const title = expanded + ? `${task.id} (${task.addon})` + : `${task.id} (${task.addon}) - ${subject} - ${task.priority ?? 0}`;
189-206: Clicking the actions menu toggles row expansion due to event bubblingThe NotificationDrawerListItem has an onClick handler to expand/collapse the row. Clicking the actions toggle or menu items will bubble and trigger expansion unintentionally. Stop propagation on those clicks.
Apply this diff:
- onClick={() => onActionsExpandToggle(!actionsExpanded)} + onClick={(e) => { + e.stopPropagation(); + onActionsExpandToggle(!actionsExpanded); + }}And for the menu items:
- {taskActionItems.map(({ title, onClick, isAriaDisabled }) => ( + {taskActionItems.map(({ title, onClick, isAriaDisabled }) => ( <DropdownItem key={title} - onClick={onClick} + onClick={(e) => { + e.stopPropagation(); + onClick(); + }} isDisabled={isAriaDisabled} > {title} </DropdownItem> ))}
🧹 Nitpick comments (28)
client/src/app/components/schema-defined-fields/SchemaDefinedFields.tsx (1)
29-31: Consider renaming the state variable for clarityThe state variable
isJsonViewcould be more accurately named since it's now initialized based on schema complexity, not just user preference. Consider renaming it toisShowingJsonEditororisCodeViewto better reflect its purpose.- const [isJsonView, setIsJsonView] = React.useState<boolean>( + const [isCodeView, setIsCodeView] = React.useState<boolean>( !jsonSchema || isComplexSchema(jsonSchema) );And update all references accordingly throughout the component.
client/src/app/api/models.ts (1)
19-22: Consider more descriptive names for utility typesThe type names
EmptyObjectandJsonDocumentcould be more descriptive:
EmptyObjectcould beEmptyRecordorNoPropertiesto better indicate it's a record with no propertiesJsonDocumentcould beJsonObjectto align with JSON terminology-export interface EmptyObject extends Record<string, never> {} +export interface EmptyRecord extends Record<string, never> {} -export interface JsonDocument extends Record<string, unknown> {} +export interface JsonObject extends Record<string, unknown> {}client/src/app/api/rest/tasks.ts (2)
44-48: Prefer params over string concatenation for query flagsBuilding the URL with "?merged=1" is brittle and risks duplication if further params are added. Use axios params for safer encoding and consistency.
Apply this diff:
- let url = `${TASKS}/${id}`; - if (merged) { - url += "?merged=1"; - } + const url = `${TASKS}/${id}`;And change the request to include params:
- return axios - .get<Task<object> | string>(url, { + return axios + .get<Task<object> | string>(url, { headers: headers, - responseType: responseType, + responseType: responseType, + params: merged ? { merged: 1 } : undefined, })
95-101: Normalize return shape to data for consistencycreateTask unwraps response.data, whereas updateTask returns the full Axios response. Consider unwrapping here as well for a uniform API surface.
Apply this diff:
-export const updateTask = <D, TaskType extends Task<D> = Task<D>>( - task: Partial<TaskType> & { id: number } -) => axios.patch<TaskType>(`${TASKS}/${task.id}`, task); +export const updateTask = <D, TaskType extends Task<D> = Task<D>>( + task: Partial<TaskType> & { id: number } +) => + axios + .patch<TaskType>(`${TASKS}/${task.id}`, task) + .then((response) => response.data);client/src/app/pages/tasks/useTaskActions.tsx (1)
54-61: Make preemption toggle explicit for undefined policy stateCurrently, undefined coerces to true on first toggle, which is fine but implicit. Consider an explicit default to improve readability.
Apply this diff:
- const togglePreemption = (task: Task<unknown>) => + const togglePreemption = (task: Task<unknown>) => updateTask({ id: task.id, policy: { - preemptEnabled: !task.policy?.preemptEnabled, + preemptEnabled: !(task.policy?.preemptEnabled ?? false), }, });client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx (1)
112-118: Use i18n for notification titles/messagesHardcoded strings bypass translation. Consider t(...) for consistency with the rest of the wizard.
- const onCreateTaskgroupError = (_error: Error | unknown) => { - pushNotification({ - title: "Taskgroup creation failed", - variant: "danger", - }); + const onCreateTaskgroupError = (_error: Error | unknown) => { + pushNotification({ + title: t("toastr.error.taskgroupCreateFailed"), + variant: "danger", + });client/src/app/pages/applications/retrieve-config-wizard/results.tsx (1)
112-124: Nit: Localize “View Task”Minor i18n consistency nit.
- <Button + <Button variant="link" component={(props) => ( <Link {...props} to={`/tasks/${result.task.id}`} /> )} > - View Task + {t("actions.viewTask")} </Button>client/src/app/pages/source-platforms/components/column-platform-name.tsx (4)
176-180: Aria label mentions “application” instead of “platform”Minor accessibility nit: update the label to reference platform, not application.
- aria-label="application task information popover" + aria-label="source platform task information popover"
151-156: Optional: Link the Id to task details for consistencyYou already link the Kind to details; linking the Id improves affordance.
- <Td>{task.id}</Td> + <Td> + <Link to={linkToDetails(task)}>{task.id}</Link> + </Td>
40-105: Optional: centralize status icon mappingThere’s a TODO about aligning with TaskStateIcon. Consider reusing or extending it to reduce duplication and keep visuals consistent with the rest of the UI.
184-187: Nit: localize static stringsPopover footer and status header texts are hardcoded. Consider using i18n keys for consistency.
- <Link to={linkToTasks(platform.name)}> - View all tasks for the platform - </Link> + <Link to={linkToTasks(platform.name)}> + {t("platforms.viewAllTasksForPlatform")} + </Link>client/src/app/components/task-manager/TaskManagerDrawer.tsx (2)
191-191: ARIA label can be “Actions for task undefined”task.name is optional; when absent, the label becomes awkward. Fall back to task.id for a stable label.
- aria-label={`Actions for task ${task.name}`} + aria-label={`Actions for task ${task.name ?? task.id}`}
218-224: Body fields: good conditional rendering; consider linkingConditional display of application and platform names looks correct. Consider turning these into links to their respective views with a pre-applied filter, per the TODO, to improve navigation.
I can wire this up to Paths with filter query params once you confirm the desired route(s).
client/src/app/pages/tasks/tasks-page.tsx (1)
47-47: Cross-page import for ManageColumnsToolbar is a coupling smellImporting ManageColumnsToolbar from an applications-specific path makes TasksPage depend on that page’s internal component. Suggest promoting ManageColumnsToolbar to a shared components module and importing it from there to avoid cross-page coupling.
client/src/app/pages/source-platforms/discover-import-wizard/useWizardReducer.ts (1)
54-66: Stabilize handler identities to prevent unnecessary rendersReturn functions are recreated on each render. Wrap them in useCallback and return them via useMemo to reduce rerenders in consumers.
- return { - state, - setFilters(filters: FilterState) { - dispatch({ type: "SET_FILTERS", payload: filters }); - }, - setResults(results: ResultsData | null) { - dispatch({ type: "SET_RESULTS", payload: results }); - }, - reset() { - dispatch({ type: "RESET" }); - }, - }; + const setFilters = React.useCallback((filters: FilterState) => { + dispatch({ type: "SET_FILTERS", payload: filters }); + }, []); + const setResults = React.useCallback((results: ResultsData | null) => { + dispatch({ type: "SET_RESULTS", payload: results }); + }, []); + const reset = React.useCallback(() => { + dispatch({ type: "RESET" }); + }, []); + + return React.useMemo( + () => ({ state, setFilters, setResults, reset }), + [state, setFilters, setResults, reset] + );client/src/app/pages/source-platforms/discover-import-wizard/useStartPlatformApplicationImport.ts (1)
38-51: Error path returns useful context; consider surfacing error detailsThe failure object includes a generic message and the original payload. Consider capturing a safe summary of the error (e.g., response status/text) to aid troubleshooting without exposing sensitive details. No blocking issue.
client/src/app/pages/source-platforms/source-platforms.tsx (1)
195-199: Consider adding null check for defensive programming.While the function is only called with valid platform objects from the table actions, adding a defensive check would make the code more robust.
Apply this diff to add defensive programming:
const discoverImport = (platform: SourcePlatform) => { - if (platform) { - setPlatformToDiscoverImport(platform); - } + setPlatformToDiscoverImport(platform); };The null check is redundant since TypeScript ensures
platformis always aSourcePlatformobject when this function is called.client/src/app/pages/source-platforms/discover-import-wizard/results.tsx (2)
51-51: Address the TODO comment for results view.The TODO comment indicates the results view needs review. Consider creating a follow-up issue to track this work.
Would you like me to create an issue to track the review and potential improvements for the results view UI?
135-142: Consider extracting inline styles to CSS classes.The inline styles for font size and spacing could be moved to CSS classes for better maintainability.
Consider creating CSS classes for these styles to improve maintainability and consistency:
-<div style={{ marginTop: "var(--pf-v5-global--spacer--sm)" }}> - <pre style={{ fontSize: "var(--pf-v5-global--FontSize--sm)" }}> +<div className="error-details-container"> + <pre className="error-details-text"> {result.cause.message} </pre> </div>client/src/app/pages/source-platforms/discover-import-wizard/filter-input.tsx (2)
63-77: Consider potential race condition in useEffect.The effect depends on
resetandgetValuesfunctions which could lead to unnecessary re-renders. Consider using a more targeted dependency array.Consider optimizing the dependency array to prevent unnecessary effect runs:
React.useEffect(() => { if (filtersSchema) { reset({ - ...getValues(), schema: filtersSchema, document: {}, }); } else { reset({ - ...getValues(), schema: undefined, document: undefined, }); } -}, [filtersSchema, reset, getValues]); +}, [filtersSchema, reset]);Since you're resetting all form values, there's no need to spread
getValues().
82-82: Good practice noting the TODO for loading state.The TODO comments indicate awareness that loading state should be shown. This is important for user experience.
Would you like me to help implement the loading state UI while the filter schema is being fetched? This would improve the user experience by providing visual feedback during the async operation.
Also applies to: 107-107
client/src/app/pages/source-platforms/useFetchPlatformsWithTasks.ts (4)
7-9: Consider adding JSDoc for theTasksGroupedByKindinterface.For consistency with the documented exports below, consider adding a brief JSDoc comment explaining the purpose of this interface.
+/** + * Represents tasks grouped by their kind/addon identifier. + */ export interface TasksGroupedByKind { [key: string]: TaskDashboard[]; }
64-80: Consider simplifying the nested ternary structure for readability.The deeply nested ternary operators make the logic harder to follow. Consider using an if-else chain or early returns for better readability.
const choosePlatformTaskStatus = ({ tasks, }: SourcePlatformWithTasks): SourcePlatformTasksStatus => { - return !tasks.exist - ? "None" - : tasks.latestHasRunning - ? "Running" - : tasks.latestHasQueued - ? "Queued" - : tasks.latestHasFailed - ? "Failed" - : tasks.latestHasCanceled - ? "Canceled" - : tasks.latestHasSuccessWithErrors - ? "SuccessWithErrors" - : "Success"; + if (!tasks.exist) return "None"; + if (tasks.latestHasRunning) return "Running"; + if (tasks.latestHasQueued) return "Queued"; + if (tasks.latestHasFailed) return "Failed"; + if (tasks.latestHasCanceled) return "Canceled"; + if (tasks.latestHasSuccessWithErrors) return "SuccessWithErrors"; + return "Success"; };
92-127: Consider optimizing the task state checks.The current implementation iterates through the
latestarray six times (once for each task state check). Consider combining these into a single iteration for better performance.const decorated = platforms.map((platform: SourcePlatform) => { const tasksByKind = tasksByIdByKind[platform.id] ?? []; const latest = listify(tasksByKind, (_kind, tasks) => tasks[0]); + // Single iteration to check all states + const stateFlags = { + canceled: false, + failed: false, + queued: false, + running: false, + success: false, + successWithErrors: false, + }; + + latest.forEach((task) => { + const state = task.state ?? ""; + if (TaskStates.Canceled.includes(state)) stateFlags.canceled = true; + if (TaskStates.Failed.includes(state)) stateFlags.failed = true; + if (TaskStates.Queued.includes(state)) stateFlags.queued = true; + if (TaskStates.Running.includes(state)) stateFlags.running = true; + if (TaskStates.Success.includes(state)) stateFlags.success = true; + if (TaskStates.SuccessWithErrors.includes(state)) stateFlags.successWithErrors = true; + }); + const da: SourcePlatformWithTasks = { ...platform, tasks: { tasksByKind, tasksStatus: "None", exist: (tasksById[platform.id]?.length ?? 0) > 0, - - latestHasCanceled: latest.some((task) => - TaskStates.Canceled.includes(task.state ?? "") - ), - latestHasFailed: latest.some((task) => - TaskStates.Failed.includes(task.state ?? "") - ), - latestHasQueued: latest.some((task) => - TaskStates.Queued.includes(task.state ?? "") - ), - latestHasRunning: latest.some((task) => - TaskStates.Running.includes(task.state ?? "") - ), - latestHasSuccess: latest.some((task) => - TaskStates.Success.includes(task.state ?? "") - ), - latestHasSuccessWithErrors: latest.some((task) => - TaskStates.SuccessWithErrors.includes(task.state ?? "") - ), + latestHasCanceled: stateFlags.canceled, + latestHasFailed: stateFlags.failed, + latestHasQueued: stateFlags.queued, + latestHasRunning: stateFlags.running, + latestHasSuccess: stateFlags.success, + latestHasSuccessWithErrors: stateFlags.successWithErrors, }, };
141-144: Consider uncommenting theplatformNamesmemo if it's needed.The commented code suggests a potential optimization that might be needed. If this is for future use, consider adding a TODO comment explaining when it should be uncommented.
- // const platformNames = useMemo( - // () => platforms.map((platform) => platform.name).sort(universalComparator), - // [platforms] - // ); + // TODO: Uncomment when platform name sorting is needed for UI components + // const platformNames = useMemo( + // () => platforms.map((platform) => platform.name).sort(universalComparator), + // [platforms] + // );client/src/app/pages/source-platforms/discover-import-wizard/discover-import-wizard.tsx (1)
144-148: Consider extracting the button text logic for clarity.The conditional logic for the next button text could be extracted for better readability.
+ const getNextButtonText = () => { + if (results) return t("actions.close"); + return t("actions.discoverApplications"); + }; + <WizardStep id="review" name={t("platformDiscoverWizard.review.stepTitle")} footer={{ - nextButtonText: results - ? t("actions.close") - : t("actions.discoverApplications"), + nextButtonText: getNextButtonText(), onNext: results ? handleCancel : onSubmitTask, isNextDisabled: !state.isReady && !results, backButtonText: t("actions.back"), isBackDisabled: !!results, isCancelHidden: !!results, }} >client/src/app/queries/tasks.ts (2)
140-163: Type casting may hide type safety issues.The explicit type cast on line 163 (
as HubPaginatedResult<TaskType>) could hide potential type mismatches. Consider letting TypeScript infer the type or ensure the data structure matches.return { - result: { + result: data ? { data: data?.data, - total: data?.total ?? 0, - params: data?.params ?? params, - } as HubPaginatedResult<TaskType>, + total: data.total, + params: data.params, + } : { + data: undefined, + total: 0, + params, + } satisfies HubPaginatedResult<TaskType>, isFetching: isLoading, fetchError: error, refetch, };
334-335: Consider using a more specific type thanobjectfor Task data.Using
Task<object>is very broad. Consider usingTask<unknown>or a more specific type if the data structure is known.- select: (task: Task<object> | null) => + select: (task: Task<unknown> | null) => !task ? null : calculateSyntheticTaskState(task),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
client/public/locales/en/translation.json(2 hunks)client/src/app/api/models.ts(7 hunks)client/src/app/api/rest.ts(1 hunks)client/src/app/api/rest/schemas.ts(1 hunks)client/src/app/api/rest/tasks.ts(1 hunks)client/src/app/components/schema-defined-fields/SchemaAsCodeEditor.tsx(3 hunks)client/src/app/components/schema-defined-fields/SchemaDefinedFields.tsx(2 hunks)client/src/app/components/task-manager/TaskManagerDrawer.tsx(3 hunks)client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx(2 hunks)client/src/app/pages/applications/applications-table/applications-table.tsx(2 hunks)client/src/app/pages/applications/retrieve-config-wizard/results.tsx(1 hunks)client/src/app/pages/applications/retrieve-config-wizard/retrieve-config-wizard.tsx(3 hunks)client/src/app/pages/applications/useDecoratedApplications.ts(3 hunks)client/src/app/pages/source-platforms/components/column-platform-name.css(1 hunks)client/src/app/pages/source-platforms/components/column-platform-name.tsx(1 hunks)client/src/app/pages/source-platforms/discover-import-wizard/discover-import-wizard.tsx(1 hunks)client/src/app/pages/source-platforms/discover-import-wizard/filter-input.tsx(1 hunks)client/src/app/pages/source-platforms/discover-import-wizard/results.tsx(1 hunks)client/src/app/pages/source-platforms/discover-import-wizard/review.tsx(1 hunks)client/src/app/pages/source-platforms/discover-import-wizard/useStartPlatformApplicationImport.ts(1 hunks)client/src/app/pages/source-platforms/discover-import-wizard/useWizardReducer.ts(1 hunks)client/src/app/pages/source-platforms/source-platforms.tsx(7 hunks)client/src/app/pages/source-platforms/useFetchPlatformsWithTasks.ts(1 hunks)client/src/app/pages/tasks/TaskActionColumn.tsx(1 hunks)client/src/app/pages/tasks/TaskDetails.tsx(0 hunks)client/src/app/pages/tasks/tasks-page.tsx(4 hunks)client/src/app/pages/tasks/useTaskActions.tsx(3 hunks)client/src/app/queries/schemas.ts(2 hunks)client/src/app/queries/tasks.ts(7 hunks)
💤 Files with no reviewable changes (1)
- client/src/app/pages/tasks/TaskDetails.tsx
🧰 Additional context used
🧬 Code Graph Analysis (22)
client/src/app/pages/tasks/TaskActionColumn.tsx (1)
client/src/app/api/models.ts (1)
Task(335-362)
client/src/app/api/rest/schemas.ts (1)
client/src/app/api/models.ts (1)
TargetedSchema(1039-1042)
client/src/app/pages/source-platforms/discover-import-wizard/results.tsx (2)
client/src/app/pages/applications/retrieve-config-wizard/results.tsx (2)
ResultsData(25-35)Results(41-212)client/src/app/api/models.ts (2)
PlatformApplicationImportTask(376-380)SourcePlatform(950-959)
client/src/app/pages/source-platforms/components/column-platform-name.tsx (6)
client/src/app/pages/source-platforms/useFetchPlatformsWithTasks.ts (2)
SourcePlatformTasksStatus(15-22)SourcePlatformWithTasks(24-37)client/src/app/utils/utils.ts (2)
formatPath(143-155)universalComparator(218-224)client/src/app/Paths.ts (1)
Paths(83-83)client/src/app/api/models.ts (1)
TaskDashboard(383-399)client/src/app/components/Icons/IconWithLabel.tsx (1)
IconWithLabel(5-36)client/src/app/components/Icons/TaskStateIcon.tsx (1)
TaskStateIcon(16-62)
client/src/app/pages/source-platforms/discover-import-wizard/review.tsx (4)
client/src/app/api/models.ts (1)
SourcePlatform(950-959)client/src/app/pages/source-platforms/discover-import-wizard/filter-input.tsx (1)
FilterState(20-25)client/src/app/components/EmptyTextMessage.tsx (1)
EmptyTextMessage(9-19)client/src/app/components/schema-defined-fields/SchemaDefinedFields.tsx (1)
SchemaDefinedField(20-85)
client/src/app/pages/applications/retrieve-config-wizard/results.tsx (2)
client/src/app/pages/source-platforms/discover-import-wizard/results.tsx (1)
ResultsData(19-29)client/src/app/api/models.ts (1)
ApplicationManifestTask(370-374)
client/src/app/pages/source-platforms/source-platforms.tsx (6)
client/src/app/components/NotificationsContext.tsx (1)
NotificationsContext(31-33)client/src/app/api/models.ts (1)
SourcePlatform(950-959)client/src/app/pages/source-platforms/useFetchPlatformsWithTasks.ts (1)
useFetchPlatformsWithTasks(132-153)client/src/app/utils/utils.ts (1)
getAxiosErrorMessage(10-16)client/src/app/pages/source-platforms/components/column-platform-name.tsx (1)
ColumnPlatformName(118-192)client/src/app/pages/source-platforms/discover-import-wizard/discover-import-wizard.tsx (1)
DiscoverImportWizard(20-35)
client/src/app/pages/source-platforms/discover-import-wizard/useWizardReducer.ts (2)
client/src/app/pages/source-platforms/discover-import-wizard/filter-input.tsx (1)
FilterState(20-25)client/src/app/pages/source-platforms/discover-import-wizard/results.tsx (1)
ResultsData(19-29)
client/src/app/pages/source-platforms/discover-import-wizard/discover-import-wizard.tsx (8)
client/src/app/api/models.ts (2)
SourcePlatform(950-959)Review(164-173)client/src/app/components/NotificationsContext.tsx (1)
NotificationsContext(31-33)client/src/app/pages/source-platforms/discover-import-wizard/useStartPlatformApplicationImport.ts (1)
useStartPlatformApplicationImport(9-68)client/src/app/pages/source-platforms/discover-import-wizard/useWizardReducer.ts (4)
useWizardReducer(47-66)reset(62-64)setResults(59-61)setFilters(56-58)client/src/app/api/rest/tasks.ts (1)
submitTask(99-101)client/src/app/pages/source-platforms/discover-import-wizard/filter-input.tsx (1)
FilterInput(27-142)client/src/app/pages/source-platforms/discover-import-wizard/review.tsx (1)
Review(17-93)client/src/app/pages/source-platforms/discover-import-wizard/results.tsx (1)
Results(35-152)
client/src/app/pages/source-platforms/discover-import-wizard/filter-input.tsx (5)
client/src/app/api/models.ts (3)
TargetedSchema(1039-1042)JsonDocument(21-21)SourcePlatform(950-959)client/src/app/components/schema-defined-fields/utils.tsx (1)
jsonSchemaToYupSchema(8-116)client/src/app/queries/schemas.ts (1)
useFetchPlatformDiscoveryFilterSchema(69-88)client/src/app/components/HookFormPFFields/HookFormPFGroupController.tsx (1)
HookFormPFGroupController(42-91)client/src/app/components/schema-defined-fields/SchemaDefinedFields.tsx (1)
SchemaDefinedField(20-85)
client/src/app/components/schema-defined-fields/SchemaDefinedFields.tsx (1)
client/src/app/components/schema-defined-fields/utils.tsx (1)
isComplexSchema(131-153)
client/src/app/pages/source-platforms/useFetchPlatformsWithTasks.ts (3)
client/src/app/api/models.ts (2)
TaskDashboard(383-399)SourcePlatform(950-959)client/src/app/queries/tasks.ts (2)
TaskStates(33-42)useFetchTaskDashboard(75-97)client/src/app/queries/platforms.ts (1)
useFetchPlatforms(17-34)
client/src/app/pages/applications/useDecoratedApplications.ts (2)
client/src/app/api/models.ts (1)
TaskDashboard(383-399)client/src/app/utils/utils.ts (1)
universalComparator(218-224)
client/src/app/pages/tasks/useTaskActions.tsx (1)
client/src/app/api/models.ts (1)
Task(335-362)
client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx (1)
client/src/app/api/models.ts (1)
AnalysisTaskData(433-466)
client/src/app/pages/source-platforms/discover-import-wizard/useStartPlatformApplicationImport.ts (3)
client/src/app/api/rest/tasks.ts (1)
createTask(91-93)client/src/app/queries/tasks.ts (1)
useCreateTaskMutation(244-260)client/src/app/api/models.ts (4)
JsonDocument(21-21)PlatformApplicationImportTask(376-380)SourcePlatform(950-959)New(9-9)
client/src/app/components/task-manager/TaskManagerDrawer.tsx (1)
client/src/app/api/models.ts (1)
Task(335-362)
client/src/app/api/rest/tasks.ts (2)
client/src/app/api/rest.ts (3)
hub(52-62)getHubPaginatedResult(129-141)HEADERS(101-117)client/src/app/api/models.ts (5)
HubRequestParams(35-45)Task(335-362)New(9-9)TaskDashboard(383-399)TaskQueue(483-494)
client/src/app/pages/applications/retrieve-config-wizard/retrieve-config-wizard.tsx (4)
client/src/app/api/rest/tasks.ts (1)
createTask(91-93)client/src/app/queries/tasks.ts (1)
useCreateTaskMutation(244-260)client/src/app/api/models.ts (3)
EmptyObject(19-19)ApplicationManifestTask(370-374)New(9-9)client/src/app/pages/applications/useDecoratedApplications.ts (1)
DecoratedApplication(39-69)
client/src/app/queries/tasks.ts (2)
client/src/app/api/models.ts (3)
Task(335-362)HubPaginatedResult(47-51)New(9-9)client/src/app/api/rest/tasks.ts (3)
updateTask(95-97)createTask(91-93)submitTask(99-101)
client/src/app/queries/schemas.ts (1)
client/src/app/api/rest/schemas.ts (1)
getPlatformDiscoveryFilterSchema(26-35)
client/src/app/pages/tasks/tasks-page.tsx (2)
client/src/app/api/models.ts (1)
Task(335-362)client/src/app/components/EmptyTextMessage.tsx (1)
EmptyTextMessage(9-19)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: unit-test
- GitHub Check: build-and-upload-for-global-ci
🔇 Additional comments (54)
client/src/app/components/schema-defined-fields/SchemaAsCodeEditor.tsx (2)
31-36: LGTM! Well-documented height prop additionThe new
heightprop is properly typed, documented with JSDoc comments, and has a sensible default value. The documentation clearly explains the different usage options including "sizeToFit" and percentage values.
45-45: LGTM! Clean implementation of dynamic heightThe height prop is correctly destructured with its default value and properly passed to the CodeEditor component, enabling flexible sizing for different UI contexts.
Also applies to: 110-110
client/src/app/pages/source-platforms/components/column-platform-name.css (1)
1-11: LGTM! Clean CSS implementation for platform popover stylingThe CSS properly uses PatternFly CSS variables and follows BEM-like naming conventions. The flex-grow properties ensure proper title area expansion within the popover.
client/src/app/components/schema-defined-fields/SchemaDefinedFields.tsx (1)
51-60: LGTM! Good UX decision for complex schemasThe logic correctly hides the JSON/Fields toggle for complex schemas and forces them into JSON view. This prevents users from switching to a fields view that wouldn't properly handle complex structures like arrays.
Also applies to: 63-63
client/public/locales/en/translation.json (2)
37-37: LGTM! Consistent naming for the new actionThe "Discover & Import" action follows the existing naming pattern and is appropriately placed in the actions section.
695-725: LGTM! Comprehensive i18n support for the new wizardThe translation keys for
platformDiscoverWizardare well-structured with:
- Clear hierarchical organization for wizard steps
- Proper use of i18n placeholders for dynamic content
- Consistent pluralization patterns
- Appropriate separation of concerns between UI sections
client/src/app/api/models.ts (3)
335-362: LGTM! Well-structured task type refactoringThe refactoring from default generic parameters to explicit task type interfaces improves type safety and makes the codebase more maintainable. The separation into
AnalysisTask,ApplicationManifestTask, andPlatformApplicationImportTaskprovides clear contracts for different task types.
364-380: Good use of discriminated unions for task typesThe specialized task interfaces use literal
kindvalues effectively, creating discriminated unions that enable TypeScript's type narrowing. This pattern will help prevent runtime errors by ensuring the correct properties are accessed for each task type.
433-466: LGTM! Clear rename improves code clarityRenaming
TaskDatatoAnalysisTaskDatabetter reflects its specific purpose for analysis tasks, making the codebase more self-documenting.client/src/app/api/rest/tasks.ts (2)
12-20: New TASKS module and hub-backed pagination look goodGood split of task APIs into a dedicated module. Strong generics on Task and reuse of getHubPaginatedResult keep the surface consistent.
106-114: Reports endpoints: straightforward and appropriateThin wrappers with data unwrapping are consistent and easy to consume.
client/src/app/api/rest.ts (2)
122-124: Re-exporting tasks and schemas from rest.ts is a good re-organizationThis centralizes the public API without bloating rest.ts, while consumers can import from a single entry.
122-124: Beware circular import edge cases with rest/tasks.tsrest/tasks.ts imports hub/HEADERS/getHubPaginatedResult from "../rest", and rest.ts re-exports "./rest/tasks". This is usually safe because hub/HEADERS are declared before the re-export, but avoid moving this export block above those declarations.
If you want to double-check runtime safety, keep a unit test that imports only the re-exported members from rest.ts and ensures rest/tasks.ts functions are defined. No code change required.
client/src/app/pages/tasks/TaskActionColumn.tsx (1)
7-10: Type alignment to Task looks goodMatches the generic Task typing refactor and keeps the component payload-agnostic.
client/src/app/pages/tasks/useTaskActions.tsx (1)
65-116: Type refinement to Task is consistent with the models refactorNo behavior change; action enablement and routing logic remains intact.
client/src/app/api/rest/schemas.ts (2)
26-36: Singular filter schema endpoint/name change looks correctEndpoint subject switched to "filter" and function renamed to getPlatformDiscoveryFilterSchema, aligning with the rest of the PR.
26-36: All references updated to singular “Filter” schemaSearch results show no lingering uses of the old
getPlatformDiscoveryFiltersSchemaor related plural constants. Only the new singular function and query key appear:
- client/src/app/api/rest/schemas.ts – defines
getPlatformDiscoveryFilterSchema- client/src/app/queries/schemas.ts – imports and uses
getPlatformDiscoveryFilterSchemaandPLATFORM_DISCOVERY_FILTER_SCHEMA_QUERY_KEYNo further changes needed.
client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx (2)
19-22: Type narrowing to AnalysisTaskData is correctImporting and using AnalysisTaskData aligns with the refactor in @app/api/models and improves type safety for wizard data.
54-73: Default task data matches AnalysisTaskData shapeThe defaultTaskData object conforms to the new AnalysisTaskData interface (tagger, verbosity, mode, targets, sources, scope). Good baseline.
client/src/app/pages/applications/retrieve-config-wizard/results.tsx (1)
22-29: Typing update to ApplicationManifestTask looks correctSwitching success.task to ApplicationManifestTask is coherent with the task model refactor; access to task.id remains valid.
client/src/app/pages/applications/applications-table/applications-table.tsx (2)
337-339: Hook return shape update is respecteduseDecoratedApplications now returns referencedPlatformRefs and it’s correctly consumed here.
499-513: New “source platforms” filter is well-integrated
- Uses OR semantics consistent with other multi-select filters.
- Options are derived from referencedPlatformRefs and will be pre-sorted in the hook.
- getItemValue correctly reads app.platform?.name.
client/src/app/pages/applications/useDecoratedApplications.ts (1)
244-251: Adding referencedPlatformRefs is a solid extensionUnique-by-id:name and sorted by name matches other referencedXxxRefs patterns. This keeps the ApplicationsTable filter options consistent.
client/src/app/pages/source-platforms/components/column-platform-name.tsx (1)
159-161: Dayjs plugins forfromNow()and localized formats are already configuredBoth
relativeTimeandlocalizedFormatplugins are globally extended inclient/src/app/dayjs.ts(lines 13–14), sostartTime.fromNow()andstartTime.format("ll, LTS")will render correctly.client/src/app/components/task-manager/TaskManagerDrawer.tsx (1)
52-58: Type tightening and underscore handle: good update to TaskUsing optional applicationName/platformName and typing the internal task as Task aligns with the new generic Task model. No functional issues here.
client/src/app/pages/source-platforms/discover-import-wizard/review.tsx (1)
80-86: Read-only schema rendering: LGTMUsing SchemaDefinedField in read-only mode provides a consistent review experience and avoids drift with the editing UI. Nicely done.
client/src/app/pages/tasks/tasks-page.tsx (3)
38-41: New UI helpers and fallbacks: LGTMImporting IconWithLabel, TaskStateIcon, NoDataEmptyState, and EmptyTextMessage matches the new cell rendering strategy. No issues spotted.
232-235: Cell typing and fallback: make sure consumers expect ReactNodeChanging the application cell to render a ReactNode fallback is fine since cells are already typed as ReactNode. Ensure any code consuming exported CSV/clipboard or tests expecting a string is updated accordingly.
If you have CSV export or test utilities that read raw cell values, double-check they don’t assume application is a string.
319-321: Generic Task adoption: LGTMMapping rows as [Task, Record<string, ReactNode>] aligns with the generic Task model and avoids unnecessary type assertions.
client/src/app/pages/source-platforms/discover-import-wizard/useWizardReducer.ts (2)
11-18: Sane initial state defaultsDefaults make sense for a two-step wizard: filters required/invalid, not ready, no results. Looks good.
26-29: Deriving isReady from filters.isValid is simple and sufficientKeeping isReady derived ensures consistent source-of-truth. Good practice.
client/src/app/pages/source-platforms/discover-import-wizard/useStartPlatformApplicationImport.ts (2)
53-68: API ergonomics: arrays for single submit are fine for compositionReturning arrays makes it easy to aggregate over multiple platforms. Hook looks solid and composes well with the wizard flow.
30-36: Confirmstatefield in task creationThe
newTaskpayload here (lines 30–36) explicitly setsstate: "Ready", which mirrors our other task creators (e.g. inretrieve-config-wizard.tsx). Before deciding whether to drop or keep this property, please verify the server’s API contract:• In
client/src/app/pages/source-platforms/discover-import-wizard/useStartPlatformApplicationImport.ts(lines 30–36), you setstate: "Ready",• In
client/src/app/pages/applications/retrieve-config-wizard/retrieve-config-wizard.tsx(around lines 216–224), we also includestate: "Ready",Check the
/tasksPOST endpoint (or OpenAPI spec) to confirm:
- Does it default
stateto"Ready"if omitted?- Must clients supply an initial
state?If the server applies a default, you can safely remove
statefrom allnewTaskpayloads; otherwise, continue setting it explicitly.client/src/app/pages/source-platforms/source-platforms.tsx (7)
1-1: LGTM!The useState import is correctly added to support the new state management for the platform discovery import wizard.
42-47: Good consolidation of imports.The imports are properly organized and aligned with the new platform discovery functionality.
56-56: Good modernization of the component export.Using the explicit React.FC type annotation improves type safety and aligns with modern React TypeScript patterns.
70-77: Well-structured modal state management.The
isModalOpencomputed value effectively tracks all modal states to control when platform data should be refetched. This approach prevents unnecessary API calls while modals are open.
94-99: Improved error handling for delete operations.The inline error handler provides better user feedback using
getAxiosErrorMessageto extract meaningful error messages. This is more maintainable than having separate onError callback functions.
279-279: Good component abstraction for platform name display.Replacing the inline Popover and TaskStateIcon with the dedicated
ColumnPlatformNamecomponent improves code organization and reusability.
361-368: Well-integrated wizard component.The Platform Discover Import Wizard is properly integrated with appropriate props and state management. The
onClosecallback correctly resets the state.client/src/app/pages/source-platforms/discover-import-wizard/results.tsx (2)
19-29: Consider using more specific types for task references.The current interface is well-structured, but consider if the task field could be more specific than the full
PlatformApplicationImportTasktype to reduce coupling.
83-102: Good UI structure for success results.The expandable section with horizontal description lists provides a clean and organized way to display successful task submissions.
client/src/app/queries/schemas.ts (2)
8-8: API naming improvements for clarity.The rename from plural to singular (
getPlatformDiscoveryFiltersSchema→getPlatformDiscoveryFilterSchema) better reflects that this fetches a single schema, not multiple schemas. Good improvement for API clarity.Also applies to: 15-16
69-87: Good addition of isSuccess to the return value.Including
isSuccessin the return object provides consumers with a clear way to check if the data has been successfully loaded, which is useful for conditional rendering logic.client/src/app/pages/source-platforms/discover-import-wizard/filter-input.tsx (2)
37-41: Robust conditional validation using Yup.The conditional validation logic correctly applies the JSON schema validation only when a schema is present, falling back to a nullable object otherwise. This provides good flexibility for optional filters.
127-135: Well-integrated schema-driven field rendering.The integration with
SchemaDefinedFieldcomponent provides a flexible way to render form fields based on the JSON schema, with proper change handling.client/src/app/pages/applications/retrieve-config-wizard/retrieve-config-wizard.tsx (4)
17-17: Good type system improvements.The migration from
ApplicationTask<EmptyTaskData>toApplicationManifestTaskprovides better type specificity and aligns with the new task model architecture. The use ofEmptyObjectinstead ofEmptyTaskDatais also more semantically clear.
64-64: Hook naming improvement for clarity.The rename from
useFetchApplicationManifesttouseStartFetchApplicationManifestbetter reflects that this hook initiates a task rather than directly fetching data.Also applies to: 189-189
209-215: Good task creation with proper state initialization.The task creation properly includes the new
state: "Ready"anddata: {}fields, aligning with the updated task model structure.
190-193: Type-safe mutation setup.The generic type parameters for
useCreateTaskMutationare properly specified, ensuring type safety throughout the task creation flow.client/src/app/pages/source-platforms/useFetchPlatformsWithTasks.ts (1)
1-5: LGTM! Clean imports and well-organized dependencies.The imports are properly structured with React hook dependencies, utility functions from radash, and necessary types/hooks from the application.
client/src/app/pages/source-platforms/discover-import-wizard/discover-import-wizard.tsx (1)
24-34: Good use of key prop for component remounting.The use of the
keyprop to force remounting whenisOpenchanges ensures a clean state reset between wizard sessions.client/src/app/queries/tasks.ts (2)
52-60: Good generalization with TypeScript generics.The generic implementation correctly preserves type safety while allowing for different task payload types. The default type parameter ensures backward compatibility.
226-242: Excellent simplification of mutation functions.The removal of explicit type parameters in the mutation function calls (lines 232, 250, 268) improves code clarity while TypeScript's inference handles the types correctly.
Also applies to: 244-260, 262-278
client/src/app/pages/source-platforms/components/column-platform-name.tsx
Show resolved
Hide resolved
client/src/app/pages/source-platforms/components/column-platform-name.tsx
Show resolved
Hide resolved
client/src/app/pages/source-platforms/discover-import-wizard/discover-import-wizard.tsx
Show resolved
Hide resolved
Signed-off-by: Scott J Dickerson <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
client/src/app/pages/source-platforms/discover-import-wizard/results.tsx (6)
25-35: Unify array type syntax with codebase styleElsewhere (retrieve-config-wizard) arrays are written with the
[]suffix style. For consistency, switch fromArray<T>toT[].-export interface ResultsData { - success: Array<{ - task: PlatformApplicationImportTask; - platform: SourcePlatform; - }>; - failure: Array<{ - message: string; - cause: Error; - platform: SourcePlatform; - }>; -} +export interface ResultsData { + success: { + task: PlatformApplicationImportTask; + platform: SourcePlatform; + }[]; + failure: { + message: string; + cause: Error; + platform: SourcePlatform; + }[]; +}
103-110: Localize aria-labels for tablesHardcoded English strings won’t be translated. Use i18n keys for accessibility labels.
- <Table aria-label="Successful task submissions"> + <Table aria-label={t("platformDiscoverWizard.results.successTableAriaLabel")}> ... - <Table aria-label="Failed task submissions"> + <Table aria-label={t("platformDiscoverWizard.results.failedTableAriaLabel")}>Follow-up: ensure the new keys exist in the translations.
Also applies to: 146-153
90-96: Replace magic spacing with PF spacer tokensInline
8pxmargins are brittle. Use PF global spacer tokens for consistency with theming.- <Icon - status="success" - isInline - style={{ marginRight: "8px" }} - > + <Icon status="success" isInline style={{ marginRight: "var(--pf-global--spacer--sm)" }}> <CheckCircleIcon /> </Icon>- <Icon status="danger" isInline style={{ marginRight: "8px" }}> + <Icon status="danger" isInline style={{ marginRight: "var(--pf-global--spacer--sm)" }}> <ExclamationTriangleIcon /> </Icon>Also applies to: 137-140
171-175: Use PF tokens instead of hex color and raw font-sizePrefer PF global tokens to maintain theme alignment.
- style={{ - fontSize: "0.8em", - color: "#6a6e73", - marginTop: "4px", - }} + style={{ + fontSize: "var(--pf-global--FontSize--sm)", + color: "var(--pf-global--Color--200)", + marginTop: "var(--pf-global--spacer--xs)", + }}
85-130: Avoid duplication with retrieve-config Results; consider a generic, reusable Results componentThis component is nearly identical to applications/retrieve-config-wizard/results.tsx with only column names and entity types differing. Extract a generic Results panel/table that accepts:
- title/summary and i18n keys,
- entity label column header,
- items for success/failure,
- renderers for entity cell and link/error cell.
This reduces maintenance and ensures consistent UX across wizards.
Also applies to: 132-189
112-124: Row keys: ensure uniqueness if multiple entries per platform are possibleUsing
platform.idas the key is fine if there’s at most one result per platform. If duplicates are possible, switch to a composite/stable key (e.g.,${platform.id}-${task.id}for success and${platform.id}-${index}for failure).Also applies to: 155-181
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
client/public/locales/en/translation.json(2 hunks)client/src/app/api/rest/tasks.ts(1 hunks)client/src/app/components/schema-defined-fields/SchemaDefinedFields.tsx(2 hunks)client/src/app/pages/source-platforms/components/column-platform-name.tsx(1 hunks)client/src/app/pages/source-platforms/discover-import-wizard/results.tsx(1 hunks)client/src/app/pages/source-platforms/discover-import-wizard/useStartPlatformApplicationImport.ts(1 hunks)client/src/app/pages/source-platforms/useFetchPlatformsWithTasks.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- client/src/app/pages/source-platforms/useFetchPlatformsWithTasks.ts
- client/src/app/pages/source-platforms/discover-import-wizard/useStartPlatformApplicationImport.ts
- client/src/app/components/schema-defined-fields/SchemaDefinedFields.tsx
- client/src/app/api/rest/tasks.ts
- client/public/locales/en/translation.json
- client/src/app/pages/source-platforms/components/column-platform-name.tsx
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: sjd78
PR: konveyor/tackle2-ui#2545
File: client/src/app/pages/source-platforms/discover-import-wizard/discover-import-wizard.tsx:59-88
Timestamp: 2025-08-13T08:51:33.637Z
Learning: The submitTask() function in the platform discover import wizard handles error trapping internally and returns results in a structured format with success and failure arrays, rather than throwing exceptions. Additional try/catch wrapping is not needed.
🧬 Code Graph Analysis (1)
client/src/app/pages/source-platforms/discover-import-wizard/results.tsx (2)
client/src/app/pages/applications/retrieve-config-wizard/results.tsx (2)
ResultsData(25-35)Results(41-212)client/src/app/api/models.ts (2)
PlatformApplicationImportTask(376-380)SourcePlatform(950-959)
🔇 Additional comments (2)
client/src/app/pages/source-platforms/discover-import-wizard/results.tsx (2)
61-70: Good use of i18n and concise header compositionTitle and summary leverage translation keys consistently. Clear and consistent with other wizards.
119-121: Verify task route path correctnessI ran searches for any
<Route>definitions or literal/tasks/:idusages in the router configuration and didn’t find a matching detail route. Please double-check that a route exists for viewing a single task at/tasks/:id. If the dynamic segment is named differently (e.g.tasks/:taskId) or uses a nested path, update this<Link>accordingly to prevent broken links.
client/src/app/pages/source-platforms/discover-import-wizard/results.tsx
Show resolved
Hide resolved
Signed-off-by: Scott J Dickerson <[email protected]>
Signed-off-by: Scott J Dickerson <[email protected]>
Signed-off-by: Scott J Dickerson <[email protected]>
rszwajko
left a comment
rszwajko
left a comment
There was a problem hiding this comment.
I had a first look at the code and smoke tested the wizard.
Beside small glitches it looks good. One small comment below.
Functional issues:
- no tasks are visible in the popover (source platform name column) although a new task was just created
- when updating the app the filter values are cleared after toggling JSON mode
| }: ISchemaDefinedFieldProps) => { | ||
| const [isJsonView, setIsJsonView] = React.useState<boolean>(!jsonSchema); | ||
| const [isJsonView, setIsJsonView] = React.useState<boolean>( | ||
| !jsonSchema || isComplexSchema(jsonSchema) |
There was a problem hiding this comment.
this condition is used 3 times below (directly or negated) - it would be nice to capture that with a named variable i.e. forceJsonView
There was a problem hiding this comment.
Yeah there will need to be some more cleanup in this component in the near future.
See #2550
|
|
||
| <PanelMain maxHeight="100%"> | ||
| {isJsonView || !jsonSchema ? ( | ||
| {!jsonSchema || isComplex || isJsonView ? ( |
There was a problem hiding this comment.
forceJsonView || isJsonView ?
There was a problem hiding this comment.
Makes sense. Punting to followup issue. See #2550
HUB PR should fix this: konveyor/tackle2-hub#874
See #2550 |
#2547) Apply the same kind of changes made to hooks and views with the `DiscoverImportWizard` to the older `RetrieveConfigWizard`. This will help keep the structure and layout the same. Needs #2545 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Retrieve Configurations wizard updated with a clearer title, description and improved flow. * Results view refined: status icons, task IDs link directly to task details, and a single “No tasks were submitted” message appears at the top when applicable. * **Style** * Wording revised to “Successful Submission(s)” and translations reorganized for clarity. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Scott J Dickerson <[email protected]>
…onveyor#2545) Resolves: https://issues.redhat.com/browse/MTA-5249 Resolves: konveyor#2289 Adds a wizard to discover and import applications from a source platform: - The wizard captures discover and import filter data from the user - The filters are added to the import task - The set of filters are defined by a schema - The schema is pulled from domain, variant, subject "platform", "cloudfoundry", "filter" - `SchemaDefinedFields` to be used to input the schema data - Task summary data for platforms are displayed in a popover on the source platform table's name column ## Summary by CodeRabbit - **New Features** - Discover & Import wizard for source platforms (filters, review, results and submit flow) and a hook to start platform imports. - Platform name popover showing status and recent tasks with quick links. - **Enhancements** - Platform-aware import flow on Source Platforms page and a new Platforms filter in Applications table. - Schema editor accepts configurable height; complex schemas default to JSON view. - Tasks table shows fallback when application is missing and improved deletion error notifications. - **Localization** - Added "Discover & Import" and full platform discovery wizard translations. --------- Signed-off-by: Scott J Dickerson <[email protected]>
konveyor#2547) Apply the same kind of changes made to hooks and views with the `DiscoverImportWizard` to the older `RetrieveConfigWizard`. This will help keep the structure and layout the same. Needs konveyor#2545 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Retrieve Configurations wizard updated with a clearer title, description and improved flow. * Results view refined: status icons, task IDs link directly to task details, and a single “No tasks were submitted” message appears at the top when applicable. * **Style** * Wording revised to “Successful Submission(s)” and translations reorganized for clarity. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Scott J Dickerson <[email protected]>
3 participants
Resolves: https://issues.redhat.com/browse/MTA-5249
Resolves: #2289
Adds a wizard to discover and import applications from a source platform:
SchemaDefinedFieldsto be used to input the schema dataSummary by CodeRabbit
New Features
Enhancements
Localization